From 8587aabfa18217e6f6621267f30028797fbb299e Mon Sep 17 00:00:00 2001 From: Aaron Pop Date: Mon, 26 Oct 2026 12:52:19 -0700 Subject: [PATCH 1/4] test --- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 204 +++++++++++++++------ MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 9 +- 2 files changed, 159 insertions(+), 54 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c index 0c153bca96..9b44f3aad5 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c @@ -895,9 +895,150 @@ UsbIoGetStringDescriptor ( return Status; } +/** + Refresh the active configuration without replacing USB I/O handles or + connecting drivers. Interface numbers must remain unchanged. + + @param Dev The device to reconfigure. + + @retval EFI_SUCCESS The configuration and descriptors are refreshed. + @retval EFI_DEVICE_ERROR The new configuration cannot use the existing handles. + @retval Others Failed to read descriptors or configure the device. + +**/ +STATIC +EFI_STATUS +UsbResetConfig ( + IN USB_DEVICE *Dev + ) +{ + 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; + + // + // 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; + } + + 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; + } + } + + Status = EFI_DEVICE_ERROR; + if ((ConfigDesc == NULL) || + (ConfigDesc->Desc.NumInterfaces != Dev->ActiveConfig->Desc.NumInterfaces) || + (ConfigDesc->Desc.NumInterfaces > USB_MAX_INTERFACE)) + { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: incompatible configuration after reset\n")); + goto ON_ERROR; + } + + 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; + } + + UsbIf = Dev->Interfaces[Index]; + if ((UsbIf != NULL) && + (UsbIf->IsHub || (UsbIf->IfSetting->Desc.InterfaceNumber != Index))) + { + DEBUG ((DEBUG_ERROR, "UsbResetConfig: cannot preserve interface %d\n", (UINT32)Index)); + goto ON_ERROR; + } + + IfDesc->ActiveIndex = SettingIndex; + } + + 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; + 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); + 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 descriptors in place when the interface layout is compatible. + A changed interface layout requires re-enumeration instead. @param This The USB IO instance. @@ -911,14 +1052,12 @@ 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; OldTpl = gBS->RaiseTPL (USB_BUS_TPL); @@ -982,55 +1121,16 @@ 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); + } else { + // MU_CHANGE [BEGIN] 291137 + UsbUpdateDescriptors (Dev); + // MU_CHANGE [END] } ON_EXIT: diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c index f401aa3281..0f39d8e733 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c @@ -415,13 +415,18 @@ 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 From ec49220177496a6b6c4a488c867a460a56cce185 Mon Sep 17 00:00:00 2001 From: Aaron Pop Date: Tue, 27 Oct 2026 14:39:31 -0700 Subject: [PATCH 2/4] Attempt #2 --- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 322 +++++++++++++-- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h | 34 +- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 416 ++++++++++++++++++-- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h | 73 +++- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c | 7 +- 5 files changed, 784 insertions(+), 68 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c index 9b44f3aad5..920d4c7a85 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c @@ -82,7 +82,7 @@ UsbIoControlTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoControlTransfer No media\n")); goto ON_EXIT; @@ -188,11 +188,20 @@ 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)) { + Dev->Connected = FALSE; + if (!Dev->Bus->RecoveryInProgress) { + Dev->Reenumerate = TRUE; + } + + DEBUG ((DEBUG_ERROR, "UsbIoControlTransfer: failed to remove old configuration - %r\n", Status)); + goto ON_EXIT; + } } if (Request->Value != 0) { - Status = UsbSelectConfig (Dev, (UINT8)Request->Value); + Status = UsbSelectConfig (Dev, (UINT8)Request->Value, TRUE); } // @@ -274,7 +283,7 @@ UsbIoBulkTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoBulkTransfer No media\n")); goto ON_EXIT; @@ -374,7 +383,7 @@ UsbIoSyncInterruptTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoSyncInterruptTransfer No media\n")); goto ON_EXIT; @@ -410,6 +419,35 @@ UsbIoSyncInterruptTransfer ( return Status; } +/** + Pin device and request state across a callback, including self-cancellation. +**/ +STATIC +EFI_STATUS +EFIAPI +UsbAsyncInterruptCallback ( + IN VOID *Data, + IN UINTN DataLength, + IN VOID *Context, + IN UINT32 Result + ) +{ + USB_ASYNC_REQUEST *Request; + EFI_STATUS Status; + + Request = Context; + Request->InCallback++; + Request->Device->AsyncCallbacks++; + Status = Request->Callback (Data, DataLength, Request->Context, Result); + Request->Device->AsyncCallbacks--; + Request->InCallback--; + if (Request->Cancelled && (Request->InCallback == 0)) { + FreePool (Request); + } + + return Status; +} + /** Queue a new asynchronous interrupt transfer, or remove the old request if (IsNewTransfer == FALSE). @@ -448,6 +486,8 @@ UsbIoAsyncInterruptTransfer ( EFI_TPL OldTpl; UINT8 Toggle; EFI_STATUS Status; + USB_ASYNC_REQUEST *Request; + UINTN AsyncIndex; if ((USB_ENDPOINT_ADDR (Endpoint) == 0) || (USB_ENDPOINT_ADDR (Endpoint) > 15)) { return EFI_INVALID_PARAMETER; @@ -457,20 +497,44 @@ UsbIoAsyncInterruptTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if ((Dev->Connected == FALSE) && (IsNewTransfer == TRUE)) { + if (((Dev->Connected == FALSE) || UsbIf->Disconnected) && (IsNewTransfer == TRUE)) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoAsyncInterruptTransfer No media\n")); goto ON_EXIT; } // MU_CHANGE [END] - EpDesc = UsbGetEndpointDesc (UsbIf, Endpoint); + AsyncIndex = USB_ENDPOINT_ADDR (Endpoint) + (((Endpoint & 0x80) != 0) ? 16 : 0); + Request = UsbIf->AsyncTransfers[AsyncIndex]; + if (!IsNewTransfer && (Request != NULL)) { + EpDesc = &Request->Endpoint; + } else { + EpDesc = UsbGetEndpointDesc (UsbIf, Endpoint); + } if ((EpDesc == NULL) || (USB_ENDPOINT_TYPE (&EpDesc->Desc) != USB_ENDPOINT_INTERRUPT)) { Status = EFI_INVALID_PARAMETER; goto ON_EXIT; } + if (IsNewTransfer) { + if ((Request != NULL) || (Callback == NULL)) { + Status = EFI_INVALID_PARAMETER; + goto ON_EXIT; + } + + Request = AllocateZeroPool (sizeof (*Request)); + if (Request == NULL) { + Status = EFI_OUT_OF_RESOURCES; + goto ON_EXIT; + } + + Request->Device = Dev; + Request->Endpoint = *EpDesc; + Request->Callback = Callback; + Request->Context = Context; + } + Toggle = EpDesc->Toggle; Status = UsbHcAsyncInterruptTransfer ( Dev->Bus, @@ -483,11 +547,25 @@ UsbIoAsyncInterruptTransfer ( PollInterval, DataLength, &Dev->Translator, - Callback, - Context + IsNewTransfer ? UsbAsyncInterruptCallback : NULL, + IsNewTransfer ? Request : NULL ); EpDesc->Toggle = Toggle; + if (IsNewTransfer) { + if (EFI_ERROR (Status)) { + FreePool (Request); + } else { + Request->Endpoint.Toggle = Toggle; + UsbIf->AsyncTransfers[AsyncIndex] = Request; + } + } else if (!EFI_ERROR (Status) && (Request != NULL)) { + UsbIf->AsyncTransfers[AsyncIndex] = NULL; + Request->Cancelled = TRUE; + if (Request->InCallback == 0) { + FreePool (Request); + } + } ON_EXIT: gBS->RestoreTPL (OldTpl); @@ -580,7 +658,7 @@ UsbIoGetDeviceDescriptor ( // MU_CHANGE [BEGIN] Status = EFI_SUCCESS; - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -627,7 +705,7 @@ UsbIoGetActiveConfigDescriptor ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -678,7 +756,7 @@ UsbIoGetInterfaceDescriptor ( Dev = UsbIf->Device; Status = EFI_SUCCESS; - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -724,7 +802,7 @@ UsbIoGetEndpointDescriptor ( Dev = UsbIf->Device; Status = EFI_SUCCESS; - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -786,7 +864,7 @@ UsbIoGetSupportedLanguages ( // MU_CHANGE [BEGIN] Status = EFI_SUCCESS; - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -839,7 +917,7 @@ UsbIoGetStringDescriptor ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -896,13 +974,67 @@ UsbIoGetStringDescriptor ( } /** - Refresh the active configuration without replacing USB I/O handles or - connecting drivers. Interface numbers must remain unchanged. + Compare default interface layouts before reusing a USB I/O instance. +**/ +STATIC +BOOLEAN +UsbInterfaceIsCompatible ( + IN USB_INTERFACE *UsbIf, + IN USB_INTERFACE_DESC *IfDesc + ) +{ + USB_INTERFACE_SETTING *OldSetting; + USB_INTERFACE_SETTING *Setting; + UINTN Index; + + if (UsbIf->Disconnected || 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; +} + +/** + Reconcile fresh descriptors with existing USB I/O instances without binding. + Compatible interfaces survive; incompatible interfaces stay offline until + their owners release them and they can be safely retired. @param Dev The device to reconfigure. @retval EFI_SUCCESS The configuration and descriptors are refreshed. - @retval EFI_DEVICE_ERROR The new configuration cannot use the existing handles. + @retval EFI_DEVICE_ERROR The new personality could not be synchronized. @retval Others Failed to read descriptors or configure the device. **/ @@ -922,7 +1054,12 @@ UsbResetConfig ( UINTN Index; UINTN SettingIndex; UINT8 Config; + BOOLEAN IdentityCompatible; + BOOLEAN KeepInterface[USB_MAX_INTERFACE]; + USB_RETIRED_CONFIG *Retired; + Retired = NULL; + ZeroMem (KeepInterface, sizeof (KeepInterface)); // // Build into a temporary device so failures leave all cached pointers valid. // @@ -944,6 +1081,24 @@ UsbResetConfig ( goto ON_ERROR; } + DEBUG (( + DEBUG_INFO, + "UsbResetConfig: device %d identity %04x:%04x -> %04x:%04x\n", + Dev->Address, + Dev->DevDesc->Desc.IdVendor, + Dev->DevDesc->Desc.IdProduct, + NewDev.DevDesc->Desc.IdVendor, + NewDev.DevDesc->Desc.IdProduct + )); + + IdentityCompatible = (BOOLEAN)( + (NewDev.DevDesc->Desc.IdVendor == Dev->DevDesc->Desc.IdVendor) && + (NewDev.DevDesc->Desc.IdProduct == Dev->DevDesc->Desc.IdProduct) && + (NewDev.DevDesc->Desc.DeviceClass == Dev->DevDesc->Desc.DeviceClass) && + (NewDev.DevDesc->Desc.DeviceSubClass == Dev->DevDesc->Desc.DeviceSubClass) && + (NewDev.DevDesc->Desc.DeviceProtocol == Dev->DevDesc->Desc.DeviceProtocol) + ); + Config = Dev->ActiveConfig->Desc.ConfigurationValue; ConfigDesc = NULL; for (Index = 0; Index < NewDev.DevDesc->Desc.NumConfigurations; Index++) { @@ -955,15 +1110,22 @@ UsbResetConfig ( } } + if ((ConfigDesc == NULL) && (NewDev.DevDesc->Desc.NumConfigurations != 0)) { + ConfigDesc = NewDev.DevDesc->Configs[0]; + } + Status = EFI_DEVICE_ERROR; if ((ConfigDesc == NULL) || - (ConfigDesc->Desc.NumInterfaces != Dev->ActiveConfig->Desc.NumInterfaces) || + (ConfigDesc->Desc.NumInterfaces == 0) || (ConfigDesc->Desc.NumInterfaces > USB_MAX_INTERFACE)) { - DEBUG ((DEBUG_ERROR, "UsbResetConfig: incompatible configuration after reset\n")); + DEBUG ((DEBUG_ERROR, "UsbResetConfig: invalid configuration after reset\n")); goto ON_ERROR; } + IdentityCompatible = (BOOLEAN)(IdentityCompatible && (ConfigDesc->Desc.ConfigurationValue == Config)); + Config = ConfigDesc->Desc.ConfigurationValue; + for (Index = 0; Index < ConfigDesc->Desc.NumInterfaces; Index++) { IfDesc = ConfigDesc->Interfaces[Index]; if (IfDesc == NULL) { @@ -990,15 +1152,25 @@ UsbResetConfig ( goto ON_ERROR; } - UsbIf = Dev->Interfaces[Index]; - if ((UsbIf != NULL) && - (UsbIf->IsHub || (UsbIf->IfSetting->Desc.InterfaceNumber != Index))) - { - DEBUG ((DEBUG_ERROR, "UsbResetConfig: cannot preserve interface %d\n", (UINT32)Index)); - goto ON_ERROR; + IfDesc->ActiveIndex = SettingIndex; + UsbIf = Dev->Interfaces[Index]; + if (UsbIf != NULL) { + KeepInterface[Index] = (BOOLEAN)(IdentityCompatible && UsbInterfaceIsCompatible (UsbIf, IfDesc)); } + } - IfDesc->ActiveIndex = SettingIndex; + for (Index = 0; Index < Dev->NumOfInterface; Index++) { + UsbIf = Dev->Interfaces[Index]; + if ((UsbIf != NULL) && !KeepInterface[Index]) { + Retired = AllocateZeroPool (sizeof (*Retired)); + if (Retired == NULL) { + Status = EFI_OUT_OF_RESOURCES; + DEBUG ((DEBUG_ERROR, "UsbResetConfig: failed to retain old interfaces\n")); + goto ON_ERROR; + } + + break; + } } Status = UsbSetConfig (&NewDev, Config); @@ -1007,8 +1179,26 @@ UsbResetConfig ( goto ON_ERROR; } + if (Retired != NULL) { + Retired->DevDesc = Dev->DevDesc; + Retired->NumOfInterface = Dev->NumOfInterface; + for (Index = 0; Index < Dev->NumOfInterface; Index++) { + UsbIf = Dev->Interfaces[Index]; + if ((UsbIf != NULL) && !KeepInterface[Index]) { + UsbIf->Disconnected = TRUE; + Retired->Interfaces[Index] = UsbIf; + Dev->Interfaces[Index] = NULL; + DEBUG ((DEBUG_INFO, "UsbResetConfig: retained disconnected interface %d\n", (UINT32)Index)); + } + } + + Retired->Next = Dev->RetiredConfigs; + Dev->RetiredConfigs = Retired; + } + OldDesc = Dev->DevDesc; Dev->DevDesc = NewDev.DevDesc; + NewDev.DevDesc = NULL; Dev->ActiveConfig = ConfigDesc; Dev->MaxPacket0 = NewDev.MaxPacket0; Dev->IsSSDev = NewDev.IsSSDev; @@ -1023,10 +1213,18 @@ UsbResetConfig ( } } - UsbFreeDevDesc (OldDesc); - return EFI_SUCCESS; + if (Retired == NULL) { + UsbFreeDevDesc (OldDesc); + } + + Dev->NumOfInterface = ConfigDesc->Desc.NumInterfaces; + return UsbSelectConfig (Dev, Config, FALSE); ON_ERROR: + if (Retired != NULL) { + FreePool (Retired); + } + if (NewDev.DevDesc != NULL) { UsbFreeDevDesc (NewDev.DevDesc); } @@ -1037,13 +1235,19 @@ UsbResetConfig ( /** Reset the device, then if that succeeds, reconfigure the device with its address and current active configuration. - Refresh descriptors in place when the interface layout is compatible. - A changed interface layout requires re-enumeration instead. + Refresh compatible interface instances and publish missing ones. Keep + incompatible instances disconnected until their consumers release them. + Once a port reset is attempted, failures defer replacement of the device's + interfaces to bus enumeration unless recovery is already in progress. + Personality replacement publishes fresh USB I/O interfaces synchronously + without connecting drivers or stopping old consumers. Callers must acquire + fresh USB I/O after replacement. @param This The USB IO instance. @retval EFI_SUCCESS The device is reset and configured. - @retval Others Failed to reset the device. + @retval EFI_NOT_READY A reset is already active. + @retval Others Failed to reset the device. Recovery may be pending. **/ EFI_STATUS @@ -1058,13 +1262,29 @@ UsbIoPortReset ( 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; + DEBUG (( + DEBUG_INFO, + "UsbIoPortReset: device %d interface %d caller TPL %d\n", + Dev->Address, + UsbIf->IfSetting->Desc.InterfaceNumber, + (UINT32)OldTpl + )); + + if (Dev->ResetInProgress) { + Status = EFI_NOT_READY; + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: reset already in progress for device %d\n", Dev->Address)); + goto ON_EXIT; + } + // MU_CHANGE [BEGIN] - if (Dev->Connected == FALSE) { + if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoPortReset No media\n")); goto ON_EXIT; @@ -1076,8 +1296,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 (( @@ -1088,7 +1310,7 @@ UsbIoPortReset ( Status )); - goto ON_EXIT; + goto RECOVER; } HubIf->HubApi->ClearPortChange (HubIf, Dev->ParentPort); @@ -1116,7 +1338,7 @@ UsbIoPortReset ( Status )); - goto ON_EXIT; + goto RECOVER; } DEBUG ((DEBUG_INFO, "UsbIoPortReset: device is now ADDRESSED at %d\n", Dev->Address)); @@ -1133,8 +1355,32 @@ UsbIoPortReset ( // MU_CHANGE [END] } + if (!EFI_ERROR (Status)) { + goto ON_EXIT; + } + +RECOVER: + // + // Keep old descriptors alive for driver teardown and transfer cancellation. + // A reset from a recovery-started driver must not create a reconnect loop. + // + Dev->Connected = FALSE; + if (!Dev->Bus->RecoveryInProgress) { + Dev->Reenumerate = TRUE; + DEBUG ((DEBUG_INFO, "UsbIoPortReset: queued recovery for device %d - %r\n", Dev->Address, Status)); + } else { + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: reset failed during recovery for device %d - %r\n", Dev->Address, Status)); + } + ON_EXIT: + // + // Restoring TPL may dispatch enumeration before this function returns. + // 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..ccb2ace283 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h @@ -206,10 +206,31 @@ struct _USB_DEVICE { UINT8 Tier; BOOLEAN Connected; // MU_CHANGE BOOLEAN DisconnectFail; + BOOLEAN ResetInProgress; + BOOLEAN Reenumerate; + UINTN BindingInProgress; + UINTN AsyncCallbacks; + struct _USB_RETIRED_CONFIG *RetiredConfigs; UINT8 EnumScript; BOOLEAN IsSSDev; }; +typedef struct _USB_RETIRED_CONFIG { + struct _USB_RETIRED_CONFIG *Next; + USB_DEVICE_DESC *DevDesc; + UINT8 NumOfInterface; + USB_INTERFACE *Interfaces[USB_MAX_INTERFACE]; +} USB_RETIRED_CONFIG; + +typedef struct { + USB_DEVICE *Device; + USB_ENDPOINT_DESC Endpoint; + EFI_ASYNC_USB_TRANSFER_CALLBACK Callback; + VOID *Context; + UINTN InCallback; + BOOLEAN Cancelled; +} USB_ASYNC_REQUEST; + // // Stands for different functions of USB device // @@ -226,6 +247,8 @@ struct _USB_INTERFACE { EFI_USB_IO_PROTOCOL UsbIo; EFI_DEVICE_PATH_PROTOCOL *DevicePath; BOOLEAN IsManaged; + BOOLEAN Disconnected; + USB_ASYNC_REQUEST *AsyncTransfers[32]; // // Hub device special data @@ -280,6 +303,7 @@ struct _USB_BUS { // for root hub. Device with address i is at Devices[i]. // USB_DEVICE *Devices[256]; + BOOLEAN RecoveryInProgress; // // USB Bus driver need to control the recursive connect policy of the bus, only those wanted @@ -663,11 +687,19 @@ UsbIoGetStringDescriptor ( /** Reset the device, then if that succeeds, reconfigure the device with its address and current active configuration. + Compatible interfaces retain existing handles even when the layout changes. + Incompatible instances are marked offline and retired only after consumers + release ownership and asynchronous work. Missing interfaces are published + immediately on the same device without stopping old drivers or binding new ones. + Failures schedule deferred recovery. + Callers must acquire fresh USB I/O after replacement. Driver binding during + recovery cannot schedule another recovery. @param This The USB IO instance. @retval EFI_SUCCESS The device is reset and configured. - @retval Others Failed to reset the device. + @retval EFI_NOT_READY A reset is already active. + @retval Others Failed to reset the device. Recovery may be pending. **/ EFI_STATUS diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c index 0f39d8e733..22961e2b27 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c @@ -55,6 +55,18 @@ UsbFreeInterface ( ) { EFI_STATUS Status; + UINTN Index; + + if (UsbIf->Device->AsyncCallbacks != 0) { + return EFI_NOT_READY; + } + + for (Index = 0; Index < ARRAY_SIZE (UsbIf->AsyncTransfers); Index++) { + if (UsbIf->AsyncTransfers[Index] != NULL) { + DEBUG ((DEBUG_ERROR, "UsbFreeInterface: asynchronous transfer is still queued\n")); + return EFI_NOT_READY; + } + } UsbCloseHostProtoByChild (UsbIf->Device->Bus, UsbIf->Handle); @@ -173,6 +185,17 @@ UsbCreateInterface ( goto ON_ERROR; } + DEBUG (( + DEBUG_INFO, + "UsbCreateInterface: published %04x:%04x interface %d at port %d@hub %d, handle %p\n", + Device->DevDesc->Desc.IdVendor, + Device->DevDesc->Desc.IdProduct, + UsbIf->IfSetting->Desc.InterfaceNumber, + Device->ParentPort, + Device->ParentAddr, + UsbIf->Handle + )); + return UsbIf; ON_ERROR: @@ -195,6 +218,8 @@ UsbFreeDevice ( IN USB_DEVICE *Device ) { + ASSERT (Device->RetiredConfigs == NULL); + if (Device->DevDesc != NULL) { UsbFreeDevDesc (Device->DevDesc); } @@ -260,6 +285,17 @@ UsbConnectDriver ( EFI_STATUS Status; EFI_TPL OldTpl; + if (UsbIf->Device->ResetInProgress || (UsbIf->Device->BindingInProgress != 0)) { + DEBUG ((DEBUG_INFO, "UsbConnectDriver: reset or binding is in progress\n")); + return EFI_NOT_READY; + } + + if (!UsbIf->Device->Connected || UsbIf->Disconnected) { + DEBUG ((DEBUG_ERROR, "UsbConnectDriver: device %d is offline\n", UsbIf->Device->Address)); + return EFI_DEVICE_ERROR; + } + + UsbIf->Device->BindingInProgress++; Status = EFI_SUCCESS; // @@ -285,18 +321,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--; return Status; } @@ -366,6 +407,7 @@ UsbSelectSetting ( @param Device The device to select configuration. @param ConfigValue The index of the configuration ( != 0). + @param ConnectDrivers Whether to connect function drivers immediately. @retval EFI_NOT_FOUND There is no configuration with the index. @retval EFI_OUT_OF_RESOURCES Failed to allocate resource. @@ -375,7 +417,8 @@ UsbSelectSetting ( EFI_STATUS UsbSelectConfig ( IN USB_DEVICE *Device, - IN UINT8 ConfigValue + IN UINT8 ConfigValue, + IN BOOLEAN ConnectDrivers ) { USB_DEVICE_DESC *DevDesc; @@ -403,6 +446,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 (( @@ -426,20 +474,37 @@ UsbSelectConfig ( // the endpoint toggles to zero for its endpoints. // IfDesc = ConfigDesc->Interfaces[Index]; - UsbSelectSetting (IfDesc, IfDesc->Settings[IfDesc->ActiveIndex]->Desc.AlternateSetting); + Status = UsbSelectSetting (IfDesc, 0); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbSelectConfig: no default setting for interface %d - %r\n", Index, Status)); + return Status; + } // // Create a USB_INTERFACE and install USB_IO and other protocols // - UsbIf = UsbCreateInterface (Device, ConfigDesc->Interfaces[Index]); - + UsbIf = Device->Interfaces[Index]; if (UsbIf == NULL) { - Device->NumOfInterface = Index; - return EFI_OUT_OF_RESOURCES; + UsbIf = UsbCreateInterface (Device, ConfigDesc->Interfaces[Index]); + if (UsbIf == NULL) { + if (ConnectDrivers) { + Device->NumOfInterface = Index; + } + + return EFI_OUT_OF_RESOURCES; + } + + Device->Interfaces[Index] = UsbIf; } ASSERT (Index < USB_MAX_INTERFACE); - Device->Interfaces[Index] = UsbIf; + if (ConnectDrivers) { + Device->NumOfInterface = (UINT8)(Index + 1); + } + + if (!ConnectDrivers) { + continue; + } // // Connect the device to drivers, if it failed, ignore @@ -455,6 +520,18 @@ UsbSelectConfig ( Status )); } + + if (!Device->Connected) { + DEBUG ((DEBUG_ERROR, "UsbSelectConfig: device went offline during driver binding\n")); + return EFI_DEVICE_ERROR; + } + + if (Device->ActiveConfig != ConfigDesc) { + // + // Reset already published the new configuration without driver binding. + // + return EFI_SUCCESS; + } } Device->NumOfInterface = Index; @@ -466,11 +543,13 @@ UsbSelectConfig ( Disconnect the USB interface with its driver. @param UsbIf The interface to disconnect driver from. + @param Force Disconnect drivers even if not connected by the bus. **/ EFI_STATUS UsbDisconnectDriver ( - IN USB_INTERFACE *UsbIf + IN USB_INTERFACE *UsbIf, + IN BOOLEAN Force ) { EFI_TPL OldTpl; @@ -483,7 +562,7 @@ UsbDisconnectDriver ( Status = EFI_SUCCESS; if (UsbIf->IsHub) { Status = UsbIf->HubApi->Release (UsbIf); - } else if (UsbIf->IsManaged) { + } else if (UsbIf->IsManaged || Force) { // // This function is called in both UsbIoControlTransfer and // the timer callback in hub enumeration. So, at least it is @@ -510,6 +589,186 @@ UsbDisconnectDriver ( return Status; } +/** + Require consumers to release an interface before software-driven replacement. +**/ +EFI_STATUS +UsbInterfaceIsQuiesced ( + IN USB_INTERFACE *UsbIf + ) +{ + EFI_OPEN_PROTOCOL_INFORMATION_ENTRY *Entries; + EFI_GUID *Protocols[2]; + USB_DEVICE *Device; + EFI_STATUS Status; + UINTN ProtocolIndex; + UINTN EntryIndex; + UINTN EntryCount; + UINTN TransferIndex; + + Device = UsbIf->Device; + if ((Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { + DEBUG ((DEBUG_INFO, "UsbInterfaceIsQuiesced: device %d has an active binding or callback\n", Device->Address)); + return EFI_NOT_READY; + } + + if (UsbIf->IsHub) { + DEBUG ((DEBUG_ERROR, "UsbInterfaceIsQuiesced: cannot replace a hub interface\n")); + return EFI_ACCESS_DENIED; + } + + for (TransferIndex = 0; TransferIndex < ARRAY_SIZE (UsbIf->AsyncTransfers); TransferIndex++) { + if (UsbIf->AsyncTransfers[TransferIndex] != NULL) { + DEBUG ((DEBUG_INFO, "UsbInterfaceIsQuiesced: device %d still has asynchronous transfers\n", Device->Address)); + return EFI_NOT_READY; + } + } + + Protocols[0] = &gEfiUsbIoProtocolGuid; + Protocols[1] = &gEfiDevicePathProtocolGuid; + for (ProtocolIndex = 0; ProtocolIndex < ARRAY_SIZE (Protocols); ProtocolIndex++) { + Status = gBS->OpenProtocolInformation (UsbIf->Handle, Protocols[ProtocolIndex], &Entries, &EntryCount); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbInterfaceIsQuiesced: failed to query interface ownership - %r\n", Status)); + return Status; + } + + Status = EFI_SUCCESS; + for (EntryIndex = 0; EntryIndex < EntryCount; EntryIndex++) { + if ((Entries[EntryIndex].Attributes & + (EFI_OPEN_PROTOCOL_BY_DRIVER | EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER | EFI_OPEN_PROTOCOL_EXCLUSIVE)) != 0) + { + Status = EFI_ACCESS_DENIED; + break; + } + } + + if (Entries != NULL) { + FreePool (Entries); + } + + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_INFO, "UsbInterfaceIsQuiesced: device %d interface %d still has an owner\n", Device->Address, UsbIf->IfSetting->Desc.InterfaceNumber)); + return Status; + } + } + + return EFI_SUCCESS; +} + +/** + Check all remaining interfaces before deferred whole-device recovery. +**/ +EFI_STATUS +UsbDeviceIsQuiesced ( + IN USB_DEVICE *Device + ) +{ + EFI_STATUS Status; + UINTN Index; + USB_RETIRED_CONFIG *Retired; + + if ((Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { + return EFI_NOT_READY; + } + + for (Index = 0; Index < Device->NumOfInterface; Index++) { + if (Device->Interfaces[Index] == NULL) { + continue; + } + + Status = UsbInterfaceIsQuiesced (Device->Interfaces[Index]); + if (EFI_ERROR (Status)) { + return Status; + } + } + + for (Retired = Device->RetiredConfigs; Retired != NULL; Retired = Retired->Next) { + for (Index = 0; Index < Retired->NumOfInterface; Index++) { + if (Retired->Interfaces[Index] == NULL) { + continue; + } + + Status = UsbInterfaceIsQuiesced (Retired->Interfaces[Index]); + if (EFI_ERROR (Status)) { + return Status; + } + } + } + + return EFI_SUCCESS; +} + +/** + Remove retained interfaces after their consumers release them, or as part + of normal configuration teardown when DisconnectDrivers is TRUE. +**/ +EFI_STATUS +UsbRemoveRetiredConfigs ( + IN USB_DEVICE *Device, + IN BOOLEAN DisconnectDrivers + ) +{ + USB_RETIRED_CONFIG **Link; + USB_RETIRED_CONFIG *Retired; + USB_INTERFACE *UsbIf; + EFI_STATUS Status; + EFI_STATUS ReturnStatus; + BOOLEAN Empty; + UINTN Index; + + if (Device->ResetInProgress || (Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { + return EFI_NOT_READY; + } + + ReturnStatus = EFI_SUCCESS; + Link = &Device->RetiredConfigs; + while (*Link != NULL) { + Retired = *Link; + Empty = TRUE; + for (Index = 0; Index < Retired->NumOfInterface; Index++) { + UsbIf = Retired->Interfaces[Index]; + if (UsbIf == NULL) { + continue; + } + + Status = DisconnectDrivers ? EFI_SUCCESS : UsbInterfaceIsQuiesced (UsbIf); + if (!EFI_ERROR (Status)) { + // + // Stop() and protocol removal can dispatch notifications. + // + Device->BindingInProgress++; + Status = UsbDisconnectDriver (UsbIf, DisconnectDrivers); + if (!EFI_ERROR (Status)) { + Status = UsbFreeInterface (UsbIf); + } + + Device->BindingInProgress--; + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbRemoveRetiredConfigs: interface teardown failed - %r\n", Status)); + } + } + + if (EFI_ERROR (Status)) { + ReturnStatus = Status; + Empty = FALSE; + } else { + Retired->Interfaces[Index] = NULL; + } + } + + if (Empty) { + *Link = Retired->Next; + UsbFreeDevDesc (Retired->DevDesc); + FreePool (Retired); + } else { + Link = &Retired->Next; + } + } + + return ReturnStatus; +} + /** Remove the current device configuration. @@ -526,6 +785,11 @@ UsbRemoveConfig ( EFI_STATUS Status; EFI_STATUS ReturnStatus; + if (Device->ResetInProgress || (Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { + DEBUG ((DEBUG_ERROR, "UsbRemoveConfig: binding or callback is still executing\n")); + return EFI_NOT_READY; + } + // // Remove each interface of the device // @@ -538,10 +802,15 @@ UsbRemoveConfig ( continue; } - Status = UsbDisconnectDriver (UsbIf); + Status = UsbRemoveRetiredConfigs (Device, TRUE); + if (EFI_ERROR (Status)) { + ReturnStatus = Status; + } + + Status = UsbDisconnectDriver (UsbIf, (BOOLEAN) !Device->Connected); if (!EFI_ERROR (Status)) { Status = UsbFreeInterface (UsbIf); - if (EFI_ERROR (Status)) { + if (EFI_ERROR (Status) && Device->Connected && !UsbIf->Disconnected) { UsbConnectDriver (UsbIf); } } @@ -553,7 +822,10 @@ UsbRemoveConfig ( } } - Device->ActiveConfig = NULL; + if (!EFI_ERROR (ReturnStatus)) { + Device->ActiveConfig = NULL; + } + return ReturnStatus; } @@ -563,6 +835,7 @@ UsbRemoveConfig ( @param Device The device to remove. @retval EFI_SUCCESS The device is removed. + @retval EFI_NOT_READY A reset, binding, or asynchronous callback is active. **/ EFI_STATUS @@ -576,6 +849,11 @@ UsbRemoveDevice ( EFI_STATUS ReturnStatus; UINTN Index; + if (Device->ResetInProgress || (Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { + DEBUG ((DEBUG_ERROR, "UsbRemoveDevice: reset, binding, or callback in progress for device %d\n", Device->Address)); + return EFI_NOT_READY; + } + Bus = Device->Bus; // @@ -897,7 +1175,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)); @@ -919,7 +1197,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 own the descriptor tree. Do not retry in place. + // + 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; @@ -958,7 +1243,8 @@ UsbEnumerateNewDev ( } /** - Process the events on the port. + Process the events on the port, including software-requested reset recovery. + Recovery does not require hardware change bits and always resets the port. @param HubIf The HUB that has the device connected. @param Port The port index of the hub (started with zero). @@ -978,9 +1264,23 @@ UsbEnumeratePort ( USB_DEVICE *Child; EFI_USB_PORT_STATUS PortState; EFI_STATUS Status; + EFI_STATUS RemoveStatus; + USB_BUS *Bus; + BOOLEAN Recover; + BOOLEAN RecoveryInProgress; + UINTN Index; - Child = NULL; - HubApi = HubIf->HubApi; + Bus = HubIf->Device->Bus; + Child = UsbFindChild (HubIf, Port); + HubApi = HubIf->HubApi; + Recover = (BOOLEAN)((Child != NULL) && Child->Reenumerate); + + if ((Child != NULL) && + (Child->ResetInProgress || (Child->BindingInProgress != 0) || (Child->AsyncCallbacks != 0))) + { + DEBUG ((DEBUG_INFO, "UsbEnumeratePort: device operation in progress on port %d\n", Port)); + return EFI_NOT_READY; + } // Zero out PortState in case GetPortStatus does not set // it and we continue on the EFI_DEVICE_ERROR path @@ -992,16 +1292,31 @@ UsbEnumeratePort ( // Status = HubApi->GetPortStatus (HubIf, Port, &PortState); - if (EFI_ERROR (Status) && (Status != EFI_DEVICE_ERROR)) { + if (EFI_ERROR (Status) && (Recover || (Status != EFI_DEVICE_ERROR))) { DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: failed to get state of port %d\n", Port)); return Status; } + if (Recover && USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION)) { + RemoveStatus = UsbDeviceIsQuiesced (Child); + if (EFI_ERROR (RemoveStatus)) { + return RemoveStatus; + } + + for (Index = 0; Index < Child->NumOfInterface; Index++) { + if (Child->Interfaces[Index] != NULL) { + Child->Interfaces[Index]->IsManaged = FALSE; + } + } + } + // // Only handle connection/enable/overcurrent/reset change. // Usb super speed hub may report other changes, such as warm reset change. Ignore them. // - if ((PortState.PortChangeStatus & (USB_PORT_STAT_C_CONNECTION | USB_PORT_STAT_C_ENABLE | USB_PORT_STAT_C_OVERCURRENT | USB_PORT_STAT_C_RESET)) == 0) { + if (!Recover && + ((PortState.PortChangeStatus & (USB_PORT_STAT_C_CONNECTION | USB_PORT_STAT_C_ENABLE | USB_PORT_STAT_C_OVERCURRENT | USB_PORT_STAT_C_RESET)) == 0)) + { return EFI_SUCCESS; } @@ -1064,9 +1379,17 @@ UsbEnumeratePort ( // Child = UsbFindChild (HubIf, Port); + if (Recover) { + DEBUG ((DEBUG_INFO, "UsbEnumeratePort: starting deferred recovery on port %d@hub %d\n", Port, HubIf->Device->Address)); + } + 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: failed to remove device on port %d - %r\n", Port, RemoveStatus)); + return RemoveStatus; + } } if (USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION)) { @@ -1074,17 +1397,26 @@ UsbEnumeratePort ( // Now, new device connected, enumerate and configure the device // DEBUG ((DEBUG_INFO, "UsbEnumeratePort: new device connected at port %d\n", Port)); - if (USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_RESET) && + RecoveryInProgress = Bus->RecoveryInProgress; + Bus->RecoveryInProgress = (BOOLEAN)(RecoveryInProgress || Recover); + if (!Recover && + USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_RESET) && (Status != EFI_DEVICE_ERROR)) { Status = UsbEnumerateNewDev (HubIf, Port, FALSE); } else { Status = UsbEnumerateNewDev (HubIf, Port, TRUE); } + + Bus->RecoveryInProgress = RecoveryInProgress; } else { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: device disconnected event on port %d\n", Port)); } + if (Recover) { + DEBUG ((DEBUG_INFO, "UsbEnumeratePort: deferred recovery on port %d@hub %d finished - %r\n", Port, HubIf->Device->Address, Status)); + } + HubApi->ClearPortChange (HubIf, Port); return Status; } @@ -1115,7 +1447,7 @@ UsbHubEnumeration ( for (Index = 0; Index < HubIf->NumOfPort; Index++) { Child = UsbFindChild (HubIf, Index); - if ((Child != NULL) && (Child->DisconnectFail == TRUE)) { + if ((Child != NULL) && (Child->DisconnectFail == TRUE) && !Child->Reenumerate) { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: The device disconnect fails at port %d from hub %p, try again\n", Index, HubIf)); UsbRemoveDevice (Child); } @@ -1163,8 +1495,12 @@ UsbRootHubEnumeration ( USB_INTERFACE *RootHub; UINT8 Index; USB_DEVICE *Child; + USB_BUS *Bus; + UINTN Address; + EFI_STATUS Status; RootHub = (USB_INTERFACE *)Context; + Bus = RootHub->Device->Bus; // MU_CHANGE [BEGIN] // MU_CHANGE Implement Enumeration delay @@ -1174,8 +1510,38 @@ UsbRootHubEnumeration ( // MU_CHANGE [END] + // + // The root timer also services devices behind external hubs, even when their + // interrupt endpoint reports no port changes. No separate event owns a child. + // + for (Address = 1; Address < Bus->MaxDevices; Address++) { + Child = Bus->Devices[Address]; + if ((Child != NULL) && Child->Reenumerate) { + Status = UsbEnumeratePort (Child->ParentIf, Child->ParentPort); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "UsbRootHubEnumeration: deferred recovery failed - %r\n", Status)); + } + } + } + + for (Address = 1; Address < Bus->MaxDevices; Address++) { + Child = Bus->Devices[Address]; + if ((Child == NULL) || (Child->RetiredConfigs == NULL)) { + continue; + } + + Status = UsbRemoveRetiredConfigs (Child, FALSE); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_INFO, "UsbRootHubEnumeration: retained interfaces await cleanup - %r\n", Status)); + } + } + for (Index = 0; Index < RootHub->NumOfPort; Index++) { Child = UsbFindChild (RootHub, Index); + if ((Child != NULL) && Child->Reenumerate) { + continue; + } + if ((Child != NULL) && (Child->DisconnectFail == TRUE)) { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: The device disconnect fails at port %d from root hub %p, try again\n", Index, RootHub)); UsbRemoveDevice (Child); diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h index 24876ae842..7dfc8c0bbf 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 immediately. @retval EFI_NOT_FOUND There is no configuration with the index. @retval EFI_OUT_OF_RESOURCES Failed to allocate resource. @@ -132,7 +133,73 @@ UsbSelectSetting ( EFI_STATUS UsbSelectConfig ( IN USB_DEVICE *Device, - IN UINT8 ConfigIndex + IN UINT8 ConfigIndex, + IN BOOLEAN ConnectDrivers + ); + +/** + Connect an interface while pinning its device across driver binding. + + @param UsbIf The interface to connect. + + @retval EFI_NOT_READY Reset or recursive binding blocks connection. + @retval Others The result of connecting the interface. +**/ +EFI_STATUS +UsbConnectDriver ( + IN USB_INTERFACE *UsbIf + ); + +/** + Check that software-requested replacement will not stop an active consumer. + + @param Device The old device, whose objects remain installed. + + @retval EFI_SUCCESS No driver ownership or asynchronous work remains. + @retval Others The device cannot yet be safely retired. +**/ +EFI_STATUS +UsbDeviceIsQuiesced ( + IN USB_DEVICE *Device + ); + +/** + Clean up disconnected interfaces and their retained descriptor trees. + + @param Device The device retaining old interface generations. + @param DisconnectDrivers Whether normal teardown may stop their drivers. + + @retval EFI_SUCCESS All retired configurations were removed. + @retval Others Active consumers or teardown failure retained state. +**/ +EFI_STATUS +UsbRemoveRetiredConfigs ( + IN USB_DEVICE *Device, + IN BOOLEAN DisconnectDrivers + ); + +/** + Check that an interface's consumers have released it before retirement. + + @param UsbIf The interface to retire. + @retval EFI_SUCCESS No owner or asynchronous work remains. + @retval Others The interface cannot yet be safely retired. +**/ +EFI_STATUS +UsbInterfaceIsQuiesced ( + IN USB_INTERFACE *UsbIf + ); + +/** + Free an interface after its consumers and transfers have released it. + + @param UsbIf The interface to free. + @retval EFI_SUCCESS The interface was uninstalled and freed. + @retval Others The interface remains installed. +**/ +EFI_STATUS +UsbFreeInterface ( + IN USB_INTERFACE *UsbIf ); /** @@ -140,7 +207,8 @@ UsbSelectConfig ( @param Device The USB device to remove configuration from. - @return None. + @retval EFI_SUCCESS All interfaces were retired. + @retval Others Active work or teardown failure retained old state. **/ EFI_STATUS @@ -154,6 +222,7 @@ UsbRemoveConfig ( @param Device The device to remove. @retval EFI_SUCCESS The device is removed. + @retval EFI_NOT_READY A reset, binding, or asynchronous callback is active. **/ EFI_STATUS diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c index 61cddf0c82..98fd0fe367 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c @@ -1100,8 +1100,11 @@ 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); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "UsbBusRecursivelyConnectWantedUsbIo: connection failed - %r\n", Status)); + } + DEBUG ((DEBUG_INFO, "UsbBusRecursivelyConnectWantedUsbIo: TPL after connect is %d\n", (UINT32)UsbGetCurrentTpl ())); } } From b717f24cbb82e240d134637cba2d69f10eb4bc3c Mon Sep 17 00:00:00 2001 From: Aaron Pop Date: Tue, 27 Oct 2026 15:44:56 -0700 Subject: [PATCH 3/4] attempt to disconnect all devices on interface change detected after reset --- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c | 270 ++++--------- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h | 38 +- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 405 +++----------------- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h | 65 +--- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c | 4 - 5 files changed, 129 insertions(+), 653 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c index 920d4c7a85..c938d0e418 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.c @@ -82,7 +82,7 @@ UsbIoControlTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoControlTransfer No media\n")); goto ON_EXIT; @@ -190,12 +190,7 @@ UsbIoControlTransfer ( if (Dev->ActiveConfig != NULL) { Status = UsbRemoveConfig (Dev); if (EFI_ERROR (Status)) { - Dev->Connected = FALSE; - if (!Dev->Bus->RecoveryInProgress) { - Dev->Reenumerate = TRUE; - } - - DEBUG ((DEBUG_ERROR, "UsbIoControlTransfer: failed to remove old configuration - %r\n", Status)); + DEBUG ((DEBUG_ERROR, "UsbIoControlTransfer: configuration teardown failed - %r\n", Status)); goto ON_EXIT; } } @@ -283,7 +278,7 @@ UsbIoBulkTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoBulkTransfer No media\n")); goto ON_EXIT; @@ -383,7 +378,7 @@ UsbIoSyncInterruptTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoSyncInterruptTransfer No media\n")); goto ON_EXIT; @@ -419,35 +414,6 @@ UsbIoSyncInterruptTransfer ( return Status; } -/** - Pin device and request state across a callback, including self-cancellation. -**/ -STATIC -EFI_STATUS -EFIAPI -UsbAsyncInterruptCallback ( - IN VOID *Data, - IN UINTN DataLength, - IN VOID *Context, - IN UINT32 Result - ) -{ - USB_ASYNC_REQUEST *Request; - EFI_STATUS Status; - - Request = Context; - Request->InCallback++; - Request->Device->AsyncCallbacks++; - Status = Request->Callback (Data, DataLength, Request->Context, Result); - Request->Device->AsyncCallbacks--; - Request->InCallback--; - if (Request->Cancelled && (Request->InCallback == 0)) { - FreePool (Request); - } - - return Status; -} - /** Queue a new asynchronous interrupt transfer, or remove the old request if (IsNewTransfer == FALSE). @@ -486,8 +452,6 @@ UsbIoAsyncInterruptTransfer ( EFI_TPL OldTpl; UINT8 Toggle; EFI_STATUS Status; - USB_ASYNC_REQUEST *Request; - UINTN AsyncIndex; if ((USB_ENDPOINT_ADDR (Endpoint) == 0) || (USB_ENDPOINT_ADDR (Endpoint) > 15)) { return EFI_INVALID_PARAMETER; @@ -497,44 +461,20 @@ UsbIoAsyncInterruptTransfer ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if (((Dev->Connected == FALSE) || UsbIf->Disconnected) && (IsNewTransfer == TRUE)) { + if ((Dev->Connected == FALSE) && (IsNewTransfer == TRUE)) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoAsyncInterruptTransfer No media\n")); goto ON_EXIT; } // MU_CHANGE [END] - AsyncIndex = USB_ENDPOINT_ADDR (Endpoint) + (((Endpoint & 0x80) != 0) ? 16 : 0); - Request = UsbIf->AsyncTransfers[AsyncIndex]; - if (!IsNewTransfer && (Request != NULL)) { - EpDesc = &Request->Endpoint; - } else { - EpDesc = UsbGetEndpointDesc (UsbIf, Endpoint); - } + EpDesc = UsbGetEndpointDesc (UsbIf, Endpoint); if ((EpDesc == NULL) || (USB_ENDPOINT_TYPE (&EpDesc->Desc) != USB_ENDPOINT_INTERRUPT)) { Status = EFI_INVALID_PARAMETER; goto ON_EXIT; } - if (IsNewTransfer) { - if ((Request != NULL) || (Callback == NULL)) { - Status = EFI_INVALID_PARAMETER; - goto ON_EXIT; - } - - Request = AllocateZeroPool (sizeof (*Request)); - if (Request == NULL) { - Status = EFI_OUT_OF_RESOURCES; - goto ON_EXIT; - } - - Request->Device = Dev; - Request->Endpoint = *EpDesc; - Request->Callback = Callback; - Request->Context = Context; - } - Toggle = EpDesc->Toggle; Status = UsbHcAsyncInterruptTransfer ( Dev->Bus, @@ -547,25 +487,11 @@ UsbIoAsyncInterruptTransfer ( PollInterval, DataLength, &Dev->Translator, - IsNewTransfer ? UsbAsyncInterruptCallback : NULL, - IsNewTransfer ? Request : NULL + Callback, + Context ); EpDesc->Toggle = Toggle; - if (IsNewTransfer) { - if (EFI_ERROR (Status)) { - FreePool (Request); - } else { - Request->Endpoint.Toggle = Toggle; - UsbIf->AsyncTransfers[AsyncIndex] = Request; - } - } else if (!EFI_ERROR (Status) && (Request != NULL)) { - UsbIf->AsyncTransfers[AsyncIndex] = NULL; - Request->Cancelled = TRUE; - if (Request->InCallback == 0) { - FreePool (Request); - } - } ON_EXIT: gBS->RestoreTPL (OldTpl); @@ -658,7 +584,7 @@ UsbIoGetDeviceDescriptor ( // MU_CHANGE [BEGIN] Status = EFI_SUCCESS; - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -705,7 +631,7 @@ UsbIoGetActiveConfigDescriptor ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -756,7 +682,7 @@ UsbIoGetInterfaceDescriptor ( Dev = UsbIf->Device; Status = EFI_SUCCESS; - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -802,7 +728,7 @@ UsbIoGetEndpointDescriptor ( Dev = UsbIf->Device; Status = EFI_SUCCESS; - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -864,7 +790,7 @@ UsbIoGetSupportedLanguages ( // MU_CHANGE [BEGIN] Status = EFI_SUCCESS; - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -917,7 +843,7 @@ UsbIoGetStringDescriptor ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; // MU_CHANGE [BEGIN] - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; goto ON_EXIT; } @@ -974,7 +900,7 @@ UsbIoGetStringDescriptor ( } /** - Compare default interface layouts before reusing a USB I/O instance. + Compare the default function and endpoints before retaining an interface. **/ STATIC BOOLEAN @@ -987,7 +913,7 @@ UsbInterfaceIsCompatible ( USB_INTERFACE_SETTING *Setting; UINTN Index; - if (UsbIf->Disconnected || UsbIf->IsHub) { + if (UsbIf->IsHub) { return FALSE; } @@ -1027,21 +953,22 @@ UsbInterfaceIsCompatible ( } /** - Reconcile fresh descriptors with existing USB I/O instances without binding. - Compatible interfaces survive; incompatible interfaces stay offline until - their owners release them and they can be safely retired. + 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_DEVICE_ERROR The new personality could not be synchronized. + @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 USB_DEVICE *Dev, + IN BOOLEAN CanReplace ) { USB_DEVICE NewDev; @@ -1054,12 +981,8 @@ UsbResetConfig ( UINTN Index; UINTN SettingIndex; UINT8 Config; - BOOLEAN IdentityCompatible; - BOOLEAN KeepInterface[USB_MAX_INTERFACE]; - USB_RETIRED_CONFIG *Retired; + BOOLEAN Replace; - Retired = NULL; - ZeroMem (KeepInterface, sizeof (KeepInterface)); // // Build into a temporary device so failures leave all cached pointers valid. // @@ -1081,24 +1004,9 @@ UsbResetConfig ( goto ON_ERROR; } - DEBUG (( - DEBUG_INFO, - "UsbResetConfig: device %d identity %04x:%04x -> %04x:%04x\n", - Dev->Address, - Dev->DevDesc->Desc.IdVendor, - Dev->DevDesc->Desc.IdProduct, - NewDev.DevDesc->Desc.IdVendor, - NewDev.DevDesc->Desc.IdProduct - )); - - IdentityCompatible = (BOOLEAN)( - (NewDev.DevDesc->Desc.IdVendor == Dev->DevDesc->Desc.IdVendor) && - (NewDev.DevDesc->Desc.IdProduct == Dev->DevDesc->Desc.IdProduct) && - (NewDev.DevDesc->Desc.DeviceClass == Dev->DevDesc->Desc.DeviceClass) && - (NewDev.DevDesc->Desc.DeviceSubClass == Dev->DevDesc->Desc.DeviceSubClass) && - (NewDev.DevDesc->Desc.DeviceProtocol == Dev->DevDesc->Desc.DeviceProtocol) - ); - + 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++) { @@ -1112,6 +1020,7 @@ UsbResetConfig ( if ((ConfigDesc == NULL) && (NewDev.DevDesc->Desc.NumConfigurations != 0)) { ConfigDesc = NewDev.DevDesc->Configs[0]; + Replace = TRUE; } Status = EFI_DEVICE_ERROR; @@ -1119,12 +1028,13 @@ UsbResetConfig ( (ConfigDesc->Desc.NumInterfaces == 0) || (ConfigDesc->Desc.NumInterfaces > USB_MAX_INTERFACE)) { - DEBUG ((DEBUG_ERROR, "UsbResetConfig: invalid configuration after reset\n")); + DEBUG ((DEBUG_ERROR, "UsbResetConfig: incompatible configuration after reset\n")); goto ON_ERROR; } - IdentityCompatible = (BOOLEAN)(IdentityCompatible && (ConfigDesc->Desc.ConfigurationValue == Config)); - Config = ConfigDesc->Desc.ConfigurationValue; + 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]; @@ -1154,22 +1064,26 @@ UsbResetConfig ( IfDesc->ActiveIndex = SettingIndex; UsbIf = Dev->Interfaces[Index]; - if (UsbIf != NULL) { - KeepInterface[Index] = (BOOLEAN)(IdentityCompatible && UsbInterfaceIsCompatible (UsbIf, IfDesc)); + if ((UsbIf != NULL) && !UsbInterfaceIsCompatible (UsbIf, IfDesc)) { + Replace = TRUE; } } - for (Index = 0; Index < Dev->NumOfInterface; Index++) { - UsbIf = Dev->Interfaces[Index]; - if ((UsbIf != NULL) && !KeepInterface[Index]) { - Retired = AllocateZeroPool (sizeof (*Retired)); - if (Retired == NULL) { - Status = EFI_OUT_OF_RESOURCES; - DEBUG ((DEBUG_ERROR, "UsbResetConfig: failed to retain old interfaces\n")); - goto ON_ERROR; - } + 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; + } - break; + // + // 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; } } @@ -1179,23 +1093,6 @@ UsbResetConfig ( goto ON_ERROR; } - if (Retired != NULL) { - Retired->DevDesc = Dev->DevDesc; - Retired->NumOfInterface = Dev->NumOfInterface; - for (Index = 0; Index < Dev->NumOfInterface; Index++) { - UsbIf = Dev->Interfaces[Index]; - if ((UsbIf != NULL) && !KeepInterface[Index]) { - UsbIf->Disconnected = TRUE; - Retired->Interfaces[Index] = UsbIf; - Dev->Interfaces[Index] = NULL; - DEBUG ((DEBUG_INFO, "UsbResetConfig: retained disconnected interface %d\n", (UINT32)Index)); - } - } - - Retired->Next = Dev->RetiredConfigs; - Dev->RetiredConfigs = Retired; - } - OldDesc = Dev->DevDesc; Dev->DevDesc = NewDev.DevDesc; NewDev.DevDesc = NULL; @@ -1213,18 +1110,20 @@ UsbResetConfig ( } } - if (Retired == NULL) { - UsbFreeDevDesc (OldDesc); + 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; } - Dev->NumOfInterface = ConfigDesc->Desc.NumInterfaces; - return UsbSelectConfig (Dev, Config, FALSE); + return EFI_SUCCESS; ON_ERROR: - if (Retired != NULL) { - FreePool (Retired); - } - if (NewDev.DevDesc != NULL) { UsbFreeDevDesc (NewDev.DevDesc); } @@ -1235,19 +1134,16 @@ UsbResetConfig ( /** Reset the device, then if that succeeds, reconfigure the device with its address and current active configuration. - Refresh compatible interface instances and publish missing ones. Keep - incompatible instances disconnected until their consumers release them. - Once a port reset is attempted, failures defer replacement of the device's - interfaces to bus enumeration unless recovery is already in progress. - Personality replacement publishes fresh USB I/O interfaces synchronously - without connecting drivers or stopping old consumers. Callers must acquire - fresh USB I/O after replacement. + 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. @retval EFI_SUCCESS The device is reset and configured. - @retval EFI_NOT_READY A reset is already active. - @retval Others Failed to reset the device. Recovery may be pending. + @retval Others Failed to reset the device. **/ EFI_STATUS @@ -1269,22 +1165,14 @@ UsbIoPortReset ( UsbIf = USB_INTERFACE_FROM_USBIO (This); Dev = UsbIf->Device; - DEBUG (( - DEBUG_INFO, - "UsbIoPortReset: device %d interface %d caller TPL %d\n", - Dev->Address, - UsbIf->IfSetting->Desc.InterfaceNumber, - (UINT32)OldTpl - )); - if (Dev->ResetInProgress) { Status = EFI_NOT_READY; - DEBUG ((DEBUG_ERROR, "UsbIoPortReset: reset already in progress for device %d\n", Dev->Address)); + DEBUG ((DEBUG_ERROR, "UsbIoPortReset: reset already active\n")); goto ON_EXIT; } // MU_CHANGE [BEGIN] - if ((Dev->Connected == FALSE) || UsbIf->Disconnected) { + if (Dev->Connected == FALSE) { Status = EFI_DEVICE_ERROR; DEBUG ((DEBUG_ERROR, "UsbIoPortReset No media\n")); goto ON_EXIT; @@ -1310,7 +1198,7 @@ UsbIoPortReset ( Status )); - goto RECOVER; + goto ON_EXIT; } HubIf->HubApi->ClearPortChange (HubIf, Dev->ParentPort); @@ -1338,7 +1226,7 @@ UsbIoPortReset ( Status )); - goto RECOVER; + goto ON_EXIT; } DEBUG ((DEBUG_INFO, "UsbIoPortReset: device is now ADDRESSED at %d\n", Dev->Address)); @@ -1348,34 +1236,18 @@ UsbIoPortReset ( // is in CONFIGURED state. // if (Dev->ActiveConfig != NULL) { - Status = UsbResetConfig (Dev); + Status = UsbResetConfig (Dev, (BOOLEAN)(OldTpl <= TPL_CALLBACK)); } else { // MU_CHANGE [BEGIN] 291137 UsbUpdateDescriptors (Dev); // MU_CHANGE [END] } - if (!EFI_ERROR (Status)) { - goto ON_EXIT; - } - -RECOVER: - // - // Keep old descriptors alive for driver teardown and transfer cancellation. - // A reset from a recovery-started driver must not create a reconnect loop. - // - Dev->Connected = FALSE; - if (!Dev->Bus->RecoveryInProgress) { - Dev->Reenumerate = TRUE; - DEBUG ((DEBUG_INFO, "UsbIoPortReset: queued recovery for device %d - %r\n", Dev->Address, Status)); - } else { - DEBUG ((DEBUG_ERROR, "UsbIoPortReset: reset failed during recovery for device %d - %r\n", Dev->Address, Status)); +ON_EXIT: + if (ResetStarted && EFI_ERROR (Status)) { + Dev->Connected = FALSE; } -ON_EXIT: - // - // Restoring TPL may dispatch enumeration before this function returns. - // gBS->RestoreTPL (OldTpl); if (ResetStarted) { Dev->ResetInProgress = FALSE; diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h index ccb2ace283..3443e1d347 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus.h @@ -207,30 +207,11 @@ struct _USB_DEVICE { BOOLEAN Connected; // MU_CHANGE BOOLEAN DisconnectFail; BOOLEAN ResetInProgress; - BOOLEAN Reenumerate; - UINTN BindingInProgress; - UINTN AsyncCallbacks; - struct _USB_RETIRED_CONFIG *RetiredConfigs; + BOOLEAN BindingInProgress; UINT8 EnumScript; BOOLEAN IsSSDev; }; -typedef struct _USB_RETIRED_CONFIG { - struct _USB_RETIRED_CONFIG *Next; - USB_DEVICE_DESC *DevDesc; - UINT8 NumOfInterface; - USB_INTERFACE *Interfaces[USB_MAX_INTERFACE]; -} USB_RETIRED_CONFIG; - -typedef struct { - USB_DEVICE *Device; - USB_ENDPOINT_DESC Endpoint; - EFI_ASYNC_USB_TRANSFER_CALLBACK Callback; - VOID *Context; - UINTN InCallback; - BOOLEAN Cancelled; -} USB_ASYNC_REQUEST; - // // Stands for different functions of USB device // @@ -247,8 +228,6 @@ struct _USB_INTERFACE { EFI_USB_IO_PROTOCOL UsbIo; EFI_DEVICE_PATH_PROTOCOL *DevicePath; BOOLEAN IsManaged; - BOOLEAN Disconnected; - USB_ASYNC_REQUEST *AsyncTransfers[32]; // // Hub device special data @@ -303,7 +282,6 @@ struct _USB_BUS { // for root hub. Device with address i is at Devices[i]. // USB_DEVICE *Devices[256]; - BOOLEAN RecoveryInProgress; // // USB Bus driver need to control the recursive connect policy of the bus, only those wanted @@ -687,19 +665,15 @@ UsbIoGetStringDescriptor ( /** Reset the device, then if that succeeds, reconfigure the device with its address and current active configuration. - Compatible interfaces retain existing handles even when the layout changes. - Incompatible instances are marked offline and retired only after consumers - release ownership and asynchronous work. Missing interfaces are published - immediately on the same device without stopping old drivers or binding new ones. - Failures schedule deferred recovery. - Callers must acquire fresh USB I/O after replacement. Driver binding during - recovery cannot schedule another recovery. + 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. @retval EFI_SUCCESS The device is reset and configured. - @retval EFI_NOT_READY A reset is already active. - @retval Others Failed to reset the device. Recovery may be pending. + @retval Others Failed to reset the device. **/ EFI_STATUS diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c index 22961e2b27..9d93c100ed 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c @@ -55,18 +55,6 @@ UsbFreeInterface ( ) { EFI_STATUS Status; - UINTN Index; - - if (UsbIf->Device->AsyncCallbacks != 0) { - return EFI_NOT_READY; - } - - for (Index = 0; Index < ARRAY_SIZE (UsbIf->AsyncTransfers); Index++) { - if (UsbIf->AsyncTransfers[Index] != NULL) { - DEBUG ((DEBUG_ERROR, "UsbFreeInterface: asynchronous transfer is still queued\n")); - return EFI_NOT_READY; - } - } UsbCloseHostProtoByChild (UsbIf->Device->Bus, UsbIf->Handle); @@ -98,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; @@ -115,7 +107,7 @@ UsbCreateInterface ( UsbIf = AllocateZeroPool (sizeof (USB_INTERFACE)); if (UsbIf == NULL) { - return NULL; + return EFI_OUT_OF_RESOURCES; } UsbIf->Signature = USB_INTERFACE_SIGNATURE; @@ -185,18 +177,8 @@ UsbCreateInterface ( goto ON_ERROR; } - DEBUG (( - DEBUG_INFO, - "UsbCreateInterface: published %04x:%04x interface %d at port %d@hub %d, handle %p\n", - Device->DevDesc->Desc.IdVendor, - Device->DevDesc->Desc.IdProduct, - UsbIf->IfSetting->Desc.InterfaceNumber, - Device->ParentPort, - Device->ParentAddr, - UsbIf->Handle - )); - - return UsbIf; + *Result = UsbIf; + return EFI_SUCCESS; ON_ERROR: if (UsbIf->DevicePath != NULL) { @@ -204,7 +186,7 @@ UsbCreateInterface ( } FreePool (UsbIf); - return NULL; + return Status; } /** @@ -218,8 +200,6 @@ UsbFreeDevice ( IN USB_DEVICE *Device ) { - ASSERT (Device->RetiredConfigs == NULL); - if (Device->DevDesc != NULL) { UsbFreeDevDesc (Device->DevDesc); } @@ -285,18 +265,13 @@ UsbConnectDriver ( EFI_STATUS Status; EFI_TPL OldTpl; - if (UsbIf->Device->ResetInProgress || (UsbIf->Device->BindingInProgress != 0)) { - DEBUG ((DEBUG_INFO, "UsbConnectDriver: reset or binding is in progress\n")); + 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; } - if (!UsbIf->Device->Connected || UsbIf->Disconnected) { - DEBUG ((DEBUG_ERROR, "UsbConnectDriver: device %d is offline\n", UsbIf->Device->Address)); - return EFI_DEVICE_ERROR; - } - - UsbIf->Device->BindingInProgress++; - Status = EFI_SUCCESS; + UsbIf->Device->BindingInProgress = TRUE; + Status = EFI_SUCCESS; // // Hub is maintained by the USB bus driver. Otherwise try to @@ -337,7 +312,7 @@ UsbConnectDriver ( } } - UsbIf->Device->BindingInProgress--; + UsbIf->Device->BindingInProgress = FALSE; return Status; } @@ -407,7 +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 immediately. + @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. @@ -474,33 +449,21 @@ UsbSelectConfig ( // the endpoint toggles to zero for its endpoints. // IfDesc = ConfigDesc->Interfaces[Index]; - Status = UsbSelectSetting (IfDesc, 0); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "UsbSelectConfig: no default setting for interface %d - %r\n", Index, Status)); - return Status; - } + UsbSelectSetting (IfDesc, IfDesc->Settings[IfDesc->ActiveIndex]->Desc.AlternateSetting); // // Create a USB_INTERFACE and install USB_IO and other protocols // - UsbIf = Device->Interfaces[Index]; - if (UsbIf == NULL) { - UsbIf = UsbCreateInterface (Device, ConfigDesc->Interfaces[Index]); - if (UsbIf == NULL) { - if (ConnectDrivers) { - Device->NumOfInterface = Index; - } + Status = UsbCreateInterface (Device, ConfigDesc->Interfaces[Index], &UsbIf); - return EFI_OUT_OF_RESOURCES; - } - - Device->Interfaces[Index] = UsbIf; + if (EFI_ERROR (Status)) { + Device->NumOfInterface = Index; + return Status; } ASSERT (Index < USB_MAX_INTERFACE); - if (ConnectDrivers) { - Device->NumOfInterface = (UINT8)(Index + 1); - } + Device->Interfaces[Index] = UsbIf; + Device->NumOfInterface = (UINT8)(Index + 1); if (!ConnectDrivers) { continue; @@ -525,13 +488,6 @@ UsbSelectConfig ( DEBUG ((DEBUG_ERROR, "UsbSelectConfig: device went offline during driver binding\n")); return EFI_DEVICE_ERROR; } - - if (Device->ActiveConfig != ConfigDesc) { - // - // Reset already published the new configuration without driver binding. - // - return EFI_SUCCESS; - } } Device->NumOfInterface = Index; @@ -543,13 +499,11 @@ UsbSelectConfig ( Disconnect the USB interface with its driver. @param UsbIf The interface to disconnect driver from. - @param Force Disconnect drivers even if not connected by the bus. **/ EFI_STATUS UsbDisconnectDriver ( - IN USB_INTERFACE *UsbIf, - IN BOOLEAN Force + IN USB_INTERFACE *UsbIf ) { EFI_TPL OldTpl; @@ -562,7 +516,7 @@ UsbDisconnectDriver ( Status = EFI_SUCCESS; if (UsbIf->IsHub) { Status = UsbIf->HubApi->Release (UsbIf); - } else if (UsbIf->IsManaged || Force) { + } else { // // This function is called in both UsbIoControlTransfer and // the timer callback in hub enumeration. So, at least it is @@ -573,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)) { @@ -581,192 +537,14 @@ UsbDisconnectDriver ( } DEBUG ((DEBUG_INFO, "UsbDisconnectDriver: TPL after disconnect is %d, %d\n", (UINT32)UsbGetCurrentTpl (), Status)); - ASSERT (UsbGetCurrentTpl () == TPL_CALLBACK); - - gBS->RaiseTPL (OldTpl); - } - - return Status; -} - -/** - Require consumers to release an interface before software-driven replacement. -**/ -EFI_STATUS -UsbInterfaceIsQuiesced ( - IN USB_INTERFACE *UsbIf - ) -{ - EFI_OPEN_PROTOCOL_INFORMATION_ENTRY *Entries; - EFI_GUID *Protocols[2]; - USB_DEVICE *Device; - EFI_STATUS Status; - UINTN ProtocolIndex; - UINTN EntryIndex; - UINTN EntryCount; - UINTN TransferIndex; - - Device = UsbIf->Device; - if ((Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { - DEBUG ((DEBUG_INFO, "UsbInterfaceIsQuiesced: device %d has an active binding or callback\n", Device->Address)); - return EFI_NOT_READY; - } - - if (UsbIf->IsHub) { - DEBUG ((DEBUG_ERROR, "UsbInterfaceIsQuiesced: cannot replace a hub interface\n")); - return EFI_ACCESS_DENIED; - } - - for (TransferIndex = 0; TransferIndex < ARRAY_SIZE (UsbIf->AsyncTransfers); TransferIndex++) { - if (UsbIf->AsyncTransfers[TransferIndex] != NULL) { - DEBUG ((DEBUG_INFO, "UsbInterfaceIsQuiesced: device %d still has asynchronous transfers\n", Device->Address)); - return EFI_NOT_READY; - } - } - - Protocols[0] = &gEfiUsbIoProtocolGuid; - Protocols[1] = &gEfiDevicePathProtocolGuid; - for (ProtocolIndex = 0; ProtocolIndex < ARRAY_SIZE (Protocols); ProtocolIndex++) { - Status = gBS->OpenProtocolInformation (UsbIf->Handle, Protocols[ProtocolIndex], &Entries, &EntryCount); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "UsbInterfaceIsQuiesced: failed to query interface ownership - %r\n", Status)); - return Status; - } - - Status = EFI_SUCCESS; - for (EntryIndex = 0; EntryIndex < EntryCount; EntryIndex++) { - if ((Entries[EntryIndex].Attributes & - (EFI_OPEN_PROTOCOL_BY_DRIVER | EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER | EFI_OPEN_PROTOCOL_EXCLUSIVE)) != 0) - { - Status = EFI_ACCESS_DENIED; - break; - } - } + ASSERT (UsbGetCurrentTpl () == ((OldTpl > TPL_CALLBACK) ? TPL_CALLBACK : OldTpl)); - if (Entries != NULL) { - FreePool (Entries); + if (OldTpl > TPL_CALLBACK) { + gBS->RaiseTPL (OldTpl); } - - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_INFO, "UsbInterfaceIsQuiesced: device %d interface %d still has an owner\n", Device->Address, UsbIf->IfSetting->Desc.InterfaceNumber)); - return Status; - } - } - - return EFI_SUCCESS; -} - -/** - Check all remaining interfaces before deferred whole-device recovery. -**/ -EFI_STATUS -UsbDeviceIsQuiesced ( - IN USB_DEVICE *Device - ) -{ - EFI_STATUS Status; - UINTN Index; - USB_RETIRED_CONFIG *Retired; - - if ((Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { - return EFI_NOT_READY; } - for (Index = 0; Index < Device->NumOfInterface; Index++) { - if (Device->Interfaces[Index] == NULL) { - continue; - } - - Status = UsbInterfaceIsQuiesced (Device->Interfaces[Index]); - if (EFI_ERROR (Status)) { - return Status; - } - } - - for (Retired = Device->RetiredConfigs; Retired != NULL; Retired = Retired->Next) { - for (Index = 0; Index < Retired->NumOfInterface; Index++) { - if (Retired->Interfaces[Index] == NULL) { - continue; - } - - Status = UsbInterfaceIsQuiesced (Retired->Interfaces[Index]); - if (EFI_ERROR (Status)) { - return Status; - } - } - } - - return EFI_SUCCESS; -} - -/** - Remove retained interfaces after their consumers release them, or as part - of normal configuration teardown when DisconnectDrivers is TRUE. -**/ -EFI_STATUS -UsbRemoveRetiredConfigs ( - IN USB_DEVICE *Device, - IN BOOLEAN DisconnectDrivers - ) -{ - USB_RETIRED_CONFIG **Link; - USB_RETIRED_CONFIG *Retired; - USB_INTERFACE *UsbIf; - EFI_STATUS Status; - EFI_STATUS ReturnStatus; - BOOLEAN Empty; - UINTN Index; - - if (Device->ResetInProgress || (Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { - return EFI_NOT_READY; - } - - ReturnStatus = EFI_SUCCESS; - Link = &Device->RetiredConfigs; - while (*Link != NULL) { - Retired = *Link; - Empty = TRUE; - for (Index = 0; Index < Retired->NumOfInterface; Index++) { - UsbIf = Retired->Interfaces[Index]; - if (UsbIf == NULL) { - continue; - } - - Status = DisconnectDrivers ? EFI_SUCCESS : UsbInterfaceIsQuiesced (UsbIf); - if (!EFI_ERROR (Status)) { - // - // Stop() and protocol removal can dispatch notifications. - // - Device->BindingInProgress++; - Status = UsbDisconnectDriver (UsbIf, DisconnectDrivers); - if (!EFI_ERROR (Status)) { - Status = UsbFreeInterface (UsbIf); - } - - Device->BindingInProgress--; - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "UsbRemoveRetiredConfigs: interface teardown failed - %r\n", Status)); - } - } - - if (EFI_ERROR (Status)) { - ReturnStatus = Status; - Empty = FALSE; - } else { - Retired->Interfaces[Index] = NULL; - } - } - - if (Empty) { - *Link = Retired->Next; - UsbFreeDevDesc (Retired->DevDesc); - FreePool (Retired); - } else { - Link = &Retired->Next; - } - } - - return ReturnStatus; + return Status; } /** @@ -785,11 +563,6 @@ UsbRemoveConfig ( EFI_STATUS Status; EFI_STATUS ReturnStatus; - if (Device->ResetInProgress || (Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { - DEBUG ((DEBUG_ERROR, "UsbRemoveConfig: binding or callback is still executing\n")); - return EFI_NOT_READY; - } - // // Remove each interface of the device // @@ -802,15 +575,10 @@ UsbRemoveConfig ( continue; } - Status = UsbRemoveRetiredConfigs (Device, TRUE); - if (EFI_ERROR (Status)) { - ReturnStatus = Status; - } - - Status = UsbDisconnectDriver (UsbIf, (BOOLEAN) !Device->Connected); + Status = UsbDisconnectDriver (UsbIf); if (!EFI_ERROR (Status)) { Status = UsbFreeInterface (UsbIf); - if (EFI_ERROR (Status) && Device->Connected && !UsbIf->Disconnected) { + if (EFI_ERROR (Status) && Device->Connected && !Device->ResetInProgress) { UsbConnectDriver (UsbIf); } } @@ -823,7 +591,8 @@ UsbRemoveConfig ( } if (!EFI_ERROR (ReturnStatus)) { - Device->ActiveConfig = NULL; + Device->ActiveConfig = NULL; + Device->NumOfInterface = 0; } return ReturnStatus; @@ -835,7 +604,6 @@ UsbRemoveConfig ( @param Device The device to remove. @retval EFI_SUCCESS The device is removed. - @retval EFI_NOT_READY A reset, binding, or asynchronous callback is active. **/ EFI_STATUS @@ -849,8 +617,8 @@ UsbRemoveDevice ( EFI_STATUS ReturnStatus; UINTN Index; - if (Device->ResetInProgress || (Device->BindingInProgress != 0) || (Device->AsyncCallbacks != 0)) { - DEBUG ((DEBUG_ERROR, "UsbRemoveDevice: reset, binding, or callback in progress for device %d\n", Device->Address)); + if (Device->ResetInProgress || Device->BindingInProgress) { + DEBUG ((DEBUG_INFO, "UsbRemoveDevice: reset or binding active\n")); return EFI_NOT_READY; } @@ -1199,7 +967,7 @@ UsbEnumerateNewDev ( OriginalStatus = Status; if ((Child != NULL) && (Child->NumOfInterface != 0)) { // - // Installed interfaces still own the descriptor tree. Do not retry in place. + // Installed interfaces still reference these descriptors. // return OriginalStatus; } @@ -1243,8 +1011,7 @@ UsbEnumerateNewDev ( } /** - Process the events on the port, including software-requested reset recovery. - Recovery does not require hardware change bits and always resets the port. + Process the events on the port. @param HubIf The HUB that has the device connected. @param Port The port index of the hub (started with zero). @@ -1264,23 +1031,11 @@ UsbEnumeratePort ( USB_DEVICE *Child; EFI_USB_PORT_STATUS PortState; EFI_STATUS Status; - EFI_STATUS RemoveStatus; - USB_BUS *Bus; - BOOLEAN Recover; - BOOLEAN RecoveryInProgress; - UINTN Index; - Bus = HubIf->Device->Bus; - Child = UsbFindChild (HubIf, Port); - HubApi = HubIf->HubApi; - Recover = (BOOLEAN)((Child != NULL) && Child->Reenumerate); + EFI_STATUS RemoveStatus; - if ((Child != NULL) && - (Child->ResetInProgress || (Child->BindingInProgress != 0) || (Child->AsyncCallbacks != 0))) - { - DEBUG ((DEBUG_INFO, "UsbEnumeratePort: device operation in progress on port %d\n", Port)); - return EFI_NOT_READY; - } + Child = NULL; + HubApi = HubIf->HubApi; // Zero out PortState in case GetPortStatus does not set // it and we continue on the EFI_DEVICE_ERROR path @@ -1292,31 +1047,16 @@ UsbEnumeratePort ( // Status = HubApi->GetPortStatus (HubIf, Port, &PortState); - if (EFI_ERROR (Status) && (Recover || (Status != EFI_DEVICE_ERROR))) { + if (EFI_ERROR (Status) && (Status != EFI_DEVICE_ERROR)) { DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: failed to get state of port %d\n", Port)); return Status; } - if (Recover && USB_BIT_IS_SET (PortState.PortStatus, USB_PORT_STAT_CONNECTION)) { - RemoveStatus = UsbDeviceIsQuiesced (Child); - if (EFI_ERROR (RemoveStatus)) { - return RemoveStatus; - } - - for (Index = 0; Index < Child->NumOfInterface; Index++) { - if (Child->Interfaces[Index] != NULL) { - Child->Interfaces[Index]->IsManaged = FALSE; - } - } - } - // // Only handle connection/enable/overcurrent/reset change. // Usb super speed hub may report other changes, such as warm reset change. Ignore them. // - if (!Recover && - ((PortState.PortChangeStatus & (USB_PORT_STAT_C_CONNECTION | USB_PORT_STAT_C_ENABLE | USB_PORT_STAT_C_OVERCURRENT | USB_PORT_STAT_C_RESET)) == 0)) - { + if ((PortState.PortChangeStatus & (USB_PORT_STAT_C_CONNECTION | USB_PORT_STAT_C_ENABLE | USB_PORT_STAT_C_OVERCURRENT | USB_PORT_STAT_C_RESET)) == 0) { return EFI_SUCCESS; } @@ -1379,15 +1119,11 @@ UsbEnumeratePort ( // Child = UsbFindChild (HubIf, Port); - if (Recover) { - DEBUG ((DEBUG_INFO, "UsbEnumeratePort: starting deferred recovery on port %d@hub %d\n", Port, HubIf->Device->Address)); - } - if (Child != NULL) { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: device at port %d removed from root hub %p\n", Port, HubIf)); RemoveStatus = UsbRemoveDevice (Child); if (EFI_ERROR (RemoveStatus)) { - DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: failed to remove device on port %d - %r\n", Port, RemoveStatus)); + DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: old device removal failed - %r\n", RemoveStatus)); return RemoveStatus; } } @@ -1397,26 +1133,17 @@ UsbEnumeratePort ( // Now, new device connected, enumerate and configure the device // DEBUG ((DEBUG_INFO, "UsbEnumeratePort: new device connected at port %d\n", Port)); - RecoveryInProgress = Bus->RecoveryInProgress; - Bus->RecoveryInProgress = (BOOLEAN)(RecoveryInProgress || Recover); - if (!Recover && - USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_RESET) && + if (USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_RESET) && (Status != EFI_DEVICE_ERROR)) { Status = UsbEnumerateNewDev (HubIf, Port, FALSE); } else { Status = UsbEnumerateNewDev (HubIf, Port, TRUE); } - - Bus->RecoveryInProgress = RecoveryInProgress; } else { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: device disconnected event on port %d\n", Port)); } - if (Recover) { - DEBUG ((DEBUG_INFO, "UsbEnumeratePort: deferred recovery on port %d@hub %d finished - %r\n", Port, HubIf->Device->Address, Status)); - } - HubApi->ClearPortChange (HubIf, Port); return Status; } @@ -1447,7 +1174,7 @@ UsbHubEnumeration ( for (Index = 0; Index < HubIf->NumOfPort; Index++) { Child = UsbFindChild (HubIf, Index); - if ((Child != NULL) && (Child->DisconnectFail == TRUE) && !Child->Reenumerate) { + if ((Child != NULL) && (Child->DisconnectFail == TRUE)) { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: The device disconnect fails at port %d from hub %p, try again\n", Index, HubIf)); UsbRemoveDevice (Child); } @@ -1495,12 +1222,8 @@ UsbRootHubEnumeration ( USB_INTERFACE *RootHub; UINT8 Index; USB_DEVICE *Child; - USB_BUS *Bus; - UINTN Address; - EFI_STATUS Status; RootHub = (USB_INTERFACE *)Context; - Bus = RootHub->Device->Bus; // MU_CHANGE [BEGIN] // MU_CHANGE Implement Enumeration delay @@ -1510,38 +1233,8 @@ UsbRootHubEnumeration ( // MU_CHANGE [END] - // - // The root timer also services devices behind external hubs, even when their - // interrupt endpoint reports no port changes. No separate event owns a child. - // - for (Address = 1; Address < Bus->MaxDevices; Address++) { - Child = Bus->Devices[Address]; - if ((Child != NULL) && Child->Reenumerate) { - Status = UsbEnumeratePort (Child->ParentIf, Child->ParentPort); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "UsbRootHubEnumeration: deferred recovery failed - %r\n", Status)); - } - } - } - - for (Address = 1; Address < Bus->MaxDevices; Address++) { - Child = Bus->Devices[Address]; - if ((Child == NULL) || (Child->RetiredConfigs == NULL)) { - continue; - } - - Status = UsbRemoveRetiredConfigs (Child, FALSE); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_INFO, "UsbRootHubEnumeration: retained interfaces await cleanup - %r\n", Status)); - } - } - for (Index = 0; Index < RootHub->NumOfPort; Index++) { Child = UsbFindChild (RootHub, Index); - if ((Child != NULL) && Child->Reenumerate) { - continue; - } - if ((Child != NULL) && (Child->DisconnectFail == TRUE)) { DEBUG ((DEBUG_INFO, "UsbEnumeratePort: The device disconnect fails at port %d from root hub %p, try again\n", Index, RootHub)); UsbRemoveDevice (Child); diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h index 7dfc8c0bbf..47e0c612aa 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.h @@ -123,7 +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 immediately. + @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. @@ -138,77 +138,19 @@ UsbSelectConfig ( ); /** - Connect an interface while pinning its device across driver binding. - - @param UsbIf The interface to connect. - - @retval EFI_NOT_READY Reset or recursive binding blocks connection. - @retval Others The result of connecting the interface. + Connect an interface, guarding bus-owned driver binding against reset teardown. **/ EFI_STATUS UsbConnectDriver ( IN USB_INTERFACE *UsbIf ); -/** - Check that software-requested replacement will not stop an active consumer. - - @param Device The old device, whose objects remain installed. - - @retval EFI_SUCCESS No driver ownership or asynchronous work remains. - @retval Others The device cannot yet be safely retired. -**/ -EFI_STATUS -UsbDeviceIsQuiesced ( - IN USB_DEVICE *Device - ); - -/** - Clean up disconnected interfaces and their retained descriptor trees. - - @param Device The device retaining old interface generations. - @param DisconnectDrivers Whether normal teardown may stop their drivers. - - @retval EFI_SUCCESS All retired configurations were removed. - @retval Others Active consumers or teardown failure retained state. -**/ -EFI_STATUS -UsbRemoveRetiredConfigs ( - IN USB_DEVICE *Device, - IN BOOLEAN DisconnectDrivers - ); - -/** - Check that an interface's consumers have released it before retirement. - - @param UsbIf The interface to retire. - @retval EFI_SUCCESS No owner or asynchronous work remains. - @retval Others The interface cannot yet be safely retired. -**/ -EFI_STATUS -UsbInterfaceIsQuiesced ( - IN USB_INTERFACE *UsbIf - ); - -/** - Free an interface after its consumers and transfers have released it. - - @param UsbIf The interface to free. - @retval EFI_SUCCESS The interface was uninstalled and freed. - @retval Others The interface remains installed. -**/ -EFI_STATUS -UsbFreeInterface ( - IN USB_INTERFACE *UsbIf - ); - /** Remove the current device configuration. @param Device The USB device to remove configuration from. - @retval EFI_SUCCESS All interfaces were retired. - @retval Others Active work or teardown failure retained old state. + @return None. **/ EFI_STATUS @@ -222,7 +164,6 @@ UsbRemoveConfig ( @param Device The device to remove. @retval EFI_SUCCESS The device is removed. - @retval EFI_NOT_READY A reset, binding, or asynchronous callback is active. **/ EFI_STATUS diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c index 98fd0fe367..6734f43183 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c @@ -1101,10 +1101,6 @@ UsbBusRecursivelyConnectWantedUsbIo ( // DEBUG ((DEBUG_INFO, "UsbBusRecursivelyConnectWantedUsbIo: TPL before connect is %d\n", (UINT32)UsbGetCurrentTpl ())); Status = UsbConnectDriver (UsbIf); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_WARN, "UsbBusRecursivelyConnectWantedUsbIo: connection failed - %r\n", Status)); - } - DEBUG ((DEBUG_INFO, "UsbBusRecursivelyConnectWantedUsbIo: TPL after connect is %d\n", (UINT32)UsbGetCurrentTpl ())); } } From a60e0891bc320b479b50265f0ea17961d2e233d9 Mon Sep 17 00:00:00 2001 From: Aaron Pop Date: Wed, 28 Oct 2026 15:58:09 -0700 Subject: [PATCH 4/4] MdeModulePkg/UsbBusDxe: Add googletest for DFU transitions Add a googletest that will exercise the Runtime->Runtime interfaces transition on UsbIo->UsbPortReset. Exercise the Runtime->DFU transition on UsbPortReset and DFU->Runtime transition on UsbPortReset --- .../GoogleTest/UsbBusDxeGoogleTest.cpp | 705 ++++++++++++++++++ .../GoogleTest/UsbBusDxeGoogleTest.inf | 43 ++ MdeModulePkg/Test/MdeModulePkgHostTest.dsc | 7 + 3 files changed, 755 insertions(+) create mode 100644 MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.cpp create mode 100644 MdeModulePkg/Bus/Usb/UsbBusDxe/GoogleTest/UsbBusDxeGoogleTest.inf 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/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 #