[RFC 04/14] cmd: eficonfig: add support for setting fdt
E Shattow
lucent at gmail.com
Sat Apr 27 19:21:46 CEST 2024
On Fri, Apr 26, 2024 at 7:25 AM Heinrich Schuchardt
<heinrich.schuchardt at 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 at 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 at gmail.com>
More information about the U-Boot
mailing list