Patchwork [7/7] pcie: Mangle types to match topology

login
register
mail settings
Submitter Alex Williamson
Date March 14, 2013, 10:01 p.m.
Message ID <20130314220135.10566.6582.stgit@bling.home>
Download mbox | patch
Permalink /patch/227812/
State New
Headers show

Comments

Alex Williamson - March 14, 2013, 10:01 p.m.
Windows will fail to start drivers for devices with an Endpoint type
PCIe capability attached to a Root Complex (code 10 - Device cannot
start).  The proper type for such a device is Root Complex Integrated
Endpoint.  Devices don't care which they are, so do this conversion
automatically.

This allows the Windows driver to load for nec-usb-xhci when attached
to pcie.0 of a q35 machine.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/pci/pcie.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)
Michael S. Tsirkin - March 17, 2013, 12:19 p.m.
On Thu, Mar 14, 2013 at 04:01:35PM -0600, Alex Williamson wrote:
> Windows will fail to start drivers for devices with an Endpoint type
> PCIe capability attached to a Root Complex (code 10 - Device cannot
> start).  The proper type for such a device is Root Complex Integrated
> Endpoint.  Devices don't care which they are, so do this conversion
> automatically.
> 
> This allows the Windows driver to load for nec-usb-xhci when attached
> to pcie.0 of a q35 machine.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

I think it's a bit ugly from the API perspective,
in that an integrated endpoint is not converted to a regular one.

I think it would be cleaner to have pcie_cap_init do exactly
what it's told to do, maybe failing if you give it an
incorrect configuration. On top of this add
pcie_endpoint_cap_init which sets the type explicitly.
Hmm?

Not critical, so applied as is for now.

> ---
>  hw/pci/pcie.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 485c94c..bcfbae4 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -48,6 +48,19 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
>  
>      assert(pci_is_express(dev));
>  
> +    /*
> +     * Mangle type to convert Endpoints to Root Complex Integrated Endpoints.
> +     * Windows will report Code 10 (device cannot start) for regular Endpoints
> +     * on the Root Complex.
> +     */
> +    if (pci_bus_is_express(dev->bus) && pci_bus_is_root(dev->bus)) {
> +        switch (type) {
> +        case PCI_EXP_TYPE_ENDPOINT:
> +            type = PCI_EXP_TYPE_RC_END;
> +            break;
> +        }
> +    }
> +
>      pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
>                                   PCI_EXP_VER2_SIZEOF);
>      if (pos < 0) {
Alex Williamson - March 18, 2013, 9:17 p.m.
On Sun, 2013-03-17 at 14:19 +0200, Michael S. Tsirkin wrote:
> On Thu, Mar 14, 2013 at 04:01:35PM -0600, Alex Williamson wrote:
> > Windows will fail to start drivers for devices with an Endpoint type
> > PCIe capability attached to a Root Complex (code 10 - Device cannot
> > start).  The proper type for such a device is Root Complex Integrated
> > Endpoint.  Devices don't care which they are, so do this conversion
> > automatically.
> > 
> > This allows the Windows driver to load for nec-usb-xhci when attached
> > to pcie.0 of a q35 machine.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> 
> I think it's a bit ugly from the API perspective,
> in that an integrated endpoint is not converted to a regular one.
> 
> I think it would be cleaner to have pcie_cap_init do exactly
> what it's told to do, maybe failing if you give it an
> incorrect configuration. On top of this add
> pcie_endpoint_cap_init which sets the type explicitly.
> Hmm?
> 
> Not critical, so applied as is for now.

Thanks!  I like the pcie_endpoint_cap_init idea, I'll make a wrapper
that adds that and selects the correct type for the bus.  Thanks,

Alex

Patch

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 485c94c..bcfbae4 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -48,6 +48,19 @@  int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
 
     assert(pci_is_express(dev));
 
+    /*
+     * Mangle type to convert Endpoints to Root Complex Integrated Endpoints.
+     * Windows will report Code 10 (device cannot start) for regular Endpoints
+     * on the Root Complex.
+     */
+    if (pci_bus_is_express(dev->bus) && pci_bus_is_root(dev->bus)) {
+        switch (type) {
+        case PCI_EXP_TYPE_ENDPOINT:
+            type = PCI_EXP_TYPE_RC_END;
+            break;
+        }
+    }
+
     pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
                                  PCI_EXP_VER2_SIZEOF);
     if (pos < 0) {