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



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

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



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

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%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-&amp;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&amp;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&amp;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&amp;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&amp;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
> 


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

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
> >
> 
> 
> 
> 



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

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
> > >
> >
> >
> > 
> >



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

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


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

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
> 
> 
> 
> 



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