Re: [edk2-devel] [PATCH edk2-platforms v1 1/1] Platform/ARM: Reduce System Memory Size for FVP with RME extensions

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Leif,

Thank you for reviewing the patch and for the feedback.

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 18/04/2023 02:30 pm, Leif Lindholm wrote:
> Hi Sami,
>
> On Mon, Apr 17, 2023 at 14:22:49 +0100, Sami Mujawar wrote:
>> For older FVPs (without support for RME extension) the top 16MB of
>> DRAM1 is reserved as Trusted DRAM. However, the latest FVP Base RevC
>> AEM Model [1] has support for RME extension. When RME extension is
>> present the top 64MB of DRAM1 (i.e. at the top of the 32bit address
>> space) is carved out for four-world support in TF-A [2].
>>
>> Therefore, reduce the System Memory size by 64MB.
> This is clearly needed in order to not break things, so
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> *but*
> this is really just extending the life of a horrible hack.
>
> Ideally, the ARM ltd. platforms should lead the way in providing
> reference implementations - since inevitably people will look to those
> for creating new platform ports. Things like this should be runtime
> discoverable.

[SAMI] TF-A has this information in the platform DT, which we should be 
able to consume and setup the system memory size dynamically. However, 
it will be good to get this information from TF-A using the firmware 
handoff protocol (https://developer.arm.com/documentation/den0135/a). I 
will check with the TF-A team to understand when they plan to implement 
this feature.
I will merge this patch for now. Once we have the firmware handoff 
protocol implemented in TF-A, we can add corresponding support in edk2.

[/SAMI]

>
> /
>      Leif
>
>> Reference:
>> [1] FVP Base RevC AEM Model (available on x86_64 / Arm64 Linux)
>> (https://developer.arm.com/Tools%20and%20Software/
>> Fixed%20Virtual%20Platforms)
>>
>> [2] commit c872072 (https://review.trustedfirmware.org/plugins/gitiles/
>> TF-A/trusted-firmware-a/+/c8720729726faffc39ec64f3a02440a48c8c305a))
>>
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>>   Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
>> index dc081794cf98a27c667ef85bd27dacd80e9e8bd2..f70a4d52ba06f570e017ab5286f06d87193753e5 100644
>> --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
>> +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
>> @@ -107,9 +107,12 @@ [PcdsFixedAtBuild.common]
>>     gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
>>     gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize|0x0
>>   
>> -  # System Memory (2GB - 16MB of Trusted DRAM at the top of the 32bit address space)
>> +  # System Memory
>> +  # When RME is supported by the FVP the top 64MB of DRAM1 (i.e. at the top
>> +  # of the 32bit address space) is reserved for four-world support in TF-A.
>> +  # Therefore, set the default System Memory size to (2GB - 64MB).
>>     gArmTokenSpaceGuid.PcdSystemMemoryBase|0x80000000
>> -  gArmTokenSpaceGuid.PcdSystemMemorySize|0x7F000000
>> +  gArmTokenSpaceGuid.PcdSystemMemorySize|0x7C000000
>>   
>>     # Size of the region used by UEFI in permanent memory (Reserved 64MB)
>>     gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000
>> -- 
>> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>>
>>
>>
>> 
>>
>>