[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[Keith Busch]

The original translation reference resulted in collisions on VPD 83 for
many devices today. Later specification revisions provided other ways
to translate based on the device's specification compliance that will
create different identifiers.

For 1.1 commplaint devices, this patch will use the EUI64 field to create
a unique NAA Registerd identifier. If 1.0, the SCSI Name String format
is required to generate unique bits.

This fixes reported problems with customer device managers, but this
is going to be even more important when multipath capable nvme devices
become more common.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-scsi.c |   94 ++++++++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 32 deletions(-)

diff --git a/drivers/block/nvme-scsi.c b/drivers/block/nvme-scsi.c
index 49f86d1..b33cff6 100644
--- a/drivers/block/nvme-scsi.c
+++ b/drivers/block/nvme-scsi.c
@@ -782,11 +782,10 @@ static int nvme_trans_device_id_page(struct nvme_ns *ns, struct sg_io_hdr *hdr,
 	struct nvme_id_ctrl *id_ctrl;
 	int res = SNTI_TRANSLATION_SUCCESS;
 	int nvme_sc;
-	u8 ieee[4];
 	int xfer_len;
 	__be32 tmp_id = cpu_to_be32(ns->ns_id);
 
-	mem = dma_alloc_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns),
+	mem = dma_alloc_coherent(&dev->pci_dev->dev, 4096,
 					&dma_addr, GFP_KERNEL);
 	if (mem == NULL) {
 		res = -ENOMEM;
@@ -804,40 +803,71 @@ static int nvme_trans_device_id_page(struct nvme_ns *ns, struct sg_io_hdr *hdr,
 	}
 	id_ctrl = mem;
 
-	/* Since SCSI tried to save 4 bits... [SPC-4(r34) Table 591] */
-	ieee[0] = id_ctrl->ieee[0] << 4;
-	ieee[1] = id_ctrl->ieee[0] >> 4 | id_ctrl->ieee[1] << 4;
-	ieee[2] = id_ctrl->ieee[1] >> 4 | id_ctrl->ieee[2] << 4;
-	ieee[3] = id_ctrl->ieee[2] >> 4;
-
-	memset(inq_response, 0, STANDARD_INQUIRY_LENGTH);
+	memset(inq_response, 0, alloc_len);
 	inq_response[1] = INQ_DEVICE_IDENTIFICATION_PAGE;    /* Page Code */
-	inq_response[3] = 20;      /* Page Length */
-	/* Designation Descriptor start */
-	inq_response[4] = 0x01;    /* Proto ID=0h | Code set=1h */
-	inq_response[5] = 0x03;    /* PIV=0b | Asso=00b | Designator Type=3h */
-	inq_response[6] = 0x00;    /* Rsvd */
-	inq_response[7] = 16;      /* Designator Length */
-	/* Designator start */
-	inq_response[8] = 0x60 | ieee[3]; /* NAA=6h | IEEE ID MSB, High nibble*/
-	inq_response[9] = ieee[2];        /* IEEE ID */
-	inq_response[10] = ieee[1];       /* IEEE ID */
-	inq_response[11] = ieee[0];       /* IEEE ID| Vendor Specific ID... */
-	inq_response[12] = (dev->pci_dev->vendor & 0xFF00) >> 8;
-	inq_response[13] = (dev->pci_dev->vendor & 0x00FF);
-	inq_response[14] = dev->serial[0];
-	inq_response[15] = dev->serial[1];
-	inq_response[16] = dev->model[0];
-	inq_response[17] = dev->model[1];
-	memcpy(&inq_response[18], &tmp_id, sizeof(u32));
-	/* Last 2 bytes are zero */
 
-	xfer_len = min(alloc_len, STANDARD_INQUIRY_LENGTH);
+	if (readl(&dev->bar->vs) >= 0x10100) {
+		/* 1.1 requires EUI64 */
+		struct nvme_id_ns *id_ns = mem;
+		u8 ieee[4];
+
+		nvme_sc = nvme_identify(dev, ns->ns_id, 0, dma_addr);
+		res = nvme_trans_status_code(hdr, nvme_sc);
+		if (res)
+			goto out_free;
+		if (nvme_sc) {
+			res = nvme_sc;
+			goto out_free;
+		}
+
+		/* Since SCSI tried to save 4 bits... [SPC-4(r34) Table 591] */
+		ieee[0] = id_ctrl->ieee[0] << 4;
+		ieee[1] = id_ctrl->ieee[0] >> 4 | id_ctrl->ieee[1] << 4;
+		ieee[2] = id_ctrl->ieee[1] >> 4 | id_ctrl->ieee[2] << 4;
+		ieee[3] = id_ctrl->ieee[2] >> 4;
+
+		inq_response[3] = 0x14;    /* Page Length */
+		/* Designation Descriptor start */
+		inq_response[4] = 0x01;    /* Proto ID=0h | Code set=1h */
+		inq_response[5] = 0x03;    /* PIV=0b | Asso=00b | Designator Type=3h */
+		inq_response[6] = 0x03;    /* Rsvd */
+		inq_response[7] = 0x10;    /* Designator Length */
+
+		/* Designator start */
+		inq_response[8] = 0x60 | ieee[3]; /* NAA=6h | IEEE ID MSB, High nibble*/
+		inq_response[9] = ieee[2];        /* IEEE ID */
+		inq_response[10] = ieee[1];       /* IEEE ID */
+		inq_response[11] = ieee[0];       /* IEEE ID| Vendor Specific ID... */
+		memcpy(&inq_response[12], id_ns->eui64, sizeof(id_ns->eui64));
+	} else {
+		u16 vid = dev->pci_dev->vendor;
+
+		if (alloc_len < 72) {
+			res = nvme_trans_completion(hdr,
+					SAM_STAT_CHECK_CONDITION,
+					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
+					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
+			goto out_free;
+		}
+
+		inq_response[3] = 0x48;    /* Page Length */
+		/* Designation Descriptor start */
+		inq_response[4] = 0x03;    /* Proto ID=0h | Code set=3h */
+		inq_response[5] = 0x08;    /* PIV=0b | Asso=00b | Designator Type=8h */
+		inq_response[6] = 0x00;    /* Rsvd */
+		inq_response[7] = 0x44;    /* Designator Length */
+
+		sprintf(&inq_response[8], "%04x", vid);
+		memcpy(&inq_response[12], dev->model, sizeof(dev->model));
+		sprintf(&inq_response[52], "%04x", tmp_id);
+		memcpy(&inq_response[56], dev->serial, sizeof(dev->serial));
+	}
+
+	xfer_len = alloc_len;
 	res = nvme_trans_copy_to_user(hdr, inq_response, xfer_len);
 
  out_free:
-	dma_free_coherent(&dev->pci_dev->dev, sizeof(struct nvme_id_ns), mem,
-			  dma_addr);
+	dma_free_coherent(&dev->pci_dev->dev, 4096, mem, dma_addr);
  out_dma:
 	return res;
 }
@@ -2222,7 +2252,7 @@ static int nvme_trans_inquiry(struct nvme_ns *ns, struct sg_io_hdr *hdr,
 	page_code = GET_INQ_PAGE_CODE(cmd);
 	alloc_len = GET_INQ_ALLOC_LENGTH(cmd);
 
-	inq_response = kmalloc(STANDARD_INQUIRY_LENGTH, GFP_KERNEL);
+	inq_response = kmalloc(alloc_len, GFP_KERNEL);
 	if (inq_response == NULL) {
 		res = -ENOMEM;
 		goto out_mem;
-- 
1.7.10.4

[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[Matthew Wilcox]

On Tue, Dec 02, 2014@12:56:16PM -0700, Keith Busch wrote:
> +	if (readl(&dev->bar->vs) >= 0x10100) {

I think we want an NVME_VERSION() macro that maybe looks something like this:

#define NVME_VERSION(major, minor) (((major) << 16) | ((minor) << 8))

Then you can make this:

	if (readl(&dev->bar->vs) >= NVME_VERSION(1, 1))

What do you think?

[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[Keith Busch]

On Tue, 2 Dec 2014, Matthew Wilcox wrote:
> On Tue, Dec 02, 2014@12:56:16PM -0700, Keith Busch wrote:
>> +	if (readl(&dev->bar->vs) >= 0x10100) {
>
> I think we want an NVME_VERSION() macro that maybe looks something like this:
>
> #define NVME_VERSION(major, minor) (((major) << 16) | ((minor) << 8))
>
> Then you can make this:
>
> 	if (readl(&dev->bar->vs) >= NVME_VERSION(1, 1))
>
> What do you think?

I didn't even know what a 1.1 revision was supposed to look like until
1.2 clarified that, so a macro to hide the weirdness sounds good.

[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[Matthew Wilcox]

On Tue, Dec 02, 2014@11:57:50PM +0300, Andrey Kuzmin wrote:
> On Dec 2, 2014 11:51 PM, "Matthew Wilcox" <willy@linux.intel.com> wrote:
> >
> > On Tue, Dec 02, 2014@12:56:16PM -0700, Keith Busch wrote:
> > > +     if (readl(&dev->bar->vs) >= 0x10100) {
> >
> > I think we want an NVME_VERSION() macro that maybe looks something like
> this:
> >
> > #define NVME_VERSION(major, minor) (((major) << 16) | ((minor) << 8))
> >
> > Then you can make this:
> >
> >         if (readl(&dev->bar->vs) >= NVME_VERSION(1, 1))
> 
> Adding #define for NVME_VERSION_MAJOR/MINOR would make it easier to
> maintain than (1,1).

I don't quite understand what you're arguing for.  Could you demonstrate?

[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[Matthew Wilcox]

On Tue, Dec 02, 2014@09:13:30PM +0000, Keith Busch wrote:
> On Tue, 2 Dec 2014, Matthew Wilcox wrote:
> >On Tue, Dec 02, 2014@12:56:16PM -0700, Keith Busch wrote:
> >>+	if (readl(&dev->bar->vs) >= 0x10100) {
> >
> >I think we want an NVME_VERSION() macro that maybe looks something like this:
> >
> >#define NVME_VERSION(major, minor) (((major) << 16) | ((minor) << 8))
> >
> >Then you can make this:
> >
> >	if (readl(&dev->bar->vs) >= NVME_VERSION(1, 1))
> >
> >What do you think?
> 
> I didn't even know what a 1.1 revision was supposed to look like until
> 1.2 clarified that, so a macro to hide the weirdness sounds good.

That was actually fixed in 1.1b ... but, yeah, there was argument within
the committee about exactly what a version number was, whether 1.12 was
greater or less than 1.2, and how we encoded versions.  I was adamant
that no matter what we settled on, the simple comparison had to work;
parsing the version to determine whether it was an earlier or later
version of the spec was not acceptable.

[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[Zakaria Abushima (zabushima)]

Hi Guys,

Off the topic, I have read that namespace in nvme is equivalent to SCSI LUN

So how can I write application to create name spaces up to 32 from the userspace.

In other word is there any application to write SCSI LUN from userspace,

Also, it says that creating name space is a vendor specific ? how can the vendor find out the way to create a namespace ?

Any explanation or clarification ?

Please guide me because I'm completely lost.

Zakaria.

System Architect Intern at Micron.

-----Original Message-----
From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf Of Matthew Wilcox
Sent: 02 December 2014 20:50
To: Keith Busch
Cc: linux-nvme at lists.infradead.org
Subject: Re: [PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

On Tue, Dec 02, 2014@12:56:16PM -0700, Keith Busch wrote:
> +	if (readl(&dev->bar->vs) >= 0x10100) {

I think we want an NVME_VERSION() macro that maybe looks something like this:

#define NVME_VERSION(major, minor) (((major) << 16) | ((minor) << 8))

Then you can make this:

	if (readl(&dev->bar->vs) >= NVME_VERSION(1, 1))

What do you think?


_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

[James R. Bergsten]

First post - please be gentle...

For all present practical purposes you can think of namespaces as logical
units, though logical unit numbers typically start at zero, while namespaces
start at one.

Presently all production drives have one namespace (numbered "one").   The
NN field in the Identify Controller command response tells you the number of
namespaces supported by the device.  They start at one and must be
contiguous numbers (no gaps).

Typically the only SCSI "devices" that support multiple logical units are
arrays which in turn map these logical units to individual drives within the
array (RAID arrays are left as an exercise for the reader).

You "create" (well, more precisely "allocate") a namespace via the Format
NVM command.  The Identify Namespace tells you what topologies are supported
by that namespace (e.g. block size).  Production drives typically ship with
a pre-formatted, empty, single namespace of 512 or 4,096 bytes (no DIF).
Formatting really doesn't "format" it generally just clears out the logical
to physical block associations (effectively unmapping everything).  In
passing, this means if you are doing performance testing, you need to write
something to the device first, otherwise reads won't access flash and you'll
get good, but inaccurate real-world numbers.

So, specific answers to your questions..

You can only "create" as many namespaces as are supported by the device
(today "one").  With the standard Linux driver, it is possible to issue SCSI
commands to an NVMe device which are then emulated by the driver
(nvme-scsi.c).  So one could send a FORMAT UNIT and hope for the best.  If
you want more "devices" on your physical device you can use a virtualization
driver or create software partitions.

Alternately, all of the vendors I know about provide utilities for doing
NVMe specific things such as Format NVM, and there is an "nvme-user" package
(of which I know almost nothing).

But, today, you can pretty much plug the device in, put a filesystem on it,
and off you go.

Hope this helps...
Jim B.

-----Original Message-----
From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf
Of Zakaria Abushima (zabushima)
Sent: Wednesday, December 3, 2014 3:42 AM
To: Matthew Wilcox; Keith Busch
Cc: linux-nvme at lists.infradead.org
Subject: RE: [PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

Hi Guys,

Off the topic, I have read that namespace in nvme is equivalent to SCSI LUN

So how can I write application to create name spaces up to 32 from the
userspace.

In other word is there any application to write SCSI LUN from userspace,

Also, it says that creating name space is a vendor specific ? how can the
vendor find out the way to create a namespace ?

Any explanation or clarification ?

Please guide me because I'm completely lost.

Zakaria.

System Architect Intern at Micron.

-----Original Message-----
From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf
Of Matthew Wilcox
Sent: 02 December 2014 20:50
To: Keith Busch
Cc: linux-nvme at lists.infradead.org
Subject: Re: [PATCH] NVMe: Update SCSI Inquiry VPD 83 translation

On Tue, Dec 02, 2014@12:56:16PM -0700, Keith Busch wrote:
> +	if (readl(&dev->bar->vs) >= 0x10100) {

I think we want an NVME_VERSION() macro that maybe looks something like
this:

#define NVME_VERSION(major, minor) (((major) << 16) | ((minor) << 8))

Then you can make this:

	if (readl(&dev->bar->vs) >= NVME_VERSION(1, 1))

What do you think?


_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme