Re: [PATCH v2 1/4] conf: restrict what type of buses will accept a pci-bridge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/16/2016 10:17 AM, Andrea Bolognani wrote:
On Mon, 2016-08-15 at 01:50 -0400, Laine Stump wrote:
A pci-bridge has *almost* the same rules as a legacy PCI endpoint
device for where it can be automatically connected, and until now both
had been considered identical. There is one pairing that is okay when
specifically requested by the user (i.e. manual assignment), but we
want to avoid it when auto-assigning addresses - plugging a pci-bridge
s/it //

Possibly, I don't know, you're the native speaker ;)

directly into pcie-root (it is cleaner to plug in a dmi-to-pci-bridge,
then plug the pci-bridge into that).
In order to allow that difference, this patch makes a separate
CONNECT_TYPE for pci-bridge, and uses it to restrict auto-assigned
addresses for pci-bridges to be only on pci-root, pci-expander-bus,
dmi-to-pci-bridge, or on another pci-bridge.
NB: As with other discouraged-but-seem-to-work configurations
(e.g. plugging a legacy PCI device into a pcie-root-port) if someone
*really* wants to, they can still force a pci-bridge to be plugged
into pcie-root (by manually specifying its PCI address.)
---
   src/conf/domain_addr.c | 29 ++++++++++++++++++++++-------
   src/conf/domain_addr.h |  4 +++-
   2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index c533edb..88e44df 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -51,11 +51,14 @@ virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model)
           return 0;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
-        /* pci-bridge is treated like a standard
-         * PCI endpoint device, because it can plug into any
-         * standard PCI slot (it just can't be hotplugged).
+        /* pci-bridge is treated like a standard PCI endpoint device
+         * when its address is manually assigned, but needs special
+         * treatment when auto-assigning addresses (in that case we
+         * avoid plugging into anything except pci-root,
+         * dmi-to-pci-bridge, pci-expander-bus, or another
+         * pci-bridge).
            */
-        return VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
+        return VIR_PCI_CONNECT_TYPE_PCI_BRIDGE;
As mentioned as part of my review of the previous PCIe series,
I don't think this kind of comment should be here. The function
just maps controller model to connect type, and if anything
this change makes the mapping even more straighforward.

That's strange. I could *swear* that I removed that comment when I was rebasing. Of course then I created a completely new branch and cherry-picked the commits I wanted using the commit id's from an instance of gitk that was sitting on the desktop - maybe it had been run prior to the deletion and I hadn't reloaded it...



See below.

       case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
           return VIR_PCI_CONNECT_TYPE_PCI_EXPANDER_BUS;
@@ -110,6 +113,12 @@ virDomainPCIAddressFlagsCompatible(virPCIDeviceAddressPtr addr,
            */
           if (devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)
               busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE;
+        /* if the device is a pci-bridge, allow manually
+         * assigning to any bus that would also accept a
+         * standard PCI device.
+         */
+        if (devFlags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE)
+            devFlags |= VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
This is the kind of comment we need :)

ACK with the nits fixed.

--
Andrea Bolognani / Red Hat / Virtualization


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]