xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers

Some Renesas controllers get into a weird state if they are reset while
programmed with 64bit addresses (they will preserve the top half of the
address in internal, non visible registers).

You end up with half the address coming from the kernel, and the other
half coming from the firmware.

Also, changing the programming leads to extra accesses even if the
controller is supposed to be halted. The controller ends up with a fatal
fault, and is then ripe for being properly reset. On the flip side,
this is completely unsafe if the defvice isn't behind an IOMMU, so
we have to make sure that this is the case. Can you say "broken"?

This is an alternative method to the one introduced in 8466489ef5ba
("xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue"),
which will subsequently be removed.

Tested-by: Domenico Andreoli <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Tested-by: Faiz Abbas <[email protected]>
Tested-by: Domenico Andreoli <[email protected]>
Acked-by: Mathias Nyman <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 85ffda85f8ab39043c5323a3799420d14c98baa8..e0a0a12871e292ac95016a039cda0af2d4323291 100644 (file)
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -196,11 +196,15 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
                xhci->quirks |= XHCI_BROKEN_STREAMS;
        }
        if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
                xhci->quirks |= XHCI_BROKEN_STREAMS;
        }
        if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&

-                       pdev->device == 0x0014)
+           pdev->device == 0x0014) {

                xhci->quirks |= XHCI_TRUST_TX_LENGTH;
                xhci->quirks |= XHCI_TRUST_TX_LENGTH;

+               xhci->quirks |= XHCI_ZERO_64B_REGS;
+       }

        if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
        if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&

-                       pdev->device == 0x0015)
+           pdev->device == 0x0015) {

                xhci->quirks |= XHCI_RESET_ON_RESUME;
                xhci->quirks |= XHCI_RESET_ON_RESUME;

+               xhci->quirks |= XHCI_ZERO_64B_REGS;
+       }

        if (pdev->vendor == PCI_VENDOR_ID_VIA)
                xhci->quirks |= XHCI_RESET_ON_RESUME;
 
        if (pdev->vendor == PCI_VENDOR_ID_VIA)
                xhci->quirks |= XHCI_RESET_ON_RESUME;
 

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 02b5ea790a905b2f5d62b812a379ec1abbe8e169..8c8da2d657fa1008c1e612e6f30d3e9716d534b6 100644 (file)
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -209,6 +209,68 @@ int xhci_reset(struct xhci_hcd *xhci)
        return ret;
 }
 
        return ret;
 }
 

+static void xhci_zero_64b_regs(struct xhci_hcd *xhci)
+{
+       struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+       int err, i;
+       u64 val;
+
+       /*
+        * Some Renesas controllers get into a weird state if they are
+        * reset while programmed with 64bit addresses (they will preserve
+        * the top half of the address in internal, non visible
+        * registers). You end up with half the address coming from the
+        * kernel, and the other half coming from the firmware. Also,
+        * changing the programming leads to extra accesses even if the
+        * controller is supposed to be halted. The controller ends up with
+        * a fatal fault, and is then ripe for being properly reset.
+        *
+        * Special care is taken to only apply this if the device is behind
+        * an iommu. Doing anything when there is no iommu is definitely
+        * unsafe...
+        */
+       if (!(xhci->quirks & XHCI_ZERO_64B_REGS) || !dev->iommu_group)
+               return;
+
+       xhci_info(xhci, "Zeroing 64bit base registers, expecting fault\n");
+
+       /* Clear HSEIE so that faults do not get signaled */
+       val = readl(&xhci->op_regs->command);
+       val &= ~CMD_HSEIE;
+       writel(val, &xhci->op_regs->command);
+
+       /* Clear HSE (aka FATAL) */
+       val = readl(&xhci->op_regs->status);
+       val |= STS_FATAL;
+       writel(val, &xhci->op_regs->status);
+
+       /* Now zero the registers, and brace for impact */
+       val = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
+       if (upper_32_bits(val))
+               xhci_write_64(xhci, 0, &xhci->op_regs->dcbaa_ptr);
+       val = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+       if (upper_32_bits(val))
+               xhci_write_64(xhci, 0, &xhci->op_regs->cmd_ring);
+
+       for (i = 0; i < HCS_MAX_INTRS(xhci->hcs_params1); i++) {
+               struct xhci_intr_reg __iomem *ir;
+
+               ir = &xhci->run_regs->ir_set[i];
+               val = xhci_read_64(xhci, &ir->erst_base);
+               if (upper_32_bits(val))
+                       xhci_write_64(xhci, 0, &ir->erst_base);
+               val= xhci_read_64(xhci, &ir->erst_dequeue);
+               if (upper_32_bits(val))
+                       xhci_write_64(xhci, 0, &ir->erst_dequeue);
+       }
+
+       /* Wait for the fault to appear. It will be cleared on reset */
+       err = xhci_handshake(&xhci->op_regs->status,
+                            STS_FATAL, STS_FATAL,
+                            XHCI_MAX_HALT_USEC);
+       if (!err)
+               xhci_info(xhci, "Fault detected\n");
+}

 
 #ifdef CONFIG_USB_PCI
 /*
 
 #ifdef CONFIG_USB_PCI
 /*


@@ -1006,6 +1068,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 
                xhci_dbg(xhci, "Stop HCD\n");
                xhci_halt(xhci);
 
                xhci_dbg(xhci, "Stop HCD\n");
                xhci_halt(xhci);

+               xhci_zero_64b_regs(xhci);

                xhci_reset(xhci);
                spin_unlock_irq(&xhci->lock);
                xhci_cleanup_msix(xhci);
                xhci_reset(xhci);
                spin_unlock_irq(&xhci->lock);
                xhci_cleanup_msix(xhci);


@@ -4917,6 +4980,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
        if (retval)
                return retval;
 
        if (retval)
                return retval;
 

+       xhci_zero_64b_regs(xhci);
+

        xhci_dbg(xhci, "Resetting HCD\n");
        /* Reset the internal HC memory state and registers. */
        retval = xhci_reset(xhci);
        xhci_dbg(xhci, "Resetting HCD\n");
        /* Reset the internal HC memory state and registers. */
        retval = xhci_reset(xhci);

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2303ee4307afde0bd74e3ba334eab2bf7f3ddad7..939e2f86b595eecbf1f1ecac7dcd7f39965d238d 100644 (file)
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1841,6 +1841,7 @@ struct xhci_hcd {
 #define XHCI_HW_LPM_DISABLE    BIT_ULL(29)
 #define XHCI_SUSPEND_DELAY     BIT_ULL(30)
 #define XHCI_INTEL_USB_ROLE_SW BIT_ULL(31)
 #define XHCI_HW_LPM_DISABLE    BIT_ULL(29)
 #define XHCI_SUSPEND_DELAY     BIT_ULL(30)
 #define XHCI_INTEL_USB_ROLE_SW BIT_ULL(31)

+#define XHCI_ZERO_64B_REGS     BIT_ULL(32)

 
        unsigned int            num_active_eps;
        unsigned int            limit_active_eps;
 
        unsigned int            num_active_eps;
        unsigned int            limit_active_eps;