* [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
@ 2023-02-09 5:14 Chiu, Chasel
2023-02-09 15:39 ` [edk2-devel] " Michael Kubacki
0 siblings, 1 reply; 6+ messages in thread
From: Chiu, Chasel @ 2023-02-09 5:14 UTC (permalink / raw)
To: devel
Cc: Chasel Chiu, Ashraf Ali S, Isaac Oram, Rangasai V Chaganty,
Ray Ni, Michael Kubacki
Platform may implement Other NVS variable 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.
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 | 7 +++++++
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 7 ++++---
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8 ++++++++
3 files changed, 19 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..bcde98131d 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
@@ -568,6 +568,13 @@ GetVariableFvInfo (
return;
}
+ //
+ // GetVariableFlashNvStorageInfo () only reports regular variable region information,
+ // if platform implemented a separate Other variable region following the regular variable region,
+ // the size should be included as overall NVS variable region size.
+ //
+ NvStoreLength += PcdGet32 (PcdFlashNvStorageOtherVariableSize);
+
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..f40067418a 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.PcdFlashNvStorageOtherVariableSize ## CONSUMES
[Sources]
FvbInfo.c
diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index 63dae756ad..7034ab93b0 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 Separate NVS Variable Region Size.<BR><BR>
+ # Platform may implement a Regular variable region and an Other variable region, which will require this PCD
+ # to tell SpiFvbService to include both regions.<BR>
+ # 0: No separate Other variable region.<BR>
+ # non-zero: The size of a separate Other variable region following the Regular variable region.<BR>
+ # @Prompt Separate NVS Variable Region Size.
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageOtherVariableSize|0x00000000|UINT32|0x0000000F
--
2.35.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
2023-02-09 5:14 [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region Chiu, Chasel
@ 2023-02-09 15:39 ` Michael Kubacki
2023-02-09 16:04 ` Isaac Oram
0 siblings, 1 reply; 6+ messages in thread
From: Michael Kubacki @ 2023-02-09 15:39 UTC (permalink / raw)
To: devel, chasel.chiu
Cc: Ashraf Ali S, Isaac Oram, Rangasai V Chaganty, Ray Ni,
Michael Kubacki
Is there a reason this other content can't go into it's own FV?
On 2/9/2023 12:14 AM, Chiu, Chasel wrote:
> Platform may implement Other NVS variable 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.
>
> 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 | 7 +++++++
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 7 ++++---
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8 ++++++++
> 3 files changed, 19 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..bcde98131d 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
> @@ -568,6 +568,13 @@ GetVariableFvInfo (
> return;
>
> }
>
>
>
> + //
>
> + // GetVariableFlashNvStorageInfo () only reports regular variable region information,
>
> + // if platform implemented a separate Other variable region following the regular variable region,
>
> + // the size should be included as overall NVS variable region size.
>
> + //
>
> + NvStoreLength += PcdGet32 (PcdFlashNvStorageOtherVariableSize);
>
> +
>
> 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..f40067418a 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.PcdFlashNvStorageOtherVariableSize ## CONSUMES
>
>
>
> [Sources]
>
> FvbInfo.c
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> index 63dae756ad..7034ab93b0 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 Separate NVS Variable Region Size.<BR><BR>
>
> + # Platform may implement a Regular variable region and an Other variable region, which will require this PCD
>
> + # to tell SpiFvbService to include both regions.<BR>
>
> + # 0: No separate Other variable region.<BR>
>
> + # non-zero: The size of a separate Other variable region following the Regular variable region.<BR>
>
> + # @Prompt Separate NVS Variable Region Size.
>
> + gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageOtherVariableSize|0x00000000|UINT32|0x0000000F
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
2023-02-09 15:39 ` [edk2-devel] " Michael Kubacki
@ 2023-02-09 16:04 ` Isaac Oram
2023-02-09 16:46 ` Michael Kubacki
0 siblings, 1 reply; 6+ messages in thread
From: Isaac Oram @ 2023-02-09 16:04 UTC (permalink / raw)
To: Michael Kubacki, devel@edk2.groups.io, Chiu, Chasel
Cc: S, Ashraf Ali, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael
It is a legacy that exists in current and past implementations. There is a complex arbitrary relationship between the runtime updateable regions in existing platform designs.
There is something like:
- Variable store (large)
- Error log (small)
- Fault tolerant working area (>= size of prior 2 regions)
- Fault tolerant metadata (small).
And there are assumptions about ordering and packing built into board flash layouts.
I don't think that we should introduce "other variable" as a concept, because variable solutions don't support two regions, so it isn't a UEFI variable region. OtherUpdatable might be ok, but still seems confusing to me.
I think that we should add the support for the *ErrorLog* region so that the open FvbServices can be used by current implementations. Then we should eliminate the "ErrorLog" use completely. My thought is that this makes the connection to legacy clear. And also motivates us to eliminate all the vestigial references to the ErrorLog in edk2 and edk2-platforms.
New updateable regions should not be hard-coded into this area and should have a cleaner solution, as Michael suggests.
I understand if we don't want to support legacy or workarounds, but I think that currently adoption and use of the open content is higher priority. Which is why we are requesting this workaround to match "proprietary" FVB services behavior.
Regards,
Isaac
-----Original Message-----
From: Michael Kubacki <mikuback@linux.microsoft.com>
Sent: Thursday, February 9, 2023 7:40 AM
To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@intel.com>
Cc: 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: Re: [edk2-devel] [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
Is there a reason this other content can't go into it's own FV?
On 2/9/2023 12:14 AM, Chiu, Chasel wrote:
> Platform may implement Other NVS variable 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.
>
> 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 | 7 +++++++
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 7 ++++---
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8 ++++++++
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> index 942abf95a6..bcde98131d 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceCommon.c
> @@ -568,6 +568,13 @@ GetVariableFvInfo (
> return;
>
> }
>
>
>
> + //
>
> + // GetVariableFlashNvStorageInfo () only reports regular variable
> + region information,
>
> + // if platform implemented a separate Other variable region
> + following the regular variable region,
>
> + // the size should be included as overall NVS variable region size.
>
> + //
>
> + NvStoreLength += PcdGet32 (PcdFlashNvStorageOtherVariableSize);
>
> +
>
> 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/SpiFvbServ
> iceSmm.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> index 73049eceb2..f40067418a 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceSmm.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.PcdFlashNvStorageOtherVariableSize
> + ## CONSUMES
>
>
>
> [Sources]
>
> FvbInfo.c
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> index 63dae756ad..7034ab93b0 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|0x
> 0000000E
>
> +
>
> + ## Declares Separate NVS Variable Region Size.<BR><BR>
>
> + # Platform may implement a Regular variable region and an Other
> + variable region, which will require this PCD
>
> + # to tell SpiFvbService to include both regions.<BR>
>
> + # 0: No separate Other variable region.<BR>
>
> + # non-zero: The size of a separate Other variable region following
> + the Regular variable region.<BR>
>
> + # @Prompt Separate NVS Variable Region Size.
>
> +
> + gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageOtherVariableSize|0x
> + 00000000|UINT32|0x0000000F
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
2023-02-09 16:04 ` Isaac Oram
@ 2023-02-09 16:46 ` Michael Kubacki
2023-02-09 17:25 ` Chiu, Chasel
[not found] ` <174237FCE67B704B.15261@groups.io>
0 siblings, 2 replies; 6+ messages in thread
From: Michael Kubacki @ 2023-02-09 16:46 UTC (permalink / raw)
To: devel, isaac.w.oram, Chiu, Chasel
Cc: S, Ashraf Ali, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael
Thanks, that's useful background. @chasel, you should probably put this
info in the commit message so it is captured in source history.
Given the default value is zero, it seems reasonable. I was also
initially confused by the name of the PCD.
Another idea would be something like "PcdFlashNvStorageAdditionalSize".
Please do at least update the commit message to include additional context.
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
On 2/9/2023 11:04 AM, Isaac Oram wrote:
> It is a legacy that exists in current and past implementations. There is a complex arbitrary relationship between the runtime updateable regions in existing platform designs.
> There is something like:
> - Variable store (large)
> - Error log (small)
> - Fault tolerant working area (>= size of prior 2 regions)
> - Fault tolerant metadata (small).
> And there are assumptions about ordering and packing built into board flash layouts.
>
> I don't think that we should introduce "other variable" as a concept, because variable solutions don't support two regions, so it isn't a UEFI variable region. OtherUpdatable might be ok, but still seems confusing to me.
> I think that we should add the support for the *ErrorLog* region so that the open FvbServices can be used by current implementations. Then we should eliminate the "ErrorLog" use completely. My thought is that this makes the connection to legacy clear. And also motivates us to eliminate all the vestigial references to the ErrorLog in edk2 and edk2-platforms.
>
> New updateable regions should not be hard-coded into this area and should have a cleaner solution, as Michael suggests.
>
> I understand if we don't want to support legacy or workarounds, but I think that currently adoption and use of the open content is higher priority. Which is why we are requesting this workaround to match "proprietary" FVB services behavior.
>
> Regards,
> Isaac
>
> -----Original Message-----
> From: Michael Kubacki <mikuback@linux.microsoft.com>
> Sent: Thursday, February 9, 2023 7:40 AM
> To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@intel.com>
> Cc: 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: Re: [edk2-devel] [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
>
> Is there a reason this other content can't go into it's own FV?
>
> On 2/9/2023 12:14 AM, Chiu, Chasel wrote:
>> Platform may implement Other NVS variable 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.
>>
>> 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 | 7 +++++++
>> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 7 ++++---
>> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8 ++++++++
>> 3 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git
>> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
>> iceCommon.c
>> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
>> iceCommon.c
>> index 942abf95a6..bcde98131d 100644
>> ---
>> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
>> iceCommon.c
>> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
>> +++ ServiceCommon.c
>> @@ -568,6 +568,13 @@ GetVariableFvInfo (
>> return;
>>
>> }
>>
>>
>>
>> + //
>>
>> + // GetVariableFlashNvStorageInfo () only reports regular variable
>> + region information,
>>
>> + // if platform implemented a separate Other variable region
>> + following the regular variable region,
>>
>> + // the size should be included as overall NVS variable region size.
>>
>> + //
>>
>> + NvStoreLength += PcdGet32 (PcdFlashNvStorageOtherVariableSize);
>>
>> +
>>
>> 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/SpiFvbServ
>> iceSmm.inf
>> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
>> iceSmm.inf
>> index 73049eceb2..f40067418a 100644
>> ---
>> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
>> iceSmm.inf
>> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
>> +++ ServiceSmm.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.PcdFlashNvStorageOtherVariableSize
>> + ## CONSUMES
>>
>>
>>
>> [Sources]
>>
>> FvbInfo.c
>>
>> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
>> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
>> index 63dae756ad..7034ab93b0 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|0x
>> 0000000E
>>
>> +
>>
>> + ## Declares Separate NVS Variable Region Size.<BR><BR>
>>
>> + # Platform may implement a Regular variable region and an Other
>> + variable region, which will require this PCD
>>
>> + # to tell SpiFvbService to include both regions.<BR>
>>
>> + # 0: No separate Other variable region.<BR>
>>
>> + # non-zero: The size of a separate Other variable region following
>> + the Regular variable region.<BR>
>>
>> + # @Prompt Separate NVS Variable Region Size.
>>
>> +
>> + gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageOtherVariableSize|0x
>> + 00000000|UINT32|0x0000000F
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
2023-02-09 16:46 ` Michael Kubacki
@ 2023-02-09 17:25 ` Chiu, Chasel
[not found] ` <174237FCE67B704B.15261@groups.io>
1 sibling, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2023-02-09 17:25 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com, Oram, Isaac W
Cc: S, Ashraf Ali, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael
Thanks for good suggestions Isaac and Michael!
I have sent V3 patch to apply all the suggestions, please help to review again.
Thanks,
Chasel
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Thursday, February 9, 2023 8:47 AM
> To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>
> 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-devel] [edk2-platforms: PATCH]
> IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
>
> Thanks, that's useful background. @chasel, you should probably put this info in
> the commit message so it is captured in source history.
>
> Given the default value is zero, it seems reasonable. I was also initially confused
> by the name of the PCD.
>
> Another idea would be something like "PcdFlashNvStorageAdditionalSize".
>
> Please do at least update the commit message to include additional context.
>
> Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
>
> On 2/9/2023 11:04 AM, Isaac Oram wrote:
> > It is a legacy that exists in current and past implementations. There is a
> complex arbitrary relationship between the runtime updateable regions in
> existing platform designs.
> > There is something like:
> > - Variable store (large)
> > - Error log (small)
> > - Fault tolerant working area (>= size of prior 2 regions)
> > - Fault tolerant metadata (small).
> > And there are assumptions about ordering and packing built into board flash
> layouts.
> >
> > I don't think that we should introduce "other variable" as a concept, because
> variable solutions don't support two regions, so it isn't a UEFI variable region.
> OtherUpdatable might be ok, but still seems confusing to me.
> > I think that we should add the support for the *ErrorLog* region so that the
> open FvbServices can be used by current implementations. Then we should
> eliminate the "ErrorLog" use completely. My thought is that this makes the
> connection to legacy clear. And also motivates us to eliminate all the vestigial
> references to the ErrorLog in edk2 and edk2-platforms.
> >
> > New updateable regions should not be hard-coded into this area and should
> have a cleaner solution, as Michael suggests.
> >
> > I understand if we don't want to support legacy or workarounds, but I think
> that currently adoption and use of the open content is higher priority. Which is
> why we are requesting this workaround to match "proprietary" FVB services
> behavior.
> >
> > Regards,
> > Isaac
> >
> > -----Original Message-----
> > From: Michael Kubacki <mikuback@linux.microsoft.com>
> > Sent: Thursday, February 9, 2023 7:40 AM
> > To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@intel.com>
> > Cc: 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: Re: [edk2-devel] [edk2-platforms: PATCH]
> IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
> >
> > Is there a reason this other content can't go into it's own FV?
> >
> > On 2/9/2023 12:14 AM, Chiu, Chasel wrote:
> >> Platform may implement Other NVS variable 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.
> >>
> >> 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 | 7 +++++++
> >>
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> | 7 ++++---
> >> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8
> ++++++++
> >> 3 files changed, 19 insertions(+), 3 deletions(-)
> >>
> >> diff --git
> >> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer
> >> v
> >> iceCommon.c
> >> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer
> >> v
> >> iceCommon.c
> >> index 942abf95a6..bcde98131d 100644
> >> ---
> >> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer
> >> v
> >> iceCommon.c
> >> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFv
> >> +++ b
> >> +++ ServiceCommon.c
> >> @@ -568,6 +568,13 @@ GetVariableFvInfo (
> >> return;
> >>
> >> }
> >>
> >>
> >>
> >> + //
> >>
> >> + // GetVariableFlashNvStorageInfo () only reports regular variable
> >> + region information,
> >>
> >> + // if platform implemented a separate Other variable region
> >> + following the regular variable region,
> >>
> >> + // the size should be included as overall NVS variable region size.
> >>
> >> + //
> >>
> >> + NvStoreLength += PcdGet32 (PcdFlashNvStorageOtherVariableSize);
> >>
> >> +
> >>
> >> 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/SpiFvbSer
> >> v
> >> iceSmm.inf
> >> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer
> >> v
> >> iceSmm.inf
> >> index 73049eceb2..f40067418a 100644
> >> ---
> >> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSer
> >> v
> >> iceSmm.inf
> >> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFv
> >> +++ b
> >> +++ ServiceSmm.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.PcdFlashNvStorageOtherVariableSize
> >> + ## CONSUMES
> >>
> >>
> >>
> >> [Sources]
> >>
> >> FvbInfo.c
> >>
> >> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> >> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> >> index 63dae756ad..7034ab93b0 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|0
> >> x
> >> 0000000E
> >>
> >> +
> >>
> >> + ## Declares Separate NVS Variable Region Size.<BR><BR>
> >>
> >> + # Platform may implement a Regular variable region and an Other
> >> + variable region, which will require this PCD
> >>
> >> + # to tell SpiFvbService to include both regions.<BR>
> >>
> >> + # 0: No separate Other variable region.<BR>
> >>
> >> + # non-zero: The size of a separate Other variable region
> >> + following the Regular variable region.<BR>
> >>
> >> + # @Prompt Separate NVS Variable Region Size.
> >>
> >> +
> >> + gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageOtherVariableSize|0
> >> + x
> >> + 00000000|UINT32|0x0000000F
> >>
> >
> >
> >
> >
> >
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
[not found] ` <174237FCE67B704B.15261@groups.io>
@ 2023-02-09 18:28 ` Chiu, Chasel
0 siblings, 0 replies; 6+ messages in thread
From: Chiu, Chasel @ 2023-02-09 18:28 UTC (permalink / raw)
To: devel@edk2.groups.io, Chiu, Chasel, mikuback@linux.microsoft.com,
Oram, Isaac W
Cc: S, Ashraf Ali, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael
Hello,
Another V4 patch sent for applying more comment/commit message feedbacks form Isaac.
Please help to review again.
Thanks,
Chasel
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
> Sent: Thursday, February 9, 2023 9:26 AM
> To: devel@edk2.groups.io; mikuback@linux.microsoft.com; Oram, Isaac W
> <isaac.w.oram@intel.com>
> 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-devel] [edk2-platforms: PATCH]
> IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
>
>
> Thanks for good suggestions Isaac and Michael!
> I have sent V3 patch to apply all the suggestions, please help to review again.
>
> Thanks,
> Chasel
>
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> > Kubacki
> > Sent: Thursday, February 9, 2023 8:47 AM
> > To: devel@edk2.groups.io; Oram, Isaac W <isaac.w.oram@intel.com>;
> > Chiu, Chasel <chasel.chiu@intel.com>
> > 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-devel] [edk2-platforms: PATCH]
> > IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
> >
> > Thanks, that's useful background. @chasel, you should probably put
> > this info in the commit message so it is captured in source history.
> >
> > Given the default value is zero, it seems reasonable. I was also
> > initially confused by the name of the PCD.
> >
> > Another idea would be something like "PcdFlashNvStorageAdditionalSize".
> >
> > Please do at least update the commit message to include additional context.
> >
> > Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > On 2/9/2023 11:04 AM, Isaac Oram wrote:
> > > It is a legacy that exists in current and past implementations.
> > > There is a
> > complex arbitrary relationship between the runtime updateable regions
> > in existing platform designs.
> > > There is something like:
> > > - Variable store (large)
> > > - Error log (small)
> > > - Fault tolerant working area (>= size of prior 2 regions)
> > > - Fault tolerant metadata (small).
> > > And there are assumptions about ordering and packing built into
> > > board flash
> > layouts.
> > >
> > > I don't think that we should introduce "other variable" as a
> > > concept, because
> > variable solutions don't support two regions, so it isn't a UEFI variable region.
> > OtherUpdatable might be ok, but still seems confusing to me.
> > > I think that we should add the support for the *ErrorLog* region so
> > > that the
> > open FvbServices can be used by current implementations. Then we
> > should eliminate the "ErrorLog" use completely. My thought is that
> > this makes the connection to legacy clear. And also motivates us to
> > eliminate all the vestigial references to the ErrorLog in edk2 and edk2-
> platforms.
> > >
> > > New updateable regions should not be hard-coded into this area and
> > > should
> > have a cleaner solution, as Michael suggests.
> > >
> > > I understand if we don't want to support legacy or workarounds, but
> > > I think
> > that currently adoption and use of the open content is higher
> > priority. Which is why we are requesting this workaround to match
> > "proprietary" FVB services behavior.
> > >
> > > Regards,
> > > Isaac
> > >
> > > -----Original Message-----
> > > From: Michael Kubacki <mikuback@linux.microsoft.com>
> > > Sent: Thursday, February 9, 2023 7:40 AM
> > > To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@intel.com>
> > > Cc: 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: Re: [edk2-devel] [edk2-platforms: PATCH]
> > IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region.
> > >
> > > Is there a reason this other content can't go into it's own FV?
> > >
> > > On 2/9/2023 12:14 AM, Chiu, Chasel wrote:
> > >> Platform may implement Other NVS variable 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.
> > >>
> > >> 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/SpiFvbServic
> > eCommon
> > .c | 7 +++++++
> > >>
> > Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServic
> > eSmm.inf
> > | 7 ++++---
> > >> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8
> > ++++++++
> > >> 3 files changed, 19 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git
> > >> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbS
> > >> er
> > >> v
> > >> iceCommon.c
> > >> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbS
> > >> er
> > >> v
> > >> iceCommon.c
> > >> index 942abf95a6..bcde98131d 100644
> > >> ---
> > >> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbS
> > >> er
> > >> v
> > >> iceCommon.c
> > >> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Spi
> > >> +++ Fv
> > >> +++ b
> > >> +++ ServiceCommon.c
> > >> @@ -568,6 +568,13 @@ GetVariableFvInfo (
> > >> return;
> > >>
> > >> }
> > >>
> > >>
> > >>
> > >> + //
> > >>
> > >> + // GetVariableFlashNvStorageInfo () only reports regular
> > >> + variable region information,
> > >>
> > >> + // if platform implemented a separate Other variable region
> > >> + following the regular variable region,
> > >>
> > >> + // the size should be included as overall NVS variable region size.
> > >>
> > >> + //
> > >>
> > >> + NvStoreLength += PcdGet32 (PcdFlashNvStorageOtherVariableSize);
> > >>
> > >> +
> > >>
> > >> 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/SpiFvbS
> > >> er
> > >> v
> > >> iceSmm.inf
> > >> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbS
> > >> er
> > >> v
> > >> iceSmm.inf
> > >> index 73049eceb2..f40067418a 100644
> > >> ---
> > >> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbS
> > >> er
> > >> v
> > >> iceSmm.inf
> > >> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/Spi
> > >> +++ Fv
> > >> +++ b
> > >> +++ ServiceSmm.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.PcdFlashNvStorageOtherVariableSize
> > >> + ## CONSUMES
> > >>
> > >>
> > >>
> > >> [Sources]
> > >>
> > >> FvbInfo.c
> > >>
> > >> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> > >> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> > >> index 63dae756ad..7034ab93b0 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
> > >> |0
> > >> x
> > >> 0000000E
> > >>
> > >> +
> > >>
> > >> + ## Declares Separate NVS Variable Region Size.<BR><BR>
> > >>
> > >> + # Platform may implement a Regular variable region and an Other
> > >> + variable region, which will require this PCD
> > >>
> > >> + # to tell SpiFvbService to include both regions.<BR>
> > >>
> > >> + # 0: No separate Other variable region.<BR>
> > >>
> > >> + # non-zero: The size of a separate Other variable region
> > >> + following the Regular variable region.<BR>
> > >>
> > >> + # @Prompt Separate NVS Variable Region Size.
> > >>
> > >> +
> > >> + gIntelSiliconPkgTokenSpaceGuid.PcdFlashNvStorageOtherVariableSize
> > >> + |0
> > >> + x
> > >> + 00000000|UINT32|0x0000000F
> > >>
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-09 18:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-09 5:14 [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Support Other NVS variable region Chiu, Chasel
2023-02-09 15:39 ` [edk2-devel] " Michael Kubacki
2023-02-09 16:04 ` Isaac Oram
2023-02-09 16:46 ` Michael Kubacki
2023-02-09 17:25 ` Chiu, Chasel
[not found] ` <174237FCE67B704B.15261@groups.io>
2023-02-09 18:28 ` Chiu, Chasel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox