[PATCHv2] nvme: add bug report info for global duplicate id

[PATCHv2] nvme: add bug report info for global duplicate id

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

The recent global id check is finding poorly implemented devices in the
wild. Include relavant device information in the output to help quicken
an appropriate quirk patch.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v1->v2:

  Allow transport drivers to define their own device info op. For PCI,
  this will let us print the device id in addition to the vendor.

 drivers/nvme/host/core.c |  1 +
 drivers/nvme/host/nvme.h | 28 ++++++++++++++++++++++++++++
 drivers/nvme/host/pci.c  | 16 ++++++++++++++++
 3 files changed, 45 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 24165daee3c8..5c9f3648c7d6 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3863,6 +3863,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
 	if (ret) {
 		dev_err(ctrl->device,
 			"globally duplicate IDs for nsid %d\n", nsid);
+		nvme_print_device_info(ctrl);
 		return ret;
 	}
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9b72b6ecf33c..0da94b233fed 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -503,6 +503,7 @@ struct nvme_ctrl_ops {
 	void (*submit_async_event)(struct nvme_ctrl *ctrl);
 	void (*delete_ctrl)(struct nvme_ctrl *ctrl);
 	int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size);
+	void (*print_device_info)(struct nvme_ctrl *ctrl);
 };
 
 /*
@@ -548,6 +549,33 @@ static inline struct request *nvme_cid_to_rq(struct blk_mq_tags *tags,
 	return blk_mq_tag_to_rq(tags, nvme_tag_from_cid(command_id));
 }
 
+/*
+ * Return the length of the string without the space padding
+ */
+static inline int nvme_strlen(char *s, int len)
+{
+	while (s[len - 1] == ' ')
+		len--;
+	return len;
+}
+
+static inline void nvme_print_device_info(struct nvme_ctrl *ctrl)
+{
+	struct nvme_subsystem *subsys = ctrl->subsys;
+
+	if (ctrl->ops->print_device_info) {
+		ctrl->ops->print_device_info(ctrl);
+		return;
+	}
+
+	dev_err(ctrl->device,
+		"VID:%04x model:%.*s firmware:%.*s\n", subsys->vendor_id,
+		nvme_strlen(subsys->model, sizeof(subsys->model)),
+		subsys->model, nvme_strlen(subsys->firmware_rev,
+					   sizeof(subsys->firmware_rev)),
+		subsys->firmware_rev);
+}
+
 #ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
 void nvme_fault_inject_init(struct nvme_fault_inject *fault_inj,
 			    const char *dev_name);
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 9d963f6cdbae..be053d943731 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2984,6 +2984,21 @@ static int nvme_pci_get_address(struct nvme_ctrl *ctrl, char *buf, int size)
 	return snprintf(buf, size, "%s\n", dev_name(&pdev->dev));
 }
 
+
+static void nvme_pci_print_device_info(struct nvme_ctrl *ctrl)
+{
+	struct pci_dev *pdev = to_pci_dev(to_nvme_dev(ctrl)->dev);
+	struct nvme_subsystem *subsys = ctrl->subsys;
+
+	dev_err(ctrl->device,
+		"VID:DID %04x:%04x model:%.*s firmware:%.*s\n",
+		pdev->vendor, pdev->device,
+		nvme_strlen(subsys->model, sizeof(subsys->model)),
+		subsys->model, nvme_strlen(subsys->firmware_rev,
+					   sizeof(subsys->firmware_rev)),
+		subsys->firmware_rev);
+}
+
 static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
 	.name			= "pcie",
 	.module			= THIS_MODULE,
@@ -2995,6 +3010,7 @@ static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
 	.free_ctrl		= nvme_pci_free_ctrl,
 	.submit_async_event	= nvme_pci_submit_async_event,
 	.get_address		= nvme_pci_get_address,
+	.print_device_info	= nvme_pci_print_device_info,
 };
 
 static int nvme_dev_map(struct nvme_dev *dev)
-- 
2.30.2

Re: [PATCHv2] nvme: add bug report info for global duplicate id

[Keith Busch]

On Tue, Jun 07, 2022 at 08:30:29AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The recent global id check is finding poorly implemented devices in the
> wild. Include relavant device information in the output to help quicken
> an appropriate quirk patch.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>

This problem seems more widespread than expected, so I think it would help to
apply this patch now.

And since this appears to be a gap in compliance testing, I feel I should
report this. I think UNH is still handling compliance certification.

Re: [PATCHv2] nvme: add bug report info for global duplicate id

[Christoph Hellwig]

On Mon, Jun 13, 2022 at 08:53:30AM -0600, Keith Busch wrote:
> This problem seems more widespread than expected, so I think it would help to
> apply this patch now.

Yes, that's the plan.  But as Jens was out for last week I didn't
bother with a pull request.

> And since this appears to be a gap in compliance testing, I feel I should
> report this. I think UNH is still handling compliance certification.

UNH provides the test suite, but the specs are handled in the nvme
interoperability workgroup.  I already asked them to test this, but
nothing happened, but I think you asking again is definitively a good
idea.

Re: [PATCHv2] nvme: add bug report info for global duplicate id

[Keith Busch]

On Mon, Jun 13, 2022 at 05:04:32PM +0200, Christoph Hellwig wrote:
> On Mon, Jun 13, 2022 at 08:53:30AM -0600, Keith Busch wrote:
> 
> > And since this appears to be a gap in compliance testing, I feel I should
> > report this. I think UNH is still handling compliance certification.
> 
> UNH provides the test suite, but the specs are handled in the nvme
> interoperability workgroup.  I already asked them to test this, but
> nothing happened, but I think you asking again is definitively a good
> idea.

Message sent, and I cc'ed the email you use for nvme workgroup discussion.

Re: [PATCHv2] nvme: add bug report info for global duplicate id

[Christoph Hellwig]

Thanks,

applied to nvme-5.19.