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);
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
