[PATCH v4 1/2] hw/nvme: Add properties for PCI vendor/device IDs

Saif Abrar posted 2 patches 2 months, 1 week ago
Maintainers: Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Jesper Devantier <foss@defmacro.it>
[PATCH v4 1/2] hw/nvme: Add properties for PCI vendor/device IDs
Posted by Saif Abrar 2 months, 1 week ago
Add properties for user specified PCI vendor, device, subsystem vendor
and subsystem IDs.

e.g. PCI IDs to be specified as follows:
-device nvme,id_vendor=0xABCD,id_device=0xA0B0,id_subsys_vendor=0xEF00,id_subsys=0xEF01

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
v3 -> v4: Resolve merge commits when moving to the latest repo.

 hw/nvme/ctrl.c | 28 +++++++++++++++++++++++++---
 hw/nvme/nvme.h |  5 +++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index cc4593cd42..d857be5496 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8877,7 +8877,10 @@ static bool nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset,
                             Error **errp)
 {
     uint16_t vf_dev_id = n->params.use_intel_id ?
-                         PCI_DEVICE_ID_INTEL_NVME : PCI_DEVICE_ID_REDHAT_NVME;
+                         PCI_DEVICE_ID_INTEL_NVME :
+                         (n->params.id_device ?
+                         n->params.id_device : PCI_DEVICE_ID_REDHAT_NVME);
+
     NvmePriCtrlCap *cap = &n->pri_ctrl_cap;
     uint64_t bar_size = nvme_mbar_size(le16_to_cpu(cap->vqfrsm),
                                       le16_to_cpu(cap->vifrsm),
@@ -8953,8 +8956,22 @@ 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);
+        uint16_t id_vendor = n->params.id_vendor ?
+                             n->params.id_vendor : PCI_VENDOR_ID_REDHAT;
+        pci_config_set_vendor_id(pci_conf, id_vendor);
+
+        uint16_t id_device = n->params.id_device ?
+                             n->params.id_device : PCI_DEVICE_ID_REDHAT_NVME;
+        pci_config_set_device_id(pci_conf, id_device);
+
+        if (n->params.id_subsys_vendor) {
+            pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID,
+                                                   n->params.id_subsys_vendor);
+        }
+
+        if (n->params.id_subsys) {
+            pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, n->params.id_subsys);
+        }
     }
 
     pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
@@ -9409,6 +9426,11 @@ static const Property nvme_props[] = {
     DEFINE_PROP_UINT16("atomic.awun", NvmeCtrl, params.atomic_awun, 0),
     DEFINE_PROP_UINT16("atomic.awupf", NvmeCtrl, params.atomic_awupf, 0),
     DEFINE_PROP_BOOL("ocp", NvmeCtrl, params.ocp, false),
+    DEFINE_PROP_UINT16("id_vendor", NvmeCtrl, params.id_vendor, 0),
+    DEFINE_PROP_UINT16("id_device", NvmeCtrl, params.id_device, 0),
+    DEFINE_PROP_UINT16("id_subsys_vendor", NvmeCtrl,
+                                                    params.id_subsys_vendor, 0),
+    DEFINE_PROP_UINT16("id_subsys", NvmeCtrl, params.id_subsys, 0),
 };
 
 static void nvme_get_smart_warning(Object *obj, Visitor *v, const char *name,
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 8f8c78c850..65fb168bc9 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -571,6 +571,11 @@ typedef struct NvmeParams {
     uint16_t atomic_awun;
     uint16_t atomic_awupf;
     bool     atomic_dn;
+
+    uint16_t id_vendor;
+    uint16_t id_device;
+    uint16_t id_subsys_vendor;
+    uint16_t id_subsys;
 } NvmeParams;
 
 typedef struct NvmeCtrl {
-- 
2.47.3
Re: [PATCH v4 1/2] hw/nvme: Add properties for PCI vendor/device IDs
Posted by Philippe Mathieu-Daudé 2 weeks ago
Hi Saif,

On 27/11/25 13:02, Saif Abrar wrote:
> Add properties for user specified PCI vendor, device, subsystem vendor
> and subsystem IDs.
> 
> e.g. PCI IDs to be specified as follows:
> -device nvme,id_vendor=0xABCD,id_device=0xA0B0,id_subsys_vendor=0xEF00,id_subsys=0xEF01
> 
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
> ---
> v3 -> v4: Resolve merge commits when moving to the latest repo.
> 
>   hw/nvme/ctrl.c | 28 +++++++++++++++++++++++++---
>   hw/nvme/nvme.h |  5 +++++
>   2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index cc4593cd42..d857be5496 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -8877,7 +8877,10 @@ static bool nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset,
>                               Error **errp)
>   {
>       uint16_t vf_dev_id = n->params.use_intel_id ?
> -                         PCI_DEVICE_ID_INTEL_NVME : PCI_DEVICE_ID_REDHAT_NVME;
> +                         PCI_DEVICE_ID_INTEL_NVME :
> +                         (n->params.id_device ?
> +                         n->params.id_device : PCI_DEVICE_ID_REDHAT_NVME);
> +
>       NvmePriCtrlCap *cap = &n->pri_ctrl_cap;
>       uint64_t bar_size = nvme_mbar_size(le16_to_cpu(cap->vqfrsm),
>                                         le16_to_cpu(cap->vifrsm),
> @@ -8953,8 +8956,22 @@ 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);
> +        uint16_t id_vendor = n->params.id_vendor ?
> +                             n->params.id_vendor : PCI_VENDOR_ID_REDHAT;
> +        pci_config_set_vendor_id(pci_conf, id_vendor);
> +
> +        uint16_t id_device = n->params.id_device ?
> +                             n->params.id_device : PCI_DEVICE_ID_REDHAT_NVME;
> +        pci_config_set_device_id(pci_conf, id_device);
> +
> +        if (n->params.id_subsys_vendor) {

Just set it directly?

> +            pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID,
> +                                                   n->params.id_subsys_vendor);
> +        }
> +
> +        if (n->params.id_subsys) {

Just set it directly?

> +            pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, n->params.id_subsys);
> +        }
>       }
>   
>       pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
> @@ -9409,6 +9426,11 @@ static const Property nvme_props[] = {
>       DEFINE_PROP_UINT16("atomic.awun", NvmeCtrl, params.atomic_awun, 0),
>       DEFINE_PROP_UINT16("atomic.awupf", NvmeCtrl, params.atomic_awupf, 0),
>       DEFINE_PROP_BOOL("ocp", NvmeCtrl, params.ocp, false),
> +    DEFINE_PROP_UINT16("id_vendor", NvmeCtrl, params.id_vendor, 0),
> +    DEFINE_PROP_UINT16("id_device", NvmeCtrl, params.id_device, 0),
> +    DEFINE_PROP_UINT16("id_subsys_vendor", NvmeCtrl,
> +                                                    params.id_subsys_vendor, 0),
> +    DEFINE_PROP_UINT16("id_subsys", NvmeCtrl, params.id_subsys, 0),

Just curious (I missed the previous version), why not use the default
value here instead of 0? For example:

   DEFINE_PROP_UINT16("id_device", NvmeCtrl, params.id_device,
                      PCI_DEVICE_ID_REDHAT_NVME),

>   };
>   
>   static void nvme_get_smart_warning(Object *obj, Visitor *v, const char *name,
> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> index 8f8c78c850..65fb168bc9 100644
> --- a/hw/nvme/nvme.h
> +++ b/hw/nvme/nvme.h
> @@ -571,6 +571,11 @@ typedef struct NvmeParams {
>       uint16_t atomic_awun;
>       uint16_t atomic_awupf;
>       bool     atomic_dn;
> +
> +    uint16_t id_vendor;
> +    uint16_t id_device;
> +    uint16_t id_subsys_vendor;
> +    uint16_t id_subsys;
>   } NvmeParams;
>   
>   typedef struct NvmeCtrl {
Re: [PATCH v4 1/2] hw/nvme: Add properties for PCI vendor/device IDs
Posted by Saif Abrar 2 days, 3 hours ago
Hello Philippe,

Changes suggested in my commits are now picked up by Jesper Devantier 
<foss@defmacro.it>

in the patch

https://lore.kernel.org/qemu-devel/20260106-q-saif-pci-oui-props-v1-1-c8a1a4ec0dfc@samsung.com/


Regards,

Saif


On 25-01-2026 11:46 pm, Philippe Mathieu-Daudé wrote:
> Hi Saif,
>
> On 27/11/25 13:02, Saif Abrar wrote:
>> Add properties for user specified PCI vendor, device, subsystem vendor
>> and subsystem IDs.
>>
>> e.g. PCI IDs to be specified as follows:
>> -device 
>> nvme,id_vendor=0xABCD,id_device=0xA0B0,id_subsys_vendor=0xEF00,id_subsys=0xEF01
>>
>> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
>> ---
>> v3 -> v4: Resolve merge commits when moving to the latest repo.
>>
>>   hw/nvme/ctrl.c | 28 +++++++++++++++++++++++++---
>>   hw/nvme/nvme.h |  5 +++++
>>   2 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>> index cc4593cd42..d857be5496 100644
>> --- a/hw/nvme/ctrl.c
>> +++ b/hw/nvme/ctrl.c
>> @@ -8877,7 +8877,10 @@ static bool nvme_init_sriov(NvmeCtrl *n, 
>> PCIDevice *pci_dev, uint16_t offset,
>>                               Error **errp)
>>   {
>>       uint16_t vf_dev_id = n->params.use_intel_id ?
>> -                         PCI_DEVICE_ID_INTEL_NVME : 
>> PCI_DEVICE_ID_REDHAT_NVME;
>> +                         PCI_DEVICE_ID_INTEL_NVME :
>> +                         (n->params.id_device ?
>> +                         n->params.id_device : 
>> PCI_DEVICE_ID_REDHAT_NVME);
>> +
>>       NvmePriCtrlCap *cap = &n->pri_ctrl_cap;
>>       uint64_t bar_size = nvme_mbar_size(le16_to_cpu(cap->vqfrsm),
>> le16_to_cpu(cap->vifrsm),
>> @@ -8953,8 +8956,22 @@ 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);
>> +        uint16_t id_vendor = n->params.id_vendor ?
>> +                             n->params.id_vendor : 
>> PCI_VENDOR_ID_REDHAT;
>> +        pci_config_set_vendor_id(pci_conf, id_vendor);
>> +
>> +        uint16_t id_device = n->params.id_device ?
>> +                             n->params.id_device : 
>> PCI_DEVICE_ID_REDHAT_NVME;
>> +        pci_config_set_device_id(pci_conf, id_device);
>> +
>> +        if (n->params.id_subsys_vendor) {
>
> Just set it directly?
>
>> +            pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID,
>> + n->params.id_subsys_vendor);
>> +        }
>> +
>> +        if (n->params.id_subsys) {
>
> Just set it directly?
>
>> +            pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, 
>> n->params.id_subsys);
>> +        }
>>       }
>>         pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_EXPRESS);
>> @@ -9409,6 +9426,11 @@ static const Property nvme_props[] = {
>>       DEFINE_PROP_UINT16("atomic.awun", NvmeCtrl, params.atomic_awun, 
>> 0),
>>       DEFINE_PROP_UINT16("atomic.awupf", NvmeCtrl, 
>> params.atomic_awupf, 0),
>>       DEFINE_PROP_BOOL("ocp", NvmeCtrl, params.ocp, false),
>> +    DEFINE_PROP_UINT16("id_vendor", NvmeCtrl, params.id_vendor, 0),
>> +    DEFINE_PROP_UINT16("id_device", NvmeCtrl, params.id_device, 0),
>> +    DEFINE_PROP_UINT16("id_subsys_vendor", NvmeCtrl,
>> + params.id_subsys_vendor, 0),
>> +    DEFINE_PROP_UINT16("id_subsys", NvmeCtrl, params.id_subsys, 0),
>
> Just curious (I missed the previous version), why not use the default
> value here instead of 0? For example:
>
>   DEFINE_PROP_UINT16("id_device", NvmeCtrl, params.id_device,
>                      PCI_DEVICE_ID_REDHAT_NVME),
>
>>   };
>>     static void nvme_get_smart_warning(Object *obj, Visitor *v, const 
>> char *name,
>> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
>> index 8f8c78c850..65fb168bc9 100644
>> --- a/hw/nvme/nvme.h
>> +++ b/hw/nvme/nvme.h
>> @@ -571,6 +571,11 @@ typedef struct NvmeParams {
>>       uint16_t atomic_awun;
>>       uint16_t atomic_awupf;
>>       bool     atomic_dn;
>> +
>> +    uint16_t id_vendor;
>> +    uint16_t id_device;
>> +    uint16_t id_subsys_vendor;
>> +    uint16_t id_subsys;
>>   } NvmeParams;
>>     typedef struct NvmeCtrl {
>