On 23 January 2015 at 19:38, Laszlo Ersek <[email protected]> wrote:
> 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 <[email protected]>
>> ---
>>  .../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.
>

Actually, now that you put it like that, it is quite obvious that
patchable PCDs are a lot more appropriate here: we wouldn't be using
the deploy time patch tools, but it would make the build tools report
inadvertent FixedPcd references to PCDs that cannot be guaranteed to
retain their build time value.

-- 
Ard.

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to