* [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region.
@ 2023-02-09 18:27 Chiu, Chasel
2023-02-09 18:34 ` Isaac Oram
0 siblings, 1 reply; 3+ messages in thread
From: Chiu, Chasel @ 2023-02-09 18:27 UTC (permalink / raw)
To: devel
Cc: Chasel Chiu, Ashraf Ali S, Isaac Oram, Rangasai V Chaganty,
Ray Ni, Michael Kubacki
Platform may implement an additional NVS region following
Regular variable region and in this case SpiFvbService should include
both region size when calculating the total NVS region size.
The PcdFlashNvStorageAdditionalSize is for compatible with legacy
usages that should be deprecated. The new usage model should define
separate regions without implicit connections to UEFI Variable or
FTW regions.
Example NVS flash map for such legacy usage:
Note: PcdFlashNvStorageAdditionalSize is equal to platform
PcdFlashFvNvStorageEventLogSize.
---------------
|UEFI Variable|
---------------
|EventLog | <= this is Additional NVS region
---------------
|FTW Working |
---------------
|FTW Spare |
---------------
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 22 ++++++++++++++++++++++
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 7 ++++---
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 11 +++++++++++
3 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
index 942abf95a6..fcdc715263 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
@@ -568,6 +568,28 @@ GetVariableFvInfo (
return;
}
+ //
+ // GetVariableFlashNvStorageInfo () only reports regular variable region information,
+ // if platform implemented an additional NVS region following the regular variable region,
+ // then both region size should be included as overall NVS region size.
+ //
+ // The below PcdFlashNvStorageAdditionalSize is for compatible with legacy usages that should be deprecated.
+ // The new usage model should define separate regions without implicit connections to UEFI Variable or FTW regions.
+ //
+ // Example NVS flash map for such legacy usage:
+ // Note: PcdFlashNvStorageAdditionalSize is equal to platform PcdFlashFvNvStorageEventLogSize.
+ // ---------------
+ // |UEFI Variable|
+ // ---------------
+ // |EventLog | <= this is Additional NVS region
+ // ---------------
+ // |FTW Working |
+ // ---------------
+ // |FTW Spare |
+ // ---------------
+ //
+ NvStoreLength += PcdGet32 (PcdFlashNvStorageAdditionalSize);
+
Status = GetVariableFlashFtwSpareInfo (&NvBaseAddress, &Length64);
if (!EFI_ERROR (Status)) {
// Stay within the current UINT32 size assumptions in the variable stack.
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
index 73049eceb2..f4009d8d8c 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
@@ -43,9 +43,10 @@
IntelSiliconPkg/IntelSiliconPkg.dec
[Pcd]
- gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
- gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
- gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ## SOMETIMES_CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ## SOMETIMES_CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize ## CONSUMES
[Sources]
FvbInfo.c
diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index 63dae756ad..d73a51ca52 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -194,3 +194,14 @@
# Other value: reserved for future use.<BR>
# @Prompt Flash Variable Store type.
gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x0000000E
+
+ ## Declares Additional NVS Region Size.<BR><BR>
+ # Platform may implement a Regular variable region and an additional region, which will require this PCD
+ # to tell SpiFvbService to include both regions.
+ # Note: This PCD is for compatible with legacy usages that should be deprecated.
+ # The new usage model should define separate regions without implicit connections to UEFI Variable or FTW regions.<BR>
+ # Example legacy usage is to set this PCD equal to platform PcdFlashFvNvStorageEventLogSize.
+ # 0: No additional NVS region.<BR>
+ # non-zero: The size of an additional NVS region following the Regular variable region.<BR>
+ # @Prompt Additional NVS Region Size.
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize|0x00000000|UINT32|0x0000000F
--
2.35.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region. 2023-02-09 18:27 [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region Chiu, Chasel @ 2023-02-09 18:34 ` Isaac Oram 2023-02-09 18:37 ` Chiu, Chasel 0 siblings, 1 reply; 3+ messages in thread From: Isaac Oram @ 2023-02-09 18:34 UTC (permalink / raw) To: Chiu, Chasel, devel@edk2.groups.io Cc: S, Ashraf Ali, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael Reviewed-by: Isaac Oram <isaac.w.oram@intel.com> -----Original Message----- From: Chiu, Chasel <chasel.chiu@intel.com> Sent: Thursday, February 9, 2023 10:27 AM To: devel@edk2.groups.io Cc: Chiu, Chasel <chasel.chiu@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Kubacki, Michael <michael.kubacki@microsoft.com> Subject: [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region. Platform may implement an additional NVS region following Regular variable region and in this case SpiFvbService should include both region size when calculating the total NVS region size. The PcdFlashNvStorageAdditionalSize is for compatible with legacy usages that should be deprecated. The new usage model should define separate regions without implicit connections to UEFI Variable or FTW regions. Example NVS flash map for such legacy usage: Note: PcdFlashNvStorageAdditionalSize is equal to platform PcdFlashFvNvStorageEventLogSize. --------------- |UEFI Variable| --------------- |EventLog | <= this is Additional NVS region --------------- |FTW Working | --------------- |FTW Spare | --------------- Cc: Ashraf Ali S <ashraf.ali.s@intel.com> Cc: Isaac Oram <isaac.w.oram@intel.com> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Michael Kubacki <michael.kubacki@microsoft.com> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com> Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com> --- Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 22 ++++++++++++++++++++++ Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 7 ++++--- Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 11 +++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c index 942abf95a6..fcdc715263 100644 --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe +++ rviceCommon.c @@ -568,6 +568,28 @@ GetVariableFvInfo ( return; } + //+ // GetVariableFlashNvStorageInfo () only reports regular variable region information,+ // if platform implemented an additional NVS region following the regular variable region,+ // then both region size should be included as overall NVS region size.+ //+ // The below PcdFlashNvStorageAdditionalSize is for compatible with legacy usages that should be deprecated.+ // The new usage model should define separate regions without implicit connections to UEFI Variable or FTW regions.+ //+ // Example NVS flash map for such legacy usage:+ // Note: PcdFlashNvStorageAdditionalSize is equal to platform PcdFlashFvNvStorageEventLogSize.+ // ---------------+ // |UEFI Variable|+ // ---------------+ // |EventLog | <= this is Additional NVS region+ // ---------------+ // |FTW Working |+ // ---------------+ // |FTW Spare |+ // ---------------+ //+ NvStoreLength += PcdGet32 (PcdFlashNvStorageAdditionalSize);+ Status = GetVariableFlashFtwSpareInfo (&NvBaseAddress, &Length64); if (!EFI_ERROR (Status)) { // Stay within the current UINT32 size assumptions in the variable stack.diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf index 73049eceb2..f4009d8d8c 100644 --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe +++ rviceSmm.inf @@ -43,9 +43,10 @@ IntelSiliconPkg/IntelSiliconPkg.dec [Pcd]- gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES- gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES- gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ## SOMETIMES_CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ## SOMETIMES_CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize ## CONSUMES [Sources] FvbInfo.cdiff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec index 63dae756ad..d73a51ca52 100644 --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec @@ -194,3 +194,14 @@ # Other value: reserved for future use.<BR> # @Prompt Flash Variable Store type. gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x0000000E++ ## Declares Additional NVS Region Size.<BR><BR>+ # Platform may implement a Regular variable region and an additional region, which will require this PCD+ # to tell SpiFvbService to include both regions.+ # Note: This PCD is for compatible with legacy usages that should be deprecated.+ # The new usage model should define separate regions without implicit connections to UEFI Variable or FTW regions.<BR>+ # Example legacy usage is to set this PCD equal to platform PcdFlashFvNvStorageEventLogSize.+ # 0: No additional NVS region.<BR>+ # non-zero: The size of an additional NVS region following the Regular variable region.<BR>+ # @Prompt Additional NVS Region Size.+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize|0x00000000|UINT32|0x0000000F-- 2.35.0.windows.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region. 2023-02-09 18:34 ` Isaac Oram @ 2023-02-09 18:37 ` Chiu, Chasel 0 siblings, 0 replies; 3+ messages in thread From: Chiu, Chasel @ 2023-02-09 18:37 UTC (permalink / raw) To: Oram, Isaac W, devel@edk2.groups.io Cc: S, Ashraf Ali, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael Thanks for promptly reviewing and good suggestions Michael, Isaac! I have merged this patch: https://github.com/tianocore/edk2-platforms/commit/88d44c563d9fd5c95be93e706f9420352ee4c053 Thanks, Chasel > -----Original Message----- > From: Oram, Isaac W <isaac.w.oram@intel.com> > Sent: Thursday, February 9, 2023 10:34 AM > To: Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io > Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chaganty, Rangasai V > <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Kubacki, > Michael <michael.kubacki@microsoft.com> > Subject: RE: [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: > Support Additional NVS region. > > Reviewed-by: Isaac Oram <isaac.w.oram@intel.com> > > -----Original Message----- > From: Chiu, Chasel <chasel.chiu@intel.com> > Sent: Thursday, February 9, 2023 10:27 AM > To: devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.chiu@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; > Oram, Isaac W <isaac.w.oram@intel.com>; Chaganty, Rangasai V > <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Kubacki, > Michael <michael.kubacki@microsoft.com> > Subject: [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: Support > Additional NVS region. > > Platform may implement an additional NVS region following Regular variable > region and in this case SpiFvbService should include both region size when > calculating the total NVS region size. > > The PcdFlashNvStorageAdditionalSize is for compatible with legacy usages that > should be deprecated. The new usage model should define separate regions > without implicit connections to UEFI Variable or FTW regions. > > Example NVS flash map for such legacy usage: > Note: PcdFlashNvStorageAdditionalSize is equal to platform > PcdFlashFvNvStorageEventLogSize. > > --------------- > |UEFI Variable| > --------------- > |EventLog | <= this is Additional NVS region > --------------- > |FTW Working | > --------------- > |FTW Spare | > --------------- > > Cc: Ashraf Ali S <ashraf.ali.s@intel.com> > Cc: Isaac Oram <isaac.w.oram@intel.com> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Michael Kubacki <michael.kubacki@microsoft.com> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com> > Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com> > --- > > Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon > .c | 22 ++++++++++++++++++++++ > Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf > | 7 ++++--- > Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 11 > +++++++++++ > 3 files changed, 37 insertions(+), 3 deletions(-) > > diff --git > a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceComm > on.c > b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceComm > on.c > index 942abf95a6..fcdc715263 100644 > --- > a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceComm > on.c > +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe > +++ rviceCommon.c > @@ -568,6 +568,28 @@ GetVariableFvInfo ( > return; } + //+ // GetVariableFlashNvStorageInfo () only reports regular > variable region information,+ // if platform implemented an additional NVS > region following the regular variable region,+ // then both region size should be > included as overall NVS region size.+ //+ // The below > PcdFlashNvStorageAdditionalSize is for compatible with legacy usages that > should be deprecated.+ // The new usage model should define separate regions > without implicit connections to UEFI Variable or FTW regions.+ //+ // Example > NVS flash map for such legacy usage:+ // Note: > PcdFlashNvStorageAdditionalSize is equal to platform > PcdFlashFvNvStorageEventLogSize.+ // ---------------+ // |UEFI Variable|+ // - > --------------+ // |EventLog | <= this is Additional NVS region+ // --------------- > + // |FTW Working |+ // ---------------+ // |FTW Spare |+ // ---------------+ > //+ NvStoreLength += PcdGet32 (PcdFlashNvStorageAdditionalSize);+ Status = > GetVariableFlashFtwSpareInfo (&NvBaseAddress, &Length64); if (!EFI_ERROR > (Status)) { // Stay within the current UINT32 size assumptions in the variable > stack.diff --git > a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in > f > b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in > f > index 73049eceb2..f4009d8d8c 100644 > --- > a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in > f > +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe > +++ rviceSmm.inf > @@ -43,9 +43,10 @@ > IntelSiliconPkg/IntelSiliconPkg.dec [Pcd]- > gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES- > gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES- > gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ## > SOMETIMES_CONSUMES+ > gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## > CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize > ## CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType > ## SOMETIMES_CONSUMES+ > gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize ## > CONSUMES [Sources] FvbInfo.cdiff --git > a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec > b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec > index 63dae756ad..d73a51ca52 100644 > --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec > +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec > @@ -194,3 +194,14 @@ > # Other value: reserved for future use.<BR> # @Prompt Flash Variable Store > type. > gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x000 > 0000E++ ## Declares Additional NVS Region Size.<BR><BR>+ # Platform may > implement a Regular variable region and an additional region, which will require > this PCD+ # to tell SpiFvbService to include both regions.+ # Note: This PCD is > for compatible with legacy usages that should be deprecated.+ # The new > usage model should define separate regions without implicit connections to > UEFI Variable or FTW regions.<BR>+ # Example legacy usage is to set this PCD > equal to platform PcdFlashFvNvStorageEventLogSize.+ # 0: No additional NVS > region.<BR>+ # non-zero: The size of an additional NVS region following the > Regular variable region.<BR>+ # @Prompt Additional NVS Region Size.+ > gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize|0x00000000| > UINT32|0x0000000F-- > 2.35.0.windows.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-09 18:38 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-09 18:27 [edk2-platforms: PATCH v4] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region Chiu, Chasel 2023-02-09 18:34 ` Isaac Oram 2023-02-09 18:37 ` Chiu, Chasel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox