NACK/Cmnt: [SRU] [Jammy] [PATCH 1/1] nvme-pci: Remove NVME_QUIRK_BOGUS_NID quirk
Stefan Bader
stefan.bader at canonical.com
Wed Mar 12 09:12:06 UTC 2025
On 12.03.25 06:39, Jianlin Lv wrote:
> From: Jianlin Lv <iecedge at gmail.com>
>
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2102060
>
> After updating kernel from 5.15.0-26 to 5.15.0-127, nvme disk WWID changed:
> /dev/disk/by-id/nvme-eui.XXX -> /dev/disk/by-id/nvme-nvme.XXX
>
> the introduction of NVME_QUIRK_BOGUS_NID quirks has changed the device
> attribute file name format.
>
> Considering that naming formats need to have high stability and
> recognizability on production,this patch remove NVME_QUIRK_BOGUS_NID.
>
> Signed-off-by: Jianlin Lv <iecedge at gmail.com>
> ---
Rejected for the following reasons:
This handling for bogus NID replies pre-dates even Jammy release. Just
because one device was added there does not justify ripping it out
completely (and hence break the experience of everyone else).
-Stefan
> drivers/nvme/host/core.c | 24 +++++---------------
> drivers/nvme/host/nvme.h | 5 -----
> drivers/nvme/host/pci.c | 48 +++++-----------------------------------
> 3 files changed, 12 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 4fee20f8ce1d..253f885c9820 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1403,8 +1403,6 @@ 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:
> @@ -1413,8 +1411,6 @@ 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:
> @@ -1423,8 +1419,6 @@ 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:
> @@ -1521,18 +1515,12 @@ 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->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));
> - }
> + 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 0bdd2b5431ac..76a00a16d2ce 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -150,11 +150,6 @@ enum nvme_quirks {
> */
> NVME_QUIRK_SKIP_CID_GEN = (1 << 17),
>
> - /*
> - * Reports garbage in the namespace identifiers (eui64, nguid, uuid).
> - */
> - NVME_QUIRK_BOGUS_NID = (1 << 18),
> -
> /*
> * No temperature thresholds for channels other than 0 (Composite).
> */
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 04fedb27e35a..838e375be484 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3343,8 +3343,7 @@ static const struct pci_device_id nvme_id_table[] = {
> { PCI_VDEVICE(INTEL, 0x0a54), /* Intel P4500/P4600 */
> .driver_data = NVME_QUIRK_STRIPE_SIZE |
> NVME_QUIRK_DEALLOCATE_ZEROES |
> - NVME_QUIRK_IGNORE_DEV_SUBNQN |
> - NVME_QUIRK_BOGUS_NID, },
> + NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> { PCI_VDEVICE(INTEL, 0x0a55), /* Dell Express Flash P4600 */
> .driver_data = NVME_QUIRK_STRIPE_SIZE |
> NVME_QUIRK_DEALLOCATE_ZEROES, },
> @@ -3357,15 +3356,11 @@ static const struct pci_device_id nvme_id_table[] = {
> .driver_data = NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> { PCI_VDEVICE(INTEL, 0x5845), /* Qemu emulated controller */
> .driver_data = NVME_QUIRK_IDENTIFY_CNS |
> - NVME_QUIRK_DISABLE_WRITE_ZEROES |
> - NVME_QUIRK_BOGUS_NID, },
> - { PCI_VDEVICE(REDHAT, 0x0010), /* Qemu emulated controller */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> + NVME_QUIRK_DISABLE_WRITE_ZEROES, },
> { PCI_DEVICE(0x1217, 0x8760), /* O2 Micro 64GB Steam Deck */
> .driver_data = NVME_QUIRK_QDEPTH_ONE },
> { PCI_DEVICE(0x126f, 0x2262), /* Silicon Motion generic */
> - .driver_data = NVME_QUIRK_NO_DEEPEST_PS |
> - NVME_QUIRK_BOGUS_NID, },
> + .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
> { PCI_DEVICE(0x126f, 0x2263), /* Silicon Motion unidentified */
> .driver_data = NVME_QUIRK_NO_NS_DESC_LIST, },
> { PCI_DEVICE(0x1bb1, 0x0100), /* Seagate Nytro Flash Storage */
> @@ -3384,23 +3379,17 @@ static const struct pci_device_id nvme_id_table[] = {
> NVME_QUIRK_DISABLE_WRITE_ZEROES|
> NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> { PCI_DEVICE(0x1987, 0x5016), /* Phison E16 */
> - .driver_data = NVME_QUIRK_IGNORE_DEV_SUBNQN |
> - NVME_QUIRK_BOGUS_NID, },
> + .driver_data = NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> { PCI_DEVICE(0x1b4b, 0x1092), /* Lexar 256 GB SSD */
> .driver_data = NVME_QUIRK_NO_NS_DESC_LIST |
> NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> - { PCI_DEVICE(0x1cc1, 0x33f8), /* ADATA IM2P33F8ABR1 1 TB */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> { PCI_DEVICE(0x10ec, 0x5762), /* ADATA SX6000LNP */
> - .driver_data = NVME_QUIRK_IGNORE_DEV_SUBNQN |
> - NVME_QUIRK_BOGUS_NID, },
> + .driver_data = NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> { PCI_DEVICE(0x1cc1, 0x8201), /* ADATA SX8200PNP 512GB */
> .driver_data = NVME_QUIRK_NO_DEEPEST_PS |
> NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> { PCI_DEVICE(0x1344, 0x5407), /* Micron Technology Inc NVMe SSD */
> .driver_data = NVME_QUIRK_IGNORE_DEV_SUBNQN },
> - { PCI_DEVICE(0x1344, 0x6001), /* Micron Nitro NVMe */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> { PCI_DEVICE(0x1c5c, 0x1504), /* SK Hynix PC400 */
> .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
> { PCI_DEVICE(0x15b7, 0x2001), /* Sandisk Skyhawk */
> @@ -3423,37 +3412,12 @@ static const struct pci_device_id nvme_id_table[] = {
> .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
> { PCI_DEVICE(0x2646, 0x501E), /* KINGSTON OM3PGP4xxxxQ OS21011 NVMe SSD */
> .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
> - { PCI_DEVICE(0x1f40, 0x1202), /* Netac Technologies Co. NV3000 NVMe SSD */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1f40, 0x5236), /* Netac Technologies Co. NV7000 NVMe SSD */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1e4B, 0x1001), /* MAXIO MAP1001 */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1e4B, 0x1002), /* MAXIO MAP1002 */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1e4B, 0x1202), /* MAXIO MAP1202 */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1e4B, 0x1602), /* MAXIO MAP1602 */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1cc1, 0x5350), /* ADATA XPG GAMMIX S50 */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> { PCI_DEVICE(0x1e49, 0x0021), /* ZHITAI TiPro5000 NVMe SSD */
> .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
> { PCI_DEVICE(0x1e49, 0x0041), /* ZHITAI TiPro7000 NVMe SSD */
> .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
> - { PCI_DEVICE(0xc0a9, 0x540a), /* Crucial P2 */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1d97, 0x2263), /* Lexar NM610 */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> { PCI_DEVICE(0x1d97, 0x2269), /* Lexar NM760 */
> - .driver_data = NVME_QUIRK_BOGUS_NID |
> - NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> - { PCI_DEVICE(0x10ec, 0x5763), /* TEAMGROUP T-FORCE CARDEA ZERO Z330 SSD */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x1e4b, 0x1602), /* HS-SSD-FUTURE 2048G */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> - { PCI_DEVICE(0x10ec, 0x5765), /* TEAMGROUP MP33 2TB SSD */
> - .driver_data = NVME_QUIRK_BOGUS_NID, },
> + .driver_data = NVME_QUIRK_IGNORE_DEV_SUBNQN, },
> { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0061),
> .driver_data = NVME_QUIRK_DMA_ADDRESS_BITS_48, },
> { PCI_DEVICE(PCI_VENDOR_ID_AMAZON, 0x0065),
--
- Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 47863 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20250312/65775089/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20250312/65775089/attachment-0001.sig>
More information about the kernel-team
mailing list