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

[Jan Beulich <jbeulich@xxxxxxxx>]

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