Re: [U-Boot] [PATCH v3 1/1] efi_loader: need either ACPI table or device tree

2019-04-22 Thread Mark Kettenis
> From: Heinrich Schuchardt 
> Date: Mon, 22 Apr 2019 20:01:01 +0200
> 
> The EBBR specification prescribes that we should have either an ACPI table
> or a device tree but not both. Let us enforce this condition in the
> `bootefi` command.

Why?

While I agree that it would be good if U-Boot would provide a device
tree I think you're needlessly restricting users here.  Many EFI
bootloaders (GRUB, OpenBSD's bootloader on arm/arm64) have a way to
load a device tree afterwards.  This diff makes it impossible to use
that capability on systems where U-Boot doesn't provide a device tree.

Such a system obviously wouldn't be compliant with the EBBR
specification.  But together with an appropriate bootloader it could
still run an EBBR compliant OS.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v3 1/1] efi_loader: need either ACPI table or device tree

2019-04-22 Thread Heinrich Schuchardt
The EBBR specification prescribes that we should have either an ACPI table
or a device tree but not both. Let us enforce this condition in the
`bootefi` command.

If the bootefi command is called without a device tree parameter use a
previously device tree or fall back to the internal device tree.

The fdt unit test should not be run on boards with an ACPI table.

Signed-off-by: Heinrich Schuchardt 
---
v3
The fdt unit test should not be run on boards with an ACPI table.
v2
Avoid functions defined but no used.
---
 cmd/bootefi.c | 98 ---
 lib/efi_selftest/Makefile |  1 -
 2 files changed, 72 insertions(+), 27 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index be55751830..efaa548be4 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -84,6 +84,8 @@ out:
efi_root, NULL));
 }

+#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+
 /**
  * copy_fdt() - Copy the device tree to a new location available to EFI
  *
@@ -185,6 +187,25 @@ static void efi_carve_out_dt_rsv(void *fdt)
}
 }

+/**
+ * get_config_table() - get configuration table
+ *
+ * @guid:  GUID of the configuration table
+ * Return: pointer to configuration table or NULL
+ */
+static void *get_config_table(const efi_guid_t *guid)
+{
+   size_t i;
+
+   for (i = 0; i < systab.nr_tables; i++) {
+   if (!guidcmp(guid, [i].guid))
+   return systab.tables[i].table;
+   }
+   return NULL;
+}
+
+#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
+
 /**
  * efi_install_fdt() - install fdt passed by a command argument
  * @fdt_opt:   pointer to argument
@@ -195,46 +216,71 @@ static void efi_carve_out_dt_rsv(void *fdt)
  */
 static efi_status_t efi_install_fdt(const char *fdt_opt)
 {
+   /*
+* 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_opt) {
+   printf("ERROR: can't have ACPI table and device tree.\n");
+   return EFI_LOAD_ERROR;
+   }
+#else
unsigned long fdt_addr;
void *fdt;
bootm_headers_t img = { 0 };
efi_status_t ret;

if (fdt_opt) {
-   /* Install device tree */
fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
-   if (!fdt_addr && *fdt_opt != '0')
-   return EFI_INVALID_PARAMETER;
-
-   fdt = map_sysmem(fdt_addr, 0);
-   if (fdt_check_header(fdt)) {
-   printf("ERROR: invalid device tree\n");
+   if (!fdt_addr)
return EFI_INVALID_PARAMETER;
+   } else {
+   /* Look for device tree that is already installed */
+   if (get_config_table(_guid_fdt))
+   return EFI_SUCCESS;
+   /* Use our own device tree as default */
+   fdt_opt = env_get("fdtcontroladdr");
+   if (!fdt_opt) {
+   printf("ERROR: need device tree\n");
+   return EFI_NOT_FOUND;
}
+   fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
+   if (!fdt_addr) {
+   printf("ERROR: invalid $fdtcontroladdr\n");
+   return EFI_LOAD_ERROR;
+   }
+   }

-   /* Create memory reservation as indicated by the device tree */
-   efi_carve_out_dt_rsv(fdt);
+   /* Install device tree */
+   fdt = map_sysmem(fdt_addr, 0);
+   if (fdt_check_header(fdt)) {
+   printf("ERROR: invalid device tree\n");
+   return EFI_LOAD_ERROR;
+   }

-   /* Prepare fdt for payload */
-   ret = copy_fdt();
-   if (ret)
-   return ret;
+   /* Create memory reservations as indicated by the device tree */
+   efi_carve_out_dt_rsv(fdt);

-   if (image_setup_libfdt(, fdt, 0, NULL)) {
-   printf("ERROR: failed to process device tree\n");
-   return EFI_LOAD_ERROR;
-   }
+   /* Prepare device tree for payload */
+   ret = copy_fdt();
+   if (ret) {
+   printf("ERROR: out of memory\n");
+   return EFI_OUT_OF_RESOURCES;
+   }

-   /* Link to it in the efi tables */
-   ret = efi_install_configuration_table(_guid_fdt, fdt);
-   if (ret != EFI_SUCCESS) {
-   printf("ERROR: failed to install device tree\n");
-   return ret;
-   }
-   } else {
-   /* Remove device tree. EFI_NOT_FOUND can be ignored here */
-   efi_install_configuration_table(_guid_fdt, NULL);
+   if (image_setup_libfdt(, fdt, 0, NULL)) {
+   printf("ERROR: failed to process device tree\n");
+