[PATCH 02/10] Add part_find command

Simon Glass sjg at chromium.org
Sun Dec 1 17:12:35 CET 2024


Hi Matthew,

On Sat, 23 Nov 2024 at 12:56, Matthew Garrett <mjg59 at srcf.ucam.org> wrote:
>
> From: Matthew Garrett <mgarrett at aurora.tech>
>
> part_find takes a GPT GUID and searches for a partition that matches
> that. It then sets the target_part environment variable to the media
> type, device number and partition number that matched, allowing
> $target_part to be passed directly to bootm and similar commands.
>
> Signed-off-by: Matthew Garrett <mgarrett at aurora.tech>
> ---
>
>  cmd/Kconfig     |  10 ++++
>  cmd/Makefile    |   1 +
>  cmd/part_find.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 167 insertions(+)
>  create mode 100644 cmd/part_find.c

Reviewed-by: Simon Glass <sjg at chromium.org>

Nits below. This needs an update to doc/usage/cmd/part.rst and a test.
There is a ChromeOS bootimage used in the bootflow tests which might
be helpful here, as it uses GPT.

>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 1d7ddb4ed36..ee85928ca21 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1529,6 +1529,16 @@ config CMD_PART
>           Read and display information about the partition table on
>           various media.
>
> +config CMD_PART_FIND
> +       bool "part_find"
> +       depends on PARTITIONS
> +       select HAVE_BLOCK_DEVICE
> +       select PARTITION_UUIDS
> +       select PARTITION_TYPE_GUID
> +       help
> +         Find a partition with a given type GUID and set the target_part
> +         environment variable if located.
> +
>  config CMD_PCI
>         bool "pci - Access PCI devices"
>         help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index d1f369deec0..16fd8dcd723 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -137,6 +137,7 @@ obj-$(CONFIG_CMD_NVEDIT_EFI) += nvedit_efi.o
>  obj-$(CONFIG_CMD_ONENAND) += onenand.o
>  obj-$(CONFIG_CMD_OSD) += osd.o
>  obj-$(CONFIG_CMD_PART) += part.o
> +obj-$(CONFIG_CMD_PART_FIND) += part_find.o
>  obj-$(CONFIG_CMD_PCAP) += pcap.o
>  ifdef CONFIG_PCI
>  obj-$(CONFIG_CMD_PCI) += pci.o
> diff --git a/cmd/part_find.c b/cmd/part_find.c
> new file mode 100644
> index 00000000000..8c071d3c374
> --- /dev/null
> +++ b/cmd/part_find.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Aurora Innovation, Inc. Copyright 2022.
> + *
> + */
> +
> +#include <blk.h>
> +#include <config.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <env.h>
> +#include <part.h>
> +#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)

Header files should not be behind #ifdef

> +#include <efi.h>
> +#include <efi_api.h>
> +
> +static bool partition_is_on_device(const struct efi_device_path *device,
> +                                  const struct efi_device_path *part,
> +                                  __u32 *part_no)

u32

> +{
> +       size_t d_len, p_len;
> +       const struct efi_device_path *p, *d;
> +
> +       for (d = device; d->type != DEVICE_PATH_TYPE_END; d = (void *)d + d->length) {
> +       }
> +
> +       d_len = (void *)d - (void *)device;
> +
> +       for (p = part; p->type != DEVICE_PATH_TYPE_END &&
> +                    !(p->type == DEVICE_PATH_TYPE_MEDIA_DEVICE &&
> +                      p->sub_type == DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH);
> +            p = (void *)p + p->length) {
> +       }
> +
> +       if (p->type != DEVICE_PATH_TYPE_MEDIA_DEVICE ||
> +           p->sub_type != DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH) {
> +               // Not a partition.
> +               return false;
> +       }
> +
> +       p_len = (void *)p - (void *)part;
> +
> +       if (p_len == d_len && !memcmp(device, part, p_len)) {
> +               if (part_no)
> +                       *part_no = ((__u32 *)p)[1];
> +               return true;
> +       }
> +       return false;
> +}
> +#endif
> +
> +static int part_find(int argc, char *const argv[])
> +{
> +#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)
> +       efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID;
> +       struct efi_device_path *loaded_image_path = NULL;
> +       struct efi_boot_services *boot = efi_get_boot();
> +       struct efi_priv *priv = efi_get_priv();
> +       bool part_self = false;
> +#endif
> +       struct driver *d = ll_entry_start(struct driver, driver);
> +       const int n_ents = ll_entry_count(struct driver, driver);
> +       struct disk_partition info;
> +       struct blk_desc *desc;
> +       struct driver *entry;
> +       struct udevice *udev;
> +       struct uclass *uc;
> +       int ret;
> +
> +       if (argc != 2)
> +               return CMD_RET_USAGE;
> +
> +#if defined(CONFIG_EFI) || defined (CONFIG_EFI_APP)

Please use if (IS_ENABLED(CONFIG_EFI) || ...

We try to avoid #ifdef in C code

> +       part_self = !strncmp(argv[1], "self", 6);
> +       if (part_self) {
> +               ret = boot->handle_protocol(priv->loaded_image->device_handle,
> +                                       &efi_devpath_guid,
> +                                       (void **)&loaded_image_path);
> +               if (ret)
> +                       log_warning("failed to get device path for loaded image (ret=%d)", ret);
> +       }
> +#endif
> +
> +       ret = uclass_get(UCLASS_BLK, &uc);
> +       if (ret) {
> +               puts("Could not get BLK uclass.\n");
> +               return CMD_RET_FAILURE;
> +       }
> +       for (entry = d; entry < d + n_ents; entry++) {
> +               if (entry->id != UCLASS_BLK)
> +                       continue;
> +               uclass_foreach_dev(udev, uc) {
> +                       int i;
> +
> +                       if (udev->driver != entry)
> +                               continue;

You should be able to do:

struct udevice *dev;

blk_foreach(BLKF_BOTH, dev) {

> +                       desc = dev_get_uclass_plat(udev);
> +#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)

if() again

> +                       if (part_self) {
> +                               if (desc->if_type == IF_TYPE_EFI_MEDIA) {
> +                                       struct efi_media_plat *plat =
> +                                               dev_get_plat(udev->parent);
> +                                       __u32 loader_part_no;
> +
> +                                       if (partition_is_on_device(plat->device_path,
> +                                                                  loaded_image_path,
> +                                                                  &loader_part_no)) {
> +                                               char env[256];
> +
> +                                               ret = snprintf(env, sizeof(env), "%s %d:%d", blk_get_if_type_name(desc->if_type), desc->devnum, loader_part_no);

line too long

Perhaps put this whole block in a function?

> +                                               if (ret < 0 || ret == sizeof(env))
> +                                                       return CMD_RET_FAILURE;
> +                                               if (env_set("target_part", env))
> +                                                       return CMD_RET_FAILURE;
> +                                               return CMD_RET_SUCCESS;

We normally write this as 'return 0'

> +                                       }
> +                               }
> +                       } else {
> +#endif
> +                               for (i = 1; i <= MAX_SEARCH_PARTITIONS; i++) {
> +                                       ret = part_get_info(desc, i, &info);
> +                                       if (ret)
> +                                               break;
> +                                       if (strcasecmp(argv[1], info.type_guid) == 0) {
> +                                               char env[256];
> +                                               ret = snprintf(env, sizeof(env), "%s %d:%d", blk_get_if_type_name(desc->if_type), desc->devnum, i);
> +                                               if (ret < 0 || ret == sizeof(env))
> +                                                       return CMD_RET_FAILURE;
> +                                               env_set("target_part", env);
> +                                               debug("Setting target_part to %s\n", env);
> +                                               return CMD_RET_SUCCESS;
> +                                       }
> +                               }
> +#if defined(CONFIG_EFI) || defined(CONFIG_EFI_APP)
> +                       }
> +#endif
> +               }
> +       }
> +
> +       return CMD_RET_FAILURE;
> +}
> +
> +static int do_part_find(struct cmd_tbl *cmdtp, int flag, int argc,
> +                       char *const argv[])
> +{
> +       return part_find(argc, argv);
> +}
> +
> +U_BOOT_CMD(
> +       part_find, 2, 0, do_part_find, "Find a partition",
> +       "<guid>\n"
> +       "- Examine the list of known partitions for one that has a type\n"
> +       "  GUID that matches 'guid', expressed in the standard text format.\n"
> +       "  If successful, the target_part environment variable will be set\n"
> +       "  to the corresponding 'interface dev:part'.\n"
> +);
> --
> 2.47.0
>

Regards,
Simon


More information about the U-Boot mailing list