From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-il1-f182.google.com (mail-il1-f182.google.com [209.85.166.182]) by mx.groups.io with SMTP id smtpd.web11.8468.1638512257464864987 for ; Thu, 02 Dec 2021 22:17:37 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=o2ua85k7; spf=pass (domain: linaro.org, ip: 209.85.166.182, mailfrom: masami.hiramatsu@linaro.org) Received: by mail-il1-f182.google.com with SMTP id j21so1747664ila.5 for ; Thu, 02 Dec 2021 22:17:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=JGek6fGZAzBzreTBoQSZ+R2ZSX2LQr8CKSmdNm+fMX8=; b=o2ua85k7vnmHbqKwLPVG+Kl1wcSUANXWVe2C6Hf2MoBGlPY0jteZT96IKEA8t3hp1Y wLuN0nSYLsY5BXU4e0+Tn74b4cmNMsugVvojif7GGFc5AQr0subgo6k0ZMiHburgtwln Iy/1kkeF1rKjJRR8owER+pTffwZtrJvoQPNYn0Fuf4lBxLcatoJPGe1U5DxAQSwlq8RV CS9tkyOd/pmbbSO/ffUKkLdssJRwSk1GI0YlA+lC59s5Ul5LsSegVjxTPhup3Yd7UE28 k8r7MCCNb8alVWsvf3dNRlEZ+MatWq3I2VI7HE1vJWue8hLMrBmGdGEuZc/pTZtMqiHl K0VQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=JGek6fGZAzBzreTBoQSZ+R2ZSX2LQr8CKSmdNm+fMX8=; b=Oll30pHKWbmYdRGX3u7WfnOyujlhRWmUeR7VgReY13RAUmAAY4t1A6uAAo2qNOXdp4 7Wq7EyqzJpZePK/OLqnA+yuT9iCFW59Cio3Hry5LMSCyK2sesjda6tvizNZ8UW0TA0mD 8AE6L8hg5pW2WIOmlcFquKLRIvX3CRbjet9801rqYpp5HbbeVcPYRhGi8MnE+zkY2BMX VvjYv7b7lKwpH3CW30sxSjMMmXe85qhtZDW/lqPqypOf7lU6zcOjdu6b/HDKbD6+TW4/ OUGaHWuFIJV+Fz49aObw+5e3teCGk7Gk5cWdxmT0WjgVMVgHwahG4VBYXFnVobJPBMrd /FAw== X-Gm-Message-State: AOAM532Y3zeV+kKgSN6Wc+QpJ1ytYSWjRaOjWXjvxLQyu/42zASc9217 xLR6/ygWE59QCEd/75LeweZbElQvG6UsOG9whXrzodYNt40= X-Google-Smtp-Source: ABdhPJy4ZzoJi+2vvD12b/c9I0N77B3JI5nXK4lwaWojUxxb/SjmHR5XXMMRz0KSrPP+a6s/QYofiAuY2R6HWcJ/n0c= X-Received: by 2002:a92:da4c:: with SMTP id p12mr16539973ilq.32.1638512256283; Thu, 02 Dec 2021 22:17:36 -0800 (PST) MIME-Version: 1.0 References: <163610419943.391624.9289897029386201296.stgit@localhost> <163610424153.391624.15870608392900932158.stgit@localhost> <16BC25260C31EA7F.23256@groups.io> In-Reply-To: <16BC25260C31EA7F.23256@groups.io> From: "Masami Hiramatsu" Date: Fri, 3 Dec 2021 15:17:25 +0900 Message-ID: Subject: Re: [edk2-devel] [PATCH 5/5] [edk2-platforms] Platform/DeveloperBox: Expand NvStorage sizes To: devel@edk2.groups.io, masami.hiramatsu@linaro.org Cc: Leif Lindholm , Ard Biesheuvel , Kazuhiko Sakamoto , Masahisa Kojima Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Leif, I found that "gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision" is a digit number. On the DeveloperBox, it is the same as the BUILD_NUMBER which was set when building the EDK2 as below. Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc: gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER) And now it becomes 99 in official snapshot (this will be the old series) http://snapshots.linaro.org/components/kernel/leg-96boards-developerbox-ed= k2/99/ However, this snapshot is actually not updated a while ago since the source repository is not more maintained. Anyway, my point is that the FirmwareRevision is not set as a fixed number, but it will be given at the build time. In that case, it is hard to set the PcdLowestSupportedFirmwareVersion because the PcdFirmwareRevision can be freely editable by who is releasing the firmware (and now no one officially releases it. just a bot repeating compilation without any update) My idea is to keep the PcdFirmwareRevision as is, but only set the PcdLowestSupportedFirmwareVersion as for example 1024 with the variable area is extended. The person who maintains the firmware release build needs to care about setting BUILD_NUMBER more than 1024. This will prevent users to rollback to older image accidentary, but not prevent the careless developer to build their own binary with lower BUILD_NUMBER etc. What do you think? Thank you, 2021=E5=B9=B411=E6=9C=8830=E6=97=A5(=E7=81=AB) 7:33 Masami Hiramatsu via gr= oups.io : > > Hi Leif, > > 2021=E5=B9=B411=E6=9C=8829=E6=97=A5(=E6=9C=88) 22:43 Leif Lindholm : > > > > 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 backwar= d > 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 > > > > > Reported-by: Kazuhiko Sakamoto > > > > > --- > > > > > .../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|0= x08400000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0= x00010000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase= |0x08410000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize= |0x00010000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0= x08420000 > > > > > - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0= x00010000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0= x00080000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase= |0x08480000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize= |0x00080000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0= x08500000 > > > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0= x00080000 > > > > > > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI " > > > > > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x5243= 4155514e5953 # SYNQUACR > > > > > > > > > > > > > > > > > -- > > > Masami Hiramatsu > > > > -- > Masami Hiramatsu > > >=20 > > -- Masami Hiramatsu