Hi Ray, For this patch, I just found a minor issue and also have a suggestion below. Others look good to me.
Minor issue: The following newly created functions' parameter lack IN and/or OUT modifiers: 1. BmGetLoadFileRamDisk 2. BmGetRamDiskMemoryInfo 3. BmDestroyRamDisk Suggestion: It looks like both BmGetLoadFileRamDisk and BmGetFileBufferFromLoadFileSystem) should be better to return RamDiskDevicePath instead of RamDiskHandle because both BmDestroyRamDisk and BmGetRamDiskMemoryInfo only need RamDisk's device path. In other words, if we change to return RamDiskDevicePath, we will NOT need to have DevicePathFromHandle function calls in newly added code. What do you think? By the way, if you agree with my suggestion above, we may also consider to directly use FullPath instead of adding new output parameter RamDiskHandle/RamDiskDevicePath to BmGetFileBufferFromLoadFileSystem(). For this one, I didn't take a deep look, so it maybe is not feasible. Just mention it here for your reference. Regards, Sunny Wang -----Original Message----- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Wednesday, March 30, 2016 8:29 AM To: edk2-devel@lists.01.org Cc: Ruiyu Ni <ruiyu...@intel.com>; Siyuan Fu <siyuan...@intel.com>; Feng Tian <feng.t...@intel.com> Subject: [edk2] [Patch v3 2/3] MdeModulePkg/Bds: Free resources after ram disk boot finishes The resource free includes to un-register the ram disk device and free the memory occupied by the ram disk. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu...@intel.com> Cc: Siyuan Fu <siyuan...@intel.com> Cc: Feng Tian <feng.t...@intel.com> --- MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 155 ++++++++++++++++++++- .../Library/UefiBootManagerLib/InternalBm.h | 1 + .../UefiBootManagerLib/UefiBootManagerLib.inf | 1 + 3 files changed, 153 insertions(+), 4 deletions(-) diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c index 6e5c13b..598de26 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c @@ -1095,6 +1095,7 @@ BmMatchHttpBootDevicePath ( @param LoadFileHandle The handle of LoadFile instance. @param FullPath Return the full device path pointing to the load option. @param FileSize Return the size of the load option. + @param RamDiskHandle Return the RAM Disk handle. @return The load option buffer. **/ @@ -1102,7 +1103,8 @@ VOID * BmGetFileBufferFromLoadFileSystem ( IN EFI_HANDLE LoadFileHandle, OUT EFI_DEVICE_PATH_PROTOCOL **FullPath, - OUT UINTN *FileSize + OUT UINTN *FileSize, + OUT EFI_HANDLE *RamDiskHandle ) { EFI_STATUS Status; @@ -1140,7 +1142,13 @@ BmGetFileBufferFromLoadFileSystem ( FreePool (Handles); } - if (Index != HandleCount) { + if (Index == HandleCount) { + Handle = NULL; + } + + *RamDiskHandle = Handle; + + if (Handle != NULL) { return BmExpandMediaDevicePath (DevicePathFromHandle (Handle), FullPath, FileSize); } else { return NULL; @@ -1149,6 +1157,124 @@ BmGetFileBufferFromLoadFileSystem ( /** + Return the RAM DISK handle created by LoadFile. + + @param FilePath The source file path. + + @return RAM DISK handle. +**/ +EFI_HANDLE +BmGetLoadFileRamDisk ( + EFI_DEVICE_PATH_PROTOCOL *FilePath + ) +{ + EFI_STATUS Status; + EFI_DEVICE_PATH_PROTOCOL *RamDiskDevicePath; + EFI_DEVICE_PATH_PROTOCOL *Node; + EFI_HANDLE Handle; + + Node = FilePath; + Status = gBS->LocateDevicePath (&gEfiLoadFileProtocolGuid, &Node, + &Handle); if (!EFI_ERROR (Status) && + (DevicePathType (Node) == MEDIA_DEVICE_PATH) && + (DevicePathSubType (Node) == MEDIA_RAM_DISK_DP) + ) { + + // + // Construct the device path pointing to RAM Disk + // + Node = NextDevicePathNode (Node); + RamDiskDevicePath = DuplicateDevicePath (FilePath); + ASSERT (RamDiskDevicePath != NULL); + SetDevicePathEndNode ((VOID *) ((UINTN) RamDiskDevicePath + + ((UINTN) Node - (UINTN) FilePath))); + + // + // Query the RAM Disk handle using the constructed device path. + // + Node = RamDiskDevicePath; + Status = gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid, &Node, &Handle); + ASSERT ((Status == EFI_SUCCESS) && IsDevicePathEnd (Node)); + FreePool (RamDiskDevicePath); + return Handle; + } + + return NULL; +} + +/** + Return the buffer and buffer size occupied by the RAM Disk. + + @param RamDiskHandle RAM Disk handle. + @param RamDiskSizeInPages Return RAM Disk size in pages. + + @retval RAM Disk buffer. +**/ +VOID * +BmGetRamDiskMemoryInfo ( + EFI_HANDLE RamDiskHandle, + UINTN *RamDiskSizeInPages + ) +{ + + EFI_STATUS Status; + EFI_DEVICE_PATH_PROTOCOL *DevicePath; + EFI_HANDLE Handle; + UINT64 StartingAddr; + UINT64 EndingAddr; + + *RamDiskSizeInPages = 0; + if (RamDiskHandle == NULL) { + return NULL; + } + DevicePath = DevicePathFromHandle (RamDiskHandle); + + // + // Get the buffer occupied by RAM Disk. + // + Status = gBS->LocateDevicePath (&gEfiLoadFileProtocolGuid, +&DevicePath, &Handle); + ASSERT_EFI_ERROR (Status); + ASSERT ((DevicePathType (DevicePath) == MEDIA_DEVICE_PATH) && +(DevicePathSubType (DevicePath) == MEDIA_RAM_DISK_DP)); + StartingAddr = ReadUnaligned64 ((UINT64 *) ((MEDIA_RAM_DISK_DEVICE_PATH *) DevicePath)->StartingAddr); + EndingAddr = ReadUnaligned64 ((UINT64 *) ((MEDIA_RAM_DISK_DEVICE_PATH *) DevicePath)->EndingAddr); + *RamDiskSizeInPages = EFI_SIZE_TO_PAGES ((UINTN) (EndingAddr - +StartingAddr + 1)); + return (VOID *) (UINTN) StartingAddr; } + +/** + Destroy the RAM Disk handle. + + The destroy operation includes to call RamDisk.Unregister to + unregister the RAM DISK from RAM DISK driver, free the memory + allocated for the RAM Disk. + + @param RamDiskHandle RAM Disk handle. +**/ +VOID +BmDestroyRamDisk ( + EFI_HANDLE RamDiskHandle + ) +{ + EFI_STATUS Status; + EFI_RAM_DISK_PROTOCOL *RamDisk; + VOID *RamDiskBuffer; + UINTN RamDiskSizeInPages; + + if (RamDiskHandle == NULL) { + return; + } + RamDiskBuffer = BmGetRamDiskMemoryInfo (RamDiskHandle, + &RamDiskSizeInPages); + + // + // Destroy RAM Disk. + // + Status = gBS->LocateProtocol (&gEfiRamDiskProtocolGuid, NULL, (VOID +*) &RamDisk); + ASSERT_EFI_ERROR (Status); + Status = RamDisk->Unregister (DevicePathFromHandle (RamDiskHandle)); + ASSERT_EFI_ERROR (Status); + FreePages (RamDiskBuffer, RamDiskSizeInPages); } + +/** Get the file buffer from the specified Load File instance. @param LoadFileHandle The specified Load File instance. @@ -1170,6 +1296,7 @@ BmGetFileBufferFromLoadFile ( EFI_LOAD_FILE_PROTOCOL *LoadFile; VOID *FileBuffer; BOOLEAN LoadFileSystem; + EFI_HANDLE RamDiskHandle; UINTN BufferSize; *FileSize = 0; @@ -1204,7 +1331,13 @@ BmGetFileBufferFromLoadFile ( } if (LoadFileSystem) { - FileBuffer = BmGetFileBufferFromLoadFileSystem (LoadFileHandle, FullPath, FileSize); + FileBuffer = BmGetFileBufferFromLoadFileSystem (LoadFileHandle, FullPath, FileSize, &RamDiskHandle); + if (FileBuffer == NULL) { + // + // If there is no bootable executable in the populated + // + BmDestroyRamDisk (RamDiskHandle); + } } else { *FileSize = BufferSize; *FullPath = DuplicateDevicePath (DevicePathFromHandle (LoadFileHandle)); @@ -1432,6 +1565,7 @@ EfiBootManagerBoot ( UINTN OriginalOptionNumber; EFI_DEVICE_PATH_PROTOCOL *FilePath; EFI_DEVICE_PATH_PROTOCOL *Node; + EFI_HANDLE RamDiskHandle; EFI_HANDLE FvHandle; VOID *FileBuffer; UINTN FileSize; @@ -1512,10 +1646,14 @@ EfiBootManagerBoot ( // // 5. Load EFI boot option to ImageHandle // - ImageHandle = NULL; + ImageHandle = NULL; + RamDiskHandle = NULL; if (DevicePathType (BootOption->FilePath) != BBS_DEVICE_PATH) { Status = EFI_NOT_FOUND; FileBuffer = BmGetLoadOptionBuffer (BootOption->FilePath, &FilePath, &FileSize); + if (FileBuffer != NULL) { + RamDiskHandle = BmGetLoadFileRamDisk (FilePath); + } DEBUG_CODE ( if (FileBuffer != NULL && CompareMem (BootOption->FilePath, FilePath, GetDevicePathSize (FilePath)) != 0) { DEBUG ((EFI_D_INFO, "[Bds] DevicePath expand: ")); @@ -1552,6 +1690,10 @@ EfiBootManagerBoot ( (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR) ); BootOption->Status = Status; + // + // Destroy the RAM disk + // + BmDestroyRamDisk (RamDiskHandle); return; } } @@ -1648,6 +1790,11 @@ EfiBootManagerBoot ( PERF_END_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber); // + // Destroy the RAM disk + // + BmDestroyRamDisk (RamDiskHandle); + + // // Clear the Watchdog Timer after the image returns // gBS->SetWatchdogTimer (0x0000, 0x0000, 0x0000, NULL); diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h index 7b6252a..4660323 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h @@ -43,6 +43,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include <Protocol/DriverHealth.h> #include <Protocol/FormBrowser2.h> #include <Protocol/VariableLock.h> +#include <Protocol/RamDisk.h> #include <Guid/ZeroGuid.h> #include <Guid/MemoryTypeInformation.h> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf index e9e74b1..9d62d3d 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf @@ -108,6 +108,7 @@ gEfiDiskInfoProtocolGuid ## SOMETIMES_CONSUMES gEfiDriverHealthProtocolGuid ## SOMETIMES_CONSUMES gEfiFormBrowser2ProtocolGuid ## SOMETIMES_CONSUMES + gEfiRamDiskProtocolGuid ## SOMETIMES_CONSUMES [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange ## SOMETIMES_CONSUMES -- 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
