Re: [RFC PATCH 5/7] ArmPlatformPkg/MemoryInitPeim: take MAX_ALLOC_ADDRESS into account

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Fri, 7 Dec 2018 at 13:46, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 12/07/18 12:23, Ard Biesheuvel wrote:
> > Limit the PEI memory region so it will not extend beyond what we can
> > address architecturally when running with 4 KB pages.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> > index 389a2e6f1abd..25db87fb2374 100644
> > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> > @@ -109,8 +109,8 @@ InitializeMemory (
> >
> >    SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
> >    SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
> > -  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
> > -    SystemMemoryTop = (UINT64)MAX_ADDRESS + 1;
> > +  if (SystemMemoryTop - 1 > MAX_ALLOC_ADDRESS) {
> > +    SystemMemoryTop = (UINT64)MAX_ALLOC_ADDRESS + 1;
> >    }
> >    FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
> >    FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
> >
>
> Shouldn't you also update
>
>   ASSERT (PcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ADDRESS);
>
> just above the context visible here?
>

No, it's fine for PcdSystemMemoryBase/PcdSystemMemorySize to describe
a region we cannot access in its entirety, as long as we don't use the
top part (See the next patch as well)