[edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Add access xHCI Extended Capabilities Pointer
Wu, Hao A
hao.a.wu at intel.com
Fri May 6 08:15:27 UTC 2022
Please refer to the inline comments below:
> -----Original Message-----
> From: Chiu, Ian <Ian.chiu at intel.com>
> Sent: Monday, April 25, 2022 9:45 PM
> To: devel at edk2.groups.io
> Cc: Chiu, Ian <Ian.chiu at intel.com>; Huang, Jenny <jenny.huang at intel.com>;
> Shih, More <more.shih at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Ni, Ray
> <ray.ni at intel.com>
> Subject: [PATCH v2] MdeModulePkg/XhciDxe: Add access xHCI Extended
> Capabilities Pointer
>
> From: Ian Chiu <Ian.chiu at intel.com>
>
> Add support process Port Speed field value of PORTSC according to Supported
> Protocol Capability
> (new design in xHCI spec 1.2 2019)
Please help to remove the above line. My take is that 'Supported Protocol Capability' contents already exist in the XHCI Spec 1.1 version.
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3914
>
> The value of Port Speed field in PORTSC bit[10:13] (xHCI spec 1.2 2019 section
> 5.4.8)
> should be change to use this value to query thru Protocol Speed ID (PSI)
> (xHCI spec 1.2 2019 section 7.2.1) in xHCI Supported Protocol Capability and
> return the value according the Protocol Speed ID (PSIV) Dword.
Could you help to add a brief summary on the PSI check when mapping to different port speeds (how the checks are mapping to low speed, high speed and super speed) in the log message?
>
> Cc: Jenny Huang <jenny.huang at intel.com>
> Cc: More Shih <more.shih at intel.com>
> Cc: Hao A Wu <hao.a.wu at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Signed-off-by: Ian Chiu <Ian.chiu at intel.com>
> ---
> MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 41 ++++--
> MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 2 +
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 147 ++++++++++++++++++++
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h | 87 ++++++++++++
> 4 files changed, 262 insertions(+), 15 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index b79499e225..f5b99210c9 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -398,25 +398,32 @@ XhcGetRootHubPortStatus (
> State = XhcReadOpReg (Xhc, Offset);
>
>
>
> //
>
> - // According to XHCI 1.1 spec November 2017,
>
> - // bit 10~13 of the root port status register identifies the speed of the
> attached device.
>
> + // According to XHCI 1.2 spec November 2019,
I think you can still use the reference information from the XHCI 1.1 spec.
The Support Protocol Capability related content already exists in this version of spec.
>
> + // Section 7.2 xHCI Support Protocol Capability
>
> //
>
> - switch ((State & XHC_PORTSC_PS) >> 10) {
>
> - case 2:
>
> - PortStatus->PortStatus |= USB_PORT_STAT_LOW_SPEED;
>
> - break;
>
> + PortStatus->PortStatus = XhcCheckUsbPortSpeedUsedPsic (Xhc, ((State &
> XHC_PORTSC_PS) >> 10));
>
> + if (PortStatus->PortStatus == 0) {
>
> + //
>
> + // According to XHCI 1.1 spec November 2017,
>
> + // bit 10~13 of the root port status register identifies the speed of the
> attached device.
>
> + //
>
> + switch ((State & XHC_PORTSC_PS) >> 10) {
>
> + case 2:
>
> + PortStatus->PortStatus |= USB_PORT_STAT_LOW_SPEED;
>
> + break;
>
>
>
> - case 3:
>
> - PortStatus->PortStatus |= USB_PORT_STAT_HIGH_SPEED;
>
> - break;
>
> + case 3:
>
> + PortStatus->PortStatus |= USB_PORT_STAT_HIGH_SPEED;
>
> + break;
>
>
>
> - case 4:
>
> - case 5:
>
> - PortStatus->PortStatus |= USB_PORT_STAT_SUPER_SPEED;
>
> - break;
>
> + case 4:
>
> + case 5:
>
> + PortStatus->PortStatus |= USB_PORT_STAT_SUPER_SPEED;
>
> + break;
>
>
>
> - default:
>
> - break;
>
> + default:
>
> + break;
>
> + }
>
> }
>
>
>
> //
>
> @@ -1820,6 +1827,8 @@ XhcCreateUsbHc (
> Xhc->ExtCapRegBase = ExtCapReg << 2;
>
> Xhc->UsbLegSupOffset = XhcGetCapabilityAddr (Xhc, XHC_CAP_USB_LEGACY);
>
> Xhc->DebugCapSupOffset = XhcGetCapabilityAddr (Xhc,
> XHC_CAP_USB_DEBUG);
>
> + Xhc->Usb2SupOffset = XhcGetUsbSupportedCapabilityAddr (Xhc,
> USB_SUPPORT_PROTOCOL_USB2_MAJOR_VER);
>
> + Xhc->UsbSsSupOffset = XhcGetUsbSupportedCapabilityAddr (Xhc,
> USB_SUPPORT_PROTOCOL_USB3_MAJOR_VER);
>
>
>
> DEBUG ((DEBUG_INFO, "XhcCreateUsb3Hc: Capability length 0x%x\n", Xhc-
> >CapLength));
>
> DEBUG ((DEBUG_INFO, "XhcCreateUsb3Hc: HcSParams1 0x%x\n", Xhc-
> >HcSParams1));
>
> @@ -1829,6 +1838,8 @@ XhcCreateUsbHc (
> DEBUG ((DEBUG_INFO, "XhcCreateUsb3Hc: RTSOff 0x%x\n", Xhc->RTSOff));
>
> DEBUG ((DEBUG_INFO, "XhcCreateUsb3Hc: UsbLegSupOffset 0x%x\n", Xhc-
> >UsbLegSupOffset));
>
> DEBUG ((DEBUG_INFO, "XhcCreateUsb3Hc: DebugCapSupOffset 0x%x\n", Xhc-
> >DebugCapSupOffset));
>
> + DEBUG ((DEBUG_INFO, "XhcCreateUsb3Hc: Usb2SupOffset 0x%x\n", Xhc-
> >Usb2SupOffset));
>
> + DEBUG ((DEBUG_INFO, "XhcCreateUsb3Hc: UsbSsSupOffset 0x%x\n", Xhc-
> >UsbSsSupOffset));
>
>
>
> //
>
> // Create AsyncRequest Polling Timer
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> index 5054d796b1..7eed7bd15e 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> @@ -227,6 +227,8 @@ struct _USB_XHCI_INSTANCE {
> UINT32 ExtCapRegBase;
>
> UINT32 UsbLegSupOffset;
>
> UINT32 DebugCapSupOffset;
>
> + UINT32 Usb2SupOffset;
>
> + UINT32 UsbSsSupOffset;
How about renaming to the above field to 'Usb3SupOffset'?
>
> UINT64 *DCBAA;
>
> VOID *DCBAAMap;
>
> UINT32 MaxSlotsEn;
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> index 80be3311d4..5bff698edb 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> @@ -564,7 +564,57 @@ XhcGetCapabilityAddr (
> if ((Data & 0xFF) == CapId) {
>
> return ExtCapOffset;
>
> }
>
> + //
>
> + // If not, then traverse all of the ext capability registers till finding out it.
>
> + //
>
> + NextExtCapReg = (UINT8)((Data >> 8) & 0xFF);
>
> + ExtCapOffset += (NextExtCapReg << 2);
>
> + } while (NextExtCapReg != 0);
>
> +
>
> + return 0xFFFFFFFF;
>
> +}
>
>
>
> +/**
>
> + Calculate the offset of the xHCI Supported Protocol Capability.
>
> +
>
> + @param Xhc The XHCI Instance.
>
> + @param MajorVersion The USB Major Version in xHCI Support Protocol
> Capability Field
>
> +
>
> + @return The offset of xHCI Supported Protocol capability register.
>
> +
>
> +**/
>
> +UINT32
>
> +XhcGetUsbSupportedCapabilityAddr (
Could you help to rename the function to XhcGetSupportedProtocolCapabilityAddr()?
>
> + IN USB_XHCI_INSTANCE *Xhc,
>
> + IN UINT8 MajorVersion
>
> + )
>
> +{
>
> + UINT32 ExtCapOffset;
>
> + UINT8 NextExtCapReg;
>
> + UINT32 Data;
>
> + UINT32 NameString;
>
> + XHC_SUPPORTED_PROTOCOL_DW0 UsbSupportDw0;
>
> +
>
> + if (Xhc == NULL) {
>
> + return 0;
>
> + }
>
> +
>
> + ExtCapOffset = 0;
>
> +
>
> + do {
>
> + //
>
> + // Check if the extended capability register's capability id is USB Legacy
> Support.
>
> + //
>
> + Data = XhcReadExtCapReg (Xhc, ExtCapOffset);
>
> + UsbSupportDw0.Dword = Data;
>
> + if ((Data & 0xFF) == XHC_CAP_USB_SUPPORTED) {
>
> + if (UsbSupportDw0.Data.RevMajor == MajorVersion) {
>
> + NameString = XhcReadExtCapReg (Xhc, ExtCapOffset +
> USB_SUPPORTED_NAME_STRING_OFFSET);
>
> + if (NameString == USB_SUPPORTED_PROTOCOL_NAME_STRING) {
>
> + return ExtCapOffset;
>
> + }
>
> + }
>
> + }
>
> //
>
> // If not, then traverse all of the ext capability registers till finding out it.
>
> //
>
> @@ -575,6 +625,103 @@ XhcGetCapabilityAddr (
> return 0xFFFFFFFF;
>
> }
>
>
>
> +/**
>
> + Find SpeedField value match with Port Speed ID value.
>
> +
>
> + @param Xhc The XHCI Instance.
>
> + @param ExtCapOffset The USB Major Version in xHCI Support Protocol
> Capability Field
>
> + @param SpeedField The Port Speed filed in USB PortSc register
>
> +
>
> + @return The Protocol Speed ID xHCI Supported Protocol capability register.
>
> +
>
> +**/
>
> +UINT32
>
> +XhciPsivGetPsid (
>
> + IN USB_XHCI_INSTANCE *Xhc,
>
> + IN UINT32 ExtCapOffset,
>
> + IN UINT8 SpeedField
>
> + )
>
> +{
>
> + XHC_SUPPORTED_PROTOCOL_DW2 PortId;
>
> + XHC_SUPPORTED_PROTOCOL_FIELD Reg;
>
> + UINT32 Count;
>
> +
>
> + if ((Xhc == NULL) || (ExtCapOffset == 0xFFFFFFFF)) {
>
> + return 0;
>
> + }
>
> +
>
> + //
>
> + // According to XHCI 1.2 spec November 2019,
>
> + // Section 7.2 xHCI Supported Protocol Capability
>
> + // 1. Get the PSIC(Protocol Speed ID Count) Value.
>
> + // 2. The PSID register boundary should be Base address + PSIC * 0x04
>
> + //
>
> + PortId.Dword = XhcReadExtCapReg (Xhc, ExtCapOffset +
> USB_SUPPORTED_PORT_ID_OFFSET);
>
> +
>
> + for (Count = 0; Count < PortId.Data.Psic; Count++) {
>
> + Reg.Dword = XhcReadExtCapReg (Xhc, ExtCapOffset +
> USB_SUPPORT_SPEED_ID_OFFSET + (Count << 2));
>
> + if (Reg.Data.Psiv == SpeedField) {
>
> + return Reg.Dword;
>
> + }
>
> + }
>
> + return 0;
>
> +}
>
> +
>
> +/**
>
> + Find SpeedField value match with Port Speed ID value.
>
> +
>
> + @param Xhc The XHCI Instance.
>
> + @param Speed The Port Speed filed in USB PortSc register
>
> +
>
> + @return The USB Port Speed.
>
> +
>
> +**/
>
> +UINT16
>
> +XhcCheckUsbPortSpeedUsedPsic (
>
> + IN USB_XHCI_INSTANCE *Xhc,
>
> + IN UINT8 Speed
>
> + )
>
> +{
>
> + XHC_SUPPORTED_PROTOCOL_FIELD SpField;
>
> + UINT16 ReturnSpeed;
>
> +
>
> + if (Xhc == NULL) {
>
> + return 0;
>
> + }
>
> +
>
> + SpField.Dword = 0;
>
> + ReturnSpeed = 0;
>
> + //
>
> + // Check USB3 Protocol Speed ID if ReturnSpeed didn't get match speed.
>
> + //
>
> + if ((ReturnSpeed == 0) && (Xhc->UsbSsSupOffset != 0xFFFFFFFF)) {
>
> + SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->UsbSsSupOffset, Speed);
>
> + if (SpField.Dword != 0) {
>
> + // Super Speed
>
> + ReturnSpeed = USB_PORT_STAT_SUPER_SPEED;
>
> + }
>
> + }
>
> +
>
> + //
>
> + // Check USB2 Protocol Speed ID if ReturnSpeed didn't get match speed.
>
> + //
>
> + if ((ReturnSpeed == 0) && (Xhc->Usb2SupOffset != 0xFFFFFFFF)) {
>
> + SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb2SupOffset, Speed);
>
> + if (SpField.Dword != 0) {
>
> + if (SpField.Data.Psie == 2) {
>
> + if (SpField.Data.Mantissa ==
> USB_SUPPORT_PROTOCOL_USB2_HIGH_SPEED_PSIM) {
>
> + // High Speed
>
> + ReturnSpeed = USB_PORT_STAT_HIGH_SPEED;
>
> + }
>
> + } else if (SpField.Data.Psie == 1) {
>
> + // Low speed
>
> + ReturnSpeed = USB_PORT_STAT_LOW_SPEED;
I suggest to add a similar Psim field check (whether equals to 1500) as the high speed check above.
>
> + }
>
> + }
>
> + }
>
> + return ReturnSpeed;
>
> +}
>
> +
>
> /**
>
> Whether the XHCI host controller is halted.
>
>
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> index 4950eed272..4f83b49027 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> @@ -27,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
> #define XHC_CAP_USB_LEGACY 0x01
>
> #define XHC_CAP_USB_DEBUG 0x0A
>
> +#define XHC_CAP_USB_SUPPORTED 0x02
Could you help to rename the above macro to XHC_CAP_USB_SUPPORTED_PROTOCOL?
>
>
>
> // ============================================//
>
> // XHCI register offset //
>
> @@ -74,6 +75,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #define USBLEGSP_BIOS_SEMAPHORE BIT16 // HC BIOS Owned
> Semaphore
>
> #define USBLEGSP_OS_SEMAPHORE BIT24 // HC OS Owned Semaphore
>
>
>
> +//
>
> +// xHCI Supported Protocol Capability
>
> +//
>
> +#define USB_SUPPORTED_PROTOCOL_NAME_STRING 0x20425355
>
> +#define USB_SUPPORTED_NAME_STRING_OFFSET 0x04
>
> +#define USB_SUPPORTED_PORT_ID_OFFSET 0x08
>
> +#define USB_SUPPORT_SPEED_ID_OFFSET 0x10
>
> +#define USB_SUPPORT_PROTOCOL_USB2_MAJOR_VER 0x02
>
> +#define USB_SUPPORT_PROTOCOL_USB3_MAJOR_VER 0x03
>
> +#define USB_SUPPORT_PROTOCOL_USB2_HIGH_SPEED_PSIM 480
Could you please help to update the above macro definitions to:
#define XHC_SUPPORTED_PROTOCOL_DW0_MAJOR_REVISION_USB2 0x02
#define XHC_SUPPORTED_PROTOCOL_DW0_MAJOR_REVISION_USB3 0x03
#define XHC_SUPPORTED_PROTOCOL_NAME_STRING_OFFSET 0x04
#define XHC_SUPPORTED_PROTOCOL_NAME_STRING_VALUE 0x20425355
#define XHC_SUPPORTED_PROTOCOL_DW2_OFFSET 0x08
#define XHC_SUPPORTED_PROTOCOL_PSI_OFFSET 0x10
#define XHC_SUPPORTED_PROTOCOL_USB2_HIGH_SPEED_PSIM 480
>
> +
>
> #pragma pack (1)
>
> typedef struct {
>
> UINT8 MaxSlots; // Number of Device Slots
>
> @@ -130,6 +142,52 @@ typedef union {
> HCCPARAMS Data;
>
> } XHC_HCCPARAMS;
>
>
>
> +//
>
> +// xHCI Supported Protocol Cabability
>
> +//
>
> +typedef struct {
>
> + UINT8 CapId;
>
> + UINT8 NextExtCapReg;
>
> + UINT8 RevMinor;
>
> + UINT8 RevMajor;
>
> +} SUPP_PROTOCOL_DW0;
Could you help to rename the above structure name to SUPPORTED_PROTOCOL_DW0?
>
> +
>
> +typedef union {
>
> + UINT32 Dword;
>
> + SUPP_PROTOCOL_DW0 Data;
>
> +} XHC_SUPPORTED_PROTOCOL_DW0;
>
> +
>
> +typedef struct {
>
> + UINT32 NameString;
>
> +} XHC_SUPPORTED_PROTOCOL_DW1;
>
> +
>
> +typedef struct {
>
> + UINT8 CompPortOffset : 8;
>
> + UINT8 CompPortCount : 8;
>
> + UINT16 ProtocolDef :12;
>
> + UINT16 Psic : 4;
>
> +} SUPP_PROTOCOL_DW2;
Could you help to refine the above structure definition to:
typedef struct {
UINT8 CompPortOffset;
UINT8 CompPortCount;
UINT16 ProtocolDef :12;
UINT16 Psic : 4;
} SUPPORTED_PROTOCOL_DW2;
>
> +
>
> +typedef union {
>
> + UINT32 Dword;
>
> + SUPP_PROTOCOL_DW2 Data;
>
> +} XHC_SUPPORTED_PROTOCOL_DW2;
>
> +
>
> +typedef struct {
>
> + UINT16 Psiv : 4;
>
> + UINT16 Psie : 2;
>
> + UINT16 Plt : 2;
>
> + UINT16 Pfd : 1;
>
> + UINT16 RsvdP : 5;
>
> + UINT16 Lp : 2;
>
> + UINT16 Mantissa :16;
>
> +} XHCI_PROTOCOL_FIELD;
Could you help to refine the above structure definition to:
typedef struct {
UINT16 Psiv : 4;
UINT16 Psie : 2;
UINT16 Plt : 2;
UINT16 Pfd : 1;
UINT16 RsvdP : 5;
UINT16 Lp : 2;
UINT16 Psim;
} SUPPORTED_PROTOCOL_PROTOCOL_SPEED_ID;
>
> +
>
> +typedef union {
>
> + UINT32 Dword;
>
> + XHCI_PROTOCOL_FIELD Data;
>
> +} XHC_SUPPORTED_PROTOCOL_FIELD;
Could you help to refine the above structure definition to:
typedef union {
UINT32 Dword;
SUPPORTED_PROTOCOL_PROTOCOL_SPEED_ID Data;
} XHC_SUPPORTED_PROTOCOL_PROTOCOL_SPEED_ID;
Best Regards,
Hao Wu
>
> +
>
> #pragma pack ()
>
>
>
> //
>
> @@ -546,4 +604,33 @@ XhcGetCapabilityAddr (
> IN UINT8 CapId
>
> );
>
>
>
> +/**
>
> + Calculate the offset of the xHCI Supported Protocol Capability.
>
> +
>
> + @param Xhc The XHCI Instance.
>
> + @param MajorVersion The USB Major Version in xHCI Support Protocol
> Capability Field
>
> +
>
> + @return The offset of xHCI Supported Protocol capability register.
>
> +
>
> +**/
>
> +UINT32
>
> +XhcGetUsbSupportedCapabilityAddr (
>
> + IN USB_XHCI_INSTANCE *Xhc,
>
> + IN UINT8 MajorVersion
>
> + );
>
> +
>
> +/**
>
> + Find SpeedField value match with Port Speed ID value.
>
> +
>
> + @param Xhc The XHCI Instance.
>
> + @param Speed The Port Speed filed in USB PortSc register
>
> +
>
> + @return The USB Port Speed.
>
> +
>
> +**/
>
> +UINT16
>
> +XhcCheckUsbPortSpeedUsedPsic (
>
> + IN USB_XHCI_INSTANCE *Xhc,
>
> + IN UINT8 Speed
>
> + );
>
> #endif
>
> --
> 2.26.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89553): https://edk2.groups.io/g/devel/message/89553
Mute This Topic: https://groups.io/mt/90684916/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list