1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH 0/5] Series short description
  4. View patch

[edk2-devel] [PATCH 5/5] [edk2-platforms] Platform/DeveloperBox: Expand NvStorage sizes

Masami Hiramatsu posted 5 patches 3 years ago
There is a newer version of this series
[edk2-devel] [PATCH 5/5] [edk2-platforms] Platform/DeveloperBox: Expand NvStorage sizes
Posted by Masami Hiramatsu 3 years ago 
Expand NvStorage Variable size and FTW spare/working size
for the DeveloperBox platform.

Since the size of the NvStorage VariableSize is not enough
large, FWTS uefirttime test, which updates the NV
variables in runtime, failes. This expands the size to fix
this issue.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Reported-by: Kazuhiko Sakamoto <sakamoto.kazuhiko@socionext.com>
---
 .../Socionext/DeveloperBox/DeveloperBox.dsc.inc    |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
index 0a364bc457..3baf97ecc0 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
@@ -280,11 +280,11 @@
   gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress|0x08000000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x08400000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08410000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00080000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08480000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00080000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08500000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00080000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI   "
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x52434155514e5953 # SYNQUACR



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83380): https://edk2.groups.io/g/devel/message/83380
Mute This Topic: https://groups.io/mt/86836385/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 5/5] [edk2-platforms] Platform/DeveloperBox: Expand NvStorage sizes
Posted by Leif Lindholm 2 years, 12 months ago 
On Fri, Nov 05, 2021 at 18:24:01 +0900, Masami Hiramatsu wrote:
> Expand NvStorage Variable size and FTW spare/working size
> for the DeveloperBox platform.
> 
> Since the size of the NvStorage VariableSize is not enough
> large, FWTS uefirttime test, which updates the NV
> variables in runtime, failes. This expands the size to fix
> this issue.

Does this change erase all existing variables?

If so, I think it is worth introducing this as a non-default build
option, in order to not wreck existing installations on a firmware
update.

I think it would also be worth considering whether to update
PcdLowestSupportedFirmwareVersion. PcdFirmwareRevision should
definitely be updated.

/
    Leif

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> Reported-by: Kazuhiko Sakamoto <sakamoto.kazuhiko@socionext.com>
> ---
>  .../Socionext/DeveloperBox/DeveloperBox.dsc.inc    |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> index 0a364bc457..3baf97ecc0 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> @@ -280,11 +280,11 @@
>    gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress|0x08000000
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x08400000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08410000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00080000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08480000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00080000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08500000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00080000
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI   "
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x52434155514e5953 # SYNQUACR
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84104): https://edk2.groups.io/g/devel/message/84104
Mute This Topic: https://groups.io/mt/86836385/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 5/5] [edk2-platforms] Platform/DeveloperBox: Expand NvStorage sizes
Posted by Masami Hiramatsu 2 years, 12 months ago 
Hi Leif,

2021年11月27日(土) 3:19 Leif Lindholm <leif@nuviainc.com>:
>
> On Fri, Nov 05, 2021 at 18:24:01 +0900, Masami Hiramatsu wrote:
> > Expand NvStorage Variable size and FTW spare/working size
> > for the DeveloperBox platform.
> >
> > Since the size of the NvStorage VariableSize is not enough
> > large, FWTS uefirttime test, which updates the NV
> > variables in runtime, failes. This expands the size to fix
> > this issue.
>
> Does this change erase all existing variables?

Ah, indeed. It may need to erase all variables.

>
> If so, I think it is worth introducing this as a non-default build
> option, in order to not wreck existing installations on a firmware
> update.
>
> I think it would also be worth considering whether to update
> PcdLowestSupportedFirmwareVersion. PcdFirmwareRevision
> should definitely be updated.

I'm not sure about this point.
You meant we should have 2 different revisions like a branch?
- Branch A(current version): keep the variable area size the same.
- Branch B(new version): expand the variable area.
And a build option will change the branch by updating the
PcdFirmwareRevision?

Also PcdLowestSupportedFirmwareVersion you meant is
in the capsule file?

Thank you,


>
> /
>     Leif
>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > Reported-by: Kazuhiko Sakamoto <sakamoto.kazuhiko@socionext.com>
> > ---
> >  .../Socionext/DeveloperBox/DeveloperBox.dsc.inc    |   10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > index 0a364bc457..3baf97ecc0 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > @@ -280,11 +280,11 @@
> >    gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress|0x08000000
> >
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x08400000
> > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08410000
> > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
> > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000
> > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00080000
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08480000
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00080000
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08500000
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00080000
> >
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI   "
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x52434155514e5953 # SYNQUACR
> >



-- 
Masami Hiramatsu


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84114): https://edk2.groups.io/g/devel/message/84114
Mute This Topic: https://groups.io/mt/86836385/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 5/5] [edk2-platforms] Platform/DeveloperBox: Expand NvStorage sizes
Posted by Leif Lindholm 2 years, 12 months ago 
On Sat, Nov 27, 2021 at 16:48:45 +0900, Masami Hiramatsu wrote:
> > On Fri, Nov 05, 2021 at 18:24:01 +0900, Masami Hiramatsu wrote:
> > > Expand NvStorage Variable size and FTW spare/working size
> > > for the DeveloperBox platform.
> > >
> > > Since the size of the NvStorage VariableSize is not enough
> > > large, FWTS uefirttime test, which updates the NV
> > > variables in runtime, failes. This expands the size to fix
> > > this issue.
> >
> > Does this change erase all existing variables?
> 
> Ah, indeed. It may need to erase all variables.

That is quite likely to lead to upset users.

> > If so, I think it is worth introducing this as a non-default build
> > option, in order to not wreck existing installations on a firmware
> > update.
> >
> > I think it would also be worth considering whether to update
> > PcdLowestSupportedFirmwareVersion. PcdFirmwareRevision
> > should definitely be updated.
> 
> I'm not sure about this point.
> You meant we should have 2 different revisions like a branch?
> - Branch A(current version): keep the variable area size the same.
> - Branch B(new version): expand the variable area.
> And a build option will change the branch by updating the
> PcdFirmwareRevision?

Not a branch - just that you need to explicitly build for the size of
flash area you want to use, and if you provide pre-built downloadable
ones - provide two variants.

This becomes a bit of a maintenance nightmare over time.

A better solution would be for the firmware to (somehow) resize the
parameter area - retaining existing values - if it encounters the
smaller version. I don't think we have an example of that.

PcdLowestSupportedFirmwareVersion still needs to be set, to the same
value as the new PcdFirmwareRevision, to prevent downgrading to a
version that does not support the larger size.

> Also PcdLowestSupportedFirmwareVersion you meant is
> in the capsule file?

I meant to change the Pcd value. That implements the change in
SystemFirmwareDescriptorTable.aslc.

Regards,

Leif

> Thank you,
> 
> 
> >
> > /
> >     Leif
> >
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > Reported-by: Kazuhiko Sakamoto <sakamoto.kazuhiko@socionext.com>
> > > ---
> > >  .../Socionext/DeveloperBox/DeveloperBox.dsc.inc    |   10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > > index 0a364bc457..3baf97ecc0 100644
> > > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > > @@ -280,11 +280,11 @@
> > >    gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress|0x08000000
> > >
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x08400000
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08410000
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00080000
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08480000
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00080000
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08500000
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00080000
> > >
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI   "
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x52434155514e5953 # SYNQUACR
> > >
> 
> 
> 
> -- 
> Masami Hiramatsu


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84127): https://edk2.groups.io/g/devel/message/84127
Mute This Topic: https://groups.io/mt/86836385/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 5/5] [edk2-platforms] Platform/DeveloperBox: Expand NvStorage sizes
Posted by Masami Hiramatsu 2 years, 12 months ago 
Hi Leif,

2021年11月29日(月) 22:43 Leif Lindholm <leif@nuviainc.com>:
>
> On Sat, Nov 27, 2021 at 16:48:45 +0900, Masami Hiramatsu wrote:
> > > On Fri, Nov 05, 2021 at 18:24:01 +0900, Masami Hiramatsu wrote:
> > > > Expand NvStorage Variable size and FTW spare/working size
> > > > for the DeveloperBox platform.
> > > >
> > > > Since the size of the NvStorage VariableSize is not enough
> > > > large, FWTS uefirttime test, which updates the NV
> > > > variables in runtime, failes. This expands the size to fix
> > > > this issue.
> > >
> > > Does this change erase all existing variables?
> >
> > Ah, indeed. It may need to erase all variables.
>
> That is quite likely to lead to upset users.

OK.

> > > If so, I think it is worth introducing this as a non-default build
> > > option, in order to not wreck existing installations on a firmware
> > > update.
> > >
> > > I think it would also be worth considering whether to update
> > > PcdLowestSupportedFirmwareVersion. PcdFirmwareRevision
> > > should definitely be updated.
> >
> > I'm not sure about this point.
> > You meant we should have 2 different revisions like a branch?
> > - Branch A(current version): keep the variable area size the same.
> > - Branch B(new version): expand the variable area.
> > And a build option will change the branch by updating the
> > PcdFirmwareRevision?
>
> Not a branch - just that you need to explicitly build for the size of
> flash area you want to use, and if you provide pre-built downloadable
> ones - provide two variants.

I got it.

> This becomes a bit of a maintenance nightmare over time.

Actually, I'm considering a kind of "leap" firmware release, which
involves all firmware update by manual (not automatic), because the
SCP-firmware is too old anymore and the new SCP firmware (OSS version)
requires to update TF-A, which is not compatible with old ones.
Obviously, this must be done by manual.

So, afterwards, we will not release old version anymore. Anyway, the
old firmware snapshot image is not updated in one year (since source
repository has not been updated). The firmware on LVFS is released in
2019.
(BTW, can I change the UUID which fwupd detects too?)

I will provide a build option for the users who update EDK2 by themselves.

> A better solution would be for the firmware to (somehow) resize the
> parameter area - retaining existing values - if it encounters the
> smaller version. I don't think we have an example of that.

Hmm, I rather like to erase it while the "leap" update, since the backward
compatibility is not guaranteed. And after the update, user will be able
to choose the U-Boot on the DeveloperBox.

> PcdLowestSupportedFirmwareVersion still needs to be set, to the same
> value as the new PcdFirmwareRevision, to prevent downgrading to a
> version that does not support the larger size.

OK, so this is for protecting rollback. But this means, do I need to make
it optional (switched by build option) too?

>
> > Also PcdLowestSupportedFirmwareVersion you meant is
> > in the capsule file?
>
> I meant to change the Pcd value. That implements the change in
> SystemFirmwareDescriptorTable.aslc.

OK.

Thank you,

>
> Regards,
>
> Leif
>
> > Thank you,
> >
> >
> > >
> > > /
> > >     Leif
> > >
> > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > > > Reported-by: Kazuhiko Sakamoto <sakamoto.kazuhiko@socionext.com>
> > > > ---
> > > >  .../Socionext/DeveloperBox/DeveloperBox.dsc.inc    |   10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > > > index 0a364bc457..3baf97ecc0 100644
> > > > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > > > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > > > @@ -280,11 +280,11 @@
> > > >    gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress|0x08000000
> > > >
> > > >    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x08400000
> > > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
> > > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08410000
> > > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
> > > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000
> > > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00080000
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08480000
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00080000
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08500000
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00080000
> > > >
> > > >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI   "
> > > >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x52434155514e5953 # SYNQUACR
> > > >
> >
> >
> >
> > --
> > Masami Hiramatsu



-- 
Masami Hiramatsu


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84137): https://edk2.groups.io/g/devel/message/84137
Mute This Topic: https://groups.io/mt/86836385/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.