On Fri, Apr 15, 2022 at 08:07:08AM +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. > > This patch is to call CcProbe instead of TdIsEnabled in IsTdxGuest. > Null instance of CcProbe always returns CCGuestTypeNonEncrypted. Its > OvmfPkg version returns the guest type in Ovmf work area.
Hi! I ran through our tests on stable-202205-rc1, and I'm finding that all of the tests using 2M FD_SIZE & SMM_REQUIRE=TRUE are failing with QEMU hanging w/o output. Equivalent tests w/ 4M FD_SIZE are working fine. I bisected it down to this commit, and also confirmed that reverting this commit on top of 202205-rc1 also avoids the problem. I might have a chance to debug more tomorrow, but for now I just wanted to flag it. -dann > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Zhiguang Liu <zhiguang....@intel.com> > Cc: James Bottomley <j...@linux.ibm.com> > Cc: James Bottomley <james.bottom...@hansenpartnership.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Signed-off-by: Min Xu <min.m...@intel.com> > --- > .../BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 1 + > .../Library/BaseIoLibIntrinsic/IoLibInternalTdx.c | 13 ++----------- > 2 files changed, 3 insertions(+), 11 deletions(-) > > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf > b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf > index 7fe1c60f046e..e1b8298ac451 100644 > --- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf > +++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf > @@ -55,6 +55,7 @@ > DebugLib > BaseLib > RegisterFilterLib > + CcProbeLib > > [LibraryClasses.X64] > TdxLib > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c > b/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c > index 1e539dbfbbad..8af6fc35c591 100644 > --- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c > +++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c > @@ -10,6 +10,7 @@ > #include <Include/IndustryStandard/Tdx.h> > #include <Library/TdxLib.h> > #include <Register/Intel/Cpuid.h> > +#include <Library/CcProbeLib.h> > #include "IoLibTdx.h" > > // Size of TDVMCALL Access, including IO and MMIO > @@ -22,9 +23,6 @@ > #define TDVMCALL_ACCESS_READ 0 > #define TDVMCALL_ACCESS_WRITE 1 > > -BOOLEAN mTdxEnabled = FALSE; > -BOOLEAN mTdxProbed = FALSE; > - > /** > Check if it is Tdx guest. > > @@ -38,14 +36,7 @@ IsTdxGuest ( > VOID > ) > { > - if (mTdxProbed) { > - return mTdxEnabled; > - } > - > - mTdxEnabled = TdIsEnabled (); > - mTdxProbed = TRUE; > - > - return mTdxEnabled; > + return CcProbe () == CCGuestTypeIntelTdx; > } > > /** -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#89782): https://edk2.groups.io/g/devel/message/89782 Mute This Topic: https://groups.io/mt/90477280/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-