From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web08.2149.1603848081556312655 for ; Tue, 27 Oct 2020 18:21:21 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=ULC3T78L; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: hao.a.wu@intel.com) IronPort-SDR: UZvuSZ5YBDdtpMax/pyyVmbmuU1ipkSbyUQvRXuk7Kzy1XOHLSviHpO6NkUKV936puSmTKByep 43uVbdsZt4/A== X-IronPort-AV: E=McAfee;i="6000,8403,9787"; a="167408518" X-IronPort-AV: E=Sophos;i="5.77,424,1596524400"; d="scan'208";a="167408518" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Oct 2020 18:21:16 -0700 IronPort-SDR: qGKlU6lK9qpBmxrmxeA/aoGMKo8ZJxwxEoWBgbVZDcK1JV9ay2HaagPXJxjSl+8iz23zjy6JpG PUhUwlBsY5rQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,424,1596524400"; d="scan'208";a="304081284" Received: from orsmsx605.amr.corp.intel.com ([10.22.229.18]) by fmsmga007.fm.intel.com with ESMTP; 27 Oct 2020 18:21:16 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX605.amr.corp.intel.com (10.22.229.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 27 Oct 2020 18:21:15 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) 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 via Frontend Transport; Tue, 27 Oct 2020 18:21:15 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.171) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Tue, 27 Oct 2020 18:21:15 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bY+eKI61ED5tmIYLXx3C8FWS21jyoFDY/gwK/vRGWllgLfq7vhUjdKSa4PW1SBjb5L+gn/7ou5gisVJGBpK5zqXxopZEK600tcgpVAOJuW2sSYxJ9pNwkDbVTVPLFAuk0fBilSc4jXk2B1TJIzYz0IIlshZ9eyTZ+Hx/0u8yAYecq9V9BcXvV5s/GDl866MmCMxluOS5YcZNgiRdtv3R+0wN4dX5sM8S5r0IoLOorpADOSs3SbZPVTTHjzAcYbUJNOwtsWBeGTL3bjItSSvsW5akvYBGisv9fXaGMACLKMY9UTxkP8UrOfsOZPd28Ep+bywX2Os+fqXgZhFmrdpDeg== 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=1GZuWuf7zEIvf7qsUUWHOyPP+TsNC58Pt5fBcs3pFj4=; b=eKl8AUflJ1jjMvDJvclMo10PDLRg34xhSpX0kzw+QrdVBv/DNsi7M4sXdGZdrEIkWlUh0vx8nZVFj2i1O62C3n+NFToexZDiVFgnshhbPR+moRIwhN+UABJUAlgAAHGeATWtIwlJrFz+wM7MqjZFoz/PddDq+ULjPmh1Z3BV0xOG1zMY2xiFCjVepJwSU/0UikkeXWeYt6CNqBzD26nDug7HiLAQEczDRgpsN5Mv0EuJOvsdXdFWw/PXy+63dzd01lywAxK47M2nVBnYCRonxYsEVQSrzcDF0kRuuyzpaS8TBPskCEBeaCqINKa2rEFw5pD76j/9nnmsbwF5Mt3jMw== 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=1GZuWuf7zEIvf7qsUUWHOyPP+TsNC58Pt5fBcs3pFj4=; b=ULC3T78LSEpJ4buQGAC/vcn5vRZmEo5SZPiQTTcCU+FPRL4yZ53rJEnj09EFnfUHb9h2P54WRgFOKNANRMquyPrHhR/Tfk+7Ec04UVTGarvQdLetxVj0JLABQZS8nF1stK+sEs5sUOntC6nP2InhlMLTUUgnqc6k0ShyCX9YUyQ= Received: from BN8PR11MB3666.namprd11.prod.outlook.com (2603:10b6:408:8c::19) by BN6PR11MB3905.namprd11.prod.outlook.com (2603:10b6:405:82::36) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.18; Wed, 28 Oct 2020 01:21:13 +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.3477.028; Wed, 28 Oct 2020 01:21:13 +0000 From: "Wu, Hao A" To: Jeff Brasen , "devel@edk2.groups.io" CC: "Luo, Heng" , "Ni, Ray" , Jon Hunter Subject: Re: [PATCH v2 ] MdeModulePkg/XhciDxe: Retry device slot init on failure Thread-Topic: [PATCH v2 ] MdeModulePkg/XhciDxe: Retry device slot init on failure Thread-Index: AQHWrHpkxuRa/4zyZkKDkTde7rBtEqmsN3Iw Date: Wed, 28 Oct 2020 01:21:12 +0000 Message-ID: References: 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: nvidia.com; dkim=none (message not signed) header.d=none;nvidia.com; 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: a4211640-1793-49a5-6f25-08d87adfc284 x-ms-traffictypediagnostic: BN6PR11MB3905: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6790; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: zsY6V4DJLz9g9328QB5bd0IBHrzWuOp2nZ19flLhcdq6RM1CQpkPFeSSwCE1+sZaMtmWy0Xxb4csBWJJYmv3uBb4XsQteAEGPZzO5ChCswc6IxrN0SZ4dizjasov8nYm++TfjSOXUFTifogMXJcmgfkxPN6oLGA5EGHRfwpJQ/xOQxT1HYWK8+O/BU6YjTUCER85srWEKqU0RTNztWQvi3zJH6JdHhHA6yZkA8rhe713rdjAAdezMi84oFVl1DtjpkaR4KSVlFPyHQuZjvhpWvZ48dCtVa1FHGAvEJqqWh7QIBHnC7nsx/AZqQ9NPTWAU+BBt9WuaiH9IO8UodaKCA== 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)(376002)(396003)(346002)(136003)(39860400002)(366004)(53546011)(110136005)(8936002)(66946007)(86362001)(26005)(66446008)(478600001)(54906003)(186003)(8676002)(316002)(4326008)(9686003)(7696005)(76116006)(2906002)(64756008)(66476007)(71200400001)(55016002)(66556008)(19627235002)(33656002)(83380400001)(52536014)(6506007)(5660300002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: yoOl96BJ8Qk/ULD3k7vrBitX7DR1a5PYRaiv9qC83r8Eg3kwGJD1xQwZaxt+MtG/FJfm982p8/nwnkh4KN6mn940UttkFPr2sQwD8oM2lTdCDTv0ZjoVPkjvhFc0XcJK/iI8A4kiY4kC8sXy1I/1kq/JN3ILMP+3Uz4tb0D8nfh+Ksmx9nj2cyvjyzXbRIsHL6uvGoZ0Ru+DC/kZQ/9U1eTK5ywtsMMkb7FpJKhqMujtkPhVfhZ6yOEMTL4TK9cV7/fbqA5fpo34AQzp/OPhO49BFQZZjRH95LhKBU7YumIosq/Jyq2qsqLs80CzR9qV20Eesr8N4ijEDhYp9QnYnyfa0S6MmIUoZcmFzGM2Wv/LAb3zgZCzwx7frZ1YLU/LS+eqTKNP/Eeun8CJ8aTXal+PkTfRpu55UiZMb0OYySpjGEZMC8rHRwoDGWLnASyBUnClKF2B4VvDBbJ+hAfs32JmoKgVFF+f0nU33/HGQ3tqVApLUqgpMS69mkkFMiS1UFXig8WhTMulr2NE9pSPD/K4ncg1B/iZewkYq7OAO/mF0lBKjYPl1fbWjvAAu0AAyHSikZFVp5gC6v0VrBndY0AIjuRfBA8FgDx9HzEwN6qkqHbWRkrFxkIuSaRPaZG+mlLKI9aodzLor1SgZi4HXw== 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: a4211640-1793-49a5-6f25-08d87adfc284 X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Oct 2020 01:21:12.8422 (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: gDmCB8lcir0t8eBnzIdRV0yf4jy57I59xF/oh+N8P/TznvrXIzeRZlVmvDrKAsYQuGjkFyqeiqrvJW+yUzSa9Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB3905 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 Thanks for the patch, inline comments below: > -----Original Message----- > From: Jeff Brasen > Sent: Wednesday, October 28, 2020 12:00 AM > To: devel@edk2.groups.io > Cc: Wu, Hao A ; Luo, Heng ; Ni, > Ray ; Jon Hunter ; Jeff Brasen > > Subject: [PATCH v2 ] MdeModulePkg/XhciDxe: Retry device slot init on > failure >=20 > From: Jon Hunter >=20 > 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. >=20 > According the the xHCI specification (section 4.6.5) ... >=20 > "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." >=20 > To fix this, retry the device slot initialization if it fails due to a de= vice error. >=20 > 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(-) >=20 > 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_ >=20 > #define XHC_URB_SIG SIGNATURE_32 ('U', 'S', 'B', 'R') > +#define XHC_INIT_DEVICE_SLOT_RETRIES 1 >=20 > // > // 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..77664940a791 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; >=20 > Status =3D EFI_SUCCESS; > + Retries =3D XHC_INIT_DEVICE_SLOT_RETRIES; >=20 > 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 co= ntext > 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, Por= t, > 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, Por= t, > RouteChart, Speed); > + } else { > + Status =3D XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, P= ort, > RouteChart, Speed); > + } > } > - } > + > + // > + // According to the xHCI specification (section 4.6.5), "a USB Tra= nsaction > + // 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 thi= s > error." > + // Therefore, retry the device slot initialization if it fails due= to a > + // device error. > + // > + } while ((Status =3D=3D EFI_DEVICE_ERROR) && (Retries--)); Please help to refine the above line to=20 } while ((Status =3D=3D EFI_DEVICE_ERROR) && (Retries-- !=3D 0)); to align with the edk2 coding style. Really sorry for not catching this on = V1 patch. Also, could you help to add the information on what kind of tests are done = for the patch? Best Regards, Hao Wu > } >=20 > return Status; > -- > 2.25.1