linux-kernel - Re: [PATCH v2] xhci: Add quirk to workaround the errata seen on Cavium Thunder-X2 Soc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 Oct 2018 11:50:08 +0200
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     "Cherian, George" <George.Cherian@...ium.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Cc:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "mathias.nyman@...el.com" <mathias.nyman@...el.com>,
        "stern@...land.harvard.edu" <stern@...land.harvard.edu>
Subject: Re: [PATCH v2] xhci: Add quirk to workaround the errata seen on
 Cavium Thunder-X2 Soc

On 29.10.2018 18:54, Cherian, George wrote:
> Implement workaround for ThunderX2 Errata-129 (documented in
> CN99XX Known Issues" available at Cavium support site).
> As per ThunderX2errata-129, USB 2 device may come up as USB 1
> if a connection to a USB 1 device is followed by another connection to
> a USB 2 device, the link will come up as USB 1 for the USB 2 device.
> 
> Resolution: Reset the PHY after the USB 1 device is disconnected.
> The PHY reset sequence is done using private registers in XHCI register
> space. After the PHY is reset we check for the PLL lock status and retry
> the operation if it fails. From our tests, retrying 4 times is sufficient.
> 
> Add a new quirk flag XHCI_RESET_PLL_ON_DISCONNECT to invoke the workaround
> in handle_xhci_port_status().
> 
> Signed-off-by: George Cherian <george.cherian@...ium.com>
> ---
>   drivers/usb/host/xhci-pci.c  |  5 +++++
>   drivers/usb/host/xhci-ring.c | 35 ++++++++++++++++++++++++++++++++++-
>   drivers/usb/host/xhci.h      |  1 +
>   3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 51dd8e0..334c009 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -231,6 +231,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>   	if (pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241)
>   		xhci->quirks |= XHCI_LIMIT_ENDPOINT_INTERVAL_7;
>   
> +	if ((pdev->vendor == PCI_VENDOR_ID_BROADCOM ||
> +	     pdev->vendor == PCI_VENDOR_ID_CAVIUM) &&
> +	     pdev->device == 0x9026)
> +		xhci->quirks |= XHCI_RESET_PLL_ON_DISCONNECT;
> +
>   	if (xhci->quirks & XHCI_RESET_ON_RESUME)
>   		xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
>   				"QUIRK: Resetting on resume");
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index f0a99aa..342dd5ac 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1517,6 +1517,35 @@ static void handle_device_notification(struct xhci_hcd *xhci,
>   		usb_wakeup_notification(udev->parent, udev->portnum);
>   }
>   
> +/*
> + * Quirk hanlder for errata seen on Cavium ThunderX2 processor XHCI
> + * Controller.
> + * As per ThunderX2errata-129 USB 2 device may come up as USB 1
> + * If a connection to a USB 1 device is followed by another connection
> + * to a USB 2 device.
> + *
> + * Reset the PHY after the USB device is disconnected if device speed
> + * is less than HCD_USB3.
> + * Retry the reset sequence max of 4 times checking the PLL lock status.
> + *
> + */
> +static void xhci_handle_tx2_wrapper_reset(struct xhci_hcd *xhci)

Maybe rename this to make it more obvious it's about a quirk.
xhci_cavium_reset_phy_quirk() maybe?

> +{
> +	struct usb_hcd *hcd = xhci_to_hcd(xhci);
> +	u32 pll_lock_check;
> +	u32 retry_count = 4;
> +
> +	do {
> +		/* Assert PHY reset */
> +		writel(0x6F, hcd->regs + 0x1048);
> +		udelay(10);
> +		/* De-assert the PHY reset */
> +		writel(0x7F, hcd->regs + 0x1048);
> +		udelay(200);
> +		pll_lock_check = readl(hcd->regs + 0x1070);
> +	} while (!(pll_lock_check & 0x1) && --retry_count);
> +}
> +
>   static void handle_port_status(struct xhci_hcd *xhci,
>   		union xhci_trb *event)
>   {
> @@ -1642,8 +1671,12 @@ static void handle_port_status(struct xhci_hcd *xhci,
>   		goto cleanup;
>   	}
>   
> -	if (hcd->speed < HCD_USB3)
> +	if (hcd->speed < HCD_USB3) {
>   		xhci_test_and_clear_bit(xhci, port, PORT_PLC);
> +		if ((portsc & PORT_CSC) && !(portsc & 0x1) &&
> +		    (xhci->quirks & XHCI_RESET_PLL_ON_DISCONNECT))
> +			xhci_handle_tx2_wrapper_reset(xhci);
> +	}

Check for the quirk first, this way I can ignore the rest of the code when
debugging other issues,  and use PORT_CONNECT instead of 0x1

something like:

if ((xhci->quirks & XHCI_RESET_PLL_ON_DISCONNECT) &&
     (portsc & PORT_CSC) && !(portsc & PORT_CONNECT))

Thanks
-Mathias