* [PATCH v17 01/14] hw/pci: Rename has_power to enabled
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device Akihiko Odaki
` (12 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
The renamed state will not only represent powering state of PFs, but
also represent SR-IOV VF enablement in the future.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/hw/pci/pci.h | 7 ++++++-
include/hw/pci/pci_device.h | 2 +-
hw/pci/pci.c | 14 +++++++-------
hw/pci/pci_host.c | 4 ++--
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index eb26cac81098..fe04b4fafd04 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -678,6 +678,11 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev)
}
MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
-void pci_set_power(PCIDevice *pci_dev, bool state);
+void pci_set_enabled(PCIDevice *pci_dev, bool state);
+
+static inline void pci_set_power(PCIDevice *pci_dev, bool state)
+{
+ pci_set_enabled(pci_dev, state);
+}
#endif
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index 91df40f98973..cbc42349d561 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -57,7 +57,7 @@ typedef struct PCIReqIDCache PCIReqIDCache;
struct PCIDevice {
DeviceState qdev;
bool partially_hotplugged;
- bool has_power;
+ bool enabled;
/* PCI config space */
uint8_t *config;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 87da35ca9ba6..eff408b4e9f4 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1530,7 +1530,7 @@ static void pci_update_mappings(PCIDevice *d)
continue;
new_addr = pci_bar_address(d, i, r->type, r->size);
- if (!d->has_power) {
+ if (!d->enabled) {
new_addr = PCI_BAR_UNMAPPED;
}
@@ -1618,7 +1618,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val_in, int
pci_update_irq_disabled(d, was_irq_disabled);
memory_region_set_enabled(&d->bus_master_enable_region,
(pci_get_word(d->config + PCI_COMMAND)
- & PCI_COMMAND_MASTER) && d->has_power);
+ & PCI_COMMAND_MASTER) && d->enabled);
}
msi_write_config(d, addr, val_in, l);
@@ -2893,18 +2893,18 @@ MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
return msg;
}
-void pci_set_power(PCIDevice *d, bool state)
+void pci_set_enabled(PCIDevice *d, bool state)
{
- if (d->has_power == state) {
+ if (d->enabled == state) {
return;
}
- d->has_power = state;
+ d->enabled = state;
pci_update_mappings(d);
memory_region_set_enabled(&d->bus_master_enable_region,
(pci_get_word(d->config + PCI_COMMAND)
- & PCI_COMMAND_MASTER) && d->has_power);
- if (!d->has_power) {
+ & PCI_COMMAND_MASTER) && d->enabled);
+ if (!d->enabled) {
pci_device_reset(d);
}
}
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index dfe6fe618401..0d82727cc9dd 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -86,7 +86,7 @@ void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
* allowing direct removal of unexposed functions.
*/
if ((pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev)) ||
- !pci_dev->has_power || is_pci_dev_ejected(pci_dev)) {
+ !pci_dev->enabled || is_pci_dev_ejected(pci_dev)) {
return;
}
@@ -111,7 +111,7 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
* allowing direct removal of unexposed functions.
*/
if ((pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev)) ||
- !pci_dev->has_power || is_pci_dev_ejected(pci_dev)) {
+ !pci_dev->enabled || is_pci_dev_ejected(pci_dev)) {
return ~0x0;
}
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 01/14] hw/pci: Rename has_power to enabled Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-28 3:08 ` Shivaprasad G Bhat
2024-10-22 8:36 ` [PATCH v17 03/14] hw/ppc/spapr_pci: Do not reject VFs created after a PF Akihiko Odaki
` (11 subsequent siblings)
13 siblings, 1 reply; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
Disabled means it is a disabled SR-IOV VF and hidden from the guest.
Do not create DT when starting the system and also keep the disabled PCI
device not linked to DRC, which generates DT in case of hotplug.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
hw/ppc/spapr_pci.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 5c0024bef9c4..679a22fe4e79 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1291,8 +1291,7 @@ static void spapr_dt_pci_device_cb(PCIBus *bus, PCIDevice *pdev,
PciWalkFdt *p = opaque;
int err;
- if (p->err) {
- /* Something's already broken, don't keep going */
+ if (p->err || !pdev->enabled) {
return;
}
@@ -1592,10 +1591,10 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
uint32_t slotnr = PCI_SLOT(pdev->devfn);
/*
- * If DR is disabled we don't need to do anything in the case of
- * hotplug or coldplug callbacks.
+ * If DR or the PCI device is disabled we don't need to do anything
+ * in the case of hotplug or coldplug callbacks.
*/
- if (!phb->dr_enabled) {
+ if (!phb->dr_enabled || !pdev->enabled) {
return;
}
@@ -1680,6 +1679,11 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
}
g_assert(drc);
+
+ if (!drc->dev) {
+ return;
+ }
+
g_assert(drc->dev == plugged_dev);
if (!spapr_drc_unplug_requested(drc)) {
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device
2024-10-22 8:36 ` [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device Akihiko Odaki
@ 2024-10-28 3:08 ` Shivaprasad G Bhat
2024-10-28 5:58 ` Akihiko Odaki
0 siblings, 1 reply; 18+ messages in thread
From: Shivaprasad G Bhat @ 2024-10-28 3:08 UTC (permalink / raw)
To: Akihiko Odaki, Philippe Mathieu-Daudé, Michael S. Tsirkin,
Marcel Apfelbaum, Alex Williamson, Cédric Le Goater,
Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost,
Sriram Yagnaraman, Jason Wang, Keith Busch, Klaus Jensen,
Markus Armbruster, Matthew Rosato, Eric Farman,
Harsh Prateek Bora
Cc: qemu-devel, qemu-block
On 10/22/24 2:06 PM, Akihiko Odaki wrote:
> Disabled means it is a disabled SR-IOV VF and hidden from the guest.
> Do not create DT when starting the system and also keep the disabled PCI
> device not linked to DRC, which generates DT in case of hotplug.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> hw/ppc/spapr_pci.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 5c0024bef9c4..679a22fe4e79 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1291,8 +1291,7 @@ static void spapr_dt_pci_device_cb(PCIBus *bus, PCIDevice *pdev,
> PciWalkFdt *p = opaque;
> int err;
>
> - if (p->err) {
> - /* Something's already broken, don't keep going */
> + if (p->err || !pdev->enabled) {
> return;
> }
>
> @@ -1592,10 +1591,10 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
> uint32_t slotnr = PCI_SLOT(pdev->devfn);
>
> /*
> - * If DR is disabled we don't need to do anything in the case of
> - * hotplug or coldplug callbacks.
> + * If DR or the PCI device is disabled we don't need to do anything
> + * in the case of hotplug or coldplug callbacks.
> */
> - if (!phb->dr_enabled) {
> + if (!phb->dr_enabled || !pdev->enabled) {
> return;
> }
Thank you. This is the right place to be called from the hotplug handler
instead of the spapr_pci_dt_populate() unlike I mentioned before..
>
> @@ -1680,6 +1679,11 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
> }
>
> g_assert(drc);
> +
> + if (!drc->dev) {
> + return;
I agree with the change here, but were you able to get to this path? I
don't see
this if condition being entered with,
qemu-system-ppc64 -nographic -serial none -device
spapr-pci-host-bridge,index=4,id=pci.1 -device igb,id=igb0 <<<
'device_del igb0'
Regards,
Shivaprasad
> + }
> +
> g_assert(drc->dev == plugged_dev);
>
> if (!spapr_drc_unplug_requested(drc)) {
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device
2024-10-28 3:08 ` Shivaprasad G Bhat
@ 2024-10-28 5:58 ` Akihiko Odaki
2024-11-04 5:01 ` Shivaprasad G Bhat
0 siblings, 1 reply; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-28 5:58 UTC (permalink / raw)
To: Shivaprasad G Bhat, Philippe Mathieu-Daudé,
Michael S. Tsirkin, Marcel Apfelbaum, Alex Williamson,
Cédric Le Goater, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Sriram Yagnaraman, Jason Wang, Keith Busch,
Klaus Jensen, Markus Armbruster, Matthew Rosato, Eric Farman,
Harsh Prateek Bora
Cc: qemu-devel, qemu-block
On 2024/10/28 12:08, Shivaprasad G Bhat wrote:
>
> On 10/22/24 2:06 PM, Akihiko Odaki wrote:
>> Disabled means it is a disabled SR-IOV VF and hidden from the guest.
>> Do not create DT when starting the system and also keep the disabled PCI
>> device not linked to DRC, which generates DT in case of hotplug.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>> hw/ppc/spapr_pci.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 5c0024bef9c4..679a22fe4e79 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -1291,8 +1291,7 @@ static void spapr_dt_pci_device_cb(PCIBus *bus,
>> PCIDevice *pdev,
>> PciWalkFdt *p = opaque;
>> int err;
>> - if (p->err) {
>> - /* Something's already broken, don't keep going */
>> + if (p->err || !pdev->enabled) {
>> return;
>> }
>> @@ -1592,10 +1591,10 @@ static void spapr_pci_plug(HotplugHandler
>> *plug_handler,
>> uint32_t slotnr = PCI_SLOT(pdev->devfn);
>> /*
>> - * If DR is disabled we don't need to do anything in the case of
>> - * hotplug or coldplug callbacks.
>> + * If DR or the PCI device is disabled we don't need to do anything
>> + * in the case of hotplug or coldplug callbacks.
>> */
>> - if (!phb->dr_enabled) {
>> + if (!phb->dr_enabled || !pdev->enabled) {
>> return;
>> }
>
> Thank you. This is the right place to be called from the hotplug handler
>
> instead of the spapr_pci_dt_populate() unlike I mentioned before..
>
>
>> @@ -1680,6 +1679,11 @@ static void
>> spapr_pci_unplug_request(HotplugHandler *plug_handler,
>> }
>> g_assert(drc);
>> +
>> + if (!drc->dev) {
>> + return;
>
> I agree with the change here, but were you able to get to this path? I
> don't see
>
> this if condition being entered with,
>
> qemu-system-ppc64 -nographic -serial none -device spapr-pci-host-
> bridge,index=4,id=pci.1 -device igb,id=igb0 <<< 'device_del igb0'
No. VFs bypass the hotplug path when unplugging. For context, see
unparent_vfs() in "[PATCH v17 09/14] pcie_sriov: Reuse SR-IOV VF device
instances.
Regards,
Akihiko Odaki
>
>
> Regards,
>
> Shivaprasad
>
>> + }
>> +
>> g_assert(drc->dev == plugged_dev);
>> if (!spapr_drc_unplug_requested(drc)) {
>>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device
2024-10-28 5:58 ` Akihiko Odaki
@ 2024-11-04 5:01 ` Shivaprasad G Bhat
0 siblings, 0 replies; 18+ messages in thread
From: Shivaprasad G Bhat @ 2024-11-04 5:01 UTC (permalink / raw)
To: Akihiko Odaki, Philippe Mathieu-Daudé, Michael S. Tsirkin,
Marcel Apfelbaum, Alex Williamson, Cédric Le Goater,
Paolo Bonzini, Daniel P. Berrangé, Eduardo Habkost,
Sriram Yagnaraman, Jason Wang, Keith Busch, Klaus Jensen,
Markus Armbruster, Matthew Rosato, Eric Farman,
Harsh Prateek Bora
Cc: qemu-devel, qemu-block
On 10/28/24 11:28 AM, Akihiko Odaki wrote:
> On 2024/10/28 12:08, Shivaprasad G Bhat wrote:
>>
>> On 10/22/24 2:06 PM, Akihiko Odaki wrote:
>>> Disabled means it is a disabled SR-IOV VF and hidden from the guest.
>>> Do not create DT when starting the system and also keep the disabled
>>> PCI
>>> device not linked to DRC, which generates DT in case of hotplug.
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>> hw/ppc/spapr_pci.c | 14 +++++++++-----
>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index 5c0024bef9c4..679a22fe4e79 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -1291,8 +1291,7 @@ static void spapr_dt_pci_device_cb(PCIBus
>>> *bus, PCIDevice *pdev,
>>> PciWalkFdt *p = opaque;
>>> int err;
>>> - if (p->err) {
>>> - /* Something's already broken, don't keep going */
>>> + if (p->err || !pdev->enabled) {
>>> return;
>>> }
>>> @@ -1592,10 +1591,10 @@ static void spapr_pci_plug(HotplugHandler
>>> *plug_handler,
>>> uint32_t slotnr = PCI_SLOT(pdev->devfn);
>>> /*
>>> - * If DR is disabled we don't need to do anything in the case of
>>> - * hotplug or coldplug callbacks.
>>> + * If DR or the PCI device is disabled we don't need to do
>>> anything
>>> + * in the case of hotplug or coldplug callbacks.
>>> */
>>> - if (!phb->dr_enabled) {
>>> + if (!phb->dr_enabled || !pdev->enabled) {
>>> return;
>>> }
>>
>> Thank you. This is the right place to be called from the hotplug handler
>>
>> instead of the spapr_pci_dt_populate() unlike I mentioned before..
>>
>>
>>> @@ -1680,6 +1679,11 @@ static void
>>> spapr_pci_unplug_request(HotplugHandler *plug_handler,
>>> }
>>> g_assert(drc);
>>> +
>>> + if (!drc->dev) {
>>> + return;
>>
>> I agree with the change here, but were you able to get to this path?
>> I don't see
>>
>> this if condition being entered with,
>>
>> qemu-system-ppc64 -nographic -serial none -device spapr-pci-host-
>> bridge,index=4,id=pci.1 -device igb,id=igb0 <<< 'device_del igb0'
>
> No. VFs bypass the hotplug path when unplugging. For context, see
> unparent_vfs() in "[PATCH v17 09/14] pcie_sriov: Reuse SR-IOV VF
> device instances.
>
Thanks. Looks good to me.
Reviewed-by: Shivaprasad G Bhat<sbhat@linux.ibm.com>
Tested-by: Shivaprasad G Bhat<sbhat@linux.ibm.com>
Regards,
Shivaprasad
> Regards,
> Akihiko Odaki
>
>>
>>
>> Regards,
>>
>> Shivaprasad
>>
>>> + }
>>> +
>>> g_assert(drc->dev == plugged_dev);
>>> if (!spapr_drc_unplug_requested(drc)) {
>>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v17 03/14] hw/ppc/spapr_pci: Do not reject VFs created after a PF
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 01/14] hw/pci: Rename has_power to enabled Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 02/14] hw/ppc/spapr_pci: Do not create DT for disabled PCI device Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 04/14] s390x/pci: Avoid creating zpci for VFs Akihiko Odaki
` (10 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
A PF may automatically create VFs and the PF may be function 0.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Tested-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
hw/ppc/spapr_pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 679a22fe4e79..31ffe8ba4340 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1568,7 +1568,9 @@ static void spapr_pci_pre_plug(HotplugHandler *plug_handler,
* hotplug, we do not allow functions to be hotplugged to a
* slot that already has function 0 present
*/
- if (plugged_dev->hotplugged && bus->devices[PCI_DEVFN(slotnr, 0)] &&
+ if (plugged_dev->hotplugged &&
+ !pci_is_vf(pdev) &&
+ bus->devices[PCI_DEVFN(slotnr, 0)] &&
PCI_FUNC(pdev->devfn) != 0) {
error_setg(errp, "PCI: slot %d function 0 already occupied by %s,"
" additional functions can no longer be exposed to guest.",
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 04/14] s390x/pci: Avoid creating zpci for VFs
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (2 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 03/14] hw/ppc/spapr_pci: Do not reject VFs created after a PF Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 05/14] s390x/pci: Allow plugging SR-IOV devices Akihiko Odaki
` (9 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
VFs are automatically created by PF, and creating zpci for them will
result in unexpected usage of fids. Currently QEMU does not support
multifunction for s390x so we don't need zpci for VFs anyway.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
hw/s390x/s390-pci-bus.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 40b2567aa706..f06df757a34b 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -1080,6 +1080,16 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
pbdev = s390_pci_find_dev_by_target(s, dev->id);
if (!pbdev) {
+ /*
+ * VFs are automatically created by PF, and creating zpci for them
+ * will result in unexpected usage of fids. Currently QEMU does not
+ * support multifunction for s390x so we don't need zpci for VFs
+ * anyway.
+ */
+ if (pci_is_vf(pdev)) {
+ return;
+ }
+
pbdev = s390_pci_device_new(s, dev->id, errp);
if (!pbdev) {
return;
@@ -1167,7 +1177,10 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
int32_t devfn;
pbdev = s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev));
- g_assert(pbdev);
+ if (!pbdev) {
+ g_assert(pci_is_vf(pci_dev));
+ return;
+ }
s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
pbdev->fh, pbdev->fid);
@@ -1206,7 +1219,11 @@ static void s390_pcihost_unplug_request(HotplugHandler *hotplug_dev,
* we've checked the PCI device already (to prevent endless recursion).
*/
pbdev = s390_pci_find_dev_by_pci(s, PCI_DEVICE(dev));
- g_assert(pbdev);
+ if (!pbdev) {
+ g_assert(pci_is_vf(PCI_DEVICE(dev)));
+ return;
+ }
+
pbdev->pci_unplug_request_processed = true;
qdev_unplug(DEVICE(pbdev), errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 05/14] s390x/pci: Allow plugging SR-IOV devices
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (3 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 04/14] s390x/pci: Avoid creating zpci for VFs Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 06/14] s390x/pci: Check for multifunction after device realization Akihiko Odaki
` (8 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
The guest cannot use VFs due to the lack of multifunction support but
can use PFs.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
hw/s390x/s390-pci-bus.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index f06df757a34b..0c8866f8298d 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -974,7 +974,14 @@ static void s390_pcihost_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
PCIDevice *pdev = PCI_DEVICE(dev);
- if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+ /*
+ * Multifunction is not supported due to the lack of CLP. However,
+ * do not check for multifunction capability for SR-IOV devices because
+ * SR-IOV devices automatically add the multifunction capability whether
+ * the user intends to use the functions other than the PF.
+ */
+ if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION &&
+ !pdev->exp.sriov_cap) {
error_setg(errp, "multifunction not supported in s390");
return;
}
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 06/14] s390x/pci: Check for multifunction after device realization
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (4 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 05/14] s390x/pci: Allow plugging SR-IOV devices Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 07/14] pcie_sriov: Do not manually unrealize Akihiko Odaki
` (7 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
The SR-IOV PFs set the multifunction bit during device realization so
check them after that. There is no functional change because we
explicitly ignore the multifunction bit for SR-IOV devices.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
hw/s390x/s390-pci-bus.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 0c8866f8298d..6ad47f4fbebe 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -971,21 +971,7 @@ static void s390_pcihost_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
"this device");
}
- if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
- PCIDevice *pdev = PCI_DEVICE(dev);
-
- /*
- * Multifunction is not supported due to the lack of CLP. However,
- * do not check for multifunction capability for SR-IOV devices because
- * SR-IOV devices automatically add the multifunction capability whether
- * the user intends to use the functions other than the PF.
- */
- if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION &&
- !pdev->exp.sriov_cap) {
- error_setg(errp, "multifunction not supported in s390");
- return;
- }
- } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
+ if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
S390PCIBusDevice *pbdev = S390_PCI_DEVICE(dev);
if (!s390_pci_alloc_idx(s, pbdev)) {
@@ -1076,6 +1062,18 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
pdev = PCI_DEVICE(dev);
+ /*
+ * Multifunction is not supported due to the lack of CLP. However,
+ * do not check for multifunction capability for SR-IOV devices because
+ * SR-IOV devices automatically add the multifunction capability whether
+ * the user intends to use the functions other than the PF.
+ */
+ if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION &&
+ !pdev->exp.sriov_cap) {
+ error_setg(errp, "multifunction not supported in s390");
+ return;
+ }
+
if (!dev->id) {
/* In the case the PCI device does not define an id */
/* we generate one based on the PCI address */
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 07/14] pcie_sriov: Do not manually unrealize
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (5 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 06/14] s390x/pci: Check for multifunction after device realization Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 08/14] pcie_sriov: Ensure VF addr does not overflow Akihiko Odaki
` (6 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
A device gets automatically unrealized when being unparented.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
hw/pci/pcie_sriov.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index e9b23221d713..499becd5273f 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -204,11 +204,7 @@ static void unregister_vfs(PCIDevice *dev)
trace_sriov_unregister_vfs(dev->name, PCI_SLOT(dev->devfn),
PCI_FUNC(dev->devfn), num_vfs);
for (i = 0; i < num_vfs; i++) {
- Error *err = NULL;
PCIDevice *vf = dev->exp.sriov_pf.vf[i];
- if (!object_property_set_bool(OBJECT(vf), "realized", false, &err)) {
- error_reportf_err(err, "Failed to unplug: ");
- }
object_unparent(OBJECT(vf));
object_unref(OBJECT(vf));
}
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 08/14] pcie_sriov: Ensure VF addr does not overflow
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (6 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 07/14] pcie_sriov: Do not manually unrealize Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 09/14] pcie_sriov: Reuse SR-IOV VF device instances Akihiko Odaki
` (5 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
pci_new() aborts when creating a VF with addr >= PCI_DEVFN_MAX.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
docs/pcie_sriov.txt | 8 +++++---
include/hw/pci/pcie_sriov.h | 5 +++--
hw/net/igb.c | 10 +++++++---
hw/nvme/ctrl.c | 22 ++++++++++++++--------
hw/pci/pcie_sriov.c | 14 ++++++++++++--
5 files changed, 41 insertions(+), 18 deletions(-)
diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
index a47aad0bfab0..ab2142807f79 100644
--- a/docs/pcie_sriov.txt
+++ b/docs/pcie_sriov.txt
@@ -52,9 +52,11 @@ setting up a BAR for a VF.
...
/* Add and initialize the SR/IOV capability */
- pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
- vf_devid, initial_vfs, total_vfs,
- fun_offset, stride);
+ if (!pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
+ vf_devid, initial_vfs, total_vfs,
+ fun_offset, stride, errp)) {
+ return;
+ }
/* Set up individual VF BARs (parameters as for normal BARs) */
pcie_sriov_pf_init_vf_bar( ... )
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index 450cbef6c201..aa704e8f9d9f 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -27,10 +27,11 @@ typedef struct PCIESriovVF {
uint16_t vf_number; /* Logical VF number of this function */
} PCIESriovVF;
-void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
+bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
const char *vfname, uint16_t vf_dev_id,
uint16_t init_vfs, uint16_t total_vfs,
- uint16_t vf_offset, uint16_t vf_stride);
+ uint16_t vf_offset, uint16_t vf_stride,
+ Error **errp);
void pcie_sriov_pf_exit(PCIDevice *dev);
/* Set up a VF bar in the SR/IOV bar area */
diff --git a/hw/net/igb.c b/hw/net/igb.c
index b92bba402e0d..dbbf20682ca1 100644
--- a/hw/net/igb.c
+++ b/hw/net/igb.c
@@ -446,9 +446,13 @@ static void igb_pci_realize(PCIDevice *pci_dev, Error **errp)
pcie_ari_init(pci_dev, 0x150);
- pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, TYPE_IGBVF,
- IGB_82576_VF_DEV_ID, IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
- IGB_VF_OFFSET, IGB_VF_STRIDE);
+ if (!pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, TYPE_IGBVF,
+ IGB_82576_VF_DEV_ID, IGB_MAX_VF_FUNCTIONS,
+ IGB_MAX_VF_FUNCTIONS, IGB_VF_OFFSET, IGB_VF_STRIDE,
+ errp)) {
+ igb_cleanup_msix(s);
+ return;
+ }
pcie_sriov_pf_init_vf_bar(pci_dev, IGBVF_MMIO_BAR_IDX,
PCI_BASE_ADDRESS_MEM_TYPE_64 | PCI_BASE_ADDRESS_MEM_PREFETCH,
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f4e89203c1a6..bd7f1d7bc8d6 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8440,7 +8440,8 @@ out:
return pow2ceil(bar_size);
}
-static void nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset)
+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;
@@ -8449,12 +8450,16 @@ static void nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset)
le16_to_cpu(cap->vifrsm),
NULL, NULL);
- pcie_sriov_pf_init(pci_dev, offset, "nvme", vf_dev_id,
- n->params.sriov_max_vfs, n->params.sriov_max_vfs,
- NVME_VF_OFFSET, NVME_VF_STRIDE);
+ if (!pcie_sriov_pf_init(pci_dev, offset, "nvme", vf_dev_id,
+ n->params.sriov_max_vfs, n->params.sriov_max_vfs,
+ NVME_VF_OFFSET, NVME_VF_STRIDE, errp)) {
+ return false;
+ }
pcie_sriov_pf_init_vf_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_TYPE_64, bar_size);
+
+ return true;
}
static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset)
@@ -8579,6 +8584,11 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
return false;
}
+ if (!pci_is_vf(pci_dev) && n->params.sriov_max_vfs &&
+ !nvme_init_sriov(n, pci_dev, 0x120, errp)) {
+ return false;
+ }
+
nvme_update_msixcap_ts(pci_dev, n->conf_msix_qsize);
pcie_cap_deverr_init(pci_dev);
@@ -8608,10 +8618,6 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
nvme_init_pmr(n, pci_dev);
}
- if (!pci_is_vf(pci_dev) && n->params.sriov_max_vfs) {
- nvme_init_sriov(n, pci_dev, 0x120);
- }
-
return true;
}
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 499becd5273f..91c64c988eb4 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -24,14 +24,22 @@ static PCIDevice *register_vf(PCIDevice *pf, int devfn,
const char *name, uint16_t vf_num);
static void unregister_vfs(PCIDevice *dev);
-void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
+bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
const char *vfname, uint16_t vf_dev_id,
uint16_t init_vfs, uint16_t total_vfs,
- uint16_t vf_offset, uint16_t vf_stride)
+ uint16_t vf_offset, uint16_t vf_stride,
+ Error **errp)
{
+ int32_t devfn = dev->devfn + vf_offset;
uint8_t *cfg = dev->config + offset;
uint8_t *wmask;
+ if (total_vfs &&
+ (uint32_t)devfn + (uint32_t)(total_vfs - 1) * vf_stride >= PCI_DEVFN_MAX) {
+ error_setg(errp, "VF addr overflows");
+ return false;
+ }
+
pcie_add_capability(dev, PCI_EXT_CAP_ID_SRIOV, 1,
offset, PCI_EXT_CAP_SRIOV_SIZEOF);
dev->exp.sriov_cap = offset;
@@ -69,6 +77,8 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
pci_set_word(wmask + PCI_SRIOV_SYS_PGSIZE, 0x553);
qdev_prop_set_bit(&dev->qdev, "multifunction", true);
+
+ return true;
}
void pcie_sriov_pf_exit(PCIDevice *dev)
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 09/14] pcie_sriov: Reuse SR-IOV VF device instances
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (7 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 08/14] pcie_sriov: Ensure VF addr does not overflow Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 10/14] pcie_sriov: Release VFs failed to realize Akihiko Odaki
` (4 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
Disable SR-IOV VF devices by reusing code to power down PCI devices
instead of removing them when the guest requests to disable VFs. This
allows to realize devices and report VF realization errors at PF
realization time.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/hw/pci/pci.h | 5 ---
include/hw/pci/pci_device.h | 15 ++++++++
include/hw/pci/pcie_sriov.h | 1 -
hw/pci/pci.c | 2 +-
hw/pci/pcie_sriov.c | 94 +++++++++++++++++++--------------------------
5 files changed, 55 insertions(+), 62 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index fe04b4fafd04..14a869eeaa71 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -680,9 +680,4 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev)
MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
void pci_set_enabled(PCIDevice *pci_dev, bool state);
-static inline void pci_set_power(PCIDevice *pci_dev, bool state)
-{
- pci_set_enabled(pci_dev, state);
-}
-
#endif
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index cbc42349d561..5dcabb94d679 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -215,6 +215,21 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
}
+static inline void pci_set_power(PCIDevice *pci_dev, bool state)
+{
+ /*
+ * Don't change the enabled state of VFs when powering on/off the device.
+ *
+ * When powering on, VFs must not be enabled immediately but they must
+ * wait until the guest configures SR-IOV.
+ * When powering off, their corresponding PFs will be reset and disable
+ * VFs.
+ */
+ if (!pci_is_vf(pci_dev)) {
+ pci_set_enabled(pci_dev, state);
+ }
+}
+
uint16_t pci_requester_id(PCIDevice *dev);
/* DMA access functions */
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index aa704e8f9d9f..70649236c18a 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -18,7 +18,6 @@
typedef struct PCIESriovPF {
uint16_t num_vfs; /* Number of virtual functions created */
uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */
- const char *vfname; /* Reference to the device type used for the VFs */
PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */
} PCIESriovPF;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index eff408b4e9f4..7ea751fb869d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2904,7 +2904,7 @@ void pci_set_enabled(PCIDevice *d, bool state)
memory_region_set_enabled(&d->bus_master_enable_region,
(pci_get_word(d->config + PCI_COMMAND)
& PCI_COMMAND_MASTER) && d->enabled);
- if (!d->enabled) {
+ if (d->qdev.realized) {
pci_device_reset(d);
}
}
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 91c64c988eb4..f1993bc553c0 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -20,9 +20,16 @@
#include "qapi/error.h"
#include "trace.h"
-static PCIDevice *register_vf(PCIDevice *pf, int devfn,
- const char *name, uint16_t vf_num);
-static void unregister_vfs(PCIDevice *dev);
+static void unparent_vfs(PCIDevice *dev, uint16_t total_vfs)
+{
+ for (uint16_t i = 0; i < total_vfs; i++) {
+ PCIDevice *vf = dev->exp.sriov_pf.vf[i];
+ object_unparent(OBJECT(vf));
+ object_unref(OBJECT(vf));
+ }
+ g_free(dev->exp.sriov_pf.vf);
+ dev->exp.sriov_pf.vf = NULL;
+}
bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
const char *vfname, uint16_t vf_dev_id,
@@ -30,6 +37,7 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
uint16_t vf_offset, uint16_t vf_stride,
Error **errp)
{
+ BusState *bus = qdev_get_parent_bus(&dev->qdev);
int32_t devfn = dev->devfn + vf_offset;
uint8_t *cfg = dev->config + offset;
uint8_t *wmask;
@@ -44,7 +52,6 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
offset, PCI_EXT_CAP_SRIOV_SIZEOF);
dev->exp.sriov_cap = offset;
dev->exp.sriov_pf.num_vfs = 0;
- dev->exp.sriov_pf.vfname = g_strdup(vfname);
dev->exp.sriov_pf.vf = NULL;
pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
@@ -78,14 +85,34 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
qdev_prop_set_bit(&dev->qdev, "multifunction", true);
+ dev->exp.sriov_pf.vf = g_new(PCIDevice *, total_vfs);
+
+ for (uint16_t i = 0; i < total_vfs; i++) {
+ PCIDevice *vf = pci_new(devfn, vfname);
+ vf->exp.sriov_vf.pf = dev;
+ vf->exp.sriov_vf.vf_number = i;
+
+ if (!qdev_realize(&vf->qdev, bus, errp)) {
+ unparent_vfs(dev, i);
+ return false;
+ }
+
+ /* set vid/did according to sr/iov spec - they are not used */
+ pci_config_set_vendor_id(vf->config, 0xffff);
+ pci_config_set_device_id(vf->config, 0xffff);
+
+ dev->exp.sriov_pf.vf[i] = vf;
+ devfn += vf_stride;
+ }
+
return true;
}
void pcie_sriov_pf_exit(PCIDevice *dev)
{
- unregister_vfs(dev);
- g_free((char *)dev->exp.sriov_pf.vfname);
- dev->exp.sriov_pf.vfname = NULL;
+ uint8_t *cfg = dev->config + dev->exp.sriov_cap;
+
+ unparent_vfs(dev, pci_get_word(cfg + PCI_SRIOV_TOTAL_VF));
}
void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int region_num,
@@ -151,38 +178,11 @@ void pcie_sriov_vf_register_bar(PCIDevice *dev, int region_num,
}
}
-static PCIDevice *register_vf(PCIDevice *pf, int devfn, const char *name,
- uint16_t vf_num)
-{
- PCIDevice *dev = pci_new(devfn, name);
- dev->exp.sriov_vf.pf = pf;
- dev->exp.sriov_vf.vf_number = vf_num;
- PCIBus *bus = pci_get_bus(pf);
- Error *local_err = NULL;
-
- qdev_realize(&dev->qdev, &bus->qbus, &local_err);
- if (local_err) {
- error_report_err(local_err);
- return NULL;
- }
-
- /* set vid/did according to sr/iov spec - they are not used */
- pci_config_set_vendor_id(dev->config, 0xffff);
- pci_config_set_device_id(dev->config, 0xffff);
-
- return dev;
-}
-
static void register_vfs(PCIDevice *dev)
{
uint16_t num_vfs;
uint16_t i;
uint16_t sriov_cap = dev->exp.sriov_cap;
- uint16_t vf_offset =
- pci_get_word(dev->config + sriov_cap + PCI_SRIOV_VF_OFFSET);
- uint16_t vf_stride =
- pci_get_word(dev->config + sriov_cap + PCI_SRIOV_VF_STRIDE);
- int32_t devfn = dev->devfn + vf_offset;
assert(sriov_cap > 0);
num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
@@ -190,18 +190,10 @@ static void register_vfs(PCIDevice *dev)
return;
}
- dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
-
trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
PCI_FUNC(dev->devfn), num_vfs);
for (i = 0; i < num_vfs; i++) {
- dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn,
- dev->exp.sriov_pf.vfname, i);
- if (!dev->exp.sriov_pf.vf[i]) {
- num_vfs = i;
- break;
- }
- devfn += vf_stride;
+ pci_set_enabled(dev->exp.sriov_pf.vf[i], true);
}
dev->exp.sriov_pf.num_vfs = num_vfs;
}
@@ -214,12 +206,8 @@ static void unregister_vfs(PCIDevice *dev)
trace_sriov_unregister_vfs(dev->name, PCI_SLOT(dev->devfn),
PCI_FUNC(dev->devfn), num_vfs);
for (i = 0; i < num_vfs; i++) {
- PCIDevice *vf = dev->exp.sriov_pf.vf[i];
- object_unparent(OBJECT(vf));
- object_unref(OBJECT(vf));
+ pci_set_enabled(dev->exp.sriov_pf.vf[i], false);
}
- g_free(dev->exp.sriov_pf.vf);
- dev->exp.sriov_pf.vf = NULL;
dev->exp.sriov_pf.num_vfs = 0;
}
@@ -241,14 +229,10 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
PCI_FUNC(dev->devfn), off, val, len);
if (range_covers_byte(off, len, PCI_SRIOV_CTRL)) {
- if (dev->exp.sriov_pf.num_vfs) {
- if (!(val & PCI_SRIOV_CTRL_VFE)) {
- unregister_vfs(dev);
- }
+ if (val & PCI_SRIOV_CTRL_VFE) {
+ register_vfs(dev);
} else {
- if (val & PCI_SRIOV_CTRL_VFE) {
- register_vfs(dev);
- }
+ unregister_vfs(dev);
}
}
}
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 10/14] pcie_sriov: Release VFs failed to realize
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (8 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 09/14] pcie_sriov: Reuse SR-IOV VF device instances Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 11/14] pcie_sriov: Remove num_vfs from PCIESriovPF Akihiko Odaki
` (3 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
Release VFs failed to realize just as we do in unregister_vfs().
Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
hw/pci/pcie_sriov.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index f1993bc553c0..db087bb9330c 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -93,6 +93,8 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
vf->exp.sriov_vf.vf_number = i;
if (!qdev_realize(&vf->qdev, bus, errp)) {
+ object_unparent(OBJECT(vf));
+ object_unref(vf);
unparent_vfs(dev, i);
return false;
}
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 11/14] pcie_sriov: Remove num_vfs from PCIESriovPF
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (9 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 10/14] pcie_sriov: Release VFs failed to realize Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 12/14] pcie_sriov: Register VFs after migration Akihiko Odaki
` (2 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
num_vfs is not migrated so use PCI_SRIOV_CTRL_VFE and PCI_SRIOV_NUM_VF
instead.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/hw/pci/pcie_sriov.h | 1 -
hw/pci/pcie_sriov.c | 38 +++++++++++++++++++++++++++-----------
hw/pci/trace-events | 2 +-
3 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index 70649236c18a..5148c5b77dd1 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -16,7 +16,6 @@
#include "hw/pci/pci.h"
typedef struct PCIESriovPF {
- uint16_t num_vfs; /* Number of virtual functions created */
uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */
PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */
} PCIESriovPF;
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index db087bb9330c..69609c112e31 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -51,7 +51,6 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
pcie_add_capability(dev, PCI_EXT_CAP_ID_SRIOV, 1,
offset, PCI_EXT_CAP_SRIOV_SIZEOF);
dev->exp.sriov_cap = offset;
- dev->exp.sriov_pf.num_vfs = 0;
dev->exp.sriov_pf.vf = NULL;
pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
@@ -188,29 +187,28 @@ static void register_vfs(PCIDevice *dev)
assert(sriov_cap > 0);
num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
- if (num_vfs > pci_get_word(dev->config + sriov_cap + PCI_SRIOV_TOTAL_VF)) {
- return;
- }
trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
PCI_FUNC(dev->devfn), num_vfs);
for (i = 0; i < num_vfs; i++) {
pci_set_enabled(dev->exp.sriov_pf.vf[i], true);
}
- dev->exp.sriov_pf.num_vfs = num_vfs;
+
+ pci_set_word(dev->wmask + sriov_cap + PCI_SRIOV_NUM_VF, 0);
}
static void unregister_vfs(PCIDevice *dev)
{
- uint16_t num_vfs = dev->exp.sriov_pf.num_vfs;
+ uint8_t *cfg = dev->config + dev->exp.sriov_cap;
uint16_t i;
trace_sriov_unregister_vfs(dev->name, PCI_SLOT(dev->devfn),
- PCI_FUNC(dev->devfn), num_vfs);
- for (i = 0; i < num_vfs; i++) {
+ PCI_FUNC(dev->devfn));
+ for (i = 0; i < pci_get_word(cfg + PCI_SRIOV_TOTAL_VF); i++) {
pci_set_enabled(dev->exp.sriov_pf.vf[i], false);
}
- dev->exp.sriov_pf.num_vfs = 0;
+
+ pci_set_word(dev->wmask + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0xffff);
}
void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
@@ -236,6 +234,17 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
} else {
unregister_vfs(dev);
}
+ } else if (range_covers_byte(off, len, PCI_SRIOV_NUM_VF)) {
+ uint8_t *cfg = dev->config + sriov_cap;
+ uint8_t *wmask = dev->wmask + sriov_cap;
+ uint16_t num_vfs = pci_get_word(cfg + PCI_SRIOV_NUM_VF);
+ uint16_t wmask_val = PCI_SRIOV_CTRL_MSE | PCI_SRIOV_CTRL_ARI;
+
+ if (num_vfs <= pci_get_word(cfg + PCI_SRIOV_TOTAL_VF)) {
+ wmask_val |= PCI_SRIOV_CTRL_VFE;
+ }
+
+ pci_set_word(wmask + PCI_SRIOV_CTRL, wmask_val);
}
}
@@ -252,6 +261,8 @@ void pcie_sriov_pf_reset(PCIDevice *dev)
unregister_vfs(dev);
pci_set_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF, 0);
+ pci_set_word(dev->wmask + sriov_cap + PCI_SRIOV_CTRL,
+ PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE | PCI_SRIOV_CTRL_ARI);
/*
* Default is to use 4K pages, software can modify it
@@ -298,7 +309,7 @@ PCIDevice *pcie_sriov_get_pf(PCIDevice *dev)
PCIDevice *pcie_sriov_get_vf_at_index(PCIDevice *dev, int n)
{
assert(!pci_is_vf(dev));
- if (n < dev->exp.sriov_pf.num_vfs) {
+ if (n < pcie_sriov_num_vfs(dev)) {
return dev->exp.sriov_pf.vf[n];
}
return NULL;
@@ -306,5 +317,10 @@ PCIDevice *pcie_sriov_get_vf_at_index(PCIDevice *dev, int n)
uint16_t pcie_sriov_num_vfs(PCIDevice *dev)
{
- return dev->exp.sriov_pf.num_vfs;
+ uint16_t sriov_cap = dev->exp.sriov_cap;
+ uint8_t *cfg = dev->config + sriov_cap;
+
+ return sriov_cap &&
+ (pci_get_word(cfg + PCI_SRIOV_CTRL) & PCI_SRIOV_CTRL_VFE) ?
+ pci_get_word(cfg + PCI_SRIOV_NUM_VF) : 0;
}
diff --git a/hw/pci/trace-events b/hw/pci/trace-events
index 19643aa8c6b0..e98f575a9d19 100644
--- a/hw/pci/trace-events
+++ b/hw/pci/trace-events
@@ -14,7 +14,7 @@ msix_write_config(char *name, bool enabled, bool masked) "dev %s enabled %d mask
# hw/pci/pcie_sriov.c
sriov_register_vfs(const char *name, int slot, int function, int num_vfs) "%s %02x:%x: creating %d vf devs"
-sriov_unregister_vfs(const char *name, int slot, int function, int num_vfs) "%s %02x:%x: Unregistering %d vf devs"
+sriov_unregister_vfs(const char *name, int slot, int function) "%s %02x:%x: Unregistering vf devs"
sriov_config_write(const char *name, int slot, int fun, uint32_t offset, uint32_t val, uint32_t len) "%s %02x:%x: sriov offset 0x%x val 0x%x len %d"
# pcie.c
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 12/14] pcie_sriov: Register VFs after migration
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (10 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 11/14] pcie_sriov: Remove num_vfs from PCIESriovPF Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 13/14] hw/pci: Use -1 as the default value for rombar Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 14/14] hw/qdev: Remove opts member Akihiko Odaki
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
pcie_sriov doesn't have code to restore its state after migration, but
igb, which uses pcie_sriov, naively claimed its migration capability.
Add code to register VFs after migration and fix igb migration.
Fixes: 3a977deebe6b ("Intrdocue igb device emulation")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/hw/pci/pcie_sriov.h | 2 ++
hw/pci/pci.c | 7 +++++++
hw/pci/pcie_sriov.c | 7 +++++++
3 files changed, 16 insertions(+)
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index 5148c5b77dd1..c5d2d318d330 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -57,6 +57,8 @@ void pcie_sriov_pf_add_sup_pgsize(PCIDevice *dev, uint16_t opt_sup_pgsize);
void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
uint32_t val, int len);
+void pcie_sriov_pf_post_load(PCIDevice *dev);
+
/* Reset SR/IOV */
void pcie_sriov_pf_reset(PCIDevice *dev);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 7ea751fb869d..8120d7c4c5a3 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -735,10 +735,17 @@ static bool migrate_is_not_pcie(void *opaque, int version_id)
return !pci_is_express((PCIDevice *)opaque);
}
+static int pci_post_load(void *opaque, int version_id)
+{
+ pcie_sriov_pf_post_load(opaque);
+ return 0;
+}
+
const VMStateDescription vmstate_pci_device = {
.name = "PCIDevice",
.version_id = 2,
.minimum_version_id = 1,
+ .post_load = pci_post_load,
.fields = (const VMStateField[]) {
VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 69609c112e31..1eb4358256de 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -248,6 +248,13 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
}
}
+void pcie_sriov_pf_post_load(PCIDevice *dev)
+{
+ if (dev->exp.sriov_cap) {
+ register_vfs(dev);
+ }
+}
+
/* Reset SR/IOV */
void pcie_sriov_pf_reset(PCIDevice *dev)
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 13/14] hw/pci: Use -1 as the default value for rombar
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (11 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 12/14] pcie_sriov: Register VFs after migration Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
2024-10-22 8:36 ` [PATCH v17 14/14] hw/qdev: Remove opts member Akihiko Odaki
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
vfio_pci_size_rom() distinguishes whether rombar is explicitly set to 1
by checking dev->opts, bypassing the QOM property infrastructure.
Use -1 as the default value for rombar to tell if the user explicitly
set it to 1. The property is also converted from unsigned to signed.
-1 is signed so it is safe to give it a new meaning. The values in
[2 ^ 31, 2 ^ 32) become invalid, but nobody should have typed these
values by chance.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/pci/pci_device.h | 2 +-
hw/pci/pci.c | 2 +-
hw/vfio/pci.c | 5 ++---
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index 5dcabb94d679..c9d4a347cdfe 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -148,7 +148,7 @@ struct PCIDevice {
uint32_t romsize;
bool has_rom;
MemoryRegion rom;
- uint32_t rom_bar;
+ int32_t rom_bar;
/* INTx routing notifier */
PCIINTxRoutingNotifier intx_routing_notifier;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8120d7c4c5a3..f31a6ae804f2 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -71,7 +71,7 @@ static Property pci_props[] = {
DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, UINT32_MAX),
- DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
+ DEFINE_PROP_INT32("rombar", PCIDevice, rom_bar, -1),
DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present,
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 14bcc725c301..76a150fa2a2b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1012,7 +1012,6 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
{
uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK);
off_t offset = vdev->config_offset + PCI_ROM_ADDRESS;
- DeviceState *dev = DEVICE(vdev);
char *name;
int fd = vdev->vbasedev.fd;
@@ -1046,12 +1045,12 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
}
if (vfio_opt_rom_in_denylist(vdev)) {
- if (dev->opts && qdict_haskey(dev->opts, "rombar")) {
+ if (vdev->pdev.rom_bar > 0) {
warn_report("Device at %s is known to cause system instability"
" issues during option rom execution",
vdev->vbasedev.name);
error_printf("Proceeding anyway since user specified"
- " non zero value for rombar\n");
+ " positive value for rombar\n");
} else {
warn_report("Rom loading for device at %s has been disabled"
" due to system instability issues",
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v17 14/14] hw/qdev: Remove opts member
2024-10-22 8:36 [PATCH v17 00/14] hw/pci: SR-IOV related fixes and improvements Akihiko Odaki
` (12 preceding siblings ...)
2024-10-22 8:36 ` [PATCH v17 13/14] hw/pci: Use -1 as the default value for rombar Akihiko Odaki
@ 2024-10-22 8:36 ` Akihiko Odaki
13 siblings, 0 replies; 18+ messages in thread
From: Akihiko Odaki @ 2024-10-22 8:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster,
Matthew Rosato, Eric Farman, Harsh Prateek Bora,
Shivaprasad G Bhat
Cc: qemu-devel, qemu-block, Akihiko Odaki
It is no longer used.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/qdev-core.h | 4 ----
hw/core/qdev.c | 1 -
system/qdev-monitor.c | 12 +++++++-----
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index aa97c34a4be7..e8254f1cf302 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -237,10 +237,6 @@ struct DeviceState {
* @pending_deleted_expires_ms: optional timeout for deletion events
*/
int64_t pending_deleted_expires_ms;
- /**
- * @opts: QDict of options for the device
- */
- QDict *opts;
/**
* @hotplugged: was device added after PHASE_MACHINE_READY?
*/
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index db36f54d914a..5f47caf983a2 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -706,7 +706,6 @@ static void device_finalize(Object *obj)
dev->canonical_path = NULL;
}
- qobject_unref(dev->opts);
g_free(dev->id);
}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 44994ea0e160..9d5ffa3baaab 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -630,6 +630,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
char *id;
DeviceState *dev = NULL;
BusState *bus = NULL;
+ QDict *properties;
driver = qdict_get_try_str(opts, "driver");
if (!driver) {
@@ -711,13 +712,14 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
}
/* set properties */
- dev->opts = qdict_clone_shallow(opts);
- qdict_del(dev->opts, "driver");
- qdict_del(dev->opts, "bus");
- qdict_del(dev->opts, "id");
+ properties = qdict_clone_shallow(opts);
+ qdict_del(properties, "driver");
+ qdict_del(properties, "bus");
+ qdict_del(properties, "id");
- object_set_properties_from_keyval(&dev->parent_obj, dev->opts, from_json,
+ object_set_properties_from_keyval(&dev->parent_obj, properties, from_json,
errp);
+ qobject_unref(properties);
if (*errp) {
goto err_del_dev;
}
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread