[PATCH 0/4] Introduce TdProbe in MdePkg

[PATCH 0/4] Introduce TdProbe in MdePkg

[Min Xu]

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.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

Min Xu (4):
  MdePkg: Add TdProbeLib
  OvmfPkg/IntelTdx: Add TdProbeLib
  MdePkg: Probe Td guest in BaseIoLibIntrinsicSev
  OvmfPkg: Add TdProbeLib in *.dsc

 MdePkg/Include/Library/TdProbeLib.h           | 26 +++++++++++++
 .../BaseIoLibIntrinsicSev.inf                 |  1 +
 .../BaseIoLibIntrinsic/IoLibInternalTdx.c     | 13 +------
 .../Library/TdProbeLibNull/TdProbeLibNull.c   | 25 ++++++++++++
 .../Library/TdProbeLibNull/TdProbeLibNull.inf | 21 ++++++++++
 MdePkg/MdePkg.dec                             |  5 +++
 MdePkg/MdePkg.dsc                             |  1 +
 OvmfPkg/AmdSev/AmdSevX64.dsc                  |  1 +
 OvmfPkg/Bhyve/BhyveX64.dsc                    |  1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc                |  1 +
 OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  1 +
 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.c      | 38 +++++++++++++++++++
 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf    | 25 ++++++++++++
 OvmfPkg/Microvm/MicrovmX64.dsc                |  1 +
 OvmfPkg/OvmfPkgIa32.dsc                       |  1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                    |  1 +
 OvmfPkg/OvmfPkgX64.dsc                        |  1 +
 OvmfPkg/OvmfXen.dsc                           |  1 +
 18 files changed, 153 insertions(+), 11 deletions(-)
 create mode 100644 MdePkg/Include/Library/TdProbeLib.h
 create mode 100644 MdePkg/Library/TdProbeLibNull/TdProbeLibNull.c
 create mode 100644 MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
 create mode 100644 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.c
 create mode 100644 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf

-- 
2.29.2.windows.2

[PATCH 1/4] MdePkg: Add TdProbeLib

[Min Xu]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902

TdProbeLib is used to probe if the working system is of Td guest. This
library is designed to run on SEC / PEI / DXE phases. A null instance
of the library returns Non-Td anyway. A platform specific TdProbeLib
will be implemented, for example, in OvmfPkg.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 MdePkg/Include/Library/TdProbeLib.h           | 26 +++++++++++++++++++
 .../Library/TdProbeLibNull/TdProbeLibNull.c   | 25 ++++++++++++++++++
 .../Library/TdProbeLibNull/TdProbeLibNull.inf | 21 +++++++++++++++
 MdePkg/MdePkg.dec                             |  5 ++++
 MdePkg/MdePkg.dsc                             |  1 +
 5 files changed, 78 insertions(+)
 create mode 100644 MdePkg/Include/Library/TdProbeLib.h
 create mode 100644 MdePkg/Library/TdProbeLibNull/TdProbeLibNull.c
 create mode 100644 MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf

diff --git a/MdePkg/Include/Library/TdProbeLib.h b/MdePkg/Include/Library/TdProbeLib.h
new file mode 100644
index 000000000000..363a30fce649
--- /dev/null
+++ b/MdePkg/Include/Library/TdProbeLib.h
@@ -0,0 +1,26 @@
+/** @file
+
+Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef TD_PROBE_LIB_H_
+#define TD_PROBE_LIB_H_
+
+#define TD_PROBE_NON  0
+#define TD_PROBE_TDX  1
+
+/**
+  Probe if it is Tdx guest.
+
+  @return TD_PROBE_TDX if it is Tdx guest. Otherwise return TD_PROBE_NON.
+
+**/
+UINTN
+EFIAPI
+TdProbe (
+  VOID
+  );
+
+#endif
diff --git a/MdePkg/Library/TdProbeLibNull/TdProbeLibNull.c b/MdePkg/Library/TdProbeLibNull/TdProbeLibNull.c
new file mode 100644
index 000000000000..8aa2baf9053d
--- /dev/null
+++ b/MdePkg/Library/TdProbeLibNull/TdProbeLibNull.c
@@ -0,0 +1,25 @@
+/** @file
+
+  Null stub of TdProbeLib
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/TdProbeLib.h>
+
+/**
+  Probe if it is Tdx guest.
+
+  @return TD_PROBE_TDX if it is Tdx guest. Otherwise return TD_PROBE_NON.
+
+**/
+UINTN
+EFIAPI
+TdProbe (
+  VOID
+  )
+{
+  return TD_PROBE_NON;
+}
diff --git a/MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf b/MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
new file mode 100644
index 000000000000..7db961e6ea2d
--- /dev/null
+++ b/MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
@@ -0,0 +1,21 @@
+## @file
+# TdProbeLib null instance.
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = TdProbeLibNull
+  FILE_GUID                      = B15D67FE-0DAC-4316-8E26-8A6b85E43782
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = TdProbeLib
+
+[Sources]
+  TdProbeLibNull.c
+
+[Packages]
+  MdePkg/MdePkg.dec
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 1934c9840423..9cc713e3d5af 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -267,6 +267,11 @@
   #
   RegisterFilterLib|Include/Library/RegisterFilterLib.h
 
+  ##  @libraryclass  This library provides interfances to probe Td guest.
+  #
+  #
+  TdProbeLib|Include/Library/TdProbeLib.h
+
 [LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH64]
   ##  @libraryclass  Provides services to generate random number.
   #
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index d6a7af412be7..c2a68056f48a 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -130,6 +130,7 @@
   MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
 
   MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
+  MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
 
 [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
   #
-- 
2.29.2.windows.2

[PATCH 2/4] OvmfPkg/IntelTdx: Add TdProbeLib

[Min Xu]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902

This is the OvmfPkg specific TdProbeLib. It checks the Ovmf WorkArea
(PcdOvmfWorkAreaBase) to determine if the working system is of Td guest.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.c   | 38 ++++++++++++++++++++++
 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf | 25 ++++++++++++++
 2 files changed, 63 insertions(+)
 create mode 100644 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.c
 create mode 100644 OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf

diff --git a/OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.c b/OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.c
new file mode 100644
index 000000000000..85b9c5b614e5
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.c
@@ -0,0 +1,38 @@
+/** @file
+
+  TdProbeLib is used to probe the Td guest.
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/TdProbeLib.h>
+#include <WorkArea.h>
+#include <ConfidentialComputingGuestAttr.h>
+
+/**
+  Probe if it is Tdx guest.
+
+  @return TD_PROBE_TDX if it is Tdx guest. Otherwise return TD_PROBE_NON.
+
+**/
+UINTN
+EFIAPI
+TdProbe (
+  VOID
+  )
+{
+  OVMF_WORK_AREA  *WorkArea;
+
+  WorkArea = (OVMF_WORK_AREA *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
+
+  //
+  // Check if it is TDX guest.
+  //
+  if ((WorkArea != NULL) && (WorkArea->Header.GuestType == GUEST_TYPE_INTEL_TDX)) {
+    return TD_PROBE_TDX;
+  } else {
+    return TD_PROBE_NON;
+  }
+}
diff --git a/OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf b/OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf
new file mode 100644
index 000000000000..7535008b3a51
--- /dev/null
+++ b/OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf
@@ -0,0 +1,25 @@
+## @file
+# TdProbeLib is used to probe Td guest.
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = TdProbeLib
+  FILE_GUID                      = 05184ec9-abb0-4491-8584-e388639a7c48
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = TdProbeLib
+
+[Sources]
+  TdProbeLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
-- 
2.29.2.windows.2

[PATCH 3/4] MdePkg: Probe Td guest in BaseIoLibIntrinsicSev

[Min Xu]

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 TdProbe instead of IsTdxGuest. Null instance of
TdProbe returns TD_PROBE_NON anyway. Its OvmfPkg version checks the
Ovmf work area to determine the Td guest type.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@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..76f11438f40b 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
@@ -55,6 +55,7 @@
   DebugLib
   BaseLib
   RegisterFilterLib
+  TdProbeLib
 
 [LibraryClasses.X64]
   TdxLib
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c
index 1e539dbfbbad..9aed9f312858 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/TdProbeLib.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 TdProbe () == TD_PROBE_TDX;
 }
 
 /**
-- 
2.29.2.windows.2

[PATCH 4/4] OvmfPkg: Add TdProbeLib in *.dsc

[Min Xu]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902

TdProbeLib is imported in BaseIoLibIntrinsicSev.
OvmfPkg/IntelTdx/TdProbeLib is the OvmfPkg version which checks
OvmfWorkArea to determine the Td guest type. It is included
in OvmfPkgX64.dsc and IntelTdx/IntelTdxX64.dsc.

Other .dsc include the MdePkg/Library/TdProbeLibNull because Td guest
is not supported in those projects.

Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc     | 1 +
 OvmfPkg/Bhyve/BhyveX64.dsc       | 1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc   | 1 +
 OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
 OvmfPkg/Microvm/MicrovmX64.dsc   | 1 +
 OvmfPkg/OvmfPkgIa32.dsc          | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc       | 1 +
 OvmfPkg/OvmfPkgX64.dsc           | 1 +
 OvmfPkg/OvmfXen.dsc              | 1 +
 9 files changed, 9 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index fcdc3efab204..5b4b5e397aa1 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -149,6 +149,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index e1b6b8e15f36..f3f5e1921e6d 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -146,6 +146,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index 20f3bc340807..6b75980ba414 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -158,6 +158,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 245155d41b30..007a140b7ca7 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -135,6 +135,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 59580ccd4691..a55603c5a634 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -156,6 +156,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index e4218b01f0fc..020e3b476917 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -154,6 +154,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a80cdaacb8bc..c4a789e35d05 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -158,6 +158,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index fb2899f8a1be..ad43bcd9776d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -165,6 +165,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|OvmfPkg/IntelTdx/TdProbeLib/TdProbeLib.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 7bd594c6e263..ff9b8786beb6 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -147,6 +147,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  TdProbeLib|MdePkg/Library/TdProbeLibNull/TdProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
-- 
2.29.2.windows.2

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

[James Bottomley]

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

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

[Yao, Jiewen]

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
> 
> 
> 
> 
> 
> 

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

[Min Xu]

On April 13, 2022 9:12 PM, Yao Jiewen wrote:
> 
> 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.
>
Thanks for the suggestion. It will be renamed to CcProbeLib in the next version.
> 
> 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.
Agree. I will create a separate patch-set to rename the lib.
> 

Thanks
Min