[edk2] [PATCH 2/2] ArmVirtPkg/HighMemDxe: check new regions against GCD memory space map

Ard Biesheuvel posted 2 patches 7 years, 7 months ago
[edk2] [PATCH 2/2] ArmVirtPkg/HighMemDxe: check new regions against GCD memory space map
Posted by Ard Biesheuvel 7 years, 7 months ago
Instead of looking at the PCD gArmTokenSpaceGuid.PcdSystemMemoryBase
to decide which DT node covers the memory we are already using, query
the GCD memory space map, which is the authoritative source for this
kind of information

This fixes a problem observed by Michael on platforms where this PCD
is of the 'Patchable' type, which means updates to its value do not
propagate to other modules.

Reported-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/HighMemDxe/HighMemDxe.c   | 30 +++++++++++++-------
 ArmVirtPkg/HighMemDxe/HighMemDxe.inf |  1 -
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.c b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
index 4e41120deff3..aa3f5f6d8956 100644
--- a/ArmVirtPkg/HighMemDxe/HighMemDxe.c
+++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
@@ -30,16 +30,17 @@ InitializeHighMemDxe (
   IN EFI_SYSTEM_TABLE     *SystemTable
   )
 {
-  FDT_CLIENT_PROTOCOL   *FdtClient;
-  EFI_CPU_ARCH_PROTOCOL *Cpu;
-  EFI_STATUS            Status, FindNodeStatus;
-  INT32                 Node;
-  CONST UINT32          *Reg;
-  UINT32                RegSize;
-  UINTN                 AddressCells, SizeCells;
-  UINT64                CurBase;
-  UINT64                CurSize;
-  UINT64                Attributes;
+  FDT_CLIENT_PROTOCOL               *FdtClient;
+  EFI_CPU_ARCH_PROTOCOL             *Cpu;
+  EFI_STATUS                        Status, FindNodeStatus;
+  INT32                             Node;
+  CONST UINT32                      *Reg;
+  UINT32                            RegSize;
+  UINTN                             AddressCells, SizeCells;
+  UINT64                            CurBase;
+  UINT64                            CurSize;
+  UINT64                            Attributes;
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR   GcdDescriptor;
 
   Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
                   (VOID **)&FdtClient);
@@ -73,7 +74,14 @@ InitializeHighMemDxe (
       }
       RegSize -= (AddressCells + SizeCells) * sizeof (UINT32);
 
-      if (PcdGet64 (PcdSystemMemoryBase) != CurBase) {
+      Status = gDS->GetMemorySpaceDescriptor (CurBase, &GcdDescriptor);
+      if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_WARN,
+          "%a: Region 0x%lx - 0x%lx not found in the GCD memory space map\n",
+          __FUNCTION__, CurBase, CurBase + CurSize - 1));
+          continue;
+      }
+      if (GcdDescriptor.GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
         Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, CurBase,
                         CurSize, EFI_MEMORY_WB);
 
diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
index ac1761974f52..a7072e38d09d 100644
--- a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
+++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
@@ -45,7 +45,6 @@ [Protocols]
   gFdtClientProtocolGuid                  ## CONSUMES
 
 [Pcd]
-  gArmTokenSpaceGuid.PcdSystemMemoryBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy
 
 [Depex]
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/2] ArmVirtPkg/HighMemDxe: check new regions against GCD memory space map
Posted by Laszlo Ersek 7 years, 7 months ago
On 03/21/17 10:23, Ard Biesheuvel wrote:
> Instead of looking at the PCD gArmTokenSpaceGuid.PcdSystemMemoryBase
> to decide which DT node covers the memory we are already using, query
> the GCD memory space map, which is the authoritative source for this
> kind of information
> 
> This fixes a problem observed by Michael on platforms where this PCD
> is of the 'Patchable' type, which means updates to its value do not
> propagate to other modules.
> 
> Reported-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/HighMemDxe/HighMemDxe.c   | 30 +++++++++++++-------
>  ArmVirtPkg/HighMemDxe/HighMemDxe.inf |  1 -
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.c b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
> index 4e41120deff3..aa3f5f6d8956 100644
> --- a/ArmVirtPkg/HighMemDxe/HighMemDxe.c
> +++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.c
> @@ -30,16 +30,17 @@ InitializeHighMemDxe (
>    IN EFI_SYSTEM_TABLE     *SystemTable
>    )
>  {
> -  FDT_CLIENT_PROTOCOL   *FdtClient;
> -  EFI_CPU_ARCH_PROTOCOL *Cpu;
> -  EFI_STATUS            Status, FindNodeStatus;
> -  INT32                 Node;
> -  CONST UINT32          *Reg;
> -  UINT32                RegSize;
> -  UINTN                 AddressCells, SizeCells;
> -  UINT64                CurBase;
> -  UINT64                CurSize;
> -  UINT64                Attributes;
> +  FDT_CLIENT_PROTOCOL               *FdtClient;
> +  EFI_CPU_ARCH_PROTOCOL             *Cpu;
> +  EFI_STATUS                        Status, FindNodeStatus;
> +  INT32                             Node;
> +  CONST UINT32                      *Reg;
> +  UINT32                            RegSize;
> +  UINTN                             AddressCells, SizeCells;
> +  UINT64                            CurBase;
> +  UINT64                            CurSize;
> +  UINT64                            Attributes;
> +  EFI_GCD_MEMORY_SPACE_DESCRIPTOR   GcdDescriptor;
>  
>    Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
>                    (VOID **)&FdtClient);
> @@ -73,7 +74,14 @@ InitializeHighMemDxe (
>        }
>        RegSize -= (AddressCells + SizeCells) * sizeof (UINT32);
>  
> -      if (PcdGet64 (PcdSystemMemoryBase) != CurBase) {
> +      Status = gDS->GetMemorySpaceDescriptor (CurBase, &GcdDescriptor);
> +      if (EFI_ERROR (Status)) {
> +        DEBUG ((DEBUG_WARN,
> +          "%a: Region 0x%lx - 0x%lx not found in the GCD memory space map\n",
> +          __FUNCTION__, CurBase, CurBase + CurSize - 1));
> +          continue;
> +      }
> +      if (GcdDescriptor.GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
>          Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, CurBase,
>                          CurSize, EFI_MEMORY_WB);
>  
> diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> index ac1761974f52..a7072e38d09d 100644
> --- a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> +++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf
> @@ -45,7 +45,6 @@ [Protocols]
>    gFdtClientProtocolGuid                  ## CONSUMES
>  
>  [Pcd]
> -  gArmTokenSpaceGuid.PcdSystemMemoryBase
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy
>  
>  [Depex]
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel