[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [RFC for-2.10 2/3] pci: Allow host bridges t
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [RFC for-2.10 2/3] pci: Allow host bridges to override PCI/PCIe hybrid device behaviour |
Date: |
Tue, 18 Apr 2017 12:21:51 +1000 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
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 <address@hidden>
> > ---
> > 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