[edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: On reset rebuild descriptor table
Jeremy Linton
jeremy.linton at arm.com
Tue Apr 28 00:44:07 UTC 2020
Hi!
On 4/27/20 7:15 PM, Jiang, Guomin wrote:
> Hi Jeremy,
>
> You can refer https://edk2.groups.io/g/devel/message/58125 for discussion about this solution.
Oh fun, odd how a bug can exist in a code base for years and then this
happens.. I will move the discussion there.
Thanks,
>
> Two issue I found:
> 1. Memory leakage may occur if doing so and I am investigating it.
It seems our solutions differ a bit?
> 2. It test pass with OVMF but fail in real platform, and I am figuring out the flow.
Hmm, I've been seeing this on a RPI with an attached USB3 hub and 5 bay
USB JBOD. (there is another problem but the reset crash is keeps the
machine from booting).
>
> Best Regards
> Guomin
>
>> -----Original Message-----
>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Jeremy
>> Linton
>> Sent: Tuesday, April 28, 2020 6:16 AM
>> To: devel at edk2.groups.io
>> Cc: Wu, Hao A <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>; Kinney,
>> Michael D <michael.d.kinney at intel.com>; Tian, Feng <feng.tian at intel.com>;
>> Wang, Jian J <jian.j.wang at intel.com>; ard.biesheuvel at arm.com; Jeremy
>> Linton <jeremy.linton at arm.com>
>> Subject: [edk2-devel] [PATCH] MdeModulePkg/UsbBusDxe: On reset rebuild
>> descriptor table
>>
>> During port reset, the device descriptors should be checked
>> before attempting to set an endpoint configuration.
>>
>> In particular this fixes a crash due to
>> ASSERT(TrsRing != NULL) in XhcSyncTrsRing(). That crash
>> happens during error recovery on devices attached to XHCI
>> hosts. This is because the port disable clears and deallocats
>> all the EP data structures. When the port is reconfigured
>> without first requesting the EP descriptors,
>> XhcSetConfigCmd[64]() is not being called because the
>> NumConfigurations remains 0.
>>
>> We could attempt to rebuild the EP descriptions directly
>> from the XHCI driver. OTOH, its probably good practice to
>> assure the device description is what we expect from within
>> the core USB subsystem during reset.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton at arm.com>
>> ---
>> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
>> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
>> index 4b4915c019..17bb691bf8 100644
>> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
>> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
>> @@ -874,6 +874,14 @@ UsbIoPortReset (
>> // is in CONFIGURED state.
>>
>> //
>>
>> if (Dev->ActiveConfig != NULL) {
>>
>> + Status = UsbBuildDescTable (Dev);
>>
>> +
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + DEBUG ((EFI_D_ERROR, "UsbIoPortReset: failed to build descriptor table
>> for %d - %r\n",
>>
>> + Dev->Address, Status));
>>
>> + goto ON_EXIT;
>>
>> + }
>>
>> +
>>
>> Status = UsbSetConfig (Dev, Dev->ActiveConfig->Desc.ConfigurationValue);
>>
>>
>>
>> if (EFI_ERROR (Status)) {
>>
>> --
>> 2.24.1
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>>
>> View/Reply Online (#58193): https://edk2.groups.io/g/devel/message/58193
>> Mute This Topic: https://groups.io/mt/73315690/4399222
>> Group Owner: devel+owner at edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [guomin.jiang at intel.com]
>> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58196): https://edk2.groups.io/g/devel/message/58196
Mute This Topic: https://groups.io/mt/73315690/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list