RE: [PATCH]: xHCI/EHCI - Windows - BIOS bug interaction.

[Melki Christian (consultant)]

Aleš,

Sorry for the late reply. Holliday vaccation. :)

> Hi Christian,
> 
> I think - generally, if OS is able to work with this Intel chipset, then
> it should handle change of XUSB2PR and USB3_PSSEN registers state during
> reboot (or sleep/resume).
> In opposite case, when OS is not able to work with this chipset, then OS
> will not touch these registers - and routing to EHCI is the default
> state, so no problem should happen.

Yes. I think this is the case with Windows if Intel drivers are used.
Otherwise I think generic Microsoft xHCI drivers are used, and they don't know 
anything about Intel specific bugs.

> 
> But I am little bit confused:
> 
> I searched XUSB2PR and USB3_PSSEN PCI registers in official xHCI
> specification "eXtensible Host Controller Interafce for Universal Serial
> Bus (xHCI)", Revision 1.0, 5/21/10 (downloaded from www.intel.com) - and
> I didn't find them.
> Additionally, as far as I understood xHCI specification, xHCI concept is
> to avoid such port switching (similar to EHCI ports switching to
> companion OHCI/UHCI controllers).

Maybe I did not clarify my late findings properly.
This is not a xHCI spec problem, but rather an Intel PantherPoint chipset 
problem.

 > Then I tried to find datasheet of Intel pantherpoint chipset - and I
> found document "Intel 7 Series / C216 Chipset Family Platform Controller
> Hub (PCH)", June 2012. And this document includes description of XUSB2PR
> and USB3_PSSEN PCI registers.

Ah, yes. Correct. :)

> So:
> Are these PCI registers some only Intel-specific special "non-official"
> extension? (It looks like that - according to comments in Linux kernel
> source.)
> Or are them some new "official" xHCI add-on mentioned in some newer
> version of xHCI specification, unknown for us yet?
> How can I distinguish if xHCI contains these (possibly non-official -
> but very important) special PCI registers or not? Only by Vendor/Device
> IDs? Or is there some system, e.g. similar to PCI Capabilities etc?

These registers are Intel Pantherpoint specific as far as I understand. By 
deviceid only.

I did this before you sent your patch last time. It includes the Panterpoint 
chipset id.
(local changes only). They still fit better in a separate quirk or xhci-driver 
module as you suggested.

Index: grub-core/bus/usb/ehci.c
===================================================================
--- grub-core/bus/usb/ehci.c    (revision 6012)
+++ grub-core/bus/usb/ehci.c    (revision 6013)
@@ -41,6 +41,9 @@
  */

 #define GRUB_EHCI_PCI_SBRN_REG  0x60
+#define GRUB_XHCI_PCI_XUSB2PR   0xD0
+#define GRUB_XHCI_PCI_USB3_PSSEN 0xD8
+#define GRUB_XHCI_INTEL_PPT_PCIID 0x1e318086

 /* Capability registers offsets */
 enum
@@ -458,6 +461,41 @@
   return GRUB_USB_ERR_NONE;
 }

+static int
+grub_xhci_quirk_fix (grub_pci_device_t dev, grub_pci_id_t pciid,
+                     void *data __attribute__ ((unused)))
+{
+  grub_uint32_t class_code;
+  grub_uint32_t interf;
+  grub_uint32_t subclass;
+  grub_uint32_t class;
+  grub_pci_address_t addr;
+
+  addr = grub_pci_make_address (dev, GRUB_PCI_REG_CLASS);
+  class_code = grub_pci_read (addr) >> 8;
+  interf = class_code & 0xFF;
+  subclass = (class_code >> 8) & 0xFF;
+  class = class_code >> 16;
+
+  /* This is an Intel Pantherpoint xHCI controller.
+   * Quirk fix to put all ports in 2.0 mode.
+   * To be removed when xHCI is properly introduced.
+   */
+  if (class == 0x0c && subclass == 0x03 && interf == 0x30 &&
+      pciid == GRUB_XHCI_INTEL_PPT_PCIID)
+    {
+      grub_dprintf("ehci", "Intel PPT, applying quirk fix.\n");
+      addr = grub_pci_make_address (dev, GRUB_XHCI_PCI_USB3_PSSEN);
+      grub_pci_write_word (addr, 0x0);
+      grub_pci_read_word(addr);
+      addr = grub_pci_make_address (dev, GRUB_XHCI_PCI_XUSB2PR);
+      grub_pci_write_word (addr, 0x0);
+      grub_pci_read_word(addr);
+    }
+
+  return 0;
+}
+
 /* PCI iteration function... */
 static int
 grub_ehci_pci_iter (grub_pci_device_t dev, grub_pci_id_t pciid,
@@ -1844,6 +1882,12 @@
   grub_pci_iterate (grub_ehci_pci_iter, NULL);
 }

+static void
+grub_xhci_quirk (void)
+{
+  grub_pci_iterate (grub_xhci_quirk_fix, NULL);
+}
+
 static grub_err_t
 grub_ehci_restore_hw (void)
 {
@@ -1967,6 +2011,7 @@
   COMPILE_TIME_ASSERT (sizeof (struct grub_ehci_qh) == 96);

   grub_boot_time ("Initing EHCI hardware");
+  grub_xhci_quirk ();
   grub_ehci_inithw ();
   grub_boot_time ("Registering EHCI driver");
   grub_usb_controller_dev_register (&usb_controller);

> Answers of these questions will be important in case when we will try to
> support xHCI in GRUB.

At first I too thought that these vere more xHCI specific than they were.
I have reduced this problem to "nice to have" from by behalf rather than
"must have bugfix".
I think this will only appear on this Intel chipset, possibly BIOS bug(?) 
selected OS (Win7+) without Intel specific xHCI driver...
Not so many users are likely to get bitten by this.

regards && late season greetings. :)
Christian

> BR,
> Ales
> 
> 
> Dne 20.12.2013 08:38, Melki Christian (consultant) napsal(a):
> > Aleš,
> >
> > You're right. The code was not meant to be final commit or anything.
> > Just a base for discussion.
> > I have added a filter for the chipset in question in my code, but I'm more
> curious about odd cases.
> > What happens if BIOS intentionally sets ports to USB 3.0 in the port routing
> registers of said chipset?
> > Will all OS drivers handle that I have routed the ports to the EHCI
> controller? Or will stuff break, but the other way around?
> > Linux does this on _shutdown_ which means that it assumes, that
> whatever BIOS does it will be beneficial for the OS booting.
> > We do this on boot, after BIOS, which means we could be breaking stuff
> instead.
> >
> > Regards,
> > Christian