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