Re: [RFC PATCH 04/11] MdeModulePkg/DxeCore: Avoid caching memory mapped FFS files

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

I don't know why the FFS file is cached. Without knowing the reason of caching, I cannot say it's good to remove the caching logic.
I will let @Gao, Liming, @Kinney, Michael D to comment.


> -----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 04/11] MdeModulePkg/DxeCore: Avoid caching memory
> mapped FFS files
> 
> If a firmware volume is memory mapped, it means we can access it
> contents directly, and so caching serves little purpose beyond
> potentially a minor performance improvement. However, given that most
> files are read only a single time, and dispatched from a decompressed
> firmware volume in DRAM, we can just avoid the redundant caching here.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c | 22 --------------------
>  1 file changed, 22 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> b/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> index 2ff22c93aad48d7e..69df96685d680868 100644
> --- a/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> @@ -284,7 +284,6 @@ FvReadFile (
>    UINT8                   *SrcPtr;
> 
>    EFI_FFS_FILE_HEADER     *FfsHeader;
> 
>    UINTN                   InputBufferSize;
> 
> -  UINTN                   WholeFileSize;
> 
> 
> 
>    if (NameGuid == NULL) {
> 
>      return EFI_INVALID_PARAMETER;
> 
> @@ -316,27 +315,6 @@ FvReadFile (
>    // Get a pointer to the header
> 
>    //
> 
>    FfsHeader = FvDevice->LastKey->FfsHeader;
> 
> -  if (FvDevice->IsMemoryMapped) {
> 
> -    //
> 
> -    // Memory mapped FV has not been cached, so here is to cache by file.
> 
> -    //
> 
> -    if (!FvDevice->LastKey->FileCached) {
> 
> -      //
> 
> -      // Cache FFS file to memory buffer.
> 
> -      //
> 
> -      WholeFileSize = IS_FFS_FILE2 (FfsHeader) ? FFS_FILE2_SIZE (FfsHeader) :
> FFS_FILE_SIZE (FfsHeader);
> 
> -      FfsHeader     = AllocateCopyPool (WholeFileSize, FfsHeader);
> 
> -      if (FfsHeader == NULL) {
> 
> -        return EFI_OUT_OF_RESOURCES;
> 
> -      }
> 
> -
> 
> -      //
> 
> -      // Let FfsHeader in FfsFileEntry point to the cached file buffer.
> 
> -      //
> 
> -      FvDevice->LastKey->FfsHeader  = FfsHeader;
> 
> -      FvDevice->LastKey->FileCached = TRUE;
> 
> -    }
> 
> -  }
> 
> 
> 
>    //
> 
>    // Remember callers buffer size
> 
> --
> 2.39.2