From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.9970.1594772459411795742 for ; Tue, 14 Jul 2020 17:20:59 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: jeremy.linton@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 812C830E; Tue, 14 Jul 2020 17:20:58 -0700 (PDT) Received: from [192.168.122.166] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 35DD73F66E; Tue, 14 Jul 2020 17:20:58 -0700 (PDT) Subject: Re: [PATCH v3 1/1] MdeModulePkg/UsbBusDxe: Rebuild the description table after Reset Device To: Guomin Jiang , devel@edk2.groups.io Cc: Jian J Wang , Hao A Wu , Ray Ni References: <20200509082651.327-1-guomin.jiang@intel.com> From: "Jeremy Linton" Message-ID: <99b5a21c-96e3-6131-76a3-78d24d6a26f3@arm.com> Date: Tue, 14 Jul 2020 19:18:50 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <20200509082651.327-1-guomin.jiang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi, On 5/9/20 3:26 AM, Guomin Jiang wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2694 > > When the USB fail and then Reset Device, it should rebuild description. I pulled the latest edk2 a few hours ago and I'm still seeing the assert TrsRing != 0 messages on a real XHCI controller with the jmicron JBOD I mentioned earlier. Is there a newer version of this patch? Thanks, > > Signed-off-by: Guomin Jiang > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Ray Ni > --- > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 7 ++ > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 152 +++++++++++++++++++++++ > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h | 14 +++ > 3 files changed, 173 insertions(+) > > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > index 4b4915c019ad..7bb9373a6adf 100644 > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c > @@ -874,6 +874,13 @@ UsbIoPortReset ( > // is in CONFIGURED state. > // > if (Dev->ActiveConfig != NULL) { > + Status = UsbRebuildDescTable (Dev); > + > + if (EFI_ERROR (Status)) { > + DEBUG (( DEBUG_ERROR, "UsbIoPortReset: failed to rebuild desc table - %r\n", Status)); > + goto ON_EXIT; > + } > + > Status = UsbSetConfig (Dev, Dev->ActiveConfig->Desc.ConfigurationValue); > > if (EFI_ERROR (Status)) { > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c > index b08188b1bc78..d8e5e50b7c5a 100644 > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c > @@ -900,6 +900,158 @@ UsbBuildDescTable ( > return EFI_SUCCESS; > } > > +/** > + Copy the interface descriptor > + > + @param[out] DescBuf The buffer of raw descriptor. > + @param[in] SrcBuf The buffer of raw descriptor > +**/ > +VOID > +UsbCopyInterfaceDesc ( > + OUT USB_INTERFACE_DESC *DescBuf, > + IN USB_INTERFACE_DESC *SrcBuf > + ) > +{ > + UINTN Index, IndexI; > + > + ASSERT ((DescBuf != NULL) && (SrcBuf != NULL)); > + > + if (DescBuf->NumOfSetting == SrcBuf->NumOfSetting) { > + DescBuf->ActiveIndex = SrcBuf->ActiveIndex; > + > + for (Index = 0; Index < DescBuf->NumOfSetting; Index++) { > + CopyMem (&DescBuf->Settings[Index]->Desc, > + &SrcBuf->Settings[Index]->Desc, > + sizeof(EFI_USB_INTERFACE_DESCRIPTOR)); > + > + if (DescBuf->Settings[Index]->Desc.NumEndpoints == > + SrcBuf->Settings[Index]->Desc.NumEndpoints) { > + > + for (IndexI = 0; IndexI < DescBuf->Settings[Index]->Desc.NumEndpoints; > + IndexI++) { > + CopyMem (DescBuf->Settings[Index]->Endpoints[IndexI], > + SrcBuf->Settings[Index]->Endpoints[IndexI], > + sizeof(USB_ENDPOINT_DESC)); > + } > + } > + } > + } > +} > + > +/** > + Copy the configuration descriptor and its interfaces. > + > + @param[out] DescBuf The buffer of raw descriptor. > + @param[in] SrcBuf The buffer of raw descriptor > +**/ > +VOID > +UsbCopyConfigDesc ( > + OUT USB_CONFIG_DESC *DescBuf, > + IN USB_CONFIG_DESC *SrcBuf > + ) > +{ > + UINTN Index; > + > + ASSERT ((DescBuf != NULL) && (SrcBuf != NULL)); > + > + if (DescBuf->Desc.NumInterfaces == SrcBuf->Desc.NumInterfaces) { > + CopyMem (&DescBuf->Desc, &SrcBuf->Desc, sizeof(EFI_USB_CONFIG_DESCRIPTOR)); > + > + for (Index = 0; Index < DescBuf->Desc.NumInterfaces; Index++) { > + UsbCopyInterfaceDesc (DescBuf->Interfaces[Index], SrcBuf->Interfaces[Index]); > + } > + } > +} > + > +/** > + Rebuild the whole array of descriptors. > + > + @param[in] UsbDev The Usb device. > + > + @retval EFI_SUCCESS The descriptor table is build. > + @retval EFI_DEVICE_ERROR Invalid config > + @retval Others Command error when get descriptor. > +**/ > +EFI_STATUS > +UsbRebuildDescTable ( > + IN USB_DEVICE *UsbDev > + ) > +{ > + EFI_USB_CONFIG_DESCRIPTOR *Config; > + USB_CONFIG_DESC *ConfigDesc; > + UINT8 NumConfig; > + EFI_STATUS Status; > + UINT8 Index; > + > + // > + // Override the device descriptor, Current device fail but the original > + // descriptor pointer array is still exist. > + // > + Status = UsbCtrlGetDesc ( > + UsbDev, > + USB_DESC_TYPE_DEVICE, > + 0, > + 0, > + UsbDev->DevDesc, > + sizeof (EFI_USB_DEVICE_DESCRIPTOR) > + ); > + > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "UsbRebuildDescTable: failed to override device descriptor - %r\n", Status)); > + return Status; > + } > + > + NumConfig = UsbDev->DevDesc->Desc.NumConfigurations; > + if (NumConfig == 0) { > + return EFI_DEVICE_ERROR; > + } > + > + DEBUG ((DEBUG_INFO, "UsbReuildDescTable: device has %d configures\n", NumConfig)); > + > + // > + // Read each configurations, then parse them > + // > + for (Index = 0; Index < NumConfig; Index++) { > + Config = UsbGetOneConfig (UsbDev, Index); > + > + if (Config == NULL) { > + DEBUG ((DEBUG_INFO, "UsbRebuildDescTable: failed to get configure (index %d)\n", Index)); > + > + // > + // If we can get the default descriptor, it is likely that the > + // device is still operational. > + // > + if (Index == 0) { > + return EFI_DEVICE_ERROR; > + } > + > + break; > + } > + > + ConfigDesc = UsbParseConfigDesc ((UINT8 *) Config, Config->TotalLength); > + > + FreePool (Config); > + > + if (ConfigDesc == NULL) { > + DEBUG ((DEBUG_ERROR, "UsbRebuildDescTable: failed to parse configure (index %d)\n", Index)); > + > + // > + // If we can get the default descriptor, it is likely that the > + // device is still operational. > + // > + if (Index == 0) { > + return EFI_DEVICE_ERROR; > + } > + > + break; > + } else { > + UsbCopyConfigDesc (UsbDev->DevDesc->Configs[Index], ConfigDesc); > + UsbFreeConfigDesc (ConfigDesc); > + } > + } > + > + return EFI_SUCCESS; > +} > > /** > Set the device's address. > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h > index 7b0c77fdc79c..54367133241a 100644 > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h > @@ -169,6 +169,20 @@ UsbBuildDescTable ( > IN USB_DEVICE *UsbDev > ); > > +/** > + Rebuild the whole array of descriptors. > + > + @param[in] UsbDev The Usb device. > + > + @retval EFI_SUCCESS The descriptor table is build. > + @retval EFI_DEVICE_ERROR Invalid config > + @retval Others Command error when get descriptor. > +**/ > +EFI_STATUS > +UsbRebuildDescTable ( > + IN USB_DEVICE *UsbDev > + ); > + > /** > Set the device's address. > >