Re: [Linaro-uefi] [RFC PATCH 3/8] ArmPlatformPkg: replace some instances of FixedPcdGetXX() with PcdGetXX()

On 12/08/14 19:22, Ard Biesheuvel wrote:

This replaces a number of instances of FixedPcdGetXX() with the respective PcdGetXX() invocation. This will prevent the PCDs from being evaluated to immediate constants, and instead refer to the global variables containing the constants.

This allows PrePi implementations that execute from RAM to override those values, for instance when self relocating to arbitrary RAM offsets.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org

---

.../EarlyFdtPL011SerialPortLib.c | 2 +- .../Library/PlatformPeiLib/PlatformPeiLib.c | 2 +- ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 22 +++++++++++----------- ArmPlatformPkg/PrePi/PrePi.c | 16 ++++++++-------- 4 files changed, 21 insertions(+), 21 deletions(-)

I can't offer an opinion wrt. to the PrePi sources.

With regard to the other two files, the one PCD whose getter you're replacing is PcdDeviceTreeInitialBaseAddress. This should indeed replace the

_PCD_VALUE_PcdDeviceTreeInitialBaseAddress

macro expansion of FixedPcdGet64 (PcdDeviceTreeInitialBaseAddress) with

_gPcd_FixedAtBuild_PcdDeviceTreeInitialBaseAddress

whose definition (incl. initialization, with the same value) is auto-generated; using type "const UINT64".

"PcdDeviceTreeInitialBaseAddress" remains listed under [PcdsFixedAtBuild.common] in "ArmVirtualizationQemu.dsc".

So that should be fine for the current code too (which executes from flash), because we'll never try to change the globals.

So I'm okay with this part of the patch (and have no opinion about the other part).

Acked-by: Laszlo Ersek lersek@redhat.com

Thanks Laszlo

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c b/ArmPlatformPkg/ArmVirtualizationPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c index ba6d277d4571..2f5222efeac8 100644 --- a/ArmPlatformPkg/ArmVirtualizationPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c +++ b/ArmPlatformPkg/ArmVirtualizationPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c @@ -71,7 +71,7 @@ SerialPortGetBaseAddress ( UINTN UartBase; RETURN_STATUS Status;

• DeviceTreeBase = (VOID *)(UINTN)FixedPcdGet64 (PcdDeviceTreeInitialBaseAddress);

• DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);

if ((DeviceTreeBase == NULL) || (fdt_check_header (DeviceTreeBase) != 0)) { return 0; diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c index d180a01415b0..aa6a419bf1b5 100644 --- a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c +++ b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c @@ -42,7 +42,7 @@ PlatformPeim ( UINT64 UartBase;

• Base = (VOID*)(UINTN)FixedPcdGet64 (PcdDeviceTreeInitialBaseAddress);

• Base = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress); ASSERT (fdt_check_header (Base) == 0);

fdt_pack (Base); diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S index fcea9496cbd5..9e1477107d30 100644 --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S @@ -42,19 +42,19 @@ _SetSVCMode: // at the top of the DRAM) _SetupStackPosition: // Compute Top of System Memory

• LoadConstantToReg (FixedPcdGet64 (PcdSystemMemoryBase), x1)
• LoadConstantToReg (FixedPcdGet64 (PcdSystemMemorySize), x2)

• ldr x1, PcdGet64 (PcdSystemMemoryBase)
• ldr x2, PcdGet64 (PcdSystemMemorySize) sub x2, x2, #1 add x1, x1, x2 // x1 = SystemMemoryTop = PcdSystemMemoryBase + PcdSystemMemorySize

// Calculate Top of the Firmware Device

• LoadConstantToReg (FixedPcdGet32(PcdFdBaseAddress), x2)
• LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3)

• ldr x2, PcdGet64 (PcdFdBaseAddress)
• ldr w3, PcdGet32 (PcdFdSize) sub x3, x3, #1 add x3, x3, x2 // x3 = FdTop = PcdFdBaseAddress + PcdFdSize

// UEFI Memory Size (stacks are allocated in this region)

• LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4)

• ldr w4, PcdGet32 (PcdSystemMemoryUefiRegionSize)

// // Reserve the memory for the UEFI region (contain stacks on its top) @@ -96,13 +96,13 @@ _GetBaseUefiMemory: _GetStackBase: // r1 = The top of the Mpcore Stacks // Stack for the primary core = PrimaryCoreStack

• LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2)

• ldr w2, PcdGet32 (PcdCPUCorePrimaryStackSize) sub x12, x1, x2

// Stack for the secondary core = Number of Cores - 1

• LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0)

• ldr w0, PcdGet32 (PcdCoreCount) sub x0, x0, #1

• LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1)

• ldr w1, PcdGet32 (PcdCPUCoreSecondaryStackSize) mul x1, x1, x0 sub x12, x12, x1

@@ -110,8 +110,8 @@ _GetStackBase: mov x0, x12 mov x1, x10 //ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, SecondaryStackSize)

• LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2)
• LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3)

• ldr w2, PcdGet32 (PcdCPUCorePrimaryStackSize)
• ldr w3, PcdGet32 (PcdCPUCoreSecondaryStackSize) bl ASM_PFX(ArmPlatformStackSet)

// Is it the Primary Core ? @@ -121,7 +121,7 @@ _GetStackBase: bne _PrepareArguments _ReserveGlobalVariable:

• LoadConstantToReg (FixedPcdGet32(PcdPeiGlobalVariableSize), x0)

• ldr w0, PcdGet32(PcdPeiGlobalVariableSize) // InitializePrimaryStack($GlobalVariableSize, $Tmp1, $Tmp2) InitializePrimaryStack(x0, x1, x2)

diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c index 9a5e067ef537..2091017a0996 100755 --- a/ArmPlatformPkg/PrePi/PrePi.c +++ b/ArmPlatformPkg/PrePi/PrePi.c @@ -30,8 +30,8 @@ #include "PrePi.h" #include "LzmaDecompress.h" -#define IS_XIP() (((UINT32)FixedPcdGet32 (PcdFdBaseAddress) > (UINT32)(FixedPcdGet64 (PcdSystemMemoryBase) + FixedPcdGet32 (PcdSystemMemorySize))) || \

•              ((FixedPcdGet32 (PcdFdBaseAddress) + FixedPcdGet32 (PcdFdSize)) < FixedPcdGet64 (PcdSystemMemoryBase)))
  

+#define IS_XIP() (((UINT32)PcdGet64 (PcdFdBaseAddress) > (UINT32)(PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize))) || \

•              ((PcdGet64 (PcdFdBaseAddress) + PcdGet32 (PcdFdSize)) < PcdGet64 (PcdSystemMemoryBase)))
  

// Not used when PrePi in run in XIP mode UINTN mGlobalVariableBase = 0; @@ -108,8 +108,8 @@ PrePiMain ( // If ensure the FD is either part of the System Memory or totally outside of the System Memory (XIP) ASSERT (IS_XIP() ||

•      ((FixedPcdGet32 (PcdFdBaseAddress) >= FixedPcdGet64 (PcdSystemMemoryBase)) &&
  

•       ((UINT32)(FixedPcdGet32 (PcdFdBaseAddress) + FixedPcdGet32 (PcdFdSize)) <= (UINT32)(FixedPcdGet64 (PcdSystemMemoryBase) + FixedPcdGet64 (PcdSystemMemorySize)))));
  

•      ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) &&
  

•       ((UINT32)(PcdGet64 (PcdFdBaseAddress) + PcdGet32 (PcdFdSize)) <= (UINT32)(PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize)))));
  

// Initialize the architecture specific bits ArchInitialize (); @@ -127,27 +127,27 @@ PrePiMain ( // Declare the PI/UEFI memory region HobList = HobConstructor ( (VOID*)UefiMemoryBase,

• FixedPcdGet32 (PcdSystemMemoryUefiRegionSize),

• PcdGet32 (PcdSystemMemoryUefiRegionSize), (VOID*)UefiMemoryBase, (VOID*)StacksBase // The top of the UEFI Memory is reserved for the stacks ); PrePeiSetHobList (HobList);

// Initialize MMU and Memory HOBs (Resource Descriptor HOBs)

• Status = MemoryPeim (UefiMemoryBase, FixedPcdGet32 (PcdSystemMemoryUefiRegionSize));

• Status = MemoryPeim (UefiMemoryBase, PcdGet32 (PcdSystemMemoryUefiRegionSize)); ASSERT_EFI_ERROR (Status);

// Create the Stacks HOB (reserve the memory for all stacks) if (ArmIsMpCore ()) { StacksSize = PcdGet32 (PcdCPUCorePrimaryStackSize) +

•             ((FixedPcdGet32 (PcdCoreCount) - 1) * FixedPcdGet32 (PcdCPUCoreSecondaryStackSize));
  

•             ((PcdGet32 (PcdCoreCount) - 1) * PcdGet32 (PcdCPUCoreSecondaryStackSize));
  

  } else { StacksSize = PcdGet32 (PcdCPUCorePrimaryStackSize); } BuildStackHob (StacksBase, StacksSize);

// Declare the Global Variable HOB

• BuildGlobalVariableHob (GlobalVariableBase, FixedPcdGet32 (PcdPeiGlobalVariableSize));

• BuildGlobalVariableHob (GlobalVariableBase, PcdGet32 (PcdPeiGlobalVariableSize));

//TODO: Call CpuPei as a library BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));