[Patch V2] MdeModulePkg/XhciDxe: Error handle for USB slot initialization failure

[Patch V2] MdeModulePkg/XhciDxe: Error handle for USB slot initialization failure

[Heng Luo]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3007

Currently UsbDevContext is not cleaned up if USB slot initialization is
failed, the wrong context data will affect next USB devices and
the USB devices can not be enumerated.
Need to disable slot if USB slot initialization is failed.

Below test cases are passed on UpXtreme:
a. USB 3.0 thumb drives can be recognized in UEFI shell
b. SUT can boot to Puppylinux  from USB3.0 mass storage,
 the storage can be recognized in linux
c. Plug in a USB  keyboard (hot plug) and enumeration is OK
 in UEFI shell and linux
d. Plug in a USB mouse(hot plug) and enumeration is OK in linux.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
---
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 9cb115363c..55f207aec4 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2,7 +2,7 @@
 
   XHCI transfer scheduling routines.
 
-Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2020, Intel Corporation. All rights reserved.<BR>
 Copyright (c) Microsoft Corporation.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -2279,6 +2279,9 @@ XhcInitializeDeviceSlot (
     DeviceAddress = (UINT8) ((DEVICE_CONTEXT *) OutputContext)->Slot.DeviceAddress;
     DEBUG ((EFI_D_INFO, "    Address %d assigned successfully\n", DeviceAddress));
     Xhc->UsbDevContext[SlotId].XhciDevAddr = DeviceAddress;
+  } else {
+    DEBUG ((DEBUG_INFO, "    Address %d assigned unsuccessfully\n"));
+    Status = XhcDisableSlotCmd (Xhc, SlotId);
   }
 
   return Status;
@@ -2489,7 +2492,11 @@ XhcInitializeDeviceSlot64 (
     DeviceAddress = (UINT8) ((DEVICE_CONTEXT_64 *) OutputContext)->Slot.DeviceAddress;
     DEBUG ((EFI_D_INFO, "    Address %d assigned successfully\n", DeviceAddress));
     Xhc->UsbDevContext[SlotId].XhciDevAddr = DeviceAddress;
+  }  else {
+    DEBUG ((DEBUG_INFO, "    Address %d assigned unsuccessfully\n"));
+    Status = XhcDisableSlotCmd (Xhc, SlotId);
   }
+
   return Status;
 }
 
-- 
2.24.0.windows.2

Re: [Patch V2] MdeModulePkg/XhciDxe: Error handle for USB slot initialization failure

[Wu, Hao A]

Sorry for top-posting. Seems my mail agent application does not display the patch content properly.

For the patch, Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
I will remove the extra space character added before the 'else' statement in function XhcInitializeDeviceSlot64() when merging the patch.

I will wait a couple of days to see if there are other comments before pushing.
Heng, could you please help ping me if I forgot to merge the patch in a week's time? Thanks in advance.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Luo, Heng <heng.luo@intel.com>
> Sent: Tuesday, October 20, 2020 11:16 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [Patch V2] MdeModulePkg/XhciDxe: Error handle for USB slot
> initialization failure
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3007
> 
> Currently UsbDevContext is not cleaned up if USB slot initialization is failed,
> the wrong context data will affect next USB devices and the USB devices can
> not be enumerated.
> Need to disable slot if USB slot initialization is failed.
> 
> Below test cases are passed on UpXtreme:
> a. USB 3.0 thumb drives can be recognized in UEFI shell b. SUT can boot to
> Puppylinux  from USB3.0 mass storage,  the storage can be recognized in
> linux c. Plug in a USB  keyboard (hot plug) and enumeration is OK  in UEFI
> shell and linux d. Plug in a USB mouse(hot plug) and enumeration is OK in
> linux.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Heng Luo <heng.luo@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 9cb115363c..55f207aec4 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -2,7 +2,7 @@
>     XHCI transfer scheduling routines. -Copyright (c) 2011 - 2018, Intel
> Corporation. All rights reserved.<BR>+Copyright (c) 2011 - 2020, Intel
> Corporation. All rights reserved.<BR> Copyright (c) Microsoft
> Corporation.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -2279,6
> +2279,9 @@ XhcInitializeDeviceSlot (
>      DeviceAddress = (UINT8) ((DEVICE_CONTEXT *) OutputContext)-
> >Slot.DeviceAddress;     DEBUG ((EFI_D_INFO, "    Address %d assigned
> successfully\n", DeviceAddress));     Xhc-
> >UsbDevContext[SlotId].XhciDevAddr = DeviceAddress;+  } else {+    DEBUG
> ((DEBUG_INFO, "    Address %d assigned unsuccessfully\n"));+    Status =
> XhcDisableSlotCmd (Xhc, SlotId);   }    return Status;@@ -2489,7 +2492,11 @@
> XhcInitializeDeviceSlot64 (
>      DeviceAddress = (UINT8) ((DEVICE_CONTEXT_64 *) OutputContext)-
> >Slot.DeviceAddress;     DEBUG ((EFI_D_INFO, "    Address %d assigned
> successfully\n", DeviceAddress));     Xhc-
> >UsbDevContext[SlotId].XhciDevAddr = DeviceAddress;+  }  else {+    DEBUG
> ((DEBUG_INFO, "    Address %d assigned unsuccessfully\n"));+    Status =
> XhcDisableSlotCmd (Xhc, SlotId);   }+   return Status; } --
> 2.24.0.windows.2