* quirk broken namespace identifiers
@ 2022-04-12 6:11 Christoph Hellwig
2022-04-12 6:11 ` [PATCH 1/3] nvme: add a quirk to disable " Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Christoph Hellwig @ 2022-04-12 6:11 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: Luis Chamberlain, Klaus Jensen, linux-nvme
Hi all,
this series adds a quirk to ignore namespace identifiers on controllers
where they are completely broken and then quirks two controllers for it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] nvme: add a quirk to disable namespace identifiers
2022-04-12 6:11 quirk broken namespace identifiers Christoph Hellwig
@ 2022-04-12 6:11 ` Christoph Hellwig
2022-04-12 6:56 ` Chaitanya Kulkarni
` (2 more replies)
2022-04-12 6:11 ` [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202 Christoph Hellwig
2022-04-12 6:11 ` [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers Christoph Hellwig
2 siblings, 3 replies; 15+ messages in thread
From: Christoph Hellwig @ 2022-04-12 6:11 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: Luis Chamberlain, Klaus Jensen, linux-nvme
Add a quirk to disable using and exporting namespace identifiers for
controllers where they are broken beyond repair.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 24 ++++++++++++++++++------
drivers/nvme/host/nvme.h | 5 +++++
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index efb85c6d8e2d5..41314dccb2209 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1282,6 +1282,8 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
switch (cur->nidt) {
case NVME_NIDT_EUI64:
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_EUI64_LEN;
if (cur->nidl != NVME_NIDT_EUI64_LEN) {
dev_warn(ctrl->device, "%s %d for NVME_NIDT_EUI64\n",
warn_str, cur->nidl);
@@ -1290,6 +1292,8 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
memcpy(ids->eui64, data + sizeof(*cur), NVME_NIDT_EUI64_LEN);
return NVME_NIDT_EUI64_LEN;
case NVME_NIDT_NGUID:
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_NGUID_LEN;
if (cur->nidl != NVME_NIDT_NGUID_LEN) {
dev_warn(ctrl->device, "%s %d for NVME_NIDT_NGUID\n",
warn_str, cur->nidl);
@@ -1298,6 +1302,8 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
memcpy(ids->nguid, data + sizeof(*cur), NVME_NIDT_NGUID_LEN);
return NVME_NIDT_NGUID_LEN;
case NVME_NIDT_UUID:
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_UUID_LEN;
if (cur->nidl != NVME_NIDT_UUID_LEN) {
dev_warn(ctrl->device, "%s %d for NVME_NIDT_UUID\n",
warn_str, cur->nidl);
@@ -1399,12 +1405,18 @@ static int nvme_identify_ns(struct nvme_ctrl *ctrl, unsigned nsid,
if ((*id)->ncap == 0) /* namespace not allocated or attached */
goto out_free_id;
- if (ctrl->vs >= NVME_VS(1, 1, 0) &&
- !memchr_inv(ids->eui64, 0, sizeof(ids->eui64)))
- memcpy(ids->eui64, (*id)->eui64, sizeof(ids->eui64));
- if (ctrl->vs >= NVME_VS(1, 2, 0) &&
- !memchr_inv(ids->nguid, 0, sizeof(ids->nguid)))
- memcpy(ids->nguid, (*id)->nguid, sizeof(ids->nguid));
+
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID) {
+ dev_info(ctrl->device,
+ "Ignoring bogus Namespace Identifiers\n");
+ } else {
+ if (ctrl->vs >= NVME_VS(1, 1, 0) &&
+ !memchr_inv(ids->eui64, 0, sizeof(ids->eui64)))
+ memcpy(ids->eui64, (*id)->eui64, sizeof(ids->eui64));
+ if (ctrl->vs >= NVME_VS(1, 2, 0) &&
+ !memchr_inv(ids->nguid, 0, sizeof(ids->nguid)))
+ memcpy(ids->nguid, (*id)->nguid, sizeof(ids->nguid));
+ }
return 0;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 1393bbf82d71e..a2b53ca633359 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -144,6 +144,11 @@ enum nvme_quirks {
* encoding the generation sequence number.
*/
NVME_QUIRK_SKIP_CID_GEN = (1 << 17),
+
+ /*
+ * Reports garbage in the namespace identifiers (eui64, nguid, uuid).
+ */
+ NVME_QUIRK_BOGUS_NID = (1 << 18),
};
/*
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202
2022-04-12 6:11 quirk broken namespace identifiers Christoph Hellwig
2022-04-12 6:11 ` [PATCH 1/3] nvme: add a quirk to disable " Christoph Hellwig
@ 2022-04-12 6:11 ` Christoph Hellwig
2022-04-12 6:57 ` Chaitanya Kulkarni
2022-04-12 10:25 ` Sagi Grimberg
2022-04-12 6:11 ` [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers Christoph Hellwig
2 siblings, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2022-04-12 6:11 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg
Cc: Luis Chamberlain, Klaus Jensen, linux-nvme, 金韬
The MAXIO MAP1202 controllers reports completely bogus Namespace
identifiers that even change after suspend cycles. Disable using
the Identifiers entirely.
Reported-by: 金韬 <me@kingtous.cn>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: 金韬 <me@kingtous.cn>
---
drivers/nvme/host/pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index d817ca17463ed..c386c91483505 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3447,6 +3447,8 @@ static const struct pci_device_id nvme_id_table[] = {
.driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
{ PCI_DEVICE(0x2646, 0x2263), /* KINGSTON A2000 NVMe SSD */
.driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
+ { PCI_DEVICE(0x1e4B, 0x1202), /* MAXIO MAP1202 */
+ .driver_data = NVME_QUIRK_BOGUS_NID, },
{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0061),
.driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
{ PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0065),
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers
2022-04-12 6:11 quirk broken namespace identifiers Christoph Hellwig
2022-04-12 6:11 ` [PATCH 1/3] nvme: add a quirk to disable " Christoph Hellwig
2022-04-12 6:11 ` [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202 Christoph Hellwig
@ 2022-04-12 6:11 ` Christoph Hellwig
2022-04-12 6:33 ` Klaus Jensen
2022-04-12 10:25 ` Sagi Grimberg
2 siblings, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2022-04-12 6:11 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg; +Cc: Luis Chamberlain, Klaus Jensen, linux-nvme
Qemu unconditionally reports a UUID, which depending on the qemu version
is either all-null (which is incorrect but harmless) or contains a single
bit set for all controllers. In addition it can also optionally report
a eui64 which needs to be manually set. Disable namespace identifiers
for Qemu controlles entirely even if in some cases they could be set
corretly through manual intervention.
Reported-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c386c91483505..b191e7dcf15ca 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3410,6 +3410,8 @@ static const struct pci_device_id nvme_id_table[] = {
{ PCI_VDEVICE(INTEL, 0x5845), /* Qemu emulated controller */
.driver_data = NVME_QUIRK_IDENTIFY_CNS |
NVME_QUIRK_DISABLE_WRITE_ZEROES, },
+ { PCI_VDEVICE(REDHAT, 0x0010), /* Qemu emulated controller */
+ .driver_data = NVME_QUIRK_BOGUS_NID, },
{ PCI_DEVICE(0x126f, 0x2263), /* Silicon Motion unidentified */
.driver_data = NVME_QUIRK_NO_NS_DESC_LIST, },
{ PCI_DEVICE(0x1bb1, 0x0100), /* Seagate Nytro Flash Storage */
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers
2022-04-12 6:11 ` [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers Christoph Hellwig
@ 2022-04-12 6:33 ` Klaus Jensen
2022-04-12 11:45 ` Christoph Hellwig
2022-04-12 10:25 ` Sagi Grimberg
1 sibling, 1 reply; 15+ messages in thread
From: Klaus Jensen @ 2022-04-12 6:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Keith Busch, Sagi Grimberg, Luis Chamberlain, linux-nvme
[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]
On Apr 12 08:11, Christoph Hellwig wrote:
> Qemu unconditionally reports a UUID, which depending on the qemu version
> is either all-null (which is incorrect but harmless) or contains a single
> bit set for all controllers. In addition it can also optionally report
> a eui64 which needs to be manually set. Disable namespace identifiers
> for Qemu controlles entirely even if in some cases they could be set
> corretly through manual intervention.
>
> Reported-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/host/pci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index c386c91483505..b191e7dcf15ca 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3410,6 +3410,8 @@ static const struct pci_device_id nvme_id_table[] = {
> { PCI_VDEVICE(INTEL, 0x5845), /* Qemu emulated controller */
> .driver_data = NVME_QUIRK_IDENTIFY_CNS |
> NVME_QUIRK_DISABLE_WRITE_ZEROES, },
> + { PCI_VDEVICE(REDHAT, 0x0010), /* Qemu emulated controller */
> + .driver_data = NVME_QUIRK_BOGUS_NID, },
> { PCI_DEVICE(0x126f, 0x2263), /* Silicon Motion unidentified */
> .driver_data = NVME_QUIRK_NO_NS_DESC_LIST, },
> { PCI_DEVICE(0x1bb1, 0x0100), /* Seagate Nytro Flash Storage */
> --
> 2.30.2
>
When I fix this in QEMU properly, can we move this quirk to the
core_quirks and match on firmware revision? That way I don't have to
request a new DID.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] nvme: add a quirk to disable namespace identifiers
2022-04-12 6:11 ` [PATCH 1/3] nvme: add a quirk to disable " Christoph Hellwig
@ 2022-04-12 6:56 ` Chaitanya Kulkarni
2022-04-12 7:01 ` Christoph Hellwig
2022-04-12 10:25 ` Sagi Grimberg
2022-04-12 14:16 ` Keith Busch
2 siblings, 1 reply; 15+ messages in thread
From: Chaitanya Kulkarni @ 2022-04-12 6:56 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Luis Chamberlain, Sagi Grimberg, Keith Busch, Klaus Jensen, linux-nvme
On 4/11/22 23:11, Christoph Hellwig wrote:
> Add a quirk to disable using and exporting namespace identifiers for
> controllers where they are broken beyond repair.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
if possible take that quirk check out see [1] it is but more
code but look much clean, either way looks good.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
[1] totally untested :-
+static int nvme_id_check_quirk(u64 nidt)
+{
+ switch (cur->nidt) {
+ case NVME_NIDT_EUI64:
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_EUI64_LEN;
+ return 0;
+ case NVME_NIDT_NGUID:
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_NGUID_LEN;
+ return 0;
+ case NVME_NIDT_UUID:
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_UUID_LEN;
+ return 0;
+ default:
+ return 0;
+ }
+}
+
static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct
nvme_ns_ids *ids,
struct nvme_ns_id_desc *cur, bool *csi_seen)
{
const char *warn_str = "ctrl returned bogus length:";
void *data = cur;
+ int ret;
+
+ ret = nvme_id_check_quirk(cur->nidt);
+ if (ret)
+ return ret;
switch (cur->nidt) {
case NVME_NIDT_EUI64:
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202
2022-04-12 6:11 ` [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202 Christoph Hellwig
@ 2022-04-12 6:57 ` Chaitanya Kulkarni
2022-04-12 10:25 ` Sagi Grimberg
1 sibling, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2022-04-12 6:57 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Luis Chamberlain, Klaus Jensen, linux-nvme, 金韬
On 4/11/22 23:11, Christoph Hellwig wrote:
> The MAXIO MAP1202 controllers reports completely bogus Namespace
> identifiers that even change after suspend cycles. Disable using
> the Identifiers entirely.
>
> Reported-by: 金韬 <me@kingtous.cn>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Tested-by: 金韬 <me@kingtous.cn>
Looks good.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] nvme: add a quirk to disable namespace identifiers
2022-04-12 6:56 ` Chaitanya Kulkarni
@ 2022-04-12 7:01 ` Christoph Hellwig
0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2022-04-12 7:01 UTC (permalink / raw)
To: Chaitanya Kulkarni
Cc: Christoph Hellwig, Luis Chamberlain, Sagi Grimberg, Keith Busch,
Klaus Jensen, linux-nvme
On Tue, Apr 12, 2022 at 06:56:07AM +0000, Chaitanya Kulkarni wrote:
> + ret = nvme_id_check_quirk(cur->nidt);
> + if (ret)
> + return ret;
We still need to process the list to find the CSI. This is actually
relevant for modern qemu, which supports multiple command sets.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] nvme: add a quirk to disable namespace identifiers
2022-04-12 6:11 ` [PATCH 1/3] nvme: add a quirk to disable " Christoph Hellwig
2022-04-12 6:56 ` Chaitanya Kulkarni
@ 2022-04-12 10:25 ` Sagi Grimberg
2022-04-12 14:16 ` Keith Busch
2 siblings, 0 replies; 15+ messages in thread
From: Sagi Grimberg @ 2022-04-12 10:25 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch; +Cc: Luis Chamberlain, Klaus Jensen, linux-nvme
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202
2022-04-12 6:11 ` [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202 Christoph Hellwig
2022-04-12 6:57 ` Chaitanya Kulkarni
@ 2022-04-12 10:25 ` Sagi Grimberg
1 sibling, 0 replies; 15+ messages in thread
From: Sagi Grimberg @ 2022-04-12 10:25 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch
Cc: Luis Chamberlain, Klaus Jensen, linux-nvme, 金韬
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers
2022-04-12 6:11 ` [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers Christoph Hellwig
2022-04-12 6:33 ` Klaus Jensen
@ 2022-04-12 10:25 ` Sagi Grimberg
1 sibling, 0 replies; 15+ messages in thread
From: Sagi Grimberg @ 2022-04-12 10:25 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch; +Cc: Luis Chamberlain, Klaus Jensen, linux-nvme
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers
2022-04-12 6:33 ` Klaus Jensen
@ 2022-04-12 11:45 ` Christoph Hellwig
2022-04-12 20:43 ` Klaus Jensen
0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2022-04-12 11:45 UTC (permalink / raw)
To: Klaus Jensen
Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg, Luis Chamberlain,
linux-nvme
On Tue, Apr 12, 2022 at 08:33:38AM +0200, Klaus Jensen wrote:
> > + { PCI_VDEVICE(REDHAT, 0x0010), /* Qemu emulated controller */
> > + .driver_data = NVME_QUIRK_BOGUS_NID, },
> > { PCI_DEVICE(0x126f, 0x2263), /* Silicon Motion unidentified */
> > .driver_data = NVME_QUIRK_NO_NS_DESC_LIST, },
> > { PCI_DEVICE(0x1bb1, 0x0100), /* Seagate Nytro Flash Storage */
> > --
> > 2.30.2
> >
>
> When I fix this in QEMU properly, can we move this quirk to the
> core_quirks and match on firmware revision? That way I don't have to
> request a new DID.
Do we known that only one firmware revision reported by Qemu is actually
broken? For now I'd like to get the regression fixed ASAP, and I don't
think Qemu ever fully got identifiers right so far. We can always
change it later if needed.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] nvme: add a quirk to disable namespace identifiers
2022-04-12 6:11 ` [PATCH 1/3] nvme: add a quirk to disable " Christoph Hellwig
2022-04-12 6:56 ` Chaitanya Kulkarni
2022-04-12 10:25 ` Sagi Grimberg
@ 2022-04-12 14:16 ` Keith Busch
2 siblings, 0 replies; 15+ messages in thread
From: Keith Busch @ 2022-04-12 14:16 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Luis Chamberlain, Klaus Jensen, linux-nvme
On Tue, Apr 12, 2022 at 08:11:24AM +0200, Christoph Hellwig wrote:
> @@ -1282,6 +1282,8 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
>
> switch (cur->nidt) {
> case NVME_NIDT_EUI64:
> + if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
> + return NVME_NIDT_EUI64_LEN;
> if (cur->nidl != NVME_NIDT_EUI64_LEN) {
> dev_warn(ctrl->device, "%s %d for NVME_NIDT_EUI64\n",
> warn_str, cur->nidl);
Shouldn't we still verify that cur->nidl is accurate rather than assume the
length? If it is the wrong length, then we have a corrupt descriptor list and
should abort the process.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers
2022-04-12 11:45 ` Christoph Hellwig
@ 2022-04-12 20:43 ` Klaus Jensen
0 siblings, 0 replies; 15+ messages in thread
From: Klaus Jensen @ 2022-04-12 20:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Keith Busch, Sagi Grimberg, Luis Chamberlain, linux-nvme
[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]
On Apr 12 13:45, Christoph Hellwig wrote:
> On Tue, Apr 12, 2022 at 08:33:38AM +0200, Klaus Jensen wrote:
> > > + { PCI_VDEVICE(REDHAT, 0x0010), /* Qemu emulated controller */
> > > + .driver_data = NVME_QUIRK_BOGUS_NID, },
> > > { PCI_DEVICE(0x126f, 0x2263), /* Silicon Motion unidentified */
> > > .driver_data = NVME_QUIRK_NO_NS_DESC_LIST, },
> > > { PCI_DEVICE(0x1bb1, 0x0100), /* Seagate Nytro Flash Storage */
> > > --
> > > 2.30.2
> > >
> >
> > When I fix this in QEMU properly, can we move this quirk to the
> > core_quirks and match on firmware revision? That way I don't have to
> > request a new DID.
>
> Do we known that only one firmware revision reported by Qemu is actually
> broken? For now I'd like to get the regression fixed ASAP, and I don't
> think Qemu ever fully got identifiers right so far. We can always
> change it later if needed.
>
To my knowledge, QEMU has only ever used 1.0 in FR, but I will check.
In any case, I did not want you to change this *now*, so I am totally
fine with this getting changed later if required.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] nvme: add a quirk to disable namespace identifiers
2022-04-13 4:49 quirk broken namespace identifiers v2 Christoph Hellwig
@ 2022-04-13 4:49 ` Christoph Hellwig
0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2022-04-13 4:49 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg
Cc: Luis Chamberlain, Klaus Jensen, linux-nvme, Chaitanya Kulkarni
Add a quirk to disable using and exporting namespace identifiers for
controllers where they are broken beyond repair.
The most directly visible problem with non-unique namespace identifiers
is that they break the /dev/disk/by-id/ links, with the link for a
supposedly unique identifier now pointing to one of multiple possible
namespaces that share the same ID, and a somewhat random selection of
which one actually shows up.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
---
drivers/nvme/host/core.c | 24 ++++++++++++++++++------
drivers/nvme/host/nvme.h | 5 +++++
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index be9fc9818e650..e1846d04817f3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1288,6 +1288,8 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
warn_str, cur->nidl);
return -1;
}
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_EUI64_LEN;
memcpy(ids->eui64, data + sizeof(*cur), NVME_NIDT_EUI64_LEN);
return NVME_NIDT_EUI64_LEN;
case NVME_NIDT_NGUID:
@@ -1296,6 +1298,8 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
warn_str, cur->nidl);
return -1;
}
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_NGUID_LEN;
memcpy(ids->nguid, data + sizeof(*cur), NVME_NIDT_NGUID_LEN);
return NVME_NIDT_NGUID_LEN;
case NVME_NIDT_UUID:
@@ -1304,6 +1308,8 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl, struct nvme_ns_ids *ids,
warn_str, cur->nidl);
return -1;
}
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID)
+ return NVME_NIDT_UUID_LEN;
uuid_copy(&ids->uuid, data + sizeof(*cur));
return NVME_NIDT_UUID_LEN;
case NVME_NIDT_CSI:
@@ -1400,12 +1406,18 @@ static int nvme_identify_ns(struct nvme_ctrl *ctrl, unsigned nsid,
if ((*id)->ncap == 0) /* namespace not allocated or attached */
goto out_free_id;
- if (ctrl->vs >= NVME_VS(1, 1, 0) &&
- !memchr_inv(ids->eui64, 0, sizeof(ids->eui64)))
- memcpy(ids->eui64, (*id)->eui64, sizeof(ids->eui64));
- if (ctrl->vs >= NVME_VS(1, 2, 0) &&
- !memchr_inv(ids->nguid, 0, sizeof(ids->nguid)))
- memcpy(ids->nguid, (*id)->nguid, sizeof(ids->nguid));
+
+ if (ctrl->quirks & NVME_QUIRK_BOGUS_NID) {
+ dev_info(ctrl->device,
+ "Ignoring bogus Namespace Identifiers\n");
+ } else {
+ if (ctrl->vs >= NVME_VS(1, 1, 0) &&
+ !memchr_inv(ids->eui64, 0, sizeof(ids->eui64)))
+ memcpy(ids->eui64, (*id)->eui64, sizeof(ids->eui64));
+ if (ctrl->vs >= NVME_VS(1, 2, 0) &&
+ !memchr_inv(ids->nguid, 0, sizeof(ids->nguid)))
+ memcpy(ids->nguid, (*id)->nguid, sizeof(ids->nguid));
+ }
return 0;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 1393bbf82d71e..a2b53ca633359 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -144,6 +144,11 @@ enum nvme_quirks {
* encoding the generation sequence number.
*/
NVME_QUIRK_SKIP_CID_GEN = (1 << 17),
+
+ /*
+ * Reports garbage in the namespace identifiers (eui64, nguid, uuid).
+ */
+ NVME_QUIRK_BOGUS_NID = (1 << 18),
};
/*
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-04-13 4:49 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-12 6:11 quirk broken namespace identifiers Christoph Hellwig
2022-04-12 6:11 ` [PATCH 1/3] nvme: add a quirk to disable " Christoph Hellwig
2022-04-12 6:56 ` Chaitanya Kulkarni
2022-04-12 7:01 ` Christoph Hellwig
2022-04-12 10:25 ` Sagi Grimberg
2022-04-12 14:16 ` Keith Busch
2022-04-12 6:11 ` [PATCH 2/3] nvme-pci: disable namespace identifiers for the MAXIO MAP1202 Christoph Hellwig
2022-04-12 6:57 ` Chaitanya Kulkarni
2022-04-12 10:25 ` Sagi Grimberg
2022-04-12 6:11 ` [PATCH 3/3] nvme-pci: disable namespace identifiers for Qemu controllers Christoph Hellwig
2022-04-12 6:33 ` Klaus Jensen
2022-04-12 11:45 ` Christoph Hellwig
2022-04-12 20:43 ` Klaus Jensen
2022-04-12 10:25 ` Sagi Grimberg
2022-04-13 4:49 quirk broken namespace identifiers v2 Christoph Hellwig
2022-04-13 4:49 ` [PATCH 1/3] nvme: add a quirk to disable namespace identifiers Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.