Re: [PATCH v2 27/28] efi_loader: Don't try to add sandbox runtime code

Hi Heinrich,

On Thu, 28 Nov 2024 at 10:43, Heinrich Schuchardt xypron.glpk@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@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