hw/nvme: add user controlled 'vid' property

[PATCH] hw/nvme: add user controlled 'vid' property

Posted by Utkarsh Singh 1 month ago

Add a user-configurable 'vid' parameter to allow overriding the default
PCI vendor ID. This enables testing of vendor-specific quirks in guest
operating systems.

The parameter accepts a 16-bit hexadecimal value and defaults to Red Hat
vendor ID (0x1b36). It is 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>
---
 docs/system/devices/nvme.rst | 8 +++++++-
 hw/nvme/ctrl.c               | 8 +++++++-
 hw/nvme/nvme.h               | 1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 98a4401043..0c0c861579 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -51,7 +51,7 @@ 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 ``vid``.
 
 ``ocp`` (default: ``off``)
   The Open Compute Project defines the Datacenter NVMe SSD Specification that
@@ -65,6 +65,12 @@ parameters.
   to more closely impersonate a particular device type. The model name
   can be a maximum of 40 characters in length.
 
+``vid`` (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``.
+
 ``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..2e57fd3585 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8610,6 +8610,11 @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp)
         return false;
     }
 
+    if (params->use_intel_id && params->vid != PCI_VENDOR_ID_REDHAT) {
+        error_setg(errp, "use_intel_id and vid 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,7 +8981,7 @@ 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_vendor_id(pci_conf, n->params.vid);
         pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_REDHAT_NVME);
     }
 
@@ -9401,6 +9406,7 @@ 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("vid", NvmeCtrl, params.vid, PCI_VENDOR_ID_REDHAT),
     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..25d46ae8d0 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -544,6 +544,7 @@ typedef struct NvmeParams {
     char     *serial;
     char     *model;
     char     *firmware_version;
+    uint16_t vid;
     uint32_t num_queues; /* deprecated since 5.1 */
     uint32_t max_ioqpairs;
     uint16_t msix_qsize;
-- 
2.54.0

Re: [PATCH] hw/nvme: add user controlled 'vid' property

Posted by Peter Maydell 1 month ago

On Tue, 23 Jun 2026 at 14:52, Utkarsh Singh <utsingh@redhat.com> wrote:
>
> Add a user-configurable 'vid' parameter to allow overriding the default
> PCI vendor ID. This enables testing of vendor-specific quirks in guest
> operating systems.
>
> The parameter accepts a 16-bit hexadecimal value and defaults to Red Hat
> vendor ID (0x1b36). It is 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>
> ---

> @@ -9401,6 +9406,7 @@ 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("vid", NvmeCtrl, params.vid, PCI_VENDOR_ID_REDHAT),

Unless there's something NVME specific that makes "vid" the right
choice here, I think that "pci-vendor-id" or "vendor-id" would be the
more usual way to name this for QEMU (compare vfio "x-pci-vendor-id",
riscv iommu "vendor-id", the "vendor-id" field in the QGA
GuestDeviceIdPCI packet, "vendor-id" in xen_pvdevice).

Should we also have a "device-id" property here? Being able
to set the vendor-ID only seems unnecessarily limited, since
PCI IDs are a (vendor,device) tuple.

thanks
-- PMM

Re: [PATCH] hw/nvme: add user controlled 'vid' property

Posted by Keith Busch 1 month ago

On Tue, Jun 23, 2026 at 03:01:12PM +0100, Peter Maydell wrote:
> Should we also have a "device-id" property here? Being able
> to set the vendor-ID only seems unnecessarily limited, since
> PCI IDs are a (vendor,device) tuple.

Yes, the quirks are usually based off the same tuple, so there should be
a way to set both when that's the justification for the patch.

Re: [PATCH] hw/nvme: add user controlled 'vid' property

Posted by Utkarsh Singh 1 month ago

On Tue, Jun 23, 2026 at 08:50:09AM -0600, Keith Busch wrote:
>On Tue, Jun 23, 2026 at 03:01:12PM +0100, Peter Maydell wrote:
>> Should we also have a "device-id" property here? Being able
>> to set the vendor-ID only seems unnecessarily limited, since
>> PCI IDs are a (vendor,device) tuple.
>
>Yes, the quirks are usually based off the same tuple, so there should be
>a way to set both when that's the justification for the patch.
Will add device-id and rename "vid" to "pci-vendor-id" in v2.
Thanks
>

Re: [PATCH] hw/nvme: add user controlled 'vid' property

Posted by Peter Maydell 1 month ago

On Tue, 23 Jun 2026 at 18:01, Utkarsh Singh <utsingh@redhat.com> wrote:
>
> On Tue, Jun 23, 2026 at 08:50:09AM -0600, Keith Busch wrote:
> >On Tue, Jun 23, 2026 at 03:01:12PM +0100, Peter Maydell wrote:
> >> Should we also have a "device-id" property here? Being able
> >> to set the vendor-ID only seems unnecessarily limited, since
> >> PCI IDs are a (vendor,device) tuple.
> >
> >Yes, the quirks are usually based off the same tuple, so there should be
> >a way to set both when that's the justification for the patch.
> Will add device-id and rename "vid" to "pci-vendor-id" in v2.

I don't have a strong opinion whether to have the pci- prefix
or not, but please be consistent about whether you have it or
not for vendor-id and device-id.

-- PMM