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