* [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices
@ 2022-12-01 20:24 Sean Rhodes
2022-12-01 20:24 ` [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV Sean Rhodes
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Sean Rhodes @ 2022-12-01 20:24 UTC (permalink / raw)
To: devel; +Cc: Matt DeVillier, Hao A Wu, Ray Ni, Sean Rhodes
From: Matt DeVillier <matt.devillier@gmail.com>
On some platforms, including Sky Lake and Kaby Lake, the PSIV (Protocol
Speed ID Value) indicesare 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 extended capability registers
for USB2 before USB3. If edk2 finds a match for a USB 2 device, don't
check for USB 3.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: I5bcf32105ce85fda95b4ba98a5e420e8f522374c
---
MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 36 +++++++++++++++-----------
MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h | 1 +
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
index 2b4a4b2444..c992323443 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
@@ -698,25 +698,11 @@ XhcCheckUsbPortSpeedUsedPsic (
SpField.Dword = 0;
UsbSpeedIdMap = 0;
- //
- // Check xHCI Supported Protocol Capability, find the PSIV field to match
- // PortSpeed definition when the Major Revision is 03h.
- //
- if (Xhc->Usb3SupOffset != 0xFFFFFFFF) {
- SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);
- if (SpField.Dword != 0) {
- //
- // Found the corresponding PORTSC value in PSIV field of USB3 offset.
- //
- UsbSpeedIdMap = USB_PORT_STAT_SUPER_SPEED;
- }
- }
-
//
// Check xHCI Supported Protocol Capability, find the PSIV field to match
// PortSpeed definition when the Major Revision is 02h.
//
- if ((UsbSpeedIdMap == 0) && (Xhc->Usb2SupOffset != 0xFFFFFFFF)) {
+ if (Xhc->Usb2SupOffset != 0xFFFFFFFF) {
SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb2SupOffset, PortSpeed);
if (SpField.Dword != 0) {
//
@@ -733,6 +719,12 @@ XhcCheckUsbPortSpeedUsedPsic (
// PSIM shows as default High-speed protocol, apply to High-speed mapping
//
UsbSpeedIdMap = USB_PORT_STAT_HIGH_SPEED;
+ } else if (SpField.Data.Psim == XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM) {
+ //
+ // PSIM shows as default Full-speed protocol, return 0
+ // to ensure no port status set
+ //
+ return 0;
}
} else if (SpField.Data.Psie == 1) {
//
@@ -750,6 +742,20 @@ XhcCheckUsbPortSpeedUsedPsic (
}
}
+ //
+ // Check xHCI Supported Protocol Capability, find the PSIV field to match
+ // PortSpeed definition when the Major Revision is 03h.
+ //
+ if ((UsbSpeedIdMap == 0) && (Xhc->Usb3SupOffset != 0xFFFFFFFF)) {
+ SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);
+ if (SpField.Dword != 0) {
+ //
+ // Found the corresponding PORTSC value in PSIV field of USB3 offset.
+ //
+ UsbSpeedIdMap = USB_PORT_STAT_SUPER_SPEED;
+ }
+ }
+
return UsbSpeedIdMap;
}
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
index 5fe2ba4f0e..74ac6297ba 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
@@ -85,6 +85,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define XHC_SUPPORTED_PROTOCOL_DW2_OFFSET 0x08
#define XHC_SUPPORTED_PROTOCOL_PSI_OFFSET 0x10
#define XHC_SUPPORTED_PROTOCOL_USB2_HIGH_SPEED_PSIM 480
+#define XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM 12
#define XHC_SUPPORTED_PROTOCOL_USB2_LOW_SPEED_PSIM 1500
#pragma pack (1)
--
2.37.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV
2022-12-01 20:24 [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Sean Rhodes
@ 2022-12-01 20:24 ` Sean Rhodes
2022-12-01 20:24 ` [PATCH 3/4] MdeModulePkg/BmBoot: Skip removable media if it is not present Sean Rhodes
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Sean Rhodes @ 2022-12-01 20:24 UTC (permalink / raw)
To: devel; +Cc: Matt DeVillier, Hao A Wu, Ray Ni, Sean Rhodes
From: Matt DeVillier <matt.devillier@gmail.com>
PSID matching relies on comparing the PSIV against the PortSpeed
value. This patch stops edk2 from checking for a PSIV of 0, as it
is not valid; this reduces the number of register access by
approximately 6 per second.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: If15c55ab66d2e7faa832ce8576d2e5b47157cc9a
---
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 44 ++++++++++++++++-------------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index c05431ff30..51ae57db21 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -371,6 +371,7 @@ XhcGetRootHubPortStatus (
UINT32 TotalPort;
UINTN Index;
UINTN MapSize;
+ UINTN PortSpeed;
EFI_STATUS Status;
USB_DEV_ROUTE ParentRouteChart;
EFI_TPL OldTpl;
@@ -397,32 +398,37 @@ XhcGetRootHubPortStatus (
State = XhcReadOpReg (Xhc, Offset);
+ PortSpeed = (State & XHC_PORTSC_PS) >> 10;
+
//
// According to XHCI 1.1 spec November 2017,
// Section 7.2 xHCI Support Protocol Capability
//
- 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;
+ if (PortSpeed > 0) {
+ PortStatus->PortStatus = XhcCheckUsbPortSpeedUsedPsic (Xhc, PortSpeed);
+ // If no match found in ext cap reg, fall back to PORTSC
+ 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 (PortSpeed) {
+ 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;
+ }
}
}
--
2.37.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] MdeModulePkg/BmBoot: Skip removable media if it is not present
2022-12-01 20:24 [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Sean Rhodes
2022-12-01 20:24 ` [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV Sean Rhodes
@ 2022-12-01 20:24 ` Sean Rhodes
2022-12-01 20:24 ` [PATCH 4/4] MdeModulePkg/UsbBusDxe: Adjust the MaxPacketLength to real world values Sean Rhodes
2022-12-02 6:25 ` [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Wu, Hao A
3 siblings, 0 replies; 8+ messages in thread
From: Sean Rhodes @ 2022-12-01 20:24 UTC (permalink / raw)
To: devel; +Cc: Matt DeVillier, Hao A Wu, Ray Ni, Sean Rhodes
From: Matt DeVillier <matt.devillier@gmail.com>
Only enumerate devices that have media present.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: I78a0b8be3e2f33edce2d43bbdd7670e6174d0ff8
---
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 962892d38f..bde22fa659 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2218,6 +2218,15 @@ BmEnumerateBootOptions (
continue;
}
+ //
+ // Skip removable media if not present
+ //
+ if ((BlkIo->Media->RemovableMedia == TRUE) &&
+ (BlkIo->Media->MediaPresent == FALSE))
+ {
+ continue;
+ }
+
Description = BmGetBootDescription (Handles[Index]);
BootOptions = ReallocatePool (
sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * (*BootOptionCount),
--
2.37.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] MdeModulePkg/UsbBusDxe: Adjust the MaxPacketLength to real world values
2022-12-01 20:24 [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Sean Rhodes
2022-12-01 20:24 ` [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV Sean Rhodes
2022-12-01 20:24 ` [PATCH 3/4] MdeModulePkg/BmBoot: Skip removable media if it is not present Sean Rhodes
@ 2022-12-01 20:24 ` Sean Rhodes
2022-12-02 6:25 ` [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Wu, Hao A
3 siblings, 0 replies; 8+ messages in thread
From: Sean Rhodes @ 2022-12-01 20:24 UTC (permalink / raw)
To: devel; +Cc: Sean Rhodes, Hao A Wu, Ray Ni
Adjusts the requirements for the MaxPacketLength to match what is seen on
real world devices that do not follow the USB specification.
This fixes enumeration on the multiple USB 3 devices made by SanDisk,
Integral, Kingston and other generic brands.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
---
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 51ae57db21..73bfc62512 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -906,19 +906,16 @@ XhcControlTransfer (
return EFI_INVALID_PARAMETER;
}
- if ((MaximumPacketLength != 8) && (MaximumPacketLength != 16) &&
- (MaximumPacketLength != 32) && (MaximumPacketLength != 64) &&
- (MaximumPacketLength != 512)
- )
- {
+ // Check for valid maximum packet size
+ if ((DeviceSpeed == EFI_USB_SPEED_SUPER) && (MaximumPacketLength > 1024)) {
return EFI_INVALID_PARAMETER;
}
- if ((DeviceSpeed == EFI_USB_SPEED_LOW) && (MaximumPacketLength != 8)) {
+ if ((DeviceSpeed == EFI_USB_SPEED_HIGH) && (MaximumPacketLength > 512)) {
return EFI_INVALID_PARAMETER;
}
- if ((DeviceSpeed == EFI_USB_SPEED_SUPER) && (MaximumPacketLength != 512)) {
+ if ((DeviceSpeed == EFI_USB_SPEED_FULL) && (MaximumPacketLength > 64)) {
return EFI_INVALID_PARAMETER;
}
--
2.37.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices
2022-12-01 20:24 [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Sean Rhodes
` (2 preceding siblings ...)
2022-12-01 20:24 ` [PATCH 4/4] MdeModulePkg/UsbBusDxe: Adjust the MaxPacketLength to real world values Sean Rhodes
@ 2022-12-02 6:25 ` Wu, Hao A
2022-12-02 16:38 ` Sean Rhodes
3 siblings, 1 reply; 8+ messages in thread
From: Wu, Hao A @ 2022-12-02 6:25 UTC (permalink / raw)
To: devel@edk2.groups.io, Rhodes, Sean; +Cc: Matt DeVillier, Ni, Ray
Hello,
I saw there are several CI check failures for this 4 patches:
https://github.com/tianocore/edk2/pull/3702
Could you help to resolve them? Thanks.
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> Rhodes
> Sent: Friday, December 2, 2022 4:25 AM
> To: devel@edk2.groups.io
> Cc: Matt DeVillier <matt.devillier@gmail.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Rhodes, Sean
> <sean@starlabs.systems>
> Subject: [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle
> incorrect PSIV indices
>
> From: Matt DeVillier <matt.devillier@gmail.com>
>
> On some platforms, including Sky Lake and Kaby Lake, the PSIV (Protocol Speed
> ID Value) indicesare 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 extended capability registers for USB2
> before USB3. If edk2 finds a match for a USB 2 device, don't check for USB 3.
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Reviewed-by: Sean Rhodes <sean@starlabs.systems>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> Change-Id: I5bcf32105ce85fda95b4ba98a5e420e8f522374c
> ---
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 36 +++++++++++++++-----------
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h | 1 +
> 2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> index 2b4a4b2444..c992323443 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> @@ -698,25 +698,11 @@ XhcCheckUsbPortSpeedUsedPsic (
> SpField.Dword = 0; UsbSpeedIdMap = 0; - //- // Check xHCI Supported
> Protocol Capability, find the PSIV field to match- // PortSpeed definition when
> the Major Revision is 03h.- //- if (Xhc->Usb3SupOffset != 0xFFFFFFFF) {-
> SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);- if
> (SpField.Dword != 0) {- //- // Found the corresponding PORTSC value in
> PSIV field of USB3 offset.- //- UsbSpeedIdMap =
> USB_PORT_STAT_SUPER_SPEED;- }- }- // // Check xHCI Supported Protocol
> Capability, find the PSIV field to match // PortSpeed definition when the Major
> Revision is 02h. //- if ((UsbSpeedIdMap == 0) && (Xhc->Usb2SupOffset !=
> 0xFFFFFFFF)) {+ if (Xhc->Usb2SupOffset != 0xFFFFFFFF) { SpField.Dword =
> XhciPsivGetPsid (Xhc, Xhc->Usb2SupOffset, PortSpeed); if (SpField.Dword != 0)
> { //@@ -733,6 +719,12 @@ XhcCheckUsbPortSpeedUsedPsic (
> // PSIM shows as default High-speed protocol, apply to High-speed
> mapping // UsbSpeedIdMap =
> USB_PORT_STAT_HIGH_SPEED;+ } else if (SpField.Data.Psim ==
> XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM) {+ //+ //
> PSIM shows as default Full-speed protocol, return 0+ // to ensure no port
> status set+ //+ return 0; } } else if (SpField.Data.Psie == 1)
> { //@@ -750,6 +742,20 @@ XhcCheckUsbPortSpeedUsedPsic (
> } } + //+ // Check xHCI Supported Protocol Capability, find the PSIV field to
> match+ // PortSpeed definition when the Major Revision is 03h.+ //+ if
> ((UsbSpeedIdMap == 0) && (Xhc->Usb3SupOffset != 0xFFFFFFFF)) {+
> SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);+ if
> (SpField.Dword != 0) {+ //+ // Found the corresponding PORTSC value in
> PSIV field of USB3 offset.+ //+ UsbSpeedIdMap =
> USB_PORT_STAT_SUPER_SPEED;+ }+ }+ return UsbSpeedIdMap; } diff --git
> a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> index 5fe2ba4f0e..74ac6297ba 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> @@ -85,6 +85,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #define XHC_SUPPORTED_PROTOCOL_DW2_OFFSET 0x08 #define
> XHC_SUPPORTED_PROTOCOL_PSI_OFFSET 0x10 #define
> XHC_SUPPORTED_PROTOCOL_USB2_HIGH_SPEED_PSIM 480+#define
> XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM 12 #define
> XHC_SUPPORTED_PROTOCOL_USB2_LOW_SPEED_PSIM 1500 #pragma
> pack (1)--
> 2.37.2
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#96843): https://edk2.groups.io/g/devel/message/96843
> Mute This Topic: https://groups.io/mt/95391831/1768737
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com] -=-=-
> =-=-=-=
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices
2022-12-02 6:25 ` [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Wu, Hao A
@ 2022-12-02 16:38 ` Sean Rhodes
2022-12-05 1:37 ` Wu, Hao A
0 siblings, 1 reply; 8+ messages in thread
From: Sean Rhodes @ 2022-12-02 16:38 UTC (permalink / raw)
To: devel, hao.a.wu; +Cc: Matt DeVillier, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 5877 bytes --]
Hi Hao
They are now resolved
Thanks
Sean
On Fri, 2 Dec 2022 at 06:25, Wu, Hao A <hao.a.wu@intel.com> wrote:
> Hello,
>
> I saw there are several CI check failures for this 4 patches:
> https://github.com/tianocore/edk2/pull/3702
>
> Could you help to resolve them? Thanks.
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> > Rhodes
> > Sent: Friday, December 2, 2022 4:25 AM
> > To: devel@edk2.groups.io
> > Cc: Matt DeVillier <matt.devillier@gmail.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Rhodes, Sean
> > <sean@starlabs.systems>
> > Subject: [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle
> > incorrect PSIV indices
> >
> > From: Matt DeVillier <matt.devillier@gmail.com>
> >
> > On some platforms, including Sky Lake and Kaby Lake, the PSIV (Protocol
> Speed
> > ID Value) indicesare 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 extended capability registers
> for USB2
> > before USB3. If edk2 finds a match for a USB 2 device, don't check for
> USB 3.
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Reviewed-by: Sean Rhodes <sean@starlabs.systems>
> > Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
> > Change-Id: I5bcf32105ce85fda95b4ba98a5e420e8f522374c
> > ---
> > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 36 +++++++++++++++-----------
> > MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h | 1 +
> > 2 files changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > index 2b4a4b2444..c992323443 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> > @@ -698,25 +698,11 @@ XhcCheckUsbPortSpeedUsedPsic (
> > SpField.Dword = 0; UsbSpeedIdMap = 0; - //- // Check xHCI
> Supported
> > Protocol Capability, find the PSIV field to match- // PortSpeed
> definition when
> > the Major Revision is 03h.- //- if (Xhc->Usb3SupOffset != 0xFFFFFFFF)
> {-
> > SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);-
> if
> > (SpField.Dword != 0) {- //- // Found the corresponding PORTSC
> value in
> > PSIV field of USB3 offset.- //- UsbSpeedIdMap =
> > USB_PORT_STAT_SUPER_SPEED;- }- }- // // Check xHCI Supported
> Protocol
> > Capability, find the PSIV field to match // PortSpeed definition when
> the Major
> > Revision is 02h. //- if ((UsbSpeedIdMap == 0) && (Xhc->Usb2SupOffset
> !=
> > 0xFFFFFFFF)) {+ if (Xhc->Usb2SupOffset != 0xFFFFFFFF) {
> SpField.Dword =
> > XhciPsivGetPsid (Xhc, Xhc->Usb2SupOffset, PortSpeed); if
> (SpField.Dword != 0)
> > { //@@ -733,6 +719,12 @@ XhcCheckUsbPortSpeedUsedPsic (
> > // PSIM shows as default High-speed protocol, apply to
> High-speed
> > mapping // UsbSpeedIdMap =
> > USB_PORT_STAT_HIGH_SPEED;+ } else if (SpField.Data.Psim ==
> > XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM) {+ //+ //
> > PSIM shows as default Full-speed protocol, return 0+ // to
> ensure no port
> > status set+ //+ return 0; } } else if
> (SpField.Data.Psie == 1)
> > { //@@ -750,6 +742,20 @@ XhcCheckUsbPortSpeedUsedPsic (
> > } } + //+ // Check xHCI Supported Protocol Capability, find the
> PSIV field to
> > match+ // PortSpeed definition when the Major Revision is 03h.+ //+ if
> > ((UsbSpeedIdMap == 0) && (Xhc->Usb3SupOffset != 0xFFFFFFFF)) {+
> > SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);+
> if
> > (SpField.Dword != 0) {+ //+ // Found the corresponding PORTSC
> value in
> > PSIV field of USB3 offset.+ //+ UsbSpeedIdMap =
> > USB_PORT_STAT_SUPER_SPEED;+ }+ }+ return UsbSpeedIdMap; } diff
> --git
> > a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > index 5fe2ba4f0e..74ac6297ba 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> > @@ -85,6 +85,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #define XHC_SUPPORTED_PROTOCOL_DW2_OFFSET 0x08 #define
> > XHC_SUPPORTED_PROTOCOL_PSI_OFFSET 0x10 #define
> > XHC_SUPPORTED_PROTOCOL_USB2_HIGH_SPEED_PSIM 480+#define
> > XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM 12 #define
> > XHC_SUPPORTED_PROTOCOL_USB2_LOW_SPEED_PSIM 1500 #pragma
> > pack (1)--
> > 2.37.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#96843): https://edk2.groups.io/g/devel/message/96843
> > Mute This Topic: https://groups.io/mt/95391831/1768737
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com]
> -=-=-
> > =-=-=-=
> >
>
>
>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 8717 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices
2022-12-02 16:38 ` Sean Rhodes
@ 2022-12-05 1:37 ` Wu, Hao A
0 siblings, 0 replies; 8+ messages in thread
From: Wu, Hao A @ 2022-12-05 1:37 UTC (permalink / raw)
To: Rhodes, Sean, devel@edk2.groups.io; +Cc: Matt DeVillier, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 6636 bytes --]
Hello,
Could you help to send a V2 series of the patches?
Also, a CI link with passed results will be appreciated.
Best Regards,
Hao Wu
From: Sean Rhodes <sean@starlabs.systems>
Sent: Saturday, December 3, 2022 12:38 AM
To: devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>
Cc: Matt DeVillier <matt.devillier@gmail.com>; Ni, Ray <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices
Hi Hao
They are now resolved
Thanks
Sean
On Fri, 2 Dec 2022 at 06:25, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote:
Hello,
I saw there are several CI check failures for this 4 patches:
https://github.com/tianocore/edk2/pull/3702
Could you help to resolve them? Thanks.
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Sean
> Rhodes
> Sent: Friday, December 2, 2022 4:25 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Matt DeVillier <matt.devillier@gmail.com<mailto:matt.devillier@gmail.com>>; Wu, Hao A
> <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Rhodes, Sean
> <sean@starlabs.systems<mailto:sean@starlabs.systems>>
> Subject: [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle
> incorrect PSIV indices
>
> From: Matt DeVillier <matt.devillier@gmail.com<mailto:matt.devillier@gmail.com>>
>
> On some platforms, including Sky Lake and Kaby Lake, the PSIV (Protocol Speed
> ID Value) indicesare 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 extended capability registers for USB2
> before USB3. If edk2 finds a match for a USB 2 device, don't check for USB 3.
>
> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Reviewed-by: Sean Rhodes <sean@starlabs.systems<mailto:sean@starlabs.systems>>
> Signed-off-by: Matt DeVillier <matt.devillier@gmail.com<mailto:matt.devillier@gmail.com>>
> Change-Id: I5bcf32105ce85fda95b4ba98a5e420e8f522374c
> ---
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 36 +++++++++++++++-----------
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h | 1 +
> 2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> index 2b4a4b2444..c992323443 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
> @@ -698,25 +698,11 @@ XhcCheckUsbPortSpeedUsedPsic (
> SpField.Dword = 0; UsbSpeedIdMap = 0; - //- // Check xHCI Supported
> Protocol Capability, find the PSIV field to match- // PortSpeed definition when
> the Major Revision is 03h.- //- if (Xhc->Usb3SupOffset != 0xFFFFFFFF) {-
> SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);- if
> (SpField.Dword != 0) {- //- // Found the corresponding PORTSC value in
> PSIV field of USB3 offset.- //- UsbSpeedIdMap =
> USB_PORT_STAT_SUPER_SPEED;- }- }- // // Check xHCI Supported Protocol
> Capability, find the PSIV field to match // PortSpeed definition when the Major
> Revision is 02h. //- if ((UsbSpeedIdMap == 0) && (Xhc->Usb2SupOffset !=
> 0xFFFFFFFF)) {+ if (Xhc->Usb2SupOffset != 0xFFFFFFFF) { SpField.Dword =
> XhciPsivGetPsid (Xhc, Xhc->Usb2SupOffset, PortSpeed); if (SpField.Dword != 0)
> { //@@ -733,6 +719,12 @@ XhcCheckUsbPortSpeedUsedPsic (
> // PSIM shows as default High-speed protocol, apply to High-speed
> mapping // UsbSpeedIdMap =
> USB_PORT_STAT_HIGH_SPEED;+ } else if (SpField.Data.Psim ==
> XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM) {+ //+ //
> PSIM shows as default Full-speed protocol, return 0+ // to ensure no port
> status set+ //+ return 0; } } else if (SpField.Data.Psie == 1)
> { //@@ -750,6 +742,20 @@ XhcCheckUsbPortSpeedUsedPsic (
> } } + //+ // Check xHCI Supported Protocol Capability, find the PSIV field to
> match+ // PortSpeed definition when the Major Revision is 03h.+ //+ if
> ((UsbSpeedIdMap == 0) && (Xhc->Usb3SupOffset != 0xFFFFFFFF)) {+
> SpField.Dword = XhciPsivGetPsid (Xhc, Xhc->Usb3SupOffset, PortSpeed);+ if
> (SpField.Dword != 0) {+ //+ // Found the corresponding PORTSC value in
> PSIV field of USB3 offset.+ //+ UsbSpeedIdMap =
> USB_PORT_STAT_SUPER_SPEED;+ }+ }+ return UsbSpeedIdMap; } diff --git
> a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> index 5fe2ba4f0e..74ac6297ba 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
> @@ -85,6 +85,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #define XHC_SUPPORTED_PROTOCOL_DW2_OFFSET 0x08 #define
> XHC_SUPPORTED_PROTOCOL_PSI_OFFSET 0x10 #define
> XHC_SUPPORTED_PROTOCOL_USB2_HIGH_SPEED_PSIM 480+#define
> XHC_SUPPORTED_PROTOCOL_USB2_FULL_SPEED_PSIM 12 #define
> XHC_SUPPORTED_PROTOCOL_USB2_LOW_SPEED_PSIM 1500 #pragma
> pack (1)--
> 2.37.2
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#96843): https://edk2.groups.io/g/devel/message/96843
> Mute This Topic: https://groups.io/mt/95391831/1768737
> Group Owner: devel+owner@edk2.groups.io<mailto:devel%2Bowner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>] -=-=-
> =-=-=-=
>
[-- Attachment #2: Type: text/html, Size: 12797 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV
2022-12-05 9:18 Sean Rhodes
@ 2022-12-05 9:18 ` Sean Rhodes
0 siblings, 0 replies; 8+ messages in thread
From: Sean Rhodes @ 2022-12-05 9:18 UTC (permalink / raw)
To: devel; +Cc: Matt DeVillier, Hao A Wu, Ray Ni, Sean Rhodes
From: Matt DeVillier <matt.devillier@gmail.com>
PSID matching relies on comparing the PSIV against the PortSpeed
value. This patch stops edk2 from checking for a PSIV of 0, as it
is not valid; this reduces the number of register access by
approximately 6 per second.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Change-Id: If15c55ab66d2e7faa832ce8576d2e5b47157cc9a
---
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 44 ++++++++++++++++-------------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index c05431ff30..62535cad54 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -371,6 +371,7 @@ XhcGetRootHubPortStatus (
UINT32 TotalPort;
UINTN Index;
UINTN MapSize;
+ UINT8 PortSpeed;
EFI_STATUS Status;
USB_DEV_ROUTE ParentRouteChart;
EFI_TPL OldTpl;
@@ -397,32 +398,37 @@ XhcGetRootHubPortStatus (
State = XhcReadOpReg (Xhc, Offset);
+ PortSpeed = (State & XHC_PORTSC_PS) >> 10;
+
//
// According to XHCI 1.1 spec November 2017,
// Section 7.2 xHCI Support Protocol Capability
//
- 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;
+ if (PortSpeed > 0) {
+ PortStatus->PortStatus = XhcCheckUsbPortSpeedUsedPsic (Xhc, PortSpeed);
+ // If no match found in ext cap reg, fall back to PORTSC
+ 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 (PortSpeed) {
+ 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;
+ }
}
}
--
2.37.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-12-05 9:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-01 20:24 [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Sean Rhodes
2022-12-01 20:24 ` [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV Sean Rhodes
2022-12-01 20:24 ` [PATCH 3/4] MdeModulePkg/BmBoot: Skip removable media if it is not present Sean Rhodes
2022-12-01 20:24 ` [PATCH 4/4] MdeModulePkg/UsbBusDxe: Adjust the MaxPacketLength to real world values Sean Rhodes
2022-12-02 6:25 ` [edk2-devel] [PATCH 1/4] MdeModulePkg/XhciDxe/XhciReg: Handle incorrect PSIV indices Wu, Hao A
2022-12-02 16:38 ` Sean Rhodes
2022-12-05 1:37 ` Wu, Hao A
-- strict thread matches above, loose matches on Subject: below --
2022-12-05 9:18 Sean Rhodes
2022-12-05 9:18 ` [PATCH 2/4] MdeModulePkg/XhciDxe/Xhci: Don't check for invalid PSIV Sean Rhodes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox