From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web10.6367.1603076637330467562 for ; Sun, 18 Oct 2020 20:03:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=SKYxTo6F; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: heng.luo@intel.com) IronPort-SDR: zd//UTmAbWIO3d8dgnlopfwrxLJAIx0HQU4ivmlN7T/kJG15ZR+dae5rgS34fiB2WWAf5M7Fs3 4YA1X4psrdjg== X-IronPort-AV: E=McAfee;i="6000,8403,9778"; a="163470471" X-IronPort-AV: E=Sophos;i="5.77,393,1596524400"; d="scan'208";a="163470471" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Oct 2020 20:03:56 -0700 IronPort-SDR: 6ve75IDieUY7nOlaZlI66fyJRNHpJoGo0GgICRcvZ04LhM/v7qn0x0OpN7UvKrqOABCjTiAbG/ eFfnRA3/v+uQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,393,1596524400"; d="scan'208";a="522953484" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by fmsmga005.fm.intel.com with ESMTP; 18 Oct 2020 20:03:55 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 18 Oct 2020 20:03:55 -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; Sun, 18 Oct 2020 20:03:55 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.46) 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; Sun, 18 Oct 2020 20:03:54 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KfWmT1eQG+diS327Gq3CR8hHFmLJQNBcAEpb7HLefWq+iunDij2GmeeHL8PQSMp1q6x3FKQ1euXPyA1AkSZ/tY6gaOKS0ShYreCVCH0jKxD5HEUDSp6Mm7FH9cUEkXgWM2gKP3AZ4+CSqLzyGH5nZ5/0Zwbio50/RUfu2Dk2z3WMAaedfPMSnK/V3wUViP3HDg6UXcrq9fNkN4FVsPX8hMOnD/dAQG3tGwpXUgaKxF6SY4kRdhiHyBNhriKHqLa5qb/HEkybnoL1NNpnkvKpyHpFV+ad5cWI4nEOt0dJc3KPjhzeL7iFXiPo9jAzKIh+kxk99i7S1u5nnBYx4//PjA== 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=pK3ZtDuUw3R6j/4CzqRJfecCWP6UCmcxFHxTpF/obZA=; b=gzj1LS8Kc+d7xsIKsiTAy0FiEafjGjtdFDKEG/xabKo7DAUkRrdKXuzD6iZHUBoYOmHUkZVdwGSdMf+D80gvhPvGXGVrJ6278en0NEledSWVkTrL5+N3EFgu+4eRszkbVyGm8vmFzSVmXOUZfM1yFmAt/AhotsNyIklATD1R/0VGaeXEC6dBz4ru/rvZ5o0SM7dVPLzqdfx5+2r9EOYRdbm9CMTl7eISwUbJAjtXJg2dbu1IfOKG9CuWn0kaX11SaKCVr9sLIzynR2JmSeNKqhZSFKE6s/lCcKEbzIxQNpNCPdtlveZHKSZVIm48JZKB/Da0Tu4lUq9NCtvT70hVXA== 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=pK3ZtDuUw3R6j/4CzqRJfecCWP6UCmcxFHxTpF/obZA=; b=SKYxTo6F6SbXmWR6I+xoixdHILZ5mwhUngz4Ab5nreKSwcGIMogjRO07j6VPWPfgvThZ0K36KKC7fpbz7QolUhgfL0dd+zMGWmOPjsV/jIfTRXPumcoFUZpWiCTPJWMq4n8yXoz3M4J46pwJ8U9+dKCsfIPaP3AJrBhFyc/sm7I= Received: from MWHPR11MB1805.namprd11.prod.outlook.com (2603:10b6:300:114::14) by CO1PR11MB5123.namprd11.prod.outlook.com (2603:10b6:303:94::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.25; Mon, 19 Oct 2020 03:03:53 +0000 Received: from MWHPR11MB1805.namprd11.prod.outlook.com ([fe80::bd78:5a79:d875:65e4]) by MWHPR11MB1805.namprd11.prod.outlook.com ([fe80::bd78:5a79:d875:65e4%12]) with mapi id 15.20.3477.028; Mon, 19 Oct 2020 03:03:53 +0000 From: "Heng Luo" To: "Wu, Hao A" , "devel@edk2.groups.io" CC: "Ni, Ray" Subject: Re: [PATCH] XhciDxe: Clean up UsbDevContext if USB slot initialization is failed Thread-Topic: [PATCH] XhciDxe: Clean up UsbDevContext if USB slot initialization is failed Thread-Index: AQHWopWHnBe20E7vzEm142fMV0AReKmZzOzQgARtGyA= Date: Mon, 19 Oct 2020 03:03:52 +0000 Message-ID: References: <20201015014901.1154-1-heng.luo@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.198] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c5a5507f-a218-4c68-184c-08d873db9c72 x-ms-traffictypediagnostic: CO1PR11MB5123: 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: 1qNwxxnspNYc34+iWNzq9LlEKYhWCf1ITi2xymXhybbX+LxT6WifViaJvdcWxEsEEjBCyY6KJqebij7IisCE2N+QKnVa8rM50ngerGrNiEmbQlkWUGERvg4ByAHCCWuC/AoXf+p6GilcPxzrAYi5xNfQd385rpkk1qAtlDOBxuuxevhMxNrO1EGvkAzvC98KWK54K3eGNb/N+FRowJJ39vYez9vHU2CN+/qUN4nrOAq3X4HK4ni+W/49YQEQymTKQxDXkmETFScbaryRNpCOo1yzuCKaTjNkQxjTir6tPPkZbysHlXBwx0BXJONPaCIa3TtFhxFluqhjhViM8Zusgk7th8y1eKmpdfsXcsRn/UYvD7cFJItOrxtc2eBlij5PWq+onNPt7ToFekk+pWncAA== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MWHPR11MB1805.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(346002)(396003)(39860400002)(376002)(136003)(26005)(86362001)(478600001)(33656002)(5660300002)(76116006)(66946007)(107886003)(6506007)(53546011)(66476007)(66556008)(66446008)(966005)(52536014)(7696005)(64756008)(55016002)(2906002)(83380400001)(110136005)(316002)(8676002)(71200400001)(8936002)(186003)(9686003)(45080400002)(19627235002)(4326008);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: aJv4tbjJIYP7fOfwaisP5lzG/fsVNk6HdPRrn0LzMqBl8XsNEIpSbK6r+48ImI7pwzEEjNAyt1a/ab5R1g65ap+lo20LVVWcdHthGM/Xbl/3XhCdR6QnDR2KKgjJChy9uXhkMSmAaW1FSBdai0oSfeeRL7gkvc0bMYyDc+SIMWKdzxgtfucqhuRQZz/TLk/dmJLD/RZ5t3KDHb5T58Rrf2mvDLa2iQ1PV69xWN0Khx/BU+zJPwWTk1qQs0HfTSTy8STUjcdV3eX6BXNfB+hzTTJOI81QHXJ+cr7iq8+AU77RlVolWiVKRTqaybQokPaKHAYVXxGMWoCAjGZWPI49+JqiB7NqmjiF/QwyZXDfKWIgMbvqlmXi223K7hvet+kvc1fA/dJnBQwHIUBhKFukKhodRM7JjJUJFF7m366qAdr21+MmiF+adzQqJrUwmTfh272L4ySUZhGkFpr1Y5qH1PJt2DouoYTVRvev9ZnrH9qn80BHVo66TourJMBdtWoTbrJWfoOgzyLv9gj2yCS6gmhzpx3tR3jdEFJjFmldTOXj8Fu2pniWeTYtK7OUAq8mmakmA8G4pUv21266s4Sa7tP3XwZQSHJnpgW0vy3wUP1lsqlBA6PBGdiFbV7WRXLVElFP+h9uC4KJ/Pyv69NJaw== MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1805.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c5a5507f-a218-4c68-184c-08d873db9c72 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Oct 2020 03:03:52.8862 (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: 28sTrSPzm3seVi25/0dA8937EXHaYifj1dc7EIHZC66kIYonvOVyyYEpmn2X4CPnyCq5s6mHNOwB9XOJ1d7q7w== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5123 Return-Path: heng.luo@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Hao, Thank you for your review. I think the slot Id is allocated by HW because the pointer EvtTrb point to = EvtRing->EventRingDequeue, EventRingDequeue is filled by XHCI controller(co= rrect me if I am wrong): XhcDequeue =3D (UINT64)(LShiftU64((UINT64)High, 32) | Low); PhyAddr =3D UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc->EventRing.Even= tRingDequeue, sizeof (TRB_TEMPLATE)); if ((XhcDequeue & (~0x0F)) !=3D (PhyAddr & (~0x0F))) { // // Some 3rd party XHCI external cards don't support single 64-bytes wid= th register access, // So divide it to two 32-bytes width register access. // XhcWriteRuntimeReg (Xhc, XHC_ERDP_OFFSET, XHC_LOW_32BIT (PhyAddr) | BIT= 3); XhcWriteRuntimeReg (Xhc, XHC_ERDP_OFFSET + 4, XHC_HIGH_32BIT (PhyAddr))= ; } So it should be better to use XhcDisableSlotCmd to disable slot but not dir= ectly clean up UsbDevContext, I will submit new patch if you agree. + } else { + DEBUG ((DEBUG_INFO, " Address %d assigned unsuccessfully\n")); + Status =3D XhcDisableSlotCmd (Xhc, SlotId); } Notice that even if a slot have been disable, but it is not reused. I have = a try: 1. the USB keyboard is port 0, slot 1: UsbEnumeratePort: new device connected at port 0 ... Enable Slot Successfully, The Slot ID =3D 0x1 2. remove USB keyboard, slot 1 is disabled: Disable device slot 1! ... UsbEnumeratePort: device disconnected event on port 0 3. plug in USB keyboard in port 0 again, but slot ID is 4 now. UsbEnumeratePort: new device connected at port 0 Enable Slot Successfully, The Slot ID =3D 0x4 So I think we can reuse slot ID unless previous slot ID is 255.=20 Thanks, Heng > -----Original Message----- > From: Wu, Hao A > Sent: Friday, October 16, 2020 3:05 PM > To: Luo, Heng ; devel@edk2.groups.io > Cc: Ni, Ray > Subject: RE: [PATCH] XhciDxe: Clean up UsbDevContext if USB slot > initialization is failed >=20 > > -----Original Message----- > > From: Luo, Heng > > Sent: Thursday, October 15, 2020 9:49 AM > > To: devel@edk2.groups.io > > Cc: Ni, Ray ; Wu, Hao A > > Subject: [PATCH] XhciDxe: Clean up UsbDevContext if USB slot > > initialization is failed > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3007 >=20 >=20 > Thanks for the patch Heng. >=20 > After looking into the analysis at > https://bugzilla.tianocore.org/show_bug.cgi?id=3D3007#c1: > |> Enable Slot Successfully, The Slot ID =3D 0x3 > |> Address 3 assigned successfully > |> UsbEnumerateNewDev: hub port 15 is reset > |> UsbEnumerateNewDev: device is of 3 speed > |> UsbEnumerateNewDev: device uses translator (0, 0) > |> XhcControlTransfer: SlotId =3D=3D 2 DeviceAddress=3D0 >=20 > The wrong 'SlotId' is used for the control transfer command, where SlotId= 2 is > for the device failed to be initialized. > Heng, could you help to check whether it is possible for the next device = to > use SlotId 2 again? Thanks in advance. >=20 > Best Regards, > Hao Wu >=20 >=20 > > > > 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 clean up UsbDevContext if USB slot initialization is failed. > > > > Cc: Ray Ni > > Cc: Hao A Wu > > Signed-off-by: Heng Luo > > --- > > 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..1e8430ac34 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.
+Copyright (c) 2011 - 2020, > > Intel Corporation. All rights reserved.
Copyright (c) Microsoft > > Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent @@ > > -2279,6 > > +2279,9 @@ XhcInitializeDeviceSlot ( > > DeviceAddress =3D (UINT8) ((DEVICE_CONTEXT *) OutputContext)- > > >Slot.DeviceAddress; DEBUG ((EFI_D_INFO, " Address %d assigned > > successfully\n", DeviceAddress)); Xhc- > > >UsbDevContext[SlotId].XhciDevAddr =3D DeviceAddress;+ } else {+ DE= BUG > > ((DEBUG_INFO, " Address %d assigned unsuccessfully, clean up context > > data.\n"));+ ZeroMem (&Xhc->UsbDevContext[SlotId], sizeof > > (USB_DEV_CONTEXT)); } return Status;@@ -2489,7 +2492,11 @@ > > XhcInitializeDeviceSlot64 ( > > DeviceAddress =3D (UINT8) ((DEVICE_CONTEXT_64 *) OutputContext)- > > >Slot.DeviceAddress; DEBUG ((EFI_D_INFO, " Address %d assigned > > successfully\n", DeviceAddress)); Xhc- > > >UsbDevContext[SlotId].XhciDevAddr =3D DeviceAddress;+ } else {+ D= EBUG > > ((DEBUG_INFO, " Address %d assigned unsuccessfully, clean up context > > data.\n"));+ ZeroMem (&Xhc->UsbDevContext[SlotId], sizeof > > (USB_DEV_CONTEXT)); }+ return Status; } -- > > 2.24.0.windows.2