[edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map

Brijesh Singh via groups.io posted 32 patches 4 years, 4 months ago
There is a newer version of this series
[edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Brijesh Singh via groups.io 4 years, 4 months ago
When SEV-SNP is active, the CPUID and Secrets memory range contains the
information that is used during the VM boot. The content need to be persist
across the kexec boot. Mark the memory range as Reserved in the EFI map
so that guest OS or firmware does not use the range as a system RAM.

Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/PlatformPei/PlatformPei.inf |  4 ++++
 OvmfPkg/PlatformPei/Platform.h      |  5 +++++
 OvmfPkg/PlatformPei/AmdSev.c        | 31 +++++++++++++++++++++++++++++
 OvmfPkg/PlatformPei/MemDetect.c     |  2 ++
 4 files changed, 42 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index d048b692f155..0a89e1a0760e 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -112,6 +112,8 @@ [Pcd]
 
 
 [FixedPcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
@@ -122,6 +124,8 @@ [FixedPcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index 8b1d270c2b0b..4169019b4c07 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -102,6 +102,11 @@ AmdSevInitialize (
   VOID
   );
 
+VOID
+SevInitializeRam (
+  VOID
+  );
+
 extern EFI_BOOT_MODE mBootMode;
 
 extern BOOLEAN mS3Supported;
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index b71a4a7304f7..133382407bc5 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -414,3 +414,34 @@ AmdSevInitialize (
   ASSERT_RETURN_ERROR (PcdStatus);
 
 }
+
+/**
+ The function performs SEV specific region initialization.
+
+ **/
+VOID
+SevInitializeRam (
+  VOID
+  )
+{
+  if (MemEncryptSevSnpIsEnabled ()) {
+    //
+    // If SEV-SNP is enabled, reserve the Secrets and CPUID memory area.
+    //
+    // This memory range is given to the PSP by the hypervisor to populate
+    // the information used during the SNP VM boots, and it need to persist
+    // across the kexec boots. Mark it as EfiReservedMemoryType so that
+    // the guest firmware and OS does not use it as a system memory.
+    //
+    BuildMemoryAllocationHob (
+      (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfSnpSecretsBase),
+      (UINT64)(UINTN) PcdGet32 (PcdOvmfSnpSecretsSize),
+      EfiReservedMemoryType
+      );
+    BuildMemoryAllocationHob (
+      (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfCpuidBase),
+      (UINT64)(UINTN) PcdGet32 (PcdOvmfCpuidSize),
+      EfiReservedMemoryType
+      );
+  }
+}
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index d736b85e0d90..058bb394f0df 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -821,6 +821,8 @@ InitializeRamRegions (
 {
   QemuInitializeRam ();
 
+  SevInitializeRam ();
+
   if (mS3Supported && mBootMode != BOOT_ON_S3_RESUME) {
     //
     // This is the memory range that will be used for PEI on S3 resume
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81919): https://edk2.groups.io/g/devel/message/81919
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Gerd Hoffmann 4 years, 3 months ago
On Wed, Oct 13, 2021 at 11:57:11AM -0500, Brijesh Singh wrote:
> When SEV-SNP is active, the CPUID and Secrets memory range contains the
> information that is used during the VM boot. The content need to be persist
> across the kexec boot. Mark the memory range as Reserved in the EFI map
> so that guest OS or firmware does not use the range as a system RAM.

Why is this needed?  Isn't the complete firmware memory tagged as
reserved anyway?

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82003): https://edk2.groups.io/g/devel/message/82003
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Brijesh Singh via groups.io 4 years, 3 months ago
On 10/14/21 1:58 AM, Gerd Hoffmann wrote:
> On Wed, Oct 13, 2021 at 11:57:11AM -0500, Brijesh Singh wrote:
>> When SEV-SNP is active, the CPUID and Secrets memory range contains the
>> information that is used during the VM boot. The content need to be persist
>> across the kexec boot. Mark the memory range as Reserved in the EFI map
>> so that guest OS or firmware does not use the range as a system RAM.
> Why is this needed?  Isn't the complete firmware memory tagged as
> reserved anyway?

PlatformPei detects all the guest memory and marks it as a SYSTEM_RAM
unless its an MMIO or added as reserved in e820 map file. Since the
Secrets and CPUID pages are part of system RAM so we need to explicitly
exclude these region.

thanks


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82056): https://edk2.groups.io/g/devel/message/82056
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Gerd Hoffmann 4 years, 3 months ago
On Thu, Oct 14, 2021 at 05:11:22PM -0500, Brijesh Singh wrote:
> 
> On 10/14/21 1:58 AM, Gerd Hoffmann wrote:
> > On Wed, Oct 13, 2021 at 11:57:11AM -0500, Brijesh Singh wrote:
> >> When SEV-SNP is active, the CPUID and Secrets memory range contains the
> >> information that is used during the VM boot. The content need to be persist
> >> across the kexec boot. Mark the memory range as Reserved in the EFI map
> >> so that guest OS or firmware does not use the range as a system RAM.
> > Why is this needed?  Isn't the complete firmware memory tagged as
> > reserved anyway?
> 
> PlatformPei detects all the guest memory and marks it as a SYSTEM_RAM
> unless its an MMIO or added as reserved in e820 map file. Since the
> Secrets and CPUID pages are part of system RAM so we need to explicitly
> exclude these region.

secret and cpuid are in memfd which in turn is part of the firmware
image mapping which is reserved in the e820 map:

kraxel@rhel8 ~# dmesg | grep -i e820
[ ... some lines snipped ... ]
[    0.000000] BIOS-e820: [mem 0x000000007ff7c000-0x000000007fffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000ffc00000-0x00000000ffffffff] reserved  <= here
[    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000027fffffff] usable

I think they should be covered already ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82089): https://edk2.groups.io/g/devel/message/82089
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Brijesh Singh via groups.io 4 years, 3 months ago
On 10/14/21 10:26 PM, Gerd Hoffmann wrote:
> On Thu, Oct 14, 2021 at 05:11:22PM -0500, Brijesh Singh wrote:
>> On 10/14/21 1:58 AM, Gerd Hoffmann wrote:
>>> On Wed, Oct 13, 2021 at 11:57:11AM -0500, Brijesh Singh wrote:
>>>> When SEV-SNP is active, the CPUID and Secrets memory range contains the
>>>> information that is used during the VM boot. The content need to be persist
>>>> across the kexec boot. Mark the memory range as Reserved in the EFI map
>>>> so that guest OS or firmware does not use the range as a system RAM.
>>> Why is this needed?  Isn't the complete firmware memory tagged as
>>> reserved anyway?
>> PlatformPei detects all the guest memory and marks it as a SYSTEM_RAM
>> unless its an MMIO or added as reserved in e820 map file. Since the
>> Secrets and CPUID pages are part of system RAM so we need to explicitly
>> exclude these region.
> secret and cpuid are in memfd which in turn is part of the firmware
> image mapping which is reserved in the e820 map:
>
> kraxel@rhel8 ~# dmesg | grep -i e820
> [ ... some lines snipped ... ]
> [    0.000000] BIOS-e820: [mem 0x000000007ff7c000-0x000000007fffffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000ffc00000-0x00000000ffffffff] reserved  <= here
> [    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000027fffffff] usable
>
> I think they should be covered already ...

The MEMFD range is outside of the firmware image map,  MEMFD begins with
0x800000 [1] and in my boots I don't see it reserved in e820. Here is
the snippet.

[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009ffff] usable
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x00000000007fffff] usable
[    0.000000] BIOS-e820: [mem 0x0000000000800000-0x0000000000807fff]
ACPI NVS
[    0.000000] BIOS-e820: [mem 0x0000000000808000-0x000000000080afff] usable
[    0.000000] BIOS-e820: [mem 0x000000000080b000-0x000000000080bfff]
ACPI NVS
[    0.000000] BIOS-e820: [mem 0x000000000080c000-0x000000000080ffff] usable
[    0.000000] BIOS-e820: [mem 0x0000000000810000-0x00000000008fffff]
ACPI NVS
[    0.000000] BIOS-e820: [mem 0x0000000000900000-0x000000007f4eefff] usable
[    0.000000] BIOS-e820: [mem 0x000000007f4ef000-0x000000007f76efff]
reserved
[    0.000000] BIOS-e820: [mem 0x000000007f76f000-0x000000007f77efff]
ACPI data
[    0.000000] BIOS-e820: [mem 0x000000007f77f000-0x000000007f7fefff]
ACPI NVS
[    0.000000] BIOS-e820: [mem 0x000000007f7ff000-0x000000007fcfbfff] usable
[    0.000000] BIOS-e820: [mem 0x000000007fcfc000-0x000000007fd7ffff]
reserved
[    0.000000] BIOS-e820: [mem 0x000000007fd80000-0x000000007fffffff]
ACPI NVS
[    0.000000] BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff]
reserved
[    0.000000] BIOS-e820: [mem 0x0000000100000000-0x000000017fffffff] usable

[1]
https://github.com/tianocore/edk2/blob/master/OvmfPkg/OvmfPkgDefines.fdf.inc#L97




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82143): https://edk2.groups.io/g/devel/message/82143
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Gerd Hoffmann 4 years, 3 months ago
  Hi,

> The MEMFD range is outside of the firmware image map,  MEMFD begins with
> 0x800000 [1] and in my boots I don't see it reserved in e820.

Ah, ok.

> Here is the snippet.
> 
> [ ... ]
> [    0.000000] BIOS-e820: [mem 0x0000000000800000-0x0000000000807fff] ACPI NVS
> [    0.000000] BIOS-e820: [mem 0x0000000000808000-0x000000000080afff] usable
> [    0.000000] BIOS-e820: [mem 0x000000000080b000-0x000000000080bfff] ACPI NVS
> [    0.000000] BIOS-e820: [mem 0x000000000080c000-0x000000000080ffff] usable
> [    0.000000] BIOS-e820: [mem 0x0000000000810000-0x00000000008fffff] ACPI NVS
> [ ... ]

Hmm.  Confused.  memfd size is 0xD00000, so should the block from 800000
to 8cffff be reserved?  Why does it end at 8fffff instead?

The first hole is this:

    0x008000|0x001000
    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
    0x009000|0x002000
    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize

The second hole is this (git master) ...

    0x00C000|0x001000
    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize

And IIRC the cpuid + secrets pages are added there.

So, yes, they must be reserved indeed.  What about the other pages?
Shouldn't they be reserved too?  Or will they not be used any more
at runtime?

thanks,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82211): https://edk2.groups.io/g/devel/message/82211
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Brijesh Singh via groups.io 4 years, 3 months ago
On 10/18/21 1:01 AM, Gerd Hoffmann wrote:
>   Hi,
>
>> The MEMFD range is outside of the firmware image map,  MEMFD begins with
>> 0x800000 [1] and in my boots I don't see it reserved in e820.
> Ah, ok.
>
>> Here is the snippet.
>>
>> [ ... ]
>> [    0.000000] BIOS-e820: [mem 0x0000000000800000-0x0000000000807fff] ACPI NVS
>> [    0.000000] BIOS-e820: [mem 0x0000000000808000-0x000000000080afff] usable
>> [    0.000000] BIOS-e820: [mem 0x000000000080b000-0x000000000080bfff] ACPI NVS
>> [    0.000000] BIOS-e820: [mem 0x000000000080c000-0x000000000080ffff] usable
>> [    0.000000] BIOS-e820: [mem 0x0000000000810000-0x00000000008fffff] ACPI NVS
>> [ ... ]
> Hmm.  Confused.  memfd size is 0xD00000, so should the block from 800000
> to 8cffff be reserved?  Why does it end at 8fffff instead?

There is no strong reason for block all of the MEMFD. What I see in the
current code is some selective pages gets marked reserved or other
memory type. As system boots some pages may get released as a system RAM.


> The first hole is this:
>
>     0x008000|0x001000
>     gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
>     0x009000|0x002000
>     gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
>
> The second hole is this (git master) ...
>
>     0x00C000|0x001000
>     gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
>
> And IIRC the cpuid + secrets pages are added there.
>
> So, yes, they must be reserved indeed.  What about the other pages?
> Shouldn't they be reserved too?  Or will they not be used any more
> at runtime?

As I indicated above, the other part of the code (such MemDetect.c)
makes the pages reserved as system boot. Some page can be may not be
used at all during the runtime and thus gets released.

thanks




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82357): https://edk2.groups.io/g/devel/message/82357
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 30/32] OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
Posted by Gerd Hoffmann 4 years, 3 months ago
  Hi,

> >> [ ... ]
> >> [    0.000000] BIOS-e820: [mem 0x0000000000800000-0x0000000000807fff] ACPI NVS
> >> [    0.000000] BIOS-e820: [mem 0x0000000000808000-0x000000000080afff] usable
> >> [    0.000000] BIOS-e820: [mem 0x000000000080b000-0x000000000080bfff] ACPI NVS
> >> [    0.000000] BIOS-e820: [mem 0x000000000080c000-0x000000000080ffff] usable
> >> [    0.000000] BIOS-e820: [mem 0x0000000000810000-0x00000000008fffff] ACPI NVS
> >> [ ... ]
> > Hmm.  Confused.  memfd size is 0xD00000, so should the block from 800000
> > to 8cffff be reserved?  Why does it end at 8fffff instead?
> 
> There is no strong reason for block all of the MEMFD. What I see in the
> current code is some selective pages gets marked reserved or other
> memory type. As system boots some pages may get released as a system RAM.

Ok, lets follow the existing practice then.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82407): https://edk2.groups.io/g/devel/message/82407
Mute This Topic: https://groups.io/mt/86292921/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-