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.web12.10896.1603160556101393019 for ; Mon, 19 Oct 2020 19:22:36 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=E+1wyppo; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: hao.a.wu@intel.com) IronPort-SDR: iJkf0z8JY1Q72wkrW/arApHZDrmOX1IM2KWSHI8fdfvGO5TWHo06nIWQXr7ArQpAtzV78qTjdm VVlxGs6cGr9w== X-IronPort-AV: E=McAfee;i="6000,8403,9779"; a="166365402" X-IronPort-AV: E=Sophos;i="5.77,395,1596524400"; d="scan'208";a="166365402" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Oct 2020 19:22:33 -0700 IronPort-SDR: SNPDmzLTpzmvK/7nNvGLxk8tcZoFczGkaM9ihAj+m9nz7zpKfUZ3DDgPsBz5ie3nEUIxZLYlw/ QvcffOYp6j2A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,395,1596524400"; d="scan'208";a="359026485" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orsmga007.jf.intel.com with ESMTP; 19 Oct 2020 19:22:32 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) 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; Mon, 19 Oct 2020 19:22:32 -0700 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 19 Oct 2020 19:22:31 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) 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 via Frontend Transport; Mon, 19 Oct 2020 19:22:31 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.174) 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; Mon, 19 Oct 2020 19:22:30 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ua26+zKs/vtFGUVYxI+PGokqyg2qryr5tcZVUZlpTQIr412l83RSZVextSELMVgeCcfBJHy1hUmHy9wg26wuRDdzcPbUJm2ZNJTOEe3mYP0J9XCTlhFbhVem0E+Em1rio1wwPiJ4AnKxuN4QUOShWaKKsn/OLeHKV0in2Mr8ZovPfArrF5MSBw1hA1xosb/jgV6oLxFWpqFJq8Jh2owifK/N9k3T6+myD3Cph34s2hnOsuYRIIzazQ+9ZUR4Oyw6HcukHzxhmnzEFUC0YbGK8YzKS5sj0dfOoxlKGpGbJzgycTOE+XNwnQN5wvymqmjp+SwTI1yCSUuZBfjS0mgWUA== 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=M4fEeqVa+3edwOjg/WFkvTIQ510wUeUzsofVj5yqQF0=; b=gMAlc/ORO+yVjDH8piNJgb6RZDQwJQK5Vd8+CsDl9LmUwrG6cBy+Ora/eoB3OSPBeYf8kWXoA4KeW5I0hd8ynK/FQD9m2IFW3Q7U5n+qEaLHHr2XgnC/lNkgReM0UXZ3vLC2KahaBaCHhECK/WZv3jMjFqPYm0exrt58gZJRtSmM3GEkbFX1PiBUOZTgiTYw3AAmj65pGZn3jv1Cb+pBadyFLabJwlaKqfbB4V14JSgsHgk03Qx/pwM0q85Ksup8tvzJR5t8iCSsal/I4qz85qBWKmX9xfOQm77z2lfbowkDu7dlJ8W6ahTsK8+rH/XH7Y8uqJt+c1O84iFyhjcMyg== 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=M4fEeqVa+3edwOjg/WFkvTIQ510wUeUzsofVj5yqQF0=; b=E+1wypporo35qWY+2OQDFP/YUys3Ci+WUD7zUcEkMxSCQYsVI4771oRgM6RktwFuLVnFTRntkyEX3LgqSE3QBXtwyYjh/34Bg5CAWmxgRcSU2HbYr8e31F4TeVX1nzk6T1yWze52XIp0XPdIyZQd6iZ6Rm4euo2GhGKUmoPN5QE= Received: from BN8PR11MB3666.namprd11.prod.outlook.com (2603:10b6:408:8c::19) by BN6PR11MB1475.namprd11.prod.outlook.com (2603:10b6:405:9::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.23; Tue, 20 Oct 2020 02:22:29 +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; Tue, 20 Oct 2020 02:22:29 +0000 From: "Wu, Hao A" To: "Luo, Heng" , "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: AQHWopWHnBe20E7vzEm142fMV0AReKmZzOzQgARtGyCAAGov0A== Date: Tue, 20 Oct 2020 02:22:29 +0000 Message-ID: References: <20201015014901.1154-1-heng.luo@intel.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: 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.218] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 8c1d9bd9-37fc-45f0-88ce-08d8749efe95 x-ms-traffictypediagnostic: BN6PR11MB1475: 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: B4I3IhnR3cYVqY5383OlAzsWLc4+x+1RyZly4ZASEMGOWYIk4PFp4TIlSMXBv8r6MdXDCv8h6HXfoqJv8uIegQ8vhfvQbdcBGs29AWiYQBW8Ynb1tSIuKQ1uS3KxKeX8cQPl+NEliJHn0L12G56amO/gfj+IRtykiOCgbwmdXtoZzQtKYP9bxljx/iq1BQuKi0YeMTKUw9YgKYEiLvqYtQohcYSINsK2lCSsBCgaxq1aDagklrTFzfY1nBU/OzOTML9+rjUfLryrxxVhCAA8p3u9/uXGOc+a54YUBBhMPmVRi5nd5H9eoKadm+oarPsNVkSJKJXC8/bv+fpcvHVwMnFGrNSSMvqkY+pnyBoZzt8oflxPyXuu6Cnc9chU+y7SJZwVFY/RDur2o8boVB1WKQ== 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)(136003)(366004)(39860400002)(396003)(376002)(346002)(2906002)(316002)(86362001)(8676002)(9686003)(6506007)(53546011)(33656002)(26005)(966005)(186003)(110136005)(19627235002)(107886003)(71200400001)(4326008)(66946007)(55016002)(7696005)(52536014)(76116006)(83380400001)(66556008)(66446008)(64756008)(66476007)(5660300002)(45080400002)(8936002)(478600001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: QXcKMl8yHY/T4pjGMtu0IcF/L/9+tbL343mwXClBQ/dPL0CE64qd7f51nD6Rtuca/lj3/rHCEy3oUiuQAwYRifYGFxX5+pSdozqB1kqu5+H1UtdiupBpBtxwxoB5oSMT9uWC+DbhmASus8ZLfo78R/nuFLZB+P67P8+VTCu12vBESHqrPgVGJU9nIIIgGZtaK1eKqrSxbRSeSdpcCvLdvb6YCBbsNpONV4Wj9m6nEVm4kThpDfLY83+X/ujktD2HuL0+WYcpTiJVpw34BKavyc7+V9+w6UWm+jP22JTHpgYfEuqkcrD54emcL5ZJ+D/uRmzXmlhIRI7Yy69NrXSR4Zz6e/GT/O2ixjw2/w4vNecKjJK7imadOPGYBrWWk8cXSD3kGbwuRfE+6tdy9XldZywZGzoRJ+uXk2MqC+7GUBTXFOD0lT6RIOfuEioQHey4fT8kepxJAjviKJptKCK54k5KLRDNHzcxaludA9P2mIpv2N6Txsj40Pe2r6Cdk+ZQFMcrT873tlBaLzyOe5Imr4gZ0rW339lL1J9dWvshOysCMOi6+JreRXhMoIYcCNqN+4VrlSgqBEXoCSCLwhd9IebUU/r07ntvnK+tOGCRfxznHWjDYRmyT7RLsbxqgvRLrgzjkhTKUvy3oyuZ3iDhSw== 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: 8c1d9bd9-37fc-45f0-88ce-08d8749efe95 X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Oct 2020 02:22:29.6523 (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: prQd6kLiVAhwNNpycZikXSNV8Vpw5Y2mE1krm4PCYx6yrKFrRw9jzykjklVfqfdGLptXPo1ntDZcuht4XyM8NA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB1475 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 Heng, Inline comments below: > -----Original Message----- > From: Luo, Heng > Sent: Monday, October 19, 2020 11:04 AM > To: Wu, Hao A ; devel@edk2.groups.io > Cc: Ni, Ray > Subject: RE: [PATCH] XhciDxe: Clean up UsbDevContext if USB slot > initialization is failed >=20 > Hi Hao, > Thank you for your review. > I think the slot Id is allocated by HW because the pointer EvtTrb point t= o > EvtRing->EventRingDequeue, EventRingDequeue is filled by XHCI > controller(correct me if I am wrong): >=20 > XhcDequeue =3D (UINT64)(LShiftU64((UINT64)High, 32) | Low); >=20 > PhyAddr =3D UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc- > >EventRing.EventRingDequeue, sizeof (TRB_TEMPLATE)); >=20 > if ((XhcDequeue & (~0x0F)) !=3D (PhyAddr & (~0x0F))) { > // > // Some 3rd party XHCI external cards don't support single 64-bytes w= idth > register access, > // So divide it to two 32-bytes width register access. > // > XhcWriteRuntimeReg (Xhc, XHC_ERDP_OFFSET, XHC_LOW_32BIT (PhyAddr) > | BIT3); > XhcWriteRuntimeReg (Xhc, XHC_ERDP_OFFSET + 4, XHC_HIGH_32BIT > (PhyAddr)); > } >=20 > So it should be better to use XhcDisableSlotCmd to disable slot but not > directly clean up UsbDevContext, I will submit new patch if you agree. I agree with the above proposal, please help to send a V2 patch. For the V2 patch, could you help to rename the title to: MdeModulePkg/XhciDxe: Error handle for USB slot initialization failure The package name is needed for title for easy reference. If you are able to test the PEI Xhci stack, could you help to check if a similar issue exists under: MdeModulePkg\Bus\Pci\XhciPei? If not, then only changing the XhciDxe will be fine. Also, could you help to provide the information on what tests have been don= e for the patch? Thanks in advance. > + } else { > + DEBUG ((DEBUG_INFO, " Address %d assigned unsuccessfully\n")); > + Status =3D XhcDisableSlotCmd (Xhc, SlotId); > } >=20 > Notice that even if a slot have been disable, but it is not reused. I hav= e 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, slo= t 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 >=20 > So I think we can reuse slot ID unless previous slot ID is 255. I think it is the controller's behavior to return which slot ID when a 'Enable Slot' command is received. The current proposal of using 'Disable S= lot' to inform the xHCI that a Device Slot is no longer needed looks good to me. Best Regards, Hao Wu >=20 > Thanks, > Heng >=20 > > -----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 > > > > > -----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 > > > > > > Thanks for the patch Heng. > > > > 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 > > > > 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. > > > > Best Regards, > > Hao Wu > > > > > > > > > > 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 {+ > DEBUG > > > ((DEBUG_INFO, " Address %d assigned unsuccessfully, clean up conte= xt > > > 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 {+ > DEBUG > > > ((DEBUG_INFO, " Address %d assigned unsuccessfully, clean up conte= xt > > > data.\n"));+ ZeroMem (&Xhc->UsbDevContext[SlotId], sizeof > > > (USB_DEV_CONTEXT)); }+ return Status; } -- > > > 2.24.0.windows.2