[edk2-devel] [PATCH v2 1/1] MdeModulePkg: UsbBusDxe: Rebuild Descriptor Table

[Chesley, Brit via groups.io]

[AMD Official Use Only - General]

Hello Hao,

Its no problem. I agree that the endpoint transfer rings should be allocated after the UsbSetConfig command, but this is not the case. In XhcControlTransfer, after the if statement checking for USB_REQ_SET_CONFIG, the for-loop loops through all of DevDesc.NumConfigurations and calls XhcSetConfigCmd. The issue here is that after a reset is issued XhcInitializeDeviceSlot64 is called which frees Xhc->UsbDevContext[SlotId]. This sets Xhc->UsbDevContext[SlotId].DevDesc.NumConfigurations to 0. So XhcSetConfigCmd is never called, and the other endpoint transfer rings besides the default are never reinitialized. From what I could tell the easiest way to reacquire the proper NumConfigurations value was to call UsbBuildDescTable for the device.

Best,

Brit Chesley

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu at intel.com>
> Sent: Monday, July 31, 2023 2:37 AM
> To: devel at edk2.groups.io; Chesley, Brit <Brit.Chesley at amd.com>
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming
> <gaoliming at byosoft.com.cn>; Ni, Ray <ray.ni at intel.com>; Chang, Abner
> <Abner.Chang at amd.com>
> Subject: RE: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: UsbBusDxe: Rebuild
> Descriptor Table
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > brit.chesley via groups.io
> > Sent: Saturday, July 8, 2023 1:07 AM
> > To: devel at edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang at intel.com>; Gao, Liming
> > <gaoliming at byosoft.com.cn>; Wu, Hao A <hao.a.wu at intel.com>; Ni, Ray
> > <ray.ni at intel.com>; Abner Chang <Abner.Chang at amd.com>
> > Subject: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: UsbBusDxe: Rebuild
> > Descriptor Table
> >
> > From: Britton Chesley <Brit.Chesley at amd.com>
> >
> > 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. Specifically, Xhc->UsbDevContext is freed after a reset
> > and only re-allocates the default [0] enpoint transfer ring. Added
> > build
>
>
> Really sorry for another question.
>
> My take is that the transfer ring of other endpoints (besides the Default
> Control Endpoint) will be re-initialized by the below flow:
> UsbSetConfig -> UsbCtrlRequest (USB_REQ_SET_CONFIG) ->
> XhcSetConfigCmd(64) -> XhcInitializeEndpointContext(64)
>
> Could you help to elaborate a bit more on what is the issue with the above
> (current) flow and why rebuilding the Descriptor Table before UsbSetConfig
> can resolve it?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > descriptor table call in UsbIoPortReset. BZ 4456
> >
> > Cc: Jian J Wang <jian.j.wang at intel.com>
> > Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > Cc: Hao A Wu <hao.a.wu at intel.com>
> > Cc: Ray Ni <ray.ni at intel.com>
> > Cc: Abner Chang <Abner.Chang at amd.com>
> > Signed-off-by: Britton Chesley <Brit.Chesley at amd.com>
> > ---
> >  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 26
> > ++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > index c25f3cc2f279..a5b798bd8d6c 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c
> > @@ -821,6 +821,7 @@ UsbIoPortReset (
> >    EFI_TPL        OldTpl;
> >    EFI_STATUS     Status;
> >    UINT8          DevAddress;
> > +  UINT8          Config;
> >
> >    OldTpl = gBS->RaiseTPL (USB_BUS_TPL);
> >
> > @@ -882,8 +883,26 @@ UsbIoPortReset (
> >    // is in CONFIGURED state.
> >    //
> >    if (Dev->ActiveConfig != NULL) {
> > -    Status = UsbSetConfig (Dev, Dev->ActiveConfig-
> >Desc.ConfigurationValue);
> > +    UsbFreeDevDesc (Dev->DevDesc);
> >
> > +    Status = UsbRemoveConfig (Dev);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to remove
> > configuration - %r\n", Status));
> > +    }
> > +
> > +    Status = UsbGetMaxPacketSize0 (Dev);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to get max packet
> > + size -
> >  %r\n", Status));
> > +    }
> > +
> > +    Status = UsbBuildDescTable (Dev);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to build
> > + descriptor
> > table - %r\n", Status));
> > +    }
> > +
> > +    Config = Dev->DevDesc->Configs[0]->Desc.ConfigurationValue;
> > +
> > +    Status = UsbSetConfig (Dev, Config);
> >      if (EFI_ERROR (Status)) {
> >        DEBUG ((
> >          DEBUG_ERROR,
> > @@ -892,6 +911,11 @@ UsbIoPortReset (
> >          Status
> >          ));
> >      }
> > +
> > +    Status = UsbSelectConfig (Dev, Config);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to set
> > + configuration -
> >  %r\n", Status));
> > +    }
> >    }
> >
> >  ON_EXIT:
> > --
> > 2.36.1
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107798): https://edk2.groups.io/g/devel/message/107798
Mute This Topic: https://groups.io/mt/100010162/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-