[U-Boot] [PATCH 0/2] efi_loader: Implement reset on RPi

[Alexander Graf]

On 08.08.16 23:44, Simon Glass wrote:
> Hi,
> 
> On 7 August 2016 at 10:59, Andreas Färber <afaerber at suse.de> wrote:
>> Am 14.07.2016 um 08:18 schrieb Alexander Graf:
>>>> Am 14.07.2016 um 06:48 schrieb Andreas Färber <afaerber at suse.de>:
>>>>
>>>> Hi Alex,
>>>>
>>>>> Am 05.06.2016 um 23:17 schrieb Alexander Graf:
>>>>> If Linux finds an EFI implementation it always uses the EFI reset handler to
>>>>> reboot or power down the system.
>>>>
>>>> Hm, I thought my powerdown issues on the Jetson TK1 were for lack of
>>>> CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
>>>
>>> It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
>>
>> We've confirmed in the meantime that the Jetson TK1 issues were
>> unrelated to EFI and could be worked around by enabling some as3722
>> kernel option.
>>
>>>>> Unfortunately we haven't implemented that one yet. In fact, while we prepared
>>>>> for RTS handling, we never actually implemented a single user.
>>>>>
>>>>> This is going to change today. This simple patch set enables RTS reset support
>>>>> for the RPi systems, allowing you to reboot and shut down the rpi if booted
>>>>> via EFI.
>>>>>
>>>>> It also lays the groundwork to show how to implement that functionality at all,
>>>>> so I expect more boards to follow.
>>>>>
>>>>> Alexander Graf (2):
>>>>>  efi_loader: Allow boards to implement get_time and reset_system
>>>>>  ARM: bcm283x: Implement EFI RTS reset_system
>>>>>
>>>>> arch/arm/mach-bcm283x/include/mach/wdog.h |   2 +-
>>>>> arch/arm/mach-bcm283x/reset.c             |  59 +++++++++++++++--
>>>>> cmd/bootefi.c                             |   4 ++
>>>>> include/efi_loader.h                      |  18 ++++++
>>>>> lib/efi_loader/efi_runtime.c              | 101 ++++++++++++++++++++++++++----
>>>>> 5 files changed, 166 insertions(+), 18 deletions(-)
>>>>
>>>> This all looks very non-generic and would mean that every board will
>>>> need to be patched individually, which is unrealistic to be tested by
>>>> just the two of us.
>>>>
>>>> Can't you patch the reset_cpu() declaration (common.h/sysreset.h)
>>>> instead of all its implementations? We might still need to patch
>>>> individual implementations but I don't see why any reset_cpu()
>>>> implementation should be in a different section than others.
>>>
>>> Hmm. There are 2 minor problems:
>>>
>>>   1) Efi also supports power off on top of reset
>>>   2) We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
>>
>> I don't see why we would need to convert everything at once either way.
>>
>>>
>>> And one major issue:
>>>
>>> All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
> 
> I'm worried about this. It means that any code used from this run-time
> needs to be so marked. This could be large tracts of U-Boot. In

We only need to mark the few bits that are actually executed and used
within RTS. That's usually just 2 or 3 functions.

Also, moving forward, we'll see more and more systems implement PSCI
which means we can implement a generic PSCI RTS for reset/shutdown.

> particular, as I have mentioned a few times, I think the UEFI tables
> should be 'live' and not just created before booting, which means that
> much of driver/core needs to be in the UEFI section.

I don't fully understand what you're aiming for here. The tables are
always static in a uEFI world. I don't see how they could get more
"live" than creating them right before boot.

> Should we just adjust it so that the whole of U-Boot is in there? How
> big is the UEFI run-time normally?

It's really not a problem of putting things in one section or another.
It's that if anything within a run time .text section tries to access
any memory, be it MMIO or .data memory, it will need to get relocated if
we want to be able to run it as RTS.

But really, the RTS interface is very very very slim. Take a look at
lib/efi_loader/efi_runtime.c. The only thing that could remotely pull in
more code is RTC/variable support. But I'm sure we can find a way to
solve that without pulling in all internal abstraction layers.

> 
>>>
>>> That mesns we could either make a generic, broken version. Or we just convert one by one for systems that we can verify it on :). I hope that I designed the APIs easily enough that people who are not us enable RTS support on other platforms too :)
>>
>>
>> Ping! Anyone any comments on the two open questions of uppercase vs.
>> lowercase and placement of attribute?
> 
> I prefer lower case :-)

I agree that lower case makes it look less aggressive, but should that
hold up this patch?

How about we shove everything in that we have and then I can do a
full-tree replacement of EFI_RUNTIME with __efi_runtime. I'm not a big
fan of having cosmetics hold up functional changes ;).


Alex