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.
> 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?
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index f9b7244..9b34673 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1737,8 +1737,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
> Error **errp)
> {
> VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
> VirtioPCIClass *k = VIRTIO_PCI_GET_CLASS(pci_dev);
> - bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
> - !pci_bus_is_root(pci_dev->bus);
> + bool pcie_port = pci_allow_hybrid_pcie(pci_dev);
>
> if (!kvm_has_many_ioeventfds()) {
> proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
I don't actually remember why do we disable pcie on a root port.
Marcel, do you happen to know?
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index a37a2d5..7b9a40f 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -453,6 +453,7 @@ void pci_for_each_bus(PCIBus *bus,
>
> PCIBus *pci_find_primary_bus(void);
> PCIBus *pci_device_root_bus(const PCIDevice *d);
> +bool pci_allow_hybrid_pcie(PCIDevice *pci_dev);
> const char *pci_root_bus_path(PCIDevice *dev);
> PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
> int pci_qdev_find_device(const char *id, PCIDevice **pdev);
> --
> 2.9.3
