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

Tue Jan 29 00:49:18 UTC 2019

Hi Alex,

On Mon, 28 Jan 2019 at 13:08, Alexander Graf <agraf at suse.de> wrote:
>
>
>
> On 28.01.19 20:47, Simon Glass wrote:
> > Hi Alex,
> >
> > On Mon, 28 Jan 2019 at 12:39, Alexander Graf <agraf at suse.de> wrote:
> >>
> >>
> >>
> >> 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.
> >
> > Yes that's right. Although I suspect it is only about half that. E.g.
> > Raspberry Pi 2 is 431KB. I'd hope that RTS would be very small?
> >
> >>
> >>
> >> 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.
> >
> > Yes, but at some point this all becomes unmanageable. I think we
> > should adjust the approach now.
>
> So how would we transfer device information from one piece to the next?
> How would we ensure that we don't reinitialize everything?

Well if we only need a few devices (sysreset, variables, SPI and SPI
flash) then it doesn't really matter.

>
> Adding another runtime payload really doesn't sound terribly appealing
> to me. It just adds even more complexity to a problem that shouldn't
> exist in an ideal world.

Is there any way to avoid the run-time stuff completely? If not, then
I suppose we have to address it.

>
> >
> >>
> >>
> >>>>> 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.
> >
> > Yes. In fact EFI_LOADER adds about 50KB on the same rpi_2. This is a
> > fact of life with EFI. I really don't think EFI is optimised for code
> > size.
>
> It's not, but 50kb is already on the verge. If we add 20kb more, people
> will start to disable EFI support for their boards. And that's exactly
> contrary to what we want.
>
> > I think we need a little build like SPL which pulls in just what is
> > needed, but does use DM.
>
> Again, I don't think that will work very well. Transferring state from
> one to the other is going to be tricky. Sure - you could reinitialize -
> but then the boot process will be even slower.

What state? We do have bloblist now for 'automatic' state transfer.
But what state?

>
> So if we need to play tricks, maybe we're better off adding some Kconfig
> magic that indicates to the compiler that .text is now called
> .text.runtime. which then automatically pulls everything from specific
> files into runtime.
>
> We could apply such logic to all DM files and everything we need for
> specific hardware at runtime. Then we can add a runtime flag to devices
> and limit device enumeration to only runtime enabled ones after the
> switch-over.

That might work. I suppose we need a test which zeroes out all the
non-runtime code and then calls all the runtime functions in 'fake'
mode?

Regards,
Simon