[PATCH v2] hw/nvme: add user controlled 'vendor-id' and 'device-id' properties

[PATCH v2] hw/nvme: add user controlled 'vendor-id' and 'device-id' properties

[Utkarsh Singh]

Add user-configurable 'vendor-id' and 'device-id' parameters to
allow overriding the default PCI IDs. This enables testing of
vendor-specific and device-specific quirks in guest operating systems.

Both parameters accept 16-bit hexadecimal values and default to Red Hat
IDs (vendor: 0x1b36, device: 0x0010). They are mutually exclusive with
the 'use-intel-id' flag to prevent conflicting configurations.

Suggested-by: Maurizio Lombardi <mlombard@redhat.com>
Suggested-by: John Meneghini <jmeneghi@redhat.com>
Signed-off-by: Utkarsh Singh <utsingh@redhat.com>
---
Changes in v2:
- Renamed 'vid' parameter to 'vendor-id' for consistency with Qemu 
  naming convention
- Added 'device-id' parameter to allow complete PCI ID override
- Link to v1: https://lore.kernel.org/qemu-devel/20260623131946.46879-1-utsingh@redhat.com  


 docs/system/devices/nvme.rst | 16 +++++++++++++++-
 hw/nvme/ctrl.c               | 16 ++++++++++++++--
 hw/nvme/nvme.h               |  2 ++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 98a4401043..79514afcc5 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -51,7 +51,8 @@ parameters.
 ``use-intel-id`` (default: ``off``)
   Since QEMU 5.2, the device uses a QEMU allocated "Red Hat" PCI Device and
   Vendor ID. Set this to ``on`` to revert to the unallocated Intel ID
-  previously used.
+  previously used. This parameter is mutually exclusive with ``vendor-id``
+  and ``device-id``.
 
 ``ocp`` (default: ``off``)
   The Open Compute Project defines the Datacenter NVMe SSD Specification that
@@ -65,6 +66,19 @@ parameters.
   to more closely impersonate a particular device type. The model name
   can be a maximum of 40 characters in length.
 
+``vendor-id`` (default: ``0x1b36`` for Red Hat)
+  Override the default PCI vendor ID. This can be used when needing to
+  more closely impersonate a particular device type or test vendor-specific
+  quirks. The vendor ID must be a 16-bit hexadecimal value. This parameter
+  is mutually exclusive with ``use-intel-id``.
+
+``device-id`` (default: ``0x0010`` for Red Hat NVMe)
+  Override the default PCI device ID. This can be used when needing to
+  more closely impersonate a particular device type or test device-specific
+  quirks. The device ID must be a 16-bit hexadecimal value. Since PCI IDs
+  are a (vendor, device) tuple, this should typically be used together with
+  ``vendor-id``. This parameter is mutually exclusive with ``use-intel-id``.
+
 ``firmware-version`` (default: current QEMU version number)
   Override the default reported firmware version, which can be used when
   needing to more closely impersonate a particular device type. The version
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 815f39173c..679f3b9930 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8610,6 +8610,14 @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp)
         return false;
     }
 
+    if (params->use_intel_id &&
+        (params->vendor_id != PCI_VENDOR_ID_REDHAT ||
+         params->device_id != PCI_DEVICE_ID_REDHAT_NVME)) {
+        error_setg(errp,
+                   "use_intel_id and vendor-id/device-id are mutually exclusive");
+        return false;
+    }
+
     if (params->model &&
         strlen(params->model) > NVME_ID_CTRL_MN_MAX_LEN) {
         error_setg(errp, "'model' parameter '%s' can be at most '%d' characters",
@@ -8976,8 +8984,8 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
         pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
         pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_NVME);
     } else {
-        pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_REDHAT);
-        pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_REDHAT_NVME);
+        pci_config_set_vendor_id(pci_conf, n->params.vendor_id);
+        pci_config_set_device_id(pci_conf, n->params.device_id);
     }
 
     pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
@@ -9401,6 +9409,10 @@ static const Property nvme_props[] = {
     DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
     DEFINE_PROP_STRING("model", NvmeCtrl, params.model),
     DEFINE_PROP_STRING("firmware-version", NvmeCtrl, params.firmware_version),
+    DEFINE_PROP_UINT16("vendor-id", NvmeCtrl, params.vendor_id,
+                       PCI_VENDOR_ID_REDHAT),
+    DEFINE_PROP_UINT16("device-id", NvmeCtrl, params.device_id,
+                       PCI_DEVICE_ID_REDHAT_NVME),
     DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
     DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
     DEFINE_PROP_UINT32("max_ioqpairs", NvmeCtrl, params.max_ioqpairs, 64),
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 5ef3ebee29..eb8467a641 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -544,6 +544,8 @@ typedef struct NvmeParams {
     char     *serial;
     char     *model;
     char     *firmware_version;
+    uint16_t vendor_id;
+    uint16_t device_id;
     uint32_t num_queues; /* deprecated since 5.1 */
     uint32_t max_ioqpairs;
     uint16_t msix_qsize;
-- 
2.54.0

Re: [PATCH v2] hw/nvme: add user controlled 'vendor-id' and 'device-id' properties

[Utkarsh Singh]

Hi,

Following up on my v2 patch adding vendor-id and device-id properties to QEMU's NVMe emulation.

I've cataloged the real-world NVMe quirks from the Linux kernel that this patch would help simulate. I have tested these quirks with this patch:

PCI-based Quirks (Vendor ID + Device ID):
   1)  Default Red Hat (0x1b36:0x0010) - BOGUS_NID
   2)  Intel QEMU (0x8086:0x5845) - IDENTIFY_CNS, DISABLE_WRITE_ZEROES, BOGUS_NID
   3)  Intel 750 (0x8086:0x0953) - STRIPE_SIZE, DEALLOCATE_ZEROES
   4)  Intel 600P (0x8086:0xf1a5) - NO_DEEPEST_PS, MEDIUM_PRIO_SQ, NO_TEMP_THRESH_CHANGE, DISABLE_WRITE_ZEROES
   5)  Sandisk SN530 (0x15b7:0x5008) - BROKEN_MSI
   6)  Sandisk SN550 (0x15b7:0x5009) - BROKEN_MSI, NO_DEEPEST_PS
   7)  Samsung PM1725 (0x144d:0xa821) - DELAY_BEFORE_CHK_RDY
   8)  Samsung PM1725a (0x144d:0xa822) - DELAY_BEFORE_CHK_RDY, DISABLE_WRITE_ZEROES, IGNORE_DEV_SUBNQN
   9)  Phison E12 (0x1987:0x5012) - BOGUS_NID
   10) Phison E19 (0x1987:0x5019) - DISABLE_WRITE_ZEROES
   11) Silicon Motion (0x126f:0x2262) - NO_DEEPEST_PS, BOGUS_NID

Core Quirks (Vendor ID + Model/Firmware):
   12) Toshiba (0x1179 + model) - NO_APST
   13) Samsung X5 (0x144d + model) - DELAY_BEFORE_CHK_RDY, NO_DEEPEST_PS, IGNORE_DEV_SUBNQN
   14) Kioxia (0x1e0f + model) - NO_APST
   15) LiteON (0x14a4 + firmware) - SIMPLE_SUSPEND

I'd appreciate any review on the patch.

Thanks,
Utkarsh