Re: [RFC PATCH 06/11] MdeModulePkg/DxeCore: Expose memory mapped FV protocol when possible

["Ni, Ray" <ray.ni@intel.com>]

As I replied to patch #5, we may avoid installing the MMFV protocol.

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb@kernel.org>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>; Bi, Dandan
> <dandan.bi@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Michael Kubacki <mikuback@linux.microsoft.com>
> Subject: [RFC PATCH 06/11] MdeModulePkg/DxeCore: Expose memory mapped
> FV protocol when possible
> 
> Expose the EDK2 specific memory mapped FV protocol in addition to the
> firmware volume protocol defined by PI when the underlying firmware
> volume block protocol informs us that the firmware volume is memory
> mapped. This permits the image loader to access any FFS files in the
> image directly, rather than requiring it to load a cached copy into
> memory via the ReadFile() API. This avoids a redundant memcpy().
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  MdeModulePkg/Core/Dxe/FwVol/FwVol.c       | 113 +++++++++++++++++++-
>  MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h |  31 ++++++
>  2 files changed, 140 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> index 153bfecafa7772ea..f7f236496686bc36 100644
> --- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> @@ -41,7 +41,8 @@ FV_DEVICE  mFvDevice = {
>    0,
> 
>    0,
> 
>    FALSE,
> 
> -  FALSE
> 
> +  FALSE,
> 
> +  { MemoryMappedFvGetLocationAndSize },
> 
>  };
> 
> 
> 
>  //
> 
> @@ -676,11 +677,13 @@ NotifyFwVolBlock (
>          //
> 
>          // Install an New FV protocol on the existing handle
> 
>          //
> 
> -        Status = CoreInstallProtocolInterface (
> 
> +        Status = CoreInstallMultipleProtocolInterfaces (
> 
>                     &Handle,
> 
>                     &gEfiFirmwareVolume2ProtocolGuid,
> 
> -                   EFI_NATIVE_INTERFACE,
> 
> -                   &FvDevice->Fv
> 
> +                   &FvDevice->Fv,
> 
> +                   (FvDevice->IsMemoryMapped ?
> &gEdkiiMemoryMappedFvProtocolGuid : NULL),
> 
> +                   &FvDevice->MemoryMappedFv,
> 
> +                   NULL
> 
>                     );
> 
>          ASSERT_EFI_ERROR (Status);
> 
>        } else {
> 
> @@ -722,3 +725,105 @@ FwVolDriverInit (
>                            );
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> +
> 
> +/**
> 
> +  Get the physical address and size of a file's section in a memory mapped FV
> 
> +
> 
> +  @param[in]  This          The protocol pointer
> 
> +  @param[in]  NameGuid      The name GUID of the file
> 
> +  @param[in]  SectionType   The file section from which to retrieve address and
> size
> 
> +  @param[out] FileAddress   The physical address of the file
> 
> +  @param[out] FileSize      The size of the file
> 
> +  @param[out] AuthStatus    The authentication status associated with the file
> 
> +
> 
> +  @retval EFI_SUCCESS            Information about the file was retrieved
> successfully.
> 
> +  @retval EFI_INVALID_PARAMETER  FileAddress was NULL, FileSize was NULL,
> AuthStatus
> 
> +                                 was NULL.
> 
> +  @retval EFI_NOT_FOUND          No section of the specified type could be
> located in
> 
> +                                 the specified file.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MemoryMappedFvGetLocationAndSize (
> 
> +  IN  EDKII_MEMORY_MAPPED_FV_PROTOCOL  *This,
> 
> +  IN  CONST EFI_GUID                   *NameGuid,
> 
> +  IN  EFI_SECTION_TYPE                 SectionType,
> 
> +  OUT EFI_PHYSICAL_ADDRESS             *FileAddress,
> 
> +  OUT UINTN                            *FileSize,
> 
> +  OUT UINT32                           *AuthStatus
> 
> +  )
> 
> +{
> 
> +  FV_DEVICE                  *FvDevice;
> 
> +  EFI_STATUS                 Status;
> 
> +  EFI_FV_FILETYPE            FileType;
> 
> +  EFI_FV_FILE_ATTRIBUTES     FileAttributes;
> 
> +  EFI_GUID                   FileNameGuid;
> 
> +  FFS_FILE_LIST_ENTRY        *FfsFileEntry;
> 
> +  EFI_COMMON_SECTION_HEADER  *Section;
> 
> +  UINTN                      FvFileSize;
> 
> +  UINTN                      SectionLength;
> 
> +  UINTN                      HeaderLength;
> 
> +
> 
> +  FvDevice     = FV_DEVICE_FROM_MMFV (This);
> 
> +  FfsFileEntry = NULL;
> 
> +
> 
> +  do {
> 
> +    FileType = 0;
> 
> +    Status   = FvGetNextFile (
> 
> +                 &FvDevice->Fv,
> 
> +                 &FfsFileEntry,
> 
> +                 &FileType,
> 
> +                 &FileNameGuid,
> 
> +                 &FileAttributes,
> 
> +                 &FvFileSize
> 
> +                 );
> 
> +    if (EFI_ERROR (Status)) {
> 
> +      return EFI_NOT_FOUND;
> 
> +    }
> 
> +  } while (!CompareGuid (&FileNameGuid, NameGuid));
> 
> +
> 
> +  //
> 
> +  // Skip over file header
> 
> +  //
> 
> +  if (IS_FFS_FILE2 (FfsFileEntry->FfsHeader)) {
> 
> +    Section = (VOID *)((UINT8 *)FfsFileEntry->FfsHeader +
> 
> +                       sizeof (EFI_FFS_FILE_HEADER2));
> 
> +  } else {
> 
> +    Section = (VOID *)((UINT8 *)FfsFileEntry->FfsHeader +
> 
> +                       sizeof (EFI_FFS_FILE_HEADER));
> 
> +  }
> 
> +
> 
> +  do {
> 
> +    if (IS_SECTION2 (Section)) {
> 
> +      SectionLength = SECTION2_SIZE (Section);
> 
> +      HeaderLength  = sizeof (EFI_COMMON_SECTION_HEADER2);
> 
> +    } else {
> 
> +      SectionLength = SECTION_SIZE (Section);
> 
> +      HeaderLength  = sizeof (EFI_COMMON_SECTION_HEADER);
> 
> +    }
> 
> +
> 
> +    if (SectionLength > FvFileSize) {
> 
> +      DEBUG ((DEBUG_WARN, "%a: %x > %x\n", __func__, SectionLength,
> FvFileSize));
> 
> +      break;
> 
> +    }
> 
> +
> 
> +    if (Section->Type == SectionType) {
> 
> +      *FileAddress = (UINTN)Section + HeaderLength;
> 
> +      *FileSize    = SectionLength - HeaderLength;
> 
> +      *AuthStatus  = FvDevice->AuthenticationStatus;
> 
> +
> 
> +      return EFI_SUCCESS;
> 
> +    }
> 
> +
> 
> +    //
> 
> +    // SectionLength is adjusted it is 4 byte aligned.
> 
> +    // Go to the next section
> 
> +    //
> 
> +    SectionLength = ALIGN_VALUE (SectionLength, 4);
> 
> +    Section       = (EFI_COMMON_SECTION_HEADER *)((UINT8 *)Section +
> SectionLength);
> 
> +    FvFileSize   -= SectionLength;
> 
> +  } while (TRUE);
> 
> +
> 
> +  return EFI_NOT_FOUND;
> 
> +}
> 
> diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h
> b/MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h
> index 3403c812b2ebcb45..eb1b702c1ee0bd4f 100644
> --- a/MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h
> +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h
> @@ -40,10 +40,13 @@ typedef struct {
>    UINT8                                 ErasePolarity;
> 
>    BOOLEAN                               IsFfs3Fv;
> 
>    BOOLEAN                               IsMemoryMapped;
> 
> +  EDKII_MEMORY_MAPPED_FV_PROTOCOL       MemoryMappedFv;
> 
>  } FV_DEVICE;
> 
> 
> 
>  #define FV_DEVICE_FROM_THIS(a)  CR(a, FV_DEVICE, Fv,
> FV2_DEVICE_SIGNATURE)
> 
> 
> 
> +#define FV_DEVICE_FROM_MMFV(a)  CR(a, FV_DEVICE, MemoryMappedFv,
> FV2_DEVICE_SIGNATURE)
> 
> +
> 
>  /**
> 
>    Retrieves attributes, insures positive polarity of attribute bits, returns
> 
>    resulting attributes in output parameter.
> 
> @@ -384,4 +387,32 @@ IsValidFfsFile (
>    IN EFI_FFS_FILE_HEADER  *FfsHeader
> 
>    );
> 
> 
> 
> +/**
> 
> +  Get the physical address and size of a file's section in a memory mapped FV
> 
> +
> 
> +  @param[in]  This          The protocol pointer
> 
> +  @param[in]  NameGuid      The name GUID of the file
> 
> +  @param[in]  SectionType   The file section from which to retrieve address and
> size
> 
> +  @param[out] FileAddress   The physical address of the file
> 
> +  @param[out] FileSize      The size of the file
> 
> +  @param[out] AuthStatus    The authentication status associated with the file
> 
> +
> 
> +  @retval EFI_SUCCESS            Information about the file was retrieved
> successfully.
> 
> +  @retval EFI_INVALID_PARAMETER  FileAddress was NULL, FileSize was NULL,
> AuthStatus
> 
> +                                 was NULL.
> 
> +  @retval EFI_NOT_FOUND          No section of the specified type could be
> located in
> 
> +                                 the specified file.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MemoryMappedFvGetLocationAndSize (
> 
> +  IN  EDKII_MEMORY_MAPPED_FV_PROTOCOL  *This,
> 
> +  IN  CONST EFI_GUID                   *NameGuid,
> 
> +  IN  EFI_SECTION_TYPE                 SectionType,
> 
> +  OUT EFI_PHYSICAL_ADDRESS             *FileAddress,
> 
> +  OUT UINTN                            *FileSize,
> 
> +  OUT UINT32                           *AuthStatus
> 
> +  );
> 
> +
> 
>  #endif
> 
> --
> 2.39.2