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 <ler...@redhat.com>
>> ---
>> 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 <jordan.l.jus...@intel.com>
>
> (+ 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.
>> + //
>> + 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
> _______________________________________________
> 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
