Hi Alex,

On 11 August 2016 at 05:49, Alexander Graf <ag...@suse.de> wrote:
>
>
> On 08.08.16 23:44, Simon Glass wrote:
>> Hi,
>>
>> On 7 August 2016 at 10:59, Andreas Färber <afaer...@suse.de> wrote:
>>> Am 14.07.2016 um 08:18 schrieb Alexander Graf:
>>>>> Am 14.07.2016 um 06:48 schrieb Andreas Färber <afaer...@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.
>

At present you only have reset, and you've only implemented it for one
board. Are there other calls that we need to implement? This
EFI_RUNTIME is transitive - anything it calls must be in the runtime.
Does the linker prevent us from screwing up?

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

What system can I get that uses that today?

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

I'd prefer to see the EFI requests be processed as they are received,
rather than with pre-canned data. Your original justification (e.g.
for efi_disk_add_dev()) was that there was not authoritative list of
block devices in U-Boot. But there is now.

So do you think it would be feasible to drop these tables, and efi_obj_list?

What happens with the tables if I run an app and then come back to U-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 ;).

Sounds good.

>
>
> Alex

Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to