On Mon, 26 Jul 2021 at 10:59, Yao, Jiewen <jiewen....@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
>
>
>
>
>
>
>
>
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78357): https://edk2.groups.io/g/devel/message/78357
Mute This Topic: https://groups.io/mt/84454603/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to