Hi Heinrich,
On Thu, 28 Nov 2024 at 10:43, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 28.11.24 16:47, Simon Glass 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 sure an EFI application running on the sandbox can run
> ExitBootServices() and then call the ResetSystem() runtime system service.
>
> Have a look at
>
> arch/sandbox/cpu/start.c:472:
> void __efi_runtime EFIAPI efi_reset_system().
>
> __efi_runtime implements ResetSystem() after ExitBootServices().
>
> >
> > 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 a33c025fa20..796fa99f4fb 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -755,16 +755,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);
> > + 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
>
> The memory map is consumed by an EFI application like shim, GRUB, or the
> kernel stub.
>
> The memory map entries must take address values which can be used a
> void* without conversion. This is true both on real systems and on the
> sandbox.
>
> Isn't __efi_runtime_start a pointer to the start of the efi_runtime code
> section on the sandbox?
Yes, but it is not mapped into the emulated RAM, so it doesn't have an
address. Yes, we can get a pointer to it, but it isn't in the memory
map. So there is no way to add it to EFI's tables in a sensible
fashion.
A lot of the bugs fixed in this series were the result of the EFI
sandbox test finally landing, after a year of being ignored and
blocked. I am sure we will find more.
Regards,
Simon