[U-Boot] [PATCH v3 1/3] ARM: bcm283x: Implement EFI RTS reset_system

[Stephen Warren]

On 11/06/2016 03:24 AM, Alexander Graf wrote:
>
>
> On 05/11/2016 23:01, Stephen Warren wrote:
>> On 11/02/2016 03:36 AM, Alexander Graf wrote:
>>> The rpi has a pretty simple way of resetting the whole system. All it
>>> takes
>>> is to poke a few registers at a well defined location in MMIO space.
>>>
>>> This patch adds support for the EFI loader implementation to allow an
>>> OS to
>>> reset and power off the system when we're outside of boot time.
>>
>> (As an aside, I'm not sure why someone wanting EFI wouldn't just use a
>> complete EFI implementation such as TianoCore.)
>>
>>> diff --git a/arch/arm/mach-bcm283x/reset.c
>>> b/arch/arm/mach-bcm283x/reset.c
>>
>>> +__efi_runtime_data struct bcm2835_wdog_regs *wdog_regs =
>>> +    (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR;
>>> +
>>> +void __efi_runtime reset_cpu(ulong addr)
>>>  {
>>> -    struct bcm2835_wdog_regs *regs =
>>> -        (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR;
>>
>> I'm not sure why that change is required. The value of the variable is
>> the same in both cases?
>
> Take a look a few lines down in the patch:
>
>> +void efi_reset_system_init(void)
>> +{
>> +    efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs));
>> +}
>
> What this does is register a *pointer* as run time service pointer. What
> does that mean?
>
> When we enter RTS, Linux can map any region in the EFI memory map into a
> different place in its own virtual memory map. So any pointers we use
> inside RTS have to be relocated to the new locations.
>
> For normal relocations, we move the relocations from linker time to run
> time, so that we can relocate ourselves when Linux does the switch-over
> to a new address space.
>
> However, for MMIO that's trickier. That's where the
> efi_add_runtime_mmio() function comes into play. It takes care of adding
> the page around the references address to the EFI memory map as RTS MMIO
> and relocates the pointer when Linux switches us into the new address
> space.
>
> Does that explain why we need to move from an inline address to an
> address stored in a memory location?

So EFI RTS runs in the same exception level as the rich OS, and not in 
EL3? I would have expected EFI to run in EL3 with a completely separate 
MMU configuration. If that's not the case, then this part of the patch 
does make sense.

>> Perhaps it's trying to ensure that if this gets compiled into an ldr
>> instruction, the referenced data value is in a linker section that's
>> still around when EFI runs? If so fine, but how is that ensured for all
>> the other constants that this code uses, and if that happens
>> automatically due to the __efi_runtime marker above, why doesn't it work
>> for this one constant?
>>
>> Does U-Boot have a halt/poweroff/shutdown shell command? If so, it might
>> be nice to enable it as part of this series, since the code to perform
>> that operation is now present.
>
> That's what I originally wanted, yes :). Unfortunately due to the
> relocation explained above, it's basically impossible for any reset
> function that calls into MMIO space.
>
> However, we do have it now for PSCI. If you have a PSCI enabled system,
> we don't need to call into MMIO space and thus make the common reset
> function available as RTS.

Can't the same U-Boot function be called both (a) during U-Boot runtime, 
where wdog_regs are pre-initialized to match U-Boot's MMU configuration, 
and (b) once the OS has booted, where wdog_regs has been modified 
according to the new memory map?

If not, one could implement a reset/powerdown/... function that takes 
the MMIO virtual address as a pointer, and then separate trivial 
wrappers that pass in either the static/U-Boot MMIO address, or the 
value of the EFI runtime variable that points at the MMIO mapping.