* [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
@ 2023-10-27 5:42 duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance duntan
` (7 more replies)
0 siblings, 8 replies; 27+ messages in thread
From: duntan @ 2023-10-27 5:42 UTC (permalink / raw)
To: devel
The goal is to have single BaseIoLibIntrinsic instance that can also used for sev and Tdx.
In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
Then change the source file of BaseIoLibIntrinsic to also support Tdx and sev feature. So BaseIoLibIntrinsicSev and related assembly code can be removed.
Dun Tan (7):
MdePkg: Create TdxLibNull.inf instance
MdePkg: Add CcProbeLibNull and TdxLibNull implement
MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
MdePkg:support Tdx and sev in BaseIoLibIntrinsic
OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
MdePkg:remove BaseIoLibIntrinsicSev related code
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++----
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61 -------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 -----------------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45 +++++++++++++++++++++++++++++++++++++--------
MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 ------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/TdxLib/TdxLibNull.inf | 21 +++++++++++++++++++++
MdePkg/MdeLibs.dsc.inc | 4 +++-
MdePkg/MdePkg.dsc | 2 +-
OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
OvmfPkg/OvmfPkgIa32.dsc | 2 +-
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
OvmfPkg/OvmfPkgX64.dsc | 2 +-
OvmfPkg/OvmfXen.dsc | 2 +-
21 files changed, 83 insertions(+), 1077 deletions(-)
delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110181): https://edk2.groups.io/g/devel/message/110181
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
@ 2023-10-27 5:42 ` duntan
2023-10-27 5:54 ` Ni, Ray
2023-10-27 5:42 ` [edk2-devel] [PATCH 2/7] MdePkg: Add CcProbeLibNull and TdxLibNull implement duntan
` (6 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-10-27 5:42 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Ray Ni
Create null instance for TdxLib. The IoLib instance
BaseIoLibIntrinsic.inf will consume the TdxLibNull
to pass build.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/Library/TdxLib/TdxLibNull.inf | 21 +++++++++++++++++++++
MdePkg/MdePkg.dsc | 1 +
2 files changed, 22 insertions(+)
diff --git a/MdePkg/Library/TdxLib/TdxLibNull.inf b/MdePkg/Library/TdxLib/TdxLibNull.inf
new file mode 100644
index 0000000000..ecf20e12e1
--- /dev/null
+++ b/MdePkg/Library/TdxLib/TdxLibNull.inf
@@ -0,0 +1,21 @@
+## @file
+# Tdx null instance.
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = TdxLibNull
+ FILE_GUID = AA75759C-24E0-42FE-B5A4-83678548FCEF
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxLib
+
+[Sources]
+ TdxLibNull.c
+
+[Packages]
+ MdePkg/MdePkg.dec
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 3abd1a1e23..a1be123d80 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -184,6 +184,7 @@
MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
MdePkg/Library/TdxLib/TdxLib.inf
+ MdePkg/Library/TdxLib/TdxLibNull.inf
MdePkg/Library/MipiSysTLib/MipiSysTLib.inf
MdePkg/Library/TraceHubDebugSysTLibNull/TraceHubDebugSysTLibNull.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110182): https://edk2.groups.io/g/devel/message/110182
Mute This Topic: https://groups.io/mt/102215662/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 2/7] MdePkg: Add CcProbeLibNull and TdxLibNull implement
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance duntan
@ 2023-10-27 5:42 ` duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 3/7] MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c duntan
` (5 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: duntan @ 2023-10-27 5:42 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Ray Ni
Add CcProbeLibNull and TdxLibNull implement in the
MdeLibs.dsc.inc. The IoLib instance BaseIoLibIntrinsic
will consume the two lib instance in the following
patch. Add the two null instance in this file to avoid
build failure.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/MdeLibs.dsc.inc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/MdePkg/MdeLibs.dsc.inc b/MdePkg/MdeLibs.dsc.inc
index 4580481cb5..0525283cfd 100644
--- a/MdePkg/MdeLibs.dsc.inc
+++ b/MdePkg/MdeLibs.dsc.inc
@@ -5,7 +5,7 @@
# by using "!include MdePkg/MdeLibs.dsc.inc" to specify the library instances
# of some EDKII basic/common library classes.
#
-# Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2021 - 2023, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -16,3 +16,5 @@
RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
SmmCpuRendezvousLib|MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf
+ CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
+ TdxLib|MdePkg/Library/TdxLib/TdxLibNull.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110183): https://edk2.groups.io/g/devel/message/110183
Mute This Topic: https://groups.io/mt/102215663/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 3/7] MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 2/7] MdePkg: Add CcProbeLibNull and TdxLibNull implement duntan
@ 2023-10-27 5:42 ` duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 4/7] MdePkg:support Tdx and sev in BaseIoLibIntrinsic duntan
` (4 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: duntan @ 2023-10-27 5:42 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Ray Ni
Simplify IoRead/WriteFifo implement in IoLibFifo.c
by repeatedly calling IoRead/Write API in C code.
This can avoid calling assembly code to use string
I/O instructions and avoid checking if sev feature
is enabled. Then the code flow is simplified.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 44 +++++++++++++++++++++++++++++++++++++-------
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
index 9a94bc6a05..adce1040f6 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
+++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
@@ -1,7 +1,7 @@
/** @file
IoFifo read/write routines.
- Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021-2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -39,10 +39,15 @@ IoReadFifo8 (
OUT VOID *Buffer
)
{
+ UINT8 *Buffer8;
+
if (IsTdxGuest ()) {
TdIoReadFifo8 (Port, Count, Buffer);
} else {
- SevIoReadFifo8 (Port, Count, Buffer);
+ Buffer8 = (UINT8 *)Buffer;
+ while (Count-- > 0) {
+ *Buffer8++ = IoRead8 (Port);
+ }
}
}
@@ -73,10 +78,15 @@ IoWriteFifo8 (
IN VOID *Buffer
)
{
+ UINT8 *Buffer8;
+
if (IsTdxGuest ()) {
TdIoWriteFifo8 (Port, Count, Buffer);
} else {
- SevIoWriteFifo8 (Port, Count, Buffer);
+ Buffer8 = (UINT8 *)Buffer;
+ while (Count-- > 0) {
+ IoWrite8 (Port, *Buffer8++);
+ }
}
}
@@ -107,10 +117,15 @@ IoReadFifo16 (
OUT VOID *Buffer
)
{
+ UINT16 *Buffer16;
+
if (IsTdxGuest ()) {
TdIoReadFifo16 (Port, Count, Buffer);
} else {
- SevIoReadFifo16 (Port, Count, Buffer);
+ Buffer16 = (UINT16 *)Buffer;
+ while (Count-- > 0) {
+ *Buffer16++ = IoRead16 (Port);
+ }
}
}
@@ -141,10 +156,15 @@ IoWriteFifo16 (
IN VOID *Buffer
)
{
+ UINT16 *Buffer16;
+
if (IsTdxGuest ()) {
TdIoWriteFifo16 (Port, Count, Buffer);
} else {
- SevIoWriteFifo16 (Port, Count, Buffer);
+ Buffer16 = (UINT16 *)Buffer;
+ while (Count-- > 0) {
+ IoWrite16 (Port, *Buffer16++);
+ }
}
}
@@ -175,10 +195,15 @@ IoReadFifo32 (
OUT VOID *Buffer
)
{
+ UINT32 *Buffer32;
+
if (IsTdxGuest ()) {
TdIoReadFifo32 (Port, Count, Buffer);
} else {
- SevIoReadFifo32 (Port, Count, Buffer);
+ Buffer32 = (UINT32 *)Buffer;
+ while (Count-- > 0) {
+ *Buffer32++ = IoRead32 (Port);
+ }
}
}
@@ -209,9 +234,14 @@ IoWriteFifo32 (
IN VOID *Buffer
)
{
+ UINT32 *Buffer32;
+
if (IsTdxGuest ()) {
TdIoWriteFifo32 (Port, Count, Buffer);
} else {
- SevIoWriteFifo32 (Port, Count, Buffer);
+ Buffer32 = (UINT32 *)Buffer;
+ while (Count-- > 0) {
+ IoWrite32 (Port, *Buffer32++);
+ }
}
}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110184): https://edk2.groups.io/g/devel/message/110184
Mute This Topic: https://groups.io/mt/102215664/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 4/7] MdePkg:support Tdx and sev in BaseIoLibIntrinsic
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
` (2 preceding siblings ...)
2023-10-27 5:42 ` [edk2-devel] [PATCH 3/7] MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c duntan
@ 2023-10-27 5:42 ` duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 5/7] OvmfPkg: Add CcProbeLib in PlatformInitLib.inf duntan
` (3 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: duntan @ 2023-10-27 5:42 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Ray Ni
Change IA32 and X64 arch source files for
BaseIoLibIntrinsic to support Tdx and sev.
Use IoFifoRead/Write API in IoLibFifo.c and the
IoLibInternalTdx.c instead of IoFifoSev.nasm for
BaseIoLibIntrinsic.
With this change, BaseIoLibIntrinsic can also support
Tdx guest and sev. Then the assembly code and instance
for BaseIoLibIntrinsicSev can be removed.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
index aeb072ee95..e1a2e1eed8 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
@@ -7,7 +7,7 @@
# ASSERT(). For ARM, AARCH64, RISCV64 and LoongArch, this I/O library only provides
# non I/O read and write.
#
-# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.<BR>
# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
# Portions Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
@@ -35,24 +35,26 @@
IoLibMmioBuffer.c
BaseIoLibIntrinsicInternal.h
IoHighLevel.c
- IoLibInternalTdxNull.c
IoLibTdx.h
[Sources.IA32]
IoLibGcc.c | GCC
IoLibMsc.c | MSFT
IoLib.c
- Ia32/IoFifo.nasm
+ IoLibFifo.c
+ IoLibInternalTdxNull.c
[Sources.X64]
IoLibGcc.c | GCC
IoLibMsc.c | MSFT
IoLib.c
- X64/IoFifo.nasm
+ IoLibFifo.c
+ IoLibInternalTdx.c
[Sources.EBC]
IoLibEbc.c
IoLib.c
+ IoLibInternalTdxNull.c
[Sources.ARM]
IoLibNoIo.c
@@ -74,3 +76,7 @@
BaseLib
RegisterFilterLib
+[LibraryClasses.X64]
+ TdxLib
+ CcProbeLib
+
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110185): https://edk2.groups.io/g/devel/message/110185
Mute This Topic: https://groups.io/mt/102215665/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 5/7] OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
` (3 preceding siblings ...)
2023-10-27 5:42 ` [edk2-devel] [PATCH 4/7] MdePkg:support Tdx and sev in BaseIoLibIntrinsic duntan
@ 2023-10-27 5:42 ` duntan
2023-10-30 14:37 ` Ard Biesheuvel
2023-10-27 5:42 ` [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files duntan
` (2 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-10-27 5:42 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Ray Ni
PlatformInitLib uses the CcProbe API in MemDetect.c
but doensn't add CcProbeLib in .inf LibraryClasses
section. Add CcProbeLib to fix the build error.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
---
OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
index 5a79d95b68..8ff9e699b1 100644
--- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
+++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
@@ -2,7 +2,7 @@
# Platform Initialization Lib
#
# This module provides platform specific function to detect boot mode.
-# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -52,6 +52,7 @@
PcdLib
PciLib
PeiHardwareInfoLib
+ CcProbeLib
[LibraryClasses.X64]
TdxLib
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110186): https://edk2.groups.io/g/devel/message/110186
Mute This Topic: https://groups.io/mt/102215667/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
` (4 preceding siblings ...)
2023-10-27 5:42 ` [edk2-devel] [PATCH 5/7] OvmfPkg: Add CcProbeLib in PlatformInitLib.inf duntan
@ 2023-10-27 5:42 ` duntan
2023-10-30 14:36 ` Ard Biesheuvel
2023-10-27 5:43 ` [edk2-devel] [PATCH 7/7] MdePkg:remove BaseIoLibIntrinsicSev related code duntan
2023-10-27 5:49 ` [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev Yao, Jiewen
7 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-10-27 5:42 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Ray Ni
Use BaseIoLibIntrinsic.inf in dsc files. The
BaseIoLibIntrinsic supports Tdx and sev now.
The BaseIoLibIntrinsicSev will be removed in
the following patches.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
OvmfPkg/OvmfPkgIa32.dsc | 2 +-
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
OvmfPkg/OvmfPkgX64.dsc | 2 +-
OvmfPkg/OvmfXen.dsc | 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 302c90e7c2..06086e0cc5 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -142,7 +142,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 6693342c5f..1376dd7468 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -149,7 +149,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index 35942e02df..1d2a13dd92 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -159,7 +159,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 182ec3705d..6623196c8b 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -146,7 +146,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 0f26f2a9a9..c76a135f9e 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -157,7 +157,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index fcd3a3fda5..a850465e77 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -162,7 +162,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index d0ae0b996d..9c6615b487 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -167,7 +167,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a6811eee55..9e054aa489 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -179,7 +179,7 @@
PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index ccd3a873c7..ab4bdb2628 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -150,7 +150,7 @@
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110187): https://edk2.groups.io/g/devel/message/110187
Mute This Topic: https://groups.io/mt/102215668/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [edk2-devel] [PATCH 7/7] MdePkg:remove BaseIoLibIntrinsicSev related code
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
` (5 preceding siblings ...)
2023-10-27 5:42 ` [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files duntan
@ 2023-10-27 5:43 ` duntan
2023-10-27 5:49 ` [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev Yao, Jiewen
7 siblings, 0 replies; 27+ messages in thread
From: duntan @ 2023-10-27 5:43 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Ray Ni
Remove BaseIoLibIntrinsicSev related code in MdePkg.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61 -------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 -----------------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 1 -
MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 ------------------------------------------------------------------------------------------------------------------------
MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
MdePkg/MdePkg.dsc | 1 -
8 files changed, 1055 deletions(-)
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
deleted file mode 100644
index e1b8298ac4..0000000000
--- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+++ /dev/null
@@ -1,61 +0,0 @@
-## @file
-# Instance of I/O Library using compiler intrinsics.
-#
-# I/O Library that uses compiler intrinsics to perform IN and OUT instructions
-# for IA-32 and x64.
-#
-# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
-# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
-# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
-#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-##
-
-[Defines]
- INF_VERSION = 0x00010005
- BASE_NAME = BaseIoLibIntrinsicSev
- MODULE_UNI_FILE = BaseIoLibIntrinsic.uni
- FILE_GUID = 93742f95-6e71-4581-b600-8e1da443f95a
- MODULE_TYPE = BASE
- VERSION_STRING = 1.0
- LIBRARY_CLASS = IoLib
-
-
-#
-# VALID_ARCHITECTURES = IA32 X64
-#
-
-[Sources]
- IoLibMmioBuffer.c
- BaseIoLibIntrinsicInternal.h
- IoHighLevel.c
- IoLibTdx.h
- IoLibSev.h
-
-[Sources.IA32]
- IoLibGcc.c | GCC
- IoLibMsc.c | MSFT
- IoLib.c
- IoLibInternalTdxNull.c
- Ia32/IoFifoSev.nasm
-
-[Sources.X64]
- IoLibGcc.c | GCC
- IoLibMsc.c | MSFT
- IoLib.c
- IoLibInternalTdx.c
- IoLibFifo.c
- X64/IoFifoSev.nasm
-
-[Packages]
- MdePkg/MdePkg.dec
-
-[LibraryClasses]
- DebugLib
- BaseLib
- RegisterFilterLib
- CcProbeLib
-
-[LibraryClasses.X64]
- TdxLib
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
deleted file mode 100644
index a4ae1a0053..0000000000
--- a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
+++ /dev/null
@@ -1,131 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-; Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
-;
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-;------------------------------------------------------------------------------
-
- SECTION .text
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; OUT VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo8)
-ASM_PFX(IoReadFifo8):
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insb
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; OUT VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo16)
-ASM_PFX(IoReadFifo16):
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insw
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; OUT VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo32)
-ASM_PFX(IoReadFifo32):
- push edi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-rep insd
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo8)
-ASM_PFX(IoWriteFifo8):
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsb
- pop esi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo16)
-ASM_PFX(IoWriteFifo16):
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsw
- pop esi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo32)
-ASM_PFX(IoWriteFifo32):
- push esi
- cld
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-rep outsd
- pop esi
- ret
-
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
deleted file mode 100644
index 63a4771d3c..0000000000
--- a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
+++ /dev/null
@@ -1,293 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-; Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
-;
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-;------------------------------------------------------------------------------
-
- SECTION .text
-
-;------------------------------------------------------------------------------
-; Check whether we need to unroll the String I/O under SEV guest
-;
-; Return // eax (1 - unroll, 0 - no unroll)
-;------------------------------------------------------------------------------
-global ASM_PFX(SevNoRepIo)
-ASM_PFX(SevNoRepIo):
-
- ; CPUID clobbers ebx, ecx and edx
- push ebx
- push ecx
- push edx
-
- ; Check if we are running under hypervisor
- ; CPUID(1).ECX Bit 31
- mov eax, 1
- cpuid
- bt ecx, 31
- jnc @UseRepIo
-
- ; Check if we have Memory encryption CPUID leaf
- mov eax, 0x80000000
- cpuid
- cmp eax, 0x8000001f
- jl @UseRepIo
-
- ; Check for memory encryption feature:
- ; CPUID Fn8000_001F[EAX] - Bit 1
- ;
- mov eax, 0x8000001f
- cpuid
- bt eax, 1
- jnc @UseRepIo
-
- ; Check if memory encryption is enabled
- ; MSR_0xC0010131 - Bit 0 (SEV enabled)
- ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled)
- mov ecx, 0xc0010131
- rdmsr
-
- ; Check for (SevEsEnabled == 0 && SevEnabled == 1)
- and eax, 3
- cmp eax, 1
- je @SevNoRepIo_Done
-
-@UseRepIo:
- xor eax, eax
-
-@SevNoRepIo_Done:
- pop edx
- pop ecx
- pop ebx
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; OUT VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo8)
-ASM_PFX(IoReadFifo8):
- push edi
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoReadFifo8_NoRep
-
- cld
- rep insb
- jmp @IoReadFifo8_Done
-
-@IoReadFifo8_NoRep:
- jecxz @IoReadFifo8_Done
-
-@IoReadFifo8_Loop:
- in al, dx
- mov byte [edi], al
- inc edi
- loop @IoReadFifo8_Loop
-
-@IoReadFifo8_Done:
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; OUT VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo16)
-ASM_PFX(IoReadFifo16):
- push edi
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoReadFifo16_NoRep
-
- cld
- rep insw
- jmp @IoReadFifo16_Done
-
-@IoReadFifo16_NoRep:
- jecxz @IoReadFifo16_Done
-
-@IoReadFifo16_Loop:
- in ax, dx
- mov word [edi], ax
- add edi, 2
- loop @IoReadFifo16_Loop
-
-@IoReadFifo16_Done:
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; OUT VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo32)
-ASM_PFX(IoReadFifo32):
- push edi
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov edi, [esp + 16]
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoReadFifo32_NoRep
-
- cld
- rep insd
- jmp @IoReadFifo32_Done
-
-@IoReadFifo32_NoRep:
- jecxz @IoReadFifo32_Done
-
-@IoReadFifo32_Loop:
- in eax, dx
- mov dword [edi], eax
- add edi, 4
- loop @IoReadFifo32_Loop
-
-@IoReadFifo32_Done:
- pop edi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo8)
-ASM_PFX(IoWriteFifo8):
- push esi
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoWriteFifo8_NoRep
-
- cld
- rep outsb
- jmp @IoWriteFifo8_Done
-
-@IoWriteFifo8_NoRep:
- jecxz @IoWriteFifo8_Done
-
-@IoWriteFifo8_Loop:
- mov al, byte [esi]
- out dx, al
- inc esi
- loop @IoWriteFifo8_Loop
-
-@IoWriteFifo8_Done:
- pop esi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo16 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo16)
-ASM_PFX(IoWriteFifo16):
- push esi
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoWriteFifo16_NoRep
-
- cld
- rep outsw
- jmp @IoWriteFifo16_Done
-
-@IoWriteFifo16_NoRep:
- jecxz @IoWriteFifo16_Done
-
-@IoWriteFifo16_Loop:
- mov ax, word [esi]
- out dx, ax
- add esi, 2
- loop @IoWriteFifo16_Loop
-
-@IoWriteFifo16_Done:
- pop esi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo32 (
-; IN UINTN Port,
-; IN UINTN Size,
-; IN VOID *Buffer
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo32)
-ASM_PFX(IoWriteFifo32):
- push esi
- mov dx, [esp + 8]
- mov ecx, [esp + 12]
- mov esi, [esp + 16]
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoWriteFifo32_NoRep
-
- cld
- rep outsd
- jmp @IoWriteFifo32_Done
-
-@IoWriteFifo32_NoRep:
- jecxz @IoWriteFifo32_Done
-
-@IoWriteFifo32_Loop:
- mov eax, dword [esi]
- out dx, eax
- add esi, 4
- loop @IoWriteFifo32_Loop
-
-@IoWriteFifo32_Done:
- pop esi
- ret
-
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
index adce1040f6..91c0637c7d 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
+++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c
@@ -7,7 +7,6 @@
**/
#include "BaseIoLibIntrinsicInternal.h"
-#include "IoLibSev.h"
#include "IoLibTdx.h"
#include <Uefi/UefiBaseType.h>
#include <Library/TdxLib.h>
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h b/MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
deleted file mode 100644
index 6d7cafcff2..0000000000
--- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
+++ /dev/null
@@ -1,166 +0,0 @@
-/** @file
- Header file for SEV IO library.
-
- Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
- SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#ifndef IOLIB_SEV_H_
-#define IOLIB_SEV_H_
-
-#include <Base.h>
-
-#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
-
-/**
- Reads an 8-bit I/O port fifo into a block of memory.
-
- Reads the 8-bit I/O fifo port specified by Port.
- The port is read Count times, and the read data is
- stored in the provided Buffer.
-
- This function must guarantee that all I/O read and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to read.
- @param Count The number of times to read I/O port.
- @param Buffer The buffer to store the read data into.
-
-**/
-VOID
-EFIAPI
-SevIoReadFifo8 (
- IN UINTN Port,
- IN UINTN Count,
- OUT VOID *Buffer
- );
-
-/**
- Writes a block of memory into an 8-bit I/O port fifo.
-
- Writes the 8-bit I/O fifo port specified by Port.
- The port is written Count times, and the write data is
- retrieved from the provided Buffer.
-
- This function must guarantee that all I/O write and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to write.
- @param Count The number of times to write I/O port.
- @param Buffer The buffer to retrieve the write data from.
-
-**/
-VOID
-EFIAPI
-SevIoWriteFifo8 (
- IN UINTN Port,
- IN UINTN Count,
- IN VOID *Buffer
- );
-
-/**
- Reads an 8-bit I/O port fifo into a block of memory.
-
- Reads the 8-bit I/O fifo port specified by Port.
- The port is read Count times, and the read data is
- stored in the provided Buffer.
-
- This function must guarantee that all I/O read and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to read.
- @param Count The number of times to read I/O port.
- @param Buffer The buffer to store the read data into.
-
-**/
-VOID
-EFIAPI
-SevIoReadFifo16 (
- IN UINTN Port,
- IN UINTN Count,
- OUT VOID *Buffer
- );
-
-/**
- Writes a block of memory into an 8-bit I/O port fifo.
-
- Writes the 8-bit I/O fifo port specified by Port.
- The port is written Count times, and the write data is
- retrieved from the provided Buffer.
-
- This function must guarantee that all I/O write and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to write.
- @param Count The number of times to write I/O port.
- @param Buffer The buffer to retrieve the write data from.
-
-**/
-VOID
-EFIAPI
-SevIoWriteFifo16 (
- IN UINTN Port,
- IN UINTN Count,
- IN VOID *Buffer
- );
-
-/**
- Reads an 8-bit I/O port fifo into a block of memory.
-
- Reads the 8-bit I/O fifo port specified by Port.
- The port is read Count times, and the read data is
- stored in the provided Buffer.
-
- This function must guarantee that all I/O read and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to read.
- @param Count The number of times to read I/O port.
- @param Buffer The buffer to store the read data into.
-
-**/
-VOID
-EFIAPI
-SevIoReadFifo32 (
- IN UINTN Port,
- IN UINTN Count,
- OUT VOID *Buffer
- );
-
-/**
- Writes a block of memory into an 8-bit I/O port fifo.
-
- Writes the 8-bit I/O fifo port specified by Port.
- The port is written Count times, and the write data is
- retrieved from the provided Buffer.
-
- This function must guarantee that all I/O write and write operations are
- serialized.
-
- If 8-bit I/O port operations are not supported, then ASSERT().
-
- @param Port The I/O port to write.
- @param Count The number of times to write I/O port.
- @param Buffer The buffer to retrieve the write data from.
-
-**/
-VOID
-EFIAPI
-SevIoWriteFifo32 (
- IN UINTN Port,
- IN UINTN Count,
- IN VOID *Buffer
- );
-
-#endif
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm b/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
deleted file mode 100644
index d459072f6e..0000000000
--- a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
+++ /dev/null
@@ -1,120 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-; Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
-;
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-;------------------------------------------------------------------------------
-
- DEFAULT REL
- SECTION .text
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; OUT VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo8)
-ASM_PFX(IoReadFifo8):
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insb
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; OUT VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo16)
-ASM_PFX(IoReadFifo16):
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insw
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoReadFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; OUT VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoReadFifo32)
-ASM_PFX(IoReadFifo32):
- cld
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-rep insd
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo8)
-ASM_PFX(IoWriteFifo8):
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsb
- mov rsi, r8 ; restore rsi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo16)
-ASM_PFX(IoWriteFifo16):
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsw
- mov rsi, r8 ; restore rsi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(IoWriteFifo32)
-ASM_PFX(IoWriteFifo32):
- cld
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-rep outsd
- mov rsi, r8 ; restore rsi
- ret
-
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm b/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
deleted file mode 100644
index d02286b4d5..0000000000
--- a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
+++ /dev/null
@@ -1,282 +0,0 @@
-;------------------------------------------------------------------------------
-;
-; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
-; Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
-;
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-;------------------------------------------------------------------------------
-
- DEFAULT REL
- SECTION .text
-
-;------------------------------------------------------------------------------
-; Check whether we need to unroll the String I/O in SEV guest
-;
-; Return // eax (1 - unroll, 0 - no unroll)
-;------------------------------------------------------------------------------
-global ASM_PFX(SevNoRepIo)
-ASM_PFX(SevNoRepIo):
-
- ; CPUID clobbers ebx, ecx and edx
- push rbx
- push rcx
- push rdx
-
- ; Check if we are runing under hypervisor
- ; CPUID(1).ECX Bit 31
- mov eax, 1
- cpuid
- bt ecx, 31
- jnc @UseRepIo
-
- ; Check if we have Memory encryption CPUID leaf
- mov eax, 0x80000000
- cpuid
- cmp eax, 0x8000001f
- jl @UseRepIo
-
- ; Check for memory encryption feature:
- ; CPUID Fn8000_001F[EAX] - Bit 1
- ;
- mov eax, 0x8000001f
- cpuid
- bt eax, 1
- jnc @UseRepIo
-
- ; Check if memory encryption is enabled
- ; MSR_0xC0010131 - Bit 0 (SEV enabled)
- ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled)
- mov ecx, 0xc0010131
- rdmsr
-
- ; Check for (SevEsEnabled == 0 && SevEnabled == 1)
- and eax, 3
- cmp eax, 1
- je @SevNoRepIo_Done
-
-@UseRepIo:
- xor eax, eax
-
-@SevNoRepIo_Done:
- pop rdx
- pop rcx
- pop rbx
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; SevIoReadFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; OUT VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(SevIoReadFifo8)
-ASM_PFX(SevIoReadFifo8):
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoReadFifo8_NoRep
-
- cld
- rep insb
- jmp @IoReadFifo8_Done
-
-@IoReadFifo8_NoRep:
- jrcxz @IoReadFifo8_Done
-
-@IoReadFifo8_Loop:
- in al, dx
- mov byte [rdi], al
- inc rdi
- loop @IoReadFifo8_Loop
-
-@IoReadFifo8_Done:
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; SevIoReadFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; OUT VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(SevIoReadFifo16)
-ASM_PFX(SevIoReadFifo16):
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoReadFifo16_NoRep
-
- cld
- rep insw
- jmp @IoReadFifo16_Done
-
-@IoReadFifo16_NoRep:
- jrcxz @IoReadFifo16_Done
-
-@IoReadFifo16_Loop:
- in ax, dx
- mov word [rdi], ax
- add rdi, 2
- loop @IoReadFifo16_Loop
-
-@IoReadFifo16_Done:
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; SevIoReadFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; OUT VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(SevIoReadFifo32)
-ASM_PFX(SevIoReadFifo32):
- xchg rcx, rdx
- xchg rdi, r8 ; rdi: buffer address; r8: save rdi
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoReadFifo32_NoRep
-
- cld
- rep insd
- jmp @IoReadFifo32_Done
-
-@IoReadFifo32_NoRep:
- jrcxz @IoReadFifo32_Done
-
-@IoReadFifo32_Loop:
- in eax, dx
- mov dword [rdi], eax
- add rdi, 4
- loop @IoReadFifo32_Loop
-
-@IoReadFifo32_Done:
- mov rdi, r8 ; restore rdi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; IoWriteFifo8 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(SevIoWriteFifo8)
-ASM_PFX(SevIoWriteFifo8):
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoWriteFifo8_NoRep
-
- cld
- rep outsb
- jmp @IoWriteFifo8_Done
-
-@IoWriteFifo8_NoRep:
- jrcxz @IoWriteFifo8_Done
-
-@IoWriteFifo8_Loop:
- mov al, byte [rsi]
- out dx, al
- inc rsi
- loop @IoWriteFifo8_Loop
-
-@IoWriteFifo8_Done:
- mov rsi, r8 ; restore rsi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; SevIoWriteFifo16 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(SevIoWriteFifo16)
-ASM_PFX(SevIoWriteFifo16):
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoWriteFifo16_NoRep
-
- cld
- rep outsw
- jmp @IoWriteFifo16_Done
-
-@IoWriteFifo16_NoRep:
- jrcxz @IoWriteFifo16_Done
-
-@IoWriteFifo16_Loop:
- mov ax, word [rsi]
- out dx, ax
- add rsi, 2
- loop @IoWriteFifo16_Loop
-
-@IoWriteFifo16_Done:
- mov rsi, r8 ; restore rsi
- ret
-
-;------------------------------------------------------------------------------
-; VOID
-; EFIAPI
-; SevIoWriteFifo32 (
-; IN UINTN Port, // rcx
-; IN UINTN Size, // rdx
-; IN VOID *Buffer // r8
-; );
-;------------------------------------------------------------------------------
-global ASM_PFX(SevIoWriteFifo32)
-ASM_PFX(SevIoWriteFifo32):
- xchg rcx, rdx
- xchg rsi, r8 ; rsi: buffer address; r8: save rsi
-
- ; Check if we need to unroll String I/O
- call ASM_PFX(SevNoRepIo)
- test eax, eax
- jnz @IoWriteFifo32_NoRep
-
- cld
- rep outsd
- jmp @IoWriteFifo32_Done
-
-@IoWriteFifo32_NoRep:
- jrcxz @IoWriteFifo32_Done
-
-@IoWriteFifo32_Loop:
- mov eax, dword [rsi]
- out dx, eax
- add rsi, 4
- loop @IoWriteFifo32_Loop
-
-@IoWriteFifo32_Done:
- mov rsi, r8 ; restore rsi
- ret
-
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index a1be123d80..87c7045f63 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -158,7 +158,6 @@
[Components.IA32, Components.X64]
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
- MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110188): https://edk2.groups.io/g/devel/message/110188
Mute This Topic: https://groups.io/mt/102215669/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
` (6 preceding siblings ...)
2023-10-27 5:43 ` [edk2-devel] [PATCH 7/7] MdePkg:remove BaseIoLibIntrinsicSev related code duntan
@ 2023-10-27 5:49 ` Yao, Jiewen
2023-10-27 6:31 ` duntan
7 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2023-10-27 5:49 UTC (permalink / raw)
To: devel@edk2.groups.io, Tan, Dun
HI
Since this impact TDX and SEV, would you please let me know what kind of test you have done?
Have you validated TDX and SEV before you submit the patch? Please describe that clearly in your patch description.
Also please include AMD SEV reviewer in this patch series.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Friday, October 27, 2023 1:43 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and
> remove BaseIoLibIntrinsicSev
>
> The goal is to have single BaseIoLibIntrinsic instance that can also used for sev
> and Tdx.
> In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
> Then change the source file of BaseIoLibIntrinsic to also support Tdx and sev
> feature. So BaseIoLibIntrinsicSev and related assembly code can be removed.
>
> Dun Tan (7):
> MdePkg: Create TdxLibNull.inf instance
> MdePkg: Add CcProbeLibNull and TdxLibNull implement
> MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
> MdePkg:support Tdx and sev in BaseIoLibIntrinsic
> OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
> OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
> MdePkg:remove BaseIoLibIntrinsicSev related code
>
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++----
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61 ------------------
> -------------------------------------------
> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 --------------------
> --------------------------------------------------------------------------------------------------
> -------------
> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 ------------------
> --------------------------------------------------------------------------------------------------
> --------------------------------------------------------------------------------------------------
> -------------------------------------------------------------------------------
> MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
> +++++++++++++++++++++++++++++++++++++--------
> MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ------------------------
> --------------------------------------------------------------------------------------------------
> --------------------------------------------
> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 --------------------
> --------------------------------------------------------------------------------------------------
> --
> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ------------------
> --------------------------------------------------------------------------------------------------
> --------------------------------------------------------------------------------------------------
> --------------------------------------------------------------------
> MdePkg/Library/TdxLib/TdxLibNull.inf | 21
> +++++++++++++++++++++
> MdePkg/MdeLibs.dsc.inc | 4 +++-
> MdePkg/MdePkg.dsc | 2 +-
> OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
> OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
> OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
> OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
> OvmfPkg/OvmfPkgX64.dsc | 2 +-
> OvmfPkg/OvmfXen.dsc | 2 +-
> 21 files changed, 83 insertions(+), 1077 deletions(-)
> delete mode 100644
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
> create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
>
> --
> 2.31.1.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110190): https://edk2.groups.io/g/devel/message/110190
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
2023-10-27 5:42 ` [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance duntan
@ 2023-10-27 5:54 ` Ni, Ray
2023-10-27 5:56 ` Ni, Ray
0 siblings, 1 reply; 27+ messages in thread
From: Ni, Ray @ 2023-10-27 5:54 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io
Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang
[-- Attachment #1: Type: text/plain, Size: 2604 bytes --]
TdxLibNull.c is missing in the patch.
Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, October 27, 2023 1:42 PM
To: devel@edk2.groups.io <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>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
Create null instance for TdxLib. The IoLib instance
BaseIoLibIntrinsic.inf will consume the TdxLibNull
to pass build.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/Library/TdxLib/TdxLibNull.inf | 21 +++++++++++++++++++++
MdePkg/MdePkg.dsc | 1 +
2 files changed, 22 insertions(+)
diff --git a/MdePkg/Library/TdxLib/TdxLibNull.inf b/MdePkg/Library/TdxLib/TdxLibNull.inf
new file mode 100644
index 0000000000..ecf20e12e1
--- /dev/null
+++ b/MdePkg/Library/TdxLib/TdxLibNull.inf
@@ -0,0 +1,21 @@
+## @file
+# Tdx null instance.
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = TdxLibNull
+ FILE_GUID = AA75759C-24E0-42FE-B5A4-83678548FCEF
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxLib
+
+[Sources]
+ TdxLibNull.c
+
+[Packages]
+ MdePkg/MdePkg.dec
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 3abd1a1e23..a1be123d80 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -184,6 +184,7 @@
MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
MdePkg/Library/TdxLib/TdxLib.inf
+ MdePkg/Library/TdxLib/TdxLibNull.inf
MdePkg/Library/MipiSysTLib/MipiSysTLib.inf
MdePkg/Library/TraceHubDebugSysTLibNull/TraceHubDebugSysTLibNull.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110192): https://edk2.groups.io/g/devel/message/110192
Mute This Topic: https://groups.io/mt/102215662/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 5418 bytes --]
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
2023-10-27 5:54 ` Ni, Ray
@ 2023-10-27 5:56 ` Ni, Ray
2023-10-27 6:32 ` duntan
0 siblings, 1 reply; 27+ messages in thread
From: Ni, Ray @ 2023-10-27 5:56 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io
Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang
[-- Attachment #1: Type: text/plain, Size: 3122 bytes --]
Please ignore my comments.
That's an existing file for IA32 build. Your patch adds a NULL instance for all archs.
Thanks,
Ray
________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, October 27, 2023 1:54 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io <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>
Subject: Re: [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
TdxLibNull.c is missing in the patch.
Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, October 27, 2023 1:42 PM
To: devel@edk2.groups.io <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>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
Create null instance for TdxLib. The IoLib instance
BaseIoLibIntrinsic.inf will consume the TdxLibNull
to pass build.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/Library/TdxLib/TdxLibNull.inf | 21 +++++++++++++++++++++
MdePkg/MdePkg.dsc | 1 +
2 files changed, 22 insertions(+)
diff --git a/MdePkg/Library/TdxLib/TdxLibNull.inf b/MdePkg/Library/TdxLib/TdxLibNull.inf
new file mode 100644
index 0000000000..ecf20e12e1
--- /dev/null
+++ b/MdePkg/Library/TdxLib/TdxLibNull.inf
@@ -0,0 +1,21 @@
+## @file
+# Tdx null instance.
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = TdxLibNull
+ FILE_GUID = AA75759C-24E0-42FE-B5A4-83678548FCEF
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxLib
+
+[Sources]
+ TdxLibNull.c
+
+[Packages]
+ MdePkg/MdePkg.dec
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 3abd1a1e23..a1be123d80 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -184,6 +184,7 @@
MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
MdePkg/Library/TdxLib/TdxLib.inf
+ MdePkg/Library/TdxLib/TdxLibNull.inf
MdePkg/Library/MipiSysTLib/MipiSysTLib.inf
MdePkg/Library/TraceHubDebugSysTLibNull/TraceHubDebugSysTLibNull.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110193): https://edk2.groups.io/g/devel/message/110193
Mute This Topic: https://groups.io/mt/102215662/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 7183 bytes --]
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-27 5:49 ` [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev Yao, Jiewen
@ 2023-10-27 6:31 ` duntan
2023-10-27 7:06 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-10-27 6:31 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io
Hi Jiewen,
Currently I'm working on the Tdx test. Since the patch set doesn't change the code logic when Tdx or SEV is enabled, so I want to send out the patch as soon as possible to see if there is any comments from community.
I will include AMD SEV reviewer in this patch series. Thanks for reminding.
Thanks,
Dun
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, October 27, 2023 1:49 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
HI
Since this impact TDX and SEV, would you please let me know what kind of test you have done?
Have you validated TDX and SEV before you submit the patch? Please describe that clearly in your patch description.
Also please include AMD SEV reviewer in this patch series.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Friday, October 27, 2023 1:43 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>
> The goal is to have single BaseIoLibIntrinsic instance that can also
> used for sev and Tdx.
> In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
> Then change the source file of BaseIoLibIntrinsic to also support Tdx
> and sev feature. So BaseIoLibIntrinsicSev and related assembly code can be removed.
>
> Dun Tan (7):
> MdePkg: Create TdxLibNull.inf instance
> MdePkg: Add CcProbeLibNull and TdxLibNull implement
> MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
> MdePkg:support Tdx and sev in BaseIoLibIntrinsic
> OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
> OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
> MdePkg:remove BaseIoLibIntrinsicSev related code
>
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++----
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61
> ------------------
> -------------------------------------------
> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 --------------------
> ----------------------------------------------------------------------
> ----------------------------
> -------------
> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 ------------------
> ----------------------------------------------------------------------
> ----------------------------
> ----------------------------------------------------------------------
> ----------------------------
> -------------------------------------------------------------------------------
> MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
> +++++++++++++++++++++++++++++++++++++--------
> MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ------------------------
> ----------------------------------------------------------------------
> ----------------------------
> --------------------------------------------
> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 --------------------
> ----------------------------------------------------------------------
> ----------------------------
> --
> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ------------------
> ----------------------------------------------------------------------
> ----------------------------
> ----------------------------------------------------------------------
> ----------------------------
> --------------------------------------------------------------------
> MdePkg/Library/TdxLib/TdxLibNull.inf | 21
> +++++++++++++++++++++
> MdePkg/MdeLibs.dsc.inc | 4 +++-
> MdePkg/MdePkg.dsc | 2 +-
> OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
> OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
> OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
> OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
> OvmfPkg/OvmfPkgX64.dsc | 2 +-
> OvmfPkg/OvmfXen.dsc | 2 +-
> 21 files changed, 83 insertions(+), 1077 deletions(-) delete mode
> 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> delete mode 100644
> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> delete mode 100644
> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
> create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
>
> --
> 2.31.1.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110194): https://edk2.groups.io/g/devel/message/110194
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
2023-10-27 5:56 ` Ni, Ray
@ 2023-10-27 6:32 ` duntan
0 siblings, 0 replies; 27+ messages in thread
From: duntan @ 2023-10-27 6:32 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang
[-- Attachment #1: Type: text/plain, Size: 4068 bytes --]
Yes TdxLibNull.c is an existing file for IA32 build in the TdxLib.inf
Thanks,
Dun
From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, October 27, 2023 1:56 PM
To: Tan, Dun <dun.tan@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>
Subject: Re: [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
Please ignore my comments.
That's an existing file for IA32 build. Your patch adds a NULL instance for all archs.
Thanks,
Ray
________________________________
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Friday, October 27, 2023 1:54 PM
To: Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
Subject: Re: [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
TdxLibNull.c is missing in the patch.
Thanks,
Ray
________________________________
From: Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>
Sent: Friday, October 27, 2023 1:42 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Subject: [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance
Create null instance for TdxLib. The IoLib instance
BaseIoLibIntrinsic.inf will consume the TdxLibNull
to pass build.
Signed-off-by: Dun Tan <dun.tan@intel.com<mailto:dun.tan@intel.com>>
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Zhiguang Liu <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
---
MdePkg/Library/TdxLib/TdxLibNull.inf | 21 +++++++++++++++++++++
MdePkg/MdePkg.dsc | 1 +
2 files changed, 22 insertions(+)
diff --git a/MdePkg/Library/TdxLib/TdxLibNull.inf b/MdePkg/Library/TdxLib/TdxLibNull.inf
new file mode 100644
index 0000000000..ecf20e12e1
--- /dev/null
+++ b/MdePkg/Library/TdxLib/TdxLibNull.inf
@@ -0,0 +1,21 @@
+## @file
+# Tdx null instance.
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = TdxLibNull
+ FILE_GUID = AA75759C-24E0-42FE-B5A4-83678548FCEF
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxLib
+
+[Sources]
+ TdxLibNull.c
+
+[Packages]
+ MdePkg/MdePkg.dec
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 3abd1a1e23..a1be123d80 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -184,6 +184,7 @@
MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
MdePkg/Library/TdxLib/TdxLib.inf
+ MdePkg/Library/TdxLib/TdxLibNull.inf
MdePkg/Library/MipiSysTLib/MipiSysTLib.inf
MdePkg/Library/TraceHubDebugSysTLibNull/TraceHubDebugSysTLibNull.inf
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110195): https://edk2.groups.io/g/devel/message/110195
Mute This Topic: https://groups.io/mt/102215662/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 10214 bytes --]
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-27 6:31 ` duntan
@ 2023-10-27 7:06 ` Yao, Jiewen
2023-10-27 7:34 ` duntan
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2023-10-27 7:06 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io
Here is my suggestion:
1) Please perform the test to ensure the functional part is correct.
Without that, how can people know you are doing things right?
2) If you do not run any test, before you send out patch, please call out that clearly.
That is important to reminder the maintainer: Don't merge, even if it pass review.
Otherwise, once the review passed, the maintainer may merge it.
I don't think that is the intention.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, October 27, 2023 2:32 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic
> and remove BaseIoLibIntrinsicSev
>
> Hi Jiewen,
>
> Currently I'm working on the Tdx test. Since the patch set doesn't change the
> code logic when Tdx or SEV is enabled, so I want to send out the patch as soon as
> possible to see if there is any comments from community.
>
> I will include AMD SEV reviewer in this patch series. Thanks for reminding.
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, October 27, 2023 1:49 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic
> and remove BaseIoLibIntrinsicSev
>
> HI
> Since this impact TDX and SEV, would you please let me know what kind of test
> you have done?
> Have you validated TDX and SEV before you submit the patch? Please describe
> that clearly in your patch description.
>
> Also please include AMD SEV reviewer in this patch series.
>
> Thank you
> Yao, Jiewen
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> > Sent: Friday, October 27, 2023 1:43 PM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> > BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
> >
> > The goal is to have single BaseIoLibIntrinsic instance that can also
> > used for sev and Tdx.
> > In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
> > Then change the source file of BaseIoLibIntrinsic to also support Tdx
> > and sev feature. So BaseIoLibIntrinsicSev and related assembly code can be
> removed.
> >
> > Dun Tan (7):
> > MdePkg: Create TdxLibNull.inf instance
> > MdePkg: Add CcProbeLibNull and TdxLibNull implement
> > MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
> > MdePkg:support Tdx and sev in BaseIoLibIntrinsic
> > OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
> > OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
> > MdePkg:remove BaseIoLibIntrinsicSev related code
> >
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++---
> -
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61
> > ------------------
> > -------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 -----------------
> ---
> > ----------------------------------------------------------------------
> > ----------------------------
> > -------------
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 ---------------
> ---
> > ----------------------------------------------------------------------
> > ----------------------------
> > ----------------------------------------------------------------------
> > ----------------------------
> > -------------------------------------------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
> > +++++++++++++++++++++++++++++++++++++--------
> > MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ----------------------
> --
> > ----------------------------------------------------------------------
> > ----------------------------
> > --------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 ------------------
> --
> > ----------------------------------------------------------------------
> > ----------------------------
> > --
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ----------------
> --
> > ----------------------------------------------------------------------
> > ----------------------------
> > ----------------------------------------------------------------------
> > ----------------------------
> > --------------------------------------------------------------------
> > MdePkg/Library/TdxLib/TdxLibNull.inf | 21
> > +++++++++++++++++++++
> > MdePkg/MdeLibs.dsc.inc | 4 +++-
> > MdePkg/MdePkg.dsc | 2 +-
> > OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
> > OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
> > OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
> > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
> > OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
> > OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
> > OvmfPkg/OvmfPkgIa32.dsc | 2 +-
> > OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
> > OvmfPkg/OvmfPkgX64.dsc | 2 +-
> > OvmfPkg/OvmfXen.dsc | 2 +-
> > 21 files changed, 83 insertions(+), 1077 deletions(-) delete mode
> > 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> > delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
> > delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
> > delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
> > create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110196): https://edk2.groups.io/g/devel/message/110196
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-27 7:06 ` Yao, Jiewen
@ 2023-10-27 7:34 ` duntan
2023-10-27 8:05 ` duntan
0 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-10-27 7:34 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io
Thanks for the suggestion.
I'll update the test result once I finished the test. Also the abstract message in this patch has been modified to mention that this patch should not be merged now.
Thanks,
Dun
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, October 27, 2023 3:07 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
Here is my suggestion:
1) Please perform the test to ensure the functional part is correct.
Without that, how can people know you are doing things right?
2) If you do not run any test, before you send out patch, please call out that clearly.
That is important to reminder the maintainer: Don't merge, even if it pass review.
Otherwise, once the review passed, the maintainer may merge it.
I don't think that is the intention.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, October 27, 2023 2:32 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>
> Hi Jiewen,
>
> Currently I'm working on the Tdx test. Since the patch set doesn't
> change the code logic when Tdx or SEV is enabled, so I want to send
> out the patch as soon as possible to see if there is any comments from community.
>
> I will include AMD SEV reviewer in this patch series. Thanks for reminding.
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, October 27, 2023 1:49 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>
> HI
> Since this impact TDX and SEV, would you please let me know what kind
> of test you have done?
> Have you validated TDX and SEV before you submit the patch? Please
> describe that clearly in your patch description.
>
> Also please include AMD SEV reviewer in this patch series.
>
> Thank you
> Yao, Jiewen
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > duntan
> > Sent: Friday, October 27, 2023 1:43 PM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> > BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev (Don't merge because the test hasn't been completed yet.)
> >
> > The goal is to have single BaseIoLibIntrinsic instance that can also
> > used for sev and Tdx.
> > In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
> > Then change the source file of BaseIoLibIntrinsic to also support
> > Tdx and sev feature. So BaseIoLibIntrinsicSev and related assembly
> > code can be
> removed.
> >
> > Dun Tan (7):
> > MdePkg: Create TdxLibNull.inf instance
> > MdePkg: Add CcProbeLibNull and TdxLibNull implement
> > MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
> > MdePkg:support Tdx and sev in BaseIoLibIntrinsic
> > OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
> > OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
> > MdePkg:remove BaseIoLibIntrinsicSev related code
> >
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++---
> -
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61
> > ------------------
> > -------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 -----------------
> ---
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > -------------
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 ---------------
> ---
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > -------------------------------------------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
> > +++++++++++++++++++++++++++++++++++++--------
> > MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ----------------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 ------------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ----------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > MdePkg/Library/TdxLib/TdxLibNull.inf | 21
> > +++++++++++++++++++++
> > MdePkg/MdeLibs.dsc.inc | 4 +++-
> > MdePkg/MdePkg.dsc | 2 +-
> > OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
> > OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
> > OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
> > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
> > OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
> > OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
> > OvmfPkg/OvmfPkgIa32.dsc | 2 +-
> > OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
> > OvmfPkg/OvmfPkgX64.dsc | 2 +-
> > OvmfPkg/OvmfXen.dsc | 2 +-
> > 21 files changed, 83 insertions(+), 1077 deletions(-) delete mode
> > 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
> > delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
> > create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110198): https://edk2.groups.io/g/devel/message/110198
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-27 7:34 ` duntan
@ 2023-10-27 8:05 ` duntan
2023-10-27 21:31 ` Lendacky, Thomas via groups.io
0 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-10-27 8:05 UTC (permalink / raw)
To: devel@edk2.groups.io, leo.duran@amd.com, brijesh.singh@amd.com,
Chang, Abner, Tom Lendacky, michael.roth@amd.com,
Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
Hi all,
Could you please help to review this patch set? In this patch set, the IoLib instance BaseIoLibIntrinsic is modified to support AMD SEV feature and the BaseIoLibIntrinsicSev is removed.
Also could you help to do a test on AMD processor to make sure that the SEV feature still works good with this patch set?
Thanks,
Dun
-----Original Message-----
From: Tan, Dun
Sent: Friday, October 27, 2023 3:35 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
Thanks for the suggestion.
I'll update the test result once I finished the test. Also the abstract message in this patch has been modified to mention that this patch should not be merged now.
Thanks,
Dun
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, October 27, 2023 3:07 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
Here is my suggestion:
1) Please perform the test to ensure the functional part is correct.
Without that, how can people know you are doing things right?
2) If you do not run any test, before you send out patch, please call out that clearly.
That is important to reminder the maintainer: Don't merge, even if it pass review.
Otherwise, once the review passed, the maintainer may merge it.
I don't think that is the intention.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, October 27, 2023 2:32 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>
> Hi Jiewen,
>
> Currently I'm working on the Tdx test. Since the patch set doesn't
> change the code logic when Tdx or SEV is enabled, so I want to send
> out the patch as soon as possible to see if there is any comments from community.
>
> I will include AMD SEV reviewer in this patch series. Thanks for reminding.
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, October 27, 2023 1:49 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>
> HI
> Since this impact TDX and SEV, would you please let me know what kind
> of test you have done?
> Have you validated TDX and SEV before you submit the patch? Please
> describe that clearly in your patch description.
>
> Also please include AMD SEV reviewer in this patch series.
>
> Thank you
> Yao, Jiewen
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > duntan
> > Sent: Friday, October 27, 2023 1:43 PM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
> > BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev (Don't merge
> > because the test hasn't been completed yet.)
> >
> > The goal is to have single BaseIoLibIntrinsic instance that can also
> > used for sev and Tdx.
> > In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
> > Then change the source file of BaseIoLibIntrinsic to also support
> > Tdx and sev feature. So BaseIoLibIntrinsicSev and related assembly
> > code can be
> removed.
> >
> > Dun Tan (7):
> > MdePkg: Create TdxLibNull.inf instance
> > MdePkg: Add CcProbeLibNull and TdxLibNull implement
> > MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
> > MdePkg:support Tdx and sev in BaseIoLibIntrinsic
> > OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
> > OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
> > MdePkg:remove BaseIoLibIntrinsicSev related code
> >
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++---
> -
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61
> > ------------------
> > -------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 -----------------
> ---
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > -------------
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 ---------------
> ---
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > -------------------------------------------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
> > +++++++++++++++++++++++++++++++++++++--------
> > MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ----------------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 ------------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ----------------
> --
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > --
> > ----------------------------
> > --------------------------------------------------------------------
> > MdePkg/Library/TdxLib/TdxLibNull.inf | 21
> > +++++++++++++++++++++
> > MdePkg/MdeLibs.dsc.inc | 4 +++-
> > MdePkg/MdePkg.dsc | 2 +-
> > OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
> > OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
> > OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
> > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
> > OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
> > OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
> > OvmfPkg/OvmfPkgIa32.dsc | 2 +-
> > OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
> > OvmfPkg/OvmfPkgX64.dsc | 2 +-
> > OvmfPkg/OvmfXen.dsc | 2 +-
> > 21 files changed, 83 insertions(+), 1077 deletions(-) delete mode
> > 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
> > delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
> > delete mode 100644
> > MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
> > create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110199): https://edk2.groups.io/g/devel/message/110199
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-27 8:05 ` duntan
@ 2023-10-27 21:31 ` Lendacky, Thomas via groups.io
2023-10-28 11:40 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Lendacky, Thomas via groups.io @ 2023-10-27 21:31 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io, leo.duran@amd.com,
brijesh.singh@amd.com, Chang, Abner, michael.roth@amd.com,
Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
On 10/27/23 03:05, Tan, Dun wrote:
> Hi all,
>
> Could you please help to review this patch set? In this patch set, the IoLib instance BaseIoLibIntrinsic is modified to support AMD SEV feature and the BaseIoLibIntrinsicSev is removed.
> Also could you help to do a test on AMD processor to make sure that the SEV feature still works good with this patch set?
I was able to test SEV, SEV-ES and SEV-SNP guests successfully at each
step of the patchset.
However, you are unrolling the string I/O for everyone, now, not just SEV
guests. Is that acceptable to the community? I think there need to be
comments in IoLibFifo.c around the new code about why the access is
unrolled/looping so that someone down the road doesn't come along and try
to use string I/O again.
From a commit message standpoint, you have up to 74 characters per line
to use and I see most of your messages do not make use of that. Also, you
use sev when it should be SEV. Using SEV will make grep'ing commit
messages simpler.
Thanks,
Tom
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: Tan, Dun
> Sent: Friday, October 27, 2023 3:35 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>
> Thanks for the suggestion.
> I'll update the test result once I finished the test. Also the abstract message in this patch has been modified to mention that this patch should not be merged now.
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, October 27, 2023 3:07 PM
> To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>
> Here is my suggestion:
>
> 1) Please perform the test to ensure the functional part is correct.
>
> Without that, how can people know you are doing things right?
>
> 2) If you do not run any test, before you send out patch, please call out that clearly.
> That is important to reminder the maintainer: Don't merge, even if it pass review.
>
> Otherwise, once the review passed, the maintainer may merge it.
> I don't think that is the intention.
>
>
>
> Thank you
> Yao, Jiewen
>
>
>> -----Original Message-----
>> From: Tan, Dun <dun.tan@intel.com>
>> Sent: Friday, October 27, 2023 2:32 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>
>> Hi Jiewen,
>>
>> Currently I'm working on the Tdx test. Since the patch set doesn't
>> change the code logic when Tdx or SEV is enabled, so I want to send
>> out the patch as soon as possible to see if there is any comments from community.
>>
>> I will include AMD SEV reviewer in this patch series. Thanks for reminding.
>>
>> Thanks,
>> Dun
>>
>> -----Original Message-----
>> From: Yao, Jiewen <jiewen.yao@intel.com>
>> Sent: Friday, October 27, 2023 1:49 PM
>> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>
>> HI
>> Since this impact TDX and SEV, would you please let me know what kind
>> of test you have done?
>> Have you validated TDX and SEV before you submit the patch? Please
>> describe that clearly in your patch description.
>>
>> Also please include AMD SEV reviewer in this patch series.
>>
>> Thank you
>> Yao, Jiewen
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>>> duntan
>>> Sent: Friday, October 27, 2023 1:43 PM
>>> To: devel@edk2.groups.io
>>> Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev (Don't merge
>>> because the test hasn't been completed yet.)
>>>
>>> The goal is to have single BaseIoLibIntrinsic instance that can also
>>> used for sev and Tdx.
>>> In this patch set, string I/O instructions are deleted in IoRead/WriteFifo API.
>>> Then change the source file of BaseIoLibIntrinsic to also support
>>> Tdx and sev feature. So BaseIoLibIntrinsicSev and related assembly
>>> code can be
>> removed.
>>>
>>> Dun Tan (7):
>>> MdePkg: Create TdxLibNull.inf instance
>>> MdePkg: Add CcProbeLibNull and TdxLibNull implement
>>> MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
>>> MdePkg:support Tdx and sev in BaseIoLibIntrinsic
>>> OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
>>> OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
>>> MdePkg:remove BaseIoLibIntrinsicSev related code
>>>
>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14 ++++++++++---
>> -
>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61
>>> ------------------
>>> -------------------------------------------
>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131 -----------------
>> ---
>>> --------------------------------------------------------------------
>>> --
>>> ----------------------------
>>> -------------
>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 ---------------
>> ---
>>> --------------------------------------------------------------------
>>> --
>>> ----------------------------
>>> --------------------------------------------------------------------
>>> --
>>> ----------------------------
>>> -------------------------------------------------------------------------------
>>> MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
>>> +++++++++++++++++++++++++++++++++++++--------
>>> MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166 ----------------------
>> --
>>> --------------------------------------------------------------------
>>> --
>>> ----------------------------
>>> --------------------------------------------
>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120 ------------------
>> --
>>> --------------------------------------------------------------------
>>> --
>>> ----------------------------
>>> --
>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282 ----------------
>> --
>>> --------------------------------------------------------------------
>>> --
>>> ----------------------------
>>> --------------------------------------------------------------------
>>> --
>>> ----------------------------
>>> --------------------------------------------------------------------
>>> MdePkg/Library/TdxLib/TdxLibNull.inf | 21
>>> +++++++++++++++++++++
>>> MdePkg/MdeLibs.dsc.inc | 4 +++-
>>> MdePkg/MdePkg.dsc | 2 +-
>>> OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
>>> OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
>>> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
>>> OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
>>> OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
>>> OvmfPkg/OvmfPkgIa32.dsc | 2 +-
>>> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
>>> OvmfPkg/OvmfPkgX64.dsc | 2 +-
>>> OvmfPkg/OvmfXen.dsc | 2 +-
>>> 21 files changed, 83 insertions(+), 1077 deletions(-) delete mode
>>> 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>> delete mode 100644
>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
>>> delete mode 100644
>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
>>> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
>>> delete mode 100644
>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
>>> delete mode 100644
>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>>> create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
>>>
>>> --
>>> 2.31.1.windows.1
>>>
>>>
>>>
>>>
>>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110227): https://edk2.groups.io/g/devel/message/110227
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-27 21:31 ` Lendacky, Thomas via groups.io
@ 2023-10-28 11:40 ` Laszlo Ersek
2023-10-31 9:56 ` duntan
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2023-10-28 11:40 UTC (permalink / raw)
To: devel, thomas.lendacky, Tan, Dun, leo.duran@amd.com,
brijesh.singh@amd.com, Chang, Abner, michael.roth@amd.com,
Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
On 10/27/23 23:31, Lendacky, Thomas via groups.io wrote:
> On 10/27/23 03:05, Tan, Dun wrote:
>> Hi all,
>>
>> Could you please help to review this patch set? In this patch set, the
>> IoLib instance BaseIoLibIntrinsic is modified to support AMD SEV
>> feature and the BaseIoLibIntrinsicSev is removed.
>> Also could you help to do a test on AMD processor to make sure that
>> the SEV feature still works good with this patch set?
>
> I was able to test SEV, SEV-ES and SEV-SNP guests successfully at each
> step of the patchset.
>
> However, you are unrolling the string I/O for everyone, now, not just
> SEV guests. Is that acceptable to the community?
Thank you for making this comment, Tom, because this is exactly what I
meant to raise immediately, upon reading the cover letter.
No, it is not acceptable.
The FIFO variants exist for a reason. When the guest performs multiple
individual IO Port accesses, those translate to individual traps to the
hypervisor, with significant performance impact. When IO Port string
operations are used instead, with the REP prefix, then there is just
*one* trap, and the hypervisor can perform the whole "string" transfer
in one go (up to a page size, anyways, IIRC). This has very visible
impact on OVMF debug output (via the isa-debugcon QEMU device), and/or
in case fw_cfg is used without DMA support.
(If you search OvmfPkg for IoReadFifo8 and IoWriteFifo8, you'll find the
QemuFwCfgLib and PlatformDebugLibIoPort libraries using them.)
In fact, during initial SEV enablement, the SEV enlightenment was
introduced because SEV does not handle the REP prefix with these
instructions, and so a fallback had to be added.
See commits b6d11d7c4678 ("MdePkg: BaseIoLibIntrinsic (IoLib class)
library", 2017-04-13) and 97353a9c914d ("OvmfPkg: Update dsc to use
IoLib from BaseIoLibIntrinsicSev.inf", 2017-07-10).
Accordingly, there's a *huge* performance (boot time) impact when you
boot OVMF in a SEV guest with DEBUG_VERBOSE messages enabled (and
captured; i.e., when the isa-debugcon device is active).
> I think there need to
> be comments in IoLibFifo.c around the new code about why the access is
> unrolled/looping so that someone down the road doesn't come along and
> try to use string I/O again.
String IO must be preserved for such guests that don't run in
Confidential Virtual Machines ("CVM"s).
In particular patches #6 and #7 would damage OVMF.
Nacked-by: Laszlo Ersek <lersek@redhat.com>
Laszlo
>
> From a commit message standpoint, you have up to 74 characters per line
> to use and I see most of your messages do not make use of that. Also,
> you use sev when it should be SEV. Using SEV will make grep'ing commit
> messages simpler.
>
> Thanks,
> Tom
>
>>
>> Thanks,
>> Dun
>>
>> -----Original Message-----
>> From: Tan, Dun
>> Sent: Friday, October 27, 2023 3:35 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>
>> Thanks for the suggestion.
>> I'll update the test result once I finished the test. Also the
>> abstract message in this patch has been modified to mention that this
>> patch should not be merged now.
>>
>> Thanks,
>> Dun
>>
>> -----Original Message-----
>> From: Yao, Jiewen <jiewen.yao@intel.com>
>> Sent: Friday, October 27, 2023 3:07 PM
>> To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>
>> Here is my suggestion:
>>
>> 1) Please perform the test to ensure the functional part is correct.
>>
>> Without that, how can people know you are doing things right?
>>
>> 2) If you do not run any test, before you send out patch, please call
>> out that clearly.
>> That is important to reminder the maintainer: Don't merge, even if it
>> pass review.
>>
>> Otherwise, once the review passed, the maintainer may merge it.
>> I don't think that is the intention.
>>
>>
>>
>> Thank you
>> Yao, Jiewen
>>
>>> -----Original Message-----
>>> From: Tan, Dun <dun.tan@intel.com>
>>> Sent: Friday, October 27, 2023 2:32 PM
>>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>>
>>> Hi Jiewen,
>>>
>>> Currently I'm working on the Tdx test. Since the patch set doesn't
>>> change the code logic when Tdx or SEV is enabled, so I want to send
>>> out the patch as soon as possible to see if there is any comments
>>> from community.
>>>
>>> I will include AMD SEV reviewer in this patch series. Thanks for
>>> reminding.
>>>
>>> Thanks,
>>> Dun
>>>
>>> -----Original Message-----
>>> From: Yao, Jiewen <jiewen.yao@intel.com>
>>> Sent: Friday, October 27, 2023 1:49 PM
>>> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
>>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>>
>>> HI
>>> Since this impact TDX and SEV, would you please let me know what kind
>>> of test you have done?
>>> Have you validated TDX and SEV before you submit the patch? Please
>>> describe that clearly in your patch description.
>>>
>>> Also please include AMD SEV reviewer in this patch series.
>>>
>>> Thank you
>>> Yao, Jiewen
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>>>> duntan
>>>> Sent: Friday, October 27, 2023 1:43 PM
>>>> To: devel@edk2.groups.io
>>>> Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>>>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev (Don't merge
>>>> because the test hasn't been completed yet.)
>>>>
>>>> The goal is to have single BaseIoLibIntrinsic instance that can also
>>>> used for sev and Tdx.
>>>> In this patch set, string I/O instructions are deleted in
>>>> IoRead/WriteFifo API.
>>>> Then change the source file of BaseIoLibIntrinsic to also support
>>>> Tdx and sev feature. So BaseIoLibIntrinsicSev and related assembly
>>>> code can be
>>> removed.
>>>>
>>>> Dun Tan (7):
>>>> MdePkg: Create TdxLibNull.inf instance
>>>> MdePkg: Add CcProbeLibNull and TdxLibNull implement
>>>> MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
>>>> MdePkg:support Tdx and sev in BaseIoLibIntrinsic
>>>> OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
>>>> OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
>>>> MdePkg:remove BaseIoLibIntrinsicSev related code
>>>>
>>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14
>>>> ++++++++++---
>>> -
>>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61
>>>> ------------------
>>>> -------------------------------------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131
>>>> -----------------
>>> ---
>>>> --------------------------------------------------------------------
>>>> --
>>>> ----------------------------
>>>> -------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293
>>>> ---------------
>>> ---
>>>> --------------------------------------------------------------------
>>>> --
>>>> ----------------------------
>>>> --------------------------------------------------------------------
>>>> --
>>>> ----------------------------
>>>> -------------------------------------------------------------------------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
>>>> +++++++++++++++++++++++++++++++++++++--------
>>>> MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166
>>>> ----------------------
>>> --
>>>> --------------------------------------------------------------------
>>>> --
>>>> ----------------------------
>>>> --------------------------------------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120
>>>> ------------------
>>> --
>>>> --------------------------------------------------------------------
>>>> --
>>>> ----------------------------
>>>> --
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282
>>>> ----------------
>>> --
>>>> --------------------------------------------------------------------
>>>> --
>>>> ----------------------------
>>>> --------------------------------------------------------------------
>>>> --
>>>> ----------------------------
>>>> --------------------------------------------------------------------
>>>> MdePkg/Library/TdxLib/TdxLibNull.inf | 21
>>>> +++++++++++++++++++++
>>>> MdePkg/MdeLibs.dsc.inc | 4
>>>> +++-
>>>> MdePkg/MdePkg.dsc | 2 +-
>>>> OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
>>>> OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
>>>> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
>>>> OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
>>>> OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
>>>> OvmfPkg/OvmfPkgIa32.dsc | 2 +-
>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
>>>> OvmfPkg/OvmfPkgX64.dsc | 2 +-
>>>> OvmfPkg/OvmfXen.dsc | 2 +-
>>>> 21 files changed, 83 insertions(+), 1077 deletions(-) delete mode
>>>> 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
>>>> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>>>> create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
>>>>
>>>> --
>>>> 2.31.1.windows.1
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110242): https://edk2.groups.io/g/devel/message/110242
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
2023-10-27 5:42 ` [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files duntan
@ 2023-10-30 14:36 ` Ard Biesheuvel
2023-10-31 12:30 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2023-10-30 14:36 UTC (permalink / raw)
To: devel, dun.tan
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Ray Ni
On Fri, 27 Oct 2023 at 07:43, duntan <dun.tan@intel.com> wrote:
>
> Use BaseIoLibIntrinsic.inf in dsc files. The
> BaseIoLibIntrinsic supports Tdx and sev now.
> The BaseIoLibIntrinsicSev will be removed in
> the following patches.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
> OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
> OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
> OvmfPkg/OvmfPkgX64.dsc | 2 +-
> OvmfPkg/OvmfXen.dsc | 2 +-
> 9 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index 302c90e7c2..06086e0cc5 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -142,7 +142,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
> index 6693342c5f..1376dd7468 100644
> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
> @@ -149,7 +149,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
> index 35942e02df..1d2a13dd92 100644
> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
> @@ -159,7 +159,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> index 182ec3705d..6623196c8b 100644
> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> @@ -146,7 +146,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 0f26f2a9a9..c76a135f9e 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
> @@ -157,7 +157,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index fcd3a3fda5..a850465e77 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -162,7 +162,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index d0ae0b996d..9c6615b487 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -167,7 +167,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index a6811eee55..9e054aa489 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -179,7 +179,7 @@
> PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index ccd3a873c7..ab4bdb2628 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -150,7 +150,7 @@
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> --
> 2.31.1.windows.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110320): https://edk2.groups.io/g/devel/message/110320
Mute This Topic: https://groups.io/mt/102215668/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 5/7] OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
2023-10-27 5:42 ` [edk2-devel] [PATCH 5/7] OvmfPkg: Add CcProbeLib in PlatformInitLib.inf duntan
@ 2023-10-30 14:37 ` Ard Biesheuvel
0 siblings, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2023-10-30 14:37 UTC (permalink / raw)
To: Dun Tan
Cc: devel, Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann,
Ray Ni
On Fri, 27 Oct 2023 at 07:43, Dun Tan <dun.tan@intel.com> wrote:
>
> PlatformInitLib uses the CcProbe API in MemDetect.c
> but doensn't add CcProbeLib in .inf LibraryClasses
> section. Add CcProbeLib to fix the build error.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
> index 5a79d95b68..8ff9e699b1 100644
> --- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
> +++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
> @@ -2,7 +2,7 @@
> # Platform Initialization Lib
> #
> # This module provides platform specific function to detect boot mode.
> -# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -52,6 +52,7 @@
> PcdLib
> PciLib
> PeiHardwareInfoLib
> + CcProbeLib
>
> [LibraryClasses.X64]
> TdxLib
> --
> 2.31.1.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110321): https://edk2.groups.io/g/devel/message/110321
Mute This Topic: https://groups.io/mt/102215667/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-28 11:40 ` Laszlo Ersek
@ 2023-10-31 9:56 ` duntan
2023-10-31 17:06 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-10-31 9:56 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io, thomas.lendacky@amd.com,
leo.duran@amd.com, brijesh.singh@amd.com, Chang, Abner,
michael.roth@amd.com, Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
Hi Tom,
Thanks for your testing and comments. I'll modify the patch commit message and code according to your comments.
Yes the patch changes the behavior for IoReadFifo/ IoWriteFifo API for non-Tdx and non-SEV case. Looking forward to more comments from the community.
Hi Laszlo,
May I know the actually performance gap data in the case that you mentioned?
Also, if a DEBUG binary is used, I think the boot time impact might be acceptable?
Thanks,
Dun
-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Saturday, October 28, 2023 7:40 PM
To: devel@edk2.groups.io; thomas.lendacky@amd.com; Tan, Dun <dun.tan@intel.com>; leo.duran@amd.com; brijesh.singh@amd.com; Chang, Abner <abner.chang@amd.com>; michael.roth@amd.com; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Ni, Ray <ray.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
On 10/27/23 23:31, Lendacky, Thomas via groups.io wrote:
> On 10/27/23 03:05, Tan, Dun wrote:
>> Hi all,
>>
>> Could you please help to review this patch set? In this patch set,
>> the IoLib instance BaseIoLibIntrinsic is modified to support AMD SEV
>> feature and the BaseIoLibIntrinsicSev is removed.
>> Also could you help to do a test on AMD processor to make sure that
>> the SEV feature still works good with this patch set?
>
> I was able to test SEV, SEV-ES and SEV-SNP guests successfully at each
> step of the patchset.
>
> However, you are unrolling the string I/O for everyone, now, not just
> SEV guests. Is that acceptable to the community?
Thank you for making this comment, Tom, because this is exactly what I meant to raise immediately, upon reading the cover letter.
No, it is not acceptable.
The FIFO variants exist for a reason. When the guest performs multiple individual IO Port accesses, those translate to individual traps to the hypervisor, with significant performance impact. When IO Port string operations are used instead, with the REP prefix, then there is just
*one* trap, and the hypervisor can perform the whole "string" transfer in one go (up to a page size, anyways, IIRC). This has very visible impact on OVMF debug output (via the isa-debugcon QEMU device), and/or in case fw_cfg is used without DMA support.
(If you search OvmfPkg for IoReadFifo8 and IoWriteFifo8, you'll find the QemuFwCfgLib and PlatformDebugLibIoPort libraries using them.)
In fact, during initial SEV enablement, the SEV enlightenment was introduced because SEV does not handle the REP prefix with these instructions, and so a fallback had to be added.
See commits b6d11d7c4678 ("MdePkg: BaseIoLibIntrinsic (IoLib class) library", 2017-04-13) and 97353a9c914d ("OvmfPkg: Update dsc to use IoLib from BaseIoLibIntrinsicSev.inf", 2017-07-10).
Accordingly, there's a *huge* performance (boot time) impact when you boot OVMF in a SEV guest with DEBUG_VERBOSE messages enabled (and captured; i.e., when the isa-debugcon device is active).
> I think there need to
> be comments in IoLibFifo.c around the new code about why the access is
> unrolled/looping so that someone down the road doesn't come along and
> try to use string I/O again.
String IO must be preserved for such guests that don't run in Confidential Virtual Machines ("CVM"s).
In particular patches #6 and #7 would damage OVMF.
Nacked-by: Laszlo Ersek <lersek@redhat.com>
Laszlo
>
> From a commit message standpoint, you have up to 74 characters per
> line to use and I see most of your messages do not make use of that.
> Also, you use sev when it should be SEV. Using SEV will make grep'ing
> commit messages simpler.
>
> Thanks,
> Tom
>
>>
>> Thanks,
>> Dun
>>
>> -----Original Message-----
>> From: Tan, Dun
>> Sent: Friday, October 27, 2023 3:35 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>
>> Thanks for the suggestion.
>> I'll update the test result once I finished the test. Also the
>> abstract message in this patch has been modified to mention that this
>> patch should not be merged now.
>>
>> Thanks,
>> Dun
>>
>> -----Original Message-----
>> From: Yao, Jiewen <jiewen.yao@intel.com>
>> Sent: Friday, October 27, 2023 3:07 PM
>> To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>
>> Here is my suggestion:
>>
>> 1) Please perform the test to ensure the functional part is correct.
>>
>> Without that, how can people know you are doing things right?
>>
>> 2) If you do not run any test, before you send out patch, please call
>> out that clearly.
>> That is important to reminder the maintainer: Don't merge, even if it
>> pass review.
>>
>> Otherwise, once the review passed, the maintainer may merge it.
>> I don't think that is the intention.
>>
>>
>>
>> Thank you
>> Yao, Jiewen
>>
>>> -----Original Message-----
>>> From: Tan, Dun <dun.tan@intel.com>
>>> Sent: Friday, October 27, 2023 2:32 PM
>>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>>
>>> Hi Jiewen,
>>>
>>> Currently I'm working on the Tdx test. Since the patch set doesn't
>>> change the code logic when Tdx or SEV is enabled, so I want to send
>>> out the patch as soon as possible to see if there is any comments
>>> from community.
>>>
>>> I will include AMD SEV reviewer in this patch series. Thanks for
>>> reminding.
>>>
>>> Thanks,
>>> Dun
>>>
>>> -----Original Message-----
>>> From: Yao, Jiewen <jiewen.yao@intel.com>
>>> Sent: Friday, October 27, 2023 1:49 PM
>>> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
>>> Subject: RE: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
>>>
>>> HI
>>> Since this impact TDX and SEV, would you please let me know what
>>> kind of test you have done?
>>> Have you validated TDX and SEV before you submit the patch? Please
>>> describe that clearly in your patch description.
>>>
>>> Also please include AMD SEV reviewer in this patch series.
>>>
>>> Thank you
>>> Yao, Jiewen
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>>>> duntan
>>>> Sent: Friday, October 27, 2023 1:43 PM
>>>> To: devel@edk2.groups.io
>>>> Subject: [edk2-devel] [PATCH 0/7] Support Tdx and sev in
>>>> BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev (Don't merge
>>>> because the test hasn't been completed yet.)
>>>>
>>>> The goal is to have single BaseIoLibIntrinsic instance that can
>>>> also used for sev and Tdx.
>>>> In this patch set, string I/O instructions are deleted in
>>>> IoRead/WriteFifo API.
>>>> Then change the source file of BaseIoLibIntrinsic to also support
>>>> Tdx and sev feature. So BaseIoLibIntrinsicSev and related assembly
>>>> code can be
>>> removed.
>>>>
>>>> Dun Tan (7):
>>>> MdePkg: Create TdxLibNull.inf instance
>>>> MdePkg: Add CcProbeLibNull and TdxLibNull implement
>>>> MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c
>>>> MdePkg:support Tdx and sev in BaseIoLibIntrinsic
>>>> OvmfPkg: Add CcProbeLib in PlatformInitLib.inf
>>>> OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
>>>> MdePkg:remove BaseIoLibIntrinsicSev related code
>>>>
>>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf | 14
>>>> ++++++++++---
>>> -
>>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf | 61
>>>> ------------------
>>>> -------------------------------------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm | 131
>>>> -----------------
>>> ---
>>>> -------------------------------------------------------------------
>>>> -
>>>> --
>>>> ----------------------------
>>>> -------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293
>>>> ---------------
>>> ---
>>>> -------------------------------------------------------------------
>>>> -
>>>> --
>>>> ----------------------------
>>>> -------------------------------------------------------------------
>>>> -
>>>> --
>>>> ----------------------------
>>>> -------------------------------------------------------------------
>>>> ------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/IoLibFifo.c | 45
>>>> +++++++++++++++++++++++++++++++++++++--------
>>>> MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h | 166
>>>> ----------------------
>>> --
>>>> -------------------------------------------------------------------
>>>> -
>>>> --
>>>> ----------------------------
>>>> --------------------------------------------
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm | 120
>>>> ------------------
>>> --
>>>> -------------------------------------------------------------------
>>>> -
>>>> --
>>>> ----------------------------
>>>> --
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm | 282
>>>> ----------------
>>> --
>>>> -------------------------------------------------------------------
>>>> -
>>>> --
>>>> ----------------------------
>>>> -------------------------------------------------------------------
>>>> -
>>>> --
>>>> ----------------------------
>>>> -------------------------------------------------------------------
>>>> -
>>>> MdePkg/Library/TdxLib/TdxLibNull.inf | 21
>>>> +++++++++++++++++++++
>>>> MdePkg/MdeLibs.dsc.inc | 4
>>>> +++-
>>>> MdePkg/MdePkg.dsc | 2
>>>> +-
>>>> OvmfPkg/AmdSev/AmdSevX64.dsc | 2
>>>> +-
>>>> OvmfPkg/Bhyve/BhyveX64.dsc | 2
>>>> +-
>>>> OvmfPkg/CloudHv/CloudHvX64.dsc | 2
>>>> +-
>>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2
>>>> +-
>>>> OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3
>>>> ++-
>>>> OvmfPkg/Microvm/MicrovmX64.dsc | 2
>>>> +-
>>>> OvmfPkg/OvmfPkgIa32.dsc | 2
>>>> +-
>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 2
>>>> +-
>>>> OvmfPkg/OvmfPkgX64.dsc | 2
>>>> +-
>>>> OvmfPkg/OvmfXen.dsc | 2
>>>> +-
>>>> 21 files changed, 83 insertions(+), 1077 deletions(-) delete
>>>> mode
>>>> 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.nasm
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
>>>> delete mode 100644 MdePkg/Library/BaseIoLibIntrinsic/IoLibSev.h
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.nasm
>>>> delete mode 100644
>>>> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>>>> create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.inf
>>>>
>>>> --
>>>> 2.31.1.windows.1
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110393): https://edk2.groups.io/g/devel/message/110393
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
2023-10-30 14:36 ` Ard Biesheuvel
@ 2023-10-31 12:30 ` Laszlo Ersek
2023-10-31 13:19 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2023-10-31 12:30 UTC (permalink / raw)
To: devel, ardb, dun.tan
Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Ray Ni
On 10/30/23 15:36, Ard Biesheuvel wrote:
> On Fri, 27 Oct 2023 at 07:43, duntan <dun.tan@intel.com> wrote:
>>
>> Use BaseIoLibIntrinsic.inf in dsc files. The
>> BaseIoLibIntrinsic supports Tdx and sev now.
>> The BaseIoLibIntrinsicSev will be removed in
>> the following patches.
>>
>> Signed-off-by: Dun Tan <dun.tan@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
While I agree that this patch *in itself* is (quite obviously) formally
correct, semantically it is wrong for OVMF; which is why I have NACKed
the whole series; see <https://edk2.groups.io/g/devel/message/110242>.
(Just highlighting it here too, lest it get lost in the review thread.)
Thanks,
Laszlo
>> ---
>> OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
>> OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
>> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 2 +-
>> OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
>> OvmfPkg/OvmfPkgIa32.dsc | 2 +-
>> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
>> OvmfPkg/OvmfPkgX64.dsc | 2 +-
>> OvmfPkg/OvmfXen.dsc | 2 +-
>> 9 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> index 302c90e7c2..06086e0cc5 100644
>> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
>> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> @@ -142,7 +142,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
>> index 6693342c5f..1376dd7468 100644
>> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
>> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
>> @@ -149,7 +149,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
>> index 35942e02df..1d2a13dd92 100644
>> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
>> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
>> @@ -159,7 +159,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> index 182ec3705d..6623196c8b 100644
>> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> @@ -146,7 +146,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
>> index 0f26f2a9a9..c76a135f9e 100644
>> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
>> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
>> @@ -157,7 +157,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index fcd3a3fda5..a850465e77 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -162,7 +162,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index d0ae0b996d..9c6615b487 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -167,7 +167,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index a6811eee55..9e054aa489 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -179,7 +179,7 @@
>> PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>> index ccd3a873c7..ab4bdb2628 100644
>> --- a/OvmfPkg/OvmfXen.dsc
>> +++ b/OvmfPkg/OvmfXen.dsc
>> @@ -150,7 +150,7 @@
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>> - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>> + IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>> SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
>> --
>> 2.31.1.windows.1
>>
>>
>>
>>
>>
>>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110407): https://edk2.groups.io/g/devel/message/110407
Mute This Topic: https://groups.io/mt/102215668/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files
2023-10-31 12:30 ` Laszlo Ersek
@ 2023-10-31 13:19 ` Ard Biesheuvel
0 siblings, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2023-10-31 13:19 UTC (permalink / raw)
To: Laszlo Ersek
Cc: devel, dun.tan, Ard Biesheuvel, Jiewen Yao, Jordan Justen,
Gerd Hoffmann, Ray Ni
On Tue, 31 Oct 2023 at 13:30, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 10/30/23 15:36, Ard Biesheuvel wrote:
> > On Fri, 27 Oct 2023 at 07:43, duntan <dun.tan@intel.com> wrote:
> >>
> >> Use BaseIoLibIntrinsic.inf in dsc files. The
> >> BaseIoLibIntrinsic supports Tdx and sev now.
> >> The BaseIoLibIntrinsicSev will be removed in
> >> the following patches.
> >>
> >> Signed-off-by: Dun Tan <dun.tan@intel.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> Cc: Ray Ni <ray.ni@intel.com>
> >
> > Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
>
> While I agree that this patch *in itself* is (quite obviously) formally
> correct, semantically it is wrong for OVMF; which is why I have NACKed
> the whole series; see <https://edk2.groups.io/g/devel/message/110242>.
>
> (Just highlighting it here too, lest it get lost in the review thread.)
>
Apologies, I missed that.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110410): https://edk2.groups.io/g/devel/message/110410
Mute This Topic: https://groups.io/mt/102215668/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-31 9:56 ` duntan
@ 2023-10-31 17:06 ` Laszlo Ersek
2023-10-31 22:36 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2023-10-31 17:06 UTC (permalink / raw)
To: devel, dun.tan, thomas.lendacky@amd.com, leo.duran@amd.com,
brijesh.singh@amd.com, Chang, Abner, michael.roth@amd.com,
Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
On 10/31/23 10:56, duntan wrote:
> Hi Tom,
> Thanks for your testing and comments. I'll modify the patch commit
> message and code according to your comments.
> Yes the patch changes the behavior for IoReadFifo/ IoWriteFifo API for
> non-Tdx and non-SEV case. Looking forward to more comments from the
> community.
>
> Hi Laszlo,
> May I know the actually performance gap data in the case that you
> mentioned?
Good call to have me measure this.
I cannot reproduce the performance hit *at all* with the isa-debugcon
device (that is, through the OVMF debug log).
I built OVMF X64 with PcdDebugPrintErrorLevel set to 0x8040004F, at
commit a671a14e63fd. That was the baseline. I built both NOOPT and
DEBUG.
Then I repeated the same, with your patches applied.
I cannot show any human-perceptible boot time difference.
Note that the NOOPT comparison excludes LTO (i.e., compiler advances) as
the potential factor that might mask the impact of your patches. I also
disassembled some binaries producing debug output, and indeed, without
your patches, we have rep outsb in them, and with your patches, we
don't. Yet, there is no performance difference.
This is striking news.
I want to understand why we have thought thus far that individual IO
Port traps are extremely expensive. I've now found evidence going back
as far as 2012 -- the year when I joined edk2!
Here are some facts:
(1) in commit f1ec65ba24e1 ("OvmfPkg: Add QemuFwCfgLib library class and
implementation", 2012-05-30) -- note the date! --, Jordan added the
initial implementation of QemuFwCfgLib. That version already used an
open-coded IoReadFifo8 function.
There was no particular explanation captured in the commit message.
I also happen to have the entire 2012 edk2-devel mailing list traffic
saved locally (well, the messages posted after I joined), and the
discussions around the various versions of the patch in question don't
seem to specifically justify the REP variant. So, we cannot conclude
much from this piece of evidence.
(2) Then I decided to search that entire year's traffic (i.e., 2012's)
for the word "fifo". I found relevant hits:
- In commit 1fd376d97922 ("PcAtChipsetPkg/PciHostBridgeDxe: Improve KVM
FIFO I/O read/write performance", 2012-06-01) -- note the year again!
--, Jordan brought the FIFO variants to the
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL implementation in edk2. The code was
duplicated.
Note that Jordan clearly stated in the commit message:
> KVM can substantially boost the speed of the rep insb/insw/insl
> and rep outsb/outsw/outsl instructions by transferring up to
> a page of data per VM trap.
- On the same day when Jordan posted the patch that would become
above-noted commit 1fd376d97922, Jordan also asked on the list
(subject "IoLib I/O FIFO read/write routines"):
> Regarding the routines now defined in PcAtChipsetPkg/PciHostBridgeDxe/IoFifo.h.
> IoReadFifo8, IoReadFifo16, IoReadFifo32,
> IoWriteFifo8, IoWriteFifo16, and IoWriteFifo32
>
> Would it make sense to consider adding these to MdePkg/Include/Library/IoLib.h?
Again, this was still in 2012.
So here we can conclude that Jordan saw some specific perf boost due to
employing the FIFO variants in EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL, but the
use case was not documented.
(3) During all this discussion here, I have been *distinctly* recalling
that I've experienced an *actual performance regression* sometime, after
we had unwittingly switched from the REP variant to the non-REP variant.
And now I have actually found it. I'm going to quote the commit message
in full:
> commit fb8b54694c53e73e1b6a098686e908f54f9bb7a9
> Author: Laszlo Ersek <lersek@redhat.com>
> Date: Thu Apr 7 22:28:38 2016 +0200
>
> UefiCpuPkg: CpuIo2Dxe: optimize FIFO reads and writes of IO ports
>
> * Short description:
>
> The CpuIoServiceRead() and CpuIoServiceWrite() functions transfer data
> between memory and IO ports with individual Io(Read|Write)(8|16|32)
> function calls, each in an appropriately set up loop.
>
> On the Ia32 and X64 platforms however, FIFO reads and writes can be
> optimized, by coding them in assembly, and delegating the loop to the
> CPU, with the REP prefix.
>
> On KVM virtualization hosts, this difference has a huge performance
> impact: if the loop is open-coded, then the virtual machine traps to the
> hypervisor on every single UINT8 / UINT16 / UINT32 transfer, whereas
> with the REP prefix, KVM can transfer up to a page of data per VM trap.
> This is especially noticeable with IDE PIO transfers, where all the data
> are squeezed through IO ports.
>
> * Long description:
>
> The RootBridgeIoIoRW() function in
>
> PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c
>
> used to have the exact same IO port acces optimization, dating back
> verbatim to commit 1fd376d9792:
>
> PcAtChipsetPkg/PciHostBridgeDxe: Improve KVM FIFO I/O read/write
> performance
>
> OvmfPkg cloned the "PcAtChipsetPkg/PciHostBridgeDxe" driver (for
> unrelated reasons), and inherited the optimization from PcAtChipsetPkg.
>
> The "PcAtChipsetPkg/PciHostBridgeDxe" driver was ultimately removed in
> commit 111d79db47:
>
> PcAtChipsetPkg/PciHostBridge: Remove PciHostBridge driver
>
> and OvmfPkg too was rebased to the new core Pci Host Bridge Driver, in
> commit 4014885ffd:
>
> OvmfPkg: switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe
>
> This caused the optimization to go lost. Namely, the
> RootBridgeIoIoRead() and RootBridgeIoIoWrite() functions in the new core
> Pci Host Bridge Driver delegate IO port accesses to
> EFI_CPU_IO2_PROTOCOL. And, in OvmfPkg (and likely most other Ia32 / X64
> edk2 platforms), this protocol is provided by "UefiCpuPkg/CpuIo2Dxe",
> which lacks the optimization.
>
> Therefore, this patch ports the C source code logic from commit
> 1fd376d9792 (see above) to "UefiCpuPkg/CpuIo2Dxe", plus it ports the
> NASM-converted assembly helper functions from OvmfPkg commits
> 6026bf460037 and ace1d0517b65:
>
> OvmfPkg PciHostBridgeDxe: Convert Ia32/IoFifo.asm to NASM
>
> OvmfPkg PciHostBridgeDxe: Convert X64/IoFifo.asm to NASM
>
> In order to support the MSFT and INTEL toolchains as well, the *.asm
> files are ported from OvmfPkg as well, immediately from before the above
> conversion (that is, at 6026bf460037^).
>
> * Notes about the port:
>
> - The write and read branches from commit 1fd376d9792 are split to the
> separate functions CpuIoServiceWrite() and CpuIoServiceRead().
>
> - The EfiPciWidthUintXX constants are replaced with EfiCpuIoWidthUintXX.
>
> - The cast expression "(UINTN) Address" is replaced with
> "(UINTN)Address" (i.e., no space), because that's how the receiving
> functions spell it as well.
>
> - The labels in the switch statements are unindented by one level, to
> match the edk2 coding style (and the rest of UefiCpuPkg) better.
>
> * The first signoff belongs to Jordan, because he authored all of
> 1fd376d9792, 6026bf460037 and ace1d0517b65.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> Ref: https://www.redhat.com/archives/vfio-users/2016-April/msg00029.html
> Reported-by: Mark <kram321@gmail.com>
> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/10424/focus=10432
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Mark <kram321@gmail.com>
> Tested-by: Mark <kram321@gmail.com>
> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
This is "hard evidence". Per bullet (2), Jordan had added the FIFO / REP
variants to EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL, for some (non-specific)
performance boost. and when that optimization was later lost, due to a
mistake, we saw IDE PIO performance *tank* on QEMU/KVM.
Therefore, I am now being reminded that the best performance regression
test for this particular series is to boot an operating system using IDE
(PIO) emulation, with OVMF! Because CpuIo2Dxe still calls the FIFO APIs.
That's what I'm going to do next -- later this week, if everything goes
well. I ask for your patience until I complete that regression test.
(4) In commit 19c6d9feaaf8 ("MdePkg: Expand BaseIoLibIntrinsic (IoLib
class) library", 2017-01-17), the duplicated FIFO variants were
upstreamed to the IoLib instance(s) in MdePkg.
Subsequently, commit b6d11d7c4678 ("MdePkg: BaseIoLibIntrinsic (IoLib
class) library", 2017-04-13) introduced the SEV fallback.
And then commit 97353a9c914d ("OvmfPkg: Update dsc to use IoLib from
BaseIoLibIntrinsicSev.inf", 2017-07-10) put the SEV fallback to use in
OVMF.
In commit 80886a695377 ("OvmfPkg/PlatformDebugLibIoPort: write messages
with IoWriteFifo8()", 2017-09-11), we switched PlatformDebugLibIoPort as
well over to the REP variant (which had been by then centralized). In
fact, using the REP fifo write in OVMF's DebugLib instance was
recommended by Jordan already in 2012! He wrote (while reviewing the
patch that would become commit b90aefa9e46c, "OvmfPkg: add support for
debug console on port 0x402", 2012-07-26):
> I'm not certain, but I think it might be worth-while to make the
> str-len call, and then use the I/O fifo call.
>
> It could reduce the trapping count by ~30x for writing a debug string,
> and we do write quite a lot of DEBUG... (I'm not sure it is enough to
> make a noticeable difference though.)
So that was what commit 80886a695377 implemented some five years later,
citing that "In the general case this speeds up logging".
(5) I also distinctly remember witnessing the debug logging speed
difference between SEV and non-SEV guests. (Well, this is not a "fact",
but a memory.)
In commit c09d9571300a ("OvmfPkg: save on I/O port accesses when the
debug port is not in use", 2017-11-17), we even quantified this. The
commit message says, "therefore, in SEV guests, the boot time impact is
huge (about 45 seconds _additional_ time spent writing to the debug
port)".
So, minimally commit fb8b54694c53 from bullet (3) is *hard evidence*
that this difference in performance used to exist, regardless of SEV.
As I said above, I will try to retest the same use case soon, with this
patch set. If the performance difference turns out to be invisible now,
then I can only chalk that up to QEMU/KVM technology having advanced a
lot -- and in that case, I will rescind my NACK.
To summarize, my operating memory was correct, and I wasn't wrong to
NACK this patch set; at the same time, you are also very correct to
point out that we need to measure *afresh*.
Back to your email:
On 10/31/23 10:56, duntan wrote:
> Also, if a DEBUG binary is used, I think the boot time impact might be
> acceptable?
We exclusively build and ship DEBUG binaries. RELEASE binaries contain
no ASSERTs. Many edk2 core modules -- unfortunately -- only employ
ASSERTs as error checks, so a RELEASE build means that those core
modules are built without error checks. Which is unacceptable.
(But even if all those modules had proper, explicit error checking, we'd
still want to preserve all the ASSERT()s.)
I'll report back later with the IDE PIO performance test results.
Thanks,
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110436): https://edk2.groups.io/g/devel/message/110436
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-31 17:06 ` Laszlo Ersek
@ 2023-10-31 22:36 ` Laszlo Ersek
2023-11-02 8:49 ` duntan
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2023-10-31 22:36 UTC (permalink / raw)
To: devel, dun.tan, thomas.lendacky@amd.com, leo.duran@amd.com,
brijesh.singh@amd.com, Chang, Abner, michael.roth@amd.com,
Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
On 10/31/23 18:06, Laszlo Ersek wrote:
> (3) During all this discussion here, I have been *distinctly*
> recalling that I've experienced an *actual performance regression*
> sometime, after we had unwittingly switched from the REP variant to
> the non-REP variant.
>
> And now I have actually found it. I'm going to quote the commit
> message in full:
>
>> commit fb8b54694c53e73e1b6a098686e908f54f9bb7a9
>> Author: Laszlo Ersek <lersek@redhat.com>
>> Date: Thu Apr 7 22:28:38 2016 +0200
>>
>> UefiCpuPkg: CpuIo2Dxe: optimize FIFO reads and writes of IO ports
>>
>> * Short description:
>>
>> The CpuIoServiceRead() and CpuIoServiceWrite() functions transfer data
>> between memory and IO ports with individual Io(Read|Write)(8|16|32)
>> function calls, each in an appropriately set up loop.
>>
>> On the Ia32 and X64 platforms however, FIFO reads and writes can be
>> optimized, by coding them in assembly, and delegating the loop to the
>> CPU, with the REP prefix.
>>
>> On KVM virtualization hosts, this difference has a huge performance
>> impact: if the loop is open-coded, then the virtual machine traps to the
>> hypervisor on every single UINT8 / UINT16 / UINT32 transfer, whereas
>> with the REP prefix, KVM can transfer up to a page of data per VM trap.
>> This is especially noticeable with IDE PIO transfers, where all the data
>> are squeezed through IO ports.
>>
>> * Long description:
>>
>> The RootBridgeIoIoRW() function in
>>
>> PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c
>>
>> used to have the exact same IO port acces optimization, dating back
>> verbatim to commit 1fd376d9792:
>>
>> PcAtChipsetPkg/PciHostBridgeDxe: Improve KVM FIFO I/O read/write
>> performance
>>
>> OvmfPkg cloned the "PcAtChipsetPkg/PciHostBridgeDxe" driver (for
>> unrelated reasons), and inherited the optimization from PcAtChipsetPkg.
>>
>> The "PcAtChipsetPkg/PciHostBridgeDxe" driver was ultimately removed in
>> commit 111d79db47:
>>
>> PcAtChipsetPkg/PciHostBridge: Remove PciHostBridge driver
>>
>> and OvmfPkg too was rebased to the new core Pci Host Bridge Driver, in
>> commit 4014885ffd:
>>
>> OvmfPkg: switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe
>>
>> This caused the optimization to go lost. Namely, the
>> RootBridgeIoIoRead() and RootBridgeIoIoWrite() functions in the new core
>> Pci Host Bridge Driver delegate IO port accesses to
>> EFI_CPU_IO2_PROTOCOL. And, in OvmfPkg (and likely most other Ia32 / X64
>> edk2 platforms), this protocol is provided by "UefiCpuPkg/CpuIo2Dxe",
>> which lacks the optimization.
>>
>> Therefore, this patch ports the C source code logic from commit
>> 1fd376d9792 (see above) to "UefiCpuPkg/CpuIo2Dxe", plus it ports the
>> NASM-converted assembly helper functions from OvmfPkg commits
>> 6026bf460037 and ace1d0517b65:
>>
>> OvmfPkg PciHostBridgeDxe: Convert Ia32/IoFifo.asm to NASM
>>
>> OvmfPkg PciHostBridgeDxe: Convert X64/IoFifo.asm to NASM
>>
>> In order to support the MSFT and INTEL toolchains as well, the *.asm
>> files are ported from OvmfPkg as well, immediately from before the above
>> conversion (that is, at 6026bf460037^).
>>
>> * Notes about the port:
>>
>> - The write and read branches from commit 1fd376d9792 are split to the
>> separate functions CpuIoServiceWrite() and CpuIoServiceRead().
>>
>> - The EfiPciWidthUintXX constants are replaced with EfiCpuIoWidthUintXX.
>>
>> - The cast expression "(UINTN) Address" is replaced with
>> "(UINTN)Address" (i.e., no space), because that's how the receiving
>> functions spell it as well.
>>
>> - The labels in the switch statements are unindented by one level, to
>> match the edk2 coding style (and the rest of UefiCpuPkg) better.
>>
>> * The first signoff belongs to Jordan, because he authored all of
>> 1fd376d9792, 6026bf460037 and ace1d0517b65.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> Ref: https://www.redhat.com/archives/vfio-users/2016-April/msg00029.html
>> Reported-by: Mark <kram321@gmail.com>
>> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/10424/focus=10432
>> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> Cc: Mark <kram321@gmail.com>
>> Tested-by: Mark <kram321@gmail.com>
>> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
>
> This is "hard evidence". Per bullet (2), Jordan had added the FIFO /
> REP variants to EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL, for some
> (non-specific) performance boost. and when that optimization was later
> lost, due to a mistake, we saw IDE PIO performance *tank* on QEMU/KVM.
>
> Therefore, I am now being reminded that the best performance
> regression test for this particular series is to boot an operating
> system using IDE (PIO) emulation, with OVMF! Because CpuIo2Dxe still
> calls the FIFO APIs.
>
> That's what I'm going to do next -- later this week, if everything
> goes well. I ask for your patience until I complete that regression
> test.
Thankfully, we captured the original bug report in the commit message
above, in the first "Ref:" tag. That link still works today, and it is
helpful for setting up a reproducer.
So, this is my reproducer:
(0) The QEMU version is a recent development version (built at git
commit a95260486aa7, "Merge tag 'pull-tcg-20231023' of
https://gitlab.com/rth7680/qemu into staging", 2023-10-23).
My host kernel (KVM) is from RHEL-9.2: version
5.14.0-284.30.1.el9_2.x86_64.
(1) build OVMF X64 for the DEBUG target
(2) launch QEMU as follows:
qemu-system-x86_64 \
-M pc \
-enable-kvm \
-m 4096 \
-drive if=pflash,format=raw,unit=0,file=OVMF_CODE.4m.fd,readonly=on \
-drive if=pflash,format=raw,unit=1,file=OVMF_VARS.4m.fd,snapshot=on \
-debugcon file:debug.log \
-global isa-debugcon.iobase=0x402 \
-cdrom en-us_windows_server_2019_updated_aug_2021_x64_dvd_a6431a28.iso
The original bug report used a Windows 7 installer ISO; my reproducer
uses a more recent Windows ISO: Windows Server 2019.
Steps (1) and (2) were once performed at current edk2 master
(ccbe2e938386), and then another time with this patch series applied on
top.
When the Windows ISO boots, and I hit Enter to confirm booting from the
ISO, two "Loading files..." progress bars are displayed, one after the
other. I measured the duration of these progress bars, in both
scenarios.
With OVMF built at edk2 master,
- the first "Loading files..." progress bar is hardly noticeable to the
human eye,
- the second "Loading files..." progress bar takes 28 seconds to
complete.
With this patch set applied on top of master,
- the first "Loading files..." progress bar takes 2.3 seconds,
- the second "Loading files..." progress bar takes 7 minutes and 44.4
seconds (464.4 seconds), meaning a slowdown factor of ~16.5.
So... please don't merge this. The bottleneck still exists. We still
need the "rep outsb" optimization on KVM.
Thanks,
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110455): https://edk2.groups.io/g/devel/message/110455
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-10-31 22:36 ` Laszlo Ersek
@ 2023-11-02 8:49 ` duntan
2023-11-02 10:42 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: duntan @ 2023-11-02 8:49 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, thomas.lendacky@amd.com,
leo.duran@amd.com, brijesh.singh@amd.com, Chang, Abner,
michael.roth@amd.com, Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
Laszlo,
Thanks for your thorough and detailed research on this topic. With the test result that you reproduced, I agree with your opinion.
So this patch set will be dropped and I'll raise another new patch set. In the new patch set, I'll only change the code for BaseIoLibIntrinsic instance and leave the BaseIoLibIntrinsicSev as it was.
Thanks,
Dun
-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, November 1, 2023 6:37 AM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; thomas.lendacky@amd.com; leo.duran@amd.com; brijesh.singh@amd.com; Chang, Abner <abner.chang@amd.com>; michael.roth@amd.com; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Ni, Ray <ray.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
On 10/31/23 18:06, Laszlo Ersek wrote:
> (3) During all this discussion here, I have been *distinctly*
> recalling that I've experienced an *actual performance regression*
> sometime, after we had unwittingly switched from the REP variant to
> the non-REP variant.
>
> And now I have actually found it. I'm going to quote the commit
> message in full:
>
>> commit fb8b54694c53e73e1b6a098686e908f54f9bb7a9
>> Author: Laszlo Ersek <lersek@redhat.com>
>> Date: Thu Apr 7 22:28:38 2016 +0200
>>
>> UefiCpuPkg: CpuIo2Dxe: optimize FIFO reads and writes of IO ports
>>
>> * Short description:
>>
>> The CpuIoServiceRead() and CpuIoServiceWrite() functions transfer data
>> between memory and IO ports with individual Io(Read|Write)(8|16|32)
>> function calls, each in an appropriately set up loop.
>>
>> On the Ia32 and X64 platforms however, FIFO reads and writes can be
>> optimized, by coding them in assembly, and delegating the loop to the
>> CPU, with the REP prefix.
>>
>> On KVM virtualization hosts, this difference has a huge performance
>> impact: if the loop is open-coded, then the virtual machine traps to the
>> hypervisor on every single UINT8 / UINT16 / UINT32 transfer, whereas
>> with the REP prefix, KVM can transfer up to a page of data per VM trap.
>> This is especially noticeable with IDE PIO transfers, where all the data
>> are squeezed through IO ports.
>>
>> * Long description:
>>
>> The RootBridgeIoIoRW() function in
>>
>> PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c
>>
>> used to have the exact same IO port acces optimization, dating back
>> verbatim to commit 1fd376d9792:
>>
>> PcAtChipsetPkg/PciHostBridgeDxe: Improve KVM FIFO I/O read/write
>> performance
>>
>> OvmfPkg cloned the "PcAtChipsetPkg/PciHostBridgeDxe" driver (for
>> unrelated reasons), and inherited the optimization from PcAtChipsetPkg.
>>
>> The "PcAtChipsetPkg/PciHostBridgeDxe" driver was ultimately removed in
>> commit 111d79db47:
>>
>> PcAtChipsetPkg/PciHostBridge: Remove PciHostBridge driver
>>
>> and OvmfPkg too was rebased to the new core Pci Host Bridge Driver, in
>> commit 4014885ffd:
>>
>> OvmfPkg: switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe
>>
>> This caused the optimization to go lost. Namely, the
>> RootBridgeIoIoRead() and RootBridgeIoIoWrite() functions in the new core
>> Pci Host Bridge Driver delegate IO port accesses to
>> EFI_CPU_IO2_PROTOCOL. And, in OvmfPkg (and likely most other Ia32 / X64
>> edk2 platforms), this protocol is provided by "UefiCpuPkg/CpuIo2Dxe",
>> which lacks the optimization.
>>
>> Therefore, this patch ports the C source code logic from commit
>> 1fd376d9792 (see above) to "UefiCpuPkg/CpuIo2Dxe", plus it ports the
>> NASM-converted assembly helper functions from OvmfPkg commits
>> 6026bf460037 and ace1d0517b65:
>>
>> OvmfPkg PciHostBridgeDxe: Convert Ia32/IoFifo.asm to NASM
>>
>> OvmfPkg PciHostBridgeDxe: Convert X64/IoFifo.asm to NASM
>>
>> In order to support the MSFT and INTEL toolchains as well, the *.asm
>> files are ported from OvmfPkg as well, immediately from before the above
>> conversion (that is, at 6026bf460037^).
>>
>> * Notes about the port:
>>
>> - The write and read branches from commit 1fd376d9792 are split to the
>> separate functions CpuIoServiceWrite() and CpuIoServiceRead().
>>
>> - The EfiPciWidthUintXX constants are replaced with EfiCpuIoWidthUintXX.
>>
>> - The cast expression "(UINTN) Address" is replaced with
>> "(UINTN)Address" (i.e., no space), because that's how the receiving
>> functions spell it as well.
>>
>> - The labels in the switch statements are unindented by one level, to
>> match the edk2 coding style (and the rest of UefiCpuPkg) better.
>>
>> * The first signoff belongs to Jordan, because he authored all of
>> 1fd376d9792, 6026bf460037 and ace1d0517b65.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> Ref: https://www.redhat.com/archives/vfio-users/2016-April/msg00029.html
>> Reported-by: Mark <kram321@gmail.com>
>> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/10424/focus=10432
>> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> Cc: Mark <kram321@gmail.com>
>> Tested-by: Mark <kram321@gmail.com>
>> Reviewed-by: Jeff Fan <jeff.fan@intel.com>
>
> This is "hard evidence". Per bullet (2), Jordan had added the FIFO /
> REP variants to EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL, for some
> (non-specific) performance boost. and when that optimization was later
> lost, due to a mistake, we saw IDE PIO performance *tank* on QEMU/KVM.
>
> Therefore, I am now being reminded that the best performance
> regression test for this particular series is to boot an operating
> system using IDE (PIO) emulation, with OVMF! Because CpuIo2Dxe still
> calls the FIFO APIs.
>
> That's what I'm going to do next -- later this week, if everything
> goes well. I ask for your patience until I complete that regression
> test.
Thankfully, we captured the original bug report in the commit message above, in the first "Ref:" tag. That link still works today, and it is helpful for setting up a reproducer.
So, this is my reproducer:
(0) The QEMU version is a recent development version (built at git commit a95260486aa7, "Merge tag 'pull-tcg-20231023' of https://gitlab.com/rth7680/qemu into staging", 2023-10-23).
My host kernel (KVM) is from RHEL-9.2: version 5.14.0-284.30.1.el9_2.x86_64.
(1) build OVMF X64 for the DEBUG target
(2) launch QEMU as follows:
qemu-system-x86_64 \
-M pc \
-enable-kvm \
-m 4096 \
-drive if=pflash,format=raw,unit=0,file=OVMF_CODE.4m.fd,readonly=on \
-drive if=pflash,format=raw,unit=1,file=OVMF_VARS.4m.fd,snapshot=on \
-debugcon file:debug.log \
-global isa-debugcon.iobase=0x402 \
-cdrom en-us_windows_server_2019_updated_aug_2021_x64_dvd_a6431a28.iso
The original bug report used a Windows 7 installer ISO; my reproducer uses a more recent Windows ISO: Windows Server 2019.
Steps (1) and (2) were once performed at current edk2 master (ccbe2e938386), and then another time with this patch series applied on top.
When the Windows ISO boots, and I hit Enter to confirm booting from the ISO, two "Loading files..." progress bars are displayed, one after the other. I measured the duration of these progress bars, in both scenarios.
With OVMF built at edk2 master,
- the first "Loading files..." progress bar is hardly noticeable to the
human eye,
- the second "Loading files..." progress bar takes 28 seconds to
complete.
With this patch set applied on top of master,
- the first "Loading files..." progress bar takes 2.3 seconds,
- the second "Loading files..." progress bar takes 7 minutes and 44.4
seconds (464.4 seconds), meaning a slowdown factor of ~16.5.
So... please don't merge this. The bottleneck still exists. We still need the "rep outsb" optimization on KVM.
Thanks,
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110504): https://edk2.groups.io/g/devel/message/110504
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev
2023-11-02 8:49 ` duntan
@ 2023-11-02 10:42 ` Laszlo Ersek
0 siblings, 0 replies; 27+ messages in thread
From: Laszlo Ersek @ 2023-11-02 10:42 UTC (permalink / raw)
To: Tan, Dun, devel@edk2.groups.io, thomas.lendacky@amd.com,
leo.duran@amd.com, brijesh.singh@amd.com, Chang, Abner,
michael.roth@amd.com, Attar, AbdulLateef (Abdul Lateef)
Cc: Ni, Ray, Yao, Jiewen
On 11/2/23 09:49, Tan, Dun wrote:
> Laszlo,
>
> Thanks for your thorough and detailed research on this topic. With the test result that you reproduced, I agree with your opinion.
> So this patch set will be dropped and I'll raise another new patch set. In the new patch set, I'll only change the code for BaseIoLibIntrinsic instance and leave the BaseIoLibIntrinsicSev as it was.
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110520): https://edk2.groups.io/g/devel/message/110520
Mute This Topic: https://groups.io/mt/102215661/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2023-11-02 10:42 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-27 5:42 [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 1/7] MdePkg: Create TdxLibNull.inf instance duntan
2023-10-27 5:54 ` Ni, Ray
2023-10-27 5:56 ` Ni, Ray
2023-10-27 6:32 ` duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 2/7] MdePkg: Add CcProbeLibNull and TdxLibNull implement duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 3/7] MdePkg: simplify IoRead/WriteFifo in IoLibFifo.c duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 4/7] MdePkg:support Tdx and sev in BaseIoLibIntrinsic duntan
2023-10-27 5:42 ` [edk2-devel] [PATCH 5/7] OvmfPkg: Add CcProbeLib in PlatformInitLib.inf duntan
2023-10-30 14:37 ` Ard Biesheuvel
2023-10-27 5:42 ` [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files duntan
2023-10-30 14:36 ` Ard Biesheuvel
2023-10-31 12:30 ` Laszlo Ersek
2023-10-31 13:19 ` Ard Biesheuvel
2023-10-27 5:43 ` [edk2-devel] [PATCH 7/7] MdePkg:remove BaseIoLibIntrinsicSev related code duntan
2023-10-27 5:49 ` [edk2-devel] [PATCH 0/7] Support Tdx and sev in BaseIoLibIntrinsic and remove BaseIoLibIntrinsicSev Yao, Jiewen
2023-10-27 6:31 ` duntan
2023-10-27 7:06 ` Yao, Jiewen
2023-10-27 7:34 ` duntan
2023-10-27 8:05 ` duntan
2023-10-27 21:31 ` Lendacky, Thomas via groups.io
2023-10-28 11:40 ` Laszlo Ersek
2023-10-31 9:56 ` duntan
2023-10-31 17:06 ` Laszlo Ersek
2023-10-31 22:36 ` Laszlo Ersek
2023-11-02 8:49 ` duntan
2023-11-02 10:42 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox