On Fri, Apr 26, 2024 at 7:25 AM Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
We already support creating a load option where the device-path field contains the concatenation of the binary device-path and optionally the device path of the initrd which we expose via the EFI_LOAD_FILE2_PROTOCOL.
Allow to append another device-path pointing to the device-tree identified by the device-tree GUID.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
cmd/eficonfig.c | 68 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 5 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c index 1c57e66040b..d314051ee58 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -62,6 +62,7 @@ struct eficonfig_filepath_info { struct eficonfig_boot_option { struct eficonfig_select_file_info file_info; struct eficonfig_select_file_info initrd_info;
struct eficonfig_select_file_info fdt_info; unsigned int boot_index; u16 *description; u16 *optional_data;@@ -1308,6 +1309,10 @@ static efi_status_t eficonfig_show_boot_option(struct eficonfig_boot_option *bo, if (ret != EFI_SUCCESS) goto out;
ret = prepare_file_selection_entry(efi_menu, "Fdt File: ", &bo->fdt_info);if (ret != EFI_SUCCESS)goto out;ret = create_boot_option_entry(efi_menu, "Optional Data: ", bo->optional_data, eficonfig_boot_add_optional_data, bo); if (ret != EFI_SUCCESS)@@ -1394,21 +1399,39 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo struct efi_device_path *final_dp = NULL; struct efi_device_path *device_dp = NULL; struct efi_device_path *initrd_dp = NULL;
struct efi_device_path *fdt_dp = NULL; struct efi_device_path *initrd_device_dp = NULL;struct efi_device_path *fdt_device_dp = NULL;
const struct efi_initrd_dp id_dp = {
const struct efi_initrd_dp initrd_prefix = { .vendor = { { DEVICE_PATH_TYPE_MEDIA_DEVICE, DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
sizeof(id_dp.vendor),
sizeof(initrd_prefix.vendor), }, EFI_INITRD_MEDIA_GUID, }, .end = { DEVICE_PATH_TYPE_END, DEVICE_PATH_SUB_TYPE_END,
sizeof(id_dp.end),
sizeof(initrd_prefix.end),}};const struct efi_initrd_dp fdt_prefix = {.vendor = {{DEVICE_PATH_TYPE_MEDIA_DEVICE,DEVICE_PATH_SUB_TYPE_VENDOR_PATH,sizeof(fdt_prefix.vendor),},EFI_FDT_GUID,},.end = {DEVICE_PATH_TYPE_END,DEVICE_PATH_SUB_TYPE_END,sizeof(initrd_prefix.end), } };@@ -1424,6 +1447,12 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo goto out; }
bo->fdt_info.current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE);if (!bo->fdt_info.current_path) {ret = EFI_OUT_OF_RESOURCES;goto out;}bo->description = calloc(1, EFICONFIG_DESCRIPTION_MAX * sizeof(u16)); if (!bo->description) { ret = EFI_OUT_OF_RESOURCES;@@ -1456,13 +1485,20 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo if (lo.file_path) fill_file_info(lo.file_path, &bo->file_info, device_dp);
/* Initrd file path(optional) is placed at second instance. */
/* Initrd file path (optional) is placed at second instance. */ initrd_dp = efi_dp_from_lo(&lo, &efi_lf2_initrd_guid); if (initrd_dp) { fill_file_info(initrd_dp, &bo->initrd_info, initrd_device_dp); efi_free_pool(initrd_dp); }/* Fdt file path (optional) is placed as third instance. */fdt_dp = efi_dp_from_lo(&lo, &efi_guid_fdt);if (fdt_dp) {fill_file_info(fdt_dp, &bo->fdt_info, fdt_device_dp);efi_free_pool(fdt_dp);}if (size > 0) memcpy(bo->optional_data, lo.optional_data, size); }@@ -1484,11 +1520,23 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo ret = EFI_OUT_OF_RESOURCES; goto out; }
initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp,
initrd_dp = efi_dp_concat((const struct efi_device_path *)&initrd_prefix, dp); efi_free_pool(dp); }if (bo->fdt_info.dp_volume) {dp = eficonfig_create_device_path(bo->fdt_info.dp_volume,bo->fdt_info.current_path);if (!dp) {ret = EFI_OUT_OF_RESOURCES;goto out;}fdt_dp = efi_dp_concat((const struct efi_device_path *)&fdt_prefix,dp);efi_free_pool(dp);}dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path); if (!dp) { ret = EFI_OUT_OF_RESOURCES;@@ -1505,6 +1553,13 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo goto out; } }
if (fdt_dp) {final_dp = efi_dp_merge(final_dp, &final_dp_size, fdt_dp);if (!final_dp) {ret = EFI_OUT_OF_RESOURCES;goto out;}} if (utf16_utf8_strlen(bo->optional_data)) { len = utf16_utf8_strlen(bo->optional_data) + 1;@@ -1522,9 +1577,12 @@ out: free(bo->description); free(bo->file_info.current_path); free(bo->initrd_info.current_path);
free(bo->fdt_info.current_path); efi_free_pool(device_dp); efi_free_pool(initrd_device_dp); efi_free_pool(initrd_dp);efi_free_pool(fdt_device_dp);efi_free_pool(fdt_dp); efi_free_pool(final_dp); return ret;-- 2.43.0
Would it make sense to skip showing the user partitions that are not valid (or why does the Linux Swap partition not show here but the Linux partition with ext4 does? Neither is valid for selecting Fdt File ?) and/or extend eficonfig for user-entered data? For example if I was very sure I wanted U-Boot to search a location but I just didn't have the SD Card that configuration is meant for currently inserted, I must use efidebug to configure this?
It was always confusing how Edit action hides from view an automatic (global?) boot option i.e. 'mmc 0' that is configurable from Change Boot Order. I guess that there would have just been File EFI\BOOT\BOOTRISCV64.EFI to show and that is not interesting enough information for the added complexity of a save-disabled Edit entry. However now or in future there will be useful information to display so this should become viewable as a save-disabled entry from Edit (rename? View/Edit) action, where it is convenient to see File constant EFI\BOOT\BOOTRISCV64.EFI and also the detected values that will be used i.e. from parsing U-Boot variable $fdtfile. I do not mean that this should be a method for editing U-Boot environment variables, only that it would be useful to know for example when $fdtfile=vendor/boardname.dtb that the U-Boot EFI code heuristic has decided that this currently resolves to mmc 0:1/dtb\vendor\boardname.dtb value. If the automatic values are not what the user wants then they may create a new boot entry or leave the context of eficonfig to update something that affects the detection logic such as U-Boot env variables, and perhaps again enter eficonfig to confirm that the heuristic is now doing what they would like. Without any visibility of this global boot option from the Edit action, it is not known to need to edit the boot order after adding a boot entry where before there was apparently none. Some of that could be addressed with documentation but the best result would be it being obvious through use. It's not really obvious now that something named 'mmc 0' only appearing in the Boot Order action of eficonfig what filename it requires or if it is going to use an Initrd and/or Fdt.
Aside these more broad usability concerns there was success in adding a boot item with Fdt setting and changing the order that it be preferred entry.
Tested-by: E Shattow lucent@gmail.com