[PATCH v5 18/23] efi_loader: Don't try to add sandbox runtime code

[Ilias Apalodimas]

On Wed, 11 Dec 2024 at 17:54, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Wed, 11 Dec 2024 at 08:19, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Wed, 11 Dec 2024 at 15:54, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > This cannot work since the code is not present in the emulated memory.
> > > In any case, sandbox cannot make use of the runtime code.
> > >
> > > For now, just drop it from sandbox. We can always adjust things to copy
> > > it into memory, if needed.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > (no changes since v1)
> > >
> > >  lib/efi_loader/efi_memory.c | 26 ++++++++++++++++----------
> > >  1 file changed, 16 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > > index 0f149f99c7d..6475b94f951 100644
> > > --- a/lib/efi_loader/efi_memory.c
> > > +++ b/lib/efi_loader/efi_memory.c
> > > @@ -767,16 +767,22 @@ static void add_u_boot_and_runtime(void)
> > >         runtime_mask = SZ_64K - 1;
> > >  #endif
> > >
> > > -       /*
> > > -        * Add Runtime Services. We mark surrounding boottime code as runtime as
> > > -        * well to fulfill the runtime alignment constraints but avoid padding.
> > > -        */
> > > -       runtime_start = (uintptr_t)__efi_runtime_start & ~runtime_mask;
> > > -       runtime_end = (uintptr_t)__efi_runtime_stop;
> > > -       runtime_end = (runtime_end + runtime_mask) & ~runtime_mask;
> > > -       runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> > > -       efi_add_memory_map_pg(runtime_start, runtime_pages,
> > > -                             EFI_RUNTIME_SERVICES_CODE, false);
> >
> >
> > Can sandbox boot an OS? If not there's no point adding this. But if
> > you insist, I prefer
>
> No sandbox can't really boot an OS. The primary reason for this change
> is to avoid strange things appearing in the EFI memory map (and
> therefore the EFI log). I found it very confusing.
>
> > if (IS_ENABLED(CONFIG_SANDBOX)
> >     return
> >
> > at the top of the function
>
> OK

But can't we call ExitBootServices  from sandbox and test some of those?

Thanks
/Ilias
>
> >
> > Thanks
> > /Ilias
> >
> > > +       if (!IS_ENABLED(CONFIG_SANDBOX)) {
> > > +               /*
> > > +                * Add Runtime Services. We mark surrounding boottime code as
> > > +                * runtime as well to fulfill the runtime alignment constraints
> > > +                * but avoid padding.
> > > +                *
> > > +                * This is not enabled for sandbox, since we cannot map the
> > > +                * sandbox code into emulated SDRAM
> > > +                */
> > > +               runtime_start = (uintptr_t)__efi_runtime_start & ~runtime_mask;
> > > +               runtime_end = (uintptr_t)__efi_runtime_stop;
> > > +               runtime_end = (runtime_end + runtime_mask) & ~runtime_mask;
> > > +               runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> > > +               efi_add_memory_map_pg(runtime_start, runtime_pages,
> > > +                                     EFI_RUNTIME_SERVICES_CODE, false);
> > > +       }
> > >  }
> > >
> > >  int efi_memory_init(void)
> > > --
> > > 2.34.1
> > >
>
> Regards,
> Simon