linux-kernel - Re: [PATCH RESEND] PCI: Check for USB xHCI class for HAPS platform

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>] [day] [month] [year] [list]

Message-ID: <20190206232223.GF7268@google.com>
Date:   Wed, 6 Feb 2019 17:22:23 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Thinh Nguyen <thinh.nguyen@...opsys.com>
Cc:     linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
        "Lukas F.Hartmann" <lukas@...mn.com>,
        Trent Piepho <tpiepho@...inj.com>,
        John Youn <john.youn@...opsys.com>
Subject: Re: [PATCH RESEND] PCI: Check for USB xHCI class for HAPS platform

On Tue, Feb 05, 2019 at 01:04:28PM -0800, Thinh Nguyen wrote:
> The Synopsys HAPS USB controller has a VID PID (16c3,abcd) that matches
> to an existing PCIe controller. This quirk is intended for USB HAPS
> devices only. To fix this, check for the PCI class USB xHCI to prevent
> matching the PCIe controllers.
> 
> Fixes: 03e6742584af ("PCI: Override Synopsys USB 3.x HAPS device class")
> Signed-off-by: Thinh Nguyen <thinhn@...opsys.com>

I applied this as below to for-linus for v5.0, thanks!

I *suspect* that this pending patch [1] would make the Root Ports work
correctly as bridges even if they had the wrong class code, e.g., if
we didn't have this fix to the quirk.

But the portdrv wouldn't claim the Root Ports, so PCIe services still
wouldn't work correctly, so we absolutely still need this patch.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?id=b2fb5cc57469

> ---
>  drivers/pci/quirks.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b0a413f3f7ca..e2a879e93d86 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -639,8 +639,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
>  		break;
>  	}
>  }
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
> -			 quirk_synopsys_haps);
> +DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
> +			       PCI_CLASS_SERIAL_USB_XHCI, 0,
> +			       quirk_synopsys_haps);
>  
>  /*
>   * Let's make the southbridge information explicit instead of having to

commit f57a98e1b71357713e44c57268a53d9c803f0626
Author: Thinh Nguyen <thinh.nguyen@...opsys.com>
Date:   Wed Feb 6 17:17:27 2019 -0600

    PCI: Work around Synopsys duplicate Device ID (HAPS USB3, NXP i.MX)
    
    There are at least four different parts with the same Vendor and Device
    ID ([16c3:abcd]):
    
      1) Synopsys HAPS USB3 controller
      2) Synopsys PCIe Root Port in Freescale/NXP i.MX6Q (reported by Lucas)
      3) Synopsys PCIe Root Port in Freescale/NXP i.MX6QP (reported by Lukas)
      4) Synopsys PCIe Root Port in Freescale/NXP i.MX7D (reported by Trent)
    
    The HAPS USB3 controller has a Class Code of PCI_CLASS_SERIAL_USB_XHCI,
    which means the XHCI driver would normally claim it.  Previously,
    quirk_synopsys_haps() changed the Class Code of all [16c3:abcd] devices,
    including the Root Ports, to PCI_CLASS_SERIAL_USB_DEVICE to prevent the
    XHCI driver from claiming them so dwc3-haps can claim them instead.
    
    Changing the Class Code of the Root Ports prevents the PCI core from
    handling them as bridges, so devices below them don't work.
    
    Restrict the quirk so it only changes the Class Code for devices that start
    with the PCI_CLASS_SERIAL_USB_XHCI Class Code, leaving the Root Ports
    alone.
    
    Fixes: 03e6742584af ("PCI: Override Synopsys USB 3.x HAPS device class")
    Reported-by: Lukas F. Hartmann <lukas@...mn.com>
    Reported-by: Trent Piepho <tpiepho@...inj.com>
    Reported-by: Lucas Stach <l.stach@...gutronix.de>
    Signed-off-by: Thinh Nguyen <thinhn@...opsys.com>
    [bhelgaas: changelog]
    Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b0a413f3f7ca..e2a879e93d86 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -639,8 +639,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
 		break;
 	}
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
-			 quirk_synopsys_haps);
+DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
+			       PCI_CLASS_SERIAL_USB_XHCI, 0,
+			       quirk_synopsys_haps);
 
 /*
  * Let's make the southbridge information explicit instead of having to