From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web09.24460.1604280737875451177 for ; Sun, 01 Nov 2020 17:32:18 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=RNApCY00; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: hao.a.wu@intel.com) IronPort-SDR: FjSKbpsKmgnqXPSLHGKMUJwliVYw/WEeGVD6esaETJ+LkLpcwl4sW95LPfBCbVEFKOhPNdY40x N8Ut8fdzsU4Q== X-IronPort-AV: E=McAfee;i="6000,8403,9792"; a="232993546" X-IronPort-AV: E=Sophos;i="5.77,443,1596524400"; d="scan'208";a="232993546" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Nov 2020 17:32:16 -0800 IronPort-SDR: ka9Mxyo+Ju0F8GIlWvATyZEmG24b6immuA6aa4RhYvYz3jm5R5nALBXxv8IQxUDqLw0vh6wyJ0 akU+1A20CRIw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,443,1596524400"; d="scan'208";a="352572900" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga004.fm.intel.com with ESMTP; 01 Nov 2020 17:32:16 -0800 Received: from orsmsx602.amr.corp.intel.com (10.22.229.15) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 1 Nov 2020 17:32:15 -0800 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Sun, 1 Nov 2020 17:32:15 -0800 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (104.47.36.52) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Sun, 1 Nov 2020 17:32:15 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=APjLgtaLJ0lhCv/GWcG3HGFg4DMU83xh8RdCTec9Mb1T2rbC3XQpvAALL+PeQ4XQD4Euc03KLhq/9aV/Ud+4f3Y06hFLkvDFhtOn5rB0q5mqNOwD7xaL4sdUeSV7HrA0S42MGdgH4YbhAHC6wURFxykhLqjre92MB0HKY8HD7GKQzarKIIej/q3eMqL3s/YI8pQXRuD9fw+9uxAMWgDO4AoL1ZgeDfK2qcxXbVkwzv4JMpsLPpee/BtNpBjtZjvvVV8UetfdkOsH4AJy3NCHEykLmJwJ1GQAP198DSq+4JkcJelJDcZR72Rfg+Pc8N/AS5liE7ydZtyZP66+3np1CQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=rRcXAY9/GfJztxhVxF4d/TzIN7LvzEoHFNHu05S7FNQ=; b=d8LTVSKfOQ/PniIa8IbnWTd2RDfMJxnGgTqM1FuhCZbTEbmTrLGPK4XAGukamP9HQJudwvcaiMMDVZfUuJBu/4yjz8nDVY5Sr2TVsiPC/+QAw+mhMvC8oVtsLLeuIlmmV4/Qo4fpz2z/c2bujrVXMMtizsoRWuITO/EVfAqoXxCmEYz4MCakYNeSofqjKTak2bhdQ9Yl5qrz24FCwTZyTM153QBFza+nfE6fyyrw4BQvhF0AwnR9Ctga+iMLcVbsn8vnoGLRQVHtQpbYKbPmq+3ZYh9HjY12mLqni34P7UxzBHE1kf8d48tHSF6JT26ecRyITn2m+W2dBw8OSv3eaw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=rRcXAY9/GfJztxhVxF4d/TzIN7LvzEoHFNHu05S7FNQ=; b=RNApCY00B0PlopaexSxihHR9NfwQKQflSJ3u7ggqoP1XDzNlvuyi/f08+o2LwO+9Rk9+zgI+x3YaAIcBN4Rg1OV0itGeGEFCNMGxnx7Xs/JPr1IohquEyxHQyLR6a0ceA/f/u50HHVrn/DzLKDDOUWvmbCVJV4ZuBCFyW3rw3f8= Received: from BN8PR11MB3666.namprd11.prod.outlook.com (2603:10b6:408:8c::19) by BN6PR11MB3987.namprd11.prod.outlook.com (2603:10b6:405:78::33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.27; Mon, 2 Nov 2020 01:32:11 +0000 Received: from BN8PR11MB3666.namprd11.prod.outlook.com ([fe80::c123:faac:1da3:f807]) by BN8PR11MB3666.namprd11.prod.outlook.com ([fe80::c123:faac:1da3:f807%5]) with mapi id 15.20.3499.029; Mon, 2 Nov 2020 01:32:11 +0000 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Wu, Hao A" , Jon Hunter , Jeff Brasen CC: "Luo, Heng" , "Ni, Ray" Subject: Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Retry device slot init on failure Thread-Topic: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Retry device slot init on failure Thread-Index: AQHWrU6qj9zXY/HJTkmuPLgnbg7ADamtw2NggAZRv+A= Date: Mon, 2 Nov 2020 01:32:11 +0000 Message-ID: References: <5a3b90de7f0c824e1a2c29ce06592b263e4c760e.1603905582.git.jbrasen@nvidia.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.218] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 874e6b52-3c6f-45c4-fb00-08d87ecf1f15 x-ms-traffictypediagnostic: BN6PR11MB3987: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:1227; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: NVFIHPQRlGgAeScoKPKS/G5tIM95TchYzXmEQkKcGai7j84lKrAYLwfEoreBQumHnmZXzLAbkb2taRcpUv6ttEVB0hbRXEAevyDTbakpg8ZQrxuoKay6+ipGTnB3WvkBC88CJXgCAZPw2Sz5Afhmx3N5gt946X36IrhcOtVD6q1v95wRjAhupBgEYnaBvUrIdxbBNt8Cc5lY2q/3eCTxqgxsV6n26kFRDvf23sQyuVh6EOffggxcLKHvbxaBVqmt7MtuWoql2PQvIDevlkT5gDzbQyunsnJE+xmzzwYfoTDy+g/egisG7IPKQQnZ/g2pbav2/VpKltj3FO6Uvnedjo7z/ejy9KQ6kNMzHb0oxfh8dUXsFet42DifREwSKCDyiTPafZLPlYdYDdvFHMlm9A== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BN8PR11MB3666.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(346002)(39860400002)(376002)(366004)(396003)(136003)(8936002)(54906003)(33656002)(8676002)(71200400001)(4326008)(110136005)(966005)(2906002)(186003)(478600001)(53546011)(86362001)(107886003)(9686003)(19627235002)(66556008)(66446008)(55016002)(66476007)(64756008)(66946007)(316002)(6506007)(52536014)(83380400001)(7696005)(26005)(5660300002)(76116006);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: UMn29ETXuZvQrJsI+fM4wsb1jjmhzzsMVxK4pjWp6ff9bm2T+U8idpDmI+J4wpk4wsN1i4PdeB2KBMJ+7tz5t/kOiIhTkhmJ/AXPGqqOfKZZzaU8+/5hSVNuFnQfU3jYpHlvOfVIiZrmi5HjQr4LsmqBvuTDWJF3Dv9qc3We4mpZPMXeQZEMqLX4LZvESwuT8XyjIIrE0T/Xgeu7Tz/ieOjY1DM8euAsbLFRAlNzm3+3nvzktTrvwowm5l3xx+7/2AxYdSFVuJ+73+Vj/uAM+LeH97Awl6RQl/WbYxOJk68m5GulTlA16QZ1Xuv+Z2Jt5EMdrwORtiIGnmrufX8JdH2ktOO5W+nLnp6AymtPfp1jsr9wcH1ItOuZIbJCCOHnLEbFLEphYj9XXYIGKEYTg/eXMenTCd+ZbgcV4VSkjDxewVBGZ6QJznL6gQthUGsKpbsd+7lpkgXeroX4ZMkdB3Phbq7gjZxa3C0L1+RABx8YZQe7hbaruo5Gxke5B0hBUmODr+EPHk/0vAh7DnWfde4K+wW/ATIAQRe1F1CiXqG36HgH8hPqB1oWvzmNx3IF9NW3CNij0tPx22pmf2ueIc0FoXfU60KztwkP8imEzg3sPOAo1kuYldeZINqW/bAlFNCAM3UjDdO0+nXMWP4Phw== MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN8PR11MB3666.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 874e6b52-3c6f-45c4-fb00-08d87ecf1f15 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Nov 2020 01:32:11.6540 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: o/p7c8KPwCUjqmFNCP4LC8rVv4bWY0oSd6dqvSTAXi4FdeEQdOJjfUrMlfKhuHgO+IDorI6K3pmHNr40VqDRFw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB3987 Return-Path: hao.a.wu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Patch was merged via: Pull request - https://github.com/tianocore/edk2/pull/1063 Commit - https://github.com/tianocore/edk2/commit/2363c6926098ee5c75c8780d= 07f88f5c21010683 Best Regards, Hao Wu > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Wu, Hao > A > Sent: Thursday, October 29, 2020 9:03 AM > To: Jon Hunter ; Jeff Brasen ; > devel@edk2.groups.io > Cc: Luo, Heng ; Ni, Ray > Subject: Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Retry > device slot init on failure >=20 > > -----Original Message----- > > From: Jeff Brasen > > Sent: Thursday, October 29, 2020 1:20 AM > > To: devel@edk2.groups.io > > Cc: Wu, Hao A ; Luo, Heng ; > > Ni, Ray ; Jon Hunter ; Jeff > > Brasen > > Subject: [PATCH v3 1/1] MdeModulePkg/XhciDxe: Retry device slot init > > on failure > > > > From: Jon Hunter > > > > With some super-speed USB mass storage devices it has been observed > > that a USB transaction error may occur when attempting the set the > > device address during enumeration. > > > > According the the xHCI specification (section 4.6.5) ... > > > > "A USB Transaction ErrorCompletion Code for an Address Device Command > > may be due to a Stall response from a device. Software should issue a > > Disable Slot Commandfor the Device Slot then an Enable Slot Command > > to recover from this error." > > > > To fix this, retry the device slot initialization if it fails due to a= device error. > > > > Change was verified using a superspeed mass storage device that was > > occasionally failing to enumerate in UEFI. With this change this > > failure to enumerate was resolved. This failure was also only seen in > > UEFI and not in the OS. > > > > Signed-off-by: Jon Hunter > > Signed-off-by: Jeff Brasen > > --- > > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 1 + > > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 34 +++++++++++++++++-- > -- > > --- > > 2 files changed, 25 insertions(+), 10 deletions(-) > > > > 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 > > > > // > > // Transfer types, used in URB to identify the transfer type diff > > --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > > index 00e9cc63d63e..dc36945962a0 100644 > > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > > @@ -1717,9 +1717,11 @@ XhcPollPortStatusChange ( > > EFI_STATUS Status; > > UINT8 Speed; > > UINT8 SlotId; > > + UINT8 Retries; > > USB_DEV_ROUTE RouteChart; > > > > Status =3D EFI_SUCCESS; > > + Retries =3D 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)) =3D=3D 0) { > > return EFI_SUCCESS; > > @@ -1761,17 +1763,29 @@ XhcPollPortStatusChange ( > > } else if ((PortState->PortStatus & USB_PORT_STAT_SUPER_SPEED) != =3D 0) > { > > Speed =3D EFI_USB_SPEED_SUPER; > > } > > - // > > - // Execute Enable_Slot cmd for attached device, initialize device= context > > and assign device address. > > - // > > - SlotId =3D XhcRouteStringToSlotId (Xhc, RouteChart); > > - if ((SlotId =3D=3D 0) && ((PortState->PortChangeStatus & > > USB_PORT_STAT_C_RESET) !=3D 0)) { > > - if (Xhc->HcCParams.Data.Csz =3D=3D 0) { > > - Status =3D XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Po= rt, > > RouteChart, Speed); > > - } else { > > - Status =3D XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, = Port, > > RouteChart, Speed); > > + > > + do { > > + // > > + // Execute Enable_Slot cmd for attached device, initialize > > + device context > > and assign device address. > > + // > > + SlotId =3D XhcRouteStringToSlotId (Xhc, RouteChart); > > + if ((SlotId =3D=3D 0) && ((PortState->PortChangeStatus & > > USB_PORT_STAT_C_RESET) !=3D 0)) { > > + if (Xhc->HcCParams.Data.Csz =3D=3D 0) { > > + Status =3D XhcInitializeDeviceSlot (Xhc, ParentRouteChart, > > + Port, > > RouteChart, Speed); > > + } else { > > + Status =3D 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 =3D=3D EFI_DEVICE_ERROR) && (Retries-- !=3D 0)); >=20 >=20 > Thanks Jon and Jeff, >=20 > Patch looks good to me: > Reviewed-by: Hao A Wu >=20 > I will wait a couple of days to see if there is any comment from others = before > merging the patch. >=20 > Best Regards, > Hao Wu >=20 >=20 > > } > > > > return Status; > > -- > > 2.25.1 >=20 >=20 >=20 >=20 >=20