[PATCH 1/1] efi_loader: create memory reservations in ACPI case

ACPI tables cannot convey memory reservations for least ARM and RISC-V. x86 uses the BIOS E820 table for this purpose. We cannot simply ignore the device-tree when booting via ACPI. We have to assign EfiReservedMemory according to the prior stage device-tree ($fdtaddr) or as fallback the control device-tree ($fdtcontroladdr).
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- cmd/bootefi.c | 25 ++++++++++--------------- lib/efi_loader/Makefile | 2 -- 2 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 20e5c94a33..395b0629de 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -162,8 +162,6 @@ static efi_status_t efi_env_set_load_options(efi_handle_t handle, return ret; }
-#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) - /** * copy_fdt() - Copy the device tree to a new location available to EFI * @@ -237,8 +235,6 @@ static void *get_config_table(const efi_guid_t *guid) return NULL; }
-#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */ - /** * efi_install_fdt() - install device tree * @@ -258,18 +254,15 @@ static void *get_config_table(const efi_guid_t *guid) */ efi_status_t efi_install_fdt(void *fdt) { + struct bootm_headers img = { 0 }; + efi_status_t ret; + /* * The EBBR spec requires that we have either an FDT or an ACPI table * but not both. */ -#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) - if (fdt) { + if (CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) && fdt) log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n"); - return EFI_SUCCESS; - } -#else - struct bootm_headers img = { 0 }; - efi_status_t ret;
if (fdt == EFI_FDT_USE_INTERNAL) { const char *fdt_opt; @@ -302,6 +295,12 @@ efi_status_t efi_install_fdt(void *fdt) return EFI_LOAD_ERROR; }
+ /* Create memory reservations as indicated by the device tree */ + efi_carve_out_dt_rsv(fdt); + + if (CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) + return EFI_SUCCESS; + /* Prepare device tree for payload */ ret = copy_fdt(&fdt); if (ret) { @@ -314,9 +313,6 @@ efi_status_t efi_install_fdt(void *fdt) return EFI_LOAD_ERROR; }
- /* Create memory reservations as indicated by the device tree */ - efi_carve_out_dt_rsv(fdt); - efi_try_purge_kaslr_seed(fdt);
if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) { @@ -333,7 +329,6 @@ efi_status_t efi_install_fdt(void *fdt) log_err("ERROR: failed to install device tree\n"); return ret; } -#endif /* GENERATE_ACPI_TABLE */
return EFI_SUCCESS; } diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8d31fc61c6..d476df112b 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -51,9 +51,7 @@ obj-y += efi_console.o obj-y += efi_device_path.o obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o obj-$(CONFIG_EFI_DEVICE_PATH_UTIL) += efi_device_path_utilities.o -ifeq ($(CONFIG_GENERATE_ACPI_TABLE),) obj-y += efi_dt_fixup.o -endif obj-y += efi_file.o obj-$(CONFIG_EFI_LOADER_HII) += efi_hii.o obj-y += efi_image_loader.o

Hi Heinrich,
On Thu, 16 Nov 2023 at 02:29, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
ACPI tables cannot convey memory reservations for least ARM and RISC-V. x86 uses the BIOS E820 table for this purpose. We cannot simply ignore the device-tree when booting via ACPI.
Why is that? I had thought that we had to use one or the other. Isn't the devicetree irrelevant when booting with ACPI, so we can just drop it?
We have to assign EfiReservedMemory according to the prior stage device-tree ($fdtaddr) or as fallback the control device-tree ($fdtcontroladdr).
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
cmd/bootefi.c | 25 ++++++++++--------------- lib/efi_loader/Makefile | 2 -- 2 files changed, 10 insertions(+), 17 deletions(-)
The code looks fine, but I would like to better understand why this is needed.
Regards, Simon

On 11/18/23 18:10, Simon Glass wrote:
Hi Heinrich,
On Thu, 16 Nov 2023 at 02:29, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
ACPI tables cannot convey memory reservations for least ARM and RISC-V. x86 uses the BIOS E820 table for this purpose. We cannot simply ignore the device-tree when booting via ACPI.
Why is that? I had thought that we had to use one or the other. Isn't the devicetree irrelevant when booting with ACPI, so we can just drop it?
Linux Documentation/arch/arm64/acpi_object_usage.rst, line 718f describes the mechanism to convey memory information as follows:
"For arm64, we will only support UEFI for booting with ACPI, hence the UEFI GetMemoryMap() boot service is the only mechanism that will be used."
With the patch we tell the UEFI sub-system which memory areas are reserved (e.g. for TF-A on ARM or for OpenSBI on RISC-V). This information is inferred from the device-tree. The operating system will collect this information by calling GetMemoryMap() immediately before ExitBootServices().
Without the patch Linux on RISC-V using ACPI crashed when illegally accessing the memory reserved for OpenSBI.
Best regards
Heinrich
We have to assign EfiReservedMemory according to the prior stage device-tree ($fdtaddr) or as fallback the control device-tree ($fdtcontroladdr).
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
cmd/bootefi.c | 25 ++++++++++--------------- lib/efi_loader/Makefile | 2 -- 2 files changed, 10 insertions(+), 17 deletions(-)
The code looks fine, but I would like to better understand why this is needed.
Regards, Simon

Hi Heinrich,
On Sat, 18 Nov 2023 at 10:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/18/23 18:10, Simon Glass wrote:
Hi Heinrich,
On Thu, 16 Nov 2023 at 02:29, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
ACPI tables cannot convey memory reservations for least ARM and RISC-V. x86 uses the BIOS E820 table for this purpose. We cannot simply ignore the device-tree when booting via ACPI.
Why is that? I had thought that we had to use one or the other. Isn't the devicetree irrelevant when booting with ACPI, so we can just drop it?
Linux Documentation/arch/arm64/acpi_object_usage.rst, line 718f describes the mechanism to convey memory information as follows:
"For arm64, we will only support UEFI for booting with ACPI, hence the UEFI GetMemoryMap() boot service is the only mechanism that will be used."
With the patch we tell the UEFI sub-system which memory areas are reserved (e.g. for TF-A on ARM or for OpenSBI on RISC-V). This information is inferred from the device-tree. The operating system will collect this information by calling GetMemoryMap() immediately before ExitBootServices().
Without the patch Linux on RISC-V using ACPI crashed when illegally accessing the memory reserved for OpenSBI.
OK thank you, I misunderstood it.
Reviewed-by: Simon Glass sjg@chromium.org
Perhaps someone could convert efi_carve_out_dt_rsv() to use the ofnode API?
Regards, Simon

On 11/18/23 18:58, Simon Glass wrote:
Hi Heinrich,
On Sat, 18 Nov 2023 at 10:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/18/23 18:10, Simon Glass wrote:
Hi Heinrich,
On Thu, 16 Nov 2023 at 02:29, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
ACPI tables cannot convey memory reservations for least ARM and RISC-V. x86 uses the BIOS E820 table for this purpose. We cannot simply ignore the device-tree when booting via ACPI.
Why is that? I had thought that we had to use one or the other. Isn't the devicetree irrelevant when booting with ACPI, so we can just drop it?
Linux Documentation/arch/arm64/acpi_object_usage.rst, line 718f describes the mechanism to convey memory information as follows:
"For arm64, we will only support UEFI for booting with ACPI, hence the UEFI GetMemoryMap() boot service is the only mechanism that will be used."
With the patch we tell the UEFI sub-system which memory areas are reserved (e.g. for TF-A on ARM or for OpenSBI on RISC-V). This information is inferred from the device-tree. The operating system will collect this information by calling GetMemoryMap() immediately before ExitBootServices().
Without the patch Linux on RISC-V using ACPI crashed when illegally accessing the memory reserved for OpenSBI.
OK thank you, I misunderstood it.
Reviewed-by: Simon Glass sjg@chromium.org
Perhaps someone could convert efi_carve_out_dt_rsv() to use the ofnode API?
Is there any ofnode API to retrieve reservations as easily as with fdt_get_mem_rsv(), fdt_num_mem_rsv()?
boot/image-fdt.c, boot/fdt_region.c, boot/fdt_support.c, cmd/fdt.c all use the libfdt API directly.
Best regards
Heinrich

Hi Heinrich,
On Sat, 18 Nov 2023 at 16:06, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/18/23 18:58, Simon Glass wrote:
Hi Heinrich,
On Sat, 18 Nov 2023 at 10:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/18/23 18:10, Simon Glass wrote:
Hi Heinrich,
On Thu, 16 Nov 2023 at 02:29, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
ACPI tables cannot convey memory reservations for least ARM and RISC-V. x86 uses the BIOS E820 table for this purpose. We cannot simply ignore the device-tree when booting via ACPI.
Why is that? I had thought that we had to use one or the other. Isn't the devicetree irrelevant when booting with ACPI, so we can just drop it?
Linux Documentation/arch/arm64/acpi_object_usage.rst, line 718f describes the mechanism to convey memory information as follows:
"For arm64, we will only support UEFI for booting with ACPI, hence the UEFI GetMemoryMap() boot service is the only mechanism that will be used."
With the patch we tell the UEFI sub-system which memory areas are reserved (e.g. for TF-A on ARM or for OpenSBI on RISC-V). This information is inferred from the device-tree. The operating system will collect this information by calling GetMemoryMap() immediately before ExitBootServices().
Without the patch Linux on RISC-V using ACPI crashed when illegally accessing the memory reserved for OpenSBI.
OK thank you, I misunderstood it.
Reviewed-by: Simon Glass sjg@chromium.org
Perhaps someone could convert efi_carve_out_dt_rsv() to use the ofnode API?
Is there any ofnode API to retrieve reservations as easily as with fdt_get_mem_rsv(), fdt_num_mem_rsv()?
Not yet :-)
boot/image-fdt.c, boot/fdt_region.c, boot/fdt_support.c, cmd/fdt.c all use the libfdt API directly.
Yes, but it is starting to change. Ultimately we will likely use livetree for fixups due to efficiency.
Regards, Simon
participants (2)
-
Heinrich Schuchardt
-
Simon Glass