diff mbox

[V2] Move RTSM VExpress variable storage to 256k flash blocks

Message ID 1386886130-12494-2-git-send-email-roy.franz@linaro.org
State New
Headers show

Commit Message

Roy Franz Dec. 12, 2013, 10:08 p.m. UTC
Change the addresses/sizes of the variable storage areas to use 256k
blocks so UEFI is compatible with both the RTSM models and QEMU.

The VExpress flash has non-uniform block sizes, with most blocks being
256k and the top 4 blocks being 64k.  UEFI has been using these top 64k
blocks for persistent variable storage.  The RTSM models the non-uniform
sizes, while QEMU only supports emulating flash with uniform block sizes
which results in the top 256k (the 4 64k blocks) of flash being unusable
for writing in QEMU.  The ARM UEFI NOR flash driver currently requires
that firmware volumes start at the base of a flash region, so the
variables are now stored at the base the region that consists of
the 256k blocks.  It was previously at the base of the region
of 64k blocks.

Note that this change will require RTSM flash images to be updated, as
the variable storage has moved.  Currently only the A15 model is supported
by QEMU RTSM VExpress configurations.  This patch only changes
the A15 configurations.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc     |   12 ++++++------
 .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc         |   12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

Comments

Ryan Harkin Dec. 19, 2013, 3:40 p.m. UTC | #1
I just wanted to confirm that this works in the 13.12 release and in my own
builds from the latest tree.

Thanks Roy!


On 12 December 2013 22:08, Roy Franz <roy.franz@linaro.org> wrote:

> Change the addresses/sizes of the variable storage areas to use 256k
> blocks so UEFI is compatible with both the RTSM models and QEMU.
>
> The VExpress flash has non-uniform block sizes, with most blocks being
> 256k and the top 4 blocks being 64k.  UEFI has been using these top 64k
> blocks for persistent variable storage.  The RTSM models the non-uniform
> sizes, while QEMU only supports emulating flash with uniform block sizes
> which results in the top 256k (the 4 64k blocks) of flash being unusable
> for writing in QEMU.  The ARM UEFI NOR flash driver currently requires
> that firmware volumes start at the base of a flash region, so the
> variables are now stored at the base the region that consists of
> the 256k blocks.  It was previously at the base of the region
> of 64k blocks.
>
> Note that this change will require RTSM flash images to be updated, as
> the variable storage has moved.  Currently only the A15 model is supported
> by QEMU RTSM VExpress configurations.  This patch only changes
> the A15 configurations.
>
> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> ---
>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc     |   12
> ++++++------
>  .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc         |   12
> ++++++------
>  2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> index 2d12f4b..c0196d9 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> @@ -77,12 +77,12 @@
>    #
>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
>    #
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> -
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD0000
> -
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> +
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C040000
> +
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
>
>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> index efd80ab..69088ff 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> @@ -79,12 +79,12 @@
>    #
>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
>    #
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> -
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD0000
> -
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> +
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C040000
> +
>  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
>
>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>
> --
> 1.7.10.4
>
>
Roy Franz Jan. 10, 2014, 4:10 p.m. UTC | #2
Hi Olivier - Any thoughts on this?

Thanks,
Roy


On Thu, Dec 19, 2013 at 7:40 AM, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> I just wanted to confirm that this works in the 13.12 release and in my own
> builds from the latest tree.
>
> Thanks Roy!
>
>
> On 12 December 2013 22:08, Roy Franz <roy.franz@linaro.org> wrote:
>>
>> Change the addresses/sizes of the variable storage areas to use 256k
>> blocks so UEFI is compatible with both the RTSM models and QEMU.
>>
>> The VExpress flash has non-uniform block sizes, with most blocks being
>> 256k and the top 4 blocks being 64k.  UEFI has been using these top 64k
>> blocks for persistent variable storage.  The RTSM models the non-uniform
>> sizes, while QEMU only supports emulating flash with uniform block sizes
>> which results in the top 256k (the 4 64k blocks) of flash being unusable
>> for writing in QEMU.  The ARM UEFI NOR flash driver currently requires
>> that firmware volumes start at the base of a flash region, so the
>> variables are now stored at the base the region that consists of
>> the 256k blocks.  It was previously at the base of the region
>> of 64k blocks.
>>
>> Note that this change will require RTSM flash images to be updated, as
>> the variable storage has moved.  Currently only the A15 model is supported
>> by QEMU RTSM VExpress configurations.  This patch only changes
>> the A15 configurations.
>>
>> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> ---
>>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc     |   12
>> ++++++------
>>  .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc         |   12
>> ++++++------
>>  2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> index 2d12f4b..c0196d9 100644
>> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> @@ -77,12 +77,12 @@
>>    #
>>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
>>    #
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
>> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD0000
>> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C040000
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
>>
>>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>>
>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
>> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
>> index efd80ab..69088ff 100644
>> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
>> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
>> @@ -79,12 +79,12 @@
>>    #
>>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
>>    #
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
>> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD0000
>> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C040000
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
>>
>>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>>
>> --
>> 1.7.10.4
>>
>
Olivier Martin Jan. 13, 2014, 7:16 p.m. UTC | #3
Hi Roy,

I need to think more about it. And I also need your point of view on some points.

Here are few questions and thoughts:
- Do you think the flash topology is the only difference between qEmu and RTSM? I believe it is not the only difference. I think the Cortex A15 used to not support the Security extension in qEmu. IS it still the case?
- The correct approach when you have flash with 2 regions (small and large block size regions) is to use the fine grain for the UEFI variables (faster accesses in NVM). Using the large grain when the fine grain is available would not be appropriate. But at least it would ease the support between qEmu and RTSM.
- In the past the RTSM team told me that the goal of RTSM was to be able to run the same binary you would run on your HW onto the model (RTSM). But it is actually not the case with RTSM A15 (its HW equivalent should be the ARM Test Chip ver1 (TC1)) as the HW and RTSM memory map are different...

There are two solutions:
1) either I accept your patch - that will ease the maintenance (no difference between qEmu and RTSM support)
2) or I reject your patch. And I ask you to submit a new version that would introduce a build flag (eg: SUPPORT_QEMU) to differentiate RTSM and qEmu settings in Tianocore.

I have to admit I prefer the solution 2) - but I am quite open to any valid arguments. My argument is I would prefer to expose the correct implementation when possible. And if qEmu adds support for the missing VExpress IP block in the future then it will be easier to restore the correct approach (ie: the default RTSM approach) for qEmu.

Thanks,
Olivier


> -----Original Message-----
> From: Roy Franz [mailto:roy.franz@linaro.org]
> Sent: 10 January 2014 16:11
> To: ryan.harkin@linaro.org; Olivier Martin
> Cc: edk2-devel@lists.sourceforge.net; linaro-uefi; Patch Tracking
> Subject: Re: [PATCH V2] Move RTSM VExpress variable storage to 256k
> flash blocks
> 
> Hi Olivier - Any thoughts on this?
> 
> Thanks,
> Roy
> 
> 
> On Thu, Dec 19, 2013 at 7:40 AM, Ryan Harkin <ryan.harkin@linaro.org>
> wrote:
> > I just wanted to confirm that this works in the 13.12 release and in
> my own
> > builds from the latest tree.
> >
> > Thanks Roy!
> >
> >
> > On 12 December 2013 22:08, Roy Franz <roy.franz@linaro.org> wrote:
> >>
> >> Change the addresses/sizes of the variable storage areas to use 256k
> >> blocks so UEFI is compatible with both the RTSM models and QEMU.
> >>
> >> The VExpress flash has non-uniform block sizes, with most blocks
> being
> >> 256k and the top 4 blocks being 64k.  UEFI has been using these top
> 64k
> >> blocks for persistent variable storage.  The RTSM models the non-
> uniform
> >> sizes, while QEMU only supports emulating flash with uniform block
> sizes
> >> which results in the top 256k (the 4 64k blocks) of flash being
> unusable
> >> for writing in QEMU.  The ARM UEFI NOR flash driver currently
> requires
> >> that firmware volumes start at the base of a flash region, so the
> >> variables are now stored at the base the region that consists of
> >> the 256k blocks.  It was previously at the base of the region
> >> of 64k blocks.
> >>
> >> Note that this change will require RTSM flash images to be updated,
> as
> >> the variable storage has moved.  Currently only the A15 model is
> supported
> >> by QEMU RTSM VExpress configurations.  This patch only changes
> >> the A15 configurations.
> >>
> >> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> ---
> >>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc     |   12
> >> ++++++------
> >>  .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc         |   12
> >> ++++++------
> >>  2 files changed, 12 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> index 2d12f4b..c0196d9 100644
> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> @@ -77,12 +77,12 @@
> >>    #
> >>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
> >>    #
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00
> 00
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100
> 00
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400
> 00
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400
> 00
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
> >>
> >>    gArmTokenSpaceGuid.PcdVFPEnabled|1
> >>
> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-
> A15_MPCore.dsc
> >> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> >> index efd80ab..69088ff 100644
> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> >> @@ -79,12 +79,12 @@
> >>    #
> >>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
> >>    #
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00
> 00
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100
> 00
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400
> 00
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400
> 00
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
> >>
> >>    gArmTokenSpaceGuid.PcdVFPEnabled|1
> >>
> >> --
> >> 1.7.10.4
> >>
> >





------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
Olivier Martin Jan. 13, 2014, 7:16 p.m. UTC | #4
Hi Roy,

I need to think more about it. And I also need your point of view on some points.

Here are few questions and thoughts:
- Do you think the flash topology is the only difference between qEmu and RTSM? I believe it is not the only difference. I think the Cortex A15 used to not support the Security extension in qEmu. IS it still the case?
- The correct approach when you have flash with 2 regions (small and large block size regions) is to use the fine grain for the UEFI variables (faster accesses in NVM). Using the large grain when the fine grain is available would not be appropriate. But at least it would ease the support between qEmu and RTSM.
- In the past the RTSM team told me that the goal of RTSM was to be able to run the same binary you would run on your HW onto the model (RTSM). But it is actually not the case with RTSM A15 (its HW equivalent should be the ARM Test Chip ver1 (TC1)) as the HW and RTSM memory map are different...

There are two solutions:
1) either I accept your patch - that will ease the maintenance (no difference between qEmu and RTSM support)
2) or I reject your patch. And I ask you to submit a new version that would introduce a build flag (eg: SUPPORT_QEMU) to differentiate RTSM and qEmu settings in Tianocore.

I have to admit I prefer the solution 2) - but I am quite open to any valid arguments. My argument is I would prefer to expose the correct implementation when possible. And if qEmu adds support for the missing VExpress IP block in the future then it will be easier to restore the correct approach (ie: the default RTSM approach) for qEmu.

Thanks,
Olivier


> -----Original Message-----
> From: Roy Franz [mailto:roy.franz@linaro.org]
> Sent: 10 January 2014 16:11
> To: ryan.harkin@linaro.org; Olivier Martin
> Cc: edk2-devel@lists.sourceforge.net; linaro-uefi; Patch Tracking
> Subject: Re: [PATCH V2] Move RTSM VExpress variable storage to 256k
> flash blocks
> 
> Hi Olivier - Any thoughts on this?
> 
> Thanks,
> Roy
> 
> 
> On Thu, Dec 19, 2013 at 7:40 AM, Ryan Harkin <ryan.harkin@linaro.org>
> wrote:
> > I just wanted to confirm that this works in the 13.12 release and in
> my own
> > builds from the latest tree.
> >
> > Thanks Roy!
> >
> >
> > On 12 December 2013 22:08, Roy Franz <roy.franz@linaro.org> wrote:
> >>
> >> Change the addresses/sizes of the variable storage areas to use 256k
> >> blocks so UEFI is compatible with both the RTSM models and QEMU.
> >>
> >> The VExpress flash has non-uniform block sizes, with most blocks
> being
> >> 256k and the top 4 blocks being 64k.  UEFI has been using these top
> 64k
> >> blocks for persistent variable storage.  The RTSM models the non-
> uniform
> >> sizes, while QEMU only supports emulating flash with uniform block
> sizes
> >> which results in the top 256k (the 4 64k blocks) of flash being
> unusable
> >> for writing in QEMU.  The ARM UEFI NOR flash driver currently
> requires
> >> that firmware volumes start at the base of a flash region, so the
> >> variables are now stored at the base the region that consists of
> >> the 256k blocks.  It was previously at the base of the region
> >> of 64k blocks.
> >>
> >> Note that this change will require RTSM flash images to be updated,
> as
> >> the variable storage has moved.  Currently only the A15 model is
> supported
> >> by QEMU RTSM VExpress configurations.  This patch only changes
> >> the A15 configurations.
> >>
> >> Signed-off-by: Roy Franz <roy.franz@linaro.org>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> ---
> >>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc     |   12
> >> ++++++------
> >>  .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc         |   12
> >> ++++++------
> >>  2 files changed, 12 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> index 2d12f4b..c0196d9 100644
> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
> >> @@ -77,12 +77,12 @@
> >>    #
> >>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
> >>    #
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00
> 00
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100
> 00
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400
> 00
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400
> 00
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
> >>
> >>    gArmTokenSpaceGuid.PcdVFPEnabled|1
> >>
> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-
> A15_MPCore.dsc
> >> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> >> index efd80ab..69088ff 100644
> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
> >> @@ -79,12 +79,12 @@
> >>    #
> >>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
> >>    #
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00
> 00
> >> -
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100
> 00
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
> >> -
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400
> 00
> >> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400
> 00
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
> >> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
> >>
> >>    gArmTokenSpaceGuid.PcdVFPEnabled|1
> >>
> >> --
> >> 1.7.10.4
> >>
> >
Roy Franz Jan. 14, 2014, 1:31 a.m. UTC | #5
Hi Olivier,

On Mon, Jan 13, 2014 at 11:16 AM, Olivier Martin <olivier.martin@arm.com> wrote:
> Hi Roy,
>
> I need to think more about it. And I also need your point of view on some points.
>
> Here are few questions and thoughts:
> - Do you think the flash topology is the only difference between qEmu and RTSM? I believe it is not the only difference. I think the Cortex A15 used to not support the Security extension in qEmu. IS it still the case?

QEMU reports that it does implement security extensions, but only
partially does.  As part of supporting UEFI  VBAR register support was
added to QEMU.  Other users had posted patches as they wanted this as
well, and these patches got merged since UEFI needed them as well.  I
am sure that there are many remaining differences between RTSM and
QEMU, but only a few matter to UEFI, and I think I have addressed all
of them.  (some by changing QEMU, and some by changing UEFI.)



> - The correct approach when you have flash with 2 regions (small and large block size regions) is to use the fine grain for the UEFI variables (faster accesses in NVM). Using the large grain when the fine grain is available would not be appropriate. But at least it would ease the support between qEmu and RTSM.
I agree that in general using the smaller blocks is preferable, but I
think moving them to the larger blocks supported by both models is a
reasonable compromise to allow a single binary to boot on both.

> - In the past the RTSM team told me that the goal of RTSM was to be able to run the same binary you would run on your HW onto the model (RTSM). But it is actually not the case with RTSM A15 (its HW equivalent should be the ARM Test Chip ver1 (TC1)) as the HW and RTSM memory map are different.
>
> There are two solutions:
> 1) either I accept your patch - that will ease the maintenance (no difference between qEmu and RTSM support)
> 2) or I reject your patch. And I ask you to submit a new version that would introduce a build flag (eg: SUPPORT_QEMU) to differentiate RTSM and qEmu settings in Tianocore.
>
> I have to admit I prefer the solution 2) - but I am quite open to any valid arguments. My argument is I would prefer to expose the correct implementation when possible. And if qEmu adds support for the missing VExpress IP block in the future then it will be easier to restore the correct approach (ie: the default RTSM approach) for qEmu.  I don't think that the flash will every be fixed in QEMU, since accurate modeling of flash writing is not all that valuable of a feature.

I can add a compile option for the flash, as I have done for the
networking.  The networking required this due to conflicting ethernet
devices.  (And here QEMU matches real hardware, and it is RTSM that is
'wrong'.)  I think that the value of a common binary for a common case
is worth the cost of moving the variable storage to larger blocks, but
if you remain unconvinced I'll resubmit the patch with a build option
:)  (and in that case I will also resubmit the networking patch so the
same build option is used for both QEMU related changes.)

Roy


>
> Thanks,
> Olivier
>
>
>> -----Original Message-----
>> From: Roy Franz [mailto:roy.franz@linaro.org]
>> Sent: 10 January 2014 16:11
>> To: ryan.harkin@linaro.org; Olivier Martin
>> Cc: edk2-devel@lists.sourceforge.net; linaro-uefi; Patch Tracking
>> Subject: Re: [PATCH V2] Move RTSM VExpress variable storage to 256k
>> flash blocks
>>
>> Hi Olivier - Any thoughts on this?
>>
>> Thanks,
>> Roy
>>
>>
>> On Thu, Dec 19, 2013 at 7:40 AM, Ryan Harkin <ryan.harkin@linaro.org>
>> wrote:
>> > I just wanted to confirm that this works in the 13.12 release and in
>> my own
>> > builds from the latest tree.
>> >
>> > Thanks Roy!
>> >
>> >
>> > On 12 December 2013 22:08, Roy Franz <roy.franz@linaro.org> wrote:
>> >>
>> >> Change the addresses/sizes of the variable storage areas to use 256k
>> >> blocks so UEFI is compatible with both the RTSM models and QEMU.
>> >>
>> >> The VExpress flash has non-uniform block sizes, with most blocks
>> being
>> >> 256k and the top 4 blocks being 64k.  UEFI has been using these top
>> 64k
>> >> blocks for persistent variable storage.  The RTSM models the non-
>> uniform
>> >> sizes, while QEMU only supports emulating flash with uniform block
>> sizes
>> >> which results in the top 256k (the 4 64k blocks) of flash being
>> unusable
>> >> for writing in QEMU.  The ARM UEFI NOR flash driver currently
>> requires
>> >> that firmware volumes start at the base of a flash region, so the
>> >> variables are now stored at the base the region that consists of
>> >> the 256k blocks.  It was previously at the base of the region
>> >> of 64k blocks.
>> >>
>> >> Note that this change will require RTSM flash images to be updated,
>> as
>> >> the variable storage has moved.  Currently only the A15 model is
>> supported
>> >> by QEMU RTSM VExpress configurations.  This patch only changes
>> >> the A15 configurations.
>> >>
>> >> Signed-off-by: Roy Franz <roy.franz@linaro.org>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> ---
>> >>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc     |   12
>> >> ++++++------
>> >>  .../ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc         |   12
>> >> ++++++------
>> >>  2 files changed, 12 insertions(+), 12 deletions(-)
>> >>
>> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> >> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> >> index 2d12f4b..c0196d9 100644
>> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
>> >> @@ -77,12 +77,12 @@
>> >>    #
>> >>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
>> >>    #
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
>> >> -
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00
>> 00
>> >> -
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100
>> 00
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
>> >> +
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400
>> 00
>> >> +
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400
>> 00
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
>> >>
>> >>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>> >>
>> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-
>> A15_MPCore.dsc
>> >> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
>> >> index efd80ab..69088ff 100644
>> >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
>> >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
>> >> @@ -79,12 +79,12 @@
>> >>    #
>> >>    # NV Storage PCDs. Use base of 0x0C000000 for NOR1
>> >>    #
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
>> >> -
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD00
>> 00
>> >> -
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000100
>> 00
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
>> >> -
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
>> >> +
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C0400
>> 00
>> >> +
>> >>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x000400
>> 00
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
>> >> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
>> >>
>> >>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>> >>
>> >> --
>> >> 1.7.10.4
>> >>
>> >
>
>
>
>
Olivier Martin Jan. 14, 2014, 6:09 p.m. UTC | #6
> -----Original Message-----
> From: Roy Franz [mailto:roy.franz@linaro.org]
> Sent: 14 January 2014 01:31
> To: Olivier Martin
> Cc: ryan.harkin@linaro.org; edk2-devel@lists.sourceforge.net; linaro-
> uefi; Patch Tracking
> Subject: Re: [PATCH V2] Move RTSM VExpress variable storage to 256k
> flash blocks
> 
> > I have to admit I prefer the solution 2) - but I am quite open to any
> valid arguments. My argument is I would prefer to expose the correct
> implementation when possible. And if qEmu adds support for the missing
> VExpress IP block in the future then it will be easier to restore the
> correct approach (ie: the default RTSM approach) for qEmu.  I don't
> think that the flash will every be fixed in QEMU, since accurate
> modeling of flash writing is not all that valuable of a feature.
> 
> I can add a compile option for the flash, as I have done for the
> networking.  The networking required this due to conflicting ethernet
> devices.  (And here QEMU matches real hardware, and it is RTSM that is
> 'wrong'.)  I think that the value of a common binary for a common case
> is worth the cost of moving the variable storage to larger blocks, but
> if you remain unconvinced I'll resubmit the patch with a build option
> :)  (and in that case I will also resubmit the networking patch so the
> same build option is used for both QEMU related changes.)
> 

Nice try! but I remain unconvinced :-)
>From what you said, we will need a compiler flag for the Ethernet driver anyway. So let's go for it :-)
And it will quite easy to 'grep' for SUPPORT_QEMU to highlight the difference between the different models than to go through the git history to find out the workaround which has been made to use UEFI on qEmu.




------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
Olivier Martin Jan. 14, 2014, 6:09 p.m. UTC | #7
> -----Original Message-----
> From: Roy Franz [mailto:roy.franz@linaro.org]
> Sent: 14 January 2014 01:31
> To: Olivier Martin
> Cc: ryan.harkin@linaro.org; edk2-devel@lists.sourceforge.net; linaro-
> uefi; Patch Tracking
> Subject: Re: [PATCH V2] Move RTSM VExpress variable storage to 256k
> flash blocks
> 
> > I have to admit I prefer the solution 2) - but I am quite open to any
> valid arguments. My argument is I would prefer to expose the correct
> implementation when possible. And if qEmu adds support for the missing
> VExpress IP block in the future then it will be easier to restore the
> correct approach (ie: the default RTSM approach) for qEmu.  I don't
> think that the flash will every be fixed in QEMU, since accurate
> modeling of flash writing is not all that valuable of a feature.
> 
> I can add a compile option for the flash, as I have done for the
> networking.  The networking required this due to conflicting ethernet
> devices.  (And here QEMU matches real hardware, and it is RTSM that is
> 'wrong'.)  I think that the value of a common binary for a common case
> is worth the cost of moving the variable storage to larger blocks, but
> if you remain unconvinced I'll resubmit the patch with a build option
> :)  (and in that case I will also resubmit the networking patch so the
> same build option is used for both QEMU related changes.)
> 

Nice try! but I remain unconvinced :-)
From what you said, we will need a compiler flag for the Ethernet driver anyway. So let's go for it :-)
And it will quite easy to 'grep' for SUPPORT_QEMU to highlight the difference between the different models than to go through the git history to find out the workaround which has been made to use UEFI on qEmu.
Roy Franz Jan. 14, 2014, 6:10 p.m. UTC | #8
On Tue, Jan 14, 2014 at 10:09 AM, Olivier Martin <olivier.martin@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Roy Franz [mailto:roy.franz@linaro.org]
>> Sent: 14 January 2014 01:31
>> To: Olivier Martin
>> Cc: ryan.harkin@linaro.org; edk2-devel@lists.sourceforge.net; linaro-
>> uefi; Patch Tracking
>> Subject: Re: [PATCH V2] Move RTSM VExpress variable storage to 256k
>> flash blocks
>>
>> > I have to admit I prefer the solution 2) - but I am quite open to any
>> valid arguments. My argument is I would prefer to expose the correct
>> implementation when possible. And if qEmu adds support for the missing
>> VExpress IP block in the future then it will be easier to restore the
>> correct approach (ie: the default RTSM approach) for qEmu.  I don't
>> think that the flash will every be fixed in QEMU, since accurate
>> modeling of flash writing is not all that valuable of a feature.
>>
>> I can add a compile option for the flash, as I have done for the
>> networking.  The networking required this due to conflicting ethernet
>> devices.  (And here QEMU matches real hardware, and it is RTSM that is
>> 'wrong'.)  I think that the value of a common binary for a common case
>> is worth the cost of moving the variable storage to larger blocks, but
>> if you remain unconvinced I'll resubmit the patch with a build option
>> :)  (and in that case I will also resubmit the networking patch so the
>> same build option is used for both QEMU related changes.)
>>
>
> Nice try! but I remain unconvinced :-)
> From what you said, we will need a compiler flag for the Ethernet driver anyway. So let's go for it :-)
> And it will quite easy to 'grep' for SUPPORT_QEMU to highlight the difference between the different models than to go through the git history to find out the workaround which has been made to use UEFI on qEmu.
>
>
>
OK :)  I'll redo both changes into 1 series, both keying off the
SUPPORT_QEMU build define.

Thanks,
Roy
diff mbox

Patch

diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
index 2d12f4b..c0196d9 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15.dsc
@@ -77,12 +77,12 @@ 
   #
   # NV Storage PCDs. Use base of 0x0C000000 for NOR1
   #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD0000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C040000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
 
   gArmTokenSpaceGuid.PcdVFPEnabled|1
   
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
index efd80ab..69088ff 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-RTSM-A15_MPCore.dsc
@@ -79,12 +79,12 @@ 
   #
   # NV Storage PCDs. Use base of 0x0C000000 for NOR1
   #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0FFC0000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0FFD0000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0FFE0000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x0C000000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00040000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x0C040000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00040000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x0C080000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00040000
 
   gArmTokenSpaceGuid.PcdVFPEnabled|1