Re: [edk2-devel] [PATCH] MdeModulePkg/UsbBus: Get device/config descriptor after port reset

["Ashish Singhal via groups.io" <ashishsingha=nvidia.com@groups.io>]

[-- 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 --]