Re: [PATCH v3] usb: pci-quirks: always assert xHCI OS ownership

[Mario Limonciello]

On 8/31/26 16:11, Rishabh Jain wrote:
> The xHCI ownership protocol requires the OS driver to assert the HC OS
> Owned semaphore before using the host controller, then wait for HC BIOS
> Owned to clear if firmware owns it.
>
> quirk_usb_handoff_xhci() currently asserts OS Owned only when BIOS Owned
> is already set. If firmware leaves BIOS Owned clear, Linux uses the xHC
> while both ownership semaphores remain clear.
>
> On an AMD PROM21 xHCI controller (1022:43fc), this caused every S3
> resume to terminate Controller Restore State with USBSTS 0x401. Linux
> then reset the host controller, both root hubs and the USB Bluetooth
> adapter.
>
> The controller entered resume ready and halted with USBSTS 0x1.
> Endpoint state, 100 ms save/restore delays, scratchpads, the DCBAA,
> device contexts and command, event and transfer rings were verified not
> to cause the restore error.
>
> Asserting only HC OS Owned changed USBLEGSUP from 0x00000801 to
> 0x01000801 and eliminated the restore failure across four S3 cycles,
> including a stock-kernel test. Clearing USBLEGCTLSTS was independently
> verified to be unnecessary.
>
> Always assert OS Owned when the xHCI Legacy Support capability is
> present. Use the existing ownership masks with a single initial register
> read/write. Keep the existing BIOS handoff recovery and legacy SMI
> cleanup unchanged.
>
> Fixes: 66d4eadd8d06 ("USB: xhci: BIOS handoff and HW initialization.")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Rishabh Jain <rishabh.jain1198@xxxxxxxxx>
Reviewed-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>> ---
> Changes in v3:
> - Drop the unnecessary BIOS ownership debug message.
> - Drop the redundant self Tested-by tag.
>
> Changes in v2:
> - Use XHCI_HC_OS_OWNED instead of byte-offset access.
> - Fold the TI/Renesas forced handoff into the ownership-register write.
> - Make the BIOS handoff wait unconditional and gate recovery on the initial
>    BIOS ownership state.
>
> Additional context:
>
> * Kernel Bugzilla #216470 documents the same USBSTS 0x401/reinitialize
>    behavior and its impact on attached USB devices:
>    https://bugzilla.kernel.org/show_bug.cgi?id=216470
>
> * Commit a7d57abcc8a5 ("xhci: workaround CSS timeout on AMD SNPS 3.0
>    xHC") is related workaround history: it tolerates a distinct AMD CSS
>    timeout and resets the controller on resume:
>    https://github.com/torvalds/linux/commit/a7d57abcc8a5bdeb53bbf8e87558e8e0a2c2a29d
>
> The external reports do not record their ownership semaphore values but
> are included as corroborating failure signatures that this might fix.
>
>   drivers/usb/host/pci-quirks.c | 27 ++++++++++++---------------
>   1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 0404489c2f6a9..52d41ac5daa85 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -1191,25 +1191,22 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
>   	if ((pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241) ||
>   			(pdev->vendor == PCI_VENDOR_ID_RENESAS
>   			 && pdev->device == 0x0014)) {
> -		val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
> -		writel(val, base + ext_cap_offset);
> +		val &= ~XHCI_HC_BIOS_OWNED;
>   	}
>   
> -	/* If the BIOS owns the HC, signal that the OS wants it, and wait */
> -	if (val & XHCI_HC_BIOS_OWNED) {
> -		writel(val | XHCI_HC_OS_OWNED, base + ext_cap_offset);
> +	val |= XHCI_HC_OS_OWNED;
> +	writel(val, base + ext_cap_offset);
>   
> -		/* Wait for 1 second with 10 microsecond polling interval */
> -		timeout = handshake(base + ext_cap_offset, XHCI_HC_BIOS_OWNED,
> -				0, 1000000, 10);
> +	/* Wait for 1 second with 10 microsecond polling interval */
> +	timeout = handshake(base + ext_cap_offset, XHCI_HC_BIOS_OWNED,
> +			    0, 1000000, 10);
>   
> -		/* Assume a buggy BIOS and take HC ownership anyway */
> -		if (timeout) {
> -			dev_warn(&pdev->dev,
> -				 "xHCI BIOS handoff failed (BIOS bug ?) %08x\n",
> -				 val);
> -			writel(val & ~XHCI_HC_BIOS_OWNED, base + ext_cap_offset);
> -		}
> +	/* Assume a buggy BIOS and take HC ownership anyway */
> +	if (timeout && (val & XHCI_HC_BIOS_OWNED)) {
> +		dev_warn(&pdev->dev,
> +			 "xHCI BIOS handoff failed (BIOS bug ?) %08x\n",
> +			 val);
> +		writel(val & ~XHCI_HC_BIOS_OWNED, base + ext_cap_offset);
>   	}
>   
>   	val = readl(base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);