* [PATCH 1/4] MdePkg: Add TdProbeLib
2022-04-13 9:08 [PATCH 0/4] Introduce TdProbe in MdePkg Min Xu
@ 2022-04-13 9:08 ` Min Xu
2022-04-13 9:08 ` [PATCH 2/4] OvmfPkg/IntelTdx: " Min Xu
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Min Xu @ 2022-04-13 9:08 UTC (permalink / raw)
To: devel
Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
James Bottomley, Jiewen Yao, Gerd Hoffmann
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/4] OvmfPkg/IntelTdx: Add TdProbeLib
2022-04-13 9:08 [PATCH 0/4] Introduce TdProbe in MdePkg Min Xu
2022-04-13 9:08 ` [PATCH 1/4] MdePkg: Add TdProbeLib Min Xu
@ 2022-04-13 9:08 ` Min Xu
2022-04-13 9:08 ` [PATCH 3/4] MdePkg: Probe Td guest in BaseIoLibIntrinsicSev Min Xu
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Min Xu @ 2022-04-13 9:08 UTC (permalink / raw)
To: devel
Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
James Bottomley, James Bottomley, Jiewen Yao, Gerd Hoffmann,
Brijesh Singh, Erdem Aktas, Tom Lendacky
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/4] MdePkg: Probe Td guest in BaseIoLibIntrinsicSev
2022-04-13 9:08 [PATCH 0/4] Introduce TdProbe in MdePkg Min Xu
2022-04-13 9:08 ` [PATCH 1/4] MdePkg: Add TdProbeLib Min Xu
2022-04-13 9:08 ` [PATCH 2/4] OvmfPkg/IntelTdx: " Min Xu
@ 2022-04-13 9:08 ` Min Xu
2022-04-13 9:08 ` [PATCH 4/4] OvmfPkg: Add TdProbeLib in *.dsc Min Xu
2022-04-13 12:55 ` [PATCH 0/4] Introduce TdProbe in MdePkg James Bottomley
4 siblings, 0 replies; 8+ messages in thread
From: Min Xu @ 2022-04-13 9:08 UTC (permalink / raw)
To: devel
Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
James Bottomley, James Bottomley, Jiewen Yao, Gerd Hoffmann
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/4] OvmfPkg: Add TdProbeLib in *.dsc
2022-04-13 9:08 [PATCH 0/4] Introduce TdProbe in MdePkg Min Xu
` (2 preceding siblings ...)
2022-04-13 9:08 ` [PATCH 3/4] MdePkg: Probe Td guest in BaseIoLibIntrinsicSev Min Xu
@ 2022-04-13 9:08 ` Min Xu
2022-04-13 12:55 ` [PATCH 0/4] Introduce TdProbe in MdePkg James Bottomley
4 siblings, 0 replies; 8+ messages in thread
From: Min Xu @ 2022-04-13 9:08 UTC (permalink / raw)
To: devel
Cc: Min Xu, James Bottomley, James Bottomley, Jiewen Yao,
Gerd Hoffmann, Brijesh Singh, Erdem Aktas, Tom Lendacky
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 0/4] Introduce TdProbe in MdePkg
2022-04-13 9:08 [PATCH 0/4] Introduce TdProbe in MdePkg Min Xu
` (3 preceding siblings ...)
2022-04-13 9:08 ` [PATCH 4/4] OvmfPkg: Add TdProbeLib in *.dsc Min Xu
@ 2022-04-13 12:55 ` James Bottomley
2022-04-13 13:12 ` [edk2-devel] " Yao, Jiewen
4 siblings, 1 reply; 8+ messages in thread
From: James Bottomley @ 2022-04-13 12:55 UTC (permalink / raw)
To: Min Xu, devel
Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
Gerd Hoffmann, Brijesh Singh, Erdem Aktas, Tom Lendacky
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [edk2-devel] [PATCH 0/4] Introduce TdProbe in MdePkg
2022-04-13 12:55 ` [PATCH 0/4] Introduce TdProbe in MdePkg James Bottomley
@ 2022-04-13 13:12 ` Yao, Jiewen
2022-04-13 13:27 ` Min Xu
0 siblings, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2022-04-13 13:12 UTC (permalink / raw)
To: devel@edk2.groups.io, jejb@linux.ibm.com, Xu, Min M
Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Gerd Hoffmann,
Brijesh Singh, Aktas, Erdem, Tom Lendacky
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
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] Introduce TdProbe in MdePkg
2022-04-13 13:12 ` [edk2-devel] " Yao, Jiewen
@ 2022-04-13 13:27 ` Min Xu
0 siblings, 0 replies; 8+ messages in thread
From: Min Xu @ 2022-04-13 13:27 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io, jejb@linux.ibm.com
Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, Gerd Hoffmann,
Brijesh Singh, Aktas, Erdem, Tom Lendacky
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
^ permalink raw reply [flat|nested] 8+ messages in thread