[RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP) support


Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

Additional resources
---------------------
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf

APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)

The complete source is available at
https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-4

GHCB spec:
https://developer.amd.com/wp-content/resources/56421.pdf

SEV-SNP firmware specification:
https://www.amd.com/system/files/TechDocs/56860.pdf

Brijesh Singh (26):
OvmfPkg/ResetVector: move SEV specific code in a separate file
OvmfPkg/ResetVector: add the macro to invoke MSR protocol based
VMGEXIT
OvmfPkg/ResetVector: add the macro to request guest termination
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 24 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 14 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 8 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 ++
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 +++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/SecMain.c | 111 +++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 275 +++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 27 ++
.../Ia32/{PageTables64.asm => AmdSev.asm} | 415 +++++++++---------
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 404 +----------------
OvmfPkg/ResetVector/ResetVector.nasmb | 7 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++
48 files changed, 1978 insertions(+), 630 deletions(-)
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
copy OvmfPkg/ResetVector/Ia32/{PageTables64.asm => AmdSev.asm} (67%)

--
2.17.1


Yao, Jiewen
 

Hi Brijesh
I reviewed the patch set. I have some basic questions.
Please help me understand before I post my comment

If a platform supports SEV-SNP, can we assume SEV-ES is supported?
Or is it a valid case that SecSnp==YES, SevEs==NO?

I am trying to understand how many cases we need support.
I think we want to support below:
+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 0 | 0 |
| 1 | 0 | 0 |
| 1 | 1 | 0 |
| 1 | 1 | 1 |
+------------------------+


Any other combination we need support? Such as below:
+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 1 | 0 |
| 0 | 0 | 1 |
| 0 | 1 | 1 |
| 1 | 0 | 1 |
+------------------------+


Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Tuesday, June 29, 2021 1:42 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Laszlo Ersek
<lersek@redhat.com>; Erdem Aktas <erdemaktas@google.com>; Dong, Eric
<eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1
<rahul1.kumar@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
Singh <brijesh.singh@amd.com>
Subject: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP)
support

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

Additional resources
---------------------
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-
isolation-with-integrity-protection-and-more.pdf

APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)

The complete source is available at
https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-4

GHCB spec:
https://developer.amd.com/wp-content/resources/56421.pdf

SEV-SNP firmware specification:
https://www.amd.com/system/files/TechDocs/56860.pdf

Brijesh Singh (26):
OvmfPkg/ResetVector: move SEV specific code in a separate file
OvmfPkg/ResetVector: add the macro to invoke MSR protocol based
VMGEXIT
OvmfPkg/ResetVector: add the macro to request guest termination
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 24 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 14 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 8 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 ++
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 +++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/SecMain.c | 111 +++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 275 +++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 27 ++
.../Ia32/{PageTables64.asm => AmdSev.asm} | 415 +++++++++---------
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 404 +----------------
OvmfPkg/ResetVector/ResetVector.nasmb | 7 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++
48 files changed, 1978 insertions(+), 630 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
copy OvmfPkg/ResetVector/Ia32/{PageTables64.asm => AmdSev.asm} (67%)

--
2.17.1


Brijesh Singh
 

Hi Yao Jiewen,

On 7/28/21 3:16 AM, Yao, Jiewen wrote:
Hi Brijesh
I reviewed the patch set. I have some basic questions.
Please help me understand before I post my comment
If a platform supports SEV-SNP, can we assume SEV-ES is supported?
The SEV-SNP depends on SEV and SEV-ES support.

The SEV-ES depends on the SEV support.


Or is it a valid case that SecSnp==YES, SevEs==NO?
Nope.

I am trying to understand how many cases we need support.
I think we want to support below:
+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 0 | 0 |
| 1 | 0 | 0 |
| 1 | 1 | 0 |
| 1 | 1 | 1 |
+------------------------+
Yes, the above looks correct.

Any other combination we need support? Such as below:
The below cases are not applicable.

+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 1 | 0 |
| 0 | 0 | 1 |
| 0 | 1 | 1 |
| 1 | 0 | 1 |
+------------------------+
Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Tuesday, June 29, 2021 1:42 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Laszlo Ersek
<lersek@redhat.com>; Erdem Aktas <erdemaktas@google.com>; Dong, Eric
<eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1
<rahul1.kumar@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
Singh <brijesh.singh@amd.com>
Subject: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP)
support

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=BqKBPTm4RQFXsekHTH2ktc2YmZMwazn9bZy8G8%2BWSTA%3D&amp;reserved=0

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7p5Ap%2FHMiSXgxxMI35SYWcZaUcx5VjNt1wnpV9kbT6c%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=h5ZrpTSwjBVhw9Bdh%2FvcZVGK%2BaxgHre42B8evZuTkKQ%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fovmf%2Ftree%2Fsev-snp-rfc-4&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=MwXzgykRRjT0QCp%2B77zJG1nH44478OzH4HtCQJbpHLc%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fwp-content%2Fresources%2F56421.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=jU2LPonK9rQUjKQsRijBNU6uk1eN%2B7uuqYiXKvz7r4w%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6xiPHnAMKyJy6b%2B9trUlukxKYApH%2FncYM8Qg0r9%2BWlA%3D&amp;reserved=0

Brijesh Singh (26):
OvmfPkg/ResetVector: move SEV specific code in a separate file
OvmfPkg/ResetVector: add the macro to invoke MSR protocol based
VMGEXIT
OvmfPkg/ResetVector: add the macro to request guest termination
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 24 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 14 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 8 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 ++
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 +++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/SecMain.c | 111 +++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 275 +++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 27 ++
.../Ia32/{PageTables64.asm => AmdSev.asm} | 415 +++++++++---------
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 404 +----------------
OvmfPkg/ResetVector/ResetVector.nasmb | 7 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++
48 files changed, 1978 insertions(+), 630 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
copy OvmfPkg/ResetVector/Ia32/{PageTables64.asm => AmdSev.asm} (67%)

--
2.17.1


Yao, Jiewen
 

Sounds good. Thank you to confirm that.

I will send my feedback.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Wednesday, July 28, 2021 11:22 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Cc: brijesh.singh@amd.com; James Bottomley <jejb@linux.ibm.com>; Xu, Min M
<min.m.xu@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Laszlo Ersek <lersek@redhat.com>; Erdem Aktas
<erdemaktas@google.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
<ray.ni@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Kinney, Michael
D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
Zhiguang <zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>
Subject: Re: [edk2-devel] [RFC PATCH v4 00/27] Add AMD Secure Nested Paging
(SEV-SNP) support

Hi Yao Jiewen,

On 7/28/21 3:16 AM, Yao, Jiewen wrote:
Hi Brijesh
I reviewed the patch set. I have some basic questions.
Please help me understand before I post my comment

If a platform supports SEV-SNP, can we assume SEV-ES is supported?
The SEV-SNP depends on SEV and SEV-ES support.

The SEV-ES depends on the SEV support.


Or is it a valid case that SecSnp==YES, SevEs==NO?
Nope.


I am trying to understand how many cases we need support.
I think we want to support below:
+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 0 | 0 |
| 1 | 0 | 0 |
| 1 | 1 | 0 |
| 1 | 1 | 1 |
+------------------------+
Yes, the above looks correct.


Any other combination we need support? Such as below:
The below cases are not applicable.

+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 1 | 0 |
| 0 | 0 | 1 |
| 0 | 1 | 1 |
| 1 | 0 | 1 |
+------------------------+


Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Tuesday, June 29, 2021 1:42 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M
<min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Laszlo Ersek
<lersek@redhat.com>; Erdem Aktas <erdemaktas@google.com>; Dong, Eric
<eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1
<rahul1.kumar@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
Singh <brijesh.singh@amd.com>
Subject: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP)
support

BZ:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.
singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe48
84e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%
7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C1000&amp;sdata=BqKBPTm4RQFXsekHTH2ktc2YmZMwazn
9bZy8G8%2BWSTA%3D&amp;reserved=0

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-
SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the
available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is
validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-
&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a
808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6376
30571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7p5Ap
%2FHMiSXgxxMI35SYWcZaUcx5VjNt1wnpV9kbT6c%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&amp;data=04%7C01%7
Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8
961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnk
nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
aWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=h5ZrpTSwjBVhw9Bdh%2FvcZVGK
%2BaxgHre42B8evZuTkKQ%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
om%2FAMDESE%2Fovmf%2Ftree%2Fsev-snp-rfc-
4&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53
a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637
630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=MwXz
gykRRjT0QCp%2B77zJG1nH44478OzH4HtCQJbpHLc%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelop
er.amd.com%2Fwp-
content%2Fresources%2F56421.pdf&amp;data=04%7C01%7Cbrijesh.singh%40a
md.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11
a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZ
sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
%3D%7C1000&amp;sdata=jU2LPonK9rQUjKQsRijBNU6uk1eN%2B7uuqYiXKvz7r4
w%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&amp;data=04%7C01%7
Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8
961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnk
nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
aWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6xiPHnAMKyJy6b%2B9trUlukxKYA
pH%2FncYM8Qg0r9%2BWlA%3D&amp;reserved=0

Brijesh Singh (26):
OvmfPkg/ResetVector: move SEV specific code in a separate file
OvmfPkg/ResetVector: add the macro to invoke MSR protocol based
VMGEXIT
OvmfPkg/ResetVector: add the macro to request guest termination
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 24 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 14 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 8 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 ++
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 +++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/SecMain.c | 111 +++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 275 +++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 27 ++
.../Ia32/{PageTables64.asm => AmdSev.asm} | 415 +++++++++---------
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 404 +----------------
OvmfPkg/ResetVector/ResetVector.nasmb | 7 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++
48 files changed, 1978 insertions(+), 630 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
copy OvmfPkg/ResetVector/Ia32/{PageTables64.asm => AmdSev.asm} (67%)

--
2.17.1



Yao, Jiewen
 

Hi Brijesh
Thanks for the patient.
Most of my comment focus on the *common* part, and *interface* between SEV and common code.
I will leave you to decide the detailed SEV specific implementation.


Patch-04:
Can we use consistent naming conversion?
We have PcdOvmfSecGhcbPageTableBase, PcdOvmfSecGhcbBase, PcdSevLaunchSecretBase. Now we are adding PcdOvmfSnpSecretsBase.
Can we change PcdOvmfSnpSecretsBase to PcdSevSnpSecretsBase?
Or we change PcdSevLaunchSecretBase to PcdOvmfSevLaunchSecretBase?

Patch-05:
Ditto. Naming convention.

Patch-06:
I have recommendation to Min, to separate SEV stuff to a standalone file from ResetVectorVtf0.asm.
Intel can add TDX stuff to a standalone file, and make it included by ResetVectorVtf0.asm.

I am not sure if you want to do it, or you leave Min to do it.

Patch-07:
Same naming convention issue. See #04 and #05.

Patch-08:
I hope we can move all below code to AmdSev.asm, such as PostPageTableHookSev().
Then the PageTable64.asm can be SEV/TDX agnostic.

I am not sure if you want to do it, or you leave Min to do it.

==============
;
; Clear the encryption bit from the GHCB entry
;
mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0

mov ecx, GHCB_SIZE / 4
xor eax, eax
clearGhcbMemoryLoop:
mov dword[ecx * 4 + GHCB_BASE - 4], eax
loop clearGhcbMemoryLoop

;
; The page table built above cleared the memory encryption mask from the
; GHCB_BASE (aka made it shared). When SEV-SNP is enabled, to maintain
; the security guarantees, the page state transition from private to
; shared must go through the page invalidation steps. Invalidate the
; memory range before loading the page table below.
;
; NOTE: the invalidation must happen after zeroing the GHCB memory. This
; is because, in the 32-bit mode all the access are considered private.
; The invalidation before the zero'ing will cause a #VC.
;
OneTimeCall InvalidateGHCBPage
==============

Patch-10:
I am not UEFI CPU package maintainer. But I do have a little concern to add more PcdXxxIsEnable style PCD, especially when they are mutual exclusive (like TDX v.s SEV).
If we follow this pattern, we will have PcdSevEsIsEnabled, PcdSevSnpIsEnabled, PcdSevFutureIsEnabled, PcdTdxIsEnabled, PcdTdxFutureIsEnabled, ... that will be an endless list.

If possible, I suggest define one PcdConfidentialComputingType - indicate Legacy, SEV, TDX.

Patch-12:
Can we move all SEV stuff to a standalone file, such as AmdSev.c?

I am not sure if you want to do it, or you leave Min to do it.

Patch-18:
If we have a standalone AmdSev.c (#12), then we can move the function to that file, and only leave a hook call to SEV.

Patch-23:
This is UEFI CPU package update. I am thinking if we can follow same patter to move all SEV stuff to a standalone file, such as AmdSev.c, AmdSev.asm.
In the future, we may add TDX stuff as well.

Patch-26:
Same comment as #23.

Patch-27:
Can we move that function to a standalone AmdSev.c ?

Patch-28:
Would you please describe more on what is ConfidentialComputingBlob ?
Is that generic concept? Or SEV specific thing?
Who is consumer?
What is difference between ConfidentialComputingSecret and ConfidentialComputingBlob ? When to use which?

I can understand how TDX use ConfidentialComputingSecret, but how do you expect TDX use ConfidentialComputingBlob (if it is a generic concept) ?

Thank you
Yao Jiewen

-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, July 29, 2021 12:38 AM
To: devel@edk2.groups.io; brijesh.singh@amd.com
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
Tom Lendacky <thomas.lendacky@amd.com>; Justen, Jordan L
<jordan.l.justen@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
Laszlo Ersek <lersek@redhat.com>; Erdem Aktas <erdemaktas@google.com>;
Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1
<Rahul1.Kumar@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<Zhiguang.Liu@intel.com>; Michael Roth <Michael.Roth@amd.com>
Subject: RE: [edk2-devel] [RFC PATCH v4 00/27] Add AMD Secure Nested Paging
(SEV-SNP) support

Sounds good. Thank you to confirm that.

I will send my feedback.



-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Wednesday, July 28, 2021 11:22 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Cc: brijesh.singh@amd.com; James Bottomley <jejb@linux.ibm.com>; Xu, Min
M
<min.m.xu@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Laszlo Ersek <lersek@redhat.com>; Erdem
Aktas
<erdemaktas@google.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
<ray.ni@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Kinney,
Michael
D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
Liu,
Zhiguang <zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>
Subject: Re: [edk2-devel] [RFC PATCH v4 00/27] Add AMD Secure Nested
Paging
(SEV-SNP) support

Hi Yao Jiewen,

On 7/28/21 3:16 AM, Yao, Jiewen wrote:
Hi Brijesh
I reviewed the patch set. I have some basic questions.
Please help me understand before I post my comment

If a platform supports SEV-SNP, can we assume SEV-ES is supported?
The SEV-SNP depends on SEV and SEV-ES support.

The SEV-ES depends on the SEV support.


Or is it a valid case that SecSnp==YES, SevEs==NO?
Nope.


I am trying to understand how many cases we need support.
I think we want to support below:
+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 0 | 0 |
| 1 | 0 | 0 |
| 1 | 1 | 0 |
| 1 | 1 | 1 |
+------------------------+
Yes, the above looks correct.


Any other combination we need support? Such as below:
The below cases are not applicable.

+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 1 | 0 |
| 0 | 0 | 1 |
| 0 | 1 | 1 |
| 1 | 0 | 1 |
+------------------------+


Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Tuesday, June 29, 2021 1:42 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M
<min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L
<jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Laszlo Ersek
<lersek@redhat.com>; Erdem Aktas <erdemaktas@google.com>; Dong,
Eric
<eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1
<rahul1.kumar@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>;
Brijesh
Singh <brijesh.singh@amd.com>
Subject: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP)
support

BZ:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.
singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe48
84e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%
7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C1000&amp;sdata=BqKBPTm4RQFXsekHTH2ktc2YmZMwazn
9bZy8G8%2BWSTA%3D&amp;reserved=0

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated
memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-
SNP
VMs, it does not cover all the security enhancement introduced by the SEV-
SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-
SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new
PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or
unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to
communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the
available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is
validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-
&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a
808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6376
30571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7p5Ap
%2FHMiSXgxxMI35SYWcZaUcx5VjNt1wnpV9kbT6c%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&amp;data=04%7C01%7
Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8
961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnk
nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
aWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=h5ZrpTSwjBVhw9Bdh%2FvcZVGK
%2BaxgHre42B8evZuTkKQ%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
om%2FAMDESE%2Fovmf%2Ftree%2Fsev-snp-rfc-
4&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53
a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637
630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=MwXz
gykRRjT0QCp%2B77zJG1nH44478OzH4HtCQJbpHLc%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelop
er.amd.com%2Fwp-
content%2Fresources%2F56421.pdf&amp;data=04%7C01%7Cbrijesh.singh%40a
md.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11
a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZ
sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
%3D%7C1000&amp;sdata=jU2LPonK9rQUjKQsRijBNU6uk1eN%2B7uuqYiXKvz7
r4
w%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&amp;data=04%7C01%7
Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8
961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnk
nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
aWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6xiPHnAMKyJy6b%2B9trUlukxKYA
pH%2FncYM8Qg0r9%2BWlA%3D&amp;reserved=0

Brijesh Singh (26):
OvmfPkg/ResetVector: move SEV specific code in a separate file
OvmfPkg/ResetVector: add the macro to invoke MSR protocol based
VMGEXIT
OvmfPkg/ResetVector: add the macro to request guest termination
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in
PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI
map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 24 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 14 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 8 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 ++
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 +++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/SecMain.c | 111 +++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 275 +++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 27 ++
.../Ia32/{PageTables64.asm => AmdSev.asm} | 415 +++++++++---------
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 404 +----------------
OvmfPkg/ResetVector/ResetVector.nasmb | 7 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++
48 files changed, 1978 insertions(+), 630 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
copy OvmfPkg/ResetVector/Ia32/{PageTables64.asm => AmdSev.asm}
(67%)

--
2.17.1



Brijesh Singh
 

On 7/28/21 9:22 PM, Yao, Jiewen wrote:
Hi Brijesh
Thanks for the patient.
Most of my comment focus on the *common* part, and *interface* between SEV and common code.
I will leave you to decide the detailed SEV specific implementation.
Thank you Jiewen for your feedback. I will try to address the comments in next version.

Patch-04:
Can we use consistent naming conversion?
We have PcdOvmfSecGhcbPageTableBase, PcdOvmfSecGhcbBase, PcdSevLaunchSecretBase. Now we are adding PcdOvmfSnpSecretsBase.
Can we change PcdOvmfSnpSecretsBase to PcdSevSnpSecretsBase?
Or we change PcdSevLaunchSecretBase to PcdOvmfSevLaunchSecretBase?
I don't know why we choose "Ovmf" from the LaunchSecretsBase PCD. I thought PCD's specific the Uefi typically contains the Ovmf name. Maybe we can fix the LaunchSecretsBase to match with the name. I will do that as a separate patch.

Patch-05:
Ditto. Naming convention.
Patch-06:
I have recommendation to Min, to separate SEV stuff to a standalone file from ResetVectorVtf0.asm.
Intel can add TDX stuff to a standalone file, and make it included by ResetVectorVtf0.asm.
I am not sure if you want to do it, or you leave Min to do it.
For the SEV stuff, I will do it myself so that I can test it as well :)

Patch-07:
Same naming convention issue. See #04 and #05.
Patch-08:
I hope we can move all below code to AmdSev.asm, such as PostPageTableHookSev().
Then the PageTable64.asm can be SEV/TDX agnostic.
I am not sure if you want to do it, or you leave Min to do it.
==============
;
; Clear the encryption bit from the GHCB entry
;
mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
mov ecx, GHCB_SIZE / 4
xor eax, eax
clearGhcbMemoryLoop:
mov dword[ecx * 4 + GHCB_BASE - 4], eax
loop clearGhcbMemoryLoop
;
; The page table built above cleared the memory encryption mask from the
; GHCB_BASE (aka made it shared). When SEV-SNP is enabled, to maintain
; the security guarantees, the page state transition from private to
; shared must go through the page invalidation steps. Invalidate the
; memory range before loading the page table below.
;
; NOTE: the invalidation must happen after zeroing the GHCB memory. This
; is because, in the 32-bit mode all the access are considered private.
; The invalidation before the zero'ing will cause a #VC.
;
OneTimeCall InvalidateGHCBPage
==============
I will try to see if I can move that out as well.

Patch-10:
I am not UEFI CPU package maintainer. But I do have a little concern to add more PcdXxxIsEnable style PCD, especially when they are mutual exclusive (like TDX v.s SEV).
If we follow this pattern, we will have PcdSevEsIsEnabled, PcdSevSnpIsEnabled, PcdSevFutureIsEnabled, PcdTdxIsEnabled, PcdTdxFutureIsEnabled, ... that will be an endless list.
If possible, I suggest define one PcdConfidentialComputingType - indicate Legacy, SEV, TDX.
There are certain things which are applicable to SEV-ES and not for the SEV-SNP and vice versa. I am not oppose to create a generic helper e.g

enum {
AmdSev,
AmdSevEs,
AmdSevSnp,
IntelTdx,
IntelSgx,
..
..
};

bool EncryptedGuestFeatureEnabled(enum type);

But I think some of this can be done later as well.

Patch-12:
Can we move all SEV stuff to a standalone file, such as AmdSev.c?
I am not sure if you want to do it, or you leave Min to do it.
Yes, I can do it.

Patch-18:
If we have a standalone AmdSev.c (#12), then we can move the function to that file, and only leave a hook call to SEV.
I will try to consolidate it in AmdSev.c

Patch-23:
This is UEFI CPU package update. I am thinking if we can follow same patter to move all SEV stuff to a standalone file, such as AmdSev.c, AmdSev.asm.
In the future, we may add TDX stuff as well.
Patch-26:
Same comment as #23.
Patch-27:
Can we move that function to a standalone AmdSev.c ?
Patch-28:
Would you please describe more on what is ConfidentialComputingBlob ?
While launching the SEV-SNP guests, the hypervisor may need to provide
some additional information during the guest boot. When booting under the EFI based BIOS, the EFI configuration table contains an entry for the confidential computing blob that contains the required information. The Linux kernel will lookup for this EFI table during the boot to locate the secrets and cpuid page.


Is that generic concept? Or SEV specific thing?
Its designed as a generic and the current only SEV-SNP provides it.
Who is consumer?
Any guest kernel (window or Linux)

What is difference between ConfidentialComputingSecret and ConfidentialComputingBlob ? When to use which?
The confidentialComputingSecrets contains the secrets keys where the CCBlob contains the information which maybe used during the boot.

You can see some more about it on my kernel patches:

https://lore.kernel.org/lkml/20210707181506.30489-26-brijesh.singh@amd.com/

I can understand how TDX use ConfidentialComputingSecret, but how do you expect TDX use ConfidentialComputingBlob (if it is a generic concept) ?
I think in the case of TDX , the information needed during the boot is provided through the ACPI tables but in SEV-SNP those are provided throught the CCBlob. In the contianer environement there will be no EFI so in that case the Blob will be passed to the boot loader setup data. If required then TDX can use it to pass the boot information.

thanks


Yao, Jiewen
 

See my feedback below.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Tuesday, August 3, 2021 11:01 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Cc: brijesh.singh@amd.com; James Bottomley <jejb@linux.ibm.com>; Xu, Min M
<min.m.xu@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Laszlo Ersek <lersek@redhat.com>; Erdem Aktas
<erdemaktas@google.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
<ray.ni@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Kinney, Michael
D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
Zhiguang <zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>
Subject: Re: [edk2-devel] [RFC PATCH v4 00/27] Add AMD Secure Nested Paging
(SEV-SNP) support



On 7/28/21 9:22 PM, Yao, Jiewen wrote:
Hi Brijesh
Thanks for the patient.
Most of my comment focus on the *common* part, and *interface* between
SEV and common code.
I will leave you to decide the detailed SEV specific implementation.
Thank you Jiewen for your feedback. I will try to address the comments
in next version.


Patch-04:
Can we use consistent naming conversion?
We have PcdOvmfSecGhcbPageTableBase, PcdOvmfSecGhcbBase,
PcdSevLaunchSecretBase. Now we are adding PcdOvmfSnpSecretsBase.
Can we change PcdOvmfSnpSecretsBase to PcdSevSnpSecretsBase?
Or we change PcdSevLaunchSecretBase to PcdOvmfSevLaunchSecretBase?
I don't know why we choose "Ovmf" from the LaunchSecretsBase PCD. I
thought PCD's specific the Uefi typically contains the Ovmf name. Maybe
we can fix the LaunchSecretsBase to match with the name. I will do that
as a separate patch.


Patch-05:
Ditto. Naming convention.

Patch-06:
I have recommendation to Min, to separate SEV stuff to a standalone file from
ResetVectorVtf0.asm.
Intel can add TDX stuff to a standalone file, and make it included by
ResetVectorVtf0.asm.

I am not sure if you want to do it, or you leave Min to do it.
For the SEV stuff, I will do it myself so that I can test it as well :)

Patch-07:
Same naming convention issue. See #04 and #05.

Patch-08:
I hope we can move all below code to AmdSev.asm, such as
PostPageTableHookSev().
Then the PageTable64.asm can be SEV/TDX agnostic.

I am not sure if you want to do it, or you leave Min to do it.

==============
;
; Clear the encryption bit from the GHCB entry
;
mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0

mov ecx, GHCB_SIZE / 4
xor eax, eax
clearGhcbMemoryLoop:
mov dword[ecx * 4 + GHCB_BASE - 4], eax
loop clearGhcbMemoryLoop

;
; The page table built above cleared the memory encryption mask from the
; GHCB_BASE (aka made it shared). When SEV-SNP is enabled, to maintain
; the security guarantees, the page state transition from private to
; shared must go through the page invalidation steps. Invalidate the
; memory range before loading the page table below.
;
; NOTE: the invalidation must happen after zeroing the GHCB memory. This
; is because, in the 32-bit mode all the access are considered private.
; The invalidation before the zero'ing will cause a #VC.
;
OneTimeCall InvalidateGHCBPage
==============
I will try to see if I can move that out as well.

Patch-10:
I am not UEFI CPU package maintainer. But I do have a little concern to add
more PcdXxxIsEnable style PCD, especially when they are mutual exclusive (like
TDX v.s SEV).
If we follow this pattern, we will have PcdSevEsIsEnabled, PcdSevSnpIsEnabled,
PcdSevFutureIsEnabled, PcdTdxIsEnabled, PcdTdxFutureIsEnabled, ... that will be
an endless list.

If possible, I suggest define one PcdConfidentialComputingType - indicate
Legacy, SEV, TDX.
There are certain things which are applicable to SEV-ES and not for the
SEV-SNP and vice versa. I am not oppose to create a generic helper e.g

enum {
AmdSev,
AmdSevEs,
AmdSevSnp,
IntelTdx,
IntelSgx,
..
..
};

bool EncryptedGuestFeatureEnabled(enum type);

But I think some of this can be done later as well.

Patch-12:
Can we move all SEV stuff to a standalone file, such as AmdSev.c?

I am not sure if you want to do it, or you leave Min to do it.
Yes, I can do it.

Patch-18:
If we have a standalone AmdSev.c (#12), then we can move the function to
that file, and only leave a hook call to SEV.
I will try to consolidate it in AmdSev.c

Patch-23:
This is UEFI CPU package update. I am thinking if we can follow same patter to
move all SEV stuff to a standalone file, such as AmdSev.c, AmdSev.asm.
In the future, we may add TDX stuff as well.

Patch-26:
Same comment as #23.

Patch-27:
Can we move that function to a standalone AmdSev.c ?

Patch-28:
Would you please describe more on what is ConfidentialComputingBlob ?
While launching the SEV-SNP guests, the hypervisor may need to provide
some additional information during the guest boot. When booting under
the EFI based BIOS, the EFI configuration table contains an entry for
the confidential computing blob that contains the required information.
The Linux kernel will lookup for this EFI table during the boot to
locate the secrets and cpuid page.


Is that generic concept? Or SEV specific thing?
Its designed as a generic and the current only SEV-SNP provides it.
Who is consumer?
Any guest kernel (window or Linux)

What is difference between ConfidentialComputingSecret and
ConfidentialComputingBlob ? When to use which?
The confidentialComputingSecrets contains the secrets keys where the
CCBlob contains the information which maybe used during the boot.

You can see some more about it on my kernel patches:

https://lore.kernel.org/lkml/20210707181506.30489-26-
brijesh.singh@amd.com/

I can understand how TDX use ConfidentialComputingSecret, but how do you
expect TDX use ConfidentialComputingBlob (if it is a generic concept) ?

I think in the case of TDX , the information needed during the boot is
provided through the ACPI tables but in SEV-SNP those are provided
throught the CCBlob. In the contianer environement there will be no EFI
so in that case the Blob will be passed to the boot loader setup data.
If required then TDX can use it to pass the boot information.
[Jiewen] Got it. I treat it as a generic way to pass information from Guest FW to guest OS.
If so, I have concern on having a generic name - CC_BLOB, but only includes SEV specific data structure there.

I offer two possible alternatives, and I open on other options.
A) define it as SEV_BLOB. Don’t use generic name. As such, the consume knows this blob is for SEV.
B) define it TYPE-LENGTH-VALUE list in CC_BLOB. As such, the consume can pass the TYPE to know this blob is for SEV.

If possible, I prefer to split this big patch series to smaller one, especially the CC_BLOB.
I think we may have more discussion on how to support that.
But I don’t want to block your other work such as creating standalone SEV file and add SEV-SNP stuff there.






thanks