1. Patchew
  2. EDK2
  3. [edk2-devel] [RFC PATCH 00/11] Permit DXE drivers to execute in place
  4. View patch

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

Ard Biesheuvel posted 11 patches 2 years, 8 months ago
[edk2-devel] [RFC PATCH 04/11] MdeModulePkg/DxeCore: Avoid caching memory mapped FFS files
Posted by Ard Biesheuvel 2 years, 8 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105366): https://edk2.groups.io/g/devel/message/105366
Mute This Topic: https://groups.io/mt/99197136/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 04/11] MdeModulePkg/DxeCore: Avoid caching memory mapped FFS files
Posted by Ni, Ray 2 years, 8 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105408): https://edk2.groups.io/g/devel/message/105408
Mute This Topic: https://groups.io/mt/99197136/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 04/11] MdeModulePkg/DxeCore: Avoid caching memory mapped FFS files
Posted by Ard Biesheuvel 2 years, 8 months ago 
On Tue, 30 May 2023 at 08:03, Ni, Ray <ray.ni@intel.com> wrote:
>
> 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.
>

Fair enought.

I found this commit by Start

commit eb1cace292ff0c66ca11eff4703c9fa16219c2a1
Author: Star Zeng <star.zeng@intel.com>
Date:   Wed Aug 27 08:31:44 2014 +0000

    MdeModulePkg DxeCore: Don't cache memory mapped IO FV.

which changes the caching logic to cache FFS files individually
instead of caching the entire firmware volume if it is memory mapped.

AIUI, firmware volumes could be mapped uncached, so if caching is
still needed in some cases, perhaps we might try to disambiguate these
cases based on the underlying memory region? I.e., if it is system
memory, no caching is used.

The decompressed FV does not need caching on any architecture or
platform, afaict.


>
> > -----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
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105426): https://edk2.groups.io/g/devel/message/105426
Mute This Topic: https://groups.io/mt/99197136/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.