linux-kernel - Re: [PATCH] xhci: workaround for S3 issue on AMD SNPS 3.0 xHC

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]

Message-ID: <20200916080611.GA486670@kroah.com>
Date:   Wed, 16 Sep 2020 10:06:11 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Nehal Bakulchandra Shah <Nehal-bakulchandra.Shah@....com>
Cc:     mathias.nyman@...el.com, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, Sandeep.Singh@....com,
        yuanmei@...ovo.com
Subject: Re: [PATCH] xhci: workaround for S3 issue on AMD SNPS 3.0 xHC

On Mon, Aug 31, 2020 at 06:52:46AM +0000, Nehal Bakulchandra Shah wrote:
> From: Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@....com>
> 
> On some platform of AMD, S3 fails with HCE and SRE errors.To fix this,
> sparse controller enable bit has to be disabled.
> 
> Signed-off-by: Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@....com>
> ---
>  drivers/usb/host/xhci-pci.c | 12 ++++++++++++
>  drivers/usb/host/xhci.h     |  1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 3feaafebfe58..865a16e6c1ed 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -160,6 +160,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>  	    (pdev->device == 0x15e0 || pdev->device == 0x15e1))
>  		xhci->quirks |= XHCI_SNPS_BROKEN_SUSPEND;
>  
> +	if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->device == 0x15e5)
> +		xhci->quirks |= XHCI_DISABLE_SPARSE;
> +
>  	if (pdev->vendor == PCI_VENDOR_ID_AMD)
>  		xhci->quirks |= XHCI_TRUST_TX_LENGTH;
>  
> @@ -371,6 +374,15 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	/* USB 2.0 roothub is stored in the PCI device now. */
>  	hcd = dev_get_drvdata(&dev->dev);
>  	xhci = hcd_to_xhci(hcd);
> +
> +	if (xhci->quirks & XHCI_DISABLE_SPARSE) {
> +		u32 reg;
> +
> +		reg = readl(hcd->regs + 0xC12C);
> +		reg &=  ~BIT(17);

Odd spacing :(

Anyway, what is magic bit 17?  Shouldn't that be documented somewhere?

And you forgot to handle endian issues here, right?

> +		writel(reg, hcd->regs + 0xC12C);

Same for this magic address, shouldn't you document that here please?

And is this the proper place to be testing for quirks and applying them?
Why not do the above in the xhci_pci_quirks() call?

thanks,

greg k-h