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

["Ni, Ray" <ray.ni@intel.com>]

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

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: 2177 bytes --]