diff mbox series

[RFC,2/3] drivers/char: search all buses for xhci

Message ID 34d3e4e4067b86183e6d834c8bc93736f058fe19.1670724490.git-series.marmarek@invisiblethingslab.com (mailing list archive)
State New, archived
Headers show
Series Try to fix XHCI console on AMD systems (help needed) | expand

Commit Message

Marek Marczykowski-Górecki Dec. 11, 2022, 2:10 a.m. UTC
On (at least some) AMD systems, XHCI isn't on bus 0 (in my case, it was
bus 4). Search all of them.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 xen/drivers/char/xhci-dbc.c | 84 +++++++++++++++++++++++++++++---------
 1 file changed, 66 insertions(+), 18 deletions(-)

Comments

Jan Beulich Dec. 14, 2022, 10:51 a.m. UTC | #1
On 11.12.2022 03:10, Marek Marczykowski-Górecki wrote:
> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -286,39 +286,87 @@ static void *dbc_sys_map_xhc(uint64_t phys, size_t size)
>      return fix_to_virt(FIX_XHCI_END);
>  }
>  
> +static void xhci_bios_handoff(struct dbc *dbc)
> +{
> +    const uint32_t __iomem *xcap;
> +    uint32_t xcap_val;
> +    uint32_t next;
> +    uint32_t id = 0;
> +    const void __iomem *mmio = dbc->xhc_mmio;
> +    const uint32_t __iomem *hccp1 = mmio + 0x10;
> +    const uint32_t LEGACY_ID = 0x1;
> +    int ttl = 48;
> +    int timeout = 10000;
> +
> +    xcap = mmio;
> +    /*
> +     * This is initially an offset to the first capability. All the offsets
> +     * (both in HCCP1 and then next capability pointer) are dword-based.
> +     */
> +    next = (readl(hccp1) & 0xFFFF0000) >> 16;
> +
> +    while ( id != LEGACY_ID && next && ttl-- )
> +    {
> +        xcap += next;
> +        xcap_val = readl(xcap);
> +        id = xcap_val & 0xFF;
> +        next = (xcap_val & 0xFF00) >> 8;
> +    }
> +
> +    if ( id != LEGACY_ID )
> +        return;
> +
> +    xcap_val = readl(xcap);
> +#define XHCI_HC_BIOS_OWNED (1U << 16)
> +#define XHCI_HC_OS_OWNED (1U << 24)
> +    if (xcap_val & XHCI_HC_BIOS_OWNED) {
> +        dbc_error("bios owned\n");
> +        writeb(1, (uint8_t*)xcap + 3);
> +        while ((readl(xcap) & (XHCI_HC_BIOS_OWNED | XHCI_HC_OS_OWNED)) != XHCI_HC_OS_OWNED)
> +        {
> +            cpu_relax();
> +            if (!--timeout)
> +                break;
> +        }
> +        if (!timeout)
> +            dbc_error("handoff timeout\n");
> +        xcap_val = readl(xcap + 1);
> +        xcap_val &= ((0x7 << 1) + (0xff << 5) + (0x7 << 17)); // XHCI_LEGACY_DISABLE_SMI
> +        xcap_val |= (0x7 << 29); // XHCI_LEGACY_SMI_EVENTS
> +        writel(xcap_val, xcap + 1);
> +    }
> +}
> +

Unused new function (introducing a build failure at this point of the series)?

>  static bool __init dbc_init_xhc(struct dbc *dbc)
>  {
>      uint32_t bar0;
>      uint64_t bar1;
>      uint64_t bar_val;
>      uint64_t bar_size;
> -    uint64_t devfn;
> +    unsigned int bus, devfn;
>      uint16_t cmd;
>      size_t xhc_mmio_size;
>  
>      if ( dbc->sbdf.sbdf == 0 )
>      {
> -        /*
> -         * Search PCI bus 0 for the xHC. All the host controllers supported so
> -         * far are part of the chipset and are on bus 0.
> -         */
> -        for ( devfn = 0; devfn < 256; devfn++ )
> -        {
> -            pci_sbdf_t sbdf = PCI_SBDF(0, 0, devfn);
> -            uint8_t hdr = pci_conf_read8(sbdf, PCI_HEADER_TYPE);
> -
> -            if ( hdr == 0 || hdr == 0x80 )
> +        for ( bus = 0; bus < 0x100; bus++ )

Hex here and ...

> +            for ( devfn = 0; devfn < 256; devfn++ )

... dec here looks odd. Now that you want to fully iterate segment 0,
may I suggest to move the function closer to ehci-dbgp.c:find_dbgp(),
making use of pci_device_detect() and iterating over busses, slots,
and functions separately?

Jan
Marek Marczykowski-Górecki Dec. 14, 2022, 12:55 p.m. UTC | #2
On Wed, Dec 14, 2022 at 11:51:31AM +0100, Jan Beulich wrote:
> On 11.12.2022 03:10, Marek Marczykowski-Górecki wrote:
> > --- a/xen/drivers/char/xhci-dbc.c
> > +++ b/xen/drivers/char/xhci-dbc.c
> > @@ -286,39 +286,87 @@ static void *dbc_sys_map_xhc(uint64_t phys, size_t size)
> >      return fix_to_virt(FIX_XHCI_END);
> >  }
> >  
> > +static void xhci_bios_handoff(struct dbc *dbc)
> > +{
> > +    const uint32_t __iomem *xcap;
> > +    uint32_t xcap_val;
> > +    uint32_t next;
> > +    uint32_t id = 0;
> > +    const void __iomem *mmio = dbc->xhc_mmio;
> > +    const uint32_t __iomem *hccp1 = mmio + 0x10;
> > +    const uint32_t LEGACY_ID = 0x1;
> > +    int ttl = 48;
> > +    int timeout = 10000;
> > +
> > +    xcap = mmio;
> > +    /*
> > +     * This is initially an offset to the first capability. All the offsets
> > +     * (both in HCCP1 and then next capability pointer) are dword-based.
> > +     */
> > +    next = (readl(hccp1) & 0xFFFF0000) >> 16;
> > +
> > +    while ( id != LEGACY_ID && next && ttl-- )
> > +    {
> > +        xcap += next;
> > +        xcap_val = readl(xcap);
> > +        id = xcap_val & 0xFF;
> > +        next = (xcap_val & 0xFF00) >> 8;
> > +    }
> > +
> > +    if ( id != LEGACY_ID )
> > +        return;
> > +
> > +    xcap_val = readl(xcap);
> > +#define XHCI_HC_BIOS_OWNED (1U << 16)
> > +#define XHCI_HC_OS_OWNED (1U << 24)
> > +    if (xcap_val & XHCI_HC_BIOS_OWNED) {
> > +        dbc_error("bios owned\n");
> > +        writeb(1, (uint8_t*)xcap + 3);
> > +        while ((readl(xcap) & (XHCI_HC_BIOS_OWNED | XHCI_HC_OS_OWNED)) != XHCI_HC_OS_OWNED)
> > +        {
> > +            cpu_relax();
> > +            if (!--timeout)
> > +                break;
> > +        }
> > +        if (!timeout)
> > +            dbc_error("handoff timeout\n");
> > +        xcap_val = readl(xcap + 1);
> > +        xcap_val &= ((0x7 << 1) + (0xff << 5) + (0x7 << 17)); // XHCI_LEGACY_DISABLE_SMI
> > +        xcap_val |= (0x7 << 29); // XHCI_LEGACY_SMI_EVENTS
> > +        writel(xcap_val, xcap + 1);
> > +    }
> > +}
> > +
> 
> Unused new function (introducing a build failure at this point of the series)?

Oh, sorry, it wasn't supposed to be included in this patch.

> >  static bool __init dbc_init_xhc(struct dbc *dbc)
> >  {
> >      uint32_t bar0;
> >      uint64_t bar1;
> >      uint64_t bar_val;
> >      uint64_t bar_size;
> > -    uint64_t devfn;
> > +    unsigned int bus, devfn;
> >      uint16_t cmd;
> >      size_t xhc_mmio_size;
> >  
> >      if ( dbc->sbdf.sbdf == 0 )
> >      {
> > -        /*
> > -         * Search PCI bus 0 for the xHC. All the host controllers supported so
> > -         * far are part of the chipset and are on bus 0.
> > -         */
> > -        for ( devfn = 0; devfn < 256; devfn++ )
> > -        {
> > -            pci_sbdf_t sbdf = PCI_SBDF(0, 0, devfn);
> > -            uint8_t hdr = pci_conf_read8(sbdf, PCI_HEADER_TYPE);
> > -
> > -            if ( hdr == 0 || hdr == 0x80 )
> > +        for ( bus = 0; bus < 0x100; bus++ )
> 
> Hex here and ...
> 
> > +            for ( devfn = 0; devfn < 256; devfn++ )
> 
> ... dec here looks odd. Now that you want to fully iterate segment 0,
> may I suggest to move the function closer to ehci-dbgp.c:find_dbgp(),
> making use of pci_device_detect() and iterating over busses, slots,
> and functions separately?

Ok, will look into it.

But still, this all is not enough to get it working on AMD, so I'd wait
with v2 until it starts being functional.
diff mbox series

Patch

diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c
index 60b781f87202..62b0ce88b6bf 100644
--- a/xen/drivers/char/xhci-dbc.c
+++ b/xen/drivers/char/xhci-dbc.c
@@ -286,39 +286,87 @@  static void *dbc_sys_map_xhc(uint64_t phys, size_t size)
     return fix_to_virt(FIX_XHCI_END);
 }
 
+static void xhci_bios_handoff(struct dbc *dbc)
+{
+    const uint32_t __iomem *xcap;
+    uint32_t xcap_val;
+    uint32_t next;
+    uint32_t id = 0;
+    const void __iomem *mmio = dbc->xhc_mmio;
+    const uint32_t __iomem *hccp1 = mmio + 0x10;
+    const uint32_t LEGACY_ID = 0x1;
+    int ttl = 48;
+    int timeout = 10000;
+
+    xcap = mmio;
+    /*
+     * This is initially an offset to the first capability. All the offsets
+     * (both in HCCP1 and then next capability pointer) are dword-based.
+     */
+    next = (readl(hccp1) & 0xFFFF0000) >> 16;
+
+    while ( id != LEGACY_ID && next && ttl-- )
+    {
+        xcap += next;
+        xcap_val = readl(xcap);
+        id = xcap_val & 0xFF;
+        next = (xcap_val & 0xFF00) >> 8;
+    }
+
+    if ( id != LEGACY_ID )
+        return;
+
+    xcap_val = readl(xcap);
+#define XHCI_HC_BIOS_OWNED (1U << 16)
+#define XHCI_HC_OS_OWNED (1U << 24)
+    if (xcap_val & XHCI_HC_BIOS_OWNED) {
+        dbc_error("bios owned\n");
+        writeb(1, (uint8_t*)xcap + 3);
+        while ((readl(xcap) & (XHCI_HC_BIOS_OWNED | XHCI_HC_OS_OWNED)) != XHCI_HC_OS_OWNED)
+        {
+            cpu_relax();
+            if (!--timeout)
+                break;
+        }
+        if (!timeout)
+            dbc_error("handoff timeout\n");
+        xcap_val = readl(xcap + 1);
+        xcap_val &= ((0x7 << 1) + (0xff << 5) + (0x7 << 17)); // XHCI_LEGACY_DISABLE_SMI
+        xcap_val |= (0x7 << 29); // XHCI_LEGACY_SMI_EVENTS
+        writel(xcap_val, xcap + 1);
+    }
+}
+
 static bool __init dbc_init_xhc(struct dbc *dbc)
 {
     uint32_t bar0;
     uint64_t bar1;
     uint64_t bar_val;
     uint64_t bar_size;
-    uint64_t devfn;
+    unsigned int bus, devfn;
     uint16_t cmd;
     size_t xhc_mmio_size;
 
     if ( dbc->sbdf.sbdf == 0 )
     {
-        /*
-         * Search PCI bus 0 for the xHC. All the host controllers supported so
-         * far are part of the chipset and are on bus 0.
-         */
-        for ( devfn = 0; devfn < 256; devfn++ )
-        {
-            pci_sbdf_t sbdf = PCI_SBDF(0, 0, devfn);
-            uint8_t hdr = pci_conf_read8(sbdf, PCI_HEADER_TYPE);
-
-            if ( hdr == 0 || hdr == 0x80 )
+        for ( bus = 0; bus < 0x100; bus++ )
+            for ( devfn = 0; devfn < 256; devfn++ )
             {
-                if ( (pci_conf_read32(sbdf, PCI_CLASS_REVISION) >> 8) ==
-                     DBC_XHC_CLASSC )
+                pci_sbdf_t sbdf = PCI_SBDF(0, bus, devfn);
+                uint8_t hdr = pci_conf_read8(sbdf, PCI_HEADER_TYPE);
+
+                if ( hdr == 0 || hdr == 0x80 )
                 {
-                    if ( dbc->xhc_num-- )
-                        continue;
-                    dbc->sbdf = sbdf;
-                    break;
+                    if ( (pci_conf_read32(sbdf, PCI_CLASS_REVISION) >> 8) ==
+                         DBC_XHC_CLASSC )
+                    {
+                        if ( dbc->xhc_num-- )
+                            continue;
+                        dbc->sbdf = sbdf;
+                        break;
+                    }
                 }
             }
-        }
     }
     else
     {