[PATCH 1/1] efi_loader: improve block device integration with DM
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 --- 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
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
Hi Heinrich,
On Thu, 17 Jun 2021 at 09:20, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
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
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;}
Huge flashing NAK on this.
Way back in the early days I mentioned that UEFI should not have its own tables but should use driver model directly. It was discussed but 'do it later' was the answer.
To the extend that UEFI needs extra info to be tacked onto devices we should figure out a consistent way to do it.
What I proposed 5-ish years ago is lots in the mists of time, but it is something like:
- use driver model for *everything* - use struct udevice everywhere - avoid using parallel tables - things on the UEFI side then become dynamic rather than static - so there is no need for fixups - figure out what extra info is needed and come up with a generic way to attach it to devices
So this is definitely not the right approach.
Regards, Simon
Can we just (temporarily) revert
f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")
until this issue is resolved properly? Because, currently it seems like the commit breaking the things was introduced at some point and cannot be fixed within a reasonable amount of time since it seems that the issue is related to the EFI implementation design.
сб, 26 июн. 2021 г. в 21:31, Simon Glass sjg@chromium.org:
Hi Heinrich,
On Thu, 17 Jun 2021 at 09:20, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
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
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;}Huge flashing NAK on this.
Way back in the early days I mentioned that UEFI should not have its own tables but should use driver model directly. It was discussed but 'do it later' was the answer.
To the extend that UEFI needs extra info to be tacked onto devices we should figure out a consistent way to do it.
What I proposed 5-ish years ago is lots in the mists of time, but it is something like:
- use driver model for *everything*
- use struct udevice everywhere
- avoid using parallel tables
- things on the UEFI side then become dynamic rather than static
- so there is no need for fixups
- figure out what extra info is needed and come up with a generic way
to attach it to devices
So this is definitely not the right approach.
Regards, Simon
Am 2021-07-07 09:45, schrieb Matwey V. Kornilov:
Can we just (temporarily) revert
f3866909e350 ("distro_bootcmd: call EFI bootmgr even without having /EFI/boot")
until this issue is resolved properly? Because, currently it seems like the commit breaking the things was introduced at some point and cannot be fixed within a reasonable amount of time since it seems that the issue is related to the EFI implementation design.
Mh, that would then break booting boards with an EFI boot entry and without an /EFI/boot/ binary.
Given that this is only a change in the environment, can't you workaround your issue by changing your environment until there is a proper fix?
-michael
participants (4)
-
Heinrich Schuchardt -
Matwey V. Kornilov -
Michael Walle -
Simon Glass