On Wed, Apr 26, 2017 at 06:23:58PM +0300, Michael S. Tsirkin wrote:
> On Tue, Mar 28, 2017 at 01:16:49PM +1100, David Gibson wrote:
> > virtio-pci and XHCI are "hybrid" devices in the sense that they can present
> > themselves as either PCIe or plain PCI devices depending on the machine
> > and bus they're connected to.
> >
> > For virtio-pci to present as PCIe it requires that it's connected to a PCIe
> > bus and that it's not a root bus - this is to ensure that the device is
> > connected via a PCIe root port or downstream port rather than being a
> > integrated endpoint. Some guests (Windows in particular AIUI) don't really
> > cope with PCIe integrated endpoints.
> >
> > For XHCI it only checks that the bus is PCIe, but that probably means it
> > would cause problems if attached as an integrated devices directly to a
> > PCIe root bus.
> >
> > This patch makes the test consistent between XHCI and virtio-pci, and
> > clarifies things by having them both use a new 'pci_allow_hybrid_pcie()'
> > helper which performs the same check as virtio-pci.
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > hw/pci/pci.c | 7 +++++++
> > hw/usb/hcd-xhci.c | 2 +-
> > hw/virtio/virtio-pci.c | 3 +--
> > include/hw/pci/pci.h | 1 +
> > 4 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index bd8043c..779787b 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -390,6 +390,13 @@ bool pci_bus_is_root(PCIBus *bus)
> > return PCI_BUS_GET_CLASS(bus)->is_root(bus);
> > }
> >
> > +bool pci_allow_hybrid_pcie(PCIDevice *pci_dev)
> > +{
> > + PCIBus *bus = pci_dev->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,
> > const char *name,
> > MemoryRegion *address_space_mem,
>
> I'd prefer pci_allow_hybrid_pci_pcie.
Ok, I've made that change for the next spin (aimed at 2.11, obviously).
>
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index f0af852..a7ff4fd 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -3619,7 +3619,7 @@ static void usb_xhci_realize(struct PCIDevice *dev,
> > Error **errp)
> >
> > PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64,
> > &xhci->mem);
> >
> > - if (pci_bus_is_express(dev->bus) ||
> > + if (pci_allow_hybrid_pcie(dev) ||
> > xhci_get_flag(xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) {
> > ret = pcie_endpoint_cap_init(dev, 0xa0);
> > assert(ret >= 0);
>
> This seems to change the behaviour for xhci on a root bus - what
> am I missing?
Nothing. I didn't consider the backwards compat implications; I'll
fix it for the next spin.
--
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
