diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.cpp b/MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.cpp new file mode 100644 index 0000000000..9aec3d7a6b --- /dev/null +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.cpp @@ -0,0 +1,705 @@ +/** @file + Exercise unchanged, runtime-to-DFU, and DFU-to-runtime USB resets. + + Copyright (c) Microsoft Corporation. + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#include +#include +#include +#include +#include + +extern "C" { + #include "../UsbBus.h" +} + +namespace { +#pragma pack(push, 1) +/// USB DFU 1.1 functional descriptor, section 4.2.3. +struct DfuFunctionalDescriptor { + UINT8 Length; + UINT8 DescriptorType; + UINT8 Attributes; + UINT16 DetachTimeout; + UINT16 TransferSize; + UINT16 DfuVersion; +}; + +#pragma pack(pop) + +static_assert (sizeof (DfuFunctionalDescriptor) == 9, "DFU functional descriptor must be nine bytes"); + +struct PublishedInterface { + UINTN Id; + EFI_DEVICE_PATH_PROTOCOL *Path; + EFI_USB_IO_PROTOCOL *UsbIo; +}; + +class UsbDfuResetTest : public testing::Test { +protected: + static UsbDfuResetTest *Active; + EFI_BOOT_SERVICES Services { }; + EFI_BOOT_SERVICES *SavedServices { }; + EFI_USB2_HC_PROTOCOL Host { }; + USB_HUB_API HubApi { }; + USB_BUS Bus { }; + USB_DEVICE Parent { }; + USB_INTERFACE Hub { }; + USB_DEVICE *Device { }; + EFI_DEVICE_PATH_PROTOCOL ParentPath { + END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE, + { END_DEVICE_PATH_LENGTH, 0 } + }; + std::vector > Published; + std::vector Removed; + std::vector Configuration; + EFI_USB_DEVICE_DESCRIPTOR Descriptor { }; + EFI_TPL CurrentTpl = TPL_APPLICATION; + UINTN InstallCount = 0; + bool NextDfu = false; + + /** + Install fake Boot Services and initialize the USB device and parent hub. + **/ + void + SetUp ( + ) override + { + Active = this; + SavedServices = gBS; + Services.RaiseTPL = RaiseTpl; + Services.RestoreTPL = RestoreTpl; + Services.Stall = Stall; + Services.FreePool = ReleasePool; + Services.InstallMultipleProtocolInterfaces = Install; + Services.UninstallMultipleProtocolInterfaces = Uninstall; + Services.OpenProtocol = Open; + Services.CloseProtocol = Close; + Services.ConnectController = Connect; + Services.DisconnectController = Disconnect; + gBS = &Services; + Host.ControlTransfer = ControlTransfer; + HubApi.ResetPort = ResetPort; + HubApi.ClearPortChange = ClearPortChange; + Bus.Usb2Hc = &Host; + Bus.Signature = USB_BUS_SIGNATURE; + Bus.MaxDevices = USB_MAX_DEVICES; + InitializeListHead (&Bus.WantedUsbIoDPList); + Parent.Bus = &Bus; + Hub.Device = &Parent; + Hub.DevicePath = &ParentPath; + Hub.HubApi = &HubApi; + Device = static_cast (AllocateZeroPool (sizeof (USB_DEVICE))); + ASSERT_NE (Device, nullptr); + Device->Bus = &Bus; + Device->ParentIf = &Hub; + Device->ParentPort = 2; + Device->Connected = TRUE; + Device->MaxPacket0 = 64; + Device->Address = 1; + Device->Speed = EFI_USB_SPEED_HIGH; + Bus.Devices[1] = Device; + } + + /** + Remove the test device, verify its protocols are gone, and restore Boot Services. + **/ + void + TearDown ( + ) override + { + if (Device != nullptr) { + EXPECT_EQ (UsbRemoveDevice (Device), EFI_SUCCESS); + } + + EXPECT_TRUE (Published.empty ()); + EXPECT_EQ (UsbBusFreeUsbDPList (&Bus.WantedUsbIoDPList), EFI_SUCCESS); + gBS = SavedServices; + Active = nullptr; + } + + /** + Build the descriptors returned by the emulated host controller. + + @param[in] Dfu True for one DFU interface; false for four runtime interfaces. + **/ + void + SetInterfaceLayout ( + bool Dfu + ) + { + ZeroMem (&Descriptor, sizeof (Descriptor)); + Descriptor.Length = sizeof (Descriptor); + Descriptor.DescriptorType = USB_DESC_TYPE_DEVICE; + Descriptor.BcdUSB = 0x200; + Descriptor.MaxPacketSize0 = 64; + Descriptor.IdVendor = 0x13D3; + Descriptor.IdProduct = Dfu ? 0x5800 : 0x5D48; + Descriptor.NumConfigurations = 1; + EFI_USB_CONFIG_DESCRIPTOR Config { }; + + Config.Length = sizeof (Config); + Config.DescriptorType = USB_DESC_TYPE_CONFIG; + Config.NumInterfaces = Dfu ? 1 : 4; + Config.ConfigurationValue = 1; + Config.Attributes = 0x80; + Config.MaxPower = 50; + Config.TotalLength = static_cast ( + sizeof (Config) + + Config.NumInterfaces * sizeof (EFI_USB_INTERFACE_DESCRIPTOR) + + (Dfu ? sizeof (DfuFunctionalDescriptor) : 0) + ); + Configuration.resize (Config.TotalLength); + CopyMem (Configuration.data (), &Config, sizeof (Config)); + UINTN Offset = sizeof (Config); + + for (UINT8 Index = 0; Index < Config.NumInterfaces; ++Index) { + EFI_USB_INTERFACE_DESCRIPTOR Function { }; + Function.Length = sizeof (Function); + Function.DescriptorType = USB_DESC_TYPE_INTERFACE; + Function.InterfaceNumber = Index; + Function.InterfaceClass = Dfu ? 0xFE : 0x0E; + Function.InterfaceSubClass = 1; + Function.InterfaceProtocol = Dfu ? 2 : 0; + CopyMem (Configuration.data () + Offset, &Function, sizeof (Function)); + Offset += sizeof (Function); + } + + if (Dfu) { + DfuFunctionalDescriptor Functional { }; + Functional.Length = sizeof (Functional); + Functional.DescriptorType = 0x21; + Functional.Attributes = 0x0B; + Functional.DetachTimeout = 0x00FF; + Functional.TransferSize = 0x0400; + Functional.DfuVersion = 0x0110; + CopyMem (Configuration.data () + Offset, &Functional, sizeof (Functional)); + } + } + + /** + Read descriptors and publish the initial interfaces without connecting drivers. + + @param[in] Dfu Whether the device starts in DFU mode. + **/ + void + StartDevice ( + bool Dfu + ) + { + SetInterfaceLayout (Dfu); + ASSERT_EQ (UsbBuildDescTable (Device), EFI_SUCCESS); + ASSERT_EQ (UsbSetConfig (Device, 1), EFI_SUCCESS); + ASSERT_EQ (UsbSelectConfig (Device, 1, FALSE), EFI_SUCCESS); + ExpectInterfaceLayout (Dfu); + } + + /** + Select the next interface layout and reset through an installed USB I/O protocol. + + @param[in] Dfu Whether the device enters DFU mode on this reset. + + @return The status returned by UsbPortReset(). + **/ + EFI_STATUS + Reset ( + bool Dfu + ) + { + NextDfu = Dfu; + EFI_USB_IO_PROTOCOL *UsbIo = Published.front ()->UsbIo; + + return UsbIo->UsbPortReset (UsbIo); + } + + /** + Snapshot installation IDs without retaining pointers that reset may invalidate. + + @return The IDs of all currently published interfaces, in publication order. + **/ + std::vector + PublishedIds ( + ) + { + std::vector Ids; + + for (const auto &Interface : Published) { + Ids.push_back (Interface->Id); + } + + return Ids; + } + + /** + Verify the installed USB I/O protocols expose the expected interface layout. + + @param[in] Dfu True to expect one DFU interface; false to expect four runtime interfaces. + **/ + void + ExpectInterfaceLayout ( + bool Dfu + ) + { + const UINT8 Count = Dfu ? 1 : 4; + + ASSERT_EQ (Published.size (), Count); + EXPECT_EQ (Device->NumOfInterface, Count); + EXPECT_TRUE (Device->Connected); + for (UINT8 Index = 0; Index < Count; ++Index) { + EFI_USB_IO_PROTOCOL *UsbIo = Published[Index]->UsbIo; + ASSERT_NE (UsbIo, nullptr); + EFI_USB_DEVICE_DESCRIPTOR DeviceDescriptor { }; + EFI_USB_INTERFACE_DESCRIPTOR InterfaceDescriptor { }; + EFI_USB_CONFIG_DESCRIPTOR ConfigDescriptor { }; + ASSERT_EQ (UsbIo->UsbGetDeviceDescriptor (UsbIo, &DeviceDescriptor), EFI_SUCCESS); + ASSERT_EQ (UsbIo->UsbGetInterfaceDescriptor (UsbIo, &InterfaceDescriptor), EFI_SUCCESS); + ASSERT_EQ (UsbIo->UsbGetConfigDescriptor (UsbIo, &ConfigDescriptor), EFI_SUCCESS); + EXPECT_EQ (DeviceDescriptor.IdProduct, Dfu ? 0x5800 : 0x5D48); + EXPECT_EQ (ConfigDescriptor.NumInterfaces, Count); + EXPECT_EQ (InterfaceDescriptor.InterfaceNumber, Index); + EXPECT_EQ (InterfaceDescriptor.InterfaceClass, Dfu ? 0xFE : 0x0E); + EXPECT_EQ (InterfaceDescriptor.InterfaceProtocol, Dfu ? 2 : 0); + } + } + + /** + Update the fake current TPL without dispatching firmware events. + + @param[in] Tpl The new TPL. + + @return The previous TPL. + **/ + static EFI_TPL EFIAPI + RaiseTpl ( + EFI_TPL Tpl + ) + { + const EFI_TPL Previous = Active->CurrentTpl; + + Active->CurrentTpl = Tpl; + return Previous; + } + + /** + Restore the fake current TPL without dispatching firmware events. + + @param[in] Tpl The TPL to restore. + **/ + static VOID EFIAPI + RestoreTpl ( + EFI_TPL Tpl + ) + { + Active->CurrentTpl = Tpl; + } + + /** + Accept a firmware delay without delaying the host test. + + @param[in] Microseconds The requested delay, ignored. + + @retval EFI_SUCCESS The delay was skipped. + **/ + static EFI_STATUS EFIAPI + Stall ( + UINTN Microseconds + ) + { + (void)Microseconds; + return EFI_SUCCESS; + } + + /** + Release a pool allocation using the host memory-allocation library. + + @param[in] Buffer The allocation to release. + + @retval EFI_SUCCESS The allocation was released. + **/ + static EFI_STATUS EFIAPI + ReleasePool ( + VOID *Buffer + ) + { + FreePool (Buffer); + return EFI_SUCCESS; + } + + /** + Record a Device Path and USB I/O installation, rejecting duplicate paths. + + @param[out] Handle Receives the fake handle for the installed protocols. + @param[in] ... Device Path and USB I/O GUID/interface pairs, followed by NULL. + + @retval EFI_SUCCESS The interface was recorded. + @retval EFI_ALREADY_STARTED The device path is already installed. + **/ + static EFI_STATUS EFIAPI + Install ( + EFI_HANDLE *Handle, + ... + ) + { + VA_LIST Args; + + VA_START (Args, Handle); + EXPECT_EQ (VA_ARG (Args, EFI_GUID *), &gEfiDevicePathProtocolGuid); + auto *Path = VA_ARG (Args, EFI_DEVICE_PATH_PROTOCOL *); + + EXPECT_EQ (VA_ARG (Args, EFI_GUID *), &gEfiUsbIoProtocolGuid); + auto *UsbIo = VA_ARG (Args, EFI_USB_IO_PROTOCOL *); + + EXPECT_EQ (VA_ARG (Args, VOID *), nullptr); + VA_END (Args); + for (const auto &Existing : Active->Published) { + if ((GetDevicePathSize (Existing->Path) == GetDevicePathSize (Path)) && + (CompareMem (Existing->Path, Path, GetDevicePathSize (Path)) == 0)) + { + return EFI_ALREADY_STARTED; + } + } + + auto Record = std::make_unique (); + + Record->Id = ++Active->InstallCount; + Record->Path = Path; + Record->UsbIo = UsbIo; + *Handle = Record.get (); + Active->Published.push_back (std::move (Record)); + return EFI_SUCCESS; + } + + /** + Remove an installed interface and record its ID for replacement assertions. + + @param[in] Handle The fake handle to remove. + @param[in] ... Protocol GUID/interface pairs, ignored by this fake. + + @retval EFI_SUCCESS The interface was removed. + @retval EFI_NOT_FOUND The handle is unknown; a test failure is also recorded. + **/ + static EFI_STATUS EFIAPI + Uninstall ( + EFI_HANDLE Handle, + ... + ) + { + const auto Found = std::find_if ( + Active->Published.begin (), + Active->Published.end (), + [Handle](const std::unique_ptr &Item) { + return Item.get () == Handle; + } + ); + + if (Found == Active->Published.end ()) { + ADD_FAILURE () << "Uninstalling an unknown handle"; + return EFI_NOT_FOUND; + } + + Active->Removed.push_back ((*Found)->Id); + Active->Published.erase (Found); + return EFI_SUCCESS; + } + + /** + Supply the fake host controller for a USB interface's child-controller open. + + @param[in] Handle The controller handle, ignored. + @param[in] Protocol Expected to identify the USB 2.0 host controller protocol. + @param[out] Interface Receives the fake host controller. + @param[in] Agent The opening driver, ignored. + @param[in] Controller The child controller, ignored. + @param[in] Attributes Expected to request a child-controller open. + + @retval EFI_SUCCESS The fake host controller was returned. + **/ + static EFI_STATUS EFIAPI + Open ( + EFI_HANDLE Handle, + EFI_GUID *Protocol, + VOID **Interface, + EFI_HANDLE Agent, + EFI_HANDLE Controller, + UINT32 Attributes + ) + { + (void)Handle; + (void)Agent; + (void)Controller; + EXPECT_EQ (Protocol, &gEfiUsb2HcProtocolGuid); + EXPECT_EQ (Attributes, static_cast (EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER)); + *Interface = &Active->Host; + return EFI_SUCCESS; + } + + /** + Accept closure of a host-controller protocol relationship without tracking it. + + @param[in] Handle The controller handle, ignored. + @param[in] Protocol The protocol GUID, ignored. + @param[in] Agent The opening driver, ignored. + @param[in] Controller The child controller, ignored. + + @retval EFI_SUCCESS The close request was accepted. + **/ + static EFI_STATUS EFIAPI + Close ( + EFI_HANDLE Handle, + EFI_GUID *Protocol, + EFI_HANDLE Agent, + EFI_HANDLE Controller + ) + { + (void)Handle; + (void)Protocol; + (void)Agent; + (void)Controller; + return EFI_SUCCESS; + } + + /** + Fail the test if reset attempts to connect function drivers. + + @param[in] Handle The controller handle, ignored. + @param[in] Drivers The driver list, ignored. + @param[in] Path The remaining device path, ignored. + @param[in] Recursive The recursive connection flag, ignored. + + @retval EFI_UNSUPPORTED Connection is forbidden by these tests. + **/ + static EFI_STATUS EFIAPI + Connect ( + EFI_HANDLE Handle, + EFI_HANDLE *Drivers, + EFI_DEVICE_PATH_PROTOCOL *Path, + BOOLEAN Recursive + ) + { + (void)Handle; + (void)Drivers; + (void)Path; + (void)Recursive; + ADD_FAILURE () << "Reset must not connect replacement drivers"; + return EFI_UNSUPPORTED; + } + + /** + Allow interface teardown without simulating function drivers or their children. + + @param[in] Handle The controller handle, ignored. + @param[in] Driver The driver handle, ignored. + @param[in] Child The child handle, ignored. + + @retval EFI_SUCCESS The disconnect request was accepted. + **/ + static EFI_STATUS EFIAPI + Disconnect ( + EFI_HANDLE Handle, + EFI_HANDLE Driver, + EFI_HANDLE Child + ) + { + (void)Handle; + (void)Driver; + (void)Child; + return EFI_SUCCESS; + } + + /** + Apply the selected interface layout when production code resets the parent port. + + @param[in] Hub The parent hub interface, ignored. + @param[in] Port Expected to identify the test device's port. + + @retval EFI_SUCCESS The selected descriptors are ready for enumeration. + **/ + static EFI_STATUS + ResetPort ( + USB_INTERFACE *Hub, + UINT8 Port + ) + { + (void)Hub; + EXPECT_EQ (Port, 2); + Active->SetInterfaceLayout (Active->NextDfu); + return EFI_SUCCESS; + } + + /** + Accept port-change acknowledgement without tracking hub change bits. + + @param[in] Hub The parent hub interface, ignored. + @param[in] Port The port number, ignored. + **/ + static VOID + ClearPortChange ( + USB_INTERFACE *Hub, + UINT8 Port + ) + { + (void)Hub; + (void)Port; + } + + /** + Serve descriptor reads and accept address and configuration requests. + + @param[in] This The host controller protocol, ignored. + @param[in] Address Expected to be zero for address assignment. + @param[in] Speed The device speed, ignored. + @param[in] MaxPacket The endpoint-zero packet size, ignored. + @param[in] Request The USB setup request. + @param[in] Direction The transfer direction, ignored. + @param[out] Data Receives descriptor bytes for descriptor reads. + @param[in,out] Length Buffer capacity on entry; bytes copied for descriptor reads. + @param[in] Timeout The transfer timeout, ignored. + @param[in] Translator The transaction translator, ignored. + @param[out] Result Receives the USB transfer result. + + @retval EFI_SUCCESS A supported request completed. + @retval EFI_DEVICE_ERROR An unavailable descriptor was requested; USB status is STALL. + @retval EFI_UNSUPPORTED An unexpected request also records a test failure. + **/ + static EFI_STATUS EFIAPI + ControlTransfer ( + EFI_USB2_HC_PROTOCOL *This, + UINT8 Address, + UINT8 Speed, + UINTN MaxPacket, + EFI_USB_DEVICE_REQUEST *Request, + EFI_USB_DATA_DIRECTION Direction, + VOID *Data, + UINTN *Length, + UINTN Timeout, + EFI_USB2_HC_TRANSACTION_TRANSLATOR *Translator, + UINT32 *Result + ) + { + (void)This; + (void)Speed; + (void)MaxPacket; + (void)Direction; + (void)Timeout; + (void)Translator; + *Result = EFI_USB_NOERROR; + if (Request->Request == USB_REQ_SET_ADDRESS) { + EXPECT_EQ (Address, 0); + EXPECT_EQ (Request->Value, 1); + return EFI_SUCCESS; + } + + if (Request->Request == USB_REQ_SET_CONFIG) { + EXPECT_EQ (Request->Value, 1); + return EFI_SUCCESS; + } + + if (Request->Request == USB_REQ_GET_DESCRIPTOR) { + const UINT8 Type = static_cast (Request->Value >> 8); + const VOID *Source = nullptr; + UINTN Size = 0; + if (Type == USB_DESC_TYPE_DEVICE) { + Source = &Active->Descriptor; + Size = sizeof (Active->Descriptor); + } else if (Type == USB_DESC_TYPE_CONFIG) { + Source = Active->Configuration.data (); + Size = Active->Configuration.size (); + } else { + // No strings are advertised by this device. + *Result = EFI_USB_ERR_STALL; + return EFI_DEVICE_ERROR; + } + + *Length = std::min (*Length, Size); + CopyMem (Data, Source, *Length); + return EFI_SUCCESS; + } + + ADD_FAILURE () << "Unexpected USB request " << static_cast (Request->Request); + return EFI_UNSUPPORTED; + } +}; + +UsbDfuResetTest *UsbDfuResetTest::Active = nullptr; + +/** + Reset a device that exposes four runtime interfaces without changing its mode. + Expect the same four handles and USB I/O pointers, with no protocol removals + or new installations. +**/ +TEST_F (UsbDfuResetTest, ResetPreservesFourInterfaces) { + ASSERT_NO_FATAL_FAILURE (StartDevice (false)); + const auto OriginalIds = PublishedIds (); + std::vector Handles; + std::vector Protocols; + + for (const auto &Interface : Published) { + Handles.push_back (Interface.get ()); + Protocols.push_back (Interface->UsbIo); + } + + ASSERT_EQ (Reset (false), EFI_SUCCESS); + ASSERT_NO_FATAL_FAILURE (ExpectInterfaceLayout (false)); + EXPECT_EQ (PublishedIds (), OriginalIds); + EXPECT_EQ (InstallCount, 4U); + EXPECT_TRUE (Removed.empty ()); + for (UINTN Index = 0; Index < Published.size (); ++Index) { + EXPECT_EQ (Published[Index].get (), Handles[Index]); + EXPECT_EQ (Published[Index]->UsbIo, Protocols[Index]); + } +} + +/** + Reset a device from four runtime interfaces into single-interface DFU mode. + Expect all four original installations to be removed and exactly one new + USB I/O interface exposing DFU descriptors to be available. +**/ +TEST_F (UsbDfuResetTest, ResetEntersDfu) { + ASSERT_NO_FATAL_FAILURE (StartDevice (false)); + const auto OriginalIds = PublishedIds (); + + ASSERT_EQ (Reset (true), EFI_SUCCESS); + ASSERT_NO_FATAL_FAILURE (ExpectInterfaceLayout (true)); + EXPECT_EQ (Removed, OriginalIds); + EXPECT_EQ (InstallCount, 5U); + EXPECT_GT (Published[0]->Id, OriginalIds.back ()); +} + +/** + Start with one DFU interface and reset the device into runtime mode. + Expect the original DFU installation to be removed and four new USB I/O + interfaces exposing runtime descriptors to be available. +**/ +TEST_F (UsbDfuResetTest, ResetExitsDfu) { + ASSERT_NO_FATAL_FAILURE (StartDevice (true)); + const auto OriginalIds = PublishedIds (); + + ASSERT_EQ (Reset (false), EFI_SUCCESS); + ASSERT_NO_FATAL_FAILURE (ExpectInterfaceLayout (false)); + EXPECT_EQ (Removed, OriginalIds); + EXPECT_EQ (InstallCount, 5U); + for (const auto &Interface : Published) { + EXPECT_GT (Interface->Id, OriginalIds.back ()); + } +} +} // namespace + +/** + Initialize Google Test and run the USB reset scenarios. + + @param[in] Argc The command-line argument count. + @param[in] Argv The command-line arguments. + + @return Zero if all tests pass; nonzero otherwise. +**/ +int +main ( + int Argc, + char *Argv[] + ) +{ + testing::InitGoogleTest (&Argc, Argv); + return RUN_ALL_TESTS (); +} diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.inf b/MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.inf new file mode 100644 index 0000000000..3928e45f26 --- /dev/null +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.inf @@ -0,0 +1,43 @@ +## @file +# Host tests for USB reset personality transitions. +# +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: BSD-2-Clause-Patent +## + +[Defines] + INF_VERSION = 0x00010017 + BASE_NAME = UsbBusDxeGoogleTest + FILE_GUID = 4D361CEA-33F9-4B91-B989-EA64B02CED3F + MODULE_TYPE = HOST_APPLICATION + VERSION_STRING = 1.0 + +[Sources] + UsbBusDxeGoogleTest.cpp + ../UsbBus.c + ../UsbEnumer.c + ../UsbDesc.c + ../UsbUtility.c + ../UsbHub.c + ../ComponentName.c + +[Packages] + MdePkg/MdePkg.dec + UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec + +[LibraryClasses] + GoogleTestLib + BaseLib + BaseMemoryLib + DebugLib + MemoryAllocationLib + DevicePathLib + UefiBootServicesTableLib + UefiLib + ReportStatusCodeLib + +[Protocols] + gEfiUsbIoProtocolGuid + gEfiDevicePathProtocolGuid + gEfiUsb2HcProtocolGuid + diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c index 0c153bca96..c938d0e418 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c @@ -188,11 +188,15 @@ UsbIoControlTransfer ( DEBUG ((DEBUG_INFO, "UsbIoControlTransfer: configure changed!!! Do NOT use old UsbIo!!!\n")); if (Dev->ActiveConfig != NULL) { - UsbRemoveConfig (Dev); + Status = UsbRemoveConfig (Dev); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbIoControlTransfer: configuration teardown failed - %r\n", Status)); + goto ON_EXIT; + } } if (Request->Value != 0) { - Status = UsbSelectConfig (Dev, (UINT8)Request->Value); + Status = UsbSelectConfig (Dev, (UINT8)Request->Value, TRUE); } // @@ -895,9 +899,246 @@ UsbIoGetStringDescriptor ( return Status; } +/** + Compare the default function and endpoints before retaining an interface. +**/ +STATIC +BOOLEAN +UsbInterfaceIsCompatible ( + IN USB_INTERFACE *UsbIf, + IN USB_INTERFACE_DESC *IfDesc + ) +{ + USB_INTERFACE_SETTING *OldSetting; + USB_INTERFACE_SETTING *Setting; + UINTN Index; + + if (UsbIf->IsHub) { + return FALSE; + } + + OldSetting = NULL; + for (Index = 0; Index < UsbIf->IfDesc->NumOfSetting; Index++) { + if ((UsbIf->IfDesc->Settings[Index] != NULL) && + (UsbIf->IfDesc->Settings[Index]->Desc.AlternateSetting == 0)) + { + OldSetting = UsbIf->IfDesc->Settings[Index]; + break; + } + } + + Setting = IfDesc->Settings[IfDesc->ActiveIndex]; + if ((OldSetting == NULL) || + (Setting->Desc.InterfaceNumber != OldSetting->Desc.InterfaceNumber) || + (Setting->Desc.InterfaceClass != OldSetting->Desc.InterfaceClass) || + (Setting->Desc.InterfaceSubClass != OldSetting->Desc.InterfaceSubClass) || + (Setting->Desc.InterfaceProtocol != OldSetting->Desc.InterfaceProtocol) || + (Setting->Desc.NumEndpoints != OldSetting->Desc.NumEndpoints)) + { + return FALSE; + } + + for (Index = 0; Index < Setting->Desc.NumEndpoints; Index++) { + if (CompareMem ( + &Setting->Endpoints[Index]->Desc, + &OldSetting->Endpoints[Index]->Desc, + sizeof (EFI_USB_ENDPOINT_DESCRIPTOR) + ) != 0) + { + return FALSE; + } + } + + return TRUE; +} + +/** + Refresh compatible interfaces, or disconnect and replace a changed personality + without connecting drivers. The caller must coordinate safe consumer teardown. + + @param Dev The device to reconfigure. + @param CanReplace The caller's TPL permits synchronous teardown. + + @retval EFI_SUCCESS The configuration and descriptors are refreshed. + @retval EFI_NOT_READY Changed interfaces cannot be replaced during binding. + @retval Others Failed to read descriptors or configure the device. + +**/ +STATIC +EFI_STATUS +UsbResetConfig ( + IN USB_DEVICE *Dev, + IN BOOLEAN CanReplace + ) +{ + USB_DEVICE NewDev; + USB_DEVICE_DESC *OldDesc; + USB_CONFIG_DESC *ConfigDesc; + USB_INTERFACE_DESC *IfDesc; + USB_INTERFACE_SETTING *Setting; + USB_INTERFACE *UsbIf; + EFI_STATUS Status; + UINTN Index; + UINTN SettingIndex; + UINT8 Config; + BOOLEAN Replace; + + // + // Build into a temporary device so failures leave all cached pointers valid. + // + CopyMem (&NewDev, Dev, sizeof (NewDev)); + NewDev.DevDesc = NULL; + NewDev.IsSSDev = FALSE; + NewDev.TotalLangId = 0; + ZeroMem (NewDev.LangId, sizeof (NewDev.LangId)); + + Status = UsbGetMaxPacketSize0 (&NewDev); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: failed to get max packet size - %r\n", Status)); + return Status; + } + + Status = UsbBuildDescTable (&NewDev); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: failed to build descriptor table - %r\n", Status)); + goto ON_ERROR; + } + + Replace = (BOOLEAN)( + (CompareMem (&NewDev.DevDesc->Desc, &Dev->DevDesc->Desc, sizeof (EFI_USB_DEVICE_DESCRIPTOR)) != 0) + ); + Config = Dev->ActiveConfig->Desc.ConfigurationValue; + ConfigDesc = NULL; + for (Index = 0; Index < NewDev.DevDesc->Desc.NumConfigurations; Index++) { + if ((NewDev.DevDesc->Configs[Index] != NULL) && + (NewDev.DevDesc->Configs[Index]->Desc.ConfigurationValue == Config)) + { + ConfigDesc = NewDev.DevDesc->Configs[Index]; + break; + } + } + + if ((ConfigDesc == NULL) && (NewDev.DevDesc->Desc.NumConfigurations != 0)) { + ConfigDesc = NewDev.DevDesc->Configs[0]; + Replace = TRUE; + } + + Status = EFI_DEVICE_ERROR; + if ((ConfigDesc == NULL) || + (ConfigDesc->Desc.NumInterfaces == 0) || + (ConfigDesc->Desc.NumInterfaces > USB_MAX_INTERFACE)) + { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: incompatible configuration after reset\n")); + goto ON_ERROR; + } + + Replace = (BOOLEAN)(Replace || + (ConfigDesc->Desc.NumInterfaces != Dev->ActiveConfig->Desc.NumInterfaces)); + Config = ConfigDesc->Desc.ConfigurationValue; + + for (Index = 0; Index < ConfigDesc->Desc.NumInterfaces; Index++) { + IfDesc = ConfigDesc->Interfaces[Index]; + if (IfDesc == NULL) { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: missing interface %d\n", (UINT32)Index)); + goto ON_ERROR; + } + + // + // SET_CONFIGURATION selects alternate setting zero, not the old setting. + // + Setting = NULL; + for (SettingIndex = 0; SettingIndex < IfDesc->NumOfSetting; SettingIndex++) { + Setting = IfDesc->Settings[SettingIndex]; + if ((Setting != NULL) && (Setting->Desc.AlternateSetting == 0)) { + break; + } + } + + if ((SettingIndex == IfDesc->NumOfSetting) || + (Setting->Desc.InterfaceNumber != Index) || + (Setting->Desc.InterfaceClass == USB_HUB_CLASS_CODE)) + { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: incompatible interface %d after reset\n", (UINT32)Index)); + goto ON_ERROR; + } + + IfDesc->ActiveIndex = SettingIndex; + UsbIf = Dev->Interfaces[Index]; + if ((UsbIf != NULL) && !UsbInterfaceIsCompatible (UsbIf, IfDesc)) { + Replace = TRUE; + } + } + + if (Replace) { + if (Dev->BindingInProgress || !CanReplace) { + Status = EFI_NOT_READY; + DEBUG ((DEBUG_ERROR, "UsbResetConfig: changed personality requires coordinated teardown outside binding\n")); + goto ON_ERROR; + } + + // + // Keep descriptors alive until every old USB I/O and device path is removed. + // + Dev->Connected = FALSE; + Status = UsbRemoveConfig (Dev); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: old interface teardown failed - %r\n", Status)); + goto ON_ERROR; + } + } + + Status = UsbSetConfig (&NewDev, Config); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: failed to set configuration - %r\n", Status)); + goto ON_ERROR; + } + + OldDesc = Dev->DevDesc; + Dev->DevDesc = NewDev.DevDesc; + NewDev.DevDesc = NULL; + Dev->ActiveConfig = ConfigDesc; + Dev->MaxPacket0 = NewDev.MaxPacket0; + Dev->IsSSDev = NewDev.IsSSDev; + Dev->TotalLangId = NewDev.TotalLangId; + CopyMem (Dev->LangId, NewDev.LangId, sizeof (Dev->LangId)); + + for (Index = 0; Index < ConfigDesc->Desc.NumInterfaces; Index++) { + UsbIf = Dev->Interfaces[Index]; + if (UsbIf != NULL) { + UsbIf->IfDesc = ConfigDesc->Interfaces[Index]; + UsbIf->IfSetting = UsbIf->IfDesc->Settings[UsbIf->IfDesc->ActiveIndex]; + } + } + + UsbFreeDevDesc (OldDesc); + if (Replace) { + Status = UsbSelectConfig (Dev, Config, FALSE); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: replacement publication failed - %r\n", Status)); + return Status; + } + + Dev->Connected = TRUE; + } + + return EFI_SUCCESS; + +ON_ERROR: + if (NewDev.DevDesc != NULL) { + UsbFreeDevDesc (NewDev.DevDesc); + } + + return Status; +} + /** Reset the device, then if that succeeds, reconfigure the device with its address and current active configuration. + Refresh compatible interfaces in place. A changed personality requires caller- + coordinated driver disconnection and invalidates old USB I/O pointers. + Replacement publishes interfaces without connecting drivers. It is rejected + during bus-owned binding or above TPL_CALLBACK. Failures leave the device + offline; no automatic recovery or reconnect is scheduled. @param This The USB IO instance. @@ -911,19 +1152,25 @@ UsbIoPortReset ( IN EFI_USB_IO_PROTOCOL *This ) { - USB_INTERFACE *UsbIf; - USB_INTERFACE *HubIf; - USB_DEVICE *Dev; - EFI_TPL OldTpl; - EFI_STATUS Status; - UINT8 DevAddress; - UINT8 Config; - USB_DEVICE_DESC *DevDesc; + USB_INTERFACE *UsbIf; + USB_INTERFACE *HubIf; + USB_DEVICE *Dev; + EFI_TPL OldTpl; + EFI_STATUS Status; + UINT8 DevAddress; + BOOLEAN ResetStarted; - OldTpl = gBS->RaiseTPL (USB_BUS_TPL); + ResetStarted = FALSE; + OldTpl = gBS->RaiseTPL (USB_BUS_TPL); UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; + if (Dev->ResetInProgress) { + Status = EFI_NOT_READY; + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: reset already active\n")); + goto ON_EXIT; + } + // MU_CHANGE [BEGIN] if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; @@ -937,8 +1184,10 @@ UsbIoPortReset ( goto ON_EXIT; } - HubIf = Dev->ParentIf; - Status = HubIf->HubApi->ResetPort (HubIf, Dev->ParentPort); + Dev->ResetInProgress = TRUE; + ResetStarted = TRUE; + HubIf = Dev->ParentIf; + Status = HubIf->HubApi->ResetPort (HubIf, Dev->ParentPort); if (EFI_ERROR (Status)) { DEBUG (( @@ -982,59 +1231,28 @@ UsbIoPortReset ( DEBUG ((DEBUG_INFO, "UsbIoPortReset: device is now ADDRESSED at %d\n", Dev->Address)); - // MU_CHANGE [BEGIN] 291137 - // - // Endpoint descriptor state needs to be updated following a reset. - // - UsbUpdateDescriptors (Dev); - // MU_CHANGE [END] - // // Reset the current active configure, after this device // is in CONFIGURED state. // if (Dev->ActiveConfig != NULL) { - Status = UsbGetMaxPacketSize0 (Dev); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to get max packet size - %r\n", Status)); - } - - // Expect the Device Descriptor should not be changed. - // UEFI spec 2.11 Ch. 17.2.18 EFI_USB_IO_PROTOCOL.UsbPortReset - // This Reset function does not change the bus topology. - - // Save the device descriptor before resetting and then restore it after build descriptor table - DevDesc = Dev->DevDesc; - Dev->DevDesc = NULL; - - Status = UsbBuildDescTable (Dev); - - if (Dev->DevDesc != NULL) { - UsbFreeDevDesc (Dev->DevDesc); - } - - Dev->DevDesc = DevDesc; - - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "UsbIoPortReset: Failed to build descriptor table - %r\n", Status)); - goto ON_EXIT; - } - - Config = Dev->DevDesc->Configs[0]->Desc.ConfigurationValue; - - Status = UsbSetConfig (Dev, Config); - if (EFI_ERROR (Status)) { - DEBUG (( - DEBUG_ERROR, - "UsbIoPortReset: failed to set configure for device %d - %r\n", - Dev->Address, - Status - )); - } + Status = UsbResetConfig (Dev, (BOOLEAN)(OldTpl <= TPL_CALLBACK)); + } else { + // MU_CHANGE [BEGIN] 291137 + UsbUpdateDescriptors (Dev); + // MU_CHANGE [END] } ON_EXIT: + if (ResetStarted && EFI_ERROR (Status)) { + Dev->Connected = FALSE; + } + gBS->RestoreTPL (OldTpl); + if (ResetStarted) { + Dev->ResetInProgress = FALSE; + } + return Status; } diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h index d775ca1042..3443e1d347 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h @@ -206,6 +206,8 @@ struct _USB_DEVICE { UINT8 Tier; BOOLEAN Connected; // MU_CHANGE BOOLEAN DisconnectFail; + BOOLEAN ResetInProgress; + BOOLEAN BindingInProgress; UINT8 EnumScript; BOOLEAN IsSSDev; }; @@ -663,6 +665,10 @@ UsbIoGetStringDescriptor ( /** Reset the device, then if that succeeds, reconfigure the device with its address and current active configuration. + Changed personalities disconnect and uninstall all old interfaces before + publishing replacements, without connecting drivers. Callers must coordinate + safe teardown, cancel outstanding transfers, and reacquire USB I/O afterward. + Replacement is rejected during bus-owned binding or above TPL_CALLBACK. @param This The USB IO instance. diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c index f401aa3281..9d93c100ed 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c @@ -86,13 +86,17 @@ UsbFreeInterface ( @param Device The device has the interface descriptor. @param IfDesc The interface descriptor. - @return The created USB interface for the descriptor, or NULL. + @param Result The published interface on success. + + @retval EFI_SUCCESS The interface was published. + @retval Others Allocation or protocol installation failed. **/ -USB_INTERFACE * +EFI_STATUS UsbCreateInterface ( IN USB_DEVICE *Device, - IN USB_INTERFACE_DESC *IfDesc + IN USB_INTERFACE_DESC *IfDesc, + OUT USB_INTERFACE **Result ) { USB_DEVICE_PATH UsbNode; @@ -103,7 +107,7 @@ UsbCreateInterface ( UsbIf = AllocateZeroPool (sizeof (USB_INTERFACE)); if (UsbIf == NULL) { - return NULL; + return EFI_OUT_OF_RESOURCES; } UsbIf->Signature = USB_INTERFACE_SIGNATURE; @@ -173,7 +177,8 @@ UsbCreateInterface ( goto ON_ERROR; } - return UsbIf; + *Result = UsbIf; + return EFI_SUCCESS; ON_ERROR: if (UsbIf->DevicePath != NULL) { @@ -181,7 +186,7 @@ UsbCreateInterface ( } FreePool (UsbIf); - return NULL; + return Status; } /** @@ -260,7 +265,13 @@ UsbConnectDriver ( EFI_STATUS Status; EFI_TPL OldTpl; - Status = EFI_SUCCESS; + if (!UsbIf->Device->Connected || UsbIf->Device->ResetInProgress || UsbIf->Device->BindingInProgress) { + DEBUG ((DEBUG_INFO, "UsbConnectDriver: device offline or reset/binding active\n")); + return EFI_NOT_READY; + } + + UsbIf->Device->BindingInProgress = TRUE; + Status = EFI_SUCCESS; // // Hub is maintained by the USB bus driver. Otherwise try to @@ -285,18 +296,23 @@ UsbConnectDriver ( OldTpl = UsbGetCurrentTpl (); DEBUG ((DEBUG_INFO, "UsbConnectDriver: TPL before connect is %d, %p\n", (UINT32)OldTpl, UsbIf->Handle)); - gBS->RestoreTPL (TPL_CALLBACK); + if (OldTpl > TPL_CALLBACK) { + gBS->RestoreTPL (TPL_CALLBACK); + } Status = gBS->ConnectController (UsbIf->Handle, NULL, NULL, TRUE); UsbIf->IsManaged = (BOOLEAN) !EFI_ERROR (Status); DEBUG ((DEBUG_INFO, "UsbConnectDriver: TPL after connect is %d\n", (UINT32)UsbGetCurrentTpl ())); - ASSERT (UsbGetCurrentTpl () == TPL_CALLBACK); + ASSERT (UsbGetCurrentTpl () == ((OldTpl > TPL_CALLBACK) ? TPL_CALLBACK : OldTpl)); - gBS->RaiseTPL (OldTpl); + if (OldTpl > TPL_CALLBACK) { + gBS->RaiseTPL (OldTpl); + } } } + UsbIf->Device->BindingInProgress = FALSE; return Status; } @@ -366,6 +382,7 @@ UsbSelectSetting ( @param Device The device to select configuration. @param ConfigValue The index of the configuration ( != 0). + @param ConnectDrivers Whether to connect function drivers. @retval EFI_NOT_FOUND There is no configuration with the index. @retval EFI_OUT_OF_RESOURCES Failed to allocate resource. @@ -375,7 +392,8 @@ UsbSelectSetting ( EFI_STATUS UsbSelectConfig ( IN USB_DEVICE *Device, - IN UINT8 ConfigValue + IN UINT8 ConfigValue, + IN BOOLEAN ConnectDrivers ) { USB_DEVICE_DESC *DevDesc; @@ -403,6 +421,11 @@ UsbSelectConfig ( return EFI_NOT_FOUND; } + if (ConfigDesc->Desc.NumInterfaces > USB_MAX_INTERFACE) { + DEBUG ((DEBUG_ERROR, "UsbSelectConfig: too many interfaces\n")); + return EFI_UNSUPPORTED; + } + Device->ActiveConfig = ConfigDesc; DEBUG (( @@ -415,26 +438,36 @@ UsbSelectConfig ( // // Create interfaces for each USB interface descriptor. // - for (Index = 0; Index < ConfigDesc->Desc.NumInterfaces; Index++) { + for (Index = 0; Index < Device->ActiveConfig->Desc.NumInterfaces; Index++) { + // + // A driver's Start() may reset the device and replace its descriptors. + // + ConfigDesc = Device->ActiveConfig; + // // First select the default interface setting, and reset // the endpoint toggles to zero for its endpoints. // IfDesc = ConfigDesc->Interfaces[Index]; - UsbSelectSetting (IfDesc, IfDesc->Settings[0]->Desc.AlternateSetting); + UsbSelectSetting (IfDesc, IfDesc->Settings[IfDesc->ActiveIndex]->Desc.AlternateSetting); // // Create a USB_INTERFACE and install USB_IO and other protocols // - UsbIf = UsbCreateInterface (Device, ConfigDesc->Interfaces[Index]); + Status = UsbCreateInterface (Device, ConfigDesc->Interfaces[Index], &UsbIf); - if (UsbIf == NULL) { + if (EFI_ERROR (Status)) { Device->NumOfInterface = Index; - return EFI_OUT_OF_RESOURCES; + return Status; } ASSERT (Index < USB_MAX_INTERFACE); Device->Interfaces[Index] = UsbIf; + Device->NumOfInterface = (UINT8)(Index + 1); + + if (!ConnectDrivers) { + continue; + } // // Connect the device to drivers, if it failed, ignore @@ -450,6 +483,11 @@ UsbSelectConfig ( Status )); } + + if (!Device->Connected) { + DEBUG ((DEBUG_ERROR, "UsbSelectConfig: device went offline during driver binding\n")); + return EFI_DEVICE_ERROR; + } } Device->NumOfInterface = Index; @@ -478,7 +516,7 @@ UsbDisconnectDriver ( Status = EFI_SUCCESS; if (UsbIf->IsHub) { Status = UsbIf->HubApi->Release (UsbIf); - } else if (UsbIf->IsManaged) { + } else { // // This function is called in both UsbIoControlTransfer and // the timer callback in hub enumeration. So, at least it is @@ -489,7 +527,9 @@ UsbDisconnectDriver ( OldTpl = UsbGetCurrentTpl (); DEBUG ((DEBUG_INFO, "UsbDisconnectDriver: old TPL is %d, %p\n", (UINT32)OldTpl, UsbIf->Handle)); - gBS->RestoreTPL (TPL_CALLBACK); + if (OldTpl > TPL_CALLBACK) { + gBS->RestoreTPL (TPL_CALLBACK); + } Status = gBS->DisconnectController (UsbIf->Handle, NULL, NULL); if (!EFI_ERROR (Status)) { @@ -497,9 +537,11 @@ UsbDisconnectDriver ( } DEBUG ((DEBUG_INFO, "UsbDisconnectDriver: TPL after disconnect is %d, %d\n", (UINT32)UsbGetCurrentTpl (), Status)); - ASSERT (UsbGetCurrentTpl () == TPL_CALLBACK); + ASSERT (UsbGetCurrentTpl () == ((OldTpl > TPL_CALLBACK) ? TPL_CALLBACK : OldTpl)); - gBS->RaiseTPL (OldTpl); + if (OldTpl > TPL_CALLBACK) { + gBS->RaiseTPL (OldTpl); + } } return Status; @@ -536,7 +578,7 @@ UsbRemoveConfig ( Status = UsbDisconnectDriver (UsbIf); if (!EFI_ERROR (Status)) { Status = UsbFreeInterface (UsbIf); - if (EFI_ERROR (Status)) { + if (EFI_ERROR (Status) && Device->Connected && !Device->ResetInProgress) { UsbConnectDriver (UsbIf); } } @@ -548,7 +590,11 @@ UsbRemoveConfig ( } } - Device->ActiveConfig = NULL; + if (!EFI_ERROR (ReturnStatus)) { + Device->ActiveConfig = NULL; + Device->NumOfInterface = 0; + } + return ReturnStatus; } @@ -571,6 +617,11 @@ UsbRemoveDevice ( EFI_STATUS ReturnStatus; UINTN Index; + if (Device->ResetInProgress || Device->BindingInProgress) { + DEBUG ((DEBUG_INFO, "UsbRemoveDevice: reset or binding active\n")); + return EFI_NOT_READY; + } + Bus = Device->Bus; // @@ -892,7 +943,7 @@ UsbEnumerateNewDev ( // // Host assigns and loads a device driver. // - Status = UsbSelectConfig (Child, Config); + Status = UsbSelectConfig (Child, Config, TRUE); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "UsbEnumerateNewDev: failed to create interfaces - %r\n", Status)); @@ -914,7 +965,14 @@ UsbEnumerateNewDev ( // Do the error handling with retry counter. // OriginalStatus = Status; - Status = HubApi->GetPortStatus (HubIf, Port, &PortState); + if ((Child != NULL) && (Child->NumOfInterface != 0)) { + // + // Installed interfaces still reference these descriptors. + // + return OriginalStatus; + } + + Status = HubApi->GetPortStatus (HubIf, Port, &PortState); if (EFI_ERROR (Status) || (!USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION))) { DEBUG ((DEBUG_ERROR, "Device is gone. Don't reset the port\n")); Status = EFI_NOT_FOUND; @@ -974,6 +1032,8 @@ UsbEnumeratePort ( EFI_USB_PORT_STATUS PortState; EFI_STATUS Status; + EFI_STATUS RemoveStatus; + Child = NULL; HubApi = HubIf->HubApi; @@ -1061,7 +1121,11 @@ UsbEnumeratePort ( if (Child != NULL) { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: device at port %d removed from root hub %p\n", Port, HubIf)); - UsbRemoveDevice (Child); + RemoveStatus = UsbRemoveDevice (Child); + if (EFI_ERROR (RemoveStatus)) { + DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: old device removal failed - %r\n", RemoveStatus)); + return RemoveStatus; + } } if (USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION)) { diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h index 24876ae842..47e0c612aa 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h @@ -123,6 +123,7 @@ UsbSelectSetting ( @param Device The device to select configuration. @param ConfigIndex The index of the configuration ( != 0). + @param ConnectDrivers Whether to connect function drivers. @retval EFI_NOT_FOUND There is no configuration with the index. @retval EFI_OUT_OF_RESOURCES Failed to allocate resource. @@ -132,7 +133,16 @@ UsbSelectSetting ( EFI_STATUS UsbSelectConfig ( IN USB_DEVICE *Device, - IN UINT8 ConfigIndex + IN UINT8 ConfigIndex, + IN BOOLEAN ConnectDrivers + ); + +/** + Connect an interface, guarding bus-owned driver binding against reset teardown. +**/ +EFI_STATUS +UsbConnectDriver ( + IN USB_INTERFACE *UsbIf ); /** diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c index 61cddf0c82..6734f43183 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c @@ -1100,8 +1100,7 @@ UsbBusRecursivelyConnectWantedUsbIo ( // Recursively connect the wanted Usb Io handle // DEBUG ((DEBUG_INFO, "UsbBusRecursivelyConnectWantedUsbIo: TPL before connect is %d\n", (UINT32)UsbGetCurrentTpl ())); - Status = gBS->ConnectController (UsbIf->Handle, NULL, NULL, TRUE); - UsbIf->IsManaged = (BOOLEAN) !EFI_ERROR (Status); + Status = UsbConnectDriver (UsbIf); DEBUG ((DEBUG_INFO, "UsbBusRecursivelyConnectWantedUsbIo: TPL after connect is %d\n", (UINT32)UsbGetCurrentTpl ())); } } diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc index 90b9533b8f..a7ea94f049 100644 --- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc +++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc @@ -24,6 +24,13 @@ SecurityLockAuditLib|MdeModulePkg/Library/SecurityLockAuditLibNull/SecurityLockAuditLibNull.inf # MU_CHANGE [Components] + MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.inf { + + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf + UefiLib|MdePkg/Test/Mock/Library/Stub/StubUefiLib/StubUefiLib.inf + ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf + } + # MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServicesTableLib.inf # MU_CHANGE - Move lib to correct home #