[edk2-rfc] [edk2-devel] RFC: Common Design Proposal on Confidential Computing Support in OVMF

[edk2-rfc] [edk2-devel] RFC: Common Design Proposal on Confidential Computing Support in OVMF

[Yao, Jiewen]

[-- Attachment #1: Type: text/plain, Size: 7851 bytes --]

Hi
I would like to raise the topic on a confidential computing support in OVMF.

The main target is AMD SEV feature and Intel TDX feature in OVMF package.

The goal is to create a guidance for our future confidential computing work and to better support review and maintenance.


[Background]

AMD is adding AMD Secure Encrypted Virtualization (SEV), SEV-Encrypted State (SEV-ES), SEV-Secure Nested Paging (SEV-SNP) features to OVMF package. (https://developer.amd.com/sev/)
Intel is adding Intel Trust Domain Extensions (TDX) features to OVMF package. (https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html)

Both of them support confidential computing use case.

ARM is creating Realm Management Extension (RME). It might be considered in the future. (https://www.arm.com/why-arm/architecture/security-features/arm-confidential-compute-architecture)

So we need a good Confidential Computing infrastructure for EDKII.


[Problem Statement]

1) Current OVMF package integrated some AMD SEV features. But not all features.
Some basic SEV features are in OvmfPkg and enabled as default build. Some advanced SEV features are in OvmfPkg/AmdSev and only enable in AmdSev build.
However, the criteria is NOT clear.

It also brings problem when we want to add more TDX stuff. Where we should go?
For example, I feel PlatformBootManagerLibGrub should be in OvmfPkg/AmdSev. Why it is not there?
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/PlatformBootManagerLibGrub

We need a clear and consistent rule on where the confidential computing module should go, instead of making ad-hoc decision for each features.

2) Ideally, when we integrate SEV feature or TDX feature, there is some level of isolation, such as
               A) standalone driver
               B) standalone library
               C) standalone file, if it has to be in one module
               D) standalone function, if it has to be in one file
The preference is from A to D. A is most preferred. D is least preferred.
As such, when people find something wrong, they can just focus on some SEV or TDX specific files.

We do see good examples, such as SecretDxe (BTW: The name should be SevSecretDxe), AmdSevDxe.
However, some code (ResetVector and Sec) mixed SEV support with normal OVMF code together. That makes it extremely hard to review TDX extensions or maintain a non-SEV code.
https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/Ia32/PageTables64.asm
https://github.com/tianocore/edk2/blob/master/OvmfPkg/Sec/SecMain.c

For latter (such as ResetVector and Sec), I suggest we make a clear isolation. That can help the reviewer to understand better on SEV flow, TDX flow and normal OVMF flow.

3) We may have more problem. For example, how to align the OVMF design between SEV and TDX?
I think, the most SEV OVMF design is good. The TDX OVMF should just follow. For example,
https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib


[Proposal]

The current SEV OVMF design is understandable, because when the SEV code was added years ago, it was the first example. We did not know what would be the best way to handle that, and we did not know what TDX would look like.
Today, we have more concrete answer, and let do some refinement step by step.

Confidential computing (CC) == SEV or TDX (it may include RME in the future)

1) CC Feature support (DSC/FDF)

* Try to limit the impact to existing normal OVMF binary.

1.1 - The OVMF packet common DSC/FDF supports OVMF boot in all CC modes or normal mode.

The one OVMF image can boot in normal OVMF mode, SEV mode, or TDX mode.

1.2 - The OVMF packet common DSC/FDF includes *mature* CC feature.

The minimal scope is the image shall boot to OS successfully.
The maximal scope is the feature shall be adopted by OS and will not change for a period of time.

Any immature, under discussion or under review feature shall NOT be put here, such as attestation.

1.3 - The OVMF package CC specific DSC/FDF includes *all* CC feature.

The CC specific DSC/FDF shall be in OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).

The full feature scope may include any feature excluded in 1.2.

Once we believe it is mature and it is well cross-evaluated with other CC infrastructure, this feature may be added to 1.2 later. (step by step approach)

2) CC feature module location

* Try to balance the situation: put too many modules under one dir v.s. create too many layers

2.1 - If it is CC hardware architecture compliant (irrelevant to OVMF), it may be put to non-OvmfPkg.

For example, https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseIoLibIntrinsic

2.2 - If it is a *basic* OVMF CC feature, it shall be put to OvmfPkg directly.

Basic means the OVMF cannot boot without it.

2.3 - If it is an *advanced* OVMF CC feature, it shall be put to OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).

Advanced means the OVMF may still boot without it, just lose some functionality.

3) CC feature convergence.

* Try to help design review and maintenance.

3.1 - A CC feature should be standalone module (driver or library), if it is possible.

Good example:
https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib

3.2 - If we have to merge CC feature to a module, then the CC related code shall be isolated to a file.

The file name could be Xxx<Cc>.{c,asm}

A common pattern can be used:

3.2.A - C function.

3.2.A.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:
====================
               PreFeatureFunctionHookSev ();
               PreFeatureFunctionHookTdx ();
               FeatureFunction ();
               PostFeatureFunctionHookSev ();
               PostFeatureFunctionHookTdx ();
====================
3.2.A.2 - If CC function is a replacement for non-CC function. The main function can check current mode and decide to call which function. For example:
====================
               if (IsSev()) {
                              FeatureFunctionSev();
               } else if (IsTdx()) {
                              FeatureFunctionTdx();
               } else {
                              FeatureFunction();
               }
====================

3.2.B - ASM function.

3.2.B.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:
====================
               OneTimeCall   PreFeatureFunctionHookSev
               OneTimeCall   PreFeatureFunctionHookTdx
FeatureFunction:
               XXXXXX
FeatureFunctionEnd:
               OneTimeCall   PostMainFunctionHookSev
               OneTimeCall   PostMainFunctionHookTdx
====================
3.2.B.2 - If CC function is a replacement for non-CC function. The main function can call CC replacement function, then check the return status to decide next step. For example:
====================
               OneTimeCallRet   FeatureFunctionSev
               Jz FeatureFunctionEnd
               OneTimeCallRet   FeatureFunctionTdx
               Jz FeatureFunctionEnd
FeatureFunction:
               XXXXXX
FeatureFunctionEnd:
====================


Thank you
Yao Jiewen






[-- Attachment #2: Type: text/html, Size: 17946 bytes --]

Re: [edk2-rfc] [edk2-devel] RFC: Common Design Proposal on Confidential Computing Support in OVMF

[Ard Biesheuvel]

On Mon, 26 Jul 2021 at 10:59, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Hi
>
> I would like to raise the topic on a confidential computing support in OVMF.
>
>
>
> The main target is AMD SEV feature and Intel TDX feature in OVMF package.
>
>
>
> The goal is to create a guidance for our future confidential computing work and to better support review and maintenance.
>

Hello Jiewen,

Thanks for writing this up. As you know, ARM is a bit behind in the
CCA space, and so I will not be able to take part in these discussions
in great detail.

I will leave it to the contributors and other stakeholders to comment
on your proposal below. To me, it looks reasonable.

-- 
Ard.


>
>
>
>
> [Background]
>
>
>
> AMD is adding AMD Secure Encrypted Virtualization (SEV), SEV-Encrypted State (SEV-ES), SEV-Secure Nested Paging (SEV-SNP) features to OVMF package. (https://developer.amd.com/sev/)
>
> Intel is adding Intel Trust Domain Extensions (TDX) features to OVMF package. (https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html)
>
>
>
> Both of them support confidential computing use case.
>
>
>
> ARM is creating Realm Management Extension (RME). It might be considered in the future. (https://www.arm.com/why-arm/architecture/security-features/arm-confidential-compute-architecture)
>
>
>
> So we need a good Confidential Computing infrastructure for EDKII.
>
>
>
>
>
> [Problem Statement]
>
>
>
> 1) Current OVMF package integrated some AMD SEV features. But not all features.
>
> Some basic SEV features are in OvmfPkg and enabled as default build. Some advanced SEV features are in OvmfPkg/AmdSev and only enable in AmdSev build.
>
> However, the criteria is NOT clear.
>
>
>
> It also brings problem when we want to add more TDX stuff. Where we should go?
>
> For example, I feel PlatformBootManagerLibGrub should be in OvmfPkg/AmdSev. Why it is not there?
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/PlatformBootManagerLibGrub
>
>
>
> We need a clear and consistent rule on where the confidential computing module should go, instead of making ad-hoc decision for each features.
>
>
>
> 2) Ideally, when we integrate SEV feature or TDX feature, there is some level of isolation, such as
>
>                A) standalone driver
>
>                B) standalone library
>
>                C) standalone file, if it has to be in one module
>
>                D) standalone function, if it has to be in one file
>
> The preference is from A to D. A is most preferred. D is least preferred.
>
> As such, when people find something wrong, they can just focus on some SEV or TDX specific files.
>
>
>
> We do see good examples, such as SecretDxe (BTW: The name should be SevSecretDxe), AmdSevDxe.
>
> However, some code (ResetVector and Sec) mixed SEV support with normal OVMF code together. That makes it extremely hard to review TDX extensions or maintain a non-SEV code.
>
> https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>
> https://github.com/tianocore/edk2/blob/master/OvmfPkg/Sec/SecMain.c
>
>
>
> For latter (such as ResetVector and Sec), I suggest we make a clear isolation. That can help the reviewer to understand better on SEV flow, TDX flow and normal OVMF flow.
>
>
>
> 3) We may have more problem. For example, how to align the OVMF design between SEV and TDX?
>
> I think, the most SEV OVMF design is good. The TDX OVMF should just follow. For example,
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib
>
>
>
>
>
> [Proposal]
>
>
>
> The current SEV OVMF design is understandable, because when the SEV code was added years ago, it was the first example. We did not know what would be the best way to handle that, and we did not know what TDX would look like.
>
> Today, we have more concrete answer, and let do some refinement step by step.
>
>
>
> Confidential computing (CC) == SEV or TDX (it may include RME in the future)
>
>
>
> 1) CC Feature support (DSC/FDF)
>
>
>
> * Try to limit the impact to existing normal OVMF binary.
>
>
>
> 1.1 - The OVMF packet common DSC/FDF supports OVMF boot in all CC modes or normal mode.
>
>
>
> The one OVMF image can boot in normal OVMF mode, SEV mode, or TDX mode.
>
>
>
> 1.2 - The OVMF packet common DSC/FDF includes *mature* CC feature.
>
>
>
> The minimal scope is the image shall boot to OS successfully.
>
> The maximal scope is the feature shall be adopted by OS and will not change for a period of time.
>
>
>
> Any immature, under discussion or under review feature shall NOT be put here, such as attestation.
>
>
>
> 1.3 - The OVMF package CC specific DSC/FDF includes *all* CC feature.
>
>
>
> The CC specific DSC/FDF shall be in OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).
>
>
>
> The full feature scope may include any feature excluded in 1.2.
>
>
>
> Once we believe it is mature and it is well cross-evaluated with other CC infrastructure, this feature may be added to 1.2 later. (step by step approach)
>
>
>
> 2) CC feature module location
>
>
>
> * Try to balance the situation: put too many modules under one dir v.s. create too many layers
>
>
>
> 2.1 - If it is CC hardware architecture compliant (irrelevant to OVMF), it may be put to non-OvmfPkg.
>
>
>
> For example, https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseIoLibIntrinsic
>
>
>
> 2.2 - If it is a *basic* OVMF CC feature, it shall be put to OvmfPkg directly.
>
>
>
> Basic means the OVMF cannot boot without it.
>
>
>
> 2.3 - If it is an *advanced* OVMF CC feature, it shall be put to OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).
>
>
>
> Advanced means the OVMF may still boot without it, just lose some functionality.
>
>
>
> 3) CC feature convergence.
>
>
>
> * Try to help design review and maintenance.
>
>
>
> 3.1 - A CC feature should be standalone module (driver or library), if it is possible.
>
>
>
> Good example:
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib
>
>
>
> 3.2 - If we have to merge CC feature to a module, then the CC related code shall be isolated to a file.
>
>
>
> The file name could be Xxx<Cc>.{c,asm}
>
>
>
> A common pattern can be used:
>
>
>
> 3.2.A - C function.
>
>
>
> 3.2.A.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:
>
> ====================
>
>                PreFeatureFunctionHookSev ();
>
>                PreFeatureFunctionHookTdx ();
>
>                FeatureFunction ();
>
>                PostFeatureFunctionHookSev ();
>
>                PostFeatureFunctionHookTdx ();
>
> ====================
>
> 3.2.A.2 - If CC function is a replacement for non-CC function. The main function can check current mode and decide to call which function. For example:
>
> ====================
>
>                if (IsSev()) {
>
>                               FeatureFunctionSev();
>
>                } else if (IsTdx()) {
>
>                               FeatureFunctionTdx();
>
>                } else {
>
>                               FeatureFunction();
>
>                }
>
> ====================
>
>
>
> 3.2.B - ASM function.
>
>
>
> 3.2.B.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:
>
> ====================
>
>                OneTimeCall   PreFeatureFunctionHookSev
>
>                OneTimeCall   PreFeatureFunctionHookTdx
>
> FeatureFunction:
>
>                XXXXXX
>
> FeatureFunctionEnd:
>
>                OneTimeCall   PostMainFunctionHookSev
>
>                OneTimeCall   PostMainFunctionHookTdx
>
> ====================
>
> 3.2.B.2 - If CC function is a replacement for non-CC function. The main function can call CC replacement function, then check the return status to decide next step. For example:
>
> ====================
>
>                OneTimeCallRet   FeatureFunctionSev
>
>                Jz FeatureFunctionEnd
>
>                OneTimeCallRet   FeatureFunctionTdx
>
>                Jz FeatureFunctionEnd
>
> FeatureFunction:
>
>                XXXXXX
>
> FeatureFunctionEnd:
>
> ====================
>
>
>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
>
>
>
>
>
>
>