On Wed, Sep 09, 2015 at 05:11:33PM +0200, Ard Biesheuvel wrote:
> On 9 September 2015 at 16:53, Leif Lindholm <[email protected]> wrote:
> > On Wed, Sep 09, 2015 at 04:38:08PM +0200, Ard Biesheuvel wrote:
> >> On 9 September 2015 at 16:31, Leif Lindholm <[email protected]>
> >> 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 <[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) {
> >> >> + 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
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
