Re: [Qemu-devel] [RFC for-2.10 2/3] pci: Allow host bridges to override PCI/PCIe hybrid device behaviour

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2890 bytes --]

On Wed, Apr 26, 2017 at 06:29:05PM +0300, Michael S. Tsirkin 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 <david@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)
> >  {
> > -    PCIBus *bus = pci_dev->bus;
> > +    PCIHostState *host_bridge = PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)->qbus.parent);
> > +    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,
> 
> I think I'd prefer adding some flags in PCIBus. While we are at it,
> is_root can become a flag too.

Good idea!  Looking at it the is_root() method really is bogus, I have
a draft patch which replaces it with a flag.  (In fact the other
PCIBusClass methods look kind of bogus too, I'll see if I can find
time to do something about that).

> 
> > 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]