Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg: Save PcdConfidentialComputingGuestAttr in mCcGuestAttr

["Min Xu" <min.m.xu@intel.com>]

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

Thanks for the suggestion. I tried the solution and it does work.
In a summary, the solution imports 2 CpuMpPei in OvmfPkgX64. Each CpuMpPei driver owns unique FILE_GUID so that both 2 drivers can be built into one image. A set of MpInitLibDepLib are imported. These libs are very simple and they just depends on the PPIs. Different PPI is installed based on the working guest type. With the help of these MpInitLibDepLib, we can leave the CpuMpPei untouched but in fact it depends on the PPIs.
The PoC is in: https://github.com/tianocore/edk2/pull/2852

I will send out the patch-set after some comments/words refinement.

From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, May 5, 2022 9:41 AM
To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg: Save PcdConfidentialComputingGuestAttr in mCcGuestAttr


On Mon, May 2, 2022 at 03:03 PM, Min Xu wrote:

The reason why C global variable cannot be used in PEIM is that in some scenario PEIM is executed in FLASH so that the value of C global variable cannot be kept in different calls. But I don't think it is a problem in this situation. 1. This global variable is to keep the PcdConfidentialComputingGuestAttribute in mCcGuestAttr. So if this is Tdx guest, then the global variable can be kept (CC_GUEST_IS_TDX (mCcGuestAttr) == TRUE). 2. If this is Non-Td guest, then even the global variable cannot be kept, CC_GUEST_IS_TDX (mCcGuestAttr) is FALSE. So mCcGuestAttr can still work.

Directly writing to flash area might cause some side effects. Usually we need to avoid that.

There is another solution that we can use CcProbe (which is in MdePkg/CcProbeLib). CcProbe checks the work area to fetch the guest type. It calls FixedPcdGet32 (PcdOvmfWorkAreaBase) so there is no SMP safe issue in PcdLib.

It seems ok. But it's unclear to me how the work area is built.

As to a new field in CPU_MP_DATA, Tdx guest doesn't create CPU_MP_DATA object in MpInitLibInitialize, instead it return right after it detects the working guest is Tdx guest. So this fix will be more complicated than above 2 solutions.

I agree it'll be more complicated to let the TDX version of MpLib use the CPU_MP_DATA. But, I am very curious why not leave the MpInitLib untouched. The solution is: 1. Platform FDF includes two CpuMpPeim drivers. One links to PeiMpInitLib, the other links to MpInitLibUp. Change platform DSC to let first PEIM depend on "FullMpPpi" Change platform DSC to let second PEIM depend on "UpMpPpi" 2. SEC code passes "FullMpPpi" to PEI Core when it's not a TDX boot. It passes "UpMpPpi" when it's a TDX boot.

I think this solution doesn't even change the MP code. Or saying in another way, MP and TDX are decoupled. I prefer this solution.

Ray, What's your thought?

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