Re: [edk2-devel] [PATCH V3 29/29] OvmfPkg: Update IoMmuDxe to support TDX

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

Hi
> 
> > +  if (CC_GUEST_IS_SEV (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> > +    //
> > +    // Clear the memory encryption mask on the plaintext buffer.
> > +    //
> > +    Status = MemEncryptSevClearPageEncMask (
> > +               0,
> > +               MapInfo->PlainTextAddress,
> > +               MapInfo->NumberOfPages
> > +               );
> > +  } else if (CC_GUEST_IS_TDX (PcdGet64
> (PcdConfidentialComputingGuestAttr))) {
> > +    //
> > +    // Set the memory shared bit.
> > +    //
> > +    Status = MemEncryptTdxSetPageSharedBit (
> > +               0,
> > +               MapInfo->PlainTextAddress,
> > +               MapInfo->NumberOfPages
> > +               );
> 
> Again, this looks very simliar and like a great opportunity to share code.
> 
MemEncryptSevClearPageEncMask () is implemented in MemEncryptSevLib.
MemEncryptTdxSetPageSharedBit () is implemented in MemEncryptTdxlib. 
Yes, we have considered to merge these 2 EncryptLib into one lib (for example: MemoryEncryptCcLib). But after investigation and some PoC, we find it will make the code complicated and hard to maintain. (many if-else checking in the code)
1. From the naming perspective (in SEV/TDX documentation), SEV's bit is Enc bit, but TDX's bit is shared bit.
2. In SEV's SetMemoryEncDec () it handles differently for the different version of SEV (for example,  Sev-Snp). I am not sure if there will be more specific process will be added in the future.
3. In TDX's SetMemorySharedOrPrivate, currently it is simple and clean. But there maybe some new features added in the future.

I am thinking if it is a better choice that every vendor takes their responsibility to maintain their own lib/code? 
In the current EDK2 CI there is no test case for SEV or TDX, I am a little nervous if some changes will impact the existing feature. 

Thanks
Min