* [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
@ 2023-09-25 1:54 Rick Tseng via groups.io
2023-10-13 2:13 ` Ashish Singhal via groups.io
0 siblings, 1 reply; 4+ messages in thread
From: Rick Tseng via groups.io @ 2023-09-25 1:54 UTC (permalink / raw)
To: devel
Cc: hao.a.wu, ray.ni, jian.j.wang, gaoliming, ashishsingha, jbrasen,
Rick Tseng
To fix the assert due to ASSERT(TrsRing !=NULL) in XhcSyncTrsRing.
There is a recovery in usb mass stroage driver to do port reset after
fail in transfer. And port reset operation makes that all memory
resources(descriptors, endpoint memory) belonging to this device are
reclaimed in underlying Xhci driver.
Because USB descriptors are reclaimed in underlying Xhci driver, the
next "set config" would not trigger the "Configuration Endpoint"
command in Xhci driver as there is no existing configuation
descriptor. It would prevent from allocating memory from xhci for
endpoints.
Thus there is no memmory allocated for transfering on non-default
endpoint, it makes the assert happens when usb mass stroage tries
to transfer after reset.
This change is to refetch the device and config descriptor after
port reset, it would rebuild device and configuration descriptor in
Xhci driver. So the "set config" command would trigger the
"Configuration Endpoint" in xhci and allocate memory for endpoints.
Signed-off-by: Rick Tseng <rtseng@nvidia.com>
---
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 77 ++++++++++++++++++++++--
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h | 25 ++++++++
2 files changed, 96 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
index c25f3cc2f2..f6e1f88b3d 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
@@ -815,12 +815,16 @@ UsbIoPortReset (
IN EFI_USB_IO_PROTOCOL *This
)
{
- USB_INTERFACE *UsbIf;
- USB_INTERFACE *HubIf;
- USB_DEVICE *Dev;
- EFI_TPL OldTpl;
- EFI_STATUS Status;
- UINT8 DevAddress;
+ USB_INTERFACE *UsbIf;
+ USB_INTERFACE *HubIf;
+ USB_DEVICE *Dev;
+ EFI_TPL OldTpl;
+ EFI_STATUS Status;
+ UINT8 DevAddress;
+ USB_DEVICE_DESC DevDesc;
+ UINT8 NumConfig;
+ VOID *Buf;
+ UINT8 Index;
OldTpl = gBS->RaiseTPL (USB_BUS_TPL);
@@ -882,6 +886,67 @@ UsbIoPortReset (
// is in CONFIGURED state.
//
if (Dev->ActiveConfig != NULL) {
+ //
+ // We just do a port reset, so we need to do as a new device enumeration.
+ // Need to get the device descriptor, configuration descriptor before set config
+ //
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_DEVICE, 0, 0, &DevDesc, sizeof (DevDesc));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get device desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ NumConfig = Dev->DevDesc->Desc.NumConfigurations;
+ if (NumConfig == 0) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ for (Index = 0; Index < NumConfig; Index++) {
+ if (Dev->DevDesc->Configs[Index] == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for index %d - %r\n",
+ Index,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Buf = AllocateZeroPool (Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ if (Buf == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: allocation memory fail for configuration desc - %r\n",
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_CONFIG, Index, 0, Buf, Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ FreePool (Buf);
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get config desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+ }
+
Status = UsbSetConfig (Dev, Dev->ActiveConfig->Desc.ConfigurationValue);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
index ce205e706d..5ae5c4056b 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
@@ -224,4 +224,29 @@ UsbIoClearFeature (
IN UINT16 Index
);
+/**
+ Get the standard descriptors.
+
+ @param UsbDev The USB device to read descriptor from.
+ @param DescType The type of descriptor to read.
+ @param DescIndex The index of descriptor to read.
+ @param LangId Language ID, only used to get string, otherwise set
+ it to 0.
+ @param Buf The buffer to hold the descriptor read.
+ @param Length The length of the buffer.
+
+ @retval EFI_SUCCESS The descriptor is read OK.
+ @retval Others Failed to retrieve the descriptor.
+
+**/
+EFI_STATUS
+UsbCtrlGetDesc (
+ IN USB_DEVICE *UsbDev,
+ IN UINTN DescType,
+ IN UINTN DescIndex,
+ IN UINT16 LangId,
+ OUT VOID *Buf,
+ IN UINTN Length
+ );
+
#endif
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109042): https://edk2.groups.io/g/devel/message/109042
Mute This Topic: https://groups.io/mt/101576341/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
2023-09-25 1:54 [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset Rick Tseng via groups.io
@ 2023-10-13 2:13 ` Ashish Singhal via groups.io
2023-10-16 1:54 ` Wu, Hao A
0 siblings, 1 reply; 4+ messages in thread
From: Ashish Singhal via groups.io @ 2023-10-13 2:13 UTC (permalink / raw)
To: Rick Tseng, devel@edk2.groups.io, hao.a.wu@intel.com,
ray.ni@intel.com, jian.j.wang@intel.com, gaoliming@byosoft.com.cn
Cc: Jeff Brasen
[-- Attachment #1: Type: text/plain, Size: 6331 bytes --]
Hello Hao/Ray/Jian/Gao,
Any feedback on this patch?
Thanks
Ashish
________________________________
From: Rick Tseng <rtseng@nvidia.com>
Sent: Sunday, September 24, 2023 7:54 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: hao.a.wu@intel.com <hao.a.wu@intel.com>; ray.ni@intel.com <ray.ni@intel.com>; jian.j.wang@intel.com <jian.j.wang@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; Ashish Singhal <ashishsingha@nvidia.com>; Jeff Brasen <jbrasen@nvidia.com>; Rick Tseng <rtseng@nvidia.com>
Subject: [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
To fix the assert due to ASSERT(TrsRing !=NULL) in XhcSyncTrsRing.
There is a recovery in usb mass stroage driver to do port reset after
fail in transfer. And port reset operation makes that all memory
resources(descriptors, endpoint memory) belonging to this device are
reclaimed in underlying Xhci driver.
Because USB descriptors are reclaimed in underlying Xhci driver, the
next "set config" would not trigger the "Configuration Endpoint"
command in Xhci driver as there is no existing configuation
descriptor. It would prevent from allocating memory from xhci for
endpoints.
Thus there is no memmory allocated for transfering on non-default
endpoint, it makes the assert happens when usb mass stroage tries
to transfer after reset.
This change is to refetch the device and config descriptor after
port reset, it would rebuild device and configuration descriptor in
Xhci driver. So the "set config" command would trigger the
"Configuration Endpoint" in xhci and allocate memory for endpoints.
Signed-off-by: Rick Tseng <rtseng@nvidia.com>
---
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 77 ++++++++++++++++++++++--
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h | 25 ++++++++
2 files changed, 96 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
index c25f3cc2f2..f6e1f88b3d 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
@@ -815,12 +815,16 @@ UsbIoPortReset (
IN EFI_USB_IO_PROTOCOL *This
)
{
- USB_INTERFACE *UsbIf;
- USB_INTERFACE *HubIf;
- USB_DEVICE *Dev;
- EFI_TPL OldTpl;
- EFI_STATUS Status;
- UINT8 DevAddress;
+ USB_INTERFACE *UsbIf;
+ USB_INTERFACE *HubIf;
+ USB_DEVICE *Dev;
+ EFI_TPL OldTpl;
+ EFI_STATUS Status;
+ UINT8 DevAddress;
+ USB_DEVICE_DESC DevDesc;
+ UINT8 NumConfig;
+ VOID *Buf;
+ UINT8 Index;
OldTpl = gBS->RaiseTPL (USB_BUS_TPL);
@@ -882,6 +886,67 @@ UsbIoPortReset (
// is in CONFIGURED state.
//
if (Dev->ActiveConfig != NULL) {
+ //
+ // We just do a port reset, so we need to do as a new device enumeration.
+ // Need to get the device descriptor, configuration descriptor before set config
+ //
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_DEVICE, 0, 0, &DevDesc, sizeof (DevDesc));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get device desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ NumConfig = Dev->DevDesc->Desc.NumConfigurations;
+ if (NumConfig == 0) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ for (Index = 0; Index < NumConfig; Index++) {
+ if (Dev->DevDesc->Configs[Index] == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for index %d - %r\n",
+ Index,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Buf = AllocateZeroPool (Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ if (Buf == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: allocation memory fail for configuration desc - %r\n",
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_CONFIG, Index, 0, Buf, Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ FreePool (Buf);
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get config desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+ }
+
Status = UsbSetConfig (Dev, Dev->ActiveConfig->Desc.ConfigurationValue);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
index ce205e706d..5ae5c4056b 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
@@ -224,4 +224,29 @@ UsbIoClearFeature (
IN UINT16 Index
);
+/**
+ Get the standard descriptors.
+
+ @param UsbDev The USB device to read descriptor from.
+ @param DescType The type of descriptor to read.
+ @param DescIndex The index of descriptor to read.
+ @param LangId Language ID, only used to get string, otherwise set
+ it to 0.
+ @param Buf The buffer to hold the descriptor read.
+ @param Length The length of the buffer.
+
+ @retval EFI_SUCCESS The descriptor is read OK.
+ @retval Others Failed to retrieve the descriptor.
+
+**/
+EFI_STATUS
+UsbCtrlGetDesc (
+ IN USB_DEVICE *UsbDev,
+ IN UINTN DescType,
+ IN UINTN DescIndex,
+ IN UINT16 LangId,
+ OUT VOID *Buf,
+ IN UINTN Length
+ );
+
#endif
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109578): https://edk2.groups.io/g/devel/message/109578
Mute This Topic: https://groups.io/mt/101576341/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 12784 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
2023-10-13 2:13 ` Ashish Singhal via groups.io
@ 2023-10-16 1:54 ` Wu, Hao A
2023-10-16 13:23 ` Chesley, Brit via groups.io
0 siblings, 1 reply; 4+ messages in thread
From: Wu, Hao A @ 2023-10-16 1:54 UTC (permalink / raw)
To: Ashish Singhal, Rick Tseng, devel@edk2.groups.io, Ni, Ray,
Wang, Jian J, Gao, Liming
Cc: Jeff Brasen
[-- Attachment #1: Type: text/plain, Size: 7322 bytes --]
Please grant me some time to review this patch.
Really sorry that I am not able to provide an estimated time for the review.
Best Regards,
Hao Wu
From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Friday, October 13, 2023 10:13 AM
To: Rick Tseng <rtseng@nvidia.com>; devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Cc: Jeff Brasen <jbrasen@nvidia.com>
Subject: Re: [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
Hello Hao/Ray/Jian/Gao,
Any feedback on this patch?
Thanks
Ashish
________________________________
From: Rick Tseng <rtseng@nvidia.com<mailto:rtseng@nvidia.com>>
Sent: Sunday, September 24, 2023 7:54 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: hao.a.wu@intel.com<mailto:hao.a.wu@intel.com> <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; ray.ni@intel.com<mailto:ray.ni@intel.com> <ray.ni@intel.com<mailto:ray.ni@intel.com>>; jian.j.wang@intel.com<mailto:jian.j.wang@intel.com> <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Rick Tseng <rtseng@nvidia.com<mailto:rtseng@nvidia.com>>
Subject: [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
To fix the assert due to ASSERT(TrsRing !=NULL) in XhcSyncTrsRing.
There is a recovery in usb mass stroage driver to do port reset after
fail in transfer. And port reset operation makes that all memory
resources(descriptors, endpoint memory) belonging to this device are
reclaimed in underlying Xhci driver.
Because USB descriptors are reclaimed in underlying Xhci driver, the
next "set config" would not trigger the "Configuration Endpoint"
command in Xhci driver as there is no existing configuation
descriptor. It would prevent from allocating memory from xhci for
endpoints.
Thus there is no memmory allocated for transfering on non-default
endpoint, it makes the assert happens when usb mass stroage tries
to transfer after reset.
This change is to refetch the device and config descriptor after
port reset, it would rebuild device and configuration descriptor in
Xhci driver. So the "set config" command would trigger the
"Configuration Endpoint" in xhci and allocate memory for endpoints.
Signed-off-by: Rick Tseng <rtseng@nvidia.com<mailto:rtseng@nvidia.com>>
---
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 77 ++++++++++++++++++++++--
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h | 25 ++++++++
2 files changed, 96 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
index c25f3cc2f2..f6e1f88b3d 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
@@ -815,12 +815,16 @@ UsbIoPortReset (
IN EFI_USB_IO_PROTOCOL *This
)
{
- USB_INTERFACE *UsbIf;
- USB_INTERFACE *HubIf;
- USB_DEVICE *Dev;
- EFI_TPL OldTpl;
- EFI_STATUS Status;
- UINT8 DevAddress;
+ USB_INTERFACE *UsbIf;
+ USB_INTERFACE *HubIf;
+ USB_DEVICE *Dev;
+ EFI_TPL OldTpl;
+ EFI_STATUS Status;
+ UINT8 DevAddress;
+ USB_DEVICE_DESC DevDesc;
+ UINT8 NumConfig;
+ VOID *Buf;
+ UINT8 Index;
OldTpl = gBS->RaiseTPL (USB_BUS_TPL);
@@ -882,6 +886,67 @@ UsbIoPortReset (
// is in CONFIGURED state.
//
if (Dev->ActiveConfig != NULL) {
+ //
+ // We just do a port reset, so we need to do as a new device enumeration.
+ // Need to get the device descriptor, configuration descriptor before set config
+ //
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_DEVICE, 0, 0, &DevDesc, sizeof (DevDesc));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get device desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ NumConfig = Dev->DevDesc->Desc.NumConfigurations;
+ if (NumConfig == 0) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ for (Index = 0; Index < NumConfig; Index++) {
+ if (Dev->DevDesc->Configs[Index] == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for index %d - %r\n",
+ Index,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Buf = AllocateZeroPool (Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ if (Buf == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: allocation memory fail for configuration desc - %r\n",
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_CONFIG, Index, 0, Buf, Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ FreePool (Buf);
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get config desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+ }
+
Status = UsbSetConfig (Dev, Dev->ActiveConfig->Desc.ConfigurationValue);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
index ce205e706d..5ae5c4056b 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
@@ -224,4 +224,29 @@ UsbIoClearFeature (
IN UINT16 Index
);
+/**
+ Get the standard descriptors.
+
+ @param UsbDev The USB device to read descriptor from.
+ @param DescType The type of descriptor to read.
+ @param DescIndex The index of descriptor to read.
+ @param LangId Language ID, only used to get string, otherwise set
+ it to 0.
+ @param Buf The buffer to hold the descriptor read.
+ @param Length The length of the buffer.
+
+ @retval EFI_SUCCESS The descriptor is read OK.
+ @retval Others Failed to retrieve the descriptor.
+
+**/
+EFI_STATUS
+UsbCtrlGetDesc (
+ IN USB_DEVICE *UsbDev,
+ IN UINTN DescType,
+ IN UINTN DescIndex,
+ IN UINT16 LangId,
+ OUT VOID *Buf,
+ IN UINTN Length
+ );
+
#endif
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109619): https://edk2.groups.io/g/devel/message/109619
Mute This Topic: https://groups.io/mt/101576341/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 15983 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
2023-10-16 1:54 ` Wu, Hao A
@ 2023-10-16 13:23 ` Chesley, Brit via groups.io
0 siblings, 0 replies; 4+ messages in thread
From: Chesley, Brit via groups.io @ 2023-10-16 13:23 UTC (permalink / raw)
To: devel@edk2.groups.io, hao.a.wu@intel.com, Ashish Singhal,
Rick Tseng, Ni, Ray, Wang, Jian J, Gao, Liming
Cc: Jeff Brasen
[-- Attachment #1.1: Type: text/plain, Size: 8521 bytes --]
[Public]
Hello Hao, Ashish, Rick,
I recently uploaded a patch to fix the same assert (BZ 4456), but it is still under review although I haven't heard anything since August. Please look at my corresponding patch which I have attached as an outlook item, and we can discuss which patch should be merged to fix the issue.
Thanks,
Brit Chesley
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A via groups.io
Sent: Sunday, October 15, 2023 8:54 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; Rick Tseng <rtseng@nvidia.com>; devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Cc: Jeff Brasen <jbrasen@nvidia.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
Please grant me some time to review this patch.
Really sorry that I am not able to provide an estimated time for the review.
Best Regards,
Hao Wu
From: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
Sent: Friday, October 13, 2023 10:13 AM
To: Rick Tseng <rtseng@nvidia.com<mailto:rtseng@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; 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>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Subject: Re: [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
Hello Hao/Ray/Jian/Gao,
Any feedback on this patch?
Thanks
Ashish
________________________________
From: Rick Tseng <rtseng@nvidia.com<mailto:rtseng@nvidia.com>>
Sent: Sunday, September 24, 2023 7:54 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: hao.a.wu@intel.com<mailto:hao.a.wu@intel.com> <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; ray.ni@intel.com<mailto:ray.ni@intel.com> <ray.ni@intel.com<mailto:ray.ni@intel.com>>; jian.j.wang@intel.com<mailto:jian.j.wang@intel.com> <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Rick Tseng <rtseng@nvidia.com<mailto:rtseng@nvidia.com>>
Subject: [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset
To fix the assert due to ASSERT(TrsRing !=NULL) in XhcSyncTrsRing.
There is a recovery in usb mass stroage driver to do port reset after
fail in transfer. And port reset operation makes that all memory
resources(descriptors, endpoint memory) belonging to this device are
reclaimed in underlying Xhci driver.
Because USB descriptors are reclaimed in underlying Xhci driver, the
next "set config" would not trigger the "Configuration Endpoint"
command in Xhci driver as there is no existing configuation
descriptor. It would prevent from allocating memory from xhci for
endpoints.
Thus there is no memmory allocated for transfering on non-default
endpoint, it makes the assert happens when usb mass stroage tries
to transfer after reset.
This change is to refetch the device and config descriptor after
port reset, it would rebuild device and configuration descriptor in
Xhci driver. So the "set config" command would trigger the
"Configuration Endpoint" in xhci and allocate memory for endpoints.
Signed-off-by: Rick Tseng <rtseng@nvidia.com<mailto:rtseng@nvidia.com>>
---
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 77 ++++++++++++++++++++++--
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h | 25 ++++++++
2 files changed, 96 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
index c25f3cc2f2..f6e1f88b3d 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
@@ -815,12 +815,16 @@ UsbIoPortReset (
IN EFI_USB_IO_PROTOCOL *This
)
{
- USB_INTERFACE *UsbIf;
- USB_INTERFACE *HubIf;
- USB_DEVICE *Dev;
- EFI_TPL OldTpl;
- EFI_STATUS Status;
- UINT8 DevAddress;
+ USB_INTERFACE *UsbIf;
+ USB_INTERFACE *HubIf;
+ USB_DEVICE *Dev;
+ EFI_TPL OldTpl;
+ EFI_STATUS Status;
+ UINT8 DevAddress;
+ USB_DEVICE_DESC DevDesc;
+ UINT8 NumConfig;
+ VOID *Buf;
+ UINT8 Index;
OldTpl = gBS->RaiseTPL (USB_BUS_TPL);
@@ -882,6 +886,67 @@ UsbIoPortReset (
// is in CONFIGURED state.
//
if (Dev->ActiveConfig != NULL) {
+ //
+ // We just do a port reset, so we need to do as a new device enumeration.
+ // Need to get the device descriptor, configuration descriptor before set config
+ //
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_DEVICE, 0, 0, &DevDesc, sizeof (DevDesc));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get device desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ NumConfig = Dev->DevDesc->Desc.NumConfigurations;
+ if (NumConfig == 0) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ for (Index = 0; Index < NumConfig; Index++) {
+ if (Dev->DevDesc->Configs[Index] == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: no configuration descriptor for index %d - %r\n",
+ Index,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Buf = AllocateZeroPool (Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ if (Buf == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: allocation memory fail for configuration desc - %r\n",
+ Status
+ ));
+ goto ON_EXIT;
+ }
+
+ Status = UsbCtrlGetDesc (Dev, USB_DESC_TYPE_CONFIG, Index, 0, Buf, Dev->DevDesc->Configs[Index]->Desc.TotalLength);
+ FreePool (Buf);
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "UsbIoPortReset: failed to get config desc for device %d - %r\n",
+ Dev->Address,
+ Status
+ ));
+ goto ON_EXIT;
+ }
+ }
+
Status = UsbSetConfig (Dev, Dev->ActiveConfig->Desc.ConfigurationValue);
if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
index ce205e706d..5ae5c4056b 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
@@ -224,4 +224,29 @@ UsbIoClearFeature (
IN UINT16 Index
);
+/**
+ Get the standard descriptors.
+
+ @param UsbDev The USB device to read descriptor from.
+ @param DescType The type of descriptor to read.
+ @param DescIndex The index of descriptor to read.
+ @param LangId Language ID, only used to get string, otherwise set
+ it to 0.
+ @param Buf The buffer to hold the descriptor read.
+ @param Length The length of the buffer.
+
+ @retval EFI_SUCCESS The descriptor is read OK.
+ @retval Others Failed to retrieve the descriptor.
+
+**/
+EFI_STATUS
+UsbCtrlGetDesc (
+ IN USB_DEVICE *UsbDev,
+ IN UINTN DescType,
+ IN UINTN DescIndex,
+ IN UINT16 LangId,
+ OUT VOID *Buf,
+ IN UINTN Length
+ );
+
#endif
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109644): https://edk2.groups.io/g/devel/message/109644
Mute This Topic: https://groups.io/mt/101576341/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.2: Type: text/html, Size: 18772 bytes --]
[-- Attachment #2: Type: message/rfc822, Size: 11900 bytes --]
From: "Chesley, Brit via groups.io" <brit.chesley=amd.com@groups.io>
To: "Wu, Hao A" <hao.a.wu@intel.com>, "devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>, "Gao, Liming" <gaoliming@byosoft.com.cn>, "Ni, Ray" <ray.ni@intel.com>, "Chang, Abner" <Abner.Chang@amd.com>
Subject: Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: UsbBusDxe: Rebuild Descriptor Table
Date: Wed, 16 Aug 2023 16:49:26 +0000
Message-ID: <177BEB17BB6F2D4E.12117@groups.io>
[AMD Official Use Only - General]
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
[AMD Official Use Only - General]
Hello Hao,
Its no problem. I agree that the endpoint transfer rings should be allocated after the UsbSetConfig command, but this is not the case. In XhcControlTransfer, after the if statement checking for USB_REQ_SET_CONFIG, the for-loop loops through all of DevDesc.NumConfigurations and calls XhcSetConfigCmd. The issue here is that after a reset is issued XhcInitializeDeviceSlot64 is called which frees Xhc->UsbDevContext[SlotId]. This sets Xhc->UsbDevContext[SlotId].DevDesc.NumConfigurations to 0. So XhcSetConfigCmd is never called, and the other endpoint transfer rings besides the default are never reinitialized. From what I could tell the easiest way to reacquire the proper NumConfigurations value was to call UsbBuildDescTable for the device.
Best,
Brit Chesley
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Monday, July 31, 2023 2:37 AM
> To: devel@edk2.groups.io; Chesley, Brit <Brit.Chesley@amd.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Chang, Abner
> <Abner.Chang@amd.com>
> Subject: RE: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: UsbBusDxe: Rebuild
> Descriptor Table
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > brit.chesley via groups.io
> > Sent: Saturday, July 8, 2023 1:07 AM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Abner Chang <Abner.Chang@amd.com>
> > Subject: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: UsbBusDxe: Rebuild
> > Descriptor Table
> >
> > From: Britton Chesley <Brit.Chesley@amd.com>
> >
> > Fixed a bug which led to an ASSERT due to the USB device context being
> > maintained after a port reset, but the underlying XHCI context was
> > uninitialized. Specifically, Xhc->UsbDevContext is freed after a reset
> > and only re-allocates the default [0] enpoint transfer ring. Added
> > build
>
>
> Really sorry for another question.
>
> My take is that the transfer ring of other endpoints (besides the Default
> Control Endpoint) will be re-initialized by the below flow:
> UsbSetConfig -> UsbCtrlRequest (USB_REQ_SET_CONFIG) ->
> XhcSetConfigCmd(64) -> XhcInitializeEndpointContext(64)
>
> Could you help to elaborate a bit more on what is the issue with the above
> (current) flow and why rebuilding the Descriptor Table before UsbSetConfig
> can resolve it?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > descriptor table call in UsbIoPortReset. BZ 4456
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Abner Chang <Abner.Chang@amd.com>
> > Signed-off-by: Britton Chesley <Brit.Chesley@amd.com>
> > ---
> > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 26
> > ++++++++++++++++++++++++-
> > 1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > index c25f3cc2f279..a5b798bd8d6c 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > @@ -821,6 +821,7 @@ UsbIoPortReset (
> > EFI_TPL OldTpl;
> > EFI_STATUS Status;
> > UINT8 DevAddress;
> > + UINT8 Config;
> >
> > OldTpl = gBS->RaiseTPL (USB_BUS_TPL);
> >
> > @@ -882,8 +883,26 @@ UsbIoPortReset (
> > // is in CONFIGURED state.
> > //
> > if (Dev->ActiveConfig != NULL) {
> > - Status = UsbSetConfig (Dev, Dev->ActiveConfig-
> >Desc.ConfigurationValue);
> > + UsbFreeDevDesc (Dev->DevDesc);
> >
> > + Status = UsbRemoveConfig (Dev);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to remove
> > configuration - %r\n", Status));
> > + }
> > +
> > + Status = UsbGetMaxPacketSize0 (Dev);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to get max packet
> > + size -
> > %r\n", Status));
> > + }
> > +
> > + Status = UsbBuildDescTable (Dev);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to build
> > + descriptor
> > table - %r\n", Status));
> > + }
> > +
> > + Config = Dev->DevDesc->Configs[0]->Desc.ConfigurationValue;
> > +
> > + Status = UsbSetConfig (Dev, Config);
> > if (EFI_ERROR (Status)) {
> > DEBUG ((
> > DEBUG_ERROR,
> > @@ -892,6 +911,11 @@ UsbIoPortReset (
> > Status
> > ));
> > }
> > +
> > + Status = UsbSelectConfig (Dev, Config);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to set
> > + configuration -
> > %r\n", Status));
> > + }
> > }
> >
> > ON_EXIT:
> > --
> > 2.36.1
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107798): https://edk2.groups.io/g/devel/message/107798
Mute This Topic: https://groups.io/mt/100010162/7581415
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [brit.chesley@amd.com]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-16 13:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-25 1:54 [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset Rick Tseng via groups.io
2023-10-13 2:13 ` Ashish Singhal via groups.io
2023-10-16 1:54 ` Wu, Hao A
2023-10-16 13:23 ` Chesley, Brit via groups.io
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox