On Wed, Sep 09, 2015 at 05:11:33PM +0200, Ard Biesheuvel wrote: > On 9 September 2015 at 16:53, Leif Lindholm <leif.lindh...@linaro.org> wrote: > > On Wed, Sep 09, 2015 at 04:38:08PM +0200, Ard Biesheuvel wrote: > >> On 9 September 2015 at 16:31, Leif Lindholm <leif.lindh...@linaro.org> > >> wrote: > >> > On Tue, Sep 08, 2015 at 07:35:40PM +0200, 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 <ard.biesheu...@linaro.org> > >> >> --- > >> >> 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) { > >> >> + SystemMemoryTop = (UINT64) MAX_ADDRESS + 1; > >> >> + } > >> >> FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress); > >> >> FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize); > >> >> > >> >> -- > >> >> 1.9.1 > >> > > >> > So, this is mainly an OCD thing on my end, but is there any real > >> > benefit to _not_ converting SystemMemoryBase to 64-bit? Doing so would > >> > get rid of the other UINTN cast. > >> > > >> > >> Not really, other than the fact that UINTN is more suitable for the > >> base of DRAM, since you won't be able to run 32-bit UEFI if RAM starts > >> above 4 GB anyway, so changing it to UINT64 seemed wrong to me. > > > > Sure, but having it 64-bit would permit an assert to noticeably tell > > someone they had put an extra 0 into their Pcd, instead of > > successfully passing this function and failing mysteriously later on. > > > > Shall I add this on top? > > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > index 25baac170c6c..e7880d30b1c8 100755 > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c > @@ -113,6 +113,7 @@ InitializeMemory ( > > // Ensure PcdSystemMemorySize has been set > ASSERT (PcdGet64 (PcdSystemMemorySize) != 0); > + ASSERT (PcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ADDRESS); > > SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase); > SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
That would be lovely, thanks. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel