[U-Boot] [PATCH 2/2] efi: device path for nvme
Patrick Wildt
patrick at blueri.se
Thu Oct 3 13:32:44 UTC 2019
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 at 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];
> > + }
> > #endif
> > default:
> > debug("%s(%u) %s: unhandled parent class: %s (%u)\n",
> > 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 =
> >
>
More information about the U-Boot
mailing list