[v8,5/6] hw/pci: warn when PCIe device is plugged into non-zero slot of downstream port

Commit Message

Ani Sinha July 5, 2023, 11:59 a.m. UTC

PCIe downstream ports only have a single device 0, so PCI Express devices can
only be plugged into slot 0 on a PCIe port. Add a warning to let users know
when the invalid configuration is used. We may enforce this more strongly later
once we get more clarity on whether we are introducing a bad regression for
users currently using the wrong configuration.

The change has been tested to not break or alter behaviors of ARI capable
devices by instantiating seven vfs on an emulated igb device (the maximum
number of vfs the igb device supports). The vfs are instantiated correctly
and are seen to have non-zero device/slot numbers in the conventional PCI BDF
representation.

CC: [e-mail]
CC: [e-mail]
CC: [e-mail]
CC: [e-mail]

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128929
Signed-off-by: Ani Sinha <[e-mail]>
Reviewed-by: Julia Suvorova <[e-mail]>
---
 hw/pci/pci.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Akihiko Odaki July 6, 2023, 1:48 a.m. UTC | #1

On 2023/07/05 20:59, Ani Sinha wrote:
> PCIe downstream ports only have a single device 0, so PCI Express devices can
> only be plugged into slot 0 on a PCIe port. Add a warning to let users know
> when the invalid configuration is used. We may enforce this more strongly later
> once we get more clarity on whether we are introducing a bad regression for
> users currently using the wrong configuration.
> 
> The change has been tested to not break or alter behaviors of ARI capable
> devices by instantiating seven vfs on an emulated igb device (the maximum
> number of vfs the igb device supports). The vfs are instantiated correctly
> and are seen to have non-zero device/slot numbers in the conventional PCI BDF
> representation.
> 
> CC: [e-mail]
> CC: [e-mail]
> CC: [e-mail]
> CC: [e-mail]
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128929
> Signed-off-by: Ani Sinha <[e-mail]>
> Reviewed-by: Julia Suvorova <[e-mail]>

Reviewed-by: Akihiko Odaki <[e-mail]>

Patch

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e2eb4c3b4a..62b393dfb7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -65,6 +65,7 @@  bool pci_available = true;
 static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static void pcibus_reset(BusState *qbus);
+static bool pcie_has_upstream_port(PCIDevice *dev);
 
 static Property pci_props[] = {
     DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
@@ -2121,6 +2122,25 @@  static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         }
     }
 
+    /*
+     * A PCIe Downstream Port that do not have ARI Forwarding enabled must
+     * associate only Device 0 with the device attached to the bus
+     * representing the Link from the Port (PCIe base spec rev 4.0 ver 0.3,
+     * sec 7.3.1).
+     * With ARI, PCI_SLOT() can return non-zero value as the traditional
+     * 5-bit Device Number and 3-bit Function Number fields in its associated
+     * Routing IDs, Requester IDs and Completer IDs are interpreted as a
+     * single 8-bit Function Number. Hence, ignore ARI capable devices.
+     */
+    if (pci_is_express(pci_dev) &&
+        !pcie_find_capability(pci_dev, PCI_EXT_CAP_ID_ARI) &&
+        pcie_has_upstream_port(pci_dev) &&
+        PCI_SLOT(pci_dev->devfn)) {
+        warn_report("PCI: slot %d is not valid for %s,"
+                    " parent device only allows plugging into slot 0.",
+                    PCI_SLOT(pci_dev->devfn), pci_dev->name);
+    }
+
     if (pci_dev->failover_pair_id) {
         if (!pci_bus_is_express(pci_get_bus(pci_dev))) {
             error_setg(errp, "failover primary device must be on "