public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms: PATCH v3] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region.
@ 2023-02-09 17:20 Chiu, Chasel
  2023-02-09 17:24 ` [edk2-devel] " Michael Kubacki
  0 siblings, 1 reply; 2+ messages in thread
From: Chiu, Chasel @ 2023-02-09 17:20 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.

One usage model is EventLog NVS region and there could be others.
Example NVS flash map for such usage model:
  --------------
  |UEI 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>
---
 Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 17 +++++++++++++++++
 Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf  |  7 ++++---
 Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec                               |  8 ++++++++
 3 files changed, 29 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..cf5a40bf27 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
@@ -568,6 +568,23 @@ GetVariableFvInfo (
     return;
   }
 
+  //
+  // GetVariableFlashNvStorageInfo () only reports regular variable region information,
+  // if platform implemented an additional NVS region following the regular variable region,
+  // the both region size should be included as overall NVS region size.
+  // Example NVS flash map for such usage model:
+  //  --------------
+  //  |UEI 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..b10529b69d 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -194,3 +194,11 @@
   #  Other value: reserved for future use.<BR>
   # @Prompt Flash Variable Store type.
   gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x0000000E
+
+  ## Declares Additional NVS Variable Region Size.<BR><BR>
+  #  Platform may implement a Regular variable region and an additional variable region, which will require this PCD
+  #  to tell SpiFvbService to include both regions.<BR>
+  #  0: No additional variable region.<BR>
+  #  non-zero: The size of an additional variable region following the Regular variable region.<BR>
+  # @Prompt Additional NVS Variable Region Size.
+  gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize|0x00000000|UINT32|0x0000000F
-- 
2.35.0.windows.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [edk2-devel] [edk2-platforms: PATCH v3] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region.
  2023-02-09 17:20 [edk2-platforms: PATCH v3] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region Chiu, Chasel
@ 2023-02-09 17:24 ` Michael Kubacki
  0 siblings, 0 replies; 2+ messages in thread
From: Michael Kubacki @ 2023-02-09 17:24 UTC (permalink / raw)
  To: devel, chasel.chiu
  Cc: Ashraf Ali S, Isaac Oram, Rangasai V Chaganty, Ray Ni,
	Michael Kubacki

Please fix the typo in the commit message and code comment before pushing.

"|UEI Variable|" -> "|UEFI Variable|"

Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 2/9/2023 12:20 PM, Chiu, Chasel wrote:
> 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.
> 
> One usage model is EventLog NVS region and there could be others.
> Example NVS flash map for such usage model:
>    --------------
>    |UEI 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>
> ---
>   Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 17 +++++++++++++++++
>   Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf  |  7 ++++---
>   Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec                               |  8 ++++++++
>   3 files changed, 29 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..cf5a40bf27 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
> @@ -568,6 +568,23 @@ GetVariableFvInfo (
>       return;
> 
>     }
> 
>   
> 
> +  //
> 
> +  // GetVariableFlashNvStorageInfo () only reports regular variable region information,
> 
> +  // if platform implemented an additional NVS region following the regular variable region,
> 
> +  // the both region size should be included as overall NVS region size.
> 
> +  // Example NVS flash map for such usage model:
> 
> +  //  --------------
> 
> +  //  |UEI 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..b10529b69d 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> @@ -194,3 +194,11 @@
>     #  Other value: reserved for future use.<BR>
> 
>     # @Prompt Flash Variable Store type.
> 
>     gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x0000000E
> 
> +
> 
> +  ## Declares Additional NVS Variable Region Size.<BR><BR>
> 
> +  #  Platform may implement a Regular variable region and an additional variable region, which will require this PCD
> 
> +  #  to tell SpiFvbService to include both regions.<BR>
> 
> +  #  0: No additional variable region.<BR>
> 
> +  #  non-zero: The size of an additional variable region following the Regular variable region.<BR>
> 
> +  # @Prompt Additional NVS Variable Region Size.
> 
> +  gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageAdditionalSize|0x00000000|UINT32|0x0000000F
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-02-09 17:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-09 17:20 [edk2-platforms: PATCH v3] IntelSiliconPkg/SpiFvbServiceSmm: Support Additional NVS region Chiu, Chasel
2023-02-09 17:24 ` [edk2-devel] " Michael Kubacki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox