On Mon, Dec 13, 2021 at 02:39:53AM +0000, Xu, Min M wrote:
> 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?
Well, I still think there is opportunity to share code, specifically the
page table handling. Have a generic page table walker which is able to
set and clear bits for a given memory range. Then the sev/tdx specific
code can just call that instead of both having their own, duplicated
page table walking logic.
Maybe the page table walking should even be a MdeModulePkg Library, i.e.
move the code for page table walking (and huge page splitting) in
MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c to a library so it can
be reused elsewhere without duplicating the code,
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84671): https://edk2.groups.io/g/devel/message/84671
Mute This Topic: https://groups.io/mt/86739898/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
