[PATCH] fix: nvme_update_ns_info method should be called even if nvme_ms_ids_equal return false

Mon Apr 11 07:20:13 PDT 2022

After applying the patch provided (based on official kernel 5.17.1), the 
suspend issue has gone, and works!

dmesg shows below:
[    0.489981] nvme 0000:03:00.0: platform quirk: setting simple suspend
[    0.490002] nvme nvme0: pci function 0000:03:00.0
[    0.494897] nvme nvme0: missing or invalid SUBNQN field.
[    0.498902] nvme nvme0: allocated 16 MiB host memory buffer.
[    0.537800] nvme nvme0: 8/0/0 default/read/poll queues
[    0.543528] nvme nvme0: Ignoring bogus Namespace Identifiers
[    0.546709]  nvme0n1: p1 p2 p3 p4
[    0.985096] EXT4-fs (nvme0n1p4): mounted filesystem with ordered data 
mode. Quota mode: none.
[    1.220554] EXT4-fs (nvme0n1p4): re-mounted. Quota mode: none.
[   36.672605] nvme nvme0: 8/0/0 default/read/poll queues
[   36.673717] nvme nvme0: Ignoring bogus Namespace Identifiers

btw, I checked the patch code, I noticed "MAXIO MAP1202" is hard coded 
in pci.c. I think not only MAP1202 has the problem mentioned in this 
thread, but also "MAXIO MAP1002"(Gloway Basic 1T NVMe SSD, which has the 
same suspend issue).
 > +	{ PCI_DEVICE(0x1e4B, 0x1202),   /* MAXIO MAP1202 */
 > +		.driver_data = NVME_QUIRK_BOGUS_NID, },

I highly doubt that MAXIO SSD controller may all have the issue of 
reporting nsid.

在 2022/4/11 14:07, Christoph Hellwig 写道:
> Plase try this patch:
> 
> ---
>  From 95cf9f00488ca5b203cb9205f108b8ec0c30d001 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch at lst.de>
> Date: Mon, 11 Apr 2022 08:05:27 +0200
> Subject: nvme: add a Namespace Identifiers quirk
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> The MAXIO MAP1202 controllers reports completely bogus Namespace
> identifiers that even change after suspend cycles.  Disable using
> the Identifiers entirely.
> 
> Reported-by: 金韬 <me at kingtous.cn>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
>   drivers/nvme/host/core.c | 24 ++++++++++++++++++------
>   drivers/nvme/host/nvme.h |  5 +++++
>   drivers/nvme/host/pci.c  |  2 ++
>   3 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index efb85c6d8e2d5..907f4a2b3de87 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 -1;
>   		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 -1;
>   		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 -1;
>   		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),
>   };
>   
>   /*
> 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),