On Tue, May 30, 2017 at 04:40:02PM +0000, Ard Biesheuvel wrote:
diff --git a/Platforms/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S b/Platforms/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S index 4cd8b3154e82..fdb688ca489a 100644 --- a/Platforms/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S +++ b/Platforms/Marvell/Armada/Library/Armada70x0Lib/ARM/ArmPlatformHelper.S @@ -26,36 +26,80 @@ #define CCU_MC_RTBR_OFFSET 0x8 #define CCU_MC_RTBR_TARGET_BASE(Base) (((Base) >> 20) << 10)
+#define SYSTEM_INFO_ADDRESS 0x4000000 +#define SYSINFO_ARRAY_SIZE 0x0 +#define SYSINFO_DRAM_CS0_SIZE 0x1
So, this is not the end of the world, but I'd prefer it if these magic values weren't duplicated across the two versions.
Yeah. I gues that applies equally to all the defines in this file.
That's probably true.
Perhaps I should simply .include a shared .S file here?
Well, it's a .S, so it could be a .h with an __ASSEMBLER__ check? But, yeah, a .include of a shared .S is fine too.
diff --git a/Platforms/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c b/Platforms/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c index 4d80426c177c..85df57164b7f 100644 --- a/Platforms/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c +++ b/Platforms/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c
@@ -71,11 +77,10 @@ ArmPlatformGetVirtualMemoryMap (
ASSERT (VirtualMemoryMap != NULL);
- MemSize = FixedPcdGet64 (PcdSystemMemorySize);
- MemLowSize = MIN (FixedPcdGet64 (PcdDramRemapTarget), MemSize);
- MemLowSize = MIN (FixedPcdGet64 (PcdDramRemapTarget), DiscoveredDramSize); MemHighStart = (UINT64)FixedPcdGet64 (PcdDramRemapTarget) + FixedPcdGet32 (PcdDramRemapSize);
- MemHighSize = MemSize - MemLowSize;
- MemHighSize = DiscoveredDramSize - MemLowSize;
Is the above operation still correct if DiscoverDramSize returned 0?
Yes. The same applies to the original MemSize: the value is only used if it is positive, given the conditional in the hunk below. C allows wrapping arithmetic of unsigned quantities, but I could just as easily but this assignment inside the if ()
Yes please, that wold be clearer.
/ Leif
ResourceAttributes = ( EFI_RESOURCE_ATTRIBUTE_PRESENT | @@ -105,7 +110,7 @@ ArmPlatformGetVirtualMemoryMap ( VirtualMemoryTable[Index].Length = 0x20000000; VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
- if (MemSize > MemLowSize) {
- if (DiscoveredDramSize > MemLowSize) { // // If we have more than MemLowSize worth of DRAM, the remainder will be // mapped at the top of the remapped window.
-- 2.9.3