[U-Boot] [PATCH v2 1/2] x86: Add efi runtime reset

[Alexander Graf]

On 28.01.19 20:36, Simon Glass wrote:
> Hi Alex,
> 
> On Mon, 28 Jan 2019 at 12:34, Alexander Graf <agraf at suse.de> wrote:
>>
>>
>>
>> On 28.01.19 20:31, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On Mon, 28 Jan 2019 at 12:27, Alexander Graf <agraf at suse.de> wrote:
>>>>
>>>>
>>>>
>>>> On 28.01.19 20:24, Simon Glass wrote:
>>>>> Hi Alex,
>>>>>
>>>>> On Mon, 28 Jan 2019 at 12:15, Alexander Graf <agraf at suse.de> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 28.01.19 20:13, Simon Glass wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Mon, 28 Jan 2019 at 08:42, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>>
>>>>>>>> Our selftest will soon test the actual runtime reset function rather than
>>>>>>>> the boot time one. For this, we need to ensure that the runtime version
>>>>>>>> actually succeeds on x86 to keep our travis tests work.
>>>>>>>>
>>>>>>>> So this patch implements an x86 runtime reset function. It is missing
>>>>>>>> shutdown functionality today, but OSs usually implement that via ACPI
>>>>>>>> and this function does more than the stub from before, so it's at least
>>>>>>>> an improvement.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>>>>>>> ---
>>>>>>>>  drivers/sysreset/sysreset_x86.c | 23 +++++++++++++++++++++++
>>>>>>>>  1 file changed, 23 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/sysreset/sysreset_x86.c b/drivers/sysreset/sysreset_x86.c
>>>>>>>> index 20b958cfd4..efed45ccb7 100644
>>>>>>>> --- a/drivers/sysreset/sysreset_x86.c
>>>>>>>> +++ b/drivers/sysreset/sysreset_x86.c
>>>>>>>> @@ -10,6 +10,29 @@
>>>>>>>>  #include <sysreset.h>
>>>>>>>>  #include <asm/io.h>
>>>>>>>>  #include <asm/processor.h>
>>>>>>>> +#include <efi_loader.h>
>>>>>>>> +
>>>>>>>> +#ifdef CONFIG_EFI_LOADER
>>>>>>>> +void __efi_runtime EFIAPI efi_reset_system(
>>>>>>>> +                       enum efi_reset_type reset_type,
>>>>>>>> +                       efi_status_t reset_status,
>>>>>>>> +                       unsigned long data_size, void *reset_data)
>>>>>>>> +{
>>>>>>>> +       u32 value = 0;
>>>>>>>> +
>>>>>>>> +       if (reset_type == EFI_RESET_COLD)
>>>>>>>> +               value = SYS_RST | RST_CPU | FULL_RST;
>>>>>>>> +       else if (reset_type == EFI_RESET_WARM)
>>>>>>>> +               value = SYS_RST | RST_CPU;
>>>>>>>
>>>>>>> The EFI should use the sysreset driver and sysreset_walk() or similar,
>>>>>>> rather than having a function called directly.
>>>>>>
>>>>>> It can't. At this point all of DM is long gone. We're in runtime space here.
>>>>>
>>>>> This has come up before. We'll end up with a lot of duplication if we
>>>>> cannot solve this. I think the run-time code will need to be built and
>>>>> linked separately so that all necessary code is pulled in.
>>>>
>>>> It's mostly a question of size. We can even transform all of U-Boot into
>>>> huge runtime services if we keep the relocation tables around and apply
>>>> relocations manually for everything.
>>>>
>>>> The problem is that if we do that, we'll become even bigger than we are
>>>> now, no?
>>>
>>> I did a change to build U-Boot as a library, perhaps it could build on
>>> that. The 'run-time U-Boot' won't be any bigger than the code that is
>>> actually used. Also I don't think memory usage is a concern in systems
>>> that use UEFI :-)
>>
>> It is, we're already exploding some constraints today. Furthermore, by
>> moving all of U-Boot into RTS you obviously waste a few MB of RAM while
>> Linux is up. And it's much harder to review that the code is only doing
>> what you want it to do.
> 
> My suggest is not to move all of U-Boot into RTS. It is to build an
> RTS version of U-Boot with just those things that are needed, a bit
> like TPL or SPL.

Yes, but the "loaded binary" to boot the system is then BTS + RTS. So if
U-Boot is 1MB and the RTS U-Boot build is 0.5MB, you need to store and
load 1.5MB from SD card or SPI or wherever your U-Boot is stored.

By morphing all of U-Boot over, the runtime cost is higher (1MB rather
than 0.5MB used while Linux is up), but the boot time cost is smaller
(only 1MB on storage).

> I did not invent the run-time aspect of EFI, but I feel we are trying
> to support it with a hack.

Well, it is a hack in edk2 as well. RTS are a terrible idea in my book.
But they're there and we need to support at least reset/shutdown and
*maybe* some sort of runtime variables.

>>> The problem with the current approach is that everything becomes
>>> parallel to DM, duplicating existing code, and that is complicated
>>> too.
>>
>> No, not everything. Only reset/shutdown. Well, and maybe variable
>> services. Which may use SPI. Meh.
> 
> That should be a fairly small build, then.

It still adds to the binary size.


Alex