From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f178.google.com (mail-oi1-f178.google.com [209.85.167.178]) by mx.groups.io with SMTP id smtpd.web11.351.1687995570819333917 for ; Wed, 28 Jun 2023 16:39:30 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20221208 header.b=KjZD0RID; spf=pass (domain: gmail.com, ip: 209.85.167.178, mailfrom: mike.maslenkin@gmail.com) Received: by mail-oi1-f178.google.com with SMTP id 5614622812f47-3a0423ea74eso75863b6e.1 for ; Wed, 28 Jun 2023 16:39:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687995570; x=1690587570; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=29vSy0C7mj+ssyxFIohmhW5Vr9TQtgGR9T+jgDZK1FU=; b=KjZD0RID9upAVZKogGOlkBhWvy7lmWgbaOPCfWUgW0Yf7BdFfyw+nSKMJznCxDGpx6 PCZB4hECa6k5m+dQ7iywSDj1x2m1YbuabAA1d+oKsaDkbnej5KSLdxUuu/axiwjlsmvO xpgNBtOlQX117t02RbSdVaZP3X4EB60JZRQxps9ZVGpKGeVeIdOPPy+LPpe5c7Z9JjOJ oJ4AP5Ee9zamXkLPfk4VkPAZkBH653Njy9MXBX+2MqljMyxz0nyL5Cws9k+jZS/QZBB2 a+RTKtEJQBu0SeKkFmz344si9HHnmGrWcVm5UoNct+d49m+Fg3DuUZ73/9d6vM9Es0eq jh3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687995570; x=1690587570; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=29vSy0C7mj+ssyxFIohmhW5Vr9TQtgGR9T+jgDZK1FU=; b=j0aIz6bgUWM9pnWaoRea7OvcsVp+xgGgl5CMyCriQFo5l2rBTq5cSpjEv5RnmdMPUC w1jckgtxSR6lx2jV/Pg19H6nC3x0Z0k+55jl/5rMgVVe9SjzEcw+4Y6kXvpKfxqWwK5T 0FM+Sjp5ohnycaE6aJRBakxitkLkVdE2Q4jtIzSnwHgagEk2rdkQEfN0EJfc7a4n6AA5 rVh8i1Z8Y3A0bA/zfWnxdn9LD4i49E9uFdYfGBI50hS2bOGZ1R7kJk6H6XfJ7ZN2lazF Dj9OVKW/UMjGjaFQbxo4U1cSc+vNVP9wwsk81WcmKHehFK400C3lT13C66mY7Wj8jDAY //Ew== X-Gm-Message-State: AC+VfDxQPmWPGwd62WIN0MO65mbu18IYn6O4sA8gWDKgatXYdJzdoXfp euYy9yjv0WjLm72s3bqJ/YkuQxcCtC6vA6+uO/KLW9UTAbE= X-Google-Smtp-Source: ACHHUZ6bMKboW1mrDbONhUl0yPTgvreSuSRdFH9bOz3GZbo3rlU/2Dj5sTlP2Slk4Mq3nN/DYrN9bQOy2lgafRLzwVo= X-Received: by 2002:a05:6808:2386:b0:3a0:5bbb:fd5 with SMTP id bp6-20020a056808238600b003a05bbb0fd5mr20777142oib.54.1687995569999; Wed, 28 Jun 2023 16:39:29 -0700 (PDT) MIME-Version: 1.0 References: <20230516221557.471-1-brit.chesley@amd.com> In-Reply-To: From: "Mike Maslenkin" Date: Thu, 29 Jun 2023 02:38:54 +0300 Message-ID: Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: UsbBusDxe: Build Descriptor table after resetting port To: devel@edk2.groups.io, hao.a.wu@intel.com Cc: "brit.chesley@amd.com" , "Wang, Jian J" , "Gao, Liming" , "Ni, Ray" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Britton and the community. I've faced with some assert in XHCI too. In my case this is: ASSERT [XhciDxe] /Users/test/edk2_workspace/edk2/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c(19= 89): TrsRing !=3D ((void *) 0) Your patch helps to avoid this assert, but as was noticed by Hao Wu,it introduces resources leak, because UsbBusDxe doesn't deallocate configs for devices. I added checks to UsbBuildDescTable like ASSERT (UsbDev->DevDesc =3D=3D NULL); ASSERT (UsbDev->DevDesc->Configs =3D= =3D NULL); and those fired. I believe there is a problem with XhciDxe, not UsbBusDxe. Meanwhile I have another workaround for initial problem described above and it works. It works fine for virtual machines, but I guess it costs a time for physical environment. My workaround is: --- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c +++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c @@ -173,7 +173,7 @@ UsbMassReadBlocks ( if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "UsbMassReadBlocks: UsbBootReadBlocks (%r) -> Reset\n", Status)); - UsbMassReset (This, TRUE); + UsbMassReset (This, FALSE); } ON_EXIT: --- Just for the context: I was told that some physical device like Zalman ZM-VE500 doesn't work with edk2. I do not have such device in hands, but I guess it is multi-lun USB device. I tried to reproduce such case in QEMU. Because of USB multi-lun devices are not emulated properly in QEMU, such config causes assert in XhciDxe. On Tue, Jun 6, 2023 at 6:14=E2=80=AFAM Wu, Hao A wrote= : > > Sorry for the late response, inline comments below: > > > > -----Original Message----- > > From: brit.chesley@amd.com > > Sent: Wednesday, May 17, 2023 6:16 AM > > To: devel@edk2.groups.io > > Cc: Wang, Jian J ; Gao, Liming > > ; Wu, Hao A ; Ni, Ray > > > > Subject: [PATCH v1 1/1] MdeModulePkg: UsbBusDxe: Build Descriptor table > > after resetting port > > > > From: Britton Chesley > > > > Fixed a bug which led to an ASSERT due to the USB device context being > > maintained after a port reset, but the underlying XHCI context was > > uninitialized. Added build descriptor table call in UsbIoPortReset > > > Could you help to elaborate on what data in XHCI is not initialized that = leads to the assert? Thanks. > > > > > > Cc: Jian J Wang > > Cc: Liming Gao > > Cc: Hao A Wu > > Cc: Ray Ni > > Signed-off-by: Britton Chesley > > --- > > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > index c25f3cc2f279..55e0e1f16cdb 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > > @@ -882,6 +882,12 @@ UsbIoPortReset ( > > // is in CONFIGURED state. > > // > > if (Dev->ActiveConfig !=3D NULL) { > > + Status =3D UsbBuildDescTable (Dev); > > > The UsbBuildDescTable function will allocate new buffer resources for and= under Dev->DevDesc (device, configuration and interface descriptors). > Could you help to double check if the old buffers are properly freed for = the proposed change? > > Best Regards, > Hao Wu > > > > + > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: failed to build descriptor > > table - %r\n", Status)); > > + } > > + > > Status =3D UsbSetConfig (Dev, Dev->ActiveConfig- > > >Desc.ConfigurationValue); > > > > if (EFI_ERROR (Status)) { > > -- > > 2.36.1 > > > >=20 > >