[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...@intel.com>
> Sent: Monday, July 31, 2023 2:37 AM
> To: devel@edk2.groups.io; Chesley, Brit <brit.ches...@amd.com>
> Cc: Wang, Jian J <jian.j.w...@intel.com>; Gao, Liming
> <gaolim...@byosoft.com.cn>; Ni, Ray <ray...@intel.com>; Chang, Abner
> <abner.ch...@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > brit.chesley via groups.io
> > Sent: Saturday, July 8, 2023 1:07 AM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.w...@intel.com>; Gao, Liming
> > <gaolim...@byosoft.com.cn>; Wu, Hao A <hao.a...@intel.com>; Ni, Ray
> > <ray...@intel.com>; Abner Chang <abner.ch...@amd.com>
> > Subject: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: UsbBusDxe: Rebuild
> > Descriptor Table
> >
> > From: Britton Chesley <brit.ches...@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.w...@intel.com>
> > Cc: Liming Gao <gaolim...@byosoft.com.cn>
> > Cc: Hao A Wu <hao.a...@intel.com>
> > Cc: Ray Ni <ray...@intel.com>
> > Cc: Abner Chang <abner.ch...@amd.com>
> > Signed-off-by: Britton Chesley <brit.ches...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
