Re: [PATCH 1/1] efi_loader: improve block device integration with DM

["Matwey V. Kornilov" <matwey.kornilov@gmail.com>]

Hi Heinrich,


This patch fixes my issue with am335x: EFI Grub works as expected now.


чт, 17 июн. 2021 г. в 18:15, Heinrich Schuchardt <xypron.glpk@gmx.de>:
>
> Up to now when devices became available after executing the UEFI
> sub-system initialization where not available for EFI applications.
>
> With the patch block devices are added to the UEFI object list whenever
> they are probed.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>


> ---
>  drivers/core/device.c               |  7 +++
>  include/efi_loader.h                |  6 +++
>  lib/efi_driver/Makefile             |  1 +
>  lib/efi_driver/efi_dm_integration.c | 36 +++++++++++++++
>  lib/efi_loader/efi_disk.c           | 72 +++++++++++++++++------------
>  5 files changed, 93 insertions(+), 29 deletions(-)
>  create mode 100644 lib/efi_driver/efi_dm_integration.c
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index cb960f8ec4..7355a5c2a9 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -14,6 +14,7 @@
>  #include <asm/global_data.h>
>  #include <asm/io.h>
>  #include <clk.h>
> +#include <efi_loader.h>
>  #include <fdtdec.h>
>  #include <fdt_support.h>
>  #include <malloc.h>
> @@ -579,6 +580,12 @@ int device_probe(struct udevice *dev)
>         if (dev->parent && device_get_uclass_id(dev) == UCLASS_PINCTRL)
>                 pinctrl_select_state(dev, "default");
>
> +       if (CONFIG_IS_ENABLED(EFI_LOADER)) {
> +               ret = efi_post_probe_device(dev);
> +               if (ret)
> +                       goto fail_uclass;
> +       }
> +
>         return 0;
>  fail_uclass:
>         if (device_remove(dev, DM_REMOVE_NORMAL)) {
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 0a9c82a257..78dd687913 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -17,6 +17,7 @@
>  #include <pe.h>
>
>  struct blk_desc;
> +struct udevice;
>
>  static inline int guidcmp(const void *g1, const void *g2)
>  {
> @@ -28,6 +29,9 @@ static inline void *guidcpy(void *dst, const void *src)
>         return memcpy(dst, src, sizeof(efi_guid_t));
>  }
>
> +/* Called by device_probe() */
> +int efi_post_probe_device(struct udevice *dev);
> +
>  /* No need for efi loader support in SPL */
>  #if CONFIG_IS_ENABLED(EFI_LOADER)
>
> @@ -420,6 +424,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition,
>  void efi_carve_out_dt_rsv(void *fdt);
>  /* Called by bootefi to make console interface available */
>  efi_status_t efi_console_register(void);
> +/* Called when a block devices has been probed */
> +efi_status_t efi_block_device_register(struct udevice *dev);
>  /* Called by bootefi to make all disk storage accessible as EFI objects */
>  efi_status_t efi_disk_register(void);
>  /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
> diff --git a/lib/efi_driver/Makefile b/lib/efi_driver/Makefile
> index 83baa1c9a4..f0d5fa5074 100644
> --- a/lib/efi_driver/Makefile
> +++ b/lib/efi_driver/Makefile
> @@ -5,6 +5,7 @@
>  # This file only gets included with CONFIG_EFI_LOADER set, so all
>  # object inclusion implicitly depends on it
>
> +obj-y += efi_dm_integration.o
>  obj-y += efi_uclass.o
>  ifeq ($(CONFIG_BLK)$(CONFIG_PARTITIONS),yy)
>  obj-y += efi_block_device.o
> diff --git a/lib/efi_driver/efi_dm_integration.c b/lib/efi_driver/efi_dm_integration.c
> new file mode 100644
> index 0000000000..9c6c339473
> --- /dev/null
> +++ b/lib/efi_driver/efi_dm_integration.c
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021, Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +
> +#define LOG_CATEGORY LOGC_EFI
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <efi_loader.h>
> +#include <log.h>
> +
> +/**
> + * efi_post_probe_device() - set up handle for probed device
> + *
> + * This function is called by device_probe(). After the UEFI sub-system is
> + * initialized this function adds handles for new devices.
> + *
> + * @dev:       probed device
> + * Return:     0 on success
> + */
> +int efi_post_probe_device(struct udevice *dev)
> +{
> +       if (!dev || !dev->uclass)
> +               return -EINVAL;
> +
> +       switch (dev->uclass->uc_drv->id) {
> +       case UCLASS_BLK:
> +               if (efi_block_device_register(dev) != EFI_SUCCESS)
> +                       log_err("Failed to register %s\n", dev->name);
> +       default:
> +               break;
> +       }
> +
> +       return 0;
> +}
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 988907ecb9..b798cab345 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -10,6 +10,7 @@
>  #include <common.h>
>  #include <blk.h>
>  #include <dm.h>
> +#include <dm/device-internal.h>
>  #include <efi_loader.h>
>  #include <fs.h>
>  #include <log.h>
> @@ -535,6 +536,41 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
>         return disks;
>  }
>
> +/**
> + * efi_block_device_register() - register a block device in the UEFI sub-system
> + *
> + * @dev:       block device
> + * Return:     status code
> + */
> +efi_status_t efi_block_device_register(struct udevice *dev)
> +{
> +       struct blk_desc *desc = dev_get_uclass_plat(dev);
> +       const char *if_typename = blk_get_if_type_name(desc->if_type);
> +       struct efi_disk_obj *disk;
> +       efi_status_t ret;
> +
> +       /* Add block device for the full device */
> +       ret = device_probe(dev);
> +       if (ret)
> +               return EFI_NOT_FOUND;
> +       log_info("Scanning disk %s...\n", dev->name);
> +       ret = efi_disk_add_dev(NULL, NULL, if_typename,
> +                               desc, desc->devnum, NULL, 0, &disk);
> +       if (ret == EFI_NOT_READY) {
> +               log_notice("Disk %s not ready\n", dev->name);
> +               return ret;
> +       } else if (ret != EFI_SUCCESS) {
> +               log_err("ERROR: failure to add disk device %s, r = %lu\n",
> +                       dev->name, ret & ~EFI_ERROR_MASK);
> +               return ret;
> +       }
> +
> +       /* Partitions show up as block devices in EFI */
> +       efi_disk_create_partitions(&disk->header, desc, if_typename,
> +                                  desc->devnum, dev->name);
> +       return ret;
> +}
> +
>  /**
>   * efi_disk_register() - register block devices
>   *
> @@ -552,38 +588,16 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
>   */
>  efi_status_t efi_disk_register(void)
>  {
> -       struct efi_disk_obj *disk;
> -       int disks = 0;
> -       efi_status_t ret;
>  #ifdef CONFIG_BLK
>         struct udevice *dev;
> -
> +       /* Probe all block devices */
>         for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
> -            uclass_next_device_check(&dev)) {
> -               struct blk_desc *desc = dev_get_uclass_plat(dev);
> -               const char *if_typename = blk_get_if_type_name(desc->if_type);
> -
> -               /* Add block device for the full device */
> -               log_info("Scanning disk %s...\n", dev->name);
> -               ret = efi_disk_add_dev(NULL, NULL, if_typename,
> -                                       desc, desc->devnum, NULL, 0, &disk);
> -               if (ret == EFI_NOT_READY) {
> -                       log_notice("Disk %s not ready\n", dev->name);
> -                       continue;
> -               }
> -               if (ret) {
> -                       log_err("ERROR: failure to add disk device %s, r = %lu\n",
> -                               dev->name, ret & ~EFI_ERROR_MASK);
> -                       return ret;
> -               }
> -               disks++;
> -
> -               /* Partitions show up as block devices in EFI */
> -               disks += efi_disk_create_partitions(
> -                                       &disk->header, desc, if_typename,
> -                                       desc->devnum, dev->name);
> -       }
> +            uclass_next_device_check(&dev))
> +               ;
>  #else
> +       struct efi_disk_obj *disk;
> +       int disks = 0;
> +       efi_status_t ret;
>         int i, if_type;
>
>         /* Search for all available disk devices */
> @@ -630,8 +644,8 @@ efi_status_t efi_disk_register(void)
>                                                  if_typename, i, devname);
>                 }
>         }
> -#endif
>         log_info("Found %d disks\n", disks);
> +#endif
>
>         return EFI_SUCCESS;
>  }
> --
> 2.30.2
>


--
With best regards,
Matwey V. Kornilov