From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web08.6352.1603348834933422910 for ; Wed, 21 Oct 2020 23:40:35 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=kIYQ5IOv; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: hao.a.wu@intel.com) IronPort-SDR: 8KsFaZ0vxMQu42zm/RCzG6j9+oyXFVIQRE+uqAskwBPT96Jb7NRb0TzVWpKq6rIJz74B5XCuiX CK3qtPgasBUQ== X-IronPort-AV: E=McAfee;i="6000,8403,9781"; a="167579508" X-IronPort-AV: E=Sophos;i="5.77,403,1596524400"; d="scan'208";a="167579508" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Oct 2020 23:40:34 -0700 IronPort-SDR: IRI2Cd8hxH8WajLCnldl6uYy4g2n3PQWBpgpYYxN9SYSoDd+EV+ajILwfTeDsjVoApEX+ENyof 03WjeVW9/luA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,403,1596524400"; d="scan'208";a="316620903" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by orsmga003.jf.intel.com with ESMTP; 21 Oct 2020 23:40:34 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 21 Oct 2020 23:40:31 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 21 Oct 2020 23:40:29 -0700 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.100) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Wed, 21 Oct 2020 23:40:18 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Pz0vwwurNpqwYyukMTlH1B4cJzdLsX4LhTAyZOxr3UVYHuybz8aZLnY96Qdaua8YVL8a6BBs+WYVwFLRHp+CVUtCF7OP7nf0vEu+gAEGprUXKZGC3Y/cAJTHqxC9xTdj1w4nMyNbcsv7yNrCvEZCgSFBAeOcV3TrKGz2X6tj46+LZHfOroMViwT+EdvX4Y//SLHecZf+EeRQiAk/joWWtCINaHaE5Ic1tqMvPkbT5PlfoBZmQO7ONTLZRMgKBDfGseOcUqkVXJPmqASBswDxTsotB7fBkOFeOInNiUGM45PPNXjSi9XhPmQ/JRt+hFAN2nm60jtZOG0UDKXXbc+LMw== 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=iErp9GWDf5/EwNdldR7ZdQ0eX47VAp9pWxOnLlSM6hw=; b=dMYNG4k24OVsZpsGDPRx5Bpw7ctGGxvC2Rw1+KM1Obe1KVjoQpQg12Au/dsQo41Q+yRFsLPPVNWkC5EgDH/HjnFwvnY6Aw+thZMSOKWz6wIsoHB6dfkSiDtiIkYOPn3ZOfcC/N5f/vBVWoqPOR+v8vUu87ApVA3SgYXCLWs/y7Dg1CYoiOqG0r2z2uRKgoHQdbo02FWjuZMSwpkEx8mI+UhcNXgdO1/aMGK3CSeTWUNLZPsVFfj1kX1o+Yrqj54w1YV6tq7gEwpIms12uYXdwlQ6RFKzTjCyHt6ozA2u20hakO7aqvfWiAL4CQ0IRrz7KKU687QvVOCkylQKGWxg2Q== 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=iErp9GWDf5/EwNdldR7ZdQ0eX47VAp9pWxOnLlSM6hw=; b=kIYQ5IOv73/b6FXy6Bfl9CYckD9KTty1oTsyuinyfqP3UZUUOMLkNxV/QjXBJwmWgp5iMu0Loq8MCGHtHXF44trceS2fMYqA+CgETUIksxXUOynA9dJ5SPpiGVVIfMAtm8j24de4lDhdXL6zxGbYpwKpsNBIRwsS9AaB5XGnmc8= Received: from BN8PR11MB3666.namprd11.prod.outlook.com (2603:10b6:408:8c::19) by BN8PR11MB3620.namprd11.prod.outlook.com (2603:10b6:408:91::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.18; Thu, 22 Oct 2020 06:40:17 +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; Thu, 22 Oct 2020 06:40:17 +0000 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "jbrasen@nvidia.com" CC: "Ni, Ray" , Jon Hunter , "Luo, Heng" Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Retry device slot init on failure Thread-Topic: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Retry device slot init on failure Thread-Index: AQHWp8EllGnyZlgxiEmc4jnJaMZE56mjEeYA Date: Thu, 22 Oct 2020 06:40:17 +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: 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: 5d8424b2-9060-4207-21b7-08d8765556d1 x-ms-traffictypediagnostic: BN8PR11MB3620: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: hTtXHTuX+0iPcrIfB1WtDWrBWsWhfBCQcpXGPgzXzpce9NsxcSNmFIkQbgxuEbWaky9cc/g1FBnDvOU9BbzebLpU8eO6a/8ivA2M5c77rAXzhEqu3PsC2GlRmY/qSRIiydF98YP9AopqKNn5IBbOIzb1f8VHS+Sx3iYsmxZnd/vLUseXxpxZO6CRASgEmDbnEa+tZZARqHYMJSh6B6izzOe/Znx2755XT4Nqzn1wwO9opC+7thyqrCs2+Qt8HIhf8EWqhgN7fpGLVZBNuvlytCKZfNmRYwEKzVZTqgeVD/9CjieLwJMuwTN7gQl2Ggm7h3RrWNHByOWNorjI2FEdCQEs41PeY9UdcoJpginK4+I7ZPAj5OYqBPJ3JPMUPBnQPpkyZQfqlnRNL17xwYqJNw== 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)(396003)(136003)(376002)(39860400002)(366004)(346002)(66946007)(64756008)(66476007)(66556008)(7696005)(5660300002)(316002)(66446008)(110136005)(76116006)(966005)(478600001)(54906003)(8936002)(83380400001)(19627235002)(33656002)(107886003)(6506007)(9686003)(52536014)(53546011)(8676002)(55016002)(2906002)(4326008)(26005)(186003)(86362001)(71200400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: oi6aPNu/ARdwFMTz8jshlswwExeJWCCsVaKwyWkKVZWPQP4uX3Yn1RbGAz9/dET1mbdUpa0/5DONehF4K0PDQgz98ZVIgcHam/RpmnamlG5APqSCUAuwmCxE3umbyKafe/8IMcUut9ao3m2hgO9lN/XNMc5fgjYp5aytnye2v4B0BWn1pvhUXGVF4I5Q04K6cxTsYnV8fC6K2il2zJr9MDIns4VOTr4e7M0JsOgY/LXeUi9VGoUL8UslnLpttI5L8fdZ2ax7tssTTwS3kIkGrWJluxiiS3Fr6TWMoNC6tZ5fLJDfOTBOCYuuKIfZIWzL4L8P1NcDYpuEey6EkCUmWqTb74ufFepIfCTa9OYt+myekMO6Gy+XOiwJ3lznwTSVj614+HZzI7r/4iV2Kvu7ShcyBibo/lJD2fj8m7jeXzP9Gi08jUECkC0Tmnpkx5PMIh/4e8NfA6exY1Jgqb2mZlkymGmMkyPdey1Ok9ALt1ldyjCTpJQNKE2zfuFCqGcfeauSxNLlHSdMOgSuSiyHFixugz7thryxrre8jnUpAzDkvnm35JPuZE4GGe/88KywDW+y4Pr/gTBkSMTMaTwembAqzj2j7WnbJk1MmCxzqCq2J2bWmT0ybZ1PIu75fMIgy7qpgl8+Fl4zscsZxVzg7Q== 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: 5d8424b2-9060-4207-21b7-08d8765556d1 X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Oct 2020 06:40:17.2539 (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: OGLY3WaMcbu5M2WbL8kklPQyyQ8bDMHogIk3FmjVcquOlTgLMS7BoKfK/WARS/J45UJSwd7dn0x6WtbNVZ6o6Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR11MB3620 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 > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Jeff > Brasen > Sent: Wednesday, October 21, 2020 11:45 PM > To: devel@edk2.groups.io > Cc: Wu, Hao A ; Ni, Ray ; Jon > Hunter ; Jeff Brasen > Subject: [edk2-devel] [PATCH] 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 d= evice error. Hello Jeff, Thanks a lot for the patch. I found that there is another patch current on the mailing list that also addresses an issue during device slot initialization: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3007 For the issue mentioned in BZ-3007, it will handle the case that: a) The device slot initialization fails; b) A call to XhcDisableSlotCmd(64) is needed. Otherwise, the initializatio= n of the next port will fail due to the content in 'Xhc->UsbDevContext' is n= ot cleared. The current resolution for BZ-3007 is to add XhcDisableSlotCmd(64) calls w= hen an error occurs in XhcInitializeDeviceSlot(64). Could you help to see if the patch provided at: https://edk2.groups.io/g/devel/message/66529=20 has any impact on your proposed patch? I think the XhcDisableSlotCmd(64) calls can be moved out of the 'do-while' loop in your proposed change. Thanks in advance. Also, please refer below for 1 inline comment: >=20 > Signed-off-by: Jeff Brasen > --- > MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 71 ++++++++++++++-------- > -- > 1 file changed, 42 insertions(+), 29 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c > index 9cb115363c..1a16864205 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 1; Could you help to introduce a macro in XhciSched.h: #define XHC_INIT_DEVICE_SLOT_RETRIES 1 and use the macro here? Best Regards, Hao Wu >=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; > @@ -1739,40 +1741,51 @@ XhcPollPortStatusChange ( > RouteChart.Route.TierNum =3D ParentRouteChart.Route.TierNum += 1; > } >=20 > - SlotId =3D XhcRouteStringToSlotId (Xhc, RouteChart); > - if (SlotId !=3D 0) { > - if (Xhc->HcCParams.Data.Csz =3D=3D 0) { > - Status =3D XhcDisableSlotCmd (Xhc, SlotId); > - } else { > - Status =3D XhcDisableSlotCmd64 (Xhc, SlotId); > - } > - } > - > - if (((PortState->PortStatus & USB_PORT_STAT_ENABLE) !=3D 0) && > - ((PortState->PortStatus & USB_PORT_STAT_CONNECTION) !=3D 0)) { > - // > - // Has a device attached, Identify device speed after port is enabl= ed. > - // > - Speed =3D EFI_USB_SPEED_FULL; > - if ((PortState->PortStatus & USB_PORT_STAT_LOW_SPEED) !=3D 0) { > - Speed =3D EFI_USB_SPEED_LOW; > - } else if ((PortState->PortStatus & USB_PORT_STAT_HIGH_SPEED) !=3D = 0) { > - Speed =3D EFI_USB_SPEED_HIGH; > - } 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 c= ontext > and assign device address. > - // > + do { > SlotId =3D XhcRouteStringToSlotId (Xhc, RouteChart); > - if ((SlotId =3D=3D 0) && ((PortState->PortChangeStatus & > USB_PORT_STAT_C_RESET) !=3D 0)) { > + if (SlotId !=3D 0) { > if (Xhc->HcCParams.Data.Csz =3D=3D 0) { > - Status =3D XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Port= , > RouteChart, Speed); > + Status =3D XhcDisableSlotCmd (Xhc, SlotId); > } else { > - Status =3D XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, Po= rt, > RouteChart, Speed); > + Status =3D XhcDisableSlotCmd64 (Xhc, SlotId); > } > } > - } > + > + if (((PortState->PortStatus & USB_PORT_STAT_ENABLE) !=3D 0) && > + ((PortState->PortStatus & USB_PORT_STAT_CONNECTION) !=3D 0)) { > + // > + // Has a device attached, Identify device speed after port is ena= bled. > + // > + Speed =3D EFI_USB_SPEED_FULL; > + if ((PortState->PortStatus & USB_PORT_STAT_LOW_SPEED) !=3D 0) { > + Speed =3D EFI_USB_SPEED_LOW; > + } else if ((PortState->PortStatus & USB_PORT_STAT_HIGH_SPEED) != =3D 0) { > + Speed =3D EFI_USB_SPEED_HIGH; > + } 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); > + } > + } > + } > + > + // > + // According to the xHCI specification (section 4.6.5), "a USB Tran= saction > + // Error Completion Code for an Address Device Command may be due t= o > a Stall > + // response from a device. Software should issue a Disable Slot Com= mand > 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--)); >=20 > return Status; > } > -- > 2.25.1 >=20 >=20 >=20 >=20 >=20