Re: [PATCH v5 07/12] efi_loader: disk: a helper function to create efi_disk objects from udevice

I has similar problem on axg meson64. Seems problem exists at least on 1Gb RAM devices, my device with 2 GB RAM did not hit this error.

19.05.2022 8:04, AKASHI Takahiro wrote:

On Wed, May 18, 2022 at 11:23:32AM +0200, Neil Armstrong wrote:

...

On 19/04/2022 03:05, AKASHI Takahiro wrote:

...

Add efi_disk_probe() function. This function creates an efi_disk object for a raw disk device (UCLASS_BLK) and additional objects for related partitions (UCLASS_PARTITION).

So this function is expected to be called through driver model's "probe" interface every time one raw disk device is detected and activated. We assume that partition devices (UCLASS_PARTITION) have been created when this function is invoked.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/efi_loader.h | 4 +- lib/efi_driver/efi_block_device.c | 34 ++--- lib/efi_loader/Kconfig | 3 + lib/efi_loader/efi_disk.c | 201 +++++++++++++++++++----------- lib/efi_loader/efi_setup.c | 4 +- 5 files changed, 143 insertions(+), 103 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 5bb8fb2acd04..ba79a9afb404 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -525,8 +525,8 @@ void efi_carve_out_dt_rsv(void *fdt); void efi_try_purge_kaslr_seed(void *fdt); /* Called by bootefi to make console interface available */ efi_status_t efi_console_register(void); -/* Called by bootefi to make all disk storage accessible as EFI objects */ -efi_status_t efi_disk_register(void); +/* Called by efi_init_obj_list() to initialize efi_disks */ +efi_status_t efi_disk_init(void); /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ efi_status_t efi_rng_register(void); /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */ diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index 04cb3ef0d4e5..5baa6f87a375 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -35,6 +35,7 @@ #include <malloc.h> #include <dm/device-internal.h> #include <dm/root.h> +#include <dm/tag.h> /* * EFI attributes of the udevice handled by this driver. @@ -106,25 +107,6 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, return blkcnt; } -/**

• • Create partions for the block device.
• • @handle: EFI handle of the block device
• • @dev: udevice of the block device
• • Return: number of partitions created
• */

-static int efi_bl_bind_partitions(efi_handle_t handle, struct udevice *dev) -{

• struct blk_desc *desc;
• const char *if_typename;
• desc = dev_get_uclass_plat(dev);
• if_typename = blk_get_if_type_name(desc->if_type);
• return efi_disk_create_partitions(handle, desc, if_typename,

• 			  desc->devnum, dev->name);
  

-}

• /**
  • Create a block device for a handle

@@ -139,7 +121,6 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) char *name; struct efi_object *obj = efi_search_obj(handle); struct efi_block_io *io = interface;

• int disks; struct efi_blk_plat *plat; EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);

@@ -173,15 +154,20 @@ static int efi_bl_bind(efi_handle_t handle, void *interface) plat->handle = handle; plat->io = interface;

• /*

• * FIXME: necessary because we won't do almost nothing in
  

• * efi_disk_create() when called from device_probe().
  

• */
  

• ret = dev_tag_set_ptr(bdev, DM_TAG_EFI, handle);
• if (ret)

• /* FIXME: cleanup for bdev */
  

• return ret;
  

• ret = device_probe(bdev); if (ret) return ret; EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);

• /* Create handles for the partions of the block device */
• disks = efi_bl_bind_partitions(handle, bdev);
• EFI_PRINT("Found %d partitions\n", disks);
• return 0; }

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index bc518d7a413b..6b245f50a726 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -14,6 +14,8 @@ config EFI_LOADER depends on DM_ETH || !NET depends on !EFI_APP default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8

• select DM_EVENT
• select EVENT_DYNAMIC select LIB_UUID imply PARTITION_UUIDS select HAVE_BLOCK_DEVICE

@@ -40,6 +42,7 @@ config CMD_BOOTEFI_BOOTMGR config EFI_SETUP_EARLY bool

• default y choice prompt "Store for non-volatile UEFI variables"

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index c905c12abc2f..d4a0edb458b8 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -10,6 +10,9 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <dm/device-internal.h> +#include <dm/tag.h> +#include <event.h> #include <efi_loader.h> #include <fs.h> #include <log.h> @@ -487,103 +490,153 @@ error: return ret; } -/**

• • efi_disk_create_partitions() - create handles and protocols for partitions

+/*

• • Create a handle for a whole raw disk
• • @dev uclass device (UCLASS_BLK)

• • Create handles and protocols for the partitions of a block device.

• • Create an efi_disk object which is associated with @dev.
• • The type of @dev must be UCLASS_BLK.

• • @parent: handle of the parent disk
• • @desc: block device
• • @if_typename: interface type
• • @diskid: device number
• • @pdevname: device name
• • Return: number of partitions created

• • @return 0 on success, -1 otherwise */

-int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,

• 	       const char *if_typename, int diskid,
  

• 	       const char *pdevname)
  

+static int efi_disk_create_raw(struct udevice *dev) {

• int disks = 0;
• char devname[32] = { 0 }; /* dp->str is u16[32] long */
• int part;
• struct efi_device_path *dp = NULL;

• struct efi_disk_obj *disk;
• struct blk_desc *desc;
• const char *if_typename;
• int diskid; efi_status_t ret;

• struct efi_handler *handler;
• /* Get the device path of the parent */
• ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
• if (ret == EFI_SUCCESS)

• dp = handler->protocol_interface;
  

• /* Add devices for each partition */
• for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {

• struct disk_partition info;
  

• if (part_get_info(desc, part, &info))
  

• 	continue;
  

• snprintf(devname, sizeof(devname), "%s:%x", pdevname,
  

• 	 part);
  

• ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
  

• 		       &info, part, NULL);
  

• if (ret != EFI_SUCCESS) {
  

• 	log_err("Adding partition %s failed\n", pdevname);
  

• 	continue;
  

• }
  

• disks++;
  

• desc = dev_get_uclass_plat(dev);
• if_typename = blk_get_if_type_name(desc->if_type);
• diskid = desc->devnum;
• ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,

• 	       diskid, NULL, 0, &disk);
  

• if (ret != EFI_SUCCESS) {

• if (ret == EFI_NOT_READY)
  

• 	log_notice("Disk %s not ready\n", dev->name);
  

• else
  

• 	log_err("Adding disk for %s failed\n", dev->name);
  

• return -1;
  

• }
• if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {

• efi_free_pool(disk->dp);
  

• efi_delete_handle(&disk->header);
  

• return -1;
  

  }

• return disks;

• return 0; }

-/**

• • efi_disk_register() - register block devices
• • U-Boot doesn't have a list of all online disk devices. So when running our
• • EFI payload, we scan through all of the potentially available ones and
• • store them in our object pool.

+/*

• • Create a handle for a disk partition

• • This function is called in efi_init_obj_list().

• • @dev uclass device (UCLASS_PARTITION)

• • TODO(sjg@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
• • Consider converting the code to look up devices as needed. The EFI device
• • could be a child of the UCLASS_BLK block device, perhaps.

• • Create an efi_disk object which is associated with @dev.
• • The type of @dev must be UCLASS_PARTITION.

• • Return: status code

• • @return 0 on success, -1 otherwise */

-efi_status_t efi_disk_register(void) +static int efi_disk_create_part(struct udevice *dev) {

• efi_handle_t parent;
• struct blk_desc *desc;
• const char *if_typename;
• struct disk_part *part_data;
• struct disk_partition *info;
• unsigned int part;
• int diskid;
• struct efi_handler *handler;
• struct efi_device_path *dp_parent; struct efi_disk_obj *disk;

• int disks = 0; efi_status_t ret;

• if (dev_tag_get_ptr(dev_get_parent(dev), DM_TAG_EFI, (void **)&parent))

• return -1;
  

• desc = dev_get_uclass_plat(dev_get_parent(dev));
• if_typename = blk_get_if_type_name(desc->if_type);
• diskid = desc->devnum;
• part_data = dev_get_uclass_plat(dev);
• part = part_data->partnum;
• info = &part_data->gpt_part_info;
• ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
• if (ret != EFI_SUCCESS)

• return -1;
  

• dp_parent = (struct efi_device_path *)handler->protocol_interface;
• ret = efi_disk_add_dev(parent, dp_parent, if_typename, desc, diskid,

• 	       info, part, &disk);
  

• if (ret != EFI_SUCCESS) {

• log_err("Adding partition for %s failed\n", dev->name);
  

• return -1;
  

• }
• if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {

• efi_free_pool(disk->dp);
  

• efi_delete_handle(&disk->header);
  

• return -1;
  

• }
• return 0;

+}

+/*

• • Create efi_disk objects for a block device
• • @dev uclass device (UCLASS_BLK)
• • Create efi_disk objects for partitions as well as a raw disk
• • which is associated with @dev.
• • The type of @dev must be UCLASS_BLK.
• • This function is expected to be called at EV_PM_POST_PROBE.
• • @return 0 on success, -1 otherwise
• */

+static int efi_disk_probe(void *ctx, struct event *event) +{ struct udevice *dev;

• enum uclass_id id;
• struct udevice *child;
• int ret;

• for (uclass_first_device_check(UCLASS_BLK, &dev); dev;

•     uclass_next_device_check(&dev)) {
  

• struct blk_desc *desc = dev_get_uclass_plat(dev);
  

• const char *if_typename = blk_get_if_type_name(desc->if_type);
  

• dev = event->data.dm.dev;
• id = device_get_uclass_id(dev);

• /* Add block device for the full device */
  

• log_info("Scanning disk %s...\n", dev->name);
  

• ret = efi_disk_add_dev(NULL, NULL, if_typename,
  

• 			desc, desc->devnum, NULL, 0, &disk);
  

• if (ret == EFI_NOT_READY) {
  

• 	log_notice("Disk %s not ready\n", dev->name);
  

• 	continue;
  

• }
  

• if (ret) {
  

• 	log_err("ERROR: failure to add disk device %s, r = %lu\n",
  

• 		dev->name, ret & ~EFI_ERROR_MASK);
  

• 	continue;
  

• }
  

• disks++;
  

• /* TODO: We won't support partitions in a partition */
• if (id != UCLASS_BLK)

• return 0;
  

• ret = efi_disk_create_raw(dev);
• if (ret)

• return -1;
  

• /* Partitions show up as block devices in EFI */
  

• disks += efi_disk_create_partitions(
  

• 			&disk->header, desc, if_typename,
  

• 			desc->devnum, dev->name);
  

• device_foreach_child(child, dev) {

• ret = efi_disk_create_part(child);
  

• if (ret)
  

• 	return -1;
  

  }

• log_info("Found %d disks\n", disks);

• return 0;

+}

+efi_status_t efi_disk_init(void) +{

• int ret;
• ret = event_register("efi_disk add", EVT_DM_POST_PROBE,

• 	     efi_disk_probe, NULL);
  

• if (ret) {

• log_err("Event registration for efi_disk add failed\n");
  

• return EFI_OUT_OF_RESOURCES;
  

• } return EFI_SUCCESS; }

diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index de2f34bab537..250eeb2fcd6b 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -198,9 +198,7 @@ static efi_status_t __efi_init_early(void) if (ret != EFI_SUCCESS) goto out; -#ifdef CONFIG_PARTITIONS

• ret = efi_disk_register();

-#endif

• ret = efi_disk_init(); out: return ret; }

This change as commit a9bf024b2933bba0e23038892970a18b72dfaeb4 in master makes Amlogic S905X board libretech-cc to crash:

Can you confirm that you do not use UEFI feature in your boot scenario here and yet you see a problem in UEFI subsystem?

...

Found U-Boot script /boot.scr 449 bytes read in 2 ms (218.8 KiB/s) ## Executing script at 08000000 36500032 bytes read in 1554 ms (22.4 MiB/s) 42873252 bytes read in 1829 ms (22.4 MiB/s) 29092 bytes read in 4 ms (6.9 MiB/s) ## Booting kernel from Legacy Image at 13000000 ... Image Name: Image Type: AArch64 Linux Kernel Image (uncompressed) Data Size: 36499968 Bytes = 34.8 MiB Load Address: 13000000 Entry Point: 13000000 Verifying Checksum ... OK ## Loading init Ramdisk from Legacy Image at 06000000 ... Image Name: Image Type: AArch64 Linux RAMDisk Image (uncompressed) Data Size: 42873188 Bytes = 40.9 MiB Load Address: 00000000 Entry Point: 00000000 Verifying Checksum ... OK ## Flattened Device Tree blob at 09000000 Booting using the fdt blob at 0x9000000 Loading Kernel Image Loading Ramdisk to 7965b000, end 7bf3e164 ... OK Loading Device Tree to 0000000079650000, end 000000007965a1a3 ... OK "Synchronous Abort" handler, esr 0x96000004 elr: 0000000001065150 lr : 0000000001054b18 (reloc)

Can you check where (functions) those two addresses (elr,lr) come from?

...

elr: 000000007dfb5150 lr : 000000007dfa4b18 x0 : d55d507f5dacfff5 x1 : 000000007dfbce48 x2 : 0000000000000010 x3 : 0000000000000000 x4 : 0000000000000000 x5 : d55d507f5dacfff5 x6 : 000000007bf55bb0 x7 : 0000000000000000 x8 : 0000000000000007 x9 : 0000000000000000 x10: 0000000000000178 x11: 000000007bf4211c x12: 00000000000000a4 x13: 000000007bf420d8 x14: 0000000079650000 x15: 0000000000000020 x16: 000000007df83454 x17: 0000000000000000 x18: 000000007bf4ddb0 x19: 000000007af42040 x20: 000000007df50b20 x21: 000000007dfbce48 x22: 0000000000001000 x23: 000000007bf55b00 x24: 000000007dfdb910 x25: 0000000001000000 x26: 0000000001000000 x27: 0000000001000000 x28: 0000000000001000 x29: 000000007bf420d0

Code: eb02009f 54000061 52800000 14000006 (386468a3)

As far as my locally-built binary is concerned, the faulted instruction is located in memcpy().

000000000106xxxx <memcmp>: 106xxxx: aa0003e5 mov x5, x0 106xxxx: d2800004 mov x4, #0x0 // #0 106xxxx: eb02009f cmp x4, x2 106xxxx: 54000061 b.ne 10653d4 <memcmp+0x18> // b.any 106xxxx: 52800000 mov w0, #0x0 // #0 106xxxx: 14000006 b 10653e8 <memcmp+0x2c> 106xxxx: 386468a3 ldrb w3, [x5, x4]

'x5' seems to be bogus. The memory content might have been corrupted at this point.

Same place:

"Synchronous Abort" handler, esr 0x96000004 elr: 000000000105ff48 lr : 00000000010504d4 (reloc) elr: 000000003ffa4f48 lr : 000000003ff954d4 x0 : 5695c9ebf0a7d8b1 x1 : 000000003ffac700 x2 : 0000000000000010 x3 : 0000000000000000 x4 : 0000000000000000 x5 : 5695c9ebf0a7d8b1 x6 : 000000003df4e3b0 x7 : 0000000000000007 x8 : 0000000000000000 x9 : 0000000000000008 x10: 0000000000008030 x11: 000000003df3496c x12: 0000000000007fa8 x13: 000000003df34928 x14: 000000003ce18000 x15: 0000000000000020 x16: 000000003ff75d74 x17: 0000000000000000 x18: 000000003df42dc0 x19: 000000003cf34040 x20: 000000003ff45b20 x21: 000000003ffac700 x22: 0000000000000300 x23: 000000003df4e300 x24: 000000003ffca5e8 x25: 0000000005300000 x26: 0000000000300000 x27: 0000000000000000 x28: 0000000000000300 x29: 000000003df34930

Code: eb02009f 54000061 52800000 14000006 (386468a3) 000000000105ff30 <memcmp>: 105ff30: aa0003e5 mov x5, x0 105ff34: d2800004 mov x4, #0x0 // #0 105ff38: eb02009f cmp x4, x2 105ff3c: 54000061 b.ne 105ff48 <memcmp+0x18> // b.any 105ff40: 52800000 mov w0, #0x0 // #0 105ff44: 14000006 b 105ff5c <memcmp+0x2c> *105ff48: 386468a3 ldrb w3, [x5, x4] 105ff4c: 38646820 ldrb w0, [x1, x4]

I don't know how to get the call stack from this information. What else can I do to investigate the problem?