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

[Laszlo Ersek <lersek@redhat.com>]

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>