[PATCH] nvme: don't create block device for inactive namespace

[Bin Meng]

Hi Heinrich,

On Sat, Jun 19, 2021 at 4:39 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> Am 18. Juni 2021 07:47:37 MESZ schrieb Bin Meng <bmeng.cn at gmail.com>:
> >+ML
> >
> >On Fri, Jun 18, 2021 at 1:47 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> >>
> >> Hi Heinrich,
> >>
> >> On Thu, Jun 17, 2021 at 7:04 PM Heinrich Schuchardt
> ><xypron.glpk at gmx.de> wrote:
> >> >
> >> > QEMU returns the highest supported namespace number NN as 255. This
> >does
> >> > not imply that there are 255 active namespaces.
> >> >
> >> > If a namespace is not active, the namespace identify command
> >returns a zero
> >> > filled data structure. We can use field NSZE (namespace size) to
> >decide if
> >> > a block device should be created.
> >>
> >> Thanks for the fix. Apparently QEMU has changed its behavior of
> >namespaces.
> >>
> >> >
> >> > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> > ---
> >> >  drivers/nvme/nvme-uclass.c | 30 +++++++++++++++++++++++++++++-
> >> >  1 file changed, 29 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/nvme/nvme-uclass.c
> >b/drivers/nvme/nvme-uclass.c
> >> > index 277e31e1f3..f7861f5287 100644
> >> > --- a/drivers/nvme/nvme-uclass.c
> >> > +++ b/drivers/nvme/nvme-uclass.c
> >> > @@ -4,22 +4,50 @@
> >> >   * Copyright (C) 2017 Bin Meng <bmeng.cn at gmail.com>
> >> >   */
> >> >
> >> > +#define LOG_CATEGORY UCLASS_NVME
> >> > +
> >> >  #include <common.h>
> >> >  #include <blk.h>
> >> >  #include <errno.h>
> >> >  #include <dm.h>
> >> >  #include <dm/device.h>
> >> > +#include <log.h>
> >> > +#include <memalign.h>
> >> > +#include <nvme.h>
> >> >  #include "nvme.h"
> >> >
> >> > +/**
> >> > + * nvme_probe_namespace() - check if namespace is active
> >> > + *
> >> > + * @dev:       NVMe controller device
> >> > + * @ns_id:     namespace ID
> >> > + * Return:     0 if namespace exists, -ve on error
> >> > + */
> >> > +static int nvme_probe_namespace(struct nvme_dev *dev, unsigned int
> >ns_id)
> >> > +{
> >> > +       ALLOC_CACHE_ALIGN_BUFFER(char, buf_ns, sizeof(struct
> >nvme_id_ns));
> >> > +       struct nvme_id_ns *id = (struct nvme_id_ns *)buf_ns;
> >> > +
> >> > +       if (nvme_identify(dev, ns_id, 0, (dma_addr_t)(long)id))
> >> > +               return -EIO;
> >> > +       log_debug("ns_id %u, nsze %llu\n", ns_id, id->nsze);
> >> > +       if (!id->nsze)
> >> > +               return -ENOENT;
> >> > +       return 0;
> >> > +}
> >> > +
> >> >  static int nvme_uclass_post_probe(struct udevice *udev)
> >> >  {
> >> >         char name[20];
> >> >         struct udevice *ns_udev;
> >> > -       int i, ret;
> >> > +       unsigned int i;
> >> > +       int ret;
> >> >         struct nvme_dev *ndev = dev_get_priv(udev);
> >> >
> >> >         /* Create a blk device for each namespace */
> >> >         for (i = 0; i < ndev->nn; i++) {
> >>
> >> I suggest we assign correct number of namespaces in
> >> nvme_get_info_from_identify() instead.
> >>
>
> The active mamespaces could be 23, 78, 167. In what way would it help here to know there are three of them?

Ah, yes.

>
> It might make sense to eliminate the offset of one.

This was changed due to QEMU commit:

commit 7f0f1acedf15 ("hw/block/nvme: support multiple namespaces")

I will see if I can do some refactoring of the existing codes to fix this.

Regards,
Bin