Hi Ray,
For this patch, I just have a suggestion below. Others look good to me.
For the memory allocation code block below, It looks better to add a ZeroMem()
call right after it like the following so that we can avoid running into some
problems caused by remaining garbage data in memory. What do you think?
+ FileBuffer = LoadFileSystem ? AllocateReservedPages (EFI_SIZE_TO_PAGES
+ (BufferSize)) : AllocatePool (BufferSize); if (FileBuffer == NULL) {
+ return NULL;
+ }
ZeroMem (FileBuffer, BufferSize);
Regards,
Sunny Wang
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Thursday, March 31, 2016 10:37 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni <ruiyu...@intel.com>; Siyuan Fu <siyuan...@intel.com>
Subject: [edk2] [Patch v3 1/3] MdeModulePkg/Bds: Allocate reserved memory for
RAM Disk boot media
Use reserved memory to hold the buffer for the RAM disk to follow the ACPI spec
requirement.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
Cc: Siyuan Fu <siyuan...@intel.com>
---
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 149 +++++++++++----------
.../Library/UefiBootManagerLib/InternalBm.h | 35 ++---
2 files changed, 95 insertions(+), 89 deletions(-)
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index f6c6845..a582b90 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -688,7 +688,6 @@ BmExpandUriDevicePath (
UINTN Index;
UINTN HandleCount;
EFI_HANDLE *Handles;
- EFI_LOAD_FILE_PROTOCOL *LoadFile;
VOID *FileBuffer;
EfiBootManagerConnectAll ();
@@ -698,37 +697,10 @@ BmExpandUriDevicePath (
Handles = NULL;
}
- FileBuffer = NULL;
-
for (Index = 0; Index < HandleCount; Index++) {
- Status = gBS->HandleProtocol (Handles[Index], &gEfiLoadFileProtocolGuid,
(VOID *) &LoadFile);
- ASSERT_EFI_ERROR (Status);
-
- FileBuffer = NULL;
- Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize,
FileBuffer);
- if (Status == EFI_BUFFER_TOO_SMALL) {
- FileBuffer = AllocatePool (*FileSize);
- if (FileBuffer == NULL) {
- break;
- }
- Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, FileSize,
FileBuffer);
- }
-
- if (!EFI_ERROR (Status)) {
- //
- // LoadFile() returns a file buffer mapping to a file system.
- //
- if (Status == EFI_WARN_FILE_SYSTEM) {
- return BmGetFileBufferFromLoadFileFileSystem (Handles[Index],
FullPath, FileSize);
- }
-
- ASSERT (Status == EFI_SUCCESS);
- *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handles[Index]));
- break;
- }
-
+ FileBuffer = BmGetFileBufferFromLoadFile (Handles[Index], FilePath,
+ FullPath, FileSize);
if (FileBuffer != NULL) {
- FreePool (FileBuffer);
+ break;
}
}
@@ -1127,7 +1099,7 @@ BmMatchHttpBootDevicePath (
@return The load option buffer.
**/
VOID *
-BmGetFileBufferFromLoadFileFileSystem (
+BmGetFileBufferFromLoadFileSystem (
IN EFI_HANDLE LoadFileHandle,
OUT EFI_DEVICE_PATH_PROTOCOL **FullPath,
OUT UINTN *FileSize
@@ -1175,8 +1147,78 @@ BmGetFileBufferFromLoadFileFileSystem (
}
}
+
/**
- Get the file buffer from Load File instance.
+ Get the file buffer from the specified Load File instance.
+
+ @param LoadFileHandle The specified Load File instance.
+ @param FilePath The file path which will pass to LoadFile().
+ @param FullPath Return the full device path pointing to the load
option.
+ @param FileSize Return the size of the load option.
+
+ @return The load option buffer or NULL if fails.
+**/
+VOID *
+BmGetFileBufferFromLoadFile (
+ EFI_HANDLE LoadFileHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
+ OUT EFI_DEVICE_PATH_PROTOCOL **FullPath,
+ OUT UINTN *FileSize
+ )
+{
+ EFI_STATUS Status;
+ EFI_LOAD_FILE_PROTOCOL *LoadFile;
+ VOID *FileBuffer;
+ BOOLEAN LoadFileSystem;
+ UINTN BufferSize;
+
+ *FileSize = 0;
+
+ Status = gBS->OpenProtocol (
+ LoadFileHandle,
+ &gEfiLoadFileProtocolGuid,
+ (VOID **) &LoadFile,
+ gImageHandle,
+ NULL,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ FileBuffer = NULL;
+ BufferSize = 0;
+ Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, &BufferSize,
+ FileBuffer); if ((Status != EFI_WARN_FILE_SYSTEM) && (Status !=
EFI_BUFFER_TOO_SMALL)) {
+ return NULL;
+ }
+
+ LoadFileSystem = (BOOLEAN) (Status == EFI_WARN_FILE_SYSTEM);
+ FileBuffer = LoadFileSystem ? AllocateReservedPages (EFI_SIZE_TO_PAGES
+ (BufferSize)) : AllocatePool (BufferSize); if (FileBuffer == NULL) {
+ return NULL;
+ }
+
+ Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, &BufferSize,
+ FileBuffer); if (EFI_ERROR (Status)) {
+ if (LoadFileSystem) {
+ FreePages (FileBuffer, EFI_SIZE_TO_PAGES (BufferSize));
+ } else {
+ FreePool (FileBuffer);
+ }
+ return NULL;
+ }
+
+ if (LoadFileSystem) {
+ FileBuffer = BmGetFileBufferFromLoadFileSystem (LoadFileHandle,
+ FullPath, FileSize); } else {
+ *FileSize = BufferSize;
+ *FullPath = DuplicateDevicePath (DevicePathFromHandle
+ (LoadFileHandle)); }
+
+ return FileBuffer;
+}
+
+/**
+ Get the file buffer from all the Load File instances.
@param FilePath The media device path pointing to a LoadFile instance.
@param FullPath Return the full device path pointing to the load option.
@@ -1185,7 +1227,7 @@ BmGetFileBufferFromLoadFileFileSystem (
@return The load option buffer.
**/
VOID *
-BmGetFileBufferFromLoadFile (
+BmGetFileBufferFromLoadFiles (
IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
OUT EFI_DEVICE_PATH_PROTOCOL **FullPath,
OUT UINTN *FileSize
@@ -1193,13 +1235,10 @@ BmGetFileBufferFromLoadFile ( {
EFI_STATUS Status;
EFI_HANDLE Handle;
- VOID *FileBuffer;
EFI_HANDLE *Handles;
UINTN HandleCount;
UINTN Index;
EFI_DEVICE_PATH_PROTOCOL *Node;
- EFI_LOAD_FILE_PROTOCOL *LoadFile;
- UINTN BufferSize;
//
// Get file buffer from load file instance.
@@ -1244,41 +1283,7 @@ BmGetFileBufferFromLoadFile (
return NULL;
}
- Status = gBS->HandleProtocol (Handle, &gEfiLoadFileProtocolGuid, (VOID **)
&LoadFile);
- ASSERT_EFI_ERROR (Status);
-
- BufferSize = 0;
- FileBuffer = NULL;
- Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, &BufferSize,
FileBuffer);
- if (Status == EFI_BUFFER_TOO_SMALL) {
- FileBuffer = AllocatePool (BufferSize);
- if (FileBuffer != NULL) {
- Status = EFI_SUCCESS;
- }
- }
-
- if (!EFI_ERROR (Status)) {
- Status = LoadFile->LoadFile (LoadFile, FilePath, TRUE, &BufferSize,
FileBuffer);
- }
-
- if (!EFI_ERROR (Status)) {
- //
- // LoadFile() returns a file buffer mapping to a file system.
- //
- if (Status == EFI_WARN_FILE_SYSTEM) {
- return BmGetFileBufferFromLoadFileFileSystem (Handle, FullPath,
FileSize);
- }
-
- ASSERT (Status == EFI_SUCCESS);
- //
- // LoadFile () may cause the device path of the Handle be updated.
- //
- *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handle));
- *FileSize = BufferSize;
- return FileBuffer;
- } else {
- return NULL;
- }
+ return BmGetFileBufferFromLoadFile (Handle, FilePath, FullPath,
+ FileSize);
}
/**
@@ -1394,7 +1399,7 @@ BmGetLoadOptionBuffer (
return FileBuffer;
}
- return BmGetFileBufferFromLoadFile (FilePath, FullPath, FileSize);
+ return BmGetFileBufferFromLoadFiles (FilePath, FullPath, FileSize);
}
/**
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index b261d76..7b6252a 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -410,23 +410,6 @@ BmCharToUint (
IN CHAR16 Char
);
-
-/**
- Get the file buffer from the file system produced by Load File instance.
-
- @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.
-
- @return The load option buffer.
-**/
-VOID *
-BmGetFileBufferFromLoadFileFileSystem (
- IN EFI_HANDLE LoadFileHandle,
- OUT EFI_DEVICE_PATH_PROTOCOL **FullPath,
- OUT UINTN *FileSize
- );
-
/**
Return the boot description for the controller.
@@ -451,4 +434,22 @@ BmMakeBootOptionDescriptionUnique (
EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
UINTN BootOptionCount
);
+
+/**
+ Get the file buffer from the specified Load File instance.
+
+ @param LoadFileHandle The specified Load File instance.
+ @param FilePath The file path which will pass to LoadFile().
+ @param FullPath Return the full device path pointing to the load
option.
+ @param FileSize Return the size of the load option.
+
+ @return The load option buffer or NULL if fails.
+**/
+VOID *
+BmGetFileBufferFromLoadFile (
+ EFI_HANDLE LoadFileHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *FilePath,
+ OUT EFI_DEVICE_PATH_PROTOCOL **FullPath,
+ OUT UINTN *FileSize
+ );
#endif // _INTERNAL_BM_H_
--
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
