[PATCH v5 00/28] efi: Improvements to U-Boot running on top of UEFI

At present U-Boot can be built as an EFI app, but it is really just for testing, with very few features. Instead, the payload build is used for booting on top of UEFI, where U-Boot takes over the machine immediately and supplies its own drivers.
But the app could be made more useful.
This series provides access to EFI block devices and the video console within U-Boot. It also restructures the app to make it possible to boot a kernel, although further work is needed to implement this.
This can be thought of as making U-Boot perform a little like 'grub', in that it runs purely based on UEFI-provided services.
Of course a lot more work is required, but this is a start.
Note: It would be very useful to include qemu tests of the app and stub in CI.
This is available at u-boot-dm/efi-working
Changes in v5: - Add new patch to avoid setting up global_data again with EFI - Add new patch to avoid using the 64-bit link script for the EFI app - Add new patch to build the 64-bit app properly - Add new patch to resolve EFI/EFI_LOADER conflict - Add new patch to round out the link script for 64-bit EFI - Add new patch to set the correct link flags for the 64-bit EFI app - Add new patch to tweak the code used for the 64-bit EFI app - Add various patches to fix up the 64-bit app so that it boots - Fix grammar in commit message
Changes in v3: - Add new patch to show the system-table revision - Drop comments that confuse sphinx - Move device_path path change to its own patch
Changes in v2: - Add MAINTAINERS entry - Add a better boot command too - Add a sentence about what the patch does - Add new patch to support the efi command in the app - Drop mention of partitions from the commit message - Fix 'as' typo - Show the correct interface type with 'part list' - Update documentation - Update the commit message to explain things better - Use log_info() instead of printf()
Simon Glass (28): efi: Rename UCLASS_EFI and IF_TYPE_EFI efi: Add EFI uclass for media efi: Add a media/block driver for EFI block devices efi: Locate all block devices in the app efi: serial: Support arrow keys bloblist: Support allocating the bloblist x86: Allow booting a kernel from the EFI app x86: Don't process the kernel command line unless enabled x86: efi: Add room for the binman definition in the dtb efi: Drop device_path from struct efi_priv efi: Add comments to struct efi_priv efi: Fix ll_boot_init() operation with the app efi: Add a few comments to the stub efi: Share struct efi_priv between the app and stub code efi: Move exit_boot_services into a function efi: Check for failure when initing the app efi: Mention that efi_info_get() is only used in the stub efi: Show when allocated pages are used efi: Allow easy selection of serial-only operation x86: efi: Update efi_get_next_mem_desc() to avoid needing a map efi: Support the efi command in the app x86: efi: Show the system-table revision x86: efi: Don't set up global_data again with EFI x86: efi: Tweak the code used for the 64-bit EFI app x86: efi: Round out the link script for 64-bit EFI x86: efi: Don't use the 64-bit link script for the EFI app x86: efi: Set the correct link flags for the 64-bit EFI app efi: Build the 64-bit app properly
MAINTAINERS | 3 + Makefile | 4 +- arch/sandbox/dts/test.dts | 4 + arch/x86/config.mk | 15 +- arch/x86/cpu/config.mk | 2 +- arch/x86/cpu/efi/payload.c | 17 +- arch/x86/cpu/x86_64/cpu.c | 5 + arch/x86/dts/Makefile | 2 +- arch/x86/lib/Makefile | 5 +- arch/x86/lib/bootm.c | 11 +- arch/x86/lib/elf_x86_64_efi.lds | 5 +- arch/x86/lib/relocate.c | 2 + arch/x86/lib/zimage.c | 13 +- cmd/Makefile | 2 +- cmd/efi.c | 78 +++++--- common/Kconfig | 15 +- common/bloblist.c | 16 +- common/board_f.c | 8 +- common/board_r.c | 5 +- disk/part.c | 5 +- doc/develop/bloblist.rst | 16 ++ doc/develop/uefi/u-boot_on_efi.rst | 6 +- doc/develop/uefi/uefi.rst | 8 +- drivers/block/Kconfig | 33 ++++ drivers/block/Makefile | 4 + drivers/block/blk-uclass.c | 6 +- drivers/block/efi-media-uclass.c | 15 ++ drivers/block/efi_blk.c | 115 ++++++++++++ drivers/block/sb_efi_media.c | 20 +++ drivers/serial/serial_efi.c | 11 +- include/blk.h | 3 +- include/configs/efi-x86_app.h | 25 +++ include/dm/uclass-id.h | 3 +- include/efi.h | 113 +++++++++++- include/efi_api.h | 15 ++ include/init.h | 2 +- lib/efi/efi.c | 97 ++++++++++ lib/efi/efi_app.c | 276 +++++++++++++++++++++++++++-- lib/efi/efi_stub.c | 95 ++++------ lib/efi_driver/efi_block_device.c | 8 +- lib/efi_driver/efi_uclass.c | 8 +- test/dm/Makefile | 1 + test/dm/efi_media.c | 24 +++ 43 files changed, 958 insertions(+), 163 deletions(-) create mode 100644 drivers/block/efi-media-uclass.c create mode 100644 drivers/block/efi_blk.c create mode 100644 drivers/block/sb_efi_media.c create mode 100644 test/dm/efi_media.c

These names are better used for access to devices provided by an EFI layer. Use EFI_LOADER instead here, since these are only available in U-Boot's EFI_LOADER layer.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Add new patch to resolve EFI/EFI_LOADER conflict
doc/develop/uefi/uefi.rst | 8 ++++---- drivers/block/blk-uclass.c | 4 ++-- include/blk.h | 2 +- include/dm/uclass-id.h | 2 +- lib/efi_driver/efi_block_device.c | 8 ++++---- lib/efi_driver/efi_uclass.c | 8 ++++---- 6 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index f17138f5c76..a3e2656ab81 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -620,12 +620,12 @@ EFI_DRIVER_BINDING_PROTOCOL implementation for the UEFI drivers.
A linker created list is used to keep track of the UEFI drivers. To create an entry in the list the UEFI driver uses the U_BOOT_DRIVER macro specifying -UCLASS_EFI as the ID of its uclass, e.g:: +UCLASS_EFI_LOADER as the ID of its uclass, e.g::
/* Identify as UEFI driver */ U_BOOT_DRIVER(efi_block) = { .name = "EFI block driver", - .id = UCLASS_EFI, + .id = UCLASS_EFI_LOADER, .ops = &driver_ops, };
@@ -651,8 +651,8 @@ UEFI block IO driver The UEFI block IO driver supports devices exposing the EFI_BLOCK_IO_PROTOCOL.
When connected it creates a new U-Boot block IO device with interface type -IF_TYPE_EFI, adds child controllers mapping the partitions, and installs the -EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on these. This can be used together with the +IF_TYPE_EFI_LOADER, adds child controllers mapping the partitions, and installs +the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on these. This can be used together with the software iPXE to boot from iSCSI network drives [4].
This driver is only available if U-Boot is configured with:: diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 83682dcc181..a7470ae28d5 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -28,7 +28,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = "sata", [IF_TYPE_HOST] = "host", [IF_TYPE_NVME] = "nvme", - [IF_TYPE_EFI] = "efi", + [IF_TYPE_EFI_LOADER] = "efiloader", [IF_TYPE_VIRTIO] = "virtio", [IF_TYPE_PVBLOCK] = "pvblock", }; @@ -44,7 +44,7 @@ static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = UCLASS_AHCI, [IF_TYPE_HOST] = UCLASS_ROOT, [IF_TYPE_NVME] = UCLASS_NVME, - [IF_TYPE_EFI] = UCLASS_EFI, + [IF_TYPE_EFI_LOADER] = UCLASS_EFI_LOADER, [IF_TYPE_VIRTIO] = UCLASS_VIRTIO, [IF_TYPE_PVBLOCK] = UCLASS_PVBLOCK, }; diff --git a/include/blk.h b/include/blk.h index f0cc7ca1a28..f0835c3fed5 100644 --- a/include/blk.h +++ b/include/blk.h @@ -34,7 +34,7 @@ enum if_type { IF_TYPE_SATA, IF_TYPE_HOST, IF_TYPE_NVME, - IF_TYPE_EFI, + IF_TYPE_EFI_LOADER, IF_TYPE_PVBLOCK, IF_TYPE_VIRTIO,
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index fd139b9b2a0..5e503960aa7 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -48,7 +48,7 @@ enum uclass_id { UCLASS_DMA, /* Direct Memory Access */ UCLASS_DSA, /* Distributed (Ethernet) Switch Architecture */ UCLASS_ECDSA, /* Elliptic curve cryptographic device */ - UCLASS_EFI, /* EFI managed devices */ + UCLASS_EFI_LOADER, /* Devices managed by EFI_LOADER */ UCLASS_ETH, /* Ethernet device */ UCLASS_ETH_PHY, /* Ethernet PHY device */ UCLASS_FIRMWARE, /* Firmware */ diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index 0937e3595a4..04cb3ef0d4e 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -147,7 +147,7 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) if (!obj) return -ENOENT;
- devnum = blk_find_max_devnum(IF_TYPE_EFI); + devnum = blk_find_max_devnum(IF_TYPE_EFI_LOADER); if (devnum == -ENODEV) devnum = 0; else if (devnum < 0) @@ -159,8 +159,8 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) sprintf(name, "efiblk#%d", devnum);
/* Create driver model udevice for the EFI block io device */ - ret = blk_create_device(parent, "efi_blk", name, IF_TYPE_EFI, devnum, - io->media->block_size, + ret = blk_create_device(parent, "efi_blk", name, IF_TYPE_EFI_LOADER, + devnum, io->media->block_size, (lbaint_t)io->media->last_block, &bdev); if (ret) return ret; @@ -209,6 +209,6 @@ static const struct efi_driver_ops driver_ops = { /* Identify as EFI driver */ U_BOOT_DRIVER(efi_block) = { .name = "EFI block driver", - .id = UCLASS_EFI, + .id = UCLASS_EFI_LOADER, .ops = &driver_ops, }; diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 382c2b477f4..b01ce89c84e 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -308,7 +308,7 @@ efi_status_t efi_driver_init(void) log_debug("Initializing EFI driver framework\n"); for (drv = ll_entry_start(struct driver, driver); drv < ll_entry_end(struct driver, driver); ++drv) { - if (drv->id == UCLASS_EFI) { + if (drv->id == UCLASS_EFI_LOADER) { ret = efi_add_driver(drv); if (ret != EFI_SUCCESS) { log_err("Failed to add EFI driver %s\n", @@ -328,7 +328,7 @@ efi_status_t efi_driver_init(void) */ static int efi_uc_init(struct uclass *class) { - log_debug("Initializing UCLASS_EFI\n"); + log_debug("Initializing UCLASS_EFI_LOADER\n"); return 0; }
@@ -340,13 +340,13 @@ static int efi_uc_init(struct uclass *class) */ static int efi_uc_destroy(struct uclass *class) { - log_debug("Destroying UCLASS_EFI\n"); + log_debug("Destroying UCLASS_EFI_LOADER\n"); return 0; }
UCLASS_DRIVER(efi) = { .name = "efi", - .id = UCLASS_EFI, + .id = UCLASS_EFI_LOADER, .init = efi_uc_init, .destroy = efi_uc_destroy, };

On 12/4/21 07:56, Simon Glass wrote:
These names are better used for access to devices provided by an EFI layer. Use EFI_LOADER instead here, since these are only available in U-Boot's EFI_LOADER layer.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v5:
Add new patch to resolve EFI/EFI_LOADER conflict
doc/develop/uefi/uefi.rst | 8 ++++---- drivers/block/blk-uclass.c | 4 ++-- include/blk.h | 2 +- include/dm/uclass-id.h | 2 +- lib/efi_driver/efi_block_device.c | 8 ++++---- lib/efi_driver/efi_uclass.c | 8 ++++---- 6 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index f17138f5c76..a3e2656ab81 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -620,12 +620,12 @@ EFI_DRIVER_BINDING_PROTOCOL implementation for the UEFI drivers.
A linker created list is used to keep track of the UEFI drivers. To create an entry in the list the UEFI driver uses the U_BOOT_DRIVER macro specifying -UCLASS_EFI as the ID of its uclass, e.g:: +UCLASS_EFI_LOADER as the ID of its uclass, e.g::
/* Identify as UEFI driver */ U_BOOT_DRIVER(efi_block) = { .name = "EFI block driver",
.id = UCLASS_EFI,
.id = UCLASS_EFI_LOADER, .ops = &driver_ops, };
@@ -651,8 +651,8 @@ UEFI block IO driver The UEFI block IO driver supports devices exposing the EFI_BLOCK_IO_PROTOCOL.
When connected it creates a new U-Boot block IO device with interface type -IF_TYPE_EFI, adds child controllers mapping the partitions, and installs the -EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on these. This can be used together with the +IF_TYPE_EFI_LOADER, adds child controllers mapping the partitions, and installs +the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on these. This can be used together with the software iPXE to boot from iSCSI network drives [4].
This driver is only available if U-Boot is configured with:: diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 83682dcc181..a7470ae28d5 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -28,7 +28,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = "sata", [IF_TYPE_HOST] = "host", [IF_TYPE_NVME] = "nvme",
- [IF_TYPE_EFI] = "efi",
- [IF_TYPE_EFI_LOADER] = "efiloader", [IF_TYPE_VIRTIO] = "virtio", [IF_TYPE_PVBLOCK] = "pvblock", };
@@ -44,7 +44,7 @@ static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = UCLASS_AHCI, [IF_TYPE_HOST] = UCLASS_ROOT, [IF_TYPE_NVME] = UCLASS_NVME,
- [IF_TYPE_EFI] = UCLASS_EFI,
- [IF_TYPE_EFI_LOADER] = UCLASS_EFI_LOADER, [IF_TYPE_VIRTIO] = UCLASS_VIRTIO, [IF_TYPE_PVBLOCK] = UCLASS_PVBLOCK, };
diff --git a/include/blk.h b/include/blk.h index f0cc7ca1a28..f0835c3fed5 100644 --- a/include/blk.h +++ b/include/blk.h @@ -34,7 +34,7 @@ enum if_type { IF_TYPE_SATA, IF_TYPE_HOST, IF_TYPE_NVME,
- IF_TYPE_EFI,
- IF_TYPE_EFI_LOADER, IF_TYPE_PVBLOCK, IF_TYPE_VIRTIO,
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index fd139b9b2a0..5e503960aa7 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -48,7 +48,7 @@ enum uclass_id { UCLASS_DMA, /* Direct Memory Access */ UCLASS_DSA, /* Distributed (Ethernet) Switch Architecture */ UCLASS_ECDSA, /* Elliptic curve cryptographic device */
- UCLASS_EFI, /* EFI managed devices */
- UCLASS_EFI_LOADER, /* Devices managed by EFI_LOADER */
"managed by EFI_LOADER" does not catch it. I will change this to "devices created by UEFI applications and drivers" when merging.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
UCLASS_ETH, /* Ethernet device */ UCLASS_ETH_PHY, /* Ethernet PHY device */ UCLASS_FIRMWARE, /* Firmware */ diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index 0937e3595a4..04cb3ef0d4e 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -147,7 +147,7 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) if (!obj) return -ENOENT;
- devnum = blk_find_max_devnum(IF_TYPE_EFI);
- devnum = blk_find_max_devnum(IF_TYPE_EFI_LOADER); if (devnum == -ENODEV) devnum = 0; else if (devnum < 0)
@@ -159,8 +159,8 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) sprintf(name, "efiblk#%d", devnum);
/* Create driver model udevice for the EFI block io device */
- ret = blk_create_device(parent, "efi_blk", name, IF_TYPE_EFI, devnum,
io->media->block_size,
- ret = blk_create_device(parent, "efi_blk", name, IF_TYPE_EFI_LOADER,
if (ret) return ret;devnum, io->media->block_size, (lbaint_t)io->media->last_block, &bdev);
@@ -209,6 +209,6 @@ static const struct efi_driver_ops driver_ops = { /* Identify as EFI driver */ U_BOOT_DRIVER(efi_block) = { .name = "EFI block driver",
- .id = UCLASS_EFI,
- .id = UCLASS_EFI_LOADER, .ops = &driver_ops, };
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 382c2b477f4..b01ce89c84e 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -308,7 +308,7 @@ efi_status_t efi_driver_init(void) log_debug("Initializing EFI driver framework\n"); for (drv = ll_entry_start(struct driver, driver); drv < ll_entry_end(struct driver, driver); ++drv) {
if (drv->id == UCLASS_EFI) {
if (drv->id == UCLASS_EFI_LOADER) { ret = efi_add_driver(drv); if (ret != EFI_SUCCESS) { log_err("Failed to add EFI driver %s\n",
@@ -328,7 +328,7 @@ efi_status_t efi_driver_init(void) */ static int efi_uc_init(struct uclass *class) {
- log_debug("Initializing UCLASS_EFI\n");
- log_debug("Initializing UCLASS_EFI_LOADER\n"); return 0; }
@@ -340,13 +340,13 @@ static int efi_uc_init(struct uclass *class) */ static int efi_uc_destroy(struct uclass *class) {
- log_debug("Destroying UCLASS_EFI\n");
log_debug("Destroying UCLASS_EFI_LOADER\n"); return 0; }
UCLASS_DRIVER(efi) = { .name = "efi",
- .id = UCLASS_EFI,
- .id = UCLASS_EFI_LOADER, .init = efi_uc_init, .destroy = efi_uc_destroy, };

At present UCLASS_EFI is used to represent an EFI filesystem among other things. The description of this uclass is "EFI managed devices" which is pretty vague. The only driver that uses this uclass is in fact not a real U-Boot driver, since its operations do not include a struct udevice.
Rather than mess with this, create a new UCLASS_EFI_MEDIA uclass to handle EFI media such as disks. Make this the uclass to use for EFI media so that it can be used with 'part list', for example.
The existing implementation using UCLASS_EFI remains as is, for discussion.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Add MAINTAINERS entry - Show the correct interface type with 'part list' - Update the commit message to explain things better
MAINTAINERS | 3 +++ arch/sandbox/dts/test.dts | 4 ++++ disk/part.c | 5 ++++- drivers/block/Kconfig | 23 +++++++++++++++++++++++ drivers/block/Makefile | 3 +++ drivers/block/blk-uclass.c | 2 ++ drivers/block/efi-media-uclass.c | 15 +++++++++++++++ drivers/block/sb_efi_media.c | 20 ++++++++++++++++++++ include/blk.h | 1 + include/dm/uclass-id.h | 1 + test/dm/Makefile | 1 + test/dm/efi_media.c | 24 ++++++++++++++++++++++++ 12 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 drivers/block/efi-media-uclass.c create mode 100644 drivers/block/sb_efi_media.c create mode 100644 test/dm/efi_media.c
diff --git a/MAINTAINERS b/MAINTAINERS index 9045e509d73..5b162ad2978 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -714,8 +714,11 @@ W: https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html F: board/efi/efi-x86_app F: configs/efi-x86_app* F: doc/develop/uefi/u-boot_on_efi.rst +F: drivers/block/efi-media-uclass.c +F: drivers/block/sb_efi_media.c F: lib/efi/efi_app.c F: scripts/build-efi.sh +F: test/dm/efi_media.c
EFI PAYLOAD M: Heinrich Schuchardt xypron.glpk@gmx.de diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index e5261bb9fa2..48ca3e1e472 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -499,6 +499,10 @@ compatible = "sandbox,clk-ccf"; };
+ efi-media { + compatible = "sandbox,efi-media"; + }; + eth@10002000 { compatible = "sandbox,eth"; reg = <0x10002000 0x1000>; diff --git a/disk/part.c b/disk/part.c index fe1ebd4adf7..e857a9f9585 100644 --- a/disk/part.c +++ b/disk/part.c @@ -296,8 +296,11 @@ static void print_part_header(const char *type, struct blk_desc *dev_desc) case IF_TYPE_VIRTIO: puts("VirtIO"); break; + case IF_TYPE_EFI_MEDIA: + puts("EFI"); + break; default: - puts ("UNKNOWN"); + puts("UNKNOWN"); break; } printf (" device %d -- Partition Type: %s\n\n", diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 56a4eec05ac..755fdccb574 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -61,6 +61,29 @@ config TPL_BLOCK_CACHE help This option enables the disk-block cache in TPL
+config EFI_MEDIA + bool "Support EFI media drivers" + default y if EFI || SANDBOX + help + Enable this to support media devices on top of UEFI. This enables + just the uclass so you also need a specific driver to make this do + anything. + + For sandbox there is a test driver. + +if EFI_MEDIA + +config EFI_MEDIA_SANDBOX + bool "Sandbox EFI media driver" + depends on SANDBOX + default y + help + Enables a simple sandbox media driver, used for testing just the + EFI_MEDIA uclass. It does not do anything useful, since sandbox does + not actually support running on top of UEFI. + +endif # EFI_MEDIA + config IDE bool "Support IDE controllers" select HAVE_BLOCK_DEVICE diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 94ab5c6f906..3778633da1d 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -14,3 +14,6 @@ obj-$(CONFIG_IDE) += ide.o endif obj-$(CONFIG_SANDBOX) += sandbox.o obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o + +obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o +obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index a7470ae28d5..4ae8af6d609 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -28,6 +28,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = "sata", [IF_TYPE_HOST] = "host", [IF_TYPE_NVME] = "nvme", + [IF_TYPE_EFI_MEDIA] = "efi", [IF_TYPE_EFI_LOADER] = "efiloader", [IF_TYPE_VIRTIO] = "virtio", [IF_TYPE_PVBLOCK] = "pvblock", @@ -44,6 +45,7 @@ static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = UCLASS_AHCI, [IF_TYPE_HOST] = UCLASS_ROOT, [IF_TYPE_NVME] = UCLASS_NVME, + [IF_TYPE_EFI_MEDIA] = UCLASS_EFI_MEDIA, [IF_TYPE_EFI_LOADER] = UCLASS_EFI_LOADER, [IF_TYPE_VIRTIO] = UCLASS_VIRTIO, [IF_TYPE_PVBLOCK] = UCLASS_PVBLOCK, diff --git a/drivers/block/efi-media-uclass.c b/drivers/block/efi-media-uclass.c new file mode 100644 index 00000000000..e012f6f2f4c --- /dev/null +++ b/drivers/block/efi-media-uclass.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Uclass for EFI media devices + * + * Copyright 2021 Google LLC + */ + +#include <common.h> +#include <dm.h> + +UCLASS_DRIVER(efi_media) = { + .id = UCLASS_EFI_MEDIA, + .name = "efi_media", + .flags = DM_UC_FLAG_SEQ_ALIAS, +}; diff --git a/drivers/block/sb_efi_media.c b/drivers/block/sb_efi_media.c new file mode 100644 index 00000000000..52af155a600 --- /dev/null +++ b/drivers/block/sb_efi_media.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * EFI_MEDIA driver for sandbox + * + * Copyright 2021 Google LLC + */ + +#include <common.h> +#include <dm.h> + +static const struct udevice_id sandbox_efi_media_ids[] = { + { .compatible = "sandbox,efi-media" }, + { } +}; + +U_BOOT_DRIVER(sandbox_efi_media) = { + .name = "sandbox_efi_media", + .id = UCLASS_EFI_MEDIA, + .of_match = sandbox_efi_media_ids, +}; diff --git a/include/blk.h b/include/blk.h index f0835c3fed5..dde21732572 100644 --- a/include/blk.h +++ b/include/blk.h @@ -37,6 +37,7 @@ enum if_type { IF_TYPE_EFI_LOADER, IF_TYPE_PVBLOCK, IF_TYPE_VIRTIO, + IF_TYPE_EFI_MEDIA,
IF_TYPE_COUNT, /* Number of interface types */ }; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 5e503960aa7..15fff409a83 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -49,6 +49,7 @@ enum uclass_id { UCLASS_DSA, /* Distributed (Ethernet) Switch Architecture */ UCLASS_ECDSA, /* Elliptic curve cryptographic device */ UCLASS_EFI_LOADER, /* Devices managed by EFI_LOADER */ + UCLASS_EFI_MEDIA, /* EFI media (e.g. a disk) */ UCLASS_ETH, /* Ethernet device */ UCLASS_ETH_PHY, /* Ethernet PHY device */ UCLASS_FIRMWARE, /* Firmware */ diff --git a/test/dm/Makefile b/test/dm/Makefile index 548649f8e82..d46552fbf32 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -37,6 +37,7 @@ obj-$(CONFIG_DMA) += dma.o obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o obj-$(CONFIG_DM_DSA) += dsa.o obj-$(CONFIG_ECDSA_VERIFY) += ecdsa.o +obj-$(CONFIG_EFI_MEDIA_SANDBOX) += efi_media.o obj-$(CONFIG_DM_ETH) += eth.o ifneq ($(CONFIG_EFI_PARTITION),) obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o diff --git a/test/dm/efi_media.c b/test/dm/efi_media.c new file mode 100644 index 00000000000..e343a0e9c85 --- /dev/null +++ b/test/dm/efi_media.c @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test for EFI_MEDIA uclass + * + * Copyright 2021 Google LLC + */ + +#include <common.h> +#include <dm.h> +#include <asm/test.h> +#include <dm/test.h> +#include <test/test.h> +#include <test/ut.h> + +/* Test that we can use the EFI_MEDIA uclass */ +static int dm_test_efi_media(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_first_device_err(UCLASS_EFI_MEDIA, &dev)); + + return 0; +} +DM_TEST(dm_test_efi_media, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);

On 12/4/21 07:56, Simon Glass wrote:
At present UCLASS_EFI is used to represent an EFI filesystem among other
%s/UCLASS_EFI/UCLASS_EFI_LOADER/
things. The description of this uclass is "EFI managed devices" which is pretty vague. The only driver that uses this uclass is in fact not a real U-Boot driver, since its operations do not include a struct udevice.
Rather than mess with this, create a new UCLASS_EFI_MEDIA uclass to handle EFI media such as disks. Make this the uclass to use for EFI media so that
The new uclass is for devices provided by an UEFI implementation loading U-Boot as an EFI application.
it can be used with 'part list', for example.
The existing implementation using UCLASS_EFI remains as is, for
%s/UCLASS_EFI/UCLASS_EFI_LOADER/
The existing uclass is for devices created by UEFI applications loaded by U-Boot.
discussion.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
Add MAINTAINERS entry
Show the correct interface type with 'part list'
Update the commit message to explain things better
MAINTAINERS | 3 +++ arch/sandbox/dts/test.dts | 4 ++++ disk/part.c | 5 ++++- drivers/block/Kconfig | 23 +++++++++++++++++++++++ drivers/block/Makefile | 3 +++ drivers/block/blk-uclass.c | 2 ++ drivers/block/efi-media-uclass.c | 15 +++++++++++++++ drivers/block/sb_efi_media.c | 20 ++++++++++++++++++++ include/blk.h | 1 + include/dm/uclass-id.h | 1 + test/dm/Makefile | 1 + test/dm/efi_media.c | 24 ++++++++++++++++++++++++ 12 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 drivers/block/efi-media-uclass.c create mode 100644 drivers/block/sb_efi_media.c create mode 100644 test/dm/efi_media.c
diff --git a/MAINTAINERS b/MAINTAINERS index 9045e509d73..5b162ad2978 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -714,8 +714,11 @@ W: https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html F: board/efi/efi-x86_app F: configs/efi-x86_app* F: doc/develop/uefi/u-boot_on_efi.rst +F: drivers/block/efi-media-uclass.c +F: drivers/block/sb_efi_media.c F: lib/efi/efi_app.c F: scripts/build-efi.sh +F: test/dm/efi_media.c
EFI PAYLOAD M: Heinrich Schuchardt xypron.glpk@gmx.de diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index e5261bb9fa2..48ca3e1e472 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -499,6 +499,10 @@ compatible = "sandbox,clk-ccf"; };
- efi-media {
compatible = "sandbox,efi-media";
- };
- eth@10002000 { compatible = "sandbox,eth"; reg = <0x10002000 0x1000>;
diff --git a/disk/part.c b/disk/part.c index fe1ebd4adf7..e857a9f9585 100644 --- a/disk/part.c +++ b/disk/part.c @@ -296,8 +296,11 @@ static void print_part_header(const char *type, struct blk_desc *dev_desc) case IF_TYPE_VIRTIO: puts("VirtIO"); break;
- case IF_TYPE_EFI_MEDIA:
puts("EFI");
default:break;
puts ("UNKNOWN");
break; } printf (" device %d -- Partition Type: %s\n\n",puts("UNKNOWN");
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 56a4eec05ac..755fdccb574 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -61,6 +61,29 @@ config TPL_BLOCK_CACHE help This option enables the disk-block cache in TPL
+config EFI_MEDIA
- bool "Support EFI media drivers"
- default y if EFI || SANDBOX
- help
Enable this to support media devices on top of UEFI. This enables
just the uclass so you also need a specific driver to make this do
anything.
For sandbox there is a test driver.
+if EFI_MEDIA
+config EFI_MEDIA_SANDBOX
- bool "Sandbox EFI media driver"
- depends on SANDBOX
- default y
- help
Enables a simple sandbox media driver, used for testing just the
EFI_MEDIA uclass. It does not do anything useful, since sandbox does
not actually support running on top of UEFI.
+endif # EFI_MEDIA
- config IDE bool "Support IDE controllers" select HAVE_BLOCK_DEVICE
diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 94ab5c6f906..3778633da1d 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -14,3 +14,6 @@ obj-$(CONFIG_IDE) += ide.o endif obj-$(CONFIG_SANDBOX) += sandbox.o obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
+obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o +obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index a7470ae28d5..4ae8af6d609 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -28,6 +28,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = "sata", [IF_TYPE_HOST] = "host", [IF_TYPE_NVME] = "nvme",
- [IF_TYPE_EFI_MEDIA] = "efi", [IF_TYPE_EFI_LOADER] = "efiloader", [IF_TYPE_VIRTIO] = "virtio", [IF_TYPE_PVBLOCK] = "pvblock",
@@ -44,6 +45,7 @@ static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = { [IF_TYPE_SATA] = UCLASS_AHCI, [IF_TYPE_HOST] = UCLASS_ROOT, [IF_TYPE_NVME] = UCLASS_NVME,
- [IF_TYPE_EFI_MEDIA] = UCLASS_EFI_MEDIA, [IF_TYPE_EFI_LOADER] = UCLASS_EFI_LOADER, [IF_TYPE_VIRTIO] = UCLASS_VIRTIO, [IF_TYPE_PVBLOCK] = UCLASS_PVBLOCK,
diff --git a/drivers/block/efi-media-uclass.c b/drivers/block/efi-media-uclass.c new file mode 100644 index 00000000000..e012f6f2f4c --- /dev/null +++ b/drivers/block/efi-media-uclass.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Uclass for EFI media devices
- Copyright 2021 Google LLC
- */
+#include <common.h> +#include <dm.h>
+UCLASS_DRIVER(efi_media) = {
- .id = UCLASS_EFI_MEDIA,
- .name = "efi_media",
- .flags = DM_UC_FLAG_SEQ_ALIAS,
+}; diff --git a/drivers/block/sb_efi_media.c b/drivers/block/sb_efi_media.c new file mode 100644 index 00000000000..52af155a600 --- /dev/null +++ b/drivers/block/sb_efi_media.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- EFI_MEDIA driver for sandbox
- Copyright 2021 Google LLC
- */
+#include <common.h> +#include <dm.h>
+static const struct udevice_id sandbox_efi_media_ids[] = {
- { .compatible = "sandbox,efi-media" },
- { }
+};
+U_BOOT_DRIVER(sandbox_efi_media) = {
- .name = "sandbox_efi_media",
- .id = UCLASS_EFI_MEDIA,
- .of_match = sandbox_efi_media_ids,
+}; diff --git a/include/blk.h b/include/blk.h index f0835c3fed5..dde21732572 100644 --- a/include/blk.h +++ b/include/blk.h @@ -37,6 +37,7 @@ enum if_type { IF_TYPE_EFI_LOADER, IF_TYPE_PVBLOCK, IF_TYPE_VIRTIO,
IF_TYPE_EFI_MEDIA,
IF_TYPE_COUNT, /* Number of interface types */ };
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 5e503960aa7..15fff409a83 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -49,6 +49,7 @@ enum uclass_id { UCLASS_DSA, /* Distributed (Ethernet) Switch Architecture */ UCLASS_ECDSA, /* Elliptic curve cryptographic device */ UCLASS_EFI_LOADER, /* Devices managed by EFI_LOADER */
- UCLASS_EFI_MEDIA, /* EFI media (e.g. a disk) */
%s/EFI media (e.g. a disk)/devices provided by UEFI firmware */
With those changes:
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
UCLASS_ETH, /* Ethernet device */ UCLASS_ETH_PHY, /* Ethernet PHY device */ UCLASS_FIRMWARE, /* Firmware */ diff --git a/test/dm/Makefile b/test/dm/Makefile index 548649f8e82..d46552fbf32 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -37,6 +37,7 @@ obj-$(CONFIG_DMA) += dma.o obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o obj-$(CONFIG_DM_DSA) += dsa.o obj-$(CONFIG_ECDSA_VERIFY) += ecdsa.o +obj-$(CONFIG_EFI_MEDIA_SANDBOX) += efi_media.o obj-$(CONFIG_DM_ETH) += eth.o ifneq ($(CONFIG_EFI_PARTITION),) obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o diff --git a/test/dm/efi_media.c b/test/dm/efi_media.c new file mode 100644 index 00000000000..e343a0e9c85 --- /dev/null +++ b/test/dm/efi_media.c @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Test for EFI_MEDIA uclass
- Copyright 2021 Google LLC
- */
+#include <common.h> +#include <dm.h> +#include <asm/test.h> +#include <dm/test.h> +#include <test/test.h> +#include <test/ut.h>
+/* Test that we can use the EFI_MEDIA uclass */ +static int dm_test_efi_media(struct unit_test_state *uts) +{
- struct udevice *dev;
- ut_assertok(uclass_first_device_err(UCLASS_EFI_MEDIA, &dev));
- return 0;
+} +DM_TEST(dm_test_efi_media, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);

Add a block driver which handles read/write for EFI block devices. This driver actually already exists ('efi_block') but is not really suitable for use as a real U-Boot driver:
- The operations do not provide a udevice - The code is designed for running as part of EFI loader, so uses EFI_PRINT() and EFI_CALL(). - The bind method probes the device, which is not permitted - It uses 'EFI' as its parent device
The new driver is more 'normal', just requiring its platform data be set up in advance.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Drop mention of partitions from the commit message
drivers/block/Kconfig | 10 ++++ drivers/block/Makefile | 1 + drivers/block/efi_blk.c | 115 ++++++++++++++++++++++++++++++++++++++++ include/efi.h | 11 ++++ 4 files changed, 137 insertions(+) create mode 100644 drivers/block/efi_blk.c
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 755fdccb574..8235430497d 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -82,6 +82,16 @@ config EFI_MEDIA_SANDBOX EFI_MEDIA uclass. It does not do anything useful, since sandbox does not actually support running on top of UEFI.
+config EFI_MEDIA_BLK + bool "EFI media block driver" + depends on EFI_APP + default y + help + Enables a block driver for providing access to UEFI devices. This + allows use of block devices detected by the underlying UEFI + implementation. With this it is possible to use filesystems on these + devices, for example. + endif # EFI_MEDIA
config IDE diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 3778633da1d..b221a7c6eea 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o +obj-$(CONFIG_EFI_MEDIA_BLK) += efi_blk.o diff --git a/drivers/block/efi_blk.c b/drivers/block/efi_blk.c new file mode 100644 index 00000000000..9d25ecbf37f --- /dev/null +++ b/drivers/block/efi_blk.c @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Block driver for EFI devices + * This supports a media driver of UCLASS_EFI with a child UCLASS_BLK + * It allows block-level access to EFI devices made available via EFI boot + * services + * + * Copyright 2021 Google LLC + */ + +#include <common.h> +#include <blk.h> +#include <dm.h> +#include <efi.h> +#include <efi_api.h> + +struct efi_block_plat { + struct efi_block_io *blkio; +}; + +/** + * Read from block device + * + * @dev: device + * @blknr: first block to be read + * @blkcnt: number of blocks to read + * @buffer: output buffer + * Return: number of blocks transferred + */ +static ulong efi_bl_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, + void *buffer) +{ + struct efi_block_plat *plat = dev_get_plat(dev); + struct efi_block_io *io = plat->blkio; + efi_status_t ret; + + log_debug("read buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr, + (ulong)blkcnt); + ret = io->read_blocks(io, io->media->media_id, blknr, + blkcnt * io->media->block_size, buffer); + log_debug("ret=%lx (dec %ld)\n", ret & ~EFI_ERROR_MASK, + ret & ~EFI_ERROR_MASK); + if (ret) + return 0; + + return blkcnt; +} + +/** + * Write to block device + * + * @dev: device + * @blknr: first block to be write + * @blkcnt: number of blocks to write + * @buffer: input buffer + * Return: number of blocks transferred + */ +static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, + const void *buffer) +{ + struct efi_block_plat *plat = dev_get_plat(dev); + struct efi_block_io *io = plat->blkio; + efi_status_t ret; + + log_debug("write buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr, + (ulong)blkcnt); + ret = io->write_blocks(io, io->media->media_id, blknr, + blkcnt * io->media->block_size, (void *)buffer); + log_debug("ret=%lx (dec %ld)\n", ret & ~EFI_ERROR_MASK, + ret & ~EFI_ERROR_MASK); + if (ret) + return 0; + + return blkcnt; +} + +/* Block device driver operators */ +static const struct blk_ops efi_blk_ops = { + .read = efi_bl_read, + .write = efi_bl_write, +}; + +U_BOOT_DRIVER(efi_block) = { + .name = "efi_block", + .id = UCLASS_BLK, + .ops = &efi_blk_ops, + .plat_auto = sizeof(struct efi_block_plat), +}; + +static int efi_media_bind(struct udevice *dev) +{ + struct efi_media_plat *plat = dev_get_plat(dev); + struct efi_block_plat *blk_plat; + struct udevice *blk; + int ret; + + ret = blk_create_devicef(dev, "efi_block", "blk", IF_TYPE_EFI_MEDIA, + dev_seq(dev), plat->blkio->media->block_size, + plat->blkio->media->last_block, &blk); + if (ret) { + debug("Cannot create block device\n"); + return ret; + } + blk_plat = dev_get_plat(blk); + blk_plat->blkio = plat->blkio; + + return 0; +} + +U_BOOT_DRIVER(efi_media) = { + .name = "efi_media", + .id = UCLASS_EFI_MEDIA, + .bind = efi_media_bind, + .plat_auto = sizeof(struct efi_media_plat), +}; diff --git a/include/efi.h b/include/efi.h index b5835422b95..0ec5913ddd1 100644 --- a/include/efi.h +++ b/include/efi.h @@ -414,6 +414,17 @@ struct efi_priv { void *next_hdr; };
+/* + * EFI attributes of the udevice handled by efi_media driver + * + * @handle: handle of the controller on which this driver is installed + * @blkio: block io protocol proxied by this driver + */ +struct efi_media_plat { + efi_handle_t handle; + struct efi_block_io *blkio; +}; + /* Base address of the EFI image */ extern char image_base[];

On 12/4/21 07:56, Simon Glass wrote:
Add a block driver which handles read/write for EFI block devices. This driver actually already exists ('efi_block') but is not really suitable for use as a real U-Boot driver:
- The operations do not provide a udevice
- The code is designed for running as part of EFI loader, so uses EFI_PRINT() and EFI_CALL().
- The bind method probes the device, which is not permitted
- It uses 'EFI' as its parent device
The new driver is more 'normal', just requiring its platform data be set up in advance.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
(no changes since v2)
Changes in v2:
Drop mention of partitions from the commit message
drivers/block/Kconfig | 10 ++++ drivers/block/Makefile | 1 + drivers/block/efi_blk.c | 115 ++++++++++++++++++++++++++++++++++++++++ include/efi.h | 11 ++++ 4 files changed, 137 insertions(+) create mode 100644 drivers/block/efi_blk.c
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 755fdccb574..8235430497d 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -82,6 +82,16 @@ config EFI_MEDIA_SANDBOX EFI_MEDIA uclass. It does not do anything useful, since sandbox does not actually support running on top of UEFI.
+config EFI_MEDIA_BLK
bool "EFI media block driver"
depends on EFI_APP
default y
help
Enables a block driver for providing access to UEFI devices. This
allows use of block devices detected by the underlying UEFI
implementation. With this it is possible to use filesystems on these
devices, for example.
endif # EFI_MEDIA
config IDE
diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 3778633da1d..b221a7c6eea 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o +obj-$(CONFIG_EFI_MEDIA_BLK) += efi_blk.o diff --git a/drivers/block/efi_blk.c b/drivers/block/efi_blk.c new file mode 100644 index 00000000000..9d25ecbf37f --- /dev/null +++ b/drivers/block/efi_blk.c @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Block driver for EFI devices
- This supports a media driver of UCLASS_EFI with a child UCLASS_BLK
- It allows block-level access to EFI devices made available via EFI boot
- services
- Copyright 2021 Google LLC
- */
+#include <common.h> +#include <blk.h> +#include <dm.h> +#include <efi.h> +#include <efi_api.h>
+struct efi_block_plat {
- struct efi_block_io *blkio;
+};
+/**
- Read from block device
- @dev: device
- @blknr: first block to be read
- @blkcnt: number of blocks to read
- @buffer: output buffer
- Return: number of blocks transferred
- */
+static ulong efi_bl_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
void *buffer)
+{
- struct efi_block_plat *plat = dev_get_plat(dev);
- struct efi_block_io *io = plat->blkio;
- efi_status_t ret;
- log_debug("read buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr,
(ulong)blkcnt);
- ret = io->read_blocks(io, io->media->media_id, blknr,
blkcnt * io->media->block_size, buffer);
- log_debug("ret=%lx (dec %ld)\n", ret & ~EFI_ERROR_MASK,
ret & ~EFI_ERROR_MASK);
- if (ret)
return 0;
- return blkcnt;
+}
+/**
- Write to block device
- @dev: device
- @blknr: first block to be write
- @blkcnt: number of blocks to write
- @buffer: input buffer
- Return: number of blocks transferred
- */
+static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
const void *buffer)
+{
- struct efi_block_plat *plat = dev_get_plat(dev);
- struct efi_block_io *io = plat->blkio;
- efi_status_t ret;
- log_debug("write buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr,
(ulong)blkcnt);
- ret = io->write_blocks(io, io->media->media_id, blknr,
blkcnt * io->media->block_size, (void *)buffer);
- log_debug("ret=%lx (dec %ld)\n", ret & ~EFI_ERROR_MASK,
ret & ~EFI_ERROR_MASK);
- if (ret)
return 0;
- return blkcnt;
+}
+/* Block device driver operators */ +static const struct blk_ops efi_blk_ops = {
- .read = efi_bl_read,
- .write = efi_bl_write,
+};
+U_BOOT_DRIVER(efi_block) = {
- .name = "efi_block",
- .id = UCLASS_BLK,
- .ops = &efi_blk_ops,
- .plat_auto = sizeof(struct efi_block_plat),
+};
+static int efi_media_bind(struct udevice *dev) +{
- struct efi_media_plat *plat = dev_get_plat(dev);
- struct efi_block_plat *blk_plat;
- struct udevice *blk;
- int ret;
- ret = blk_create_devicef(dev, "efi_block", "blk", IF_TYPE_EFI_MEDIA,
dev_seq(dev), plat->blkio->media->block_size,
plat->blkio->media->last_block, &blk);
- if (ret) {
debug("Cannot create block device\n");
return ret;
- }
- blk_plat = dev_get_plat(blk);
- blk_plat->blkio = plat->blkio;
- return 0;
+}
+U_BOOT_DRIVER(efi_media) = {
- .name = "efi_media",
- .id = UCLASS_EFI_MEDIA,
- .bind = efi_media_bind,
- .plat_auto = sizeof(struct efi_media_plat),
+}; diff --git a/include/efi.h b/include/efi.h index b5835422b95..0ec5913ddd1 100644 --- a/include/efi.h +++ b/include/efi.h @@ -414,6 +414,17 @@ struct efi_priv { void *next_hdr; };
+/*
- EFI attributes of the udevice handled by efi_media driver
- @handle: handle of the controller on which this driver is installed
- @blkio: block io protocol proxied by this driver
- */
+struct efi_media_plat {
- efi_handle_t handle;
- struct efi_block_io *blkio;
+};
- /* Base address of the EFI image */ extern char image_base[];

When starting the app, locate all block devices and make them available to U-Boot. This allows listing partitions and accessing files in filesystems.
EFI also has the concept of 'disks', meaning boot media. For now, this is not obviously useful in U-Boot, but add code to at least locate these. This can be expanded later as needed.
Series-changes; 2 - Store device path in struct efi_media_plat - Don't export efi_bind_block() - Only bind devices for media devices, not for partitions - Show devices that are processed - Update documentation
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
doc/develop/uefi/u-boot_on_efi.rst | 4 +- include/efi.h | 6 +- include/efi_api.h | 15 ++ lib/efi/efi_app.c | 223 +++++++++++++++++++++++++++++ 4 files changed, 243 insertions(+), 5 deletions(-)
diff --git a/doc/develop/uefi/u-boot_on_efi.rst b/doc/develop/uefi/u-boot_on_efi.rst index 5f2f850f071..8f81b799072 100644 --- a/doc/develop/uefi/u-boot_on_efi.rst +++ b/doc/develop/uefi/u-boot_on_efi.rst @@ -265,9 +265,7 @@ This work could be extended in a number of ways:
- Figure out how to solve the interrupt problem
-- Add more drivers to the application side (e.g. block devices, USB, - environment access). This would mostly be an academic exercise as a strong - use case is not readily apparent, but it might be fun. +- Add more drivers to the application side (e.g.USB, environment access).
- Avoid turning off boot services in the stub. Instead allow U-Boot to make use of boot services in case it wants to. It is unclear what it might want diff --git a/include/efi.h b/include/efi.h index 0ec5913ddd1..908c5dc6ebd 100644 --- a/include/efi.h +++ b/include/efi.h @@ -419,10 +419,12 @@ struct efi_priv { * * @handle: handle of the controller on which this driver is installed * @blkio: block io protocol proxied by this driver + * @device_path: EFI path to the device */ struct efi_media_plat { - efi_handle_t handle; - struct efi_block_io *blkio; + efi_handle_t handle; + struct efi_block_io *blkio; + struct efi_device_path *device_path; };
/* Base address of the EFI image */ diff --git a/include/efi_api.h b/include/efi_api.h index 80109f012bc..ec9fa89a935 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -2035,4 +2035,19 @@ struct efi_firmware_management_protocol { const u16 *package_version_name); };
+#define EFI_DISK_IO_PROTOCOL_GUID \ + EFI_GUID(0xce345171, 0xba0b, 0x11d2, 0x8e, 0x4f, \ + 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) + +struct efi_disk { + u64 revision; + efi_status_t (EFIAPI *read_disk)(struct efi_disk *this, u32 media_id, + u64 offset, efi_uintn_t buffer_size, + void *buffer); + + efi_status_t (EFIAPI *write_disk)(struct efi_disk *this, u32 media_id, + u64 offset, efi_uintn_t buffer_size, + void *buffer); +}; + #endif diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index f61665686c5..e38d46b15db 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -21,6 +21,9 @@ #include <efi.h> #include <efi_api.h> #include <sysreset.h> +#include <dm/device-internal.h> +#include <dm/lists.h> +#include <dm/root.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -46,6 +49,64 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) return -ENOSYS; }
+/** + * efi_print_str() - Print a UFT-16 string to the U-Boot console + * + * @str: String to print + */ +static void efi_print_str(const u16 *str) +{ + while (*str) { + int ch = *str++; + + if (ch > ' ' && ch < 127) + putc(ch); + } +} + +/** + * efi_bind_block() - bind a new block device to an EFI device + * + * Binds a new top-level EFI_MEDIA device as well as a child block device so + * that the block device can be accessed in U-Boot. + * + * The device can then be accessed using 'part list efi 0', 'fat ls efi 0:1', + * for example, just like any other interface type. + * + * @handle: handle of the controller on which this driver is installed + * @blkio: block io protocol proxied by this driver + * @device_path: EFI device path structure for this + * @len: Length of @device_path in bytes + * @devp: Returns the bound device + * @return 0 if OK, -ve on error + */ +int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio, + struct efi_device_path *device_path, int len, + struct udevice **devp) +{ + struct efi_media_plat plat; + struct udevice *dev; + char name[18]; + int ret; + + plat.handle = handle; + plat.blkio = blkio; + plat.device_path = malloc(device_path->length); + if (!plat.device_path) + return log_msg_ret("path", -ENOMEM); + memcpy(plat.device_path, device_path, device_path->length); + ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media", + &plat, ofnode_null(), &dev); + if (ret) + return log_msg_ret("bind", ret); + + snprintf(name, sizeof(name), "efi_media_%x", dev_seq(dev)); + device_set_name(dev, name); + *devp = dev; + + return 0; +} + static efi_status_t setup_memory(struct efi_priv *priv) { struct efi_boot_services *boot = priv->boot; @@ -105,6 +166,168 @@ static void free_memory(struct efi_priv *priv) global_data_ptr = NULL; }
+static int setup_disks(void) +{ + /* This is not fully implemented yet */ + return 0; + + efi_guid_t efi_disk_guid = EFI_DISK_IO_PROTOCOL_GUID; + struct efi_boot_services *boot = efi_get_boot(); + struct efi_disk *disk; + int ret; + + if (!boot) + return log_msg_ret("sys", -ENOSYS); + ret = boot->locate_protocol(&efi_disk_guid, NULL, (void **)&disk); + if (ret) + return log_msg_ret("prot", -ENOTSUPP); + + return 0; +} + +/** + * devpath_is_partition() - Figure out if a device path is a partition + * + * Checks if a device path refers to a partition on some media device. This + * works by checking for a valid partition number in a hard-driver media device + * as the final component of the device path. + * + * @return true if a partition, false if not (e.g. it might be media which + * contains partitions) + */ +static bool devpath_is_partition(const struct efi_device_path *path) +{ + const struct efi_device_path *p; + bool was_part; + + for (p = path; p->type != DEVICE_PATH_TYPE_END; + p = (void *)p + p->length) { + was_part = false; + if (p->type == DEVICE_PATH_TYPE_MEDIA_DEVICE && + p->sub_type == DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH) { + struct efi_device_path_hard_drive_path *hd = + (void *)path; + + if (hd->partition_number) + was_part = true; + } + } + + return was_part; +} + +/** + * setup_block() - Find all block devices and setup EFI devices for them + * + * Partitions are ignored, since U-Boot has partition handling. Errors with + * particular devices produce a warning but execution continues to try to + * find others. + * + * @return 0 if found, -ENOSYS if there is no boot-services table, -ENOTSUPP + * if a required protocol is not supported + */ +static int setup_block(void) +{ + efi_guid_t efi_blkio_guid = EFI_BLOCK_IO_PROTOCOL_GUID; + efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID; + efi_guid_t efi_pathutil_guid = EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID; + efi_guid_t efi_pathtext_guid = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; + struct efi_boot_services *boot = efi_get_boot(); + struct efi_device_path_utilities_protocol *util; + struct efi_device_path_to_text_protocol *text; + struct efi_device_path *path; + struct efi_block_io *blkio; + efi_uintn_t num_handles; + efi_handle_t *handle; + int ret, i; + + if (!boot) + return log_msg_ret("sys", -ENOSYS); + + /* Find all devices which support the block I/O protocol */ + ret = boot->locate_handle_buffer(BY_PROTOCOL, &efi_blkio_guid, NULL, + &num_handles, &handle); + if (ret) + return log_msg_ret("loc", -ENOTSUPP); + log_debug("Found %d handles:\n", (int)num_handles); + + /* We need to look up the path size and convert it to text */ + ret = boot->locate_protocol(&efi_pathutil_guid, NULL, (void **)&util); + if (ret) + return log_msg_ret("util", -ENOTSUPP); + ret = boot->locate_protocol(&efi_pathtext_guid, NULL, (void **)&text); + if (ret) + return log_msg_ret("text", -ENOTSUPP); + + for (i = 0; i < num_handles; i++) { + struct udevice *dev; + const u16 *name; + bool is_part; + int len; + + ret = boot->handle_protocol(handle[i], &efi_devpath_guid, + (void **)&path); + if (ret) { + log_warning("- devpath %d failed (ret=%d)\n", i, ret); + continue; + } + + ret = boot->handle_protocol(handle[i], &efi_blkio_guid, + (void **)&blkio); + if (ret) { + log_warning("- blkio %d failed (ret=%d)\n", i, ret); + continue; + } + + name = text->convert_device_path_to_text(path, true, false); + is_part = devpath_is_partition(path); + + if (!is_part) { + len = util->get_device_path_size(path); + ret = efi_bind_block(handle[i], blkio, path, len, &dev); + if (ret) { + log_warning("- blkio bind %d failed (ret=%d)\n", + i, ret); + continue; + } + } else { + dev = NULL; + } + + /* + * Show the device name if we created one. Otherwise indicate + * that it is a partition. + * + * We don't seem to have have a way to print unicode on the + * U-Boot console at present, so use our own function. + */ + printf("%2d: %-12s ", i, dev ? dev->name : "<partition>"); + efi_print_str(name); + printf("\n"); + } + boot->free_pool(handle); + + return 0; +} + +int dm_scan_other(bool pre_reloc_only) +{ + if (gd->flags & GD_FLG_RELOC) { + int ret; + + ret = setup_block(); + if (ret) + return ret; + + /* Not needed at present, but could be useful one day? */ + ret = setup_disks(); + if (ret) + return ret; + } + + return 0; +} + /** * efi_main() - Start an EFI image *

On 12/4/21 16:56, Simon Glass wrote:
When starting the app, locate all block devices and make them available to U-Boot. This allows listing partitions and accessing files in filesystems.
EFI also has the concept of 'disks', meaning boot media. For now, this is not obviously useful in U-Boot, but add code to at least locate these. This can be expanded later as needed.
Series-changes; 2
- Store device path in struct efi_media_plat
- Don't export efi_bind_block()
- Only bind devices for media devices, not for partitions
- Show devices that are processed
- Update documentation
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
doc/develop/uefi/u-boot_on_efi.rst | 4 +- include/efi.h | 6 +- include/efi_api.h | 15 ++ lib/efi/efi_app.c | 223 +++++++++++++++++++++++++++++ 4 files changed, 243 insertions(+), 5 deletions(-)
diff --git a/doc/develop/uefi/u-boot_on_efi.rst b/doc/develop/uefi/u-boot_on_efi.rst index 5f2f850f071..8f81b799072 100644 --- a/doc/develop/uefi/u-boot_on_efi.rst +++ b/doc/develop/uefi/u-boot_on_efi.rst @@ -265,9 +265,7 @@ This work could be extended in a number of ways:
- Figure out how to solve the interrupt problem
-- Add more drivers to the application side (e.g. block devices, USB,
- environment access). This would mostly be an academic exercise as a strong
- use case is not readily apparent, but it might be fun.
+- Add more drivers to the application side (e.g.USB, environment access).
- Avoid turning off boot services in the stub. Instead allow U-Boot to make use of boot services in case it wants to. It is unclear what it might want
diff --git a/include/efi.h b/include/efi.h index 0ec5913ddd1..908c5dc6ebd 100644 --- a/include/efi.h +++ b/include/efi.h @@ -419,10 +419,12 @@ struct efi_priv {
- @handle: handle of the controller on which this driver is installed
- @blkio: block io protocol proxied by this driver
*/ struct efi_media_plat {
- @device_path: EFI path to the device
- efi_handle_t handle;
- struct efi_block_io *blkio;
efi_handle_t handle;
struct efi_block_io *blkio;
struct efi_device_path *device_path; };
/* Base address of the EFI image */
diff --git a/include/efi_api.h b/include/efi_api.h index 80109f012bc..ec9fa89a935 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -2035,4 +2035,19 @@ struct efi_firmware_management_protocol { const u16 *package_version_name); };
+#define EFI_DISK_IO_PROTOCOL_GUID \
- EFI_GUID(0xce345171, 0xba0b, 0x11d2, 0x8e, 0x4f, \
0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+struct efi_disk {
- u64 revision;
- efi_status_t (EFIAPI *read_disk)(struct efi_disk *this, u32 media_id,
u64 offset, efi_uintn_t buffer_size,
void *buffer);
- efi_status_t (EFIAPI *write_disk)(struct efi_disk *this, u32 media_id,
u64 offset, efi_uintn_t buffer_size,
void *buffer);
+};
- #endif
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index f61665686c5..e38d46b15db 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -21,6 +21,9 @@ #include <efi.h> #include <efi_api.h> #include <sysreset.h> +#include <dm/device-internal.h> +#include <dm/lists.h> +#include <dm/root.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -46,6 +49,64 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) return -ENOSYS; }
+/**
- efi_print_str() - Print a UFT-16 string to the U-Boot console
- @str: String to print
- */
+static void efi_print_str(const u16 *str) +{
- while (*str) {
int ch = *str++;
if (ch > ' ' && ch < 127)
putc(ch);
- }
+}
+/**
- efi_bind_block() - bind a new block device to an EFI device
- Binds a new top-level EFI_MEDIA device as well as a child block device so
- that the block device can be accessed in U-Boot.
- The device can then be accessed using 'part list efi 0', 'fat ls efi 0:1',
- for example, just like any other interface type.
- @handle: handle of the controller on which this driver is installed
- @blkio: block io protocol proxied by this driver
- @device_path: EFI device path structure for this
- @len: Length of @device_path in bytes
- @devp: Returns the bound device
- @return 0 if OK, -ve on error
- */
+int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio,
struct efi_device_path *device_path, int len,
struct udevice **devp)
+{
- struct efi_media_plat plat;
- struct udevice *dev;
- char name[18];
- int ret;
- plat.handle = handle;
- plat.blkio = blkio;
- plat.device_path = malloc(device_path->length);
- if (!plat.device_path)
return log_msg_ret("path", -ENOMEM);
- memcpy(plat.device_path, device_path, device_path->length);
- ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media",
&plat, ofnode_null(), &dev);
- if (ret)
return log_msg_ret("bind", ret);
- snprintf(name, sizeof(name), "efi_media_%x", dev_seq(dev));
- device_set_name(dev, name);
- *devp = dev;
- return 0;
+}
- static efi_status_t setup_memory(struct efi_priv *priv) { struct efi_boot_services *boot = priv->boot;
@@ -105,6 +166,168 @@ static void free_memory(struct efi_priv *priv) global_data_ptr = NULL; }
Missing function description
+static int setup_disks(void) +{
- /* This is not fully implemented yet */
- return 0;
- efi_guid_t efi_disk_guid = EFI_DISK_IO_PROTOCOL_GUID;
- struct efi_boot_services *boot = efi_get_boot();
- struct efi_disk *disk;
- int ret;
- if (!boot)
return log_msg_ret("sys", -ENOSYS);
- ret = boot->locate_protocol(&efi_disk_guid, NULL, (void **)&disk);
- if (ret)
return log_msg_ret("prot", -ENOTSUPP);
- return 0;
+}
+/**
- devpath_is_partition() - Figure out if a device path is a partition
- Checks if a device path refers to a partition on some media device. This
- works by checking for a valid partition number in a hard-driver media device
- as the final component of the device path.
- @return true if a partition, false if not (e.g. it might be media which
- contains partitions)
Please, stick to the format for function descriptions defined in https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-do...
The description for parameter @path: is missing. @return does not exist. Please, use Return:.
- */
+static bool devpath_is_partition(const struct efi_device_path *path) +{
- const struct efi_device_path *p;
- bool was_part;
- for (p = path; p->type != DEVICE_PATH_TYPE_END;
p = (void *)p + p->length) {
was_part = false;
if (p->type == DEVICE_PATH_TYPE_MEDIA_DEVICE &&
p->sub_type == DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH) {
struct efi_device_path_hard_drive_path *hd =
(void *)path;
if (hd->partition_number)
was_part = true;
}
- }
- return was_part;
+}
+/**
- setup_block() - Find all block devices and setup EFI devices for them
- Partitions are ignored, since U-Boot has partition handling. Errors with
- particular devices produce a warning but execution continues to try to
- find others.
- @return 0 if found, -ENOSYS if there is no boot-services table, -ENOTSUPP
- if a required protocol is not supported
%s/@return/Return:/
- */
+static int setup_block(void) +{
- efi_guid_t efi_blkio_guid = EFI_BLOCK_IO_PROTOCOL_GUID;
- efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID;
- efi_guid_t efi_pathutil_guid = EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID;
- efi_guid_t efi_pathtext_guid = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;
- struct efi_boot_services *boot = efi_get_boot();
- struct efi_device_path_utilities_protocol *util;
- struct efi_device_path_to_text_protocol *text;
- struct efi_device_path *path;
- struct efi_block_io *blkio;
- efi_uintn_t num_handles;
- efi_handle_t *handle;
- int ret, i;
- if (!boot)
return log_msg_ret("sys", -ENOSYS);
- /* Find all devices which support the block I/O protocol */
- ret = boot->locate_handle_buffer(BY_PROTOCOL, &efi_blkio_guid, NULL,
&num_handles, &handle);
- if (ret)
return log_msg_ret("loc", -ENOTSUPP);
- log_debug("Found %d handles:\n", (int)num_handles);
- /* We need to look up the path size and convert it to text */
- ret = boot->locate_protocol(&efi_pathutil_guid, NULL, (void **)&util);
- if (ret)
return log_msg_ret("util", -ENOTSUPP);
- ret = boot->locate_protocol(&efi_pathtext_guid, NULL, (void **)&text);
- if (ret)
return log_msg_ret("text", -ENOTSUPP);
- for (i = 0; i < num_handles; i++) {
struct udevice *dev;
const u16 *name;
bool is_part;
int len;
ret = boot->handle_protocol(handle[i], &efi_devpath_guid,
(void **)&path);
if (ret) {
log_warning("- devpath %d failed (ret=%d)\n", i, ret);
continue;
}
ret = boot->handle_protocol(handle[i], &efi_blkio_guid,
(void **)&blkio);
if (ret) {
log_warning("- blkio %d failed (ret=%d)\n", i, ret);
continue;
}
name = text->convert_device_path_to_text(path, true, false);
is_part = devpath_is_partition(path);
if (!is_part) {
len = util->get_device_path_size(path);
ret = efi_bind_block(handle[i], blkio, path, len, &dev);
if (ret) {
log_warning("- blkio bind %d failed (ret=%d)\n",
i, ret);
continue;
}
} else {
dev = NULL;
}
/*
* Show the device name if we created one. Otherwise indicate
* that it is a partition.
*
* We don't seem to have have a way to print unicode on the
* U-Boot console at present, so use our own function.
*/
printf("%2d: %-12s ", i, dev ? dev->name : "<partition>");
efi_print_str(name);
printf("\n");
- }
- boot->free_pool(handle);
- return 0;
+}
Missing function description.
Best regards
Heinrich
+int dm_scan_other(bool pre_reloc_only) +{
- if (gd->flags & GD_FLG_RELOC) {
int ret;
ret = setup_block();
if (ret)
return ret;
/* Not needed at present, but could be useful one day? */
ret = setup_disks();
if (ret)
return ret;
- }
- return 0;
+}
- /**
- efi_main() - Start an EFI image

On 12/4/21 10:43, Heinrich Schuchardt wrote:
On 12/4/21 16:56, Simon Glass wrote:
When starting the app, locate all block devices and make them available to U-Boot. This allows listing partitions and accessing files in filesystems.
EFI also has the concept of 'disks', meaning boot media. For now, this is not obviously useful in U-Boot, but add code to at least locate these. This can be expanded later as needed.
Series-changes; 2
- Store device path in struct efi_media_plat
- Don't export efi_bind_block()
- Only bind devices for media devices, not for partitions
- Show devices that are processed
- Update documentation
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
doc/develop/uefi/u-boot_on_efi.rst |  4 +-  include/efi.h                     |  6 +-  include/efi_api.h                 | 15 ++  lib/efi/efi_app.c                 | 223 +++++++++++++++++++++++++++++  4 files changed, 243 insertions(+), 5 deletions(-)
diff --git a/doc/develop/uefi/u-boot_on_efi.rst b/doc/develop/uefi/u-boot_on_efi.rst index 5f2f850f071..8f81b799072 100644 --- a/doc/develop/uefi/u-boot_on_efi.rst +++ b/doc/develop/uefi/u-boot_on_efi.rst @@ -265,9 +265,7 @@ This work could be extended in a number of ways:
- Figure out how to solve the interrupt problem
-- Add more drivers to the application side (e.g. block devices, USB, -Â environment access). This would mostly be an academic exercise as a strong -Â use case is not readily apparent, but it might be fun. +- Add more drivers to the application side (e.g.USB, environment access).
- Avoid turning off boot services in the stub. Instead allow U-Boot to make    use of boot services in case it wants to. It is unclear what it might want diff --git a/include/efi.h b/include/efi.h index 0ec5913ddd1..908c5dc6ebd 100644 --- a/include/efi.h +++ b/include/efi.h @@ -419,10 +419,12 @@ struct efi_priv {   *   * @handle: handle of the controller on which this driver is installed   * @blkio: block io protocol proxied by this driver
- @device_path: EFI path to the device
*/  struct efi_media_plat { -   efi_handle_t       handle; -   struct efi_block_io   *blkio; +   efi_handle_t handle; +   struct efi_block_io *blkio; +   struct efi_device_path *device_path;  };
/* Base address of the EFI image */ diff --git a/include/efi_api.h b/include/efi_api.h index 80109f012bc..ec9fa89a935 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -2035,4 +2035,19 @@ struct efi_firmware_management_protocol { Â Â Â Â Â Â Â Â Â Â Â Â Â const u16 *package_version_name); Â };
+#define EFI_DISK_IO_PROTOCOL_GUIDÂ Â Â \ +Â Â Â EFI_GUID(0xce345171, 0xba0b, 0x11d2, 0x8e, 0x4f, \ +Â Â Â Â Â Â Â Â 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+struct efi_disk { +Â Â Â u64 revision; +Â Â Â efi_status_t (EFIAPI *read_disk)(struct efi_disk *this, u32 media_id, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â u64 offset, efi_uintn_t buffer_size, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â void *buffer);
+Â Â Â efi_status_t (EFIAPI *write_disk)(struct efi_disk *this, u32 media_id, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â u64 offset, efi_uintn_t buffer_size, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â void *buffer); +};
#endif diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index f61665686c5..e38d46b15db 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -21,6 +21,9 @@ Â #include <efi.h> Â #include <efi_api.h> Â #include <sysreset.h> +#include <dm/device-internal.h> +#include <dm/lists.h> +#include <dm/root.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -46,6 +49,64 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) Â Â Â Â Â return -ENOSYS; Â }
+/**
- efi_print_str() - Print a UFT-16 string to the U-Boot console
- @str: String to print
- */
+static void efi_print_str(const u16 *str) +{ +Â Â Â while (*str) { +Â Â Â Â Â Â Â int ch = *str++;
+Â Â Â Â Â Â Â if (ch > ' ' && ch < 127) +Â Â Â Â Â Â Â Â Â Â Â putc(ch); +Â Â Â } +}
+/**
- efi_bind_block() - bind a new block device to an EFI device
- Binds a new top-level EFI_MEDIA device as well as a child block
device so
- that the block device can be accessed in U-Boot.
- The device can then be accessed using 'part list efi 0', 'fat ls
efi 0:1',
- for example, just like any other interface type.
- @handle: handle of the controller on which this driver is installed
- @blkio: block io protocol proxied by this driver
- @device_path: EFI device path structure for this
- @len: Length of @device_path in bytes
- @devp: Returns the bound device
- @return 0 if OK, -ve on error
- */
+int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio, +Â Â Â Â Â Â Â Â Â Â struct efi_device_path *device_path, int len, +Â Â Â Â Â Â Â Â Â Â struct udevice **devp) +{ +Â Â Â struct efi_media_plat plat; +Â Â Â struct udevice *dev; +Â Â Â char name[18]; +Â Â Â int ret;
+Â Â Â plat.handle = handle; +Â Â Â plat.blkio = blkio; +Â Â Â plat.device_path = malloc(device_path->length); +Â Â Â if (!plat.device_path) +Â Â Â Â Â Â Â return log_msg_ret("path", -ENOMEM); +Â Â Â memcpy(plat.device_path, device_path, device_path->length); +Â Â Â ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media", +Â Â Â Â Â Â Â Â Â Â Â Â Â &plat, ofnode_null(), &dev); +Â Â Â if (ret) +Â Â Â Â Â Â Â return log_msg_ret("bind", ret);
+Â Â Â snprintf(name, sizeof(name), "efi_media_%x", dev_seq(dev)); +Â Â Â device_set_name(dev, name); +Â Â Â *devp = dev;
+Â Â Â return 0; +}
static efi_status_t setup_memory(struct efi_priv *priv) Â { Â Â Â Â Â struct efi_boot_services *boot = priv->boot; @@ -105,6 +166,168 @@ static void free_memory(struct efi_priv *priv) Â Â Â Â Â global_data_ptr = NULL; Â }
Missing function description
+static int setup_disks(void) +{ +Â Â Â /* This is not fully implemented yet */
see hint below.
+Â Â Â return 0;
+Â Â Â efi_guid_t efi_disk_guid = EFI_DISK_IO_PROTOCOL_GUID;
U-Boot does not implement the EFI_DISK_IO_PROTOCOL. So you will not be able to run U-Boot as an EFI app loaded by U-Boot which might be nice for testing.
The more basic protocol is the EFI_BLOCK_IO_PROTOCOL. The difference is that the EFI_BLOCK_IO_PROTOCOL requires using a properly aligned buffer.
+Â Â Â struct efi_boot_services *boot = efi_get_boot(); +Â Â Â struct efi_disk *disk; +Â Â Â int ret;
+Â Â Â if (!boot) +Â Â Â Â Â Â Â return log_msg_ret("sys", -ENOSYS); +Â Â Â ret = boot->locate_protocol(&efi_disk_guid, NULL, (void **)&disk);
If you want to find all handles implementing a protocol, you can use EFI_BOOT_SERVICES.LocateHandleBuffer() with SearchType ByProtocol.
I guess we should add this patch to U-Boot once it is completed.
Best regards
Heinrich
+Â Â Â if (ret) +Â Â Â Â Â Â Â return log_msg_ret("prot", -ENOTSUPP);
+Â Â Â return 0; +}
+/**
- devpath_is_partition() - Figure out if a device path is a partition
- Checks if a device path refers to a partition on some media
device. This
- works by checking for a valid partition number in a hard-driver
media device
- as the final component of the device path.
- @return true if a partition, false if not (e.g. it might be media
which
- *Â Â Â contains partitions)
Please, stick to the format for function descriptions defined in https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-do...
The description for parameter @path: is missing. @return does not exist. Please, use Return:.
- */
+static bool devpath_is_partition(const struct efi_device_path *path) +{ +Â Â Â const struct efi_device_path *p; +Â Â Â bool was_part;
+Â Â Â for (p = path; p->type != DEVICE_PATH_TYPE_END; +Â Â Â Â Â Â Â Â p = (void *)p + p->length) { +Â Â Â Â Â Â Â was_part = false; +Â Â Â Â Â Â Â if (p->type == DEVICE_PATH_TYPE_MEDIA_DEVICE && +Â Â Â Â Â Â Â Â Â Â Â p->sub_type == DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH) { +Â Â Â Â Â Â Â Â Â Â Â struct efi_device_path_hard_drive_path *hd = +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (void *)path;
+Â Â Â Â Â Â Â Â Â Â Â if (hd->partition_number) +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â was_part = true; +Â Â Â Â Â Â Â } +Â Â Â }
+Â Â Â return was_part; +}
+/**
- setup_block() - Find all block devices and setup EFI devices for them
- Partitions are ignored, since U-Boot has partition handling.
Errors with
- particular devices produce a warning but execution continues to
try to
- find others.
- @return 0 if found, -ENOSYS if there is no boot-services table,
-ENOTSUPP
- *Â Â Â if a required protocol is not supported
%s/@return/Return:/
- */
+static int setup_block(void) +{ +Â Â Â efi_guid_t efi_blkio_guid = EFI_BLOCK_IO_PROTOCOL_GUID; +Â Â Â efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID; +Â Â Â efi_guid_t efi_pathutil_guid = EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID; +Â Â Â efi_guid_t efi_pathtext_guid = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID; +Â Â Â struct efi_boot_services *boot = efi_get_boot(); +Â Â Â struct efi_device_path_utilities_protocol *util; +Â Â Â struct efi_device_path_to_text_protocol *text; +Â Â Â struct efi_device_path *path; +Â Â Â struct efi_block_io *blkio; +Â Â Â efi_uintn_t num_handles; +Â Â Â efi_handle_t *handle; +Â Â Â int ret, i;
+Â Â Â if (!boot) +Â Â Â Â Â Â Â return log_msg_ret("sys", -ENOSYS);
+Â Â Â /* Find all devices which support the block I/O protocol */ +Â Â Â ret = boot->locate_handle_buffer(BY_PROTOCOL, &efi_blkio_guid, NULL, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &num_handles, &handle); +Â Â Â if (ret) +Â Â Â Â Â Â Â return log_msg_ret("loc", -ENOTSUPP); +Â Â Â log_debug("Found %d handles:\n", (int)num_handles);
+Â Â Â /* We need to look up the path size and convert it to text */ +Â Â Â ret = boot->locate_protocol(&efi_pathutil_guid, NULL, (void **)&util); +Â Â Â if (ret) +Â Â Â Â Â Â Â return log_msg_ret("util", -ENOTSUPP); +Â Â Â ret = boot->locate_protocol(&efi_pathtext_guid, NULL, (void **)&text); +Â Â Â if (ret) +Â Â Â Â Â Â Â return log_msg_ret("text", -ENOTSUPP);
+Â Â Â for (i = 0; i < num_handles; i++) { +Â Â Â Â Â Â Â struct udevice *dev; +Â Â Â Â Â Â Â const u16 *name; +Â Â Â Â Â Â Â bool is_part; +Â Â Â Â Â Â Â int len;
+Â Â Â Â Â Â Â ret = boot->handle_protocol(handle[i], &efi_devpath_guid, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (void **)&path); +Â Â Â Â Â Â Â if (ret) { +Â Â Â Â Â Â Â Â Â Â Â log_warning("- devpath %d failed (ret=%d)\n", i, ret); +Â Â Â Â Â Â Â Â Â Â Â continue; +Â Â Â Â Â Â Â }
+Â Â Â Â Â Â Â ret = boot->handle_protocol(handle[i], &efi_blkio_guid, +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â (void **)&blkio); +Â Â Â Â Â Â Â if (ret) { +Â Â Â Â Â Â Â Â Â Â Â log_warning("- blkio %d failed (ret=%d)\n", i, ret); +Â Â Â Â Â Â Â Â Â Â Â continue; +Â Â Â Â Â Â Â }
+Â Â Â Â Â Â Â name = text->convert_device_path_to_text(path, true, false); +Â Â Â Â Â Â Â is_part = devpath_is_partition(path);
+Â Â Â Â Â Â Â if (!is_part) { +Â Â Â Â Â Â Â Â Â Â Â len = util->get_device_path_size(path); +Â Â Â Â Â Â Â Â Â Â Â ret = efi_bind_block(handle[i], blkio, path, len, &dev); +Â Â Â Â Â Â Â Â Â Â Â if (ret) { +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â log_warning("- blkio bind %d failed (ret=%d)\n", +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â i, ret); +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue; +Â Â Â Â Â Â Â Â Â Â Â } +Â Â Â Â Â Â Â } else { +Â Â Â Â Â Â Â Â Â Â Â dev = NULL; +Â Â Â Â Â Â Â }
+Â Â Â Â Â Â Â /* +Â Â Â Â Â Â Â Â * Show the device name if we created one. Otherwise indicate +Â Â Â Â Â Â Â Â * that it is a partition. +Â Â Â Â Â Â Â Â * +Â Â Â Â Â Â Â Â * We don't seem to have have a way to print unicode on the +Â Â Â Â Â Â Â Â * U-Boot console at present, so use our own function. +Â Â Â Â Â Â Â Â */ +Â Â Â Â Â Â Â printf("%2d: %-12s ", i, dev ? dev->name : "<partition>"); +Â Â Â Â Â Â Â efi_print_str(name); +Â Â Â Â Â Â Â printf("\n"); +Â Â Â } +Â Â Â boot->free_pool(handle);
+Â Â Â return 0; +}
Missing function description.
Best regards
Heinrich
+int dm_scan_other(bool pre_reloc_only) +{ +Â Â Â if (gd->flags & GD_FLG_RELOC) { +Â Â Â Â Â Â Â int ret;
+Â Â Â Â Â Â Â ret = setup_block(); +Â Â Â Â Â Â Â if (ret) +Â Â Â Â Â Â Â Â Â Â Â return ret;
+Â Â Â Â Â Â Â /* Not needed at present, but could be useful one day? */ +Â Â Â Â Â Â Â ret = setup_disks(); +Â Â Â Â Â Â Â if (ret) +Â Â Â Â Â Â Â Â Â Â Â return ret; +Â Â Â }
+Â Â Â return 0; +}
/**   * efi_main() - Start an EFI image   *

Hi Heinrich,
On Thu, 9 Dec 2021 at 12:23, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 10:43, Heinrich Schuchardt wrote:
On 12/4/21 16:56, Simon Glass wrote:
When starting the app, locate all block devices and make them available to U-Boot. This allows listing partitions and accessing files in filesystems.
EFI also has the concept of 'disks', meaning boot media. For now, this is not obviously useful in U-Boot, but add code to at least locate these. This can be expanded later as needed.
Series-changes; 2
- Store device path in struct efi_media_plat
- Don't export efi_bind_block()
- Only bind devices for media devices, not for partitions
- Show devices that are processed
- Update documentation
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
doc/develop/uefi/u-boot_on_efi.rst | 4 +- include/efi.h | 6 +- include/efi_api.h | 15 ++ lib/efi/efi_app.c | 223 +++++++++++++++++++++++++++++ 4 files changed, 243 insertions(+), 5 deletions(-)
[..]
+static int setup_disks(void) +{
- /* This is not fully implemented yet */
see hint below.
- return 0;
- efi_guid_t efi_disk_guid = EFI_DISK_IO_PROTOCOL_GUID;
U-Boot does not implement the EFI_DISK_IO_PROTOCOL. So you will not be able to run U-Boot as an EFI app loaded by U-Boot which might be nice for testing.
The more basic protocol is the EFI_BLOCK_IO_PROTOCOL. The difference is that the EFI_BLOCK_IO_PROTOCOL requires using a properly aligned buffer.
- struct efi_boot_services *boot = efi_get_boot();
- struct efi_disk *disk;
- int ret;
- if (!boot)
return log_msg_ret("sys", -ENOSYS);
- ret = boot->locate_protocol(&efi_disk_guid, NULL, (void **)&disk);
If you want to find all handles implementing a protocol, you can use EFI_BOOT_SERVICES.LocateHandleBuffer() with SearchType ByProtocol.
I guess we should add this patch to U-Boot once it is completed.
OK, let's drop this code then.
[..] Regards, Simon

At present only the backspace key is supported in U-Boot, when running as an EFI app. Add support for arrows, home and end as well, to make the CLI more friendly.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
drivers/serial/serial_efi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c index 33ddbd6080c..0067576389d 100644 --- a/drivers/serial/serial_efi.c +++ b/drivers/serial/serial_efi.c @@ -24,6 +24,9 @@ struct serial_efi_priv { bool have_key; };
+/* Convert a lower-case character to its ctrl-char equivalent */ +#define CTL_CH(c) ((c) - 'a' + 1) + int serial_efi_setbrg(struct udevice *dev, int baudrate) { return 0; @@ -49,6 +52,7 @@ static int serial_efi_get_key(struct serial_efi_priv *priv) static int serial_efi_getc(struct udevice *dev) { struct serial_efi_priv *priv = dev_get_priv(dev); + char conv_scan[10] = {0, 'p', 'n', 'f', 'b', 'a', 'e', 0, 8}; int ret, ch;
ret = serial_efi_get_key(priv); @@ -63,8 +67,11 @@ static int serial_efi_getc(struct udevice *dev) * key scan code of 8. Handle this so that backspace works correctly * in the U-Boot command line. */ - if (!ch && priv->key.scan_code == 8) - ch = 8; + if (!ch && priv->key.scan_code < sizeof(conv_scan)) { + ch = conv_scan[priv->key.scan_code]; + if (ch >= 'a') + ch -= 'a' - 1; + } debug(" [%x %x %x] ", ch, priv->key.unicode_char, priv->key.scan_code);
return ch;

On 12/4/21 07:56, Simon Glass wrote:
At present only the backspace key is supported in U-Boot, when running as an EFI app. Add support for arrows, home and end as well, to make the CLI more friendly.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
drivers/serial/serial_efi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c index 33ddbd6080c..0067576389d 100644 --- a/drivers/serial/serial_efi.c +++ b/drivers/serial/serial_efi.c @@ -24,6 +24,9 @@ struct serial_efi_priv { bool have_key; };
+/* Convert a lower-case character to its ctrl-char equivalent */ +#define CTL_CH(c) ((c) - 'a' + 1)
- int serial_efi_setbrg(struct udevice *dev, int baudrate) { return 0;
@@ -49,6 +52,7 @@ static int serial_efi_get_key(struct serial_efi_priv *priv) static int serial_efi_getc(struct udevice *dev) { struct serial_efi_priv *priv = dev_get_priv(dev);
char conv_scan[10] = {0, 'p', 'n', 'f', 'b', 'a', 'e', 0, 8}; int ret, ch;
ret = serial_efi_get_key(priv);
@@ -63,8 +67,11 @@ static int serial_efi_getc(struct udevice *dev) * key scan code of 8. Handle this so that backspace works correctly * in the U-Boot command line. */
- if (!ch && priv->key.scan_code == 8)
ch = 8;
- if (!ch && priv->key.scan_code < sizeof(conv_scan)) {
ch = conv_scan[priv->key.scan_code];
if (ch >= 'a')
ch -= 'a' - 1;
This is not what a serial console would use in Linux. Just try U-Boot's conitrace command. If you use the following, we get compatibility with the rest of U-Boot:
1b 5b 32 7e - insert 1b 5b 33 7e - delete 1b 5b 35 7e - page up 1b 5b 36 7e - page down 1b 5b 41 - up 1b 5b 42 - down 1b 5b 43 - right 1b 5b 44 - left 1b 5b 46 - end 1b 5b 48 - home
Best regards
Heinrich
} debug(" [%x %x %x] ", ch, priv->key.unicode_char, priv->key.scan_code);
return ch;

Hi Heinrich,
On Thu, 9 Dec 2021 at 12:44, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
At present only the backspace key is supported in U-Boot, when running as an EFI app. Add support for arrows, home and end as well, to make the CLI more friendly.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
drivers/serial/serial_efi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c index 33ddbd6080c..0067576389d 100644 --- a/drivers/serial/serial_efi.c +++ b/drivers/serial/serial_efi.c @@ -24,6 +24,9 @@ struct serial_efi_priv { bool have_key; };
+/* Convert a lower-case character to its ctrl-char equivalent */ +#define CTL_CH(c) ((c) - 'a' + 1)
- int serial_efi_setbrg(struct udevice *dev, int baudrate) { return 0;
@@ -49,6 +52,7 @@ static int serial_efi_get_key(struct serial_efi_priv *priv) static int serial_efi_getc(struct udevice *dev) { struct serial_efi_priv *priv = dev_get_priv(dev);
char conv_scan[10] = {0, 'p', 'n', 'f', 'b', 'a', 'e', 0, 8}; int ret, ch; ret = serial_efi_get_key(priv);
@@ -63,8 +67,11 @@ static int serial_efi_getc(struct udevice *dev) * key scan code of 8. Handle this so that backspace works correctly * in the U-Boot command line. */
if (!ch && priv->key.scan_code == 8)
ch = 8;
if (!ch && priv->key.scan_code < sizeof(conv_scan)) {
ch = conv_scan[priv->key.scan_code];
if (ch >= 'a')
ch -= 'a' - 1;
This is not what a serial console would use in Linux. Just try U-Boot's conitrace command. If you use the following, we get compatibility with the rest of U-Boot:
1b 5b 32 7e - insert 1b 5b 33 7e - delete 1b 5b 35 7e - page up 1b 5b 36 7e - page down 1b 5b 41 - up 1b 5b 42 - down 1b 5b 43 - right 1b 5b 44 - left 1b 5b 46 - end 1b 5b 48 - home
That's more complicated since it requires the serial driver to convert one character into multiple. We can always add it later if needed. But this does command-line history, etc. working in the app.
Regards, Simon

On 12/17/21 17:37, Simon Glass wrote:
Hi Heinrich,
On Thu, 9 Dec 2021 at 12:44, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
At present only the backspace key is supported in U-Boot, when running as an EFI app. Add support for arrows, home and end as well, to make the CLI more friendly.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
drivers/serial/serial_efi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c index 33ddbd6080c..0067576389d 100644 --- a/drivers/serial/serial_efi.c +++ b/drivers/serial/serial_efi.c @@ -24,6 +24,9 @@ struct serial_efi_priv { bool have_key; };
+/* Convert a lower-case character to its ctrl-char equivalent */ +#define CTL_CH(c) ((c) - 'a' + 1)
- int serial_efi_setbrg(struct udevice *dev, int baudrate) { return 0;
@@ -49,6 +52,7 @@ static int serial_efi_get_key(struct serial_efi_priv *priv) static int serial_efi_getc(struct udevice *dev) { struct serial_efi_priv *priv = dev_get_priv(dev);
char conv_scan[10] = {0, 'p', 'n', 'f', 'b', 'a', 'e', 0, 8}; int ret, ch; ret = serial_efi_get_key(priv);
@@ -63,8 +67,11 @@ static int serial_efi_getc(struct udevice *dev) * key scan code of 8. Handle this so that backspace works correctly * in the U-Boot command line. */
if (!ch && priv->key.scan_code == 8)
ch = 8;
if (!ch && priv->key.scan_code < sizeof(conv_scan)) {
ch = conv_scan[priv->key.scan_code];
if (ch >= 'a')
ch -= 'a' - 1;
This is not what a serial console would use in Linux. Just try U-Boot's conitrace command. If you use the following, we get compatibility with the rest of U-Boot:
1b 5b 32 7e - insert 1b 5b 33 7e - delete 1b 5b 35 7e - page up 1b 5b 36 7e - page down 1b 5b 41 - up 1b 5b 42 - down 1b 5b 43 - right 1b 5b 44 - left 1b 5b 46 - end 1b 5b 48 - home
That's more complicated since it requires the serial driver to convert one character into multiple. We can always add it later if needed. But this does command-line history, etc. working in the app.
Yes you will need a ring buffer as in the USB keyboard driver.
lib/efi_loader/efi_console.c only understands the multi-byte sequences. I do not know if there are other places which have the same limitation.
Best regards
Heinrich
Regards, Simon

Hi Heinrich,
On Fri, 17 Dec 2021 at 10:14, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/17/21 17:37, Simon Glass wrote:
Hi Heinrich,
On Thu, 9 Dec 2021 at 12:44, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
At present only the backspace key is supported in U-Boot, when running as an EFI app. Add support for arrows, home and end as well, to make the CLI more friendly.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
drivers/serial/serial_efi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c index 33ddbd6080c..0067576389d 100644 --- a/drivers/serial/serial_efi.c +++ b/drivers/serial/serial_efi.c @@ -24,6 +24,9 @@ struct serial_efi_priv { bool have_key; };
+/* Convert a lower-case character to its ctrl-char equivalent */ +#define CTL_CH(c) ((c) - 'a' + 1)
- int serial_efi_setbrg(struct udevice *dev, int baudrate) { return 0;
@@ -49,6 +52,7 @@ static int serial_efi_get_key(struct serial_efi_priv *priv) static int serial_efi_getc(struct udevice *dev) { struct serial_efi_priv *priv = dev_get_priv(dev);
char conv_scan[10] = {0, 'p', 'n', 'f', 'b', 'a', 'e', 0, 8}; int ret, ch; ret = serial_efi_get_key(priv);
@@ -63,8 +67,11 @@ static int serial_efi_getc(struct udevice *dev) * key scan code of 8. Handle this so that backspace works correctly * in the U-Boot command line. */
if (!ch && priv->key.scan_code == 8)
ch = 8;
if (!ch && priv->key.scan_code < sizeof(conv_scan)) {
ch = conv_scan[priv->key.scan_code];
if (ch >= 'a')
ch -= 'a' - 1;
This is not what a serial console would use in Linux. Just try U-Boot's conitrace command. If you use the following, we get compatibility with the rest of U-Boot:
1b 5b 32 7e - insert 1b 5b 33 7e - delete 1b 5b 35 7e - page up 1b 5b 36 7e - page down 1b 5b 41 - up 1b 5b 42 - down 1b 5b 43 - right 1b 5b 44 - left 1b 5b 46 - end 1b 5b 48 - home
That's more complicated since it requires the serial driver to convert one character into multiple. We can always add it later if needed. But this does command-line history, etc. working in the app.
Yes you will need a ring buffer as in the USB keyboard driver.
lib/efi_loader/efi_console.c only understands the multi-byte sequences. I do not know if there are other places which have the same limitation.
The ones I am using here are the official U-Boot codes. Perhaps EFI_LOADER should be fixed, but this doesn't matter for the EFI app.
In any case, this work can be done later if needed.
Regards, Simon

Typically the bloblist is positioned at a fixed address in memory until relocation. This is convenient when it is set up in SPL or before relocation.
But for EFI we want to set it up only when U-Boot proper is running. Add a way to allocate it using malloc() and update the documentation to cover this aspect of bloblist.
Note there are no tests of this feature at present, nor any direct testing of bloblist_init().
This can be added, e.g. by making this option controllable at runtime.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/Kconfig | 15 +++++++++++++-- common/bloblist.c | 16 ++++++++++++++-- common/board_f.c | 8 +++++++- doc/develop/bloblist.rst | 16 ++++++++++++++++ 4 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index 176fda9449e..50ac4331f5c 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -727,6 +727,8 @@ config TPL_BLOBLIST This enables a bloblist in TPL. The bloblist is set up in TPL and passed to SPL and U-Boot proper.
+if BLOBLIST + config BLOBLIST_SIZE hex "Size of bloblist" depends on BLOBLIST @@ -737,17 +739,24 @@ config BLOBLIST_SIZE is set up in the first part of U-Boot to run (TPL, SPL or U-Boot proper), and this sane bloblist is used for subsequent stages.
+config BLOBLIST_ALLOC + bool "Allocate bloblist" + help + Allocate the bloblist using malloc(). This avoids the need to + specify a fixed address on systems where this is unknown or can + change at runtime. + config BLOBLIST_ADDR hex "Address of bloblist" - depends on BLOBLIST default 0xc000 if SANDBOX help Sets the address of the bloblist, set up by the first part of U-Boot which runs. Subsequent U-Boot stages typically use the same address.
+ This is not used if BLOBLIST_ALLOC is selected. + config BLOBLIST_SIZE_RELOC hex "Size of bloblist after relocation" - depends on BLOBLIST default BLOBLIST_SIZE help Sets the size of the bloblist in bytes after relocation. Since U-Boot @@ -755,6 +764,8 @@ config BLOBLIST_SIZE_RELOC size than the one set up by SPL. This bloblist is set up during the relocation process.
+endif # BLOBLIST + endmenu
source "common/spl/Kconfig" diff --git a/common/bloblist.c b/common/bloblist.c index 1290fff8504..01b04103d91 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -7,6 +7,7 @@ #include <common.h> #include <bloblist.h> #include <log.h> +#include <malloc.h> #include <mapmem.h> #include <spl.h> #include <asm/global_data.h> @@ -416,10 +417,21 @@ int bloblist_init(void) ret = bloblist_check(CONFIG_BLOBLIST_ADDR, CONFIG_BLOBLIST_SIZE); if (ret) { + ulong addr; + log(LOGC_BLOBLIST, expected ? LOGL_WARNING : LOGL_DEBUG, "Existing bloblist not found: creating new bloblist\n"); - ret = bloblist_new(CONFIG_BLOBLIST_ADDR, CONFIG_BLOBLIST_SIZE, - 0); + if (IS_ENABLED(CONFIG_BLOBLIST_ALLOC)) { + void *ptr = memalign(BLOBLIST_ALIGN, + CONFIG_BLOBLIST_SIZE); + + if (!ptr) + return log_msg_ret("alloc", -ENOMEM); + addr = map_to_sysmem(ptr); + } else { + addr = CONFIG_BLOBLIST_ADDR; + } + ret = bloblist_new(addr, CONFIG_BLOBLIST_SIZE, 0); } else { log(LOGC_BLOBLIST, LOGL_DEBUG, "Found existing bloblist\n"); } diff --git a/common/board_f.c b/common/board_f.c index f7ea7c7a1e4..dd69c3b6b77 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -655,8 +655,14 @@ static int reloc_bootstage(void) static int reloc_bloblist(void) { #ifdef CONFIG_BLOBLIST - if (gd->flags & GD_FLG_SKIP_RELOC) + /* + * Relocate only if we are supposed to send it + */ + if ((gd->flags & GD_FLG_SKIP_RELOC) && + CONFIG_BLOBLIST_SIZE == CONFIG_BLOBLIST_SIZE_RELOC) { + debug("Not relocating bloblist\n"); return 0; + } if (gd->new_bloblist) { int size = CONFIG_BLOBLIST_SIZE;
diff --git a/doc/develop/bloblist.rst b/doc/develop/bloblist.rst index 317ebc4919d..47274cf8e26 100644 --- a/doc/develop/bloblist.rst +++ b/doc/develop/bloblist.rst @@ -59,6 +59,22 @@ Bloblist provides a fairly simple API which allows blobs to be created and found. All access is via the blob's tag. Blob records are zeroed when added.
+Placing the bloblist +-------------------- + +The bloblist is typically positioned at a fixed address by TPL, or SPL. This +is controlled by `CONFIG_BLOBLIST_ADDR`. But in some cases it is preferable to +allocate the bloblist in the malloc() space. Use the `CONFIG_BLOBLIST_ALLOC` +option to enable this. + +The bloblist is automatically relocated as part of U-Boot relocation. Sometimes +it is useful to expand the bloblist in U-Boot proper, since it may want to add +information for use by Linux. Note that this does not mean that Linux needs to +know anything about the bloblist format, just that it is convenient to use +bloblist to place things contiguously in memory. Set +`CONFIG_BLOBLIST_SIZE_RELOC` to define the expanded size, if needed. + + Finishing the bloblist ----------------------

At present this is disabled, but it should work so long as the kernel does not need EFI services. Enable it and add a note about remaining work.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Update documentation
arch/x86/lib/bootm.c | 11 +++++++---- doc/develop/uefi/u-boot_on_efi.rst | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c index 667e5e689e3..57cba5c65d3 100644 --- a/arch/x86/lib/bootm.c +++ b/arch/x86/lib/bootm.c @@ -179,10 +179,14 @@ int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit) * U-Boot is setting them up that way for itself in * arch/i386/cpu/cpu.c. * - * Note that we cannot currently boot a kernel while running as - * an EFI application. Please use the payload option for that. + * Note: this is incomplete for EFI kernels! + * + * This can boot a kernel while running as an EFI application, + * but if the kernel requires EFI support then that support needs + * to be enabled first (see EFI_LOADER). Also the EFI information + * must enabled with setup_efi_info(). See setup_zimage() for + * how this is done with the stub. */ -#ifndef CONFIG_EFI_APP __asm__ __volatile__ ( "movl $0, %%ebp\n" "cli\n" @@ -191,7 +195,6 @@ int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit) [boot_params] "S"(setup_base), "b"(0), "D"(0) ); -#endif }
/* We can't get to here */ diff --git a/doc/develop/uefi/u-boot_on_efi.rst b/doc/develop/uefi/u-boot_on_efi.rst index 8f81b799072..acad6397e81 100644 --- a/doc/develop/uefi/u-boot_on_efi.rst +++ b/doc/develop/uefi/u-boot_on_efi.rst @@ -269,7 +269,7 @@ This work could be extended in a number of ways:
- Avoid turning off boot services in the stub. Instead allow U-Boot to make use of boot services in case it wants to. It is unclear what it might want - though. + though. It is better to use the app.
Where is the code? ------------------

If the 'bootm' command is not enabled then this code is not available and this causes a link error. Fix it.
Note that for the EFI app, there is no indication of missing code. It just hangs!
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/x86/lib/zimage.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 7ce02226ef9..9cc04490307 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -365,11 +365,14 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, strcpy(cmd_line, (char *)cmdline_force); else build_command_line(cmd_line, auto_boot); - ret = bootm_process_cmdline(cmd_line, max_size, BOOTM_CL_ALL); - if (ret) { - printf("Cmdline setup failed (max_size=%x, bootproto=%x, err=%d)\n", - max_size, bootproto, ret); - return ret; + if (IS_ENABLED(CONFIG_CMD_BOOTM)) { + ret = bootm_process_cmdline(cmd_line, max_size, + BOOTM_CL_ALL); + if (ret) { + printf("Cmdline setup failed (max_size=%x, bootproto=%x, err=%d)\n", + max_size, bootproto, ret); + return ret; + } } printf("Kernel command line: ""); puts(cmd_line);

At present only 4KB of spare space is left in the DTB when building the EFI app. Increase this to 32KB so there is plenty of space to insert the binman definition. This cannot be expanded later (as with OF_SEPARATE) because the ELF image has already been built.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/x86/dts/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/dts/Makefile b/arch/x86/dts/Makefile index be209aaaf8f..5c8c05ec499 100644 --- a/arch/x86/dts/Makefile +++ b/arch/x86/dts/Makefile @@ -24,7 +24,7 @@ dtb-y += bayleybay.dtb \
targets += $(dtb-y)
-DTC_FLAGS += -R 4 -p 0x1000 +DTC_FLAGS += -R 4 -p $(if $(CONFIG_EFI_APP),0x8000,0x1000)
PHONY += dtbs dtbs: $(addprefix $(obj)/, $(dtb-y))

This is not used anywhere drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v3)
Changes in v3: - Move device_path path change to its own patch
include/efi.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/include/efi.h b/include/efi.h index 908c5dc6ebd..77e599c256e 100644 --- a/include/efi.h +++ b/include/efi.h @@ -402,7 +402,6 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
struct efi_priv { efi_handle_t parent_image; - struct efi_device_path *device_path; struct efi_system_table *sys_table; struct efi_boot_services *boot; struct efi_runtime_services *run;

This structure is uncommented. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v3)
Changes in v3: - Drop comments that confuse sphinx - Move device_path path change to its own patch
include/efi.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/include/efi.h b/include/efi.h index 77e599c256e..6622a733e6e 100644 --- a/include/efi.h +++ b/include/efi.h @@ -400,14 +400,37 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc( return (struct efi_mem_desc *)((ulong)desc + map->desc_size); }
+/** + * struct efi_priv - Information about the environment provided by EFI + * + * @parent_image: image passed into the EFI app or stub + * @sys_table: Pointer to system table + * @boot: Pointer to boot-services table + * @run: Pointer to runtime-services table + * + * @use_pool_for_malloc: true if all allocation should go through the EFI 'pool' + * methods allocate_pool() and free_pool(); false to use 'pages' methods + * allocate_pages() and free_pages() + * @ram_base: Base address of RAM (size CONFIG_EFI_RAM_SIZE) + * @image_data_type: Type of the loaded image (e.g. EFI_LOADER_CODE) + * + * @info: Header of the info list, holding info collected by the stub and passed + * to U-Boot + * @info_size: Size of the info list, in bytes from @info + * @next_hdr: Pointer to where to put the next header when adding to the list + */ struct efi_priv { efi_handle_t parent_image; struct efi_system_table *sys_table; struct efi_boot_services *boot; struct efi_runtime_services *run; + + /* app: */ bool use_pool_for_malloc; unsigned long ram_base; unsigned int image_data_type; + + /* stub: */ struct efi_info_hdr *info; unsigned int info_size; void *next_hdr;

This should return false when the EFI app is running, since UEFI has done the required low-level init. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/init.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/init.h b/include/init.h index f2cd46dead0..3ce7b8b95bd 100644 --- a/include/init.h +++ b/include/init.h @@ -15,7 +15,7 @@ #include <linux/types.h>
/* Avoid using CONFIG_EFI_STUB directly as we may boot from other loaders */ -#ifdef CONFIG_EFI_STUB +#ifdef CONFIG_EFI #define ll_boot_init() false #else #include <asm/global_data.h>

Comment some functions that need more information.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi/efi_stub.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index b3393e47fae..156cbf0b928 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -225,6 +225,22 @@ static int get_codeseg32(void) return cs32; }
+/** + * setup_info_table() - sets up a table containing information from EFI + * + * We must call exit_boot_services() before jumping out of the stub into U-Boot + * proper, so that U-Boot has full control of peripherals, memory, etc. + * + * Once we do this, we cannot call any boot-services functions so we must find + * out everything we need to before doing that. + * + * Set up a struct efi_info_hdr table which can hold various records (e.g. + * struct efi_entry_memmap) with information obtained from EFI. + * + * @priv: Pointer to our private information which contains the list + * @size: Size of the table to allocate + * @return 0 if OK, non-zero on error + */ static int setup_info_table(struct efi_priv *priv, int size) { struct efi_info_hdr *info; @@ -248,6 +264,12 @@ static int setup_info_table(struct efi_priv *priv, int size) return 0; }
+/** + * add_entry_addr() - Add a new entry to the efi_info list + * + * @priv: Pointer to our private information which contains the list + * + */ static void add_entry_addr(struct efi_priv *priv, enum efi_entry_t type, void *ptr1, int size1, void *ptr2, int size2) {

On 12/4/21 16:56, Simon Glass wrote:
Comment some functions that need more information.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
lib/efi/efi_stub.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index b3393e47fae..156cbf0b928 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -225,6 +225,22 @@ static int get_codeseg32(void) return cs32; }
+/**
- setup_info_table() - sets up a table containing information from EFI
- We must call exit_boot_services() before jumping out of the stub into U-Boot
- proper, so that U-Boot has full control of peripherals, memory, etc.
- Once we do this, we cannot call any boot-services functions so we must find
- out everything we need to before doing that.
- Set up a struct efi_info_hdr table which can hold various records (e.g.
- struct efi_entry_memmap) with information obtained from EFI.
- @priv: Pointer to our private information which contains the list
- @size: Size of the table to allocate
- @return 0 if OK, non-zero on error
%s/@return/Return:/
cf. https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-do...
Best regards
Heinrich
- */ static int setup_info_table(struct efi_priv *priv, int size) { struct efi_info_hdr *info;
@@ -248,6 +264,12 @@ static int setup_info_table(struct efi_priv *priv, int size) return 0; }
+/**
- add_entry_addr() - Add a new entry to the efi_info list
- @priv: Pointer to our private information which contains the list
- */ static void add_entry_addr(struct efi_priv *priv, enum efi_entry_t type, void *ptr1, int size1, void *ptr2, int size2) {

At present each of these has its own static variable and helper functions. Move them into a shared file.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/efi.h | 21 +++++++++++++++++++++ lib/efi/efi.c | 29 +++++++++++++++++++++++++++++ lib/efi/efi_app.c | 21 ++------------------- lib/efi/efi_stub.c | 7 ++++--- 4 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/include/efi.h b/include/efi.h index 6622a733e6e..ca301db7cb5 100644 --- a/include/efi.h +++ b/include/efi.h @@ -474,6 +474,27 @@ extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[]; EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \ EFI_VARIABLE_APPEND_WRITE)
+/** + * efi_get_priv() - Get access to the EFI-private information + * + * This struct it used by both the stub and the app to record things about the + * EFI environment. It is not available in U-Boot proper after the stub has + * jumped there. Use efi_info_get() to obtain info in that case. + * + * @return pointer to private info + */ +struct efi_priv *efi_get_priv(void); + +/** + * efi_set_priv() - Set up a pointer to the EFI-private information + * + * This is called in the stub and app to record the location of this + * information. + * + * @priv: New location of private data + */ +void efi_set_priv(struct efi_priv *priv); + /** * efi_get_sys_table() - Get access to the main EFI system table * diff --git a/lib/efi/efi.c b/lib/efi/efi.c index 69e52e45748..cd6bf47b180 100644 --- a/lib/efi/efi.c +++ b/lib/efi/efi.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ /* + * Functions shared by the app and stub + * * Copyright (c) 2015 Google, Inc * * EFI information obtained here: @@ -17,6 +19,33 @@ #include <efi.h> #include <efi_api.h>
+static struct efi_priv *global_priv; + +struct efi_priv *efi_get_priv(void) +{ + return global_priv; +} + +void efi_set_priv(struct efi_priv *priv) +{ + global_priv = priv; +} + +struct efi_system_table *efi_get_sys_table(void) +{ + return global_priv->sys_table; +} + +struct efi_boot_services *efi_get_boot(void) +{ + return global_priv->boot; +} + +unsigned long efi_get_ram_base(void) +{ + return global_priv->ram_base; +} + /* * Global declaration of gd. * diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index e38d46b15db..a0ae2a531ee 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -27,23 +27,6 @@
DECLARE_GLOBAL_DATA_PTR;
-static struct efi_priv *global_priv; - -struct efi_system_table *efi_get_sys_table(void) -{ - return global_priv->sys_table; -} - -struct efi_boot_services *efi_get_boot(void) -{ - return global_priv->boot; -} - -unsigned long efi_get_ram_base(void) -{ - return global_priv->ram_base; -} - int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) { return -ENOSYS; @@ -344,7 +327,7 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, /* Set up access to EFI data structures */ efi_init(priv, "App", image, sys_table);
- global_priv = priv; + efi_set_priv(priv);
/* * Set up the EFI debug UART so that printf() works. This is @@ -370,7 +353,7 @@ efi_status_t EFIAPI efi_main(efi_handle_t image,
static void efi_exit(void) { - struct efi_priv *priv = global_priv; + struct efi_priv *priv = efi_get_priv();
free_memory(priv); printf("U-Boot EFI exiting\n"); diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index 156cbf0b928..bc4c3a48720 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -31,7 +31,6 @@ #error "This file needs to be ported for use on architectures" #endif
-static struct efi_priv *global_priv; static bool use_uart;
struct __packed desctab_info { @@ -63,6 +62,8 @@ void _debug_uart_init(void)
void putc(const char ch) { + struct efi_priv *priv = efi_get_priv(); + if (ch == '\n') putc('\r');
@@ -73,7 +74,7 @@ void putc(const char ch) ; outb(ch, (ulong)&com_port->thr); } else { - efi_putc(global_priv, ch); + efi_putc(priv, ch); } }
@@ -313,7 +314,7 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, puts(" efi_init() failed\n"); return ret; } - global_priv = priv; + efi_set_priv(priv);
cs32 = get_codeseg32(); if (cs32 < 0)

At present this code is inline in the app and stub. But they do the same thing. The difference is that the stub does it immediately and the app doesn't want to do it until the end (when it boots a kernel) or not at all, if returning to UEFI.
Move it into a function so it can be called as needed.
Also store the memory map so that it can be accessed within the app if needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Add a sentence about what the patch does
include/efi.h | 32 ++++++++++++++++++++++ lib/efi/efi.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++ lib/efi/efi_app.c | 3 ++ lib/efi/efi_stub.c | 66 ++++++++------------------------------------ 4 files changed, 114 insertions(+), 55 deletions(-)
diff --git a/include/efi.h b/include/efi.h index ca301db7cb5..ed28c204140 100644 --- a/include/efi.h +++ b/include/efi.h @@ -407,6 +407,12 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc( * @sys_table: Pointer to system table * @boot: Pointer to boot-services table * @run: Pointer to runtime-services table + * @memmap_key: Key returned from get_memory_map() + * @memmap_desc: List of memory-map records + * @memmap_alloc: Amount of memory allocated for memory map list + * @memmap_size Size of memory-map list in bytes + * @memmap_desc_size: Size of an individual memory-map record, in bytes + * @memmap_version: Memory-map version * * @use_pool_for_malloc: true if all allocation should go through the EFI 'pool' * methods allocate_pool() and free_pool(); false to use 'pages' methods @@ -424,6 +430,12 @@ struct efi_priv { struct efi_system_table *sys_table; struct efi_boot_services *boot; struct efi_runtime_services *run; + efi_uintn_t memmap_key; + struct efi_mem_desc *memmap_desc; + efi_uintn_t memmap_alloc; + efi_uintn_t memmap_size; + efi_uintn_t memmap_desc_size; + u32 memmap_version;
/* app: */ bool use_pool_for_malloc; @@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch); */ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
+/** + * efi_store_memory_map() - Collect the memory-map info from EFI + * + * Collect the memory info and store it for later use, e.g. in calling + * exit_boot_services() + * + * @priv: Pointer to private EFI structure + * @return 0 if OK, non-zero on error + */ +int efi_store_memory_map(struct efi_priv *priv); + +/** + * efi_call_exit_boot_services() - Handlet eh exit-boot-service procedure + * + * Tell EFI we don't want their boot services anymore + * + * @return 0 if OK, non-zero on error + */ +int efi_call_exit_boot_services(void); + #endif /* _LINUX_EFI_H */ diff --git a/lib/efi/efi.c b/lib/efi/efi.c index cd6bf47b180..20da88c9151 100644 --- a/lib/efi/efi.c +++ b/lib/efi/efi.c @@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)
boot->free_pool(ptr); } + +int efi_store_memory_map(struct efi_priv *priv) +{ + struct efi_boot_services *boot = priv->sys_table->boottime; + efi_uintn_t size, desc_size; + efi_status_t ret; + + /* Get the memory map so we can switch off EFI */ + size = 0; + ret = boot->get_memory_map(&size, NULL, &priv->memmap_key, + &priv->memmap_desc_size, + &priv->memmap_version); + if (ret != EFI_BUFFER_TOO_SMALL) { + printhex2(EFI_BITS_PER_LONG); + putc(' '); + printhex2(ret); + puts(" No memory map\n"); + return ret; + } + /* + * Since doing a malloc() may change the memory map and also we want to + * be able to read the memory map in efi_call_exit_boot_services() + * below, after more changes have happened + */ + priv->memmap_alloc = size + 1024; + priv->memmap_size = priv->memmap_alloc; + priv->memmap_desc = efi_malloc(priv, size, &ret); + if (!priv->memmap_desc) { + printhex2(ret); + puts(" No memory for memory descriptor\n"); + return ret; + } + + ret = boot->get_memory_map(&priv->memmap_size, priv->memmap_desc, + &priv->memmap_key, &desc_size, + &priv->memmap_version); + if (ret) { + printhex2(ret); + puts(" Can't get memory map\n"); + return ret; + } + + return 0; +} + +int efi_call_exit_boot_services(void) +{ + struct efi_priv *priv = efi_get_priv(); + const struct efi_boot_services *boot = priv->boot; + efi_uintn_t size; + u32 version; + efi_status_t ret; + + size = priv->memmap_alloc; + ret = boot->get_memory_map(&size, priv->memmap_desc, + &priv->memmap_key, + &priv->memmap_desc_size, &version); + if (ret) { + printhex2(ret); + puts(" Can't get memory map\n"); + return ret; + } + ret = boot->exit_boot_services(priv->parent_image, priv->memmap_key); + if (ret) + return ret; + + return 0; +} diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index a0ae2a531ee..23a65c46fd4 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -341,6 +341,9 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, printf("Failed to set up memory: ret=%lx\n", ret); return ret; } + ret = efi_store_memory_map(priv); + if (ret) + return ret;
printf("starting\n");
diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index bc4c3a48720..5b08c1c40c7 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -297,15 +297,12 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, { struct efi_priv local_priv, *priv = &local_priv; struct efi_boot_services *boot = sys_table->boottime; - struct efi_mem_desc *desc; struct efi_entry_memmap map; struct efi_gop *gop; struct efi_entry_gopmode mode; struct efi_entry_systable table; efi_guid_t efi_gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID; - efi_uintn_t key, desc_size, size; efi_status_t ret; - u32 version; int cs32;
ret = efi_init(priv, "Payload", image, sys_table); @@ -320,24 +317,11 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, if (cs32 < 0) return EFI_UNSUPPORTED;
- /* Get the memory map so we can switch off EFI */ - size = 0; - ret = boot->get_memory_map(&size, NULL, &key, &desc_size, &version); - if (ret != EFI_BUFFER_TOO_SMALL) { - printhex2(EFI_BITS_PER_LONG); - putc(' '); - printhex2(ret); - puts(" No memory map\n"); - return ret; - } - size += 1024; /* Since doing a malloc() may change the memory map! */ - desc = efi_malloc(priv, size, &ret); - if (!desc) { - printhex2(ret); - puts(" No memory for memory descriptor\n"); + ret = efi_store_memory_map(priv); + if (ret) return ret; - } - ret = setup_info_table(priv, size + 128); + + ret = setup_info_table(priv, priv->memmap_size + 128); if (ret) return ret;
@@ -353,48 +337,20 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, sizeof(struct efi_gop_mode_info)); }
- ret = boot->get_memory_map(&size, desc, &key, &desc_size, &version); - if (ret) { - printhex2(ret); - puts(" Can't get memory map\n"); - return ret; - } - table.sys_table = (ulong)sys_table; add_entry_addr(priv, EFIET_SYS_TABLE, &table, sizeof(table), NULL, 0);
- ret = boot->exit_boot_services(image, key); - if (ret) { - /* - * Unfortunately it happens that we cannot exit boot services - * the first time. But the second time it work. I don't know - * why but this seems to be a repeatable problem. To get - * around it, just try again. - */ - printhex2(ret); - puts(" Can't exit boot services\n"); - size = sizeof(desc); - ret = boot->get_memory_map(&size, desc, &key, &desc_size, - &version); - if (ret) { - printhex2(ret); - puts(" Can't get memory map\n"); - return ret; - } - ret = boot->exit_boot_services(image, key); - if (ret) { - printhex2(ret); - puts(" Can't exit boot services 2\n"); - return ret; - } - } + ret = efi_call_exit_boot_services(); + if (ret) + return ret;
/* The EFI UART won't work now, switch to a debug one */ use_uart = true;
- map.version = version; - map.desc_size = desc_size; - add_entry_addr(priv, EFIET_MEMORY_MAP, &map, sizeof(map), desc, size); + map.version = priv->memmap_version; + map.desc_size = priv->memmap_desc_size; + add_entry_addr(priv, EFIET_MEMORY_MAP, &map, sizeof(map), + priv->memmap_desc, priv->memmap_size); add_entry_addr(priv, EFIET_END, NULL, 0, 0, 0);
memcpy((void *)CONFIG_SYS_TEXT_BASE, _binary_u_boot_bin_start,

On 12/4/21 16:56, Simon Glass wrote:
At present this code is inline in the app and stub. But they do the same thing. The difference is that the stub does it immediately and the app doesn't want to do it until the end (when it boots a kernel) or not at all, if returning to UEFI.
Move it into a function so it can be called as needed.
Also store the memory map so that it can be accessed within the app if needed.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
Add a sentence about what the patch does
include/efi.h | 32 ++++++++++++++++++++++ lib/efi/efi.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++ lib/efi/efi_app.c | 3 ++ lib/efi/efi_stub.c | 66 ++++++++------------------------------------ 4 files changed, 114 insertions(+), 55 deletions(-)
diff --git a/include/efi.h b/include/efi.h index ca301db7cb5..ed28c204140 100644 --- a/include/efi.h +++ b/include/efi.h @@ -407,6 +407,12 @@ static inline struct efi_mem_desc *efi_get_next_mem_desc(
- @sys_table: Pointer to system table
- @boot: Pointer to boot-services table
- @run: Pointer to runtime-services table
- @memmap_key: Key returned from get_memory_map()
- @memmap_desc: List of memory-map records
- @memmap_alloc: Amount of memory allocated for memory map list
- @memmap_size Size of memory-map list in bytes
- @memmap_desc_size: Size of an individual memory-map record, in bytes
- @memmap_version: Memory-map version
- @use_pool_for_malloc: true if all allocation should go through the EFI 'pool'
- methods allocate_pool() and free_pool(); false to use 'pages' methods
@@ -424,6 +430,12 @@ struct efi_priv { struct efi_system_table *sys_table; struct efi_boot_services *boot; struct efi_runtime_services *run;
efi_uintn_t memmap_key;
struct efi_mem_desc *memmap_desc;
efi_uintn_t memmap_alloc;
efi_uintn_t memmap_size;
efi_uintn_t memmap_desc_size;
u32 memmap_version;
/* app: */ bool use_pool_for_malloc;
@@ -574,4 +586,24 @@ void efi_putc(struct efi_priv *priv, const char ch); */ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
+/**
- efi_store_memory_map() - Collect the memory-map info from EFI
- Collect the memory info and store it for later use, e.g. in calling
- exit_boot_services()
- @priv: Pointer to private EFI structure
- @return 0 if OK, non-zero on error
%s/@return/Return:/
Cf. https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-do...
- */
+int efi_store_memory_map(struct efi_priv *priv);
+/**
- efi_call_exit_boot_services() - Handlet eh exit-boot-service procedure
- Tell EFI we don't want their boot services anymore
- @return 0 if OK, non-zero on error
%s/@return/Return:/
- */
+int efi_call_exit_boot_services(void);
- #endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi.c b/lib/efi/efi.c index cd6bf47b180..20da88c9151 100644 --- a/lib/efi/efi.c +++ b/lib/efi/efi.c @@ -135,3 +135,71 @@ void efi_free(struct efi_priv *priv, void *ptr)
boot->free_pool(ptr); }
+int efi_store_memory_map(struct efi_priv *priv) +{
- struct efi_boot_services *boot = priv->sys_table->boottime;
- efi_uintn_t size, desc_size;
- efi_status_t ret;
- /* Get the memory map so we can switch off EFI */
- size = 0;
- ret = boot->get_memory_map(&size, NULL, &priv->memmap_key,
&priv->memmap_desc_size,
&priv->memmap_version);
- if (ret != EFI_BUFFER_TOO_SMALL) {
printhex2(EFI_BITS_PER_LONG);
putc(' ');
printhex2(ret);
puts(" No memory map\n");
return ret;
- }
- /*
* Since doing a malloc() may change the memory map and also we want to
* be able to read the memory map in efi_call_exit_boot_services()
* below, after more changes have happened
*/
- priv->memmap_alloc = size + 1024;
- priv->memmap_size = priv->memmap_alloc;
- priv->memmap_desc = efi_malloc(priv, size, &ret);
- if (!priv->memmap_desc) {
printhex2(ret);
puts(" No memory for memory descriptor\n");
return ret;
- }
- ret = boot->get_memory_map(&priv->memmap_size, priv->memmap_desc,
&priv->memmap_key, &desc_size,
&priv->memmap_version);
- if (ret) {
printhex2(ret);
puts(" Can't get memory map\n");
return ret;
- }
- return 0;
+}
Missing function description
+int efi_call_exit_boot_services(void) +{
- struct efi_priv *priv = efi_get_priv();
- const struct efi_boot_services *boot = priv->boot;
- efi_uintn_t size;
- u32 version;
- efi_status_t ret;
- size = priv->memmap_alloc;
- ret = boot->get_memory_map(&size, priv->memmap_desc,
&priv->memmap_key,
&priv->memmap_desc_size, &version);
- if (ret) {
printhex2(ret);
puts(" Can't get memory map\n");
return ret;
- }
- ret = boot->exit_boot_services(priv->parent_image, priv->memmap_key);
- if (ret)
return ret;
- return 0;
+} diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index a0ae2a531ee..23a65c46fd4 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -341,6 +341,9 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, printf("Failed to set up memory: ret=%lx\n", ret); return ret; }
ret = efi_store_memory_map(priv);
if (ret)
return ret;
printf("starting\n");
diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index bc4c3a48720..5b08c1c40c7 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -297,15 +297,12 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, { struct efi_priv local_priv, *priv = &local_priv; struct efi_boot_services *boot = sys_table->boottime;
struct efi_mem_desc *desc; struct efi_entry_memmap map; struct efi_gop *gop; struct efi_entry_gopmode mode; struct efi_entry_systable table; efi_guid_t efi_gop_guid = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID;
efi_uintn_t key, desc_size, size; efi_status_t ret;
u32 version; int cs32;
ret = efi_init(priv, "Payload", image, sys_table);
@@ -320,24 +317,11 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, if (cs32 < 0) return EFI_UNSUPPORTED;
- /* Get the memory map so we can switch off EFI */
- size = 0;
- ret = boot->get_memory_map(&size, NULL, &key, &desc_size, &version);
- if (ret != EFI_BUFFER_TOO_SMALL) {
printhex2(EFI_BITS_PER_LONG);
putc(' ');
printhex2(ret);
puts(" No memory map\n");
return ret;
- }
- size += 1024; /* Since doing a malloc() may change the memory map! */
- desc = efi_malloc(priv, size, &ret);
- if (!desc) {
printhex2(ret);
puts(" No memory for memory descriptor\n");
- ret = efi_store_memory_map(priv);
- if (ret) return ret;
- }
- ret = setup_info_table(priv, size + 128);
- ret = setup_info_table(priv, priv->memmap_size + 128); if (ret) return ret;
@@ -353,48 +337,20 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, sizeof(struct efi_gop_mode_info)); }
ret = boot->get_memory_map(&size, desc, &key, &desc_size, &version);
if (ret) {
printhex2(ret);
puts(" Can't get memory map\n");
return ret;
}
table.sys_table = (ulong)sys_table; add_entry_addr(priv, EFIET_SYS_TABLE, &table, sizeof(table), NULL, 0);
ret = boot->exit_boot_services(image, key);
if (ret) {
/*
* Unfortunately it happens that we cannot exit boot services
* the first time. But the second time it work. I don't know
* why but this seems to be a repeatable problem. To get
* around it, just try again.
*/
printhex2(ret);
puts(" Can't exit boot services\n");
size = sizeof(desc);
ret = boot->get_memory_map(&size, desc, &key, &desc_size,
&version);
if (ret) {
printhex2(ret);
puts(" Can't get memory map\n");
return ret;
}
ret = boot->exit_boot_services(image, key);
if (ret) {
printhex2(ret);
puts(" Can't exit boot services 2\n");
return ret;
}
}
ret = efi_call_exit_boot_services();
if (ret)
return ret;
/* The EFI UART won't work now, switch to a debug one */ use_uart = true;
- map.version = version;
- map.desc_size = desc_size;
- add_entry_addr(priv, EFIET_MEMORY_MAP, &map, sizeof(map), desc, size);
map.version = priv->memmap_version;
map.desc_size = priv->memmap_desc_size;
add_entry_addr(priv, EFIET_MEMORY_MAP, &map, sizeof(map),
priv->memmap_desc, priv->memmap_size);
add_entry_addr(priv, EFIET_END, NULL, 0, 0, 0);
memcpy((void *)CONFIG_SYS_TEXT_BASE, _binary_u_boot_bin_start,

Hi Heinrich,
On Sat, 4 Dec 2021 at 12:13, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 16:56, Simon Glass wrote:
At present this code is inline in the app and stub. But they do the same thing. The difference is that the stub does it immediately and the app doesn't want to do it until the end (when it boots a kernel) or not at all, if returning to UEFI.
Move it into a function so it can be called as needed.
Also store the memory map so that it can be accessed within the app if needed.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
Add a sentence about what the patch does
include/efi.h | 32 ++++++++++++++++++++++ lib/efi/efi.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++ lib/efi/efi_app.c | 3 ++ lib/efi/efi_stub.c | 66 ++++++++------------------------------------ 4 files changed, 114 insertions(+), 55 deletions(-)
[..]
+/**
- efi_store_memory_map() - Collect the memory-map info from EFI
- Collect the memory info and store it for later use, e.g. in calling
- exit_boot_services()
- @priv: Pointer to private EFI structure
- @return 0 if OK, non-zero on error
%s/@return/Return:/
I will have to try to remember that. We are on our 3rd comment style now.
Cf. https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-do...
[..]
Missing function description
+int efi_call_exit_boot_services(void)
Putting your comment above the code you refer to defeats 'patman status -C', so could you please put it below?
The comment is in the header file as this is an exported function.
Regards, Simon

The stub checks for failure with efi_init(). Add this for the app as well. It is unlikely that anything can be done, but we may as well stop.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi/efi_app.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index 23a65c46fd4..e454f1a1564 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -325,8 +325,11 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, efi_status_t ret;
/* Set up access to EFI data structures */ - efi_init(priv, "App", image, sys_table); - + ret = efi_init(priv, "App", image, sys_table); + if (ret) { + printf("Failed to set up ARP: err=%lx\n", ret); + return ret; + } efi_set_priv(priv);
/*

On 12/4/21 07:56, Simon Glass wrote:
The stub checks for failure with efi_init(). Add this for the app as well. It is unlikely that anything can be done, but we may as well stop.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
lib/efi/efi_app.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index 23a65c46fd4..e454f1a1564 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -325,8 +325,11 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, efi_status_t ret;
/* Set up access to EFI data structures */
- efi_init(priv, "App", image, sys_table);
- ret = efi_init(priv, "App", image, sys_table);
- if (ret) {
printf("Failed to set up ARP: err=%lx\n", ret);
'ARP' is a typo. As U-Boot is not the only application that can be started by the UEFI firmware, please, provide a specific message:
%s/ARP/U-Boot/
Best regards
Heirnich
return ret;
} efi_set_priv(priv);
/*

This provides access to EFI tables after U-Boot has exited boot services. It is not needed in the app since boot services remain alive and we can just call them whenever needed.
Add a comment to explain this.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Fix 'as' typo
include/efi.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/efi.h b/include/efi.h index ed28c204140..69321cc5cc4 100644 --- a/include/efi.h +++ b/include/efi.h @@ -578,6 +578,10 @@ void efi_putc(struct efi_priv *priv, const char ch); /** * efi_info_get() - get an entry from an EFI table * + * This is called from U-Boot proper to read information set up by the EFI stub. + * It can only be used when running from the EFI stuff, not when U-Boot is + * running as an app. + * * @type: Entry type to search for * @datap: Returns pointer to entry data * @sizep: Returns pointer to entry size

On 12/4/21 07:56, Simon Glass wrote:
This provides access to EFI tables after U-Boot has exited boot services. It is not needed in the app since boot services remain alive and we can just call them whenever needed.
Add a comment to explain this.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
Fix 'as' typo
include/efi.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/efi.h b/include/efi.h index ed28c204140..69321cc5cc4 100644 --- a/include/efi.h +++ b/include/efi.h @@ -578,6 +578,10 @@ void efi_putc(struct efi_priv *priv, const char ch); /**
- efi_info_get() - get an entry from an EFI table
- This is called from U-Boot proper to read information set up by the EFI stub.
%s/This/This function/
- It can only be used when running from the EFI stuff, not when U-Boot is
'stuff' is very unspecific and colloquial.
Best regards
Heinrich
- running as an app.
- @type: Entry type to search for
- @datap: Returns pointer to entry data
- @sizep: Returns pointer to entry size

Add a message here so that both paths of memory allocation are reported.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Use log_info() instead of printf()
lib/efi/efi_app.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index e454f1a1564..36e3f1de427 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -121,13 +121,14 @@ static efi_status_t setup_memory(struct efi_priv *priv) ret = boot->allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, priv->image_data_type, pages, &addr); if (ret) { - printf("(using pool %lx) ", ret); + log_info("(using pool %lx) ", ret); priv->ram_base = (ulong)efi_malloc(priv, CONFIG_EFI_RAM_SIZE, &ret); if (!priv->ram_base) return ret; priv->use_pool_for_malloc = true; } else { + log_info("(using allocated RAM address %lx) ", (ulong)addr); priv->ram_base = addr; } gd->ram_size = pages << 12;

On 12/4/21 07:56, Simon Glass wrote:
Add a message here so that both paths of memory allocation are reported.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
Use log_info() instead of printf()
lib/efi/efi_app.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index e454f1a1564..36e3f1de427 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -121,13 +121,14 @@ static efi_status_t setup_memory(struct efi_priv *priv) ret = boot->allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, priv->image_data_type, pages, &addr); if (ret) {
If AllocatePages() fails the system is out of memory. Just return to the calling firmware with EFI_OUT_OF_RESOURCES.
Best regards
Heinrich
printf("(using pool %lx) ", ret);
priv->ram_base = (ulong)efi_malloc(priv, CONFIG_EFI_RAM_SIZE, &ret); if (!priv->ram_base) return ret; priv->use_pool_for_malloc = true; } else {log_info("(using pool %lx) ", ret);
priv->ram_base = addr; } gd->ram_size = pages << 12;log_info("(using allocated RAM address %lx) ", (ulong)addr);

Hi Heinrich,
On Thu, 9 Dec 2021 at 12:55, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
Add a message here so that both paths of memory allocation are reported.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
Use log_info() instead of printf()
lib/efi/efi_app.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index e454f1a1564..36e3f1de427 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -121,13 +121,14 @@ static efi_status_t setup_memory(struct efi_priv *priv) ret = boot->allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, priv->image_data_type, pages, &addr); if (ret) {
If AllocatePages() fails the system is out of memory. Just return to the calling firmware with EFI_OUT_OF_RESOURCES.
I have left this as it is as sometimes it is possible to allocate from the pool even if allocate_pages() fails. I hit this on an Intel platform some years ago.
In any case your comment does not relate to this patch.
Regards, Simon

Add info about how to select vidconsole or serial.
Also set up a demo boot command.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Add a better boot command too
include/configs/efi-x86_app.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/include/configs/efi-x86_app.h b/include/configs/efi-x86_app.h index 6061a6db0a4..33afb7ca0f9 100644 --- a/include/configs/efi-x86_app.h +++ b/include/configs/efi-x86_app.h @@ -10,8 +10,33 @@
#undef CONFIG_TPM_TIS_BASE_ADDRESS
+/* + * Select the output device: Put an 'x' prefix before one of these to disable it + */ + +/* + * Video output - can normally continue after exit_boot_services has been + * called, since output to the display does not require EFI services at that + * point. U-Boot sets up the console memory and does its own drawing. + */ #define CONFIG_STD_DEVICES_SETTINGS "stdin=serial\0" \ "stdout=vidconsole\0" \ "stderr=vidconsole\0"
+/* + * Serial output with no console. Run qemu with: + * + * -display none -serial mon:stdio + * + * This will hang or fail to output on the console after exit_boot_services is + * called. + */ +#define xCONFIG_STD_DEVICES_SETTINGS "stdin=serial\0" \ + "stdout=serial\0" \ + "stderr=serial\0" + +#undef CONFIG_BOOTCOMMAND + +#define CONFIG_BOOTCOMMAND "part list efi 0; fatls efi 0:1" + #endif

At present this function requires a pointer to struct efi_entry_memmap but the only field used in there is the desc_size. We want to be able to use it from the app, so update it to use desc_size directly.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/x86/cpu/efi/payload.c | 8 ++++---- cmd/efi.c | 34 ++++++++++++++++++---------------- include/efi.h | 4 ++-- 3 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c index 3a9f7d72868..d2aa889a2b9 100644 --- a/arch/x86/cpu/efi/payload.c +++ b/arch/x86/cpu/efi/payload.c @@ -50,7 +50,7 @@ ulong board_get_usable_ram_top(ulong total_size)
end = (struct efi_mem_desc *)((ulong)map + size); desc = map->desc; - for (; desc < end; desc = efi_get_next_mem_desc(map, desc)) { + for (; desc < end; desc = efi_get_next_mem_desc(desc, map->desc_size)) { if (desc->type != EFI_CONVENTIONAL_MEMORY || desc->physical_start >= 1ULL << 32) continue; @@ -88,7 +88,7 @@ int dram_init(void) end = (struct efi_mem_desc *)((ulong)map + size); gd->ram_size = 0; desc = map->desc; - for (; desc < end; desc = efi_get_next_mem_desc(map, desc)) { + for (; desc < end; desc = efi_get_next_mem_desc(desc, map->desc_size)) { if (desc->type < EFI_MMAP_IO) gd->ram_size += desc->num_pages << EFI_PAGE_SHIFT; } @@ -113,7 +113,7 @@ int dram_init_banksize(void) desc = map->desc; for (num_banks = 0; desc < end && num_banks < CONFIG_NR_DRAM_BANKS; - desc = efi_get_next_mem_desc(map, desc)) { + desc = efi_get_next_mem_desc(desc, map->desc_size)) { /* * We only use conventional memory and ignore * anything less than 1MB. @@ -196,7 +196,7 @@ unsigned int install_e820_map(unsigned int max_entries,
end = (struct efi_mem_desc *)((ulong)map + size); for (desc = map->desc; desc < end; - desc = efi_get_next_mem_desc(map, desc)) { + desc = efi_get_next_mem_desc(desc, map->desc_size)) { if (desc->num_pages == 0) continue;
diff --git a/cmd/efi.c b/cmd/efi.c index f2ed26bd4b2..d2400acbbba 100644 --- a/cmd/efi.c +++ b/cmd/efi.c @@ -75,16 +75,17 @@ static int h_cmp_entry(const void *v1, const void *v2) /** * efi_build_mem_table() - make a sorted copy of the memory table * - * @map: Pointer to EFI memory map table + * @desc_base: Pointer to EFI memory map table * @size: Size of table in bytes + * @desc_size: Size of each @desc_base record * @skip_bs: True to skip boot-time memory and merge it with conventional * memory. This will significantly reduce the number of table * entries. * Return: pointer to the new table. It should be freed with free() by the * caller. */ -static void *efi_build_mem_table(struct efi_entry_memmap *map, int size, - bool skip_bs) +static void *efi_build_mem_table(struct efi_mem_desc *desc_base, int size, + int desc_size, bool skip_bs) { struct efi_mem_desc *desc, *end, *base, *dest, *prev; int count; @@ -95,15 +96,16 @@ static void *efi_build_mem_table(struct efi_entry_memmap *map, int size, debug("%s: Cannot allocate %#x bytes\n", __func__, size); return NULL; } - end = (struct efi_mem_desc *)((ulong)map + size); - count = ((ulong)end - (ulong)map->desc) / map->desc_size; - memcpy(base, map->desc, (ulong)end - (ulong)map->desc); - qsort(base, count, map->desc_size, h_cmp_entry); + end = (void *)desc_base + size; + count = ((ulong)end - (ulong)desc_base) / desc_size; + memcpy(base, desc_base, (ulong)end - (ulong)desc_base); + qsort(base, count, desc_size, h_cmp_entry); prev = NULL; addr = 0; dest = base; - end = (struct efi_mem_desc *)((ulong)base + count * map->desc_size); - for (desc = base; desc < end; desc = efi_get_next_mem_desc(map, desc)) { + end = (struct efi_mem_desc *)((ulong)base + count * desc_size); + for (desc = base; desc < end; + desc = efi_get_next_mem_desc(desc, desc_size)) { bool merge = true; u32 type = desc->type;
@@ -116,7 +118,7 @@ static void *efi_build_mem_table(struct efi_entry_memmap *map, int size, if (skip_bs && is_boot_services(desc->type)) type = EFI_CONVENTIONAL_MEMORY;
- memcpy(dest, desc, map->desc_size); + memcpy(dest, desc, desc_size); dest->type = type; if (!skip_bs || !prev) merge = false; @@ -131,7 +133,7 @@ static void *efi_build_mem_table(struct efi_entry_memmap *map, int size, prev->num_pages += desc->num_pages; } else { prev = dest; - dest = efi_get_next_mem_desc(map, dest); + dest = efi_get_next_mem_desc(dest, desc_size); } addr = desc->physical_start + (desc->num_pages << EFI_PAGE_SHIFT); @@ -143,8 +145,8 @@ static void *efi_build_mem_table(struct efi_entry_memmap *map, int size, return base; }
-static void efi_print_mem_table(struct efi_entry_memmap *map, - struct efi_mem_desc *desc, bool skip_bs) +static void efi_print_mem_table(struct efi_mem_desc *desc, int desc_size, + bool skip_bs) { u64 attr_seen[ATTR_SEEN_MAX]; int attr_seen_count; @@ -158,7 +160,7 @@ static void efi_print_mem_table(struct efi_entry_memmap *map, attr_seen_count = 0; addr = 0; for (upto = 0; desc->type != EFI_MAX_MEMORY_TYPE; - upto++, desc = efi_get_next_mem_desc(map, desc)) { + upto++, desc = efi_get_next_mem_desc(desc, desc_size)) { const char *name; u64 size;
@@ -238,13 +240,13 @@ static int do_efi_mem(struct cmd_tbl *cmdtp, int flag, int argc, goto done; }
- desc = efi_build_mem_table(map, size, skip_bs); + desc = efi_build_mem_table(map->desc, size, map->desc_size, skip_bs); if (!desc) { ret = -ENOMEM; goto done; }
- efi_print_mem_table(map, desc, skip_bs); + efi_print_mem_table(desc, map->desc_size, skip_bs); free(desc); done: if (ret) diff --git a/include/efi.h b/include/efi.h index 69321cc5cc4..1dc806a1267 100644 --- a/include/efi.h +++ b/include/efi.h @@ -395,9 +395,9 @@ struct efi_entry_systable { };
static inline struct efi_mem_desc *efi_get_next_mem_desc( - struct efi_entry_memmap *map, struct efi_mem_desc *desc) + struct efi_mem_desc *desc, int desc_size) { - return (struct efi_mem_desc *)((ulong)desc + map->desc_size); + return (struct efi_mem_desc *)((ulong)desc + desc_size); }
/**

At present the 'efi' command only works in the EFI payload. Update it to work in the app too, so the memory map can be examined.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
cmd/Makefile | 2 +- cmd/efi.c | 48 ++++++++++++++++++++++++++++++++--------------- include/efi.h | 15 +++++++++++++++ lib/efi/efi_app.c | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 16 deletions(-)
diff --git a/cmd/Makefile b/cmd/Makefile index 891819ae0f6..df50625bde7 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -58,7 +58,7 @@ obj-$(CONFIG_CMD_EXTENSION) += extension_board.o obj-$(CONFIG_CMD_ECHO) += echo.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o -obj-$(CONFIG_EFI_STUB) += efi.o +obj-$(CONFIG_EFI) += efi.o obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_HUSH_PARSER) += exit.o diff --git a/cmd/efi.c b/cmd/efi.c index d2400acbbba..c0384e0db28 100644 --- a/cmd/efi.c +++ b/cmd/efi.c @@ -13,6 +13,8 @@ #include <sort.h> #include <asm/global_data.h>
+DECLARE_GLOBAL_DATA_PTR; + static const char *const type_name[] = { "reserved", "loader_code", @@ -217,37 +219,53 @@ static void efi_print_mem_table(struct efi_mem_desc *desc, int desc_size, static int do_efi_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - struct efi_mem_desc *desc; - struct efi_entry_memmap *map; + struct efi_mem_desc *orig, *desc; + uint version, key; + int desc_size; int size, ret; bool skip_bs;
skip_bs = !argc || *argv[0] != 'a'; - ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size); - switch (ret) { - case -ENOENT: - printf("No EFI table available\n"); - goto done; - case -EPROTONOSUPPORT: - printf("Incorrect EFI table version\n"); - goto done; + if (IS_ENABLED(CONFIG_EFI_APP)) { + ret = efi_get_mmap(&orig, &size, &key, &desc_size, &version); + if (ret) { + printf("Cannot read memory map (err=%d)\n", ret); + return CMD_RET_FAILURE; + } + } else { + struct efi_entry_memmap *map; + + ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size); + switch (ret) { + case -ENOENT: + printf("No EFI table available\n"); + goto done; + case -EPROTONOSUPPORT: + printf("Incorrect EFI table version\n"); + goto done; + } + orig = map->desc; + desc_size = map->desc_size; + version = map->version; } - printf("EFI table at %lx, memory map %p, size %x, version %x, descr. size %#x\n", - gd->arch.table, map, size, map->version, map->desc_size); - if (map->version != EFI_MEM_DESC_VERSION) { + printf("EFI table at %lx, memory map %p, size %x, key %x, version %x, descr. size %#x\n", + gd->arch.table, orig, size, key, version, desc_size); + if (version != EFI_MEM_DESC_VERSION) { printf("Incorrect memory map version\n"); ret = -EPROTONOSUPPORT; goto done; }
- desc = efi_build_mem_table(map->desc, size, map->desc_size, skip_bs); + desc = efi_build_mem_table(orig, size, desc_size, skip_bs); if (!desc) { ret = -ENOMEM; goto done; }
- efi_print_mem_table(desc, map->desc_size, skip_bs); + efi_print_mem_table(desc, desc_size, skip_bs); free(desc); + if (IS_ENABLED(CONFIG_EFI_APP)) + free(orig); done: if (ret) printf("Error: %d\n", ret); diff --git a/include/efi.h b/include/efi.h index 1dc806a1267..8a43430d3df 100644 --- a/include/efi.h +++ b/include/efi.h @@ -610,4 +610,19 @@ int efi_store_memory_map(struct efi_priv *priv); */ int efi_call_exit_boot_services(void);
+/** + * efi_get_mmap() - Get the memory map from EFI + * + * This is used in the app. The caller must free *@descp when done + * + * @descp: Returns allocated pointer to EFI memory map table + * @sizep: Returns size of table in bytes + * @keyp: Returns memory-map key + * @desc_sizep: Returns size of each @desc_base record + * @versionp: Returns version number of memory map + * @return 0 on success, -ve on error + */ +int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp, + int *desc_sizep, uint *versionp); + #endif /* _LINUX_EFI_H */ diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index 36e3f1de427..55fa1ac57d5 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -32,6 +32,39 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) return -ENOSYS; }
+int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp, + int *desc_sizep, uint *versionp) +{ + struct efi_priv *priv = efi_get_priv(); + struct efi_boot_services *boot = priv->sys_table->boottime; + efi_uintn_t size, desc_size, key; + struct efi_mem_desc *desc; + efi_status_t ret; + u32 version; + + /* Get the memory map so we can switch off EFI */ + size = 0; + ret = boot->get_memory_map(&size, NULL, &key, &desc_size, &version); + if (ret != EFI_BUFFER_TOO_SMALL) + return log_msg_ret("get", -ENOMEM); + + desc = malloc(size); + if (!desc) + return log_msg_ret("mem", -ENOMEM); + + ret = boot->get_memory_map(&size, desc, &key, &desc_size, &version); + if (ret) + return log_msg_ret("get", -EINVAL); + + *descp = desc; + *sizep = size; + *desc_sizep = desc_size; + *versionp = version; + *keyp = key; + + return 0; +} + /** * efi_print_str() - Print a UFT-16 string to the U-Boot console *

On 12/4/21 07:56, Simon Glass wrote:
At present the 'efi' command only works in the EFI payload. Update it to work in the app too, so the memory map can be examined.
cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a future patch we should try to move to using common code for commands efi and efidebug.
Best regards
Heinrich
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
cmd/Makefile | 2 +- cmd/efi.c | 48 ++++++++++++++++++++++++++++++++--------------- include/efi.h | 15 +++++++++++++++ lib/efi/efi_app.c | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 16 deletions(-)
diff --git a/cmd/Makefile b/cmd/Makefile index 891819ae0f6..df50625bde7 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -58,7 +58,7 @@ obj-$(CONFIG_CMD_EXTENSION) += extension_board.o obj-$(CONFIG_CMD_ECHO) += echo.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o obj-$(CONFIG_CMD_EEPROM) += eeprom.o -obj-$(CONFIG_EFI_STUB) += efi.o +obj-$(CONFIG_EFI) += efi.o obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o obj-$(CONFIG_CMD_ELF) += elf.o obj-$(CONFIG_HUSH_PARSER) += exit.o diff --git a/cmd/efi.c b/cmd/efi.c index d2400acbbba..c0384e0db28 100644 --- a/cmd/efi.c +++ b/cmd/efi.c @@ -13,6 +13,8 @@ #include <sort.h> #include <asm/global_data.h>
+DECLARE_GLOBAL_DATA_PTR;
- static const char *const type_name[] = { "reserved", "loader_code",
@@ -217,37 +219,53 @@ static void efi_print_mem_table(struct efi_mem_desc *desc, int desc_size, static int do_efi_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) {
- struct efi_mem_desc *desc;
- struct efi_entry_memmap *map;
struct efi_mem_desc *orig, *desc;
uint version, key;
int desc_size; int size, ret; bool skip_bs;
skip_bs = !argc || *argv[0] != 'a';
- ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size);
- switch (ret) {
- case -ENOENT:
printf("No EFI table available\n");
goto done;
- case -EPROTONOSUPPORT:
printf("Incorrect EFI table version\n");
goto done;
- if (IS_ENABLED(CONFIG_EFI_APP)) {
ret = efi_get_mmap(&orig, &size, &key, &desc_size, &version);
if (ret) {
printf("Cannot read memory map (err=%d)\n", ret);
return CMD_RET_FAILURE;
}
- } else {
struct efi_entry_memmap *map;
ret = efi_info_get(EFIET_MEMORY_MAP, (void **)&map, &size);
switch (ret) {
case -ENOENT:
printf("No EFI table available\n");
goto done;
case -EPROTONOSUPPORT:
printf("Incorrect EFI table version\n");
goto done;
}
orig = map->desc;
desc_size = map->desc_size;
}version = map->version;
- printf("EFI table at %lx, memory map %p, size %x, version %x, descr. size %#x\n",
gd->arch.table, map, size, map->version, map->desc_size);
- if (map->version != EFI_MEM_DESC_VERSION) {
- printf("EFI table at %lx, memory map %p, size %x, key %x, version %x, descr. size %#x\n",
gd->arch.table, orig, size, key, version, desc_size);
- if (version != EFI_MEM_DESC_VERSION) { printf("Incorrect memory map version\n"); ret = -EPROTONOSUPPORT; goto done; }
- desc = efi_build_mem_table(map->desc, size, map->desc_size, skip_bs);
- desc = efi_build_mem_table(orig, size, desc_size, skip_bs); if (!desc) { ret = -ENOMEM; goto done; }
- efi_print_mem_table(desc, map->desc_size, skip_bs);
- efi_print_mem_table(desc, desc_size, skip_bs); free(desc);
- if (IS_ENABLED(CONFIG_EFI_APP))
done: if (ret) printf("Error: %d\n", ret);free(orig);
diff --git a/include/efi.h b/include/efi.h index 1dc806a1267..8a43430d3df 100644 --- a/include/efi.h +++ b/include/efi.h @@ -610,4 +610,19 @@ int efi_store_memory_map(struct efi_priv *priv); */ int efi_call_exit_boot_services(void);
+/**
- efi_get_mmap() - Get the memory map from EFI
- This is used in the app. The caller must free *@descp when done
- @descp: Returns allocated pointer to EFI memory map table
- @sizep: Returns size of table in bytes
- @keyp: Returns memory-map key
- @desc_sizep: Returns size of each @desc_base record
- @versionp: Returns version number of memory map
- @return 0 on success, -ve on error
- */
+int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp,
int *desc_sizep, uint *versionp);
- #endif /* _LINUX_EFI_H */
diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c index 36e3f1de427..55fa1ac57d5 100644 --- a/lib/efi/efi_app.c +++ b/lib/efi/efi_app.c @@ -32,6 +32,39 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep) return -ENOSYS; }
+int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, uint *keyp,
int *desc_sizep, uint *versionp)
+{
- struct efi_priv *priv = efi_get_priv();
- struct efi_boot_services *boot = priv->sys_table->boottime;
- efi_uintn_t size, desc_size, key;
- struct efi_mem_desc *desc;
- efi_status_t ret;
- u32 version;
- /* Get the memory map so we can switch off EFI */
- size = 0;
- ret = boot->get_memory_map(&size, NULL, &key, &desc_size, &version);
- if (ret != EFI_BUFFER_TOO_SMALL)
return log_msg_ret("get", -ENOMEM);
- desc = malloc(size);
- if (!desc)
return log_msg_ret("mem", -ENOMEM);
- ret = boot->get_memory_map(&size, desc, &key, &desc_size, &version);
- if (ret)
return log_msg_ret("get", -EINVAL);
- *descp = desc;
- *sizep = size;
- *desc_sizep = desc_size;
- *versionp = version;
- *keyp = key;
- return 0;
+}
- /**
- efi_print_str() - Print a UFT-16 string to the U-Boot console

Hi Heinrich,
On Thu, 9 Dec 2021 at 13:27, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
At present the 'efi' command only works in the EFI payload. Update it to work in the app too, so the memory map can be examined.
cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a future patch we should try to move to using common code for commands efi and efidebug.
Yes and that depends on EFI_LOADER. Perhaps one day we can have a common helper for this, e.g. in lib/efi ?
Regards, Simon

On Fri, Dec 17, 2021 at 09:37:21AM -0700, Simon Glass wrote:
Hi Heinrich,
On Thu, 9 Dec 2021 at 13:27, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
At present the 'efi' command only works in the EFI payload. Update it to work in the app too, so the memory map can be examined.
cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a future patch we should try to move to using common code for commands efi and efidebug.
Yes and that depends on EFI_LOADER. Perhaps one day we can have a common helper for this, e.g. in lib/efi ?
It will inevitably means that we should stick to *UEFI* interfaces (not EFI_LOADER's internal interfaces like efi_xxx_yyy()) to implement those commands, including most of efidebug subcommands.
-Takahiro Akashi
Regards, Simon

Hi Takahiro,
On Sun, 19 Dec 2021 at 19:38, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
On Fri, Dec 17, 2021 at 09:37:21AM -0700, Simon Glass wrote:
Hi Heinrich,
On Thu, 9 Dec 2021 at 13:27, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
At present the 'efi' command only works in the EFI payload. Update it to work in the app too, so the memory map can be examined.
cmd/efi.c seems to be duplicating function do_efi_show_memmap(). In a future patch we should try to move to using common code for commands efi and efidebug.
Yes and that depends on EFI_LOADER. Perhaps one day we can have a common helper for this, e.g. in lib/efi ?
It will inevitably means that we should stick to *UEFI* interfaces (not EFI_LOADER's internal interfaces like efi_xxx_yyy()) to implement those commands, including most of efidebug subcommands.
That seems right to me, although of course there may be some 'inside knowledge' for some subcommands.
Regards, Simon

Show the revision of this table as it can be important.
Alo update the 'efi table' entry to show the actual address of the EFI table rather than our table that points to it. This saves a step and the intermediate table has nothing else in it.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Fix grammar in commit message
Changes in v3: - Add new patch to show the system-table revision
arch/x86/cpu/efi/payload.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c index d2aa889a2b9..b7778565b19 100644 --- a/arch/x86/cpu/efi/payload.c +++ b/arch/x86/cpu/efi/payload.c @@ -7,6 +7,7 @@ #include <common.h> #include <cpu_func.h> #include <efi.h> +#include <efi_api.h> #include <errno.h> #include <init.h> #include <log.h> @@ -296,8 +297,14 @@ void setup_efi_info(struct efi_info *efi_info) void efi_show_bdinfo(void) { struct efi_entry_systable *table = NULL; + struct efi_system_table *sys_table; int size, ret;
ret = efi_info_get(EFIET_SYS_TABLE, (void **)&table, &size); - bdinfo_print_num_l("efi_table", (ulong)table); + if (!ret) { + bdinfo_print_num_l("efi_table", table->sys_table); + sys_table = (struct efi_system_table *)(uintptr_t) + table->sys_table; + bdinfo_print_num_l(" revision", sys_table->fw_revision); + } }

On 12/4/21 07:56, Simon Glass wrote:
Show the revision of this table as it can be important.
Alo update the 'efi table' entry to show the actual address of the EFI
%s/Alo/Also/
table rather than our table that points to it. This saves a step and the intermediate table has nothing else in it.
Should this information been shown by the 'efi' command instead of 'bdinfo'?
Best regards
Heinrich
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v5:
- Fix grammar in commit message
Changes in v3:
Add new patch to show the system-table revision
arch/x86/cpu/efi/payload.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c index d2aa889a2b9..b7778565b19 100644 --- a/arch/x86/cpu/efi/payload.c +++ b/arch/x86/cpu/efi/payload.c @@ -7,6 +7,7 @@ #include <common.h> #include <cpu_func.h> #include <efi.h> +#include <efi_api.h> #include <errno.h> #include <init.h> #include <log.h> @@ -296,8 +297,14 @@ void setup_efi_info(struct efi_info *efi_info) void efi_show_bdinfo(void) { struct efi_entry_systable *table = NULL;
struct efi_system_table *sys_table; int size, ret;
ret = efi_info_get(EFIET_SYS_TABLE, (void **)&table, &size);
- bdinfo_print_num_l("efi_table", (ulong)table);
- if (!ret) {
bdinfo_print_num_l("efi_table", table->sys_table);
sys_table = (struct efi_system_table *)(uintptr_t)
table->sys_table;
bdinfo_print_num_l(" revision", sys_table->fw_revision);
- } }

Hi Heinrich,
On Thu, 9 Dec 2021 at 13:34, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/4/21 07:56, Simon Glass wrote:
Show the revision of this table as it can be important.
Alo update the 'efi table' entry to show the actual address of the EFI
%s/Alo/Also/
table rather than our table that points to it. This saves a step and the intermediate table has nothing else in it.
Should this information been shown by the 'efi' command instead of 'bdinfo'?
Perhaps we should add an 'efi info' command for this and various other things. We could show a lot more info.
But as you know, I like to have EFI more integrated into U-Boot and the bdinfo command is used for all sorts of subsystems. It is a natural place for people to look for the basics.
Regards, Simon

Since EFI does not relocate and uses the same global_data pointer throughout the board-init process, drop this unnecessary setup, to avoid a hang.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Add new patch to avoid setting up global_data again with EFI
common/board_r.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 31a59c585a8..8b5948100b1 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -817,9 +817,8 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) * TODO(sjg@chromium.org): Consider doing this for all archs, or * dropping the new_gd parameter. */ -#if CONFIG_IS_ENABLED(X86_64) - arch_setup_gd(new_gd); -#endif + if (CONFIG_IS_ENABLED(X86_64) && !IS_ENABLED(CONFIG_EFI_APP)) + arch_setup_gd(new_gd);
#ifdef CONFIG_NEEDS_MANUAL_RELOC int i;

Add an empty CPU init function to avoid fiddling with low-level CPU features in the app. Set up the C runtime correctly for 64-bit use and avoid clearing BSS, since this is done by EFI when U-Boot is loaded.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Add new patch to tweak the code used for the 64-bit EFI app
arch/x86/cpu/x86_64/cpu.c | 5 +++++ arch/x86/lib/Makefile | 5 ++--- arch/x86/lib/relocate.c | 2 ++ 3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c index a3674e8e29a..6a387612916 100644 --- a/arch/x86/cpu/x86_64/cpu.c +++ b/arch/x86/cpu/x86_64/cpu.c @@ -45,3 +45,8 @@ int cpu_phys_address_size(void) { return CONFIG_CPU_ADDR_BITS; } + +int x86_cpu_init_f(void) +{ + return 0; +} diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 18757b29aa9..e5235b7c4f4 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -65,9 +65,8 @@ endif
lib-$(CONFIG_USE_PRIVATE_LIBGCC) += div64.o
-ifeq ($(CONFIG_$(SPL_)X86_64),) -obj-$(CONFIG_EFI_APP) += crt0_ia32_efi.o reloc_ia32_efi.o -endif +obj-$(CONFIG_EFI_APP_32BIT) += crt0_ia32_efi.o reloc_ia32_efi.o +obj-$(CONFIG_EFI_APP_64BIT) += crt0_x86_64_efi.o reloc_x86_64_efi.o
ifneq ($(CONFIG_EFI_STUB),)
diff --git a/arch/x86/lib/relocate.c b/arch/x86/lib/relocate.c index 6fe51516477..9060d19d46a 100644 --- a/arch/x86/lib/relocate.c +++ b/arch/x86/lib/relocate.c @@ -35,6 +35,7 @@ int copy_uboot_to_ram(void) return 0; }
+#ifndef CONFIG_EFI_APP int clear_bss(void) { ulong dst_addr = (ulong)&__bss_start + gd->reloc_off; @@ -46,6 +47,7 @@ int clear_bss(void)
return 0; } +#endif
#if CONFIG_IS_ENABLED(X86_64) static void do_elf_reloc_fixups64(unsigned int text_base, uintptr_t size,

Make sure the linker lists are in the right place and drop the eh_frame section, which is not needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Add new patch to round out the link script for 64-bit EFI
arch/x86/lib/elf_x86_64_efi.lds | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/lib/elf_x86_64_efi.lds b/arch/x86/lib/elf_x86_64_efi.lds index b436429b33e..75727400aa4 100644 --- a/arch/x86/lib/elf_x86_64_efi.lds +++ b/arch/x86/lib/elf_x86_64_efi.lds @@ -63,6 +63,7 @@ SECTIONS *(.rela.data*) *(.rela.got) *(.rela.stab) + *(.rela.u_boot_list*) }
. = ALIGN(4096); @@ -70,9 +71,11 @@ SECTIONS . = ALIGN(4096); .dynstr : { *(.dynstr) } . = ALIGN(4096); + + /DISCARD/ : { *(.eh_frame) } + .ignored.reloc : { *(.rela.reloc) - *(.eh_frame) *(.note.GNU-stack) }

That script is not intended for use with EFI, so update the logic to avoid using it.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Add new patch to avoid using the 64-bit link script for the EFI app
arch/x86/cpu/config.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/config.mk b/arch/x86/cpu/config.mk index d3033b41603..87e242a2065 100644 --- a/arch/x86/cpu/config.mk +++ b/arch/x86/cpu/config.mk @@ -9,7 +9,7 @@ LDPPFLAGS += -DRESET_VEC_LOC=$(CONFIG_RESET_VEC_LOC) LDPPFLAGS += -DSTART_16=$(CONFIG_SYS_X86_START16)
ifdef CONFIG_X86_64 -ifndef CONFIG_SPL_BUILD +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_EFI_APP),) LDSCRIPT = $(srctree)/arch/x86/cpu/u-boot-64.lds endif endif

On 12/4/21 16:56, Simon Glass wrote:
That script is not intended for use with EFI, so update the logic to avoid using it.
Signed-off-by: Simon Glass sjg@chromium.org
Signed-off-by: Christian Melki christian.melki@t2data.com
Changes in v5:
- Add new patch to avoid using the 64-bit link script for the EFI app
arch/x86/cpu/config.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/cpu/config.mk b/arch/x86/cpu/config.mk index d3033b41603..87e242a2065 100644 --- a/arch/x86/cpu/config.mk +++ b/arch/x86/cpu/config.mk @@ -9,7 +9,7 @@ LDPPFLAGS += -DRESET_VEC_LOC=$(CONFIG_RESET_VEC_LOC) LDPPFLAGS += -DSTART_16=$(CONFIG_SYS_X86_START16)
ifdef CONFIG_X86_64 -ifndef CONFIG_SPL_BUILD +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_EFI_APP),) LDSCRIPT = $(srctree)/arch/x86/cpu/u-boot-64.lds endif endif

At present some 32-bit settings are used with the 64-bit app. Fix this by separating out the two cases.
Be careful not to break the 64-bit payload, which needs to build a 64-bit EFI stub with a 32-bit U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Add new patch to set the correct link flags for the 64-bit EFI app
arch/x86/config.mk | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/x86/config.mk b/arch/x86/config.mk index 589f2aed2bc..889497b6bd7 100644 --- a/arch/x86/config.mk +++ b/arch/x86/config.mk @@ -20,6 +20,11 @@ IS_32BIT := y endif endif
+EFI_IS_32BIT := $(IS_32BIT) +ifdef CONFIG_EFI_STUB_64BIT +EFI_IS_32BIT := +endif + ifeq ($(IS_32BIT),y) PLATFORM_CPPFLAGS += -march=i386 -m32 else @@ -44,8 +49,14 @@ CFLAGS_EFI := -fpic -fshort-wchar # Compiler flags to be removed when building UEFI applications CFLAGS_NON_EFI := -mregparm=3 -fstack-protector-strong
-ifeq ($(CONFIG_EFI_STUB_64BIT),) +ifeq ($(IS_32BIT),y) +EFIPAYLOAD_BFDARCH = i386 +else CFLAGS_EFI += $(call cc-option, -mno-red-zone) +EFIPAYLOAD_BFDARCH = x86_64 +endif + +ifeq ($(EFI_IS_32BIT),y) EFIARCH = ia32 EFIPAYLOAD_BFDTARGET = elf32-i386 else @@ -53,8 +64,6 @@ EFIARCH = x86_64 EFIPAYLOAD_BFDTARGET = elf64-x86-64 endif
-EFIPAYLOAD_BFDARCH = i386 - LDSCRIPT_EFI := $(srctree)/arch/x86/lib/elf_$(EFIARCH)_efi.lds EFISTUB := crt0_$(EFIARCH)_efi.o reloc_$(EFIARCH)_efi.o OBJCOPYFLAGS_EFI += --target=efi-app-$(EFIARCH)

On 12/4/21 16:56, Simon Glass wrote:
At present some 32-bit settings are used with the 64-bit app. Fix this by separating out the two cases.
Be careful not to break the 64-bit payload, which needs to build a 64-bit EFI stub with a 32-bit U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org
Signed-off-by: Christian Melki christian.melki@t2data.com
Changes in v5:
- Add new patch to set the correct link flags for the 64-bit EFI app
arch/x86/config.mk | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/x86/config.mk b/arch/x86/config.mk index 589f2aed2bc..889497b6bd7 100644 --- a/arch/x86/config.mk +++ b/arch/x86/config.mk @@ -20,6 +20,11 @@ IS_32BIT := y endif endif
+EFI_IS_32BIT := $(IS_32BIT) +ifdef CONFIG_EFI_STUB_64BIT +EFI_IS_32BIT := +endif
ifeq ($(IS_32BIT),y) PLATFORM_CPPFLAGS += -march=i386 -m32 else @@ -44,8 +49,14 @@ CFLAGS_EFI := -fpic -fshort-wchar # Compiler flags to be removed when building UEFI applications CFLAGS_NON_EFI := -mregparm=3 -fstack-protector-strong
-ifeq ($(CONFIG_EFI_STUB_64BIT),) +ifeq ($(IS_32BIT),y) +EFIPAYLOAD_BFDARCH = i386 +else CFLAGS_EFI += $(call cc-option, -mno-red-zone) +EFIPAYLOAD_BFDARCH = x86_64 +endif
+ifeq ($(EFI_IS_32BIT),y) EFIARCH = ia32 EFIPAYLOAD_BFDTARGET = elf32-i386 else @@ -53,8 +64,6 @@ EFIARCH = x86_64 EFIPAYLOAD_BFDTARGET = elf64-x86-64 endif
-EFIPAYLOAD_BFDARCH = i386
LDSCRIPT_EFI := $(srctree)/arch/x86/lib/elf_$(EFIARCH)_efi.lds EFISTUB := crt0_$(EFIARCH)_efi.o reloc_$(EFIARCH)_efi.o OBJCOPYFLAGS_EFI += --target=efi-app-$(EFIARCH)

Now that the linker crash is resolved, build the 64-bit EFI app, including all the required code.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v5: - Add new patch to build the 64-bit app properly - Add various patches to fix up the 64-bit app so that it boots
Changes in v2: - Add new patch to support the efi command in the app
Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 57c3643d9a8..08b848bf048 100644 --- a/Makefile +++ b/Makefile @@ -1765,9 +1765,9 @@ else quiet_cmd_u-boot__ ?= LD $@ cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@ \ -T u-boot.lds $(u-boot-init) \ - $(if $(CONFIG_EFI_APP_64BIT),,--whole-archive) \ + --whole-archive \ $(u-boot-main) \ - $(if $(CONFIG_EFI_APP_64BIT),,--no-whole-archive) \ + --no-whole-archive \ $(PLATFORM_LIBS) -Map u-boot.map; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) endif
participants (4)
-
AKASHI Takahiro
-
Christian Melki
-
Heinrich Schuchardt
-
Simon Glass