From 9de6ce807edaee02ae448a5130e9e78d60cc9b90 Mon Sep 17 00:00:00 2001 From: Pragnesh Patel Date: Tue, 19 May 2020 15:43:34 +0530 Subject: [PATCH 1/6] efi_loader: Remove unnecessary debug Remove unnecessary debug() from efi_set_variable_common(). native_name is NULL, so there is no meaning to print it. Signed-off-by: Pragnesh Patel Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/efi_variable.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index fc7ae73978e..0a43db56788 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -886,8 +886,6 @@ static efi_status_t efi_set_variable_common(u16 *variable_name, u32 attr; efi_status_t ret = EFI_SUCCESS; - debug("%s: set '%s'\n", __func__, native_name); - if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) && !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) { From 255a47333cc3e430d544fadf419b290213c5b11b Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 19 May 2020 07:20:46 +0200 Subject: [PATCH 2/6] efi_loader: add EFI_MEMORY_SP to memory attributes The UEFI 2.8 specification has introduced the EFI_MEMORY_SP memory attribute. Add it to the 'efidebug memmap' and 'efi mem' commands. Signed-off-by: Heinrich Schuchardt --- cmd/efi.c | 1 + cmd/efidebug.c | 1 + include/efi.h | 1 + 3 files changed, 3 insertions(+) diff --git a/cmd/efi.c b/cmd/efi.c index 9aeb913ff37..b3a3bf82821 100644 --- a/cmd/efi.c +++ b/cmd/efi.c @@ -44,6 +44,7 @@ static struct attr_info { { EFI_MEMORY_NV, "non-volatile" }, { EFI_MEMORY_MORE_RELIABLE, "higher reliability" }, { EFI_MEMORY_RO, "read-only" }, + { EFI_MEMORY_SP, "specific purpose" }, { EFI_MEMORY_RUNTIME, "needs runtime mapping" } }; diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 703dab0c20d..32430e62f0a 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -414,6 +414,7 @@ static const struct efi_mem_attrs { {EFI_MEMORY_NV, "NV"}, {EFI_MEMORY_MORE_RELIABLE, "REL"}, {EFI_MEMORY_RO, "RO"}, + {EFI_MEMORY_SP, "SP"}, {EFI_MEMORY_RUNTIME, "RT"}, }; diff --git a/include/efi.h b/include/efi.h index e12697a5d5b..f986aad8777 100644 --- a/include/efi.h +++ b/include/efi.h @@ -196,6 +196,7 @@ enum efi_mem_type { #define EFI_MEMORY_MORE_RELIABLE \ ((u64)0x0000000000010000ULL) /* higher reliability */ #define EFI_MEMORY_RO ((u64)0x0000000000020000ULL) /* read-only */ +#define EFI_MEMORY_SP ((u64)0x0000000000040000ULL) /* specific-purpose memory (SPM) */ #define EFI_MEMORY_RUNTIME ((u64)0x8000000000000000ULL) /* range requires runtime mapping */ #define EFI_MEM_DESC_VERSION 1 From c067cef695d8909f2c453eb9fedb718206d906a4 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 20 May 2020 21:27:29 +0200 Subject: [PATCH 3/6] efi_loader: initialize root node first With commit 16ad946f41d3 ("efi_loader: change setup sequence") the detection of block device was moved to the start of the initialization sequence. In the case of virtio devices two handles with the same device path being created. The root node handle should be created before anything else. Reported-by: Ard Biesheuvel Fixes: 16ad946f41d3 ("efi_loader: change setup sequence") Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_setup.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index 26a74232036..dd0c53fc239 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -135,6 +135,11 @@ efi_status_t efi_init_obj_list(void) /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */ switch_to_non_secure_mode(); + /* Initialize root node */ + ret = efi_root_node_register(); + if (ret != EFI_SUCCESS) + goto out; + #ifdef CONFIG_PARTITIONS ret = efi_disk_register(); if (ret != EFI_SUCCESS) @@ -175,11 +180,6 @@ efi_status_t efi_init_obj_list(void) if (ret != EFI_SUCCESS) goto out; - /* Initialize root node */ - ret = efi_root_node_register(); - if (ret != EFI_SUCCESS) - goto out; - /* Initialize EFI driver uclass */ ret = efi_driver_init(); if (ret != EFI_SUCCESS) From 19ecced71cfb92519e8ef7dddbc4138d871fe97c Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 20 May 2020 22:39:35 +0200 Subject: [PATCH 4/6] efi_loader: device path for virtio block devices The UEFI specification does not define a device sub-type for virtio. Let's use a vendor hardware node here. This avoids creation of two handles with the same device path indicating our root node. Reported-by: Ard Biesheuvel Signed-off-by: Heinrich Schuchardt --- include/efi_loader.h | 4 ++++ lib/efi_loader/efi_device_path.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/include/efi_loader.h b/include/efi_loader.h index 4b48f99773b..9533df26dc9 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -45,6 +45,10 @@ static inline void *guidcpy(void *dst, const void *src) #define U_BOOT_HOST_DEV_GUID \ EFI_GUID(0xbbe4e671, 0x5773, 0x4ea1, \ 0x9a, 0xab, 0x3a, 0x7d, 0xbf, 0x40, 0xc4, 0x82) +/* GUID used as root for virtio devices */ +#define U_BOOT_VIRTIO_DEV_GUID \ + EFI_GUID(0x63293792, 0xadf5, 0x9325, \ + 0xb9, 0x9f, 0x4e, 0x0e, 0x45, 0x5c, 0x1b, 0x1e) /* Use internal device tree when starting UEFI application */ #define EFI_FDT_USE_INTERNAL NULL diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 2b537c1e2ef..f049b9e0cd0 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -22,6 +22,9 @@ #ifdef CONFIG_SANDBOX const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID; #endif +#ifdef CONFIG_VIRTIO_BLK +const efi_guid_t efi_guid_virtio_dev = U_BOOT_VIRTIO_DEV_GUID; +#endif /* template END node: */ static const struct efi_device_path END = { @@ -469,6 +472,16 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) */ return dp_size(dev->parent) + sizeof(struct efi_device_path_vendor) + 1; +#endif +#ifdef CONFIG_VIRTIO_BLK + case UCLASS_VIRTIO: + /* + * Virtio devices will be represented as a vendor + * device node with an extra byte for the device + * number. + */ + return dp_size(dev->parent) + + sizeof(struct efi_device_path_vendor) + 1; #endif default: return dp_size(dev->parent); @@ -547,6 +560,23 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) return &dp->vendor_data[1]; } #endif +#ifdef CONFIG_VIRTIO_BLK + case UCLASS_VIRTIO: { + struct efi_device_path_vendor *dp; + struct blk_desc *desc = dev_get_uclass_platdata(dev); + + dp_fill(buf, dev->parent); + dp = buf; + ++dp; + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR; + dp->dp.length = sizeof(*dp) + 1; + memcpy(&dp->guid, &efi_guid_virtio_dev, + sizeof(efi_guid_t)); + dp->vendor_data[0] = desc->devnum; + return &dp->vendor_data[1]; + } +#endif #ifdef CONFIG_IDE case UCLASS_IDE: { struct efi_device_path_atapi *dp = From bf3bcef7fb8ef4b0bb56d00304a934a5a5eb187e Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 20 May 2020 23:12:02 +0200 Subject: [PATCH 5/6] efi_loader: device path for SATA devices Provide device path nodes for SATA devices. This avoids creation of two handles with the same device path indicating our root node. This is what the device paths for a SATA drive with four partitions could like: /VenHw(..)/Sata(0x0,0xffff,0x0) /VenHw(..)/Sata(0x0,0xffff,0x0)/HD(1,MBR,0x81ea591f,0x800,0x63ff830) /VenHw(..)/Sata(0x0,0xffff,0x0)/HD(2,MBR,0x81ea591f,0x6400800,0x9ff830) /VenHw(..)/Sata(0x0,0xffff,0x0)/HD(3,MBR,0x81ea591f,0x6e00800,0x16ef2ab0) /VenHw(..)/Sata(0x0,0xffff,0x0)/HD(4,MBR,0x81ea591f,0x1dcf3800,0x1dcedab0) Signed-off-by: Heinrich Schuchardt --- include/efi_api.h | 8 ++++++++ lib/efi_loader/efi_device_path.c | 21 +++++++++++++++++++++ lib/efi_loader/efi_device_path_to_text.c | 10 ++++++++++ 3 files changed, 39 insertions(+) diff --git a/include/efi_api.h b/include/efi_api.h index 77d6bf2660b..759d9118758 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -457,6 +457,7 @@ struct efi_device_path_acpi_path { # define DEVICE_PATH_SUB_TYPE_MSG_USB 0x05 # define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR 0x0b # define DEVICE_PATH_SUB_TYPE_MSG_USB_CLASS 0x0f +# define DEVICE_PATH_SUB_TYPE_MSG_SATA 0x12 # define DEVICE_PATH_SUB_TYPE_MSG_NVME 0x17 # define DEVICE_PATH_SUB_TYPE_MSG_SD 0x1a # define DEVICE_PATH_SUB_TYPE_MSG_MMC 0x1d @@ -480,6 +481,13 @@ struct efi_device_path_usb { u8 usb_interface; } __packed; +struct efi_device_path_sata { + struct efi_device_path dp; + u16 hba_port; + u16 port_multiplier_port; + u16 logical_unit_number; +} __packed; + struct efi_device_path_mac_addr { struct efi_device_path dp; struct efi_mac_addr mac; diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index f049b9e0cd0..7ae14f34239 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -458,6 +458,11 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev) return dp_size(dev->parent) + sizeof(struct efi_device_path_sd_mmc_path); #endif +#if defined(CONFIG_AHCI) || defined(CONFIG_SATA) + case UCLASS_AHCI: + return dp_size(dev->parent) + + sizeof(struct efi_device_path_sata); +#endif #if defined(CONFIG_NVME) case UCLASS_NVME: return dp_size(dev->parent) + @@ -623,6 +628,22 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev) return &sddp[1]; } #endif +#if defined(CONFIG_AHCI) || defined(CONFIG_SATA) + case UCLASS_AHCI: { + struct efi_device_path_sata *dp = + dp_fill(buf, dev->parent); + struct blk_desc *desc = dev_get_uclass_platdata(dev); + + dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_SATA; + dp->dp.length = sizeof(*dp); + dp->hba_port = desc->devnum; + /* default 0xffff implies no port multiplier */ + dp->port_multiplier_port = 0xffff; + dp->logical_unit_number = desc->lun; + return &dp[1]; + } +#endif #if defined(CONFIG_NVME) case UCLASS_NVME: { struct efi_device_path_nvme *dp = diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 49bebb58cc2..5ae4833fa78 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -149,6 +149,16 @@ static char *dp_msging(char *s, struct efi_device_path *dp) break; } + case DEVICE_PATH_SUB_TYPE_MSG_SATA: { + struct efi_device_path_sata *sdp = + (struct efi_device_path_sata *) dp; + + s += sprintf(s, "Sata(0x%x,0x%x,0x%x)", + sdp->hba_port, + sdp->port_multiplier_port, + sdp->logical_unit_number); + break; + } case DEVICE_PATH_SUB_TYPE_MSG_NVME: { struct efi_device_path_nvme *ndp = (struct efi_device_path_nvme *)dp; From 0a87e05dbd1b6ddafd61603e2a9c12659168ae65 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 21 May 2020 09:22:06 +0200 Subject: [PATCH 6/6] efi_loader: check device path is not installed twice Prior to corrective patches for virtio and SATA devices the same device path was installed on two different handles. This is not allowable. With this patch we will throw an error if this condition occurs for block devices. Update a comment for the installation of the simple file system protocol. Reported-by: Ard Biesheuvel Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_disk.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 9176008c0ea..670bf2b8ef0 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -356,6 +356,7 @@ static efi_status_t efi_disk_add_dev( struct efi_disk_obj **disk) { struct efi_disk_obj *diskobj; + struct efi_object *handle; efi_status_t ret; /* Don't add empty devices */ @@ -379,15 +380,25 @@ static efi_status_t efi_disk_add_dev( diskobj->dp = efi_dp_from_part(desc, part); } diskobj->part = part; - ret = efi_add_protocol(&diskobj->header, &efi_block_io_guid, - &diskobj->ops); + + /* + * Install the device path and the block IO protocol. + * + * InstallMultipleProtocolInterfaces() checks if the device path is + * already installed on an other handle and returns EFI_ALREADY_STARTED + * in this case. + */ + handle = &diskobj->header; + ret = EFI_CALL(efi_install_multiple_protocol_interfaces( + &handle, &efi_guid_device_path, diskobj->dp, + &efi_block_io_guid, &diskobj->ops, NULL)); if (ret != EFI_SUCCESS) return ret; - ret = efi_add_protocol(&diskobj->header, &efi_guid_device_path, - diskobj->dp); - if (ret != EFI_SUCCESS) - return ret; - /* partitions or whole disk without partitions */ + + /* + * On partitions or whole disks without partitions install the + * simple file system protocol if a file system is available. + */ if ((part || desc->part_type == PART_TYPE_UNKNOWN) && efi_fs_exists(desc, part)) { diskobj->volume = efi_simple_file_system(desc, part,