On Thu, Oct 03, 2019 at 03:12:45PM +0200, Heinrich Schuchardt wrote:
On 10/3/19 1:02 PM, Patrick Wildt wrote:
This allows our EFI API to create a device path node for NVMe devices. It adds the necessary device path struct, uses the nvme namespace accessor to retrieve the id and eui64, and also provides support for the device path text protocol.
Signed-off-by: Patrick Wildt patrick@blueri.se
include/efi_api.h | 7 +++++++ lib/efi_loader/efi_device_path.c | 18 ++++++++++++++++++ lib/efi_loader/efi_device_path_to_text.c | 13 +++++++++++++ 3 files changed, 38 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h index 37e56da460..22396172e1 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -422,6 +422,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_NVME 0x17 # define DEVICE_PATH_SUB_TYPE_MSG_SD 0x1a # define DEVICE_PATH_SUB_TYPE_MSG_MMC 0x1d
@@ -464,6 +465,12 @@ struct efi_device_path_sd_mmc_path { u8 slot_number; } __packed;
+struct efi_device_path_nvme {
- struct efi_device_path dp;
- u32 ns_id;
- u8 eui64[8];
+} __packed;
- #define DEVICE_PATH_TYPE_MEDIA_DEVICE 0x04 # define DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH 0x01 # define DEVICE_PATH_SUB_TYPE_CDROM_PATH 0x02
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 86297bb7c1..6a18add269 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -10,6 +10,7 @@ #include <dm.h> #include <usb.h> #include <mmc.h> +#include <nvme.h> #include <efi_loader.h> #include <part.h> #include <sandboxblockdev.h> @@ -451,6 +452,11 @@ static unsigned dp_size(struct udevice *dev) return dp_size(dev->parent) + sizeof(struct efi_device_path_sd_mmc_path); #endif +#if defined(CONFIG_NVME)
case UCLASS_NVME:return dp_size(dev->parent) +sizeof(struct efi_device_path_nvme);+#endif #ifdef CONFIG_SANDBOX case UCLASS_ROOT: /* @@ -583,6 +589,18 @@ static void *dp_fill(void *buf, struct udevice *dev) sddp->slot_number = dev->seq; return &sddp[1]; } +#endif +#if defined(CONFIG_NVME)
case UCLASS_NVME: {struct efi_device_path_nvme *dp =dp_fill(buf, dev->parent);dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_NVME;dp->dp.length = sizeof(*dp);nvme_get_namespace_id(dev, &dp->ns_id, &dp->eui64[0]);Instead of &dp->eui64[0] you could simply write dp->eui64. To me that is easier to read.
Your code does not compile with GCC 9.2.1 (as available in Debian Bullseye):
lib/efi_loader/efi_device_path.c: In function ‘dp_fill’: lib/efi_loader/efi_device_path.c:601:31: error: taking address of packed member of ‘struct efi_device_path_nvme’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 601 | nvme_get_namespace_id(dev, &dp->ns_id, &dp->eui64[0]); | ^~~~~~~~~~
Uh, that sounds bad. How can we fix/work around this? Maybe something like..
u32 ns_id; nvme_get_namespace_id(dev, &ns_id, &dp->eui64[0]); memcpy(&dp->ns_id, &ns_id, sizeof(ns_id));
?
Best regards, Patrick
return &dp[1];}diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 0f3796b373..e8a608bd07 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -148,6 +148,19 @@ static char *dp_msging(char *s, struct efi_device_path *dp)
break;}
- case DEVICE_PATH_SUB_TYPE_MSG_NVME: {
struct efi_device_path_nvme *ndp =(struct efi_device_path_nvme *)dp;int i;s += sprintf(s, "NVME(0x%x,", ndp->ns_id);In the spec this is "NVMe(", i.e. lower case 'e' at the end.
for (i = 0; i < sizeof(ndp->eui64); ++i)s += sprintf(s, "%s%02x", i ? "-" : "",The example in the spec uses capitalized hexadecimals in the example. But EDK2 uses lower case. So that should be ok.
Best regards
Heinrich
ndp->eui64[i]);s += sprintf(s, ")");break;- } case DEVICE_PATH_SUB_TYPE_MSG_SD: case DEVICE_PATH_SUB_TYPE_MSG_MMC: { const char *typename =