Re: [edk2-devel] [PATCH 3/3] MdeModulePkg/Bus/Pci/XhciDxe: Check port is compatible before getting PSIV

["Wu, Hao A" <hao.a.wu@intel.com>]

Merged via:
PR - https://github.com/tianocore/edk2/pull/3806
Commit - https://github.com/tianocore/edk2/commit/ec25e904c7da70302f2725e2005e3762f1ae891e

Best Regards,
Hao Wu

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Monday, December 19, 2022 3:46 PM
> To: devel@edk2.groups.io; Rhodes, Sean <sean@starlabs.systems>
> Subject: Re: [edk2-devel] [PATCH 3/3] MdeModulePkg/Bus/Pci/XhciDxe:
> Check port is compatible before getting PSIV
> 
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> I will make minor modification to:
> * Function description comment
> * Input parameter name
> of XhcCheckUsbPortSpeedUsedPsic() to make the declaration (in .H) and the
> definition (in .C) match.
> 
> Best Regards,
> Hao Wu
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> > Rhodes
> > Sent: Friday, December 16, 2022 4:58 PM
> > To: devel@edk2.groups.io
> > Cc: Rhodes, Sean <sean@starlabs.systems>
> > Subject: [edk2-devel] [PATCH 3/3] MdeModulePkg/Bus/Pci/XhciDxe: Check
> > port is compatible before getting PSIV
> >
> > On some platforms, including Sky Lake and Kaby Lake, the PSIV
> > (Protocol Speed ID Value) indices are shared between Protocol Speed ID
> > DWORD' in the extended capabilities registers for both USB2 (Full
> > Speed) and USB3 (Super Speed).
> >
> > An example can be found below:
> >
> >     XhcCheckUsbPortSpeedUsedPsic: checking for USB2 ext caps
> >     XhciPsivGetPsid: found 3 PSID entries
> >     XhciPsivGetPsid: looking for port speed 1
> >     XhciPsivGetPsid: PSIV 1 PSIE 2 PLT 0 PSIM 12
> >     XhciPsivGetPsid: PSIV 2 PSIE 1 PLT 0 PSIM 1500
> >     XhciPsivGetPsid: PSIV 3 PSIE 2 PLT 0 PSIM 480
> >     XhcCheckUsbPortSpeedUsedPsic: checking for USB3 ext caps
> >     XhciPsivGetPsid: found 3 PSID entries
> >     XhciPsivGetPsid: looking for port speed 1
> >     XhciPsivGetPsid: PSIV 1 PSIE 3 PLT 0 PSIM 5
> >     XhciPsivGetPsid: PSIV 2 PSIE 3 PLT 0 PSIM 10
> >     XhciPsivGetPsid: PSIV 34 PSIE 2 PLT 0 PSIM 1248
> >
> > The result is edk2 detecting USB2 devices as USB3 devices, which
> > consequently causes enumeration to fail.
> >
> > To avoid incorrect detection, check the Compatible Port Offset to find
> > the starting Port of Root Hubs that support the protocol.
> >
> > Signed-off-by: Sean Rhodes <sean@starlabs.systems>
> > ---
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c    |  2 +-
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 35 +++++++++++++++++++++--
> > ---
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h |  8 +++---
> >  3 files changed, 35 insertions(+), 10 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > index 8dd7a8fbb7..461b2cd9b5 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > @@ -405,7 +405,7 @@ XhcGetRootHubPortStatus (
> >    // Section 7.2 xHCI Support Protocol Capability
> >
> >    //
> >
> >    if (PortSpeed > 0) {
> >
> > -    PortStatus->PortStatus = XhcCheckUsbPortSpeedUsedPsic (Xhc,
> > PortSpeed);
> >
> > +    PortStatus->PortStatus = XhcCheckUsbPortSpeedUsedPsic (Xhc,
> > PortSpeed, PortNumber);
> >
> >      // If no match found in ext cap reg, fall back to PORTSC
> >
> >      if (PortStatus->PortStatus == 0) {
> >
> >        //
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > index 2b4a4b2444..5700fc5fb8 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > @@ -636,6 +636,7 @@ XhcGetSupportedProtocolCapabilityAddr (
> >    @param  Xhc            The XHCI Instance.
> >
> >    @param  ExtCapOffset   The USB Major Version in xHCI Support Protocol
> > Capability Field
> >
> >    @param  PortSpeed      The Port Speed Field in USB PortSc register
> >
> > +  @param  PortNumber     The Port Number (0-indexed)
> >
> >
> >
> >    @return The Protocol Speed ID (PSI) from xHCI Supported Protocol
> > capability register.
> >
> >
> >
> > @@ -644,12 +645,15 @@ UINT32
> >  XhciPsivGetPsid (
> >
> >    IN USB_XHCI_INSTANCE  *Xhc,
> >
> >    IN UINT32             ExtCapOffset,
> >
> > -  IN UINT8              PortSpeed
> >
> > +  IN UINT8              PortSpeed,
> >
> > +  IN UINT8              PortNumber
> >
> >    )
> >
> >  {
> >
> >    XHC_SUPPORTED_PROTOCOL_DW2                PortId;
> >
> >    XHC_SUPPORTED_PROTOCOL_PROTOCOL_SPEED_ID  Reg;
> >
> >    UINT32                                    Count;
> >
> > +  UINT32                                    MinPortIndex;
> >
> > +  UINT32                                    MaxPortIndex;
> >
> >
> >
> >    if ((Xhc == NULL) || (ExtCapOffset == 0xFFFFFFFF)) {
> >
> >      return 0;
> >
> > @@ -663,6 +667,23 @@ XhciPsivGetPsid (
> >    //
> >
> >    PortId.Dword = XhcReadExtCapReg (Xhc, ExtCapOffset +
> > XHC_SUPPORTED_PROTOCOL_DW2_OFFSET);
> >
> >
> >
> > +  //
> >
> > +  // According to XHCI 1.1 spec November 2017, valid values
> >
> > +  // for CompPortOffset are 1 to CompPortCount - 1.
> >
> > +  //
> >
> > +  // PortNumber is zero-indexed, so subtract 1.
> >
> > +  //
> >
> > +  if ((PortId.Data.CompPortOffset == 0) || (PortId.Data.CompPortCount
> > + ==
> > 0)) {
> >
> > +    return 0;
> >
> > +  }
> >
> > +
> >
> > +  MinPortIndex = PortId.Data.CompPortOffset - 1;
> >
> > +  MaxPortIndex = MinPortIndex + PortId.Data.CompPortCount - 1;
> >
> > +
> >
> > +  if ((PortNumber < MinPortIndex) || (PortNumber > MaxPortIndex)) {
> >
> > +    return 0;
> >
> > +  }
> >
> > +
> >
> >    for (Count = 0; Count < PortId.Data.Psic; Count++) {
> >
> >      Reg.Dword = XhcReadExtCapReg (Xhc, ExtCapOffset +
> > XHC_SUPPORTED_PROTOCOL_PSI_OFFSET + (Count << 2));
> >
> >      if (Reg.Data.Psiv == PortSpeed) {
> >
> > @@ -676,8 +697,9 @@ XhciPsivGetPsid (
> >  /**
> >
> >    Find PortSpeed value match case in XHCI Supported Protocol
> > Capability
> >
> >
> >
> > -  @param  Xhc        The XHCI Instance.
> >
> > -  @param  PortSpeed  The Port Speed Field in USB PortSc register
> >
> > +  @param  Xhc         The XHCI Instance.
> >
> > +  @param  PortSpeed   The Port Speed Field in USB PortSc register
> >
> > +  @param  PortNumber  The Port Number (0-indexed)
> >
> >
> >
> >    @return The USB Port Speed.
> >
> >
> >
> > @@ -685,7 +707,8 @@ XhciPsivGetPsid (
> >  UINT16
> >
> >  XhcCheckUsbPortSpeedUsedPsic (
> >
> >    IN USB_XHCI_INSTANCE  *Xhc,
> >
> > -  IN UINT8              PortSpeed
> >
> > +  IN UINT8              PortSpeed,
> >
> > +  IN UINT8              PortNumber
> >
> >    )
> >
> >  {
> >
> >    XHC_SUPPORTED_PROTOCOL_PROTOCOL_SPEED_ID  SpField;
> >
> > @@ -703,7 +726,7 @@ XhcCheckUsbPortSpeedUsedPsic (
> >    // PortSpeed definition when the Major Revision is 03h.
> >
> >    //
> >
> >    if (Xhc->Usb3SupOffset != 0xFFFFFFFF) {
> >
> > -    SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);
> >
> > +    SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset,
> > + PortSpeed,
> > PortNumber);
> >
> >      if (SpField.Dword != 0) {
> >
> >        //
> >
> >        // Found the corresponding PORTSC value in PSIV field of USB3 offset.
> >
> > @@ -717,7 +740,7 @@ XhcCheckUsbPortSpeedUsedPsic (
> >    // PortSpeed definition when the Major Revision is 02h.
> >
> >    //
> >
> >    if ((UsbSpeedIdMap == 0) && (Xhc->Usb2SupOffset != 0xFFFFFFFF)) {
> >
> > -    SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb2SupOffset, PortSpeed);
> >
> > +    SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb2SupOffset,
> > + PortSpeed,
> > PortNumber);
> >
> >      if (SpField.Dword != 0) {
> >
> >        //
> >
> >        // Found the corresponding PORTSC value in PSIV field of USB2 offset.
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > index 5fe2ba4f0e..2e4f95f8ac 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > @@ -623,8 +623,9 @@ XhcGetSupportedProtocolCapabilityAddr (
> >  /**
> >
> >    Find SpeedField value match with Port Speed ID value.
> >
> >
> >
> > -  @param  Xhc    The XHCI Instance.
> >
> > -  @param  Speed  The Port Speed filed in USB PortSc register
> >
> > +  @param  Xhc         The XHCI Instance.
> >
> > +  @param  Speed       The Port Speed filed in USB PortSc register
> >
> > +  @param  PortNumber  The Port Number (0-indexed)
> >
> >
> >
> >    @return The USB Port Speed.
> >
> >
> >
> > @@ -632,7 +633,8 @@ XhcGetSupportedProtocolCapabilityAddr (
> >  UINT16
> >
> >  XhcCheckUsbPortSpeedUsedPsic (
> >
> >    IN USB_XHCI_INSTANCE  *Xhc,
> >
> > -  IN UINT8              Speed
> >
> > +  IN UINT8              Speed,
> >
> > +  IN UINT8              PortNumber
> >
> >    );
> >
> >
> >
> >  #endif
> >
> > --
> > 2.37.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#97497):
> > https://edk2.groups.io/g/devel/message/97497
> > Mute This Topic: https://groups.io/mt/95706436/1768737
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
>