On Mon, Apr 17, 2017 at 03:30:46PM -0300, Eduardo Habkost wrote: > On Tue, Mar 28, 2017 at 01:16:50PM +1100, David Gibson wrote: > > Currently PCI/PCIe hybrid devices - that is, devices which can appear as > > either plain PCI or PCIe depending on where they're attached - will only > > appear in PCIe mode if they're attached to a PCIe bus via a root port or > > downstream port. > > > > This is correct for "standard" PCIe setups, but there are some platforms > > which need different behaviour (notably "pseries" whose paravirtualized > > PCI host bridges have some idiosyncracies). > > > > This patch allows the host bridge to override the normal behaviour. > > > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > --- > > hw/pci/pci.c | 11 +++++++++-- > > include/hw/pci/pci_host.h | 1 + > > 2 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index 779787b..ac68065 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -392,9 +392,16 @@ bool pci_bus_is_root(PCIBus *bus) > > > > bool pci_allow_hybrid_pcie(PCIDevice *pci_dev) > > Why are we asking the device about allowing hybrid pcie, and not > the bus itself? Shouldn't we be able to query this before the > device is plugged, and before pci_dev->bus is even set? > > In other words, why pci_allow_hyberid_pcie() and > pci_device_get_bus() get a PCIDevice* argument instead of a > PCIBus* argument?
Ah good point. I made it a PCIDevice simply for convenience, but you're right we should be able to query before the device is plugged. > > > { > > - PCIBus *bus = pci_dev->bus; > > + PCIHostState *host_bridge = > > PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)->qbus.parent); > > There's something I don't understand completely: what exactly > guarantees that pci_device_root_bus(...)->qbus.parent is always > going to be a PCI_HOST_BRIDGE? Well, by definition whatever is above the root bus isn't PCI, which pretty much means it has to be a PCI host bridge. A machine could break this assumption, but I think that would be a bug. We use this same pattern to find a PCI device's (or bus's) host bridge in other places - there doesn't appear to be another way. > > + PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_GET_CLASS(host_bridge); > > + > > + if (hc->allow_hybrid_pcie) { > > + return hc->allow_hybrid_pcie(host_bridge, pci_dev); > > + } else { > > + PCIBus *bus = pci_dev->bus; > > > > - return pci_bus_is_express(bus) && !pci_bus_is_root(bus); > > + return pci_bus_is_express(bus) && !pci_bus_is_root(bus); > > + } > > } > > > > void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, > > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > > index ba31595..ad03cca 100644 > > --- a/include/hw/pci/pci_host.h > > +++ b/include/hw/pci/pci_host.h > > @@ -54,6 +54,7 @@ typedef struct PCIHostBridgeClass { > > SysBusDeviceClass parent_class; > > > > const char *(*root_bus_path)(PCIHostState *, PCIBus *); > > + bool (*allow_hybrid_pcie)(PCIHostState *, PCIDevice *); > > } PCIHostBridgeClass; > > > > /* common internal helpers for PCI/PCIe hosts, cut off overflows */ > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature