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

[Alexander Graf]

On 07/11/2016 22:26, Stephen Warren wrote:
> 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.

Right, it runs in EL2/EL1 with a virtual memory layout that is provided 
by the OS.

>
>>> 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?

That's exactly what this patch does, no?

> 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.

You could, but because the runtime version would still have to rely on 
an external variable because it doesn't know where it'll end up at 
runtime, you may as well use an external variable throughout and arrive 
back at this patch ;).


Alex