From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from hqnvemgate25.nvidia.com (hqnvemgate25.nvidia.com [216.228.121.64]) by mx.groups.io with SMTP id smtpd.web08.10961.1603375028106594224 for ; Thu, 22 Oct 2020 06:57:08 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nvidia.com header.s=n1 header.b=L88V22wA; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: nvidia.com, ip: 216.228.121.64, mailfrom: jonathanh@nvidia.com) Received: from hqmail.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate25.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Thu, 22 Oct 2020 06:56:19 -0700 Received: from [10.26.45.125] (10.124.1.5) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Thu, 22 Oct 2020 13:57:05 +0000 Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Retry device slot init on failure From: Jon Hunter To: "Luo, Heng" , "Wu, Hao A" , "devel@edk2.groups.io" , "jbrasen@nvidia.com" CC: "Ni, Ray" References: <040616b6-c7e9-d28d-69ac-bccb406d4730@nvidia.com> Message-ID: <206d9891-152b-3954-1b61-bf144cab4ed5@nvidia.com> Date: Thu, 22 Oct 2020 14:57:04 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <040616b6-c7e9-d28d-69ac-bccb406d4730@nvidia.com> Return-Path: jonathanh@nvidia.com X-Originating-IP: [10.124.1.5] X-ClientProxiedBy: HQMAIL101.nvidia.com (172.20.187.10) To HQMAIL107.nvidia.com (172.20.187.13) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1603374979; bh=VasSz2uD97ZC8sRyNVTHdkJ3LPhdpq/PvXSaYhaVqxc=; h=Subject:From:To:CC:References:Message-ID:Date:User-Agent: MIME-Version:In-Reply-To:Content-Type:Content-Language: Content-Transfer-Encoding:X-Originating-IP:X-ClientProxiedBy; b=L88V22wAoOtlU8Ec8Znani/f95EA5fQDj5szMRLU9XPyDW8aLh715yB/CGykSq91V gLhor2q0fOvJOaxytCzbktiTBFDWGFLDBm+l0Dfh+FoOwzf8i6rPyebwqRuR1ANKy/ KsmeIenVf911g7f3f1OBaUp4XlcUN6uip5b/VueYEz8XsG4ruRi0b8DUiL27D92g/G kGyr+vlc8nS/i6Sb6sFVqQR/L9FjqxAr2cn7uNME1q4wyf3J3Dyo4oWTHmY/1pOHDm GT+dkCCdgSPgsH7up97keSZiZCXK5juwkLxa8o2sREzQSZLt2kHzKaf1ZjFyOTjzJg ebAdqPMCbRZmQ== Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Heng, On 22/10/2020 09:44, Jon Hunter wrote: > Hi Heng, > > On 22/10/2020 04:01, Luo, Heng wrote: >> Hi Hao, >> I applying this patch, it cannot fix BZ https://bugzilla.tianocore.org/show_bug.cgi?id=3007. > > Can you elaborate more on why that is? Unfortunately, your patch does > not work for the scenario we are testing. > > In your case does the bad USB device never enumerate even on the 2nd > attempt? > > If so, then maybe we can apply your patch to do the disable slot and in > our patch we just try the device init again once, without calling the > slot disable because that is handled by your patch. Applying the below change on top of your change, works for me. One change in the below is not to return the status from the call to XhcDisableSlotCmd64() which is necessary to retry the device initialisation. Jon diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c index 21bc9ce17556..82efd4a03f8e 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c @@ -1684,9 +1684,11 @@ XhcPollPortStatusChange ( EFI_STATUS Status; UINT8 Speed; UINT8 SlotId; + UINT8 Retries; USB_DEV_ROUTE RouteChart; Status = EFI_SUCCESS; + Retries = XHC_INIT_DEVICE_SLOT_RETRIES; if ((PortState->PortChangeStatus & (USB_PORT_STAT_C_CONNECTION | USB_PORT_STAT_C_ENABLE | USB_PORT_STAT_C_OVERCURRENT | USB_PORT_STAT_C_RESET)) == 0) { return EFI_SUCCESS; @@ -1728,17 +1730,29 @@ XhcPollPortStatusChange ( } else if ((PortState->PortStatus & USB_PORT_STAT_SUPER_SPEED) != 0) { Speed = EFI_USB_SPEED_SUPER; } - // - // Execute Enable_Slot cmd for attached device, initialize device context and assign device address. - // - SlotId = XhcRouteStringToSlotId (Xhc, RouteChart); - if ((SlotId == 0) && ((PortState->PortChangeStatus & USB_PORT_STAT_C_RESET) != 0)) { - if (Xhc->HcCParams.Data.Csz == 0) { - Status = XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Port, RouteChart, Speed); - } else { - Status = XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, Port, RouteChart, Speed); + + do { + // + // Execute Enable_Slot cmd for attached device, initialize device context and assign device address. + // + SlotId = XhcRouteStringToSlotId (Xhc, RouteChart); + if ((SlotId == 0) && ((PortState->PortChangeStatus & USB_PORT_STAT_C_RESET) != 0)) { + if (Xhc->HcCParams.Data.Csz == 0) { + Status = XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Port, RouteChart, Speed); + } else { + Status = XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, Port, RouteChart, Speed); + } } - } + + // + // According to the xHCI specification (section 4.6.5), "a USB Transaction + // Error Completion Code for an Address Device Command may be due to a Stall + // response from a device. Software should issue a Disable Slot Command for + // the Device Slot then an Enable Slot Command to recover from this error." + // Therefore, retry the device slot initialization if it fails due to a + // device error. + // + } while ((Status == EFI_DEVICE_ERROR) && (Retries--)); } return Status; @@ -2248,7 +2262,7 @@ XhcInitializeDeviceSlot ( Xhc->UsbDevContext[SlotId].XhciDevAddr = DeviceAddress; } else { DEBUG ((DEBUG_INFO, " Address %d assigned unsuccessfully\n")); - Status = XhcDisableSlotCmd (Xhc, SlotId); + XhcDisableSlotCmd (Xhc, SlotId); } return Status; @@ -2461,7 +2475,7 @@ XhcInitializeDeviceSlot64 ( Xhc->UsbDevContext[SlotId].XhciDevAddr = DeviceAddress; } else { DEBUG ((DEBUG_INFO, " Address %d assigned unsuccessfully\n")); - Status = XhcDisableSlotCmd64 (Xhc, SlotId); + XhcDisableSlotCmd64 (Xhc, SlotId); } return Status; diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h index 2f1899502151..3f9cdb1c3609 100644 --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h @@ -11,6 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define _EFI_XHCI_SCHED_H_ #define XHC_URB_SIG SIGNATURE_32 ('U', 'S', 'B', 'R') +#define XHC_INIT_DEVICE_SLOT_RETRIES 1 -- nvpublic