On 09/08/15 19:35, Ard Biesheuvel wrote: > Make sure that the PEI memory region is carved out of memory that is > 32-bit addressable, by taking MAX_ADDRESS into account (which is > defined as '4 GB - 1' on ARM) > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > index 93ab16ca4a74..9f26d26a04d3 100755 > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > @@ -96,7 +96,7 @@ InitializeMemory ( > { > EFI_STATUS Status; > UINTN SystemMemoryBase; > - UINTN SystemMemoryTop; > + UINT64 SystemMemoryTop; > UINTN FdBase; > UINTN FdTop; > UINTN UefiMemoryBase; > @@ -115,7 +115,10 @@ InitializeMemory ( > ASSERT (PcdGet64 (PcdSystemMemorySize) != 0); > > SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase); > - SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 (PcdSystemMemorySize); > + SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize); > + if (SystemMemoryTop - 1 > MAX_ADDRESS) {
MAX_ADDRESS is an inclusive limit. SystemMemoryTop is an exclusive limit (it comes from base + size, where size is exclusive). In order to see if clamping is needed, we should compare inclusive to inclusive, or exclusive to exclusive, and act if SystemMemoryTop exceeds the maximum (of the same kind). I usually prefer exclusives. Mathematically (ie. regardless of C types etc) that would be: SystemMemoryTop > (MAX_ADDRESS + 1) The RHS is problematic to evaluate in C. The way you solved it (by subtracting 1 from both sides) is okay. Also, this will never fire in the AARCH64 build. > + SystemMemoryTop = (UINT64) MAX_ADDRESS + 1; Please remove the space right after the cast operator, and repost (or fix it up at commit time). With reference to: http://thread.gmane.org/gmane.comp.bios.edk2.devel/1881/focus=1889 With that change: Reviewed-by: Laszlo Ersek <[email protected]> > + } > FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress); > FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize); > > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
