[PULL 05/18] pci: don't skip function 0 occupancy verification for devfn auto assign

Michael S. Tsirkin posted 18 patches 2 months, 1 week ago
There is a newer version of this series
[PULL 05/18] pci: don't skip function 0 occupancy verification for devfn auto assign
Posted by Michael S. Tsirkin 2 months, 1 week ago
From: Dongli Zhang <dongli.zhang@oracle.com>

When the devfn is already assigned in the command line, the
do_pci_register_device() may verify if the function 0 is already occupied.

However, when devfn < 0, the verification is skipped because it is part of
the last "else if".

For instance, suppose there is already a device at addr=00.00 of a port.

-device pcie-root-port,bus=pcie.0,chassis=115,id=port01,addr=0e.00 \
-device virtio-net-pci,bus=port01,id=vnet01,addr=00.00 \

When 'addr' is specified for the 2nd device, the hotplug is denied.

(qemu) device_add virtio-net-pci,bus=port01,id=vnet02,addr=01.00
Error: PCI: slot 0 function 0 already occupied by virtio-net-pci, new func virtio-net-pci cannot be exposed to guest.

When 'addr' is automatically assigned, the hotplug is not denied. This is
because the verification is skipped.

(qemu) device_add virtio-net-pci,bus=port01,id=vnet02
warning: PCI: slot 1 is not valid for virtio-net-pci, parent device only allows plugging into slot 0.

Fix the issue by moving the verification into an independent 'if'
statement.

Fixes: 3f1e1478db2d ("enable multi-function hot-add")
Reported-by: Aswin Unnikrishnan <aswin.u.unnikrishnan@oracle.com>
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
Message-Id: <20240708041056.54504-1-dongli.zhang@oracle.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pci.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d2caf3ee8b..87da35ca9b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1181,14 +1181,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
                    PCI_SLOT(devfn), PCI_FUNC(devfn), name,
                    bus->devices[devfn]->name, bus->devices[devfn]->qdev.id);
         return NULL;
-    } /*
-       * Populating function 0 triggers a scan from the guest that
-       * exposes other non-zero functions. Hence we need to ensure that
-       * function 0 wasn't added yet.
-       */
-    else if (dev->hotplugged &&
-             !pci_is_vf(pci_dev) &&
-             pci_get_function_0(pci_dev)) {
+    }
+
+    /*
+     * Populating function 0 triggers a scan from the guest that
+     * exposes other non-zero functions. Hence we need to ensure that
+     * function 0 wasn't added yet.
+     */
+    if (dev->hotplugged && !pci_is_vf(pci_dev) &&
+        pci_get_function_0(pci_dev)) {
         error_setg(errp, "PCI: slot %d function 0 already occupied by %s,"
                    " new func %s cannot be exposed to guest.",
                    PCI_SLOT(pci_get_function_0(pci_dev)->devfn),
-- 
MST