1. Patchew
  2. EDK2
  3. [edk2] [PATCH v2 0/5] OvmfPkg: add FD_SIZE_4MB for Windows HCK SB tests, and for future proofing
  4. View patch

[edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB

Laszlo Ersek posted 5 patches 7 years, 6 months ago
[edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB
Posted by Laszlo Ersek 7 years, 6 months ago 
The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
Certification Kit sets a 32 KB large non-authenticated variable.

In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
for the variable name and attributes as well.

Cc: Gary Ching-Pang Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - adjust to FD_SIZE_IN_KB
    - update commit msg to state 256 KB for the varstore [Jordan]

 OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
 OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
 OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 26b807dde9fa..e0779ddaa426 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -404,28 +404,34 @@ [PcdsFeatureFlag]
   gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
 !endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
 !endif
 
 [PcdsFixedAtBuild]
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
+!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+!endif
+!if $(FD_SIZE_IN_KB) == 4096
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
+!endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
 
   # DEBUG_INIT      0x00000001  // Initialization
   # DEBUG_WARN      0x00000002  // Warnings
   # DEBUG_LOAD      0x00000004  // Load events
   # DEBUG_FS        0x00000008  // EFI File system
   # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
   # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
   # DEBUG_INFO      0x00000040  // Informational debug messages
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 41f06a6b6a66..bbe26e2cf452 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -409,28 +409,34 @@ [PcdsFeatureFlag]
   gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
 !endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
 !endif
 
 [PcdsFixedAtBuild]
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
+!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+!endif
+!if $(FD_SIZE_IN_KB) == 4096
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
+!endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
 
   # DEBUG_INIT      0x00000001  // Initialization
   # DEBUG_WARN      0x00000002  // Warnings
   # DEBUG_LOAD      0x00000004  // Load events
   # DEBUG_FS        0x00000008  // EFI File system
   # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
   # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
   # DEBUG_INFO      0x00000040  // Informational debug messages
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 053c84b685c5..ff795815f65f 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -409,28 +409,34 @@ [PcdsFeatureFlag]
   gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
 !endif
 !if $(SMM_REQUIRE) == TRUE
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
 !endif
 
 [PcdsFixedAtBuild]
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
+!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+!endif
+!if $(FD_SIZE_IN_KB) == 4096
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
+!endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
 
   # DEBUG_INIT      0x00000001  // Initialization
   # DEBUG_WARN      0x00000002  // Warnings
   # DEBUG_LOAD      0x00000004  // Load events
   # DEBUG_FS        0x00000008  // EFI File system
   # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
   # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
   # DEBUG_INFO      0x00000040  // Informational debug messages
-- 
2.9.3


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB
Posted by Jordan Justen 7 years, 6 months ago 
On 2017-05-03 14:39:46, Laszlo Ersek wrote:
> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
> Certification Kit sets a 32 KB large non-authenticated variable.

According to
http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf

"The maximum supported variable size must be at least 64kB"

Should we just bump the size to match this? We should be able to make
this change later once it is in a test/spec, but for some reason I
thought the requirement was already 64k.

Aside from this question:

Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
> for the variable name and attributes as well.
> 
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - adjust to FD_SIZE_IN_KB
>     - update commit msg to state 256 KB for the varstore [Jordan]
> 
>  OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
>  OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 26b807dde9fa..e0779ddaa426 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>  !endif
>  !if $(SMM_REQUIRE) == TRUE
>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>  !endif
>  
>  [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +!endif
> +!if $(FD_SIZE_IN_KB) == 4096
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> +!endif
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>  
>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>  
>    # DEBUG_INIT      0x00000001  // Initialization
>    # DEBUG_WARN      0x00000002  // Warnings
>    # DEBUG_LOAD      0x00000004  // Load events
>    # DEBUG_FS        0x00000008  // EFI File system
>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>    # DEBUG_INFO      0x00000040  // Informational debug messages
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 41f06a6b6a66..bbe26e2cf452 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>  !endif
>  !if $(SMM_REQUIRE) == TRUE
>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>  !endif
>  
>  [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +!endif
> +!if $(FD_SIZE_IN_KB) == 4096
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> +!endif
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>  
>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>  
>    # DEBUG_INIT      0x00000001  // Initialization
>    # DEBUG_WARN      0x00000002  // Warnings
>    # DEBUG_LOAD      0x00000004  // Load events
>    # DEBUG_FS        0x00000008  // EFI File system
>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>    # DEBUG_INFO      0x00000040  // Informational debug messages
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 053c84b685c5..ff795815f65f 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>  !endif
>  !if $(SMM_REQUIRE) == TRUE
>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>  !endif
>  
>  [PcdsFixedAtBuild]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +!endif
> +!if $(FD_SIZE_IN_KB) == 4096
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> +!endif
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>  
>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>  
>    # DEBUG_INIT      0x00000001  // Initialization
>    # DEBUG_WARN      0x00000002  // Warnings
>    # DEBUG_LOAD      0x00000004  // Load events
>    # DEBUG_FS        0x00000008  // EFI File system
>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>    # DEBUG_INFO      0x00000040  // Informational debug messages
> -- 
> 2.9.3
> 
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB
Posted by Laszlo Ersek 7 years, 6 months ago 
On 05/04/17 18:36, Jordan Justen wrote:
> On 2017-05-03 14:39:46, Laszlo Ersek wrote:
>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
>> Certification Kit sets a 32 KB large non-authenticated variable.
> 
> According to
> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
> 
> "The maximum supported variable size must be at least 64kB"
> 
> Should we just bump the size to match this? We should be able to make
> this change later once it is in a test/spec, but for some reason I
> thought the requirement was already 64k.

The 32KB requirement comes from the most recent Secure Boot Logo Test. I
installed both the Windows Server 2008 R2 SP1 test controller and the
Windows 2016 Server test client just the other day, together with the
most recent filters, using the following descriptions:

https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM

Given that this limit can be bumped without breaking compatibility, as
you say, I'd like to remain frugal with it, same as we were in James's
commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
Authenticated variables", 2016-03-24).

I don't understand why the plugfest presentation and the SB Logo Test
require different limits... But, I'm certain our QE will find out in
short order once the SB Logo Test catches up with the presentation, and
I expect I'll submit the corresponding patch soon after.

I dislike the speculation in this series, but breaking compatibility is
even worse. (A lot worse, to me at least.) So I consider the varstore
restructuring the smaller of two wrongs. However, wrt.
PcdMaxVariableSize, it seems we're not being forced to either of those
wrongs (i.e., breaking compat or speculation), so we can delay the increase.

> 
> Aside from this question:
> 
> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Thanks a lot!

I'll await your ACK for the above argument before pushing the series.

Thanks,
Laszlo

>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
>> for the variable name and attributes as well.
>>
>> Cc: Gary Ching-Pang Lin <glin@suse.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     v2:
>>     - adjust to FD_SIZE_IN_KB
>>     - update commit msg to state 256 KB for the varstore [Jordan]
>>
>>  OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
>>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
>>  OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
>>  3 files changed, 18 insertions(+)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 26b807dde9fa..e0779ddaa426 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>  !endif
>>  !if $(SMM_REQUIRE) == TRUE
>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>  !endif
>>  
>>  [PcdsFixedAtBuild]
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>> +!endif
>> +!if $(FD_SIZE_IN_KB) == 4096
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>> +!endif
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>  
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>  
>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>  
>>    # DEBUG_INIT      0x00000001  // Initialization
>>    # DEBUG_WARN      0x00000002  // Warnings
>>    # DEBUG_LOAD      0x00000004  // Load events
>>    # DEBUG_FS        0x00000008  // EFI File system
>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 41f06a6b6a66..bbe26e2cf452 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>  !endif
>>  !if $(SMM_REQUIRE) == TRUE
>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>  !endif
>>  
>>  [PcdsFixedAtBuild]
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>> +!endif
>> +!if $(FD_SIZE_IN_KB) == 4096
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>> +!endif
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>  
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>  
>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>  
>>    # DEBUG_INIT      0x00000001  // Initialization
>>    # DEBUG_WARN      0x00000002  // Warnings
>>    # DEBUG_LOAD      0x00000004  // Load events
>>    # DEBUG_FS        0x00000008  // EFI File system
>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index 053c84b685c5..ff795815f65f 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>  !endif
>>  !if $(SMM_REQUIRE) == TRUE
>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>  !endif
>>  
>>  [PcdsFixedAtBuild]
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>> +!endif
>> +!if $(FD_SIZE_IN_KB) == 4096
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>> +!endif
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>  
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>  
>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>  
>>    # DEBUG_INIT      0x00000001  // Initialization
>>    # DEBUG_WARN      0x00000002  // Warnings
>>    # DEBUG_LOAD      0x00000004  // Load events
>>    # DEBUG_FS        0x00000008  // EFI File system
>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>> -- 
>> 2.9.3
>>
>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB
Posted by Jordan Justen 7 years, 6 months ago 
On 2017-05-04 09:52:48, Laszlo Ersek wrote:
> On 05/04/17 18:36, Jordan Justen wrote:
> > On 2017-05-03 14:39:46, Laszlo Ersek wrote:
> >> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
> >> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
> >> Certification Kit sets a 32 KB large non-authenticated variable.
> > 
> > According to
> > http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
> > 
> > "The maximum supported variable size must be at least 64kB"
> > 
> > Should we just bump the size to match this? We should be able to make
> > this change later once it is in a test/spec, but for some reason I
> > thought the requirement was already 64k.
> 
> The 32KB requirement comes from the most recent Secure Boot Logo Test. I

If the limit is 32k, why go with 33k? Does the test fail with a 32k
limit?

-Jordan

> installed both the Windows Server 2008 R2 SP1 test controller and the
> Windows 2016 Server test client just the other day, together with the
> most recent filters, using the following descriptions:
> 
> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM
> 
> Given that this limit can be bumped without breaking compatibility, as
> you say, I'd like to remain frugal with it, same as we were in James's
> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
> Authenticated variables", 2016-03-24).
> 
> I don't understand why the plugfest presentation and the SB Logo Test
> require different limits... But, I'm certain our QE will find out in
> short order once the SB Logo Test catches up with the presentation, and
> I expect I'll submit the corresponding patch soon after.
> 
> I dislike the speculation in this series, but breaking compatibility is
> even worse. (A lot worse, to me at least.) So I consider the varstore
> restructuring the smaller of two wrongs. However, wrt.
> PcdMaxVariableSize, it seems we're not being forced to either of those
> wrongs (i.e., breaking compat or speculation), so we can delay the increase.
> 
> > 
> > Aside from this question:
> > 
> > Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> 
> Thanks a lot!
> 
> I'll await your ACK for the above argument before pushing the series.
> 
> Thanks,
> Laszlo
> 
> >> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
> >> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
> >> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
> >> for the variable name and attributes as well.
> >>
> >> Cc: Gary Ching-Pang Lin <glin@suse.com>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >>
> >> Notes:
> >>     v2:
> >>     - adjust to FD_SIZE_IN_KB
> >>     - update commit msg to state 256 KB for the varstore [Jordan]
> >>
> >>  OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
> >>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
> >>  OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
> >>  3 files changed, 18 insertions(+)
> >>
> >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> >> index 26b807dde9fa..e0779ddaa426 100644
> >> --- a/OvmfPkg/OvmfPkgIa32.dsc
> >> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> >> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
> >>  !endif
> >>  !if $(SMM_REQUIRE) == TRUE
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> >>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> >>  !endif
> >>  
> >>  [PcdsFixedAtBuild]
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> >>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> >> +!endif
> >> +!if $(FD_SIZE_IN_KB) == 4096
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> >> +!endif
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
> >>  
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
> >>  
> >>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
> >>  
> >>    # DEBUG_INIT      0x00000001  // Initialization
> >>    # DEBUG_WARN      0x00000002  // Warnings
> >>    # DEBUG_LOAD      0x00000004  // Load events
> >>    # DEBUG_FS        0x00000008  // EFI File system
> >>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
> >>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
> >>    # DEBUG_INFO      0x00000040  // Informational debug messages
> >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> >> index 41f06a6b6a66..bbe26e2cf452 100644
> >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> >> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
> >>  !endif
> >>  !if $(SMM_REQUIRE) == TRUE
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> >>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> >>  !endif
> >>  
> >>  [PcdsFixedAtBuild]
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> >>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> >> +!endif
> >> +!if $(FD_SIZE_IN_KB) == 4096
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> >> +!endif
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
> >>  
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
> >>  
> >>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
> >>  
> >>    # DEBUG_INIT      0x00000001  // Initialization
> >>    # DEBUG_WARN      0x00000002  // Warnings
> >>    # DEBUG_LOAD      0x00000004  // Load events
> >>    # DEBUG_FS        0x00000008  // EFI File system
> >>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
> >>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
> >>    # DEBUG_INFO      0x00000040  // Informational debug messages
> >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> >> index 053c84b685c5..ff795815f65f 100644
> >> --- a/OvmfPkg/OvmfPkgX64.dsc
> >> +++ b/OvmfPkg/OvmfPkgX64.dsc
> >> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
> >>  !endif
> >>  !if $(SMM_REQUIRE) == TRUE
> >>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> >>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> >>  !endif
> >>  
> >>  [PcdsFixedAtBuild]
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> >>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> >> +!endif
> >> +!if $(FD_SIZE_IN_KB) == 4096
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> >> +!endif
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
> >>  
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
> >>  
> >>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
> >>  
> >>    # DEBUG_INIT      0x00000001  // Initialization
> >>    # DEBUG_WARN      0x00000002  // Warnings
> >>    # DEBUG_LOAD      0x00000004  // Load events
> >>    # DEBUG_FS        0x00000008  // EFI File system
> >>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
> >>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
> >>    # DEBUG_INFO      0x00000040  // Informational debug messages
> >> -- 
> >> 2.9.3
> >>
> >>
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB
Posted by Laszlo Ersek 7 years, 6 months ago 
On 05/04/17 20:50, Jordan Justen wrote:
> On 2017-05-04 09:52:48, Laszlo Ersek wrote:
>> On 05/04/17 18:36, Jordan Justen wrote:
>>> On 2017-05-03 14:39:46, Laszlo Ersek wrote:
>>>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
>>>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
>>>> Certification Kit sets a 32 KB large non-authenticated variable.
>>>
>>> According to
>>> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
>>>
>>> "The maximum supported variable size must be at least 64kB"
>>>
>>> Should we just bump the size to match this? We should be able to make
>>> this change later once it is in a test/spec, but for some reason I
>>> thought the requirement was already 64k.
>>
>> The 32KB requirement comes from the most recent Secure Boot Logo Test. I
>
> If the limit is 32k, why go with 33k? Does the test fail with a 32k
> limit?

Yes, it does. First I tried setting the PCDs to 0x8000 sharp, and then
the test failed.

Then I remembered that the PCDs present a limit on data, name, and
attributes together:

(1) In "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c", function
InitNonVolatileVariableStore(), we stash the PCDs like this:

>   mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
>   mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlobal->MaxVariableSize);

(2) In the same file, function VariableServiceSetVariable(), we have the
following size checks:

>   //
>   //  The size of the VariableName, including the Unicode Null in bytes plus
>   //  the DataSize is limited to maximum size of PcdGet32 (PcdMaxHardwareErrorVariableSize)
>   //  bytes for HwErrRec#### variable.
>   //
>   if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
>     if (StrSize (VariableName) + PayloadSize > PcdGet32 (PcdMaxHardwareErrorVariableSize) - GetVariableHeaderSize ()) {
>       return EFI_INVALID_PARAMETER;
>     }
>   } else {
>     //
>     //  The size of the VariableName, including the Unicode Null in bytes plus
>     //  the DataSize is limited to maximum size of Max(Auth)VariableSize bytes.
>     //
>     if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
>       if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
>         return EFI_INVALID_PARAMETER;
>       }
>     } else {
>       if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
>         return EFI_INVALID_PARAMETER;
>       }
>     }
>   }

That is, the PCDs limit the sum of:
- the variable name (including the terminating L'\0'),
- the variable contents,
- the internal header (which carries the attributes as well). In our
  case that's AUTHENTICATED_VARIABLE_HEADER, because we always format
  the auth varstore GUID into the template.

Furthermore, the 32KB data size for the contents is not based on
"reverse engineering"; the Secure Boot Logo Test tells you exactly what
it is trying to do. (In fact, there is no other way to fix failures in
any of the tests; you can only rely on the messages in order to see what
the test attempted.)

In this case, we have (see
<https://bugzilla.redhat.com/show_bug.cgi?id=1443351#c27>):

> NOTE: if this testcase fails, try restarting the machine and then
> running this test case on its own, for example: "te.exe
> UefiSecureBootLogo.dll
> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>
> Test creation of 1 large variable of size 32768 bytes.
>
> Test creation of testLargeVariable Unexpected Error - SetVariable
> failed with status 0xC000000D VariableName "testLargeVariable"
> VendorGuid 77fa9abd-0359-4d32-bd60-28f4e78f784b Attributes 0x00000007
> DataSize 0x00008000

It reports all the parameters of the gRT->SetVariable() call that
failed, except "Data".

In the successful test (with this patch applied), the messages are (see
<https://bugzilla.redhat.com/attachment.cgi?id=1275172>):

> NOTE: if this testcase fails, try restarting the machine and then
> running this test case on its own, for example: "te.exe
> UefiSecureBootLogo.dll
> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>
> Test creation of 1 large variable of size 32768 bytes.
>
> AreEqual(7, 7) - testLargeVariable: Attributes from GetVariable()
> match SetVariable()
>
> AreEqual(32768, 32768) - testLargeVariable: DataSize from
> GetVariable() matches SetVariable()

Thanks
Laszlo

>
>> installed both the Windows Server 2008 R2 SP1 test controller and the
>> Windows 2016 Server test client just the other day, together with the
>> most recent filters, using the following descriptions:
>>
>> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
>> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
>> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM
>>
>> Given that this limit can be bumped without breaking compatibility, as
>> you say, I'd like to remain frugal with it, same as we were in James's
>> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
>> Authenticated variables", 2016-03-24).
>>
>> I don't understand why the plugfest presentation and the SB Logo Test
>> require different limits... But, I'm certain our QE will find out in
>> short order once the SB Logo Test catches up with the presentation, and
>> I expect I'll submit the corresponding patch soon after.
>>
>> I dislike the speculation in this series, but breaking compatibility is
>> even worse. (A lot worse, to me at least.) So I consider the varstore
>> restructuring the smaller of two wrongs. However, wrt.
>> PcdMaxVariableSize, it seems we're not being forced to either of those
>> wrongs (i.e., breaking compat or speculation), so we can delay the increase.
>>
>>>
>>> Aside from this question:
>>>
>>> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
>>
>> Thanks a lot!
>>
>> I'll await your ACK for the above argument before pushing the series.
>>
>> Thanks,
>> Laszlo
>>
>>>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
>>>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
>>>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
>>>> for the variable name and attributes as well.
>>>>
>>>> Cc: Gary Ching-Pang Lin <glin@suse.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>>> ---
>>>>
>>>> Notes:
>>>>     v2:
>>>>     - adjust to FD_SIZE_IN_KB
>>>>     - update commit msg to state 256 KB for the varstore [Jordan]
>>>>
>>>>  OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
>>>>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
>>>>  OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
>>>>  3 files changed, 18 insertions(+)
>>>>
>>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>>> index 26b807dde9fa..e0779ddaa426 100644
>>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>>  !endif
>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>>  !endif
>>>>
>>>>  [PcdsFixedAtBuild]
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>>> +!endif
>>>> +!if $(FD_SIZE_IN_KB) == 4096
>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>>> +!endif
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>>
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>>
>>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>>
>>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>> index 41f06a6b6a66..bbe26e2cf452 100644
>>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>>  !endif
>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>>  !endif
>>>>
>>>>  [PcdsFixedAtBuild]
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>>> +!endif
>>>> +!if $(FD_SIZE_IN_KB) == 4096
>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>>> +!endif
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>>
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>>
>>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>>
>>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>>>> index 053c84b685c5..ff795815f65f 100644
>>>> --- a/OvmfPkg/OvmfPkgX64.dsc
>>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>>  !endif
>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>>  !endif
>>>>
>>>>  [PcdsFixedAtBuild]
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>>> +!endif
>>>> +!if $(FD_SIZE_IN_KB) == 4096
>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>>> +!endif
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>>
>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>>
>>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>>
>>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>>> --
>>>> 2.9.3
>>>>
>>>>
>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB
Posted by Laszlo Ersek 7 years, 6 months ago 
On 05/04/17 21:27, Laszlo Ersek wrote:
> On 05/04/17 20:50, Jordan Justen wrote:
>> On 2017-05-04 09:52:48, Laszlo Ersek wrote:
>>> On 05/04/17 18:36, Jordan Justen wrote:
>>>> On 2017-05-03 14:39:46, Laszlo Ersek wrote:
>>>>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
>>>>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
>>>>> Certification Kit sets a 32 KB large non-authenticated variable.
>>>>
>>>> According to
>>>> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
>>>>
>>>> "The maximum supported variable size must be at least 64kB"
>>>>
>>>> Should we just bump the size to match this? We should be able to make
>>>> this change later once it is in a test/spec, but for some reason I
>>>> thought the requirement was already 64k.
>>>
>>> The 32KB requirement comes from the most recent Secure Boot Logo Test. I
>>
>> If the limit is 32k, why go with 33k? Does the test fail with a 32k
>> limit?
> 
> Yes, it does. First I tried setting the PCDs to 0x8000 sharp, and then
> the test failed.
> 
> Then I remembered that the PCDs present a limit on data, name, and
> attributes together:
> 
> (1) In "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c", function
> InitNonVolatileVariableStore(), we stash the PCDs like this:
> 
>>   mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
>>   mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlobal->MaxVariableSize);
> 
> (2) In the same file, function VariableServiceSetVariable(), we have the
> following size checks:
> 
>>   //
>>   //  The size of the VariableName, including the Unicode Null in bytes plus
>>   //  the DataSize is limited to maximum size of PcdGet32 (PcdMaxHardwareErrorVariableSize)
>>   //  bytes for HwErrRec#### variable.
>>   //
>>   if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
>>     if (StrSize (VariableName) + PayloadSize > PcdGet32 (PcdMaxHardwareErrorVariableSize) - GetVariableHeaderSize ()) {
>>       return EFI_INVALID_PARAMETER;
>>     }
>>   } else {
>>     //
>>     //  The size of the VariableName, including the Unicode Null in bytes plus
>>     //  the DataSize is limited to maximum size of Max(Auth)VariableSize bytes.
>>     //
>>     if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
>>       if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
>>         return EFI_INVALID_PARAMETER;
>>       }
>>     } else {
>>       if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
>>         return EFI_INVALID_PARAMETER;
>>       }
>>     }
>>   }
> 
> That is, the PCDs limit the sum of:
> - the variable name (including the terminating L'\0'),
> - the variable contents,
> - the internal header (which carries the attributes as well). In our
>   case that's AUTHENTICATED_VARIABLE_HEADER, because we always format
>   the auth varstore GUID into the template.

(NB I mentioned this in the commit message, just not in such detail.)

Thanks
Laszlo

> 
> Furthermore, the 32KB data size for the contents is not based on
> "reverse engineering"; the Secure Boot Logo Test tells you exactly what
> it is trying to do. (In fact, there is no other way to fix failures in
> any of the tests; you can only rely on the messages in order to see what
> the test attempted.)
> 
> In this case, we have (see
> <https://bugzilla.redhat.com/show_bug.cgi?id=1443351#c27>):
> 
>> NOTE: if this testcase fails, try restarting the machine and then
>> running this test case on its own, for example: "te.exe
>> UefiSecureBootLogo.dll
>> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>>
>> Test creation of 1 large variable of size 32768 bytes.
>>
>> Test creation of testLargeVariable Unexpected Error - SetVariable
>> failed with status 0xC000000D VariableName "testLargeVariable"
>> VendorGuid 77fa9abd-0359-4d32-bd60-28f4e78f784b Attributes 0x00000007
>> DataSize 0x00008000
> 
> It reports all the parameters of the gRT->SetVariable() call that
> failed, except "Data".
> 
> In the successful test (with this patch applied), the messages are (see
> <https://bugzilla.redhat.com/attachment.cgi?id=1275172>):
> 
>> NOTE: if this testcase fails, try restarting the machine and then
>> running this test case on its own, for example: "te.exe
>> UefiSecureBootLogo.dll
>> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>>
>> Test creation of 1 large variable of size 32768 bytes.
>>
>> AreEqual(7, 7) - testLargeVariable: Attributes from GetVariable()
>> match SetVariable()
>>
>> AreEqual(32768, 32768) - testLargeVariable: DataSize from
>> GetVariable() matches SetVariable()
> 
> Thanks
> Laszlo
> 
>>
>>> installed both the Windows Server 2008 R2 SP1 test controller and the
>>> Windows 2016 Server test client just the other day, together with the
>>> most recent filters, using the following descriptions:
>>>
>>> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
>>> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
>>> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM
>>>
>>> Given that this limit can be bumped without breaking compatibility, as
>>> you say, I'd like to remain frugal with it, same as we were in James's
>>> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
>>> Authenticated variables", 2016-03-24).
>>>
>>> I don't understand why the plugfest presentation and the SB Logo Test
>>> require different limits... But, I'm certain our QE will find out in
>>> short order once the SB Logo Test catches up with the presentation, and
>>> I expect I'll submit the corresponding patch soon after.
>>>
>>> I dislike the speculation in this series, but breaking compatibility is
>>> even worse. (A lot worse, to me at least.) So I consider the varstore
>>> restructuring the smaller of two wrongs. However, wrt.
>>> PcdMaxVariableSize, it seems we're not being forced to either of those
>>> wrongs (i.e., breaking compat or speculation), so we can delay the increase.
>>>
>>>>
>>>> Aside from this question:
>>>>
>>>> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
>>>
>>> Thanks a lot!
>>>
>>> I'll await your ACK for the above argument before pushing the series.
>>>
>>> Thanks,
>>> Laszlo
>>>
>>>>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
>>>>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
>>>>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
>>>>> for the variable name and attributes as well.
>>>>>
>>>>> Cc: Gary Ching-Pang Lin <glin@suse.com>
>>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>>     v2:
>>>>>     - adjust to FD_SIZE_IN_KB
>>>>>     - update commit msg to state 256 KB for the varstore [Jordan]
>>>>>
>>>>>  OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
>>>>>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
>>>>>  OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
>>>>>  3 files changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>>>> index 26b807dde9fa..e0779ddaa426 100644
>>>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>>>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>>>  !endif
>>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>>>  !endif
>>>>>
>>>>>  [PcdsFixedAtBuild]
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>>>> +!endif
>>>>> +!if $(FD_SIZE_IN_KB) == 4096
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>>>> +!endif
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>>>
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>>>
>>>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>>>
>>>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>>> index 41f06a6b6a66..bbe26e2cf452 100644
>>>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>>>  !endif
>>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>>>  !endif
>>>>>
>>>>>  [PcdsFixedAtBuild]
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>>>> +!endif
>>>>> +!if $(FD_SIZE_IN_KB) == 4096
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>>>> +!endif
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>>>
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>>>
>>>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>>>
>>>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>>>>> index 053c84b685c5..ff795815f65f 100644
>>>>> --- a/OvmfPkg/OvmfPkgX64.dsc
>>>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>>>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>>>  !endif
>>>>>  !if $(SMM_REQUIRE) == TRUE
>>>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>>>  !endif
>>>>>
>>>>>  [PcdsFixedAtBuild]
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>>>> +!endif
>>>>> +!if $(FD_SIZE_IN_KB) == 4096
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>>>> +!endif
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>>>
>>>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>>>
>>>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>>>
>>>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>>>> --
>>>>> 2.9.3
>>>>>
>>>>>
>>>
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB
Posted by Laszlo Ersek 7 years, 6 months ago 
On 05/04/17 18:52, Laszlo Ersek wrote:
> On 05/04/17 18:36, Jordan Justen wrote:
>> On 2017-05-03 14:39:46, Laszlo Ersek wrote:
>>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
>>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
>>> Certification Kit sets a 32 KB large non-authenticated variable.
>>
>> According to
>> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
>>
>> "The maximum supported variable size must be at least 64kB"
>>
>> Should we just bump the size to match this? We should be able to make
>> this change later once it is in a test/spec, but for some reason I
>> thought the requirement was already 64k.
> 
> The 32KB requirement comes from the most recent Secure Boot Logo Test. I
> installed both the Windows Server 2008 R2 SP1 test controller and the
> Windows 2016 Server test client just the other day, together with the
> most recent filters, using the following descriptions:
> 
> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM
> 
> Given that this limit can be bumped without breaking compatibility, as
> you say, I'd like to remain frugal with it, same as we were in James's
> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
> Authenticated variables", 2016-03-24).
> 
> I don't understand why the plugfest presentation and the SB Logo Test
> require different limits... But, I'm certain our QE will find out in
> short order once the SB Logo Test catches up with the presentation, and
> I expect I'll submit the corresponding patch soon after.
> 
> I dislike the speculation in this series, but breaking compatibility is
> even worse. (A lot worse, to me at least.) So I consider the varstore
> restructuring the smaller of two wrongs. However, wrt.
> PcdMaxVariableSize, it seems we're not being forced to either of those
> wrongs (i.e., breaking compat or speculation), so we can delay the increase.
> 
>>
>> Aside from this question:
>>
>> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> 
> Thanks a lot!
> 
> I'll await your ACK for the above argument before pushing the series.

Based on the ack I got from Jordan on IRC, I pushed the series. Commit
range 4bf3b994e8b2..bba8dfbec3bb.

Thanks!
Laszlo


>>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
>>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
>>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
>>> for the variable name and attributes as well.
>>>
>>> Cc: Gary Ching-Pang Lin <glin@suse.com>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>>
>>> Notes:
>>>     v2:
>>>     - adjust to FD_SIZE_IN_KB
>>>     - update commit msg to state 256 KB for the varstore [Jordan]
>>>
>>>  OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
>>>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
>>>  OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
>>>  3 files changed, 18 insertions(+)
>>>
>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>> index 26b807dde9fa..e0779ddaa426 100644
>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>  !endif
>>>  !if $(SMM_REQUIRE) == TRUE
>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>  !endif
>>>  
>>>  [PcdsFixedAtBuild]
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>> +!endif
>>> +!if $(FD_SIZE_IN_KB) == 4096
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>> +!endif
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>  
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>  
>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>  
>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> index 41f06a6b6a66..bbe26e2cf452 100644
>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>  !endif
>>>  !if $(SMM_REQUIRE) == TRUE
>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>  !endif
>>>  
>>>  [PcdsFixedAtBuild]
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>> +!endif
>>> +!if $(FD_SIZE_IN_KB) == 4096
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>> +!endif
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>  
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>  
>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>  
>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>>> index 053c84b685c5..ff795815f65f 100644
>>> --- a/OvmfPkg/OvmfPkgX64.dsc
>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>>> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>  !endif
>>>  !if $(SMM_REQUIRE) == TRUE
>>>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>  !endif
>>>  
>>>  [PcdsFixedAtBuild]
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>    gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>> +!endif
>>> +!if $(FD_SIZE_IN_KB) == 4096
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>> +!endif
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>  
>>>    gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>  
>>>    gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>  
>>>    # DEBUG_INIT      0x00000001  // Initialization
>>>    # DEBUG_WARN      0x00000002  // Warnings
>>>    # DEBUG_LOAD      0x00000004  // Load events
>>>    # DEBUG_FS        0x00000008  // EFI File system
>>>    # DEBUG_POOL      0x00000010  // Alloc & Free (pool)
>>>    # DEBUG_PAGE      0x00000020  // Alloc & Free (page)
>>>    # DEBUG_INFO      0x00000040  // Informational debug messages
>>> -- 
>>> 2.9.3
>>>
>>>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.