Re: [edk2-devel] [PATCH 0/4] Introduce TdProbe in MdePkg

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thank you very much, James and Min, for the quick response.

I have one small concern on the naming - TdProbeLib.
Would it be better if we create a generic CcProbeLib, and just return the CC_GUEST_TYPE?
The benefit is that just in case SEV has some usage in the future, we don’t need create SevProbeLib.

BTW: I also propose to change name BaseIoLibIntrinsicSev.inf to BaseIoLibIntrinsicCc.inf.
It is very confusing to me, that a Sev.inf include a TdProbeLib. :-(
That can be done in a separate patch.

Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of James
> Bottomley
> Sent: Wednesday, April 13, 2022 8:55 PM
> To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Brijesh
> Singh <brijesh.singh@amd.com>; Aktas, Erdem <erdemaktas@google.com>;
> Tom Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH 0/4] Introduce TdProbe in MdePkg
> 
> On Wed, 2022-04-13 at 17:08 +0800, Min Xu wrote:
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902
> >
> > Bad IO performance in SEC phase is observed after TDX features was
> > introduced. (after commit b6b2de884864 - "MdePkg: Support mmio for
> > Tdx guest in BaseIoLibIntrinsic").
> >
> > This is because IsTdxGuest() will be called in each MMIO operation.
> > It is trying to cache the result of the probe in the efi data
> > segment. However, that doesn't work in SEC, because the data segment
> > is read only (so the write seems to succeed but a read will always
> > return the original value), leading to us calling TdIsEnabled() check
> > for every mmio we do, which is causing the slowdown because it's very
> > expensive.
> >
> > TdProbe is introduced in this patch-set. It is called in
> > BaseIoLibIntrinsicSev instead of IsTdxGuest. There are 2 versions of
> > the TdProbeLib. Null instance of TdProbe always returns TD_PROBE_NON.
> > Its OvmfPkg version checks the Ovmf work area to determine the Td
> > guest type.
> 
> I tested this out with the TPM code: it restores pretty much all of the
> lost performance, thanks!
> 
> Tested-by: James Bottomley <jejb@linux.ibm.com>
> 
> James
> 
> 
> 
> 
> 
>