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

["Masami Hiramatsu" <masami.hiramatsu@linaro.org>]

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