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

["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.

> > 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