Re: [Xen-devel] [PATCH v1 03/21] ArmVirtualizationPkg: replace instance of FixedPcdGet()

[Laszlo Ersek <lersek@xxxxxxxxxx>]

On 01/23/15 16:02, Ard Biesheuvel wrote:
> This removes an instance of FixedPcdGet () so that the self relocating
> PrePi instance can poke another value into it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---
>  .../ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c    | 2 
> +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c
>  
> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c
> index aa4ced4582e8..3e3074af72f1 100644
> --- 
> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c
> +++ 
> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c
> @@ -96,7 +96,7 @@ ArmPlatformInitializeSystemMemory (
>    ASSERT (HobData != NULL);
>    *HobData = 0;
>  
> -  DeviceTreeBase = (VOID *)(UINTN)FixedPcdGet64 
> (PcdDeviceTreeInitialBaseAddress);
> +  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
>    ASSERT (DeviceTreeBase != NULL);
>  
>    //
> 

Care to include some more rationale in the commit message? From here:

http://lists.linaro.org/pipermail/linaro-uefi/2014-December/000604.html
http://lists.linaro.org/pipermail/linaro-uefi/2014-December/000613.html

You're gearing up to something nasty here, so it bears a bit more
explanation IMO :)

Also, after the relocation, FixedPcdGet64() and PcdGet64() would not
return the same value for the same PCD (despite it being a fixed PCD),
which is presumably a "first" in edk2. I can't recommend an alternative,
but please put a warning comment in the code.

Acked-by: Laszlo Ersek <lersek@xxxxxxxxxx>

Thanks
Laszlo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel