It's better to add more detail comments on code or commit log to explain why the re-entry happens.
Reviewed-by: Feng Tian <[email protected]> Thanks Feng -----Original Message----- From: Wu, Hao A Sent: Thursday, March 17, 2016 10:45 AM To: [email protected]; Ni, Ruiyu <[email protected]>; Tian, Feng <[email protected]> Cc: Wu, Hao A <[email protected]> Subject: [PATCH] MdeModulePkg PartitionDxe: Add Re-entry handling logic for BindingStop There are scenario when the BindingStop service of PartitionDxe driver be re-entered. The current code has potential issue of referencing of already stopped child handle. This commit adds re-entry handling logic to resolve such issue. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu <[email protected]> --- .../Universal/Disk/PartitionDxe/Partition.c | 103 ++++++++++++++++----- .../Universal/Disk/PartitionDxe/Partition.h | 15 +++ 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c index 89cc540..3479f5b 100644 --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c @@ -407,6 +407,10 @@ PartitionDriverBindingStop ( Private = NULL; if (NumberOfChildren == 0) { + if (HasChildren (ControllerHandle)) { + DEBUG((EFI_D_ERROR, "PartitionDriverBindingStop: Still has child.\n")); + return EFI_DEVICE_ERROR; + } // // Close the bus driver // @@ -459,35 +463,50 @@ PartitionDriverBindingStop ( Private = PARTITION_DEVICE_FROM_BLOCK_IO_THIS (BlockIo); + if (Private->InStop) { + // + // Avoid stopping the child multiple times. + // + break; + } + Private->InStop = TRUE; - Status = gBS->CloseProtocol ( - ControllerHandle, - &gEfiDiskIoProtocolGuid, - This->DriverBindingHandle, - ChildHandleBuffer[Index] - ); + BlockIo->FlushBlocks (BlockIo); + + if (BlockIo2 != NULL) { + Status = BlockIo2->FlushBlocksEx (BlockIo2, NULL); + DEBUG((EFI_D_ERROR, "PartitionDriverBindingStop: FlushBlocksEx returned with %r\n", Status)); + } else { + Status = EFI_SUCCESS; + } + + gBS->CloseProtocol ( + ControllerHandle, + &gEfiDiskIoProtocolGuid, + This->DriverBindingHandle, + ChildHandleBuffer[Index] + ); // // All Software protocols have be freed from the handle so remove it. // Remove the BlockIo Protocol if has. // Remove the BlockIo2 Protocol if has. // if (BlockIo2 != NULL) { - BlockIo->FlushBlocks (BlockIo); - BlockIo2->FlushBlocksEx (BlockIo2, NULL); - Status = gBS->UninstallMultipleProtocolInterfaces ( - ChildHandleBuffer[Index], - &gEfiDevicePathProtocolGuid, - Private->DevicePath, - &gEfiBlockIoProtocolGuid, - &Private->BlockIo, - &gEfiBlockIo2ProtocolGuid, - &Private->BlockIo2, - Private->EspGuid, - NULL, - NULL - ); + if (Status != EFI_MEDIA_CHANGED) { + Status = gBS->UninstallMultipleProtocolInterfaces ( + ChildHandleBuffer[Index], + &gEfiDevicePathProtocolGuid, + Private->DevicePath, + &gEfiBlockIoProtocolGuid, + &Private->BlockIo, + &gEfiBlockIo2ProtocolGuid, + &Private->BlockIo2, + Private->EspGuid, + NULL, + NULL + ); + } } else { - BlockIo->FlushBlocks (BlockIo); Status = gBS->UninstallMultipleProtocolInterfaces ( ChildHandleBuffer[Index], &gEfiDevicePathProtocolGuid, @@ -501,6 +520,7 @@ PartitionDriverBindingStop ( } if (EFI_ERROR (Status)) { + Private->InStop = FALSE; gBS->OpenProtocol ( ControllerHandle, &gEfiDiskIoProtocolGuid, @@ -516,6 +536,9 @@ PartitionDriverBindingStop ( if (EFI_ERROR (Status)) { AllChildrenStopped = FALSE; + if (Status == EFI_MEDIA_CHANGED) { + break; + } } } @@ -1263,3 +1286,41 @@ InitializePartition ( return Status; } + +/** + Test to see if there is any child on ControllerHandle. + + @param[in] ControllerHandle Handle of device to test. + + @retval TRUE There are children on the ControllerHandle. + @retval FALSE No child is on the ControllerHandle. + +**/ +BOOLEAN +HasChildren ( + IN EFI_HANDLE ControllerHandle + ) +{ + EFI_OPEN_PROTOCOL_INFORMATION_ENTRY *OpenInfoBuffer; + UINTN EntryCount; + EFI_STATUS Status; + UINTN Index; + + Status = gBS->OpenProtocolInformation ( + ControllerHandle, + &gEfiDiskIoProtocolGuid, + &OpenInfoBuffer, + &EntryCount + ); + ASSERT_EFI_ERROR (Status); + + for (Index = 0; Index < EntryCount; Index++) { + if ((OpenInfoBuffer[Index].Attributes & EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) != 0) { + break; + } + } + FreePool (OpenInfoBuffer); + + return (BOOLEAN) (Index < EntryCount); } + diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h index 06470f6..7cb1988 100644 --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h @@ -61,6 +61,7 @@ typedef struct { UINT64 Start; UINT64 End; UINT32 BlockSize; + BOOLEAN InStop; EFI_GUID *EspGuid; @@ -346,6 +347,20 @@ PartitionInstallChildHandle ( ); /** + Test to see if there is any child on ControllerHandle. + + @param[in] ControllerHandle Handle of device to test. + + @retval TRUE There are children on the ControllerHandle. + @retval FALSE No child is on the ControllerHandle. + +**/ +BOOLEAN +HasChildren ( + IN EFI_HANDLE ControllerHandle + ); + +/** Install child handles if the Handle supports GPT partition structure. @param[in] This Calling context. -- 1.9.5.msysgit.0 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
