Re: [edk2] [PATCH v4 13/41] OvmfPkg: LockBox: -D SMM_REQUIRE excludes our fake lockbox

2015-11-13 Thread Laszlo Ersek 
On 11/11/15 22:25, Jordan Justen wrote:
> On 2015-11-03 13:00:49, Laszlo Ersek wrote:
>> When the user builds OVMF with -D SMM_REQUIRE, our LockBox implementation
>> must not be used, since it doesn't actually protect data in the LockBox
>> from the runtime guest OS. Add an according assert to
>> LockBoxLibInitialize().
>>
>> Furthermore, since our LockBox must not be selected with -D SMM_REQUIRE,
>> it makes sense to set aside memory for it only if -D SMM_REQUIRE is
>> absent. Modify InitializeRamRegions() accordingly.
>>
>> This patch completes the -D SMM_REQUIRE-related tweaking of the special
>> OVMF memory areas.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek 
>> ---
>>  OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf |  3 ++
>>  OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  |  3 ++
>>  OvmfPkg/Library/LockBoxLib/LockBoxLib.c   |  2 +
> 
> It seems like the LockBoxLib changes fit better with either the next
> patch, or in a patch of their own.
> 
> With those move into a new patch, or into patch 14
> 
> 13-14 Reviewed-by: Jordan Justen 
> 
> (+ the possible new patch.)

I split this patch in two:
- OvmfPkg: LockBoxLib: -D SMM_REQUIRE excludes our fake lockbox

  OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf | 3 +++
  OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  | 3 +++
  OvmfPkg/Library/LockBoxLib/LockBoxLib.c   | 2 ++
  3 files changed, 8 insertions(+)

- OvmfPkg: PlatformPei: don't allocate fake lockbox if SMM_REQUIRE

  OvmfPkg/PlatformPei/MemDetect.c | 40 +---
  1 file changed, 21 insertions(+), 19 deletions(-)

I added your R-b to both. (Also to the next patch in the series.)

Thanks!
Laszlo

> 
>>  OvmfPkg/PlatformPei/MemDetect.c   | 40 ++--
>>  4 files changed, 29 insertions(+), 19 deletions(-)
>>
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> index 7203d07..81c893e 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> @@ -42,3 +42,6 @@ [LibraryClasses]
>>  [Pcd]
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> +
>> +[FeaturePcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> index a4d27a5..08973a4 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> @@ -43,3 +43,6 @@ [LibraryClasses]
>>  [Pcd]
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> +
>> +[FeaturePcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> index 89050ac..45481b9 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> @@ -44,6 +44,8 @@ LockBoxLibInitialize (
>>  {
>>UINTN NumEntries;
>>  
>> +  ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
>> +
>>if (PcdGet32 (PcdOvmfLockBoxStorageSize) < sizeof (LOCK_BOX_GLOBAL)) {
>>  return RETURN_UNSUPPORTED;
>>}
>> diff --git a/OvmfPkg/PlatformPei/MemDetect.c 
>> b/OvmfPkg/PlatformPei/MemDetect.c
>> index 1bdc2df..455fcbb 100644
>> --- a/OvmfPkg/PlatformPei/MemDetect.c
>> +++ b/OvmfPkg/PlatformPei/MemDetect.c
>> @@ -407,25 +407,27 @@ InitializeRamRegions (
>>}
>>  
>>if (mBootMode != BOOT_ON_S3_RESUME) {
>> -//
>> -// Reserve the lock box storage area
>> -//
>> -// Since this memory range will be used on S3 resume, it must be
>> -// reserved as ACPI NVS.
>> -//
>> -// If S3 is unsupported, then various drivers might still write to the
>> -// LockBox area. We ought to prevent DXE from serving allocation 
>> requests
>> -// such that they would overlap the LockBox storage.
>> -//
>> -ZeroMem (
>> -  (VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> -  (UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
>> -  );
>> -BuildMemoryAllocationHob (
>> -  (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> -  (UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
>> -  mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>> -  );
>> +if (!FeaturePcdGet (PcdSmmSmramRequire)) {
>> +  //
>> +  // Reserve the lock box storage area
>> +  //
>> +  // Since this memory range will be used on S3 resume, it must be
>> +  // reserved as ACPI NVS.
>> +  //
>> +  // If S3 is unsupported, then various drivers might still write to the
>> +  // LockBox area. We ought to prevent DXE from serving allocation 
>> requests
>> +  // such that they would overlap the LockBox storage.
>> +

Re: [edk2] [PATCH v4 13/41] OvmfPkg: LockBox: -D SMM_REQUIRE excludes our fake lockbox

2015-11-04 Thread Paolo Bonzini 


On 03/11/2015 22:00, Laszlo Ersek wrote:
> When the user builds OVMF with -D SMM_REQUIRE, our LockBox implementation
> must not be used, since it doesn't actually protect data in the LockBox
> from the runtime guest OS. Add an according assert to
> LockBoxLibInitialize().
> 
> Furthermore, since our LockBox must not be selected with -D SMM_REQUIRE,
> it makes sense to set aside memory for it only if -D SMM_REQUIRE is
> absent. Modify InitializeRamRegions() accordingly.
> 
> This patch completes the -D SMM_REQUIRE-related tweaking of the special
> OVMF memory areas.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf |  3 ++
>  OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  |  3 ++
>  OvmfPkg/Library/LockBoxLib/LockBoxLib.c   |  2 +
>  OvmfPkg/PlatformPei/MemDetect.c   | 40 ++--
>  4 files changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf 
> b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> index 7203d07..81c893e 100644
> --- a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
> @@ -42,3 +42,6 @@ [LibraryClasses]
>  [Pcd]
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
> +
> +[FeaturePcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf 
> b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
> index a4d27a5..08973a4 100644
> --- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
> @@ -43,3 +43,6 @@ [LibraryClasses]
>  [Pcd]
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
> +
> +[FeaturePcd]
> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c 
> b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
> index 89050ac..45481b9 100644
> --- a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
> @@ -44,6 +44,8 @@ LockBoxLibInitialize (
>  {
>UINTN NumEntries;
>  
> +  ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
> +
>if (PcdGet32 (PcdOvmfLockBoxStorageSize) < sizeof (LOCK_BOX_GLOBAL)) {
>  return RETURN_UNSUPPORTED;
>}
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index 1bdc2df..455fcbb 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -407,25 +407,27 @@ InitializeRamRegions (
>}
>  
>if (mBootMode != BOOT_ON_S3_RESUME) {
> -//
> -// Reserve the lock box storage area
> -//
> -// Since this memory range will be used on S3 resume, it must be
> -// reserved as ACPI NVS.
> -//
> -// If S3 is unsupported, then various drivers might still write to the
> -// LockBox area. We ought to prevent DXE from serving allocation requests
> -// such that they would overlap the LockBox storage.
> -//
> -ZeroMem (
> -  (VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
> -  (UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
> -  );
> -BuildMemoryAllocationHob (
> -  (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
> -  (UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
> -  mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
> -  );
> +if (!FeaturePcdGet (PcdSmmSmramRequire)) {
> +  //
> +  // Reserve the lock box storage area
> +  //
> +  // Since this memory range will be used on S3 resume, it must be
> +  // reserved as ACPI NVS.
> +  //
> +  // If S3 is unsupported, then various drivers might still write to the
> +  // LockBox area. We ought to prevent DXE from serving allocation 
> requests
> +  // such that they would overlap the LockBox storage.
> +  //
> +  ZeroMem (
> +(VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
> +(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
> +);
> +  BuildMemoryAllocationHob (
> +(EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
> +(UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
> +mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
> +);
> +}
>  
>  if (FeaturePcdGet (PcdSmmSmramRequire)) {
>UINT32 TsegSize;
> 

Reviewed-by: Paolo Bonzini 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 13/41] OvmfPkg: LockBox: -D SMM_REQUIRE excludes our fake lockbox

2015-11-04 Thread Laszlo Ersek 
On 11/04/15 09:52, Paolo Bonzini wrote:
> 
> 
> On 03/11/2015 22:00, Laszlo Ersek wrote:
>> When the user builds OVMF with -D SMM_REQUIRE, our LockBox implementation
>> must not be used, since it doesn't actually protect data in the LockBox
>> from the runtime guest OS. Add an according assert to
>> LockBoxLibInitialize().
>>
>> Furthermore, since our LockBox must not be selected with -D SMM_REQUIRE,
>> it makes sense to set aside memory for it only if -D SMM_REQUIRE is
>> absent. Modify InitializeRamRegions() accordingly.
>>
>> This patch completes the -D SMM_REQUIRE-related tweaking of the special
>> OVMF memory areas.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek 
>> ---
>>  OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf |  3 ++
>>  OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  |  3 ++
>>  OvmfPkg/Library/LockBoxLib/LockBoxLib.c   |  2 +
>>  OvmfPkg/PlatformPei/MemDetect.c   | 40 ++--
>>  4 files changed, 29 insertions(+), 19 deletions(-)
>>
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> index 7203d07..81c893e 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> @@ -42,3 +42,6 @@ [LibraryClasses]
>>  [Pcd]
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> +
>> +[FeaturePcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> index a4d27a5..08973a4 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> @@ -43,3 +43,6 @@ [LibraryClasses]
>>  [Pcd]
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> +
>> +[FeaturePcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> index 89050ac..45481b9 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> @@ -44,6 +44,8 @@ LockBoxLibInitialize (
>>  {
>>UINTN NumEntries;
>>  
>> +  ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
>> +
>>if (PcdGet32 (PcdOvmfLockBoxStorageSize) < sizeof (LOCK_BOX_GLOBAL)) {
>>  return RETURN_UNSUPPORTED;
>>}
>> diff --git a/OvmfPkg/PlatformPei/MemDetect.c 
>> b/OvmfPkg/PlatformPei/MemDetect.c
>> index 1bdc2df..455fcbb 100644
>> --- a/OvmfPkg/PlatformPei/MemDetect.c
>> +++ b/OvmfPkg/PlatformPei/MemDetect.c
>> @@ -407,25 +407,27 @@ InitializeRamRegions (
>>}
>>  
>>if (mBootMode != BOOT_ON_S3_RESUME) {
>> -//
>> -// Reserve the lock box storage area
>> -//
>> -// Since this memory range will be used on S3 resume, it must be
>> -// reserved as ACPI NVS.
>> -//
>> -// If S3 is unsupported, then various drivers might still write to the
>> -// LockBox area. We ought to prevent DXE from serving allocation 
>> requests
>> -// such that they would overlap the LockBox storage.
>> -//
>> -ZeroMem (
>> -  (VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> -  (UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
>> -  );
>> -BuildMemoryAllocationHob (
>> -  (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> -  (UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
>> -  mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>> -  );
>> +if (!FeaturePcdGet (PcdSmmSmramRequire)) {
>> +  //
>> +  // Reserve the lock box storage area
>> +  //
>> +  // Since this memory range will be used on S3 resume, it must be
>> +  // reserved as ACPI NVS.
>> +  //
>> +  // If S3 is unsupported, then various drivers might still write to the
>> +  // LockBox area. We ought to prevent DXE from serving allocation 
>> requests
>> +  // such that they would overlap the LockBox storage.
>> +  //
>> +  ZeroMem (
>> +(VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> +(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
>> +);
>> +  BuildMemoryAllocationHob (
>> +(EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> +(UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
>> +mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>> +);
>> +}
>>  
>>  if (FeaturePcdGet (PcdSmmSmramRequire)) {
>>UINT32 TsegSize;
>>
> 
> Reviewed-by: Paolo Bonzini 
> 

Thanks, picked this up (and the previous one).
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v4 13/41] OvmfPkg: LockBox: -D SMM_REQUIRE excludes our fake lockbox

2015-11-03 Thread Laszlo Ersek 
When the user builds OVMF with -D SMM_REQUIRE, our LockBox implementation
must not be used, since it doesn't actually protect data in the LockBox
from the runtime guest OS. Add an according assert to
LockBoxLibInitialize().

Furthermore, since our LockBox must not be selected with -D SMM_REQUIRE,
it makes sense to set aside memory for it only if -D SMM_REQUIRE is
absent. Modify InitializeRamRegions() accordingly.

This patch completes the -D SMM_REQUIRE-related tweaking of the special
OVMF memory areas.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf |  3 ++
 OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  |  3 ++
 OvmfPkg/Library/LockBoxLib/LockBoxLib.c   |  2 +
 OvmfPkg/PlatformPei/MemDetect.c   | 40 ++--
 4 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf 
b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
index 7203d07..81c893e 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
@@ -42,3 +42,6 @@ [LibraryClasses]
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
+
+[FeaturePcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf 
b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
index a4d27a5..08973a4 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
@@ -43,3 +43,6 @@ [LibraryClasses]
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
+
+[FeaturePcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c 
b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
index 89050ac..45481b9 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
@@ -44,6 +44,8 @@ LockBoxLibInitialize (
 {
   UINTN NumEntries;
 
+  ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
+
   if (PcdGet32 (PcdOvmfLockBoxStorageSize) < sizeof (LOCK_BOX_GLOBAL)) {
 return RETURN_UNSUPPORTED;
   }
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 1bdc2df..455fcbb 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -407,25 +407,27 @@ InitializeRamRegions (
   }
 
   if (mBootMode != BOOT_ON_S3_RESUME) {
-//
-// Reserve the lock box storage area
-//
-// Since this memory range will be used on S3 resume, it must be
-// reserved as ACPI NVS.
-//
-// If S3 is unsupported, then various drivers might still write to the
-// LockBox area. We ought to prevent DXE from serving allocation requests
-// such that they would overlap the LockBox storage.
-//
-ZeroMem (
-  (VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
-  (UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
-  );
-BuildMemoryAllocationHob (
-  (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
-  (UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
-  mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
-  );
+if (!FeaturePcdGet (PcdSmmSmramRequire)) {
+  //
+  // Reserve the lock box storage area
+  //
+  // Since this memory range will be used on S3 resume, it must be
+  // reserved as ACPI NVS.
+  //
+  // If S3 is unsupported, then various drivers might still write to the
+  // LockBox area. We ought to prevent DXE from serving allocation requests
+  // such that they would overlap the LockBox storage.
+  //
+  ZeroMem (
+(VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
+(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
+);
+  BuildMemoryAllocationHob (
+(EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
+(UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
+mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
+);
+}
 
 if (FeaturePcdGet (PcdSmmSmramRequire)) {
   UINT32 TsegSize;
-- 
1.8.3.1


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