Thanks for your explanation which makes sense to me. I agree that if we don't have a need to support loading file buffer from a HTTP Boot driver mounted RAMDISK for non-BDS core module, this implementation is good and clearer.
Reviewed-by: Sunny Wang <[email protected]> -----Original Message----- From: Ni, Ruiyu [mailto:[email protected]] Sent: Friday, February 26, 2016 8:25 PM To: Wang, Sunny (HPS SW) Cc: Fu, Siyuan; [email protected] Subject: RE: [edk2] [Patch 2/4] MdeModulePkg/Bds: Wide match HTTP boot option. Importance: High Comments below. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:[email protected]] On Behalf Of >Wang, Sunny (HPS SW) >Sent: Friday, February 26, 2016 5:18 PM >To: Ni, Ruiyu <[email protected]> >Cc: Fu, Siyuan <[email protected]>; [email protected] >Subject: Re: [edk2] [Patch 2/4] MdeModulePkg/Bds: Wide match HTTP boot option. > >Hi Ray, > It looks like you replace GetFileBufferByFilePath function call with >both gEfiSimpleFileSystemProtocolGuid handling code block and >BmGetFileBufferFromLoadFile in BmGetLoadOptionBuffer, and the >BmGetFileBufferFromLoadFile is for replacing gEfiLoadFileProtocolGuid related >code block in GetFileBufferByFilePath with the additional code for wide >matching HTTP boot option. Based on the changes I mentioned, I have two >questions below: > 1. Why don't we just add the additional code for wide matching HTTP >boot option into GetFileBufferByFilePath to continue using >GetFileBufferByFilePath? This way seems easier and simpler. GetFileBufferByFilePath() is a core library function. And even we enhance it, it still cannot support the ram disk boot (implemented in Patch 4/4). So I chose to not make the GetFileBufferByFilePath complex. > 2. The GetFileBufferByFilePath function includes a code block for >gEfiLoadFile2ProtocolGuid, but the replacement in your code change >(BmGetFileBufferFromLoadFile) doesn't have code to handle >gEfiLoadFile2ProtocolGuid. Is it an issue or something I overlooked? Per UFEI spec, boot option only maps to LoadFile protocol. LoadFile2 protocol is for PCI option rom loading, not for booting. > >Regards, >Sunny Wang > >-----Original Message----- >From: edk2-devel [mailto:[email protected]] On Behalf Of >Ruiyu Ni >Sent: Wednesday, February 24, 2016 3:40 PM >To: [email protected] >Cc: Ruiyu Ni; Siyuan Fu >Subject: [edk2] [Patch 2/4] MdeModulePkg/Bds: Wide match HTTP boot option. > >Enhance BDS to wide match the HTTP boot option without matching the >specific device path data in IP device path and URI device path node. >It's to follow UEFI Spec 2.6. > >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Ruiyu Ni <[email protected]> >Cc: Siyuan Fu <[email protected]> >--- > MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 149 >+++++++++++++++++++++-- > 1 file changed, 137 insertions(+), 12 deletions(-) > >diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c >b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c >index a6826f6..35234ba 100644 >--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c >+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c >@@ -1528,6 +1528,136 @@ BmExpandMediaDevicePath ( } > > /** >+ Check whether Left and Right are the same without matching the >+ specific device path data in IP device path and URI device path node. >+ >+ @retval TRUE Left and Right are the same. >+ @retval FALSE Left and Right are the different. >+**/ >+BOOLEAN >+BmMatchHttpBootDevicePath ( >+ IN EFI_DEVICE_PATH_PROTOCOL *Left, >+ IN EFI_DEVICE_PATH_PROTOCOL *Right >+ ) >+{ >+ for (; !IsDevicePathEnd (Left) && !IsDevicePathEnd (Right) >+ ; Left = NextDevicePathNode (Left), Right = NextDevicePathNode (Right) >+ ) { >+ if (CompareMem (Left, Right, DevicePathNodeLength (Left)) != 0) { >+ if ((DevicePathType (Left) != MESSAGING_DEVICE_PATH) || >+(DevicePathType (Right) != MESSAGING_DEVICE_PATH)) >{ >+ return FALSE; >+ } >+ >+ if (((DevicePathSubType (Left) != MSG_IPv4_DP) || (DevicePathSubType >(Right) != MSG_IPv4_DP)) && >+ ((DevicePathSubType (Left) != MSG_IPv6_DP) || (DevicePathSubType >(Right) != MSG_IPv6_DP)) && >+ ((DevicePathSubType (Left) != MSG_URI_DP) || (DevicePathSubType >(Right) != MSG_URI_DP)) >+ ) { >+ return FALSE; >+ } >+ } >+ } >+ return (BOOLEAN) (IsDevicePathEnd (Left) && IsDevicePathEnd >+(Right)); } >+ >+/** >+ Get the file buffer from Load File instance. >+ >+ @param FilePath The media device path pointing to a 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 * >+BmGetFileBufferFromLoadFile ( >+ IN EFI_DEVICE_PATH_PROTOCOL *FilePath, >+ OUT EFI_DEVICE_PATH_PROTOCOL **FullPath, >+ OUT UINTN *FileSize >+ ) >+{ >+ 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. >+ // >+ Node = FilePath; >+ Status = gBS->LocateDevicePath (&gEfiLoadFileProtocolGuid, &Node, >+ &Handle); if (!EFI_ERROR (Status) && IsDevicePathEnd (Node)) { >+ // >+ // When wide match happens, pass full device path to LoadFile (), >+ // otherwise, pass remaining device path to LoadFile (). >+ // >+ FilePath = Node; >+ } else { >+ Handle = NULL; >+ // >+ // Use wide match algorithm to find one when >+ // cannot find a LoadFile instance to exactly match the FilePath >+ // >+ Status = gBS->LocateHandleBuffer ( >+ ByProtocol, >+ &gEfiLoadFileProtocolGuid, >+ NULL, >+ &HandleCount, >+ &Handles >+ ); >+ if (EFI_ERROR (Status)) { >+ Handles = NULL; >+ HandleCount = 0; >+ } >+ for (Index = 0; Index < HandleCount; Index++) { >+ if (BmMatchHttpBootDevicePath (DevicePathFromHandle (Handles[Index]), >FilePath)) { >+ Handle = Handles[Index]; >+ break; >+ } >+ } >+ if (Handles != NULL) { >+ FreePool (Handles); >+ } >+ } >+ >+ if (Handle == NULL) { >+ 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 () may cause the device path of the Handle be updated. >+ // >+ *FullPath = DuplicateDevicePath (DevicePathFromHandle (Handle)); >+ *FileSize = BufferSize; >+ return FileBuffer; >+ } else { >+ return NULL; >+ } >+} >+ >+/** > Get the load option by its device path. > > @param FilePath The device path pointing to a load option. >@@ -1622,24 +1752,19 @@ BmGetLoadOptionBuffer ( > } > > // >- // Directly reads the load option when it doesn't reside in simple file >system instance (LoadFile/LoadFile2), >- // or it directly points to a file in simple file system instance. >+ // Get file buffer from simple file system. > // > Node = FilePath; >- Status = gBS->LocateDevicePath (&gEfiLoadFileProtocolGuid, &Node, >&Handle); >- FileBuffer = GetFileBufferByFilePath (TRUE, FilePath, FileSize, >&AuthenticationStatus); >- if (FileBuffer != NULL) { >- if (EFI_ERROR (Status)) { >+ Status = gBS->LocateDevicePath (&gEfiSimpleFileSystemProtocolGuid, >+ &Node, &Handle); if (!EFI_ERROR (Status)) { >+ FileBuffer = GetFileBufferByFilePath (TRUE, FilePath, FileSize, >&AuthenticationStatus); >+ if (FileBuffer != NULL) { > *FullPath = DuplicateDevicePath (FilePath); >- } else { >- // >- // LoadFile () may cause the device path of the Handle be updated. >- // >- *FullPath = AppendDevicePath (DevicePathFromHandle (Handle), Node); > } >+ return FileBuffer; > } > >- return FileBuffer; >+ return BmGetFileBufferFromLoadFile (FilePath, FullPath, FileSize); > } > > /** >-- >2.7.0.windows.1 > >_______________________________________________ >edk2-devel mailing list >[email protected] >https://lists.01.org/mailman/listinfo/edk2-devel >_______________________________________________ >edk2-devel mailing list >[email protected] >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
