RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
TdxLib is created with functions to perform the related Tdx operation.
This includes functions for:
- TdCall : Cause a VM exit to the Intel TDX module.
- TdVmCall : It helps invoke services from the host VMM to pass/
receive information.
- TdVmCallCpuid : Enable the TD guest to request VMM to emulate CPUID
- TdAcceptPages : Accept pending private pages and initialize the pages
to all-0 using the TD ephemeral private key.
- TdExtendRtmr : Extend measurement to one of the RTMR registers.
- TdSharedPageMask: Get the Td guest shared page mask which indicates it
is a Shared or Private page.
- TdMaxVCpuNum : Get the maximum number of virtual CPUs.
- TdVCpuNum : Get the number of virtual CPUs.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
MdePkg/Include/Library/TdxLib.h | 167 +++++++++++++++++++
MdePkg/Library/TdxLib/AcceptPages.c | 137 ++++++++++++++++
MdePkg/Library/TdxLib/Rtmr.c | 83 ++++++++++
MdePkg/Library/TdxLib/TdInfo.c | 103 ++++++++++++
MdePkg/Library/TdxLib/TdxLib.inf | 39 +++++
MdePkg/Library/TdxLib/TdxLibNull.c | 192 ++++++++++++++++++++++
MdePkg/Library/TdxLib/X64/Tdcall.nasm | 85 ++++++++++
MdePkg/Library/TdxLib/X64/Tdvmcall.nasm | 207 ++++++++++++++++++++++++
MdePkg/MdePkg.dec | 3 +
MdePkg/MdePkg.dsc | 1 +
10 files changed, 1017 insertions(+)
create mode 100644 MdePkg/Include/Library/TdxLib.h
create mode 100644 MdePkg/Library/TdxLib/AcceptPages.c
create mode 100644 MdePkg/Library/TdxLib/Rtmr.c
create mode 100644 MdePkg/Library/TdxLib/TdInfo.c
create mode 100644 MdePkg/Library/TdxLib/TdxLib.inf
create mode 100644 MdePkg/Library/TdxLib/TdxLibNull.c
create mode 100644 MdePkg/Library/TdxLib/X64/Tdcall.nasm
create mode 100644 MdePkg/Library/TdxLib/X64/Tdvmcall.nasm
diff --git a/MdePkg/Include/Library/TdxLib.h b/MdePkg/Include/Library/TdxLib.h
new file mode 100644
index 000000000000..d9e0335b2300
--- /dev/null
+++ b/MdePkg/Include/Library/TdxLib.h
@@ -0,0 +1,167 @@
+/** @file
+ TdxLib definitions
+
+ Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef TDX_LIB_H_
+#define TDX_LIB_H_
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Uefi/UefiBaseType.h>
+#include <Protocol/DebugSupport.h>
+
+/**
+ This function accepts a pending private page, and initialize the page to
+ all-0 using the TD ephemeral private key.
+
+ @param[in] StartAddress Guest physical address of the private page
+ to accept.
+ @param[in] NumberOfPages Number of the pages to be accepted.
+ @param[in] PageSize GPA page size. Accept 2M/4K page size.
+
+ @return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TdAcceptPages (
+ IN UINT64 StartAddress,
+ IN UINT64 NumberOfPages,
+ IN UINT32 PageSize
+ );
+
+/**
+ This function extends one of the RTMR measurement register
+ in TDCS with the provided extension data in memory.
+ RTMR extending supports SHA384 which length is 48 bytes.
+
+ @param[in] Data Point to the data to be extended
+ @param[in] DataLen Length of the data. Must be 48
+ @param[in] Index RTMR index
+
+ @return EFI_SUCCESS
+ @return EFI_INVALID_PARAMETER
+ @return EFI_DEVICE_ERROR
+
+**/
+EFI_STATUS
+EFIAPI
+TdExtendRtmr (
+ IN UINT32 *Data,
+ IN UINT32 DataLen,
+ IN UINT8 Index
+ );
+
+
+/**
+ This function gets the Td guest shared page mask.
+
+ The guest indicates if a page is shared using the Guest Physical Address
+ (GPA) Shared (S) bit. If the GPA Width(GPAW) is 48, the S-bit is bit-47.
+ If the GPAW is 52, the S-bit is bit-51.
+
+ @return Shared page bit mask
+**/
+UINT64
+EFIAPI
+TdSharedPageMask (
+ VOID
+ );
+
+/**
+ This function gets the maximum number of Virtual CPUs that are usable for
+ Td Guest.
+
+ @return maximum Virtual CPUs number
+**/
+UINT32
+EFIAPI
+TdMaxVCpuNum (
+ VOID
+ );
+
+/**
+ This function gets the number of Virtual CPUs that are usable for Td
+ Guest.
+
+ @return Virtual CPUs number
+**/
+UINT32
+EFIAPI
+TdVCpuNum (
+ VOID
+ );
+
+
+/**
+ The TDCALL instruction causes a VM exit to the Intel TDX module. It is
+ used to call guest-side Intel TDX functions, either local or a TD exit
+ to the host VMM, as selected by Leaf.
+
+ @param[in] Leaf Leaf number of TDCALL instruction
+ @param[in] Arg1 Arg1
+ @param[in] Arg2 Arg2
+ @param[in] Arg3 Arg3
+ @param[in,out] Results Returned result of the Leaf function
+
+ @return EFI_SUCCESS
+ @return Other See individual leaf functions
+**/
+EFI_STATUS
+EFIAPI
+TdCall (
+ IN UINT64 Leaf,
+ IN UINT64 Arg1,
+ IN UINT64 Arg2,
+ IN UINT64 Arg3,
+ IN OUT VOID *Results
+ );
+
+/**
+ TDVMALL is a leaf function 0 for TDCALL. It helps invoke services from the
+ host VMM to pass/receive information.
+
+ @param[in] Leaf Number of sub-functions
+ @param[in] Arg1 Arg1
+ @param[in] Arg2 Arg2
+ @param[in] Arg3 Arg3
+ @param[in] Arg4 Arg4
+ @param[in,out] Results Returned result of the sub-function
+
+ @return EFI_SUCCESS
+ @return Other See individual sub-functions
+
+**/
+EFI_STATUS
+EFIAPI
+TdVmCall (
+ IN UINT64 Leaf,
+ IN UINT64 Arg1,
+ IN UINT64 Arg2,
+ IN UINT64 Arg3,
+ IN UINT64 Arg4,
+ IN OUT VOID *Results
+ );
+
+/**
+ This function enable the TD guest to request the VMM to emulate CPUID
+ operation, especially for non-architectural, CPUID leaves.
+
+ @param[in] Eax Main leaf of the CPUID
+ @param[in] Ecx Sub-leaf of the CPUID
+ @param[out] Results Returned result of CPUID operation
+
+ @return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TdVmCallCpuid (
+ IN UINT64 Eax,
+ IN UINT64 Ecx,
+ OUT VOID *Results
+ );
+
+#endif
diff --git a/MdePkg/Library/TdxLib/AcceptPages.c b/MdePkg/Library/TdxLib/AcceptPages.c
new file mode 100644
index 000000000000..18e94b13c351
--- /dev/null
+++ b/MdePkg/Library/TdxLib/AcceptPages.c
@@ -0,0 +1,137 @@
+/** @file
+
+ Unaccepted memory is a special type of private memory. In Td guest
+ TDCALL [TDG.MEM.PAGE.ACCEPT] is invoked to accept the unaccepted
+ memory before use it.
+
+ Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <IndustryStandard/Tdx.h>
+#include <Library/TdxLib.h>
+#include <Library/BaseMemoryLib.h>
+
+UINT64 mNumberOfDuplicatedAcceptedPages;
+
+// PageSize is mapped to PageLevel like below:
+// 4KB - 0, 2MB - 1
+UINT32 mTdxAcceptPageLevelMap[2] = {
+ SIZE_4KB,
+ SIZE_2MB
+};
+
+/**
+ This function gets the PageLevel according to the input page size.
+
+ @param[in] PageSize Page size
+
+ @return UINT32 The mapped page level
+**/
+UINT32
+GetGpaPageLevel (
+ UINT32 PageSize
+ )
+{
+ UINT32 Index;
+
+ for (Index = 0; Index < ARRAY_SIZE (mTdxAcceptPageLevelMap); Index++) {
+ if (mTdxAcceptPageLevelMap[Index] == PageSize) {
+ break;
+ }
+ }
+
+ return Index == ARRAY_SIZE (mTdxAcceptPageLevelMap) ? -1 : Index;
+}
+
+/**
+ This function accept a pending private page, and initialize the page to
+ all-0 using the TD ephemeral private key.
+
+ Sometimes TDCALL [TDG.MEM.PAGE.ACCEPT] may return
+ TDX_EXIT_REASON_PAGE_SIZE_MISMATCH. It indicates the input PageLevel is
+ not workable. In this case we need to try to fallback to a smaller
+ PageLevel if possible.
+
+ @param[in] StartAddress Guest physical address of the private
+ page to accept.
+ @param[in] NumberOfPages Number of the pages to be accepted.
+ @param[in] PageSize GPA page size. Only accept 1G/2M/4K size.
+
+ @return EFI_SUCCESS Accept successfully
+ @return others Indicate other errors
+**/
+EFI_STATUS
+EFIAPI
+TdAcceptPages (
+ IN UINT64 StartAddress,
+ IN UINT64 NumberOfPages,
+ IN UINT32 PageSize
+ )
+{
+ EFI_STATUS Status;
+ UINT64 Address;
+ UINT64 TdxStatus;
+ UINT64 Index;
+ UINT32 GpaPageLevel;
+ UINT32 PageSize2;
+
+ Address = StartAddress;
+
+ GpaPageLevel = GetGpaPageLevel (PageSize);
+ if (GpaPageLevel == -1) {
+ DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page size - 0x%llx\n", PageSize));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = EFI_SUCCESS;
+ for (Index = 0; Index < NumberOfPages; Index++) {
+ TdxStatus = TdCall (TDCALL_TDACCEPTPAGE, Address | GpaPageLevel, 0, 0, 0);
+ if (TdxStatus != TDX_EXIT_REASON_SUCCESS) {
+ if ((TdxStatus & ~0xFFFFULL) == TDX_EXIT_REASON_PAGE_ALREADY_ACCEPTED) {
+ //
+ // Already accepted
+ //
+ mNumberOfDuplicatedAcceptedPages++;
+ DEBUG ((DEBUG_WARN, "Page at Address (0x%llx) has already been accepted. - %d\n", Address, mNumberOfDuplicatedAcceptedPages));
+ } else if ((TdxStatus & ~0xFFFFULL) == TDX_EXIT_REASON_PAGE_SIZE_MISMATCH) {
+ //
+ // GpaPageLevel is mismatch, fall back to a smaller GpaPageLevel if possible
+ //
+ DEBUG ((DEBUG_VERBOSE, "Address %llx cannot be accepted in PageLevel of %d\n", Address, GpaPageLevel));
+
+ if (GpaPageLevel == 0) {
+ //
+ // Cannot fall back to smaller page level
+ //
+ DEBUG ((DEBUG_ERROR, "AcceptPage cannot fallback from PageLevel %d\n", GpaPageLevel));
+ Status = EFI_INVALID_PARAMETER;
+ break;
+ } else {
+ //
+ // Fall back to a smaller page size
+ //
+ PageSize2 = mTdxAcceptPageLevelMap [GpaPageLevel - 1];
+ Status = TdAcceptPages(Address, 512, PageSize2);
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+ }
+ }else {
+
+ //
+ // Other errors
+ //
+ DEBUG ((DEBUG_ERROR, "Address %llx (%d) failed to be accepted. Error = 0x%llx\n",
+ Address, Index, TdxStatus));
+ Status = EFI_INVALID_PARAMETER;
+ break;
+ }
+ }
+ Address += PageSize;
+ }
+ return Status;
+}
diff --git a/MdePkg/Library/TdxLib/Rtmr.c b/MdePkg/Library/TdxLib/Rtmr.c
new file mode 100644
index 000000000000..42e85eb9d9bb
--- /dev/null
+++ b/MdePkg/Library/TdxLib/Rtmr.c
@@ -0,0 +1,83 @@
+/** @file
+
+ Extends one of the RTMR measurement registers in TDCS with the provided
+ extension data in memory.
+
+ Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/TdxLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <IndustryStandard/Tpm20.h>
+#include <IndustryStandard/Tdx.h>
+
+#define RTMR_COUNT 4
+#define TD_EXTEND_BUFFER_LEN (64 + 48)
+
+UINT8 mExtendBuffer[TD_EXTEND_BUFFER_LEN];
+
+/**
+ This function extends one of the RTMR measurement register
+ in TDCS with the provided extension data in memory.
+ RTMR extending supports SHA384 which length is 48 bytes.
+
+ @param[in] Data Point to the data to be extended
+ @param[in] DataLen Length of the data. Must be 48
+ @param[in] Index RTMR index
+
+ @return EFI_SUCCESS
+ @return EFI_INVALID_PARAMETER
+ @return EFI_DEVICE_ERROR
+
+**/
+EFI_STATUS
+EFIAPI
+TdExtendRtmr (
+ IN UINT32 *Data,
+ IN UINT32 DataLen,
+ IN UINT8 Index
+ )
+{
+ EFI_STATUS Status;
+ UINT64 TdCallStatus;
+ UINT8 *ExtendBuffer;
+
+ Status = EFI_SUCCESS;
+
+ ASSERT (Data != NULL);
+ ASSERT (DataLen == SHA384_DIGEST_SIZE);
+ ASSERT (Index >= 0 && Index < RTMR_COUNT);
+
+ if (Data == NULL || DataLen != SHA384_DIGEST_SIZE || Index >= RTMR_COUNT) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // TD.RTMR.EXTEND requires 64B-aligned guest physical address of
+ // 48B-extension data. We use ALIGN_POINTER(Pointer, 64) to get
+ // the 64B-aligned guest physical address.
+ ExtendBuffer = ALIGN_POINTER (mExtendBuffer, 64);
+ ASSERT (((UINTN)ExtendBuffer & 0x3f) == 0 );
+
+ ZeroMem (ExtendBuffer, SHA384_DIGEST_SIZE);
+ CopyMem (ExtendBuffer, Data, SHA384_DIGEST_SIZE);
+
+ TdCallStatus = TdCall (TDCALL_TDEXTENDRTMR, (UINT64)(UINTN)ExtendBuffer, Index, 0, 0);
+
+ if (TdCallStatus == TDX_EXIT_REASON_SUCCESS) {
+ Status = EFI_SUCCESS;
+ } else if (TdCallStatus == TDX_EXIT_REASON_OPERAND_INVALID) {
+ Status = EFI_INVALID_PARAMETER;
+ } else {
+ Status = EFI_DEVICE_ERROR;
+ }
+
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((DEBUG_ERROR, "Error returned from TdExtendRtmr call - 0x%lx\n", TdCallStatus));
+ }
+
+ return Status;
+}
diff --git a/MdePkg/Library/TdxLib/TdInfo.c b/MdePkg/Library/TdxLib/TdInfo.c
new file mode 100644
index 000000000000..56c268e70c8d
--- /dev/null
+++ b/MdePkg/Library/TdxLib/TdInfo.c
@@ -0,0 +1,103 @@
+/** @file
+
+ Fetch the Tdx info.
+
+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <IndustryStandard/Tdx.h>
+#include <Library/TdxLib.h>
+#include <Library/BaseMemoryLib.h>
+
+UINT64 mTdSharedPageMask = 0;
+UINT32 mTdMaxVCpuNum = 0;
+UINT32 mTdVCpuNum = 0;
+
+/**
+ This function gets the Td guest shared page mask.
+
+ The guest indicates if a page is shared using the Guest Physical Address
+ (GPA) Shared (S) bit. If the GPA Width(GPAW) is 48, the S-bit is bit-47.
+ If the GPAW is 52, the S-bit is bit-51.
+
+ @return Shared page bit mask
+**/
+UINT64
+EFIAPI
+TdSharedPageMask (
+ VOID
+ )
+{
+ UINT64 Status;
+ UINT8 Gpaw;
+ TD_RETURN_DATA TdReturnData;
+
+ if (mTdSharedPageMask != 0) {
+ return mTdSharedPageMask;
+ }
+
+ Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+ ASSERT (Status == TDX_EXIT_REASON_SUCCESS);
+
+ Gpaw = (UINT8)(TdReturnData.TdInfo.Gpaw & 0x3f);
+ ASSERT(Gpaw == 48 || Gpaw == 52);
+ mTdSharedPageMask = 1ULL << (Gpaw - 1);
+ return mTdSharedPageMask;
+}
+
+/**
+ This function gets the maximum number of Virtual CPUs that are usable for
+ Td Guest.
+
+ @return maximum Virtual CPUs number
+**/
+UINT32
+EFIAPI
+TdMaxVCpuNum (
+ VOID
+ )
+{
+ UINT64 Status;
+ TD_RETURN_DATA TdReturnData;
+
+ if (mTdMaxVCpuNum != 0) {
+ return mTdMaxVCpuNum;
+ }
+
+ Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+ ASSERT (Status == TDX_EXIT_REASON_SUCCESS);
+
+ mTdMaxVCpuNum = TdReturnData.TdInfo.MaxVcpus;
+
+ return mTdMaxVCpuNum;
+}
+
+/**
+ This function gets the number of Virtual CPUs that are usable for Td
+ Guest.
+
+ @return Virtual CPUs number
+**/
+UINT32
+EFIAPI
+TdVCpuNum (
+ VOID
+ )
+{
+ UINT64 Status;
+ TD_RETURN_DATA TdReturnData;
+
+ if (mTdVCpuNum != 0) {
+ return mTdVCpuNum;
+ }
+
+ Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+ ASSERT (Status == TDX_EXIT_REASON_SUCCESS);
+
+ mTdVCpuNum = TdReturnData.TdInfo.NumVcpus;
+ return mTdVCpuNum;
+}
diff --git a/MdePkg/Library/TdxLib/TdxLib.inf b/MdePkg/Library/TdxLib/TdxLib.inf
new file mode 100644
index 000000000000..772abcc49d8b
--- /dev/null
+++ b/MdePkg/Library/TdxLib/TdxLib.inf
@@ -0,0 +1,39 @@
+## @file
+# Tdx library
+#
+# Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = TdxLib
+ FILE_GUID = 032A8E0D-0C27-40C0-9CAA-23B731C1B223
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = TdxLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources.IA32]
+ TdxLibNull.c
+
+[Sources.X64]
+ AcceptPages.c
+ Rtmr.c
+ TdInfo.c
+ X64/Tdcall.nasm
+ X64/Tdvmcall.nasm
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
diff --git a/MdePkg/Library/TdxLib/TdxLibNull.c b/MdePkg/Library/TdxLib/TdxLibNull.c
new file mode 100644
index 000000000000..d57f03b29cfe
--- /dev/null
+++ b/MdePkg/Library/TdxLib/TdxLibNull.c
@@ -0,0 +1,192 @@
+/** @file
+
+ Null stub of TdxLib
+
+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/TdxLib.h>
+
+/**
+ This function accepts a pending private page, and initialize the page to
+ all-0 using the TD ephemeral private key.
+
+ @param[in] StartAddress Guest physical address of the private page
+ to accept.
+ @param[in] NumberOfPages Number of the pages to be accepted.
+ @param[in] PageSize GPA page size. Accept 1G/2M/4K page size.
+
+ @return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TdAcceptPages (
+ IN UINT64 StartAddress,
+ IN UINT64 NumberOfPages,
+ IN UINT32 PageSize
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ This function extends one of the RTMR measurement register
+ in TDCS with the provided extension data in memory.
+ RTMR extending supports SHA384 which length is 48 bytes.
+
+ @param[in] Data Point to the data to be extended
+ @param[in] DataLen Length of the data. Must be 48
+ @param[in] Index RTMR index
+
+ @return EFI_SUCCESS
+ @return EFI_INVALID_PARAMETER
+ @return EFI_DEVICE_ERROR
+
+**/
+EFI_STATUS
+EFIAPI
+TdExtendRtmr (
+ IN UINT32 *Data,
+ IN UINT32 DataLen,
+ IN UINT8 Index
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+
+/**
+ This function gets the Td guest shared page mask.
+
+ The guest indicates if a page is shared using the Guest Physical Address
+ (GPA) Shared (S) bit. If the GPA Width(GPAW) is 48, the S-bit is bit-47.
+ If the GPAW is 52, the S-bit is bit-51.
+
+ @return Shared page bit mask
+**/
+UINT64
+EFIAPI
+TdSharedPageMask (
+ VOID
+ )
+{
+ return 0;
+}
+
+
+/**
+ This function gets the maximum number of Virtual CPUs that are usable for
+ Td Guest.
+
+ @return maximum Virtual CPUs number
+**/
+UINT32
+EFIAPI
+TdMaxVCpuNum (
+ VOID
+ )
+{
+ return 0;
+}
+
+
+/**
+ This function gets the number of Virtual CPUs that are usable for Td
+ Guest.
+
+ @return Virtual CPUs number
+**/
+UINT32
+EFIAPI
+TdVCpuNum (
+ VOID
+ )
+{
+ return 0;
+}
+
+
+/**
+ The TDCALL instruction causes a VM exit to the Intel TDX module. It is
+ used to call guest-side Intel TDX functions, either local or a TD exit
+ to the host VMM, as selected by Leaf.
+ Leaf functions are described at <https://software.intel.com/content/
+ www/us/en/develop/articles/intel-trust-domain-extensions.html>
+
+ @param[in] Leaf Leaf number of TDCALL instruction
+ @param[in] Arg1 Arg1
+ @param[in] Arg2 Arg2
+ @param[in] Arg3 Arg3
+ @param[in,out] Results Returned result of the Leaf function
+
+ @return EFI_SUCCESS
+ @return Other See individual leaf functions
+**/
+EFI_STATUS
+EFIAPI
+TdCall (
+ IN UINT64 Leaf,
+ IN UINT64 Arg1,
+ IN UINT64 Arg2,
+ IN UINT64 Arg3,
+ IN OUT VOID *Results
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+
+/**
+ TDVMALL is a leaf function 0 for TDCALL. It helps invoke services from the
+ host VMM to pass/receive information.
+
+ @param[in] Leaf Number of sub-functions
+ @param[in] Arg1 Arg1
+ @param[in] Arg2 Arg2
+ @param[in] Arg3 Arg3
+ @param[in] Arg4 Arg4
+ @param[in,out] Results Returned result of the sub-function
+
+ @return EFI_SUCCESS
+ @return Other See individual sub-functions
+
+**/
+EFI_STATUS
+EFIAPI
+TdVmCall (
+ IN UINT64 Leaf,
+ IN UINT64 Arg1,
+ IN UINT64 Arg2,
+ IN UINT64 Arg3,
+ IN UINT64 Arg4,
+ IN OUT VOID *Results
+ )
+{
+ return EFI_UNSUPPORTED;
+}
+
+
+/**
+ This function enable the TD guest to request the VMM to emulate CPUID
+ operation, especially for non-architectural, CPUID leaves.
+
+ @param[in] Eax Main leaf of the CPUID
+ @param[in] Ecx Sub-leaf of the CPUID
+ @param[out] Results Returned result of CPUID operation
+
+ @return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TdVmCallCpuid (
+ IN UINT64 Eax,
+ IN UINT64 Ecx,
+ OUT VOID *Results
+ )
+{
+ return EFI_UNSUPPORTED;
+}
diff --git a/MdePkg/Library/TdxLib/X64/Tdcall.nasm b/MdePkg/Library/TdxLib/X64/Tdcall.nasm
new file mode 100644
index 000000000000..e8a094b0eb3f
--- /dev/null
+++ b/MdePkg/Library/TdxLib/X64/Tdcall.nasm
@@ -0,0 +1,85 @@
+;------------------------------------------------------------------------------
+;*
+;* Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+;* SPDX-License-Identifier: BSD-2-Clause-Patent
+;*
+;*
+;------------------------------------------------------------------------------
+
+DEFAULT REL
+SECTION .text
+
+%macro tdcall 0
+ db 0x66,0x0f,0x01,0xcc
+%endmacro
+
+%macro tdcall_push_regs 0
+ push rbp
+ mov rbp, rsp
+ push r15
+ push r14
+ push r13
+ push r12
+ push rbx
+ push rsi
+ push rdi
+%endmacro
+
+%macro tdcall_pop_regs 0
+ pop rdi
+ pop rsi
+ pop rbx
+ pop r12
+ pop r13
+ pop r14
+ pop r15
+ pop rbp
+%endmacro
+
+%define number_of_regs_pushed 8
+%define number_of_parameters 4
+
+;
+; Keep these in sync for push_regs/pop_regs, code below
+; uses them to find 5th or greater parameters
+;
+%define first_variable_on_stack_offset \
+ ((number_of_regs_pushed * 8) + (number_of_parameters * 8) + 8)
+%define second_variable_on_stack_offset \
+ ((first_variable_on_stack_offset) + 8)
+
+; TdCall (
+; UINT64 Leaf, // Rcx
+; UINT64 P1, // Rdx
+; UINT64 P2, // R8
+; UINT64 P3, // R9
+; UINT64 Results, // rsp + 0x28
+; )
+global ASM_PFX(TdCall)
+ASM_PFX(TdCall):
+ tdcall_push_regs
+
+ mov rax, rcx
+ mov rcx, rdx
+ mov rdx, r8
+ mov r8, r9
+
+ tdcall
+
+ ; exit if tdcall reports failure.
+ test rax, rax
+ jnz .exit
+
+ ; test if caller wanted results
+ mov r12, [rsp + first_variable_on_stack_offset ]
+ test r12, r12
+ jz .exit
+ mov [r12 + 0 ], rcx
+ mov [r12 + 8 ], rdx
+ mov [r12 + 16], r8
+ mov [r12 + 24], r9
+ mov [r12 + 32], r10
+ mov [r12 + 40], r11
+.exit:
+ tdcall_pop_regs
+ ret
diff --git a/MdePkg/Library/TdxLib/X64/Tdvmcall.nasm b/MdePkg/Library/TdxLib/X64/Tdvmcall.nasm
new file mode 100644
index 000000000000..eb1cb967dc29
--- /dev/null
+++ b/MdePkg/Library/TdxLib/X64/Tdvmcall.nasm
@@ -0,0 +1,207 @@
+;------------------------------------------------------------------------------
+;*
+;* Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+;* SPDX-License-Identifier: BSD-2-Clause-Patent
+;*
+;*
+;------------------------------------------------------------------------------
+
+DEFAULT REL
+SECTION .text
+
+%define TDVMCALL_EXPOSE_REGS_MASK 0xffec
+%define TDVMCALL 0x0
+%define EXIT_REASON_CPUID 0xa
+
+%macro tdcall 0
+ db 0x66,0x0f,0x01,0xcc
+%endmacro
+
+%macro tdcall_push_regs 0
+ push rbp
+ mov rbp, rsp
+ push r15
+ push r14
+ push r13
+ push r12
+ push rbx
+ push rsi
+ push rdi
+%endmacro
+
+%macro tdcall_pop_regs 0
+ pop rdi
+ pop rsi
+ pop rbx
+ pop r12
+ pop r13
+ pop r14
+ pop r15
+ pop rbp
+%endmacro
+
+%define number_of_regs_pushed 8
+%define number_of_parameters 4
+
+;
+; Keep these in sync for push_regs/pop_regs, code below
+; uses them to find 5th or greater parameters
+;
+%define first_variable_on_stack_offset \
+ ((number_of_regs_pushed * 8) + (number_of_parameters * 8) + 8)
+%define second_variable_on_stack_offset \
+ ((first_variable_on_stack_offset) + 8)
+
+%macro tdcall_regs_preamble 2
+ mov rax, %1
+
+ xor rcx, rcx
+ mov ecx, %2
+
+ ; R10 = 0 (standard TDVMCALL)
+
+ xor r10d, r10d
+
+ ; Zero out unused (for standard TDVMCALL) registers to avoid leaking
+ ; secrets to the VMM.
+
+ xor ebx, ebx
+ xor esi, esi
+ xor edi, edi
+
+ xor edx, edx
+ xor ebp, ebp
+ xor r8d, r8d
+ xor r9d, r9d
+%endmacro
+
+%macro tdcall_regs_postamble 0
+ xor ebx, ebx
+ xor esi, esi
+ xor edi, edi
+
+ xor ecx, ecx
+ xor edx, edx
+ xor r8d, r8d
+ xor r9d, r9d
+ xor r10d, r10d
+ xor r11d, r11d
+%endmacro
+
+;------------------------------------------------------------------------------
+; 0 => RAX = TDCALL leaf
+; M => RCX = TDVMCALL register behavior
+; 1 => R10 = standard vs. vendor
+; RDI => R11 = TDVMCALL function / nr
+; RSI = R12 = p1
+; RDX => R13 = p2
+; RCX => R14 = p3
+; R8 => R15 = p4
+
+; UINT64
+; EFIAPI
+; TdVmCall (
+; UINT64 Leaf, // Rcx
+; UINT64 P1, // Rdx
+; UINT64 P2, // R8
+; UINT64 P3, // R9
+; UINT64 P4, // rsp + 0x28
+; UINT64 *Val // rsp + 0x30
+; )
+global ASM_PFX(TdVmCall)
+ASM_PFX(TdVmCall):
+ tdcall_push_regs
+
+ mov r11, rcx
+ mov r12, rdx
+ mov r13, r8
+ mov r14, r9
+ mov r15, [rsp + first_variable_on_stack_offset ]
+
+ tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
+
+ tdcall
+
+ ; ignore return dataif TDCALL reports failure.
+ test rax, rax
+ jnz .no_return_data
+
+ ; Propagate TDVMCALL success/failure to return value.
+ mov rax, r10
+
+ ; Retrieve the Val pointer.
+ mov r9, [rsp + second_variable_on_stack_offset ]
+ test r9, r9
+ jz .no_return_data
+
+ ; On success, propagate TDVMCALL output value to output param
+ test rax, rax
+ jnz .no_return_data
+ mov [r9], r11
+.no_return_data:
+ tdcall_regs_postamble
+
+ tdcall_pop_regs
+
+ ret
+
+;------------------------------------------------------------------------------
+; 0 => RAX = TDCALL leaf
+; M => RCX = TDVMCALL register behavior
+; 1 => R10 = standard vs. vendor
+; RDI => R11 = TDVMCALL function / nr
+; RSI = R12 = p1
+; RDX => R13 = p2
+; RCX => R14 = p3
+; R8 => R15 = p4
+
+; UINT64
+; EFIAPI
+; TdVmCallCpuid (
+; UINT64 EaxIn, // Rcx
+; UINT64 EcxIn, // Rdx
+; UINT64 *Results // R8
+; )
+global ASM_PFX(TdVmCallCpuid)
+ASM_PFX(TdVmCallCpuid):
+ tdcall_push_regs
+
+ mov r11, EXIT_REASON_CPUID
+ mov r12, rcx
+ mov r13, rdx
+
+ ; Save *results pointers
+ push r8
+
+ tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
+
+ tdcall
+
+ ; Panic if TDCALL reports failure.
+ test rax, rax
+ jnz .no_return_data
+
+ ; Propagate TDVMCALL success/failure to return value.
+ mov rax, r10
+ test rax, rax
+ jnz .no_return_data
+
+ ; Retrieve *Results
+ pop r8
+ test r8, r8
+ jz .no_return_data
+ ; Caller pass in buffer so store results r12-r15 contains eax-edx
+ mov [r8 + 0], r12
+ mov [r8 + 8], r13
+ mov [r8 + 16], r14
+ mov [r8 + 24], r15
+
+.no_return_data:
+ tdcall_regs_postamble
+
+ tdcall_pop_regs
+
+ ret
+
+.panic:
+ ud2
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 8b18415b107a..321a14fbaa0a 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -296,6 +296,9 @@
## @libraryclass Provides services to log the SMI handler registration.
SmiHandlerProfileLib|Include/Library/SmiHandlerProfileLib.h
+ ## @libraryclass Provides function to support TDX processing.
+ TdxLib|Include/Library/TdxLib.h
+
[Guids]
#
# GUID defined in UEFI2.1/UEFI2.0/EFI1.1
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index a94959169b2f..d6a7af412be7 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -175,6 +175,7 @@
MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.inf
MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
+ MdePkg/Library/TdxLib/TdxLib.inf
[Components.EBC]
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
--
2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82986): https://edk2.groups.io/g/devel/message/82986
Mute This Topic: https://groups.io/mt/86739959/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi,
> +UINT64 mTdSharedPageMask = 0;
> +UINT32 mTdMaxVCpuNum = 0;
> +UINT32 mTdVCpuNum = 0;
> +UINT64
> +EFIAPI
> +TdSharedPageMask (
> + VOID
> + )
> +{
> + UINT64 Status;
> + UINT8 Gpaw;
> + TD_RETURN_DATA TdReturnData;
> +
> + if (mTdSharedPageMask != 0) {
> + return mTdSharedPageMask;
> + }
Small possible optimization: you can cache the whole TD_RETURN_DATA
struct instead of the three extracted values, then ...
> + Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
> + ASSERT (Status == TDX_EXIT_REASON_SUCCESS);
... you need a single TDCALL_TDINFO call only.
> + tdcall
> +
> + ; Panic if TDCALL reports failure.
> + test rax, rax
> + jnz .no_return_data
> +.panic:
> + ud2
Comment doesn't match code. jnz .panic ?
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83135): https://edk2.groups.io/g/devel/message/83135
Mute This Topic: https://groups.io/mt/86739959/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On November 2, 2021 10:07 PM, Gerd Hoffmann wrote:
> Hi,
>
> > +UINT64 mTdSharedPageMask = 0;
> > +UINT32 mTdMaxVCpuNum = 0;
> > +UINT32 mTdVCpuNum = 0;
>
> > +UINT64
> > +EFIAPI
> > +TdSharedPageMask (
> > + VOID
> > + )
> > +{
> > + UINT64 Status;
> > + UINT8 Gpaw;
> > + TD_RETURN_DATA TdReturnData;
> > +
> > + if (mTdSharedPageMask != 0) {
> > + return mTdSharedPageMask;
> > + }
>
> Small possible optimization: you can cache the whole TD_RETURN_DATA struct
> instead of the three extracted values, then ...
>
> > + Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData); ASSERT
> > + (Status == TDX_EXIT_REASON_SUCCESS);
>
> ... you need a single TDCALL_TDINFO call only.
Thanks for reminder. It will be updated in the next version.
>
> > + tdcall
> > +
> > + ; Panic if TDCALL reports failure.
> > + test rax, rax
> > + jnz .no_return_data
>
> > +.panic:
> > + ud2
>
> Comment doesn't match code. jnz .panic ?
Ah, good catch. It should jnz .panic. It will be fixed in the next version.
Thanks
Min
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83539): https://edk2.groups.io/g/devel/message/83539
Mute This Topic: https://groups.io/mt/86739959/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Min,
Sorry for the late review. My comments and a few questions are inline.
Thanks
-Erdem
On Mon, Nov 1, 2021 at 6:16 AM Min Xu <min.m.xu@intel.com> wrote:
>
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
>
....
> +**/
> +UINT32
> +GetGpaPageLevel (
> + UINT32 PageSize
> + )
> +{
> + UINT32 Index;
> +
> + for (Index = 0; Index < ARRAY_SIZE (mTdxAcceptPageLevelMap); Index++) {
> + if (mTdxAcceptPageLevelMap[Index] == PageSize) {
> + break;
> + }
> + }
> +
> + return Index == ARRAY_SIZE (mTdxAcceptPageLevelMap) ? -1 : Index;
Is this intentional? Returning signed integer but the function returns
unsigned integer.
+/**
+ This function accepts a pending private page, and initialize the page to
+ all-0 using the TD ephemeral private key.
+
+ @param[in] StartAddress Guest physical address of the private page
+ to accept.
+ @param[in] NumberOfPages Number of the pages to be accepted.
+ @param[in] PageSize GPA page size. Accept 1G/2M/4K page size.
The comment says that 1G is acceptable but the code only accepts 2M or
4K page sizes.
+
+ @return EFI_SUCCESS
> +EFI_STATUS
> +EFIAPI
> +TdAcceptPages (
> + IN UINT64 StartAddress,
> + IN UINT64 NumberOfPages,
> + IN UINT32 PageSize
> + )
> +{
> + EFI_STATUS Status;
> + UINT64 Address;
> + UINT64 TdxStatus;
> + UINT64 Index;
> + UINT32 GpaPageLevel;
> + UINT32 PageSize2;
> +
> + Address = StartAddress;
> +
> + GpaPageLevel = GetGpaPageLevel (PageSize);
> + if (GpaPageLevel == -1) {
Comparing unsigned int to signed int. I believe this should work with
GCC with warning messages.
Just checking if this is intentional?
> + DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page size - 0x%llx\n", PageSize));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Status = EFI_SUCCESS;
> + for (Index = 0; Index < NumberOfPages; Index++) {
> + TdxStatus = TdCall (TDCALL_TDACCEPTPAGE, Address | GpaPageLevel, 0, 0, 0);
Address[11:3] and [63:52] are reserved and must be 0. The code does
not check it, spec is not clear about the behavior but I am assuming
that in that case, TDX_OPERAND_INVALID will be returned as error code
but should we check and log it properly?
> + if (TdxStatus != TDX_EXIT_REASON_SUCCESS) {
> + if ((TdxStatus & ~0xFFFFULL) == TDX_EXIT_REASON_PAGE_ALREADY_ACCEPTED) {
> + //
> + // Already accepted
> + //
> + mNumberOfDuplicatedAcceptedPages++;
Is there any legit case that a page should be accepted twice? Looks
like other than debug printing, this information is ignored.
> + DEBUG ((DEBUG_WARN, "Page at Address (0x%llx) has already been accepted. - %d\n", Address, mNumberOfDuplicatedAcceptedPages));
> + } else if ((TdxStatus & ~0xFFFFULL) == TDX_EXIT_REASON_PAGE_SIZE_MISMATCH) {
> + //
> + // GpaPageLevel is mismatch, fall back to a smaller GpaPageLevel if possible
> + //
> + DEBUG ((DEBUG_VERBOSE, "Address %llx cannot be accepted in PageLevel of %d\n", Address, GpaPageLevel));
> +
> + if (GpaPageLevel == 0) {
> + //
> + // Cannot fall back to smaller page level
> + //
Could you help me understand this? So if ,for some reason, VMM maps a
2MB private page and a guest wants to accept it in 4KB page chunks,
this will always fail. Is it not a possible use case?
> + DEBUG ((DEBUG_ERROR, "AcceptPage cannot fallback from PageLevel %d\n", GpaPageLevel));
> + Status = EFI_INVALID_PARAMETER;
> + break;
> + } else {
> + //
> + // Fall back to a smaller page size
> + //
> + PageSize2 = mTdxAcceptPageLevelMap [GpaPageLevel - 1];
> + Status = TdAcceptPages(Address, 512, PageSize2);
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> + }
> + }else {
> +
> + //
> + // Other errors
> + //
Why not handle the TDX_OPERAND_BUSY? It is not an error and tdaccept
needs to be retired, I guess, handling it like an error might cause
reliability issues.
> + DEBUG ((DEBUG_ERROR, "Address %llx (%d) failed to be accepted. Error = 0x%llx\n",
> + Address, Index, TdxStatus));
> + Status = EFI_INVALID_PARAMETER;
> + break;
> + }
> + }
> + Address += PageSize;
> + }
> + return Status;
> +}
.....
> +**/
> +EFI_STATUS
> +EFIAPI
> +TdExtendRtmr (
> + IN UINT32 *Data,
> + IN UINT32 DataLen,
> + IN UINT8 Index
> + )
> +{
> + EFI_STATUS Status;
> + UINT64 TdCallStatus;
> + UINT8 *ExtendBuffer;
> +
> + Status = EFI_SUCCESS;
> +
> + ASSERT (Data != NULL);
> + ASSERT (DataLen == SHA384_DIGEST_SIZE);
> + ASSERT (Index >= 0 && Index < RTMR_COUNT);
> +
Because of the Asserts above , the following condition will never run, right?
> + if (Data == NULL || DataLen != SHA384_DIGEST_SIZE || Index >= RTMR_COUNT) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
...
> +; UINT64
> +; EFIAPI
> +; TdVmCall (
> +; UINT64 Leaf, // Rcx
> +; UINT64 P1, // Rdx
> +; UINT64 P2, // R8
> +; UINT64 P3, // R9
> +; UINT64 P4, // rsp + 0x28
> +; UINT64 *Val // rsp + 0x30
> +; )
> +global ASM_PFX(TdVmCall)
> +ASM_PFX(TdVmCall):
> + tdcall_push_regs
> +
> + mov r11, rcx
> + mov r12, rdx
> + mov r13, r8
> + mov r14, r9
> + mov r15, [rsp + first_variable_on_stack_offset ]
> +
> + tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
> +
> + tdcall
> +
> + ; ignore return dataif TDCALL reports failure.
should we not panic here also?
> + test rax, rax
> + jnz .no_return_data
> +
> + ; Propagate TDVMCALL success/failure to return value.
> + mov rax, r10
> +
> + ; Retrieve the Val pointer.
> + mov r9, [rsp + second_variable_on_stack_offset ]
> + test r9, r9
> + jz .no_return_data
> +
> + ; On success, propagate TDVMCALL output value to output param
> + test rax, rax
> + jnz .no_return_data
> + mov [r9], r11
> +.no_return_data:
> + tdcall_regs_postamble
> +
> + tdcall_pop_regs
> +
> + ret
> +
> +;------------------------------------------------------------------------------
> +; 0 => RAX = TDCALL leaf
> +; M => RCX = TDVMCALL register behavior
> +; 1 => R10 = standard vs. vendor
> +; RDI => R11 = TDVMCALL function / nr
> +; RSI = R12 = p1
> +; RDX => R13 = p2
> +; RCX => R14 = p3
> +; R8 => R15 = p4
> +
Above comment does not match the below definition.
> +; UINT64
> +; EFIAPI
> +; TdVmCallCpuid (
> +; UINT64 EaxIn, // Rcx
> +; UINT64 EcxIn, // Rdx
> +; UINT64 *Results // R8
> +; )
> +global ASM_PFX(TdVmCallCpuid)
> +ASM_PFX(TdVmCallCpuid):
> + tdcall_push_regs
> +
> + mov r11, EXIT_REASON_CPUID
> + mov r12, rcx
> + mov r13, rdx
> +
> + ; Save *results pointers
> + push r8
> +
Looks like we are leaking r14 and r15 here.
> + tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
> +
> + tdcall
> +
> + ; Panic if TDCALL reports failure.
> + test rax, rax
> + jnz .no_return_data
> +
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83543): https://edk2.groups.io/g/devel/message/83543
Mute This Topic: https://groups.io/mt/86739959/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On November 10, 2021 6:38 PM, Erdem Aktas wrote:
> On Mon, Nov 1, 2021 at 6:16 AM Min Xu <min.m.xu@intel.com> wrote:
> >
> > RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> >
> ....
> > +**/
> > +UINT32
> > +GetGpaPageLevel (
> > + UINT32 PageSize
> > + )
> > +{
> > + UINT32 Index;
> > +
> > + for (Index = 0; Index < ARRAY_SIZE (mTdxAcceptPageLevelMap); Index++)
> {
> > + if (mTdxAcceptPageLevelMap[Index] == PageSize) {
> > + break;
> > + }
> > + }
> > +
> > + return Index == ARRAY_SIZE (mTdxAcceptPageLevelMap) ? -1 : Index;
>
> Is this intentional? Returning signed integer but the function returns
> unsigned integer.
Ah, -1 should not be returned because the function returns unsigned integer.
It will be fixed in the next version like this:
UINT32 mTdxAcceptPageLevelMap[2] = {
SIZE_4KB,
SIZE_2MB
};
#define INVALID_ACCEPT_PAGELEVEL ARRAY_SIZE(mTdxAcceptPageLevelMap)
UINT32
GetGpaPageLevel (
UINT32 PageSize
)
{
UINT32 Index;
for (Index = 0; Index < ARRAY_SIZE (mTdxAcceptPageLevelMap); Index++) {
if (mTdxAcceptPageLevelMap[Index] == PageSize) {
break;
}
}
return Index;
}
... ...
GpaPageLevel = GetGpaPageLevel (PageSize);
if (GpaPageLevel == INVALID_ACCEPT_PAGELEVEL) {
DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page size - 0x%llx\n", PageSize));
return EFI_INVALID_PARAMETER;
}
>
> +/**
> + This function accepts a pending private page, and initialize the page
> +to
> + all-0 using the TD ephemeral private key.
> +
> + @param[in] StartAddress Guest physical address of the private page
> + to accept.
> + @param[in] NumberOfPages Number of the pages to be accepted.
> + @param[in] PageSize GPA page size. Accept 1G/2M/4K page size.
>
> The comment says that 1G is acceptable but the code only accepts 2M or 4K
> page sizes.
Currently 2M/4K accept page size is supported. The comments will be fixed in the next version.
>
> +
> + @return EFI_SUCCESS
> > +EFI_STATUS
> > +EFIAPI
> > +TdAcceptPages (
> > + IN UINT64 StartAddress,
> > + IN UINT64 NumberOfPages,
> > + IN UINT32 PageSize
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT64 Address;
> > + UINT64 TdxStatus;
> > + UINT64 Index;
> > + UINT32 GpaPageLevel;
> > + UINT32 PageSize2;
> > +
> > + Address = StartAddress;
> > +
> > + GpaPageLevel = GetGpaPageLevel (PageSize); if (GpaPageLevel == -1)
> > + {
>
> Comparing unsigned int to signed int. I believe this should work with GCC
> with warning messages.
> Just checking if this is intentional?
It will be fixed. See my first comments.
>
> > + DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page
> size - 0x%llx\n", PageSize));
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + Status = EFI_SUCCESS;
> > + for (Index = 0; Index < NumberOfPages; Index++) {
> > + TdxStatus = TdCall (TDCALL_TDACCEPTPAGE, Address | GpaPageLevel,
> > + 0, 0, 0);
>
> Address[11:3] and [63:52] are reserved and must be 0. The code does not
> check it, spec is not clear about the behavior but I am assuming that in that
> case, TDX_OPERAND_INVALID will be returned as error code but should we
> check and log it properly?
Right. The input address should be checked with Address[11:3] and [63:52].
Address[2:0] should be 0 too. Because Address[2:0] will be set with Accept Page Level.
It will be fixed in the next version.
>
> > + if (TdxStatus != TDX_EXIT_REASON_SUCCESS) {
> > + if ((TdxStatus & ~0xFFFFULL) ==
> TDX_EXIT_REASON_PAGE_ALREADY_ACCEPTED) {
> > + //
> > + // Already accepted
> > + //
> > + mNumberOfDuplicatedAcceptedPages++;
>
> Is there any legit case that a page should be accepted twice? Looks like other
> than debug printing, this information is ignored.
Ideally a page should be accepted only once. But if a page is accepted twice, it is not an error (from the perspective of TdCall_Accept). In the PoC we do ran into such cases (it is a bug in the code). So we keep it as debug printing.
>
> > + DEBUG ((DEBUG_WARN, "Page at Address (0x%llx) has already been
> accepted. - %d\n", Address, mNumberOfDuplicatedAcceptedPages));
> > + } else if ((TdxStatus & ~0xFFFFULL) ==
> TDX_EXIT_REASON_PAGE_SIZE_MISMATCH) {
> > + //
> > + // GpaPageLevel is mismatch, fall back to a smaller GpaPageLevel if
> possible
> > + //
> > + DEBUG ((DEBUG_VERBOSE, "Address %llx cannot be accepted in
> > + PageLevel of %d\n", Address, GpaPageLevel));
> > +
> > + if (GpaPageLevel == 0) {
> > + //
> > + // Cannot fall back to smaller page level
> > + //
>
> Could you help me understand this? So if ,for some reason, VMM maps a
> 2MB private page and a guest wants to accept it in 4KB page chunks, this will
> always fail. Is it not a possible use case?
Guest want to accept page with 2M size, but in some case, the VMM cannot do that with 2M page size. In this case, Guest will get a returned result (TDX_EXIT_REASON_PAGE_SIZE_MISMATCH) from the TdCall. So Guest falls back to accept the page with a smaller page size. Currently only 2M/4K accept page size is supported. In the future, 1G accept page size will be supported. In that case, there may be 2 fall backs: 1G->2M and 2M->4K. But if the accept page size is 4K, it cannot fall back to a smaller size.
Guest start to accept pages, VMM just response to the accept page command from Guest.
>
> > + DEBUG ((DEBUG_ERROR, "AcceptPage cannot fallback from
> PageLevel %d\n", GpaPageLevel));
> > + Status = EFI_INVALID_PARAMETER;
> > + break;
> > + } else {
> > + //
> > + // Fall back to a smaller page size
> > + //
> > + PageSize2 = mTdxAcceptPageLevelMap [GpaPageLevel - 1];
> > + Status = TdAcceptPages(Address, 512, PageSize2);
> > + if (EFI_ERROR (Status)) {
> > + break;
> > + }
> > + }
> > + }else {
> > +
> > + //
> > + // Other errors
> > + //
>
> Why not handle the TDX_OPERAND_BUSY? It is not an error and tdaccept
> needs to be retired, I guess, handling it like an error might cause reliability
> issues.
Thanks for reminder. It will be fixed in the next version.
>
> > + DEBUG ((DEBUG_ERROR, "Address %llx (%d) failed to be accepted.
> Error = 0x%llx\n",
> > + Address, Index, TdxStatus));
> > + Status = EFI_INVALID_PARAMETER;
> > + break;
> > + }
> > + }
> > + Address += PageSize;
> > + }
> > + return Status;
> > +}
>
> .....
>
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +TdExtendRtmr (
> > + IN UINT32 *Data,
> > + IN UINT32 DataLen,
> > + IN UINT8 Index
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT64 TdCallStatus;
> > + UINT8 *ExtendBuffer;
> > +
> > + Status = EFI_SUCCESS;
> > +
> > + ASSERT (Data != NULL);
> > + ASSERT (DataLen == SHA384_DIGEST_SIZE); ASSERT (Index >= 0 &&
> > + Index < RTMR_COUNT);
> > +
>
> Because of the Asserts above , the following condition will never run, right?
ASSERT () is for debugging purpose. It will be ignored in release mode. So the following condition will run in release mode.
See https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/DebugLib.h#L385-L409
>
> > + if (Data == NULL || DataLen != SHA384_DIGEST_SIZE || Index >=
> RTMR_COUNT) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
>
> ...
>
> > +; UINT64
> > +; EFIAPI
> > +; TdVmCall (
> > +; UINT64 Leaf, // Rcx
> > +; UINT64 P1, // Rdx
> > +; UINT64 P2, // R8
> > +; UINT64 P3, // R9
> > +; UINT64 P4, // rsp + 0x28
> > +; UINT64 *Val // rsp + 0x30
> > +; )
> > +global ASM_PFX(TdVmCall)
> > +ASM_PFX(TdVmCall):
> > + tdcall_push_regs
> > +
> > + mov r11, rcx
> > + mov r12, rdx
> > + mov r13, r8
> > + mov r14, r9
> > + mov r15, [rsp + first_variable_on_stack_offset ]
> > +
> > + tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
> > +
> > + tdcall
> > +
> > + ; ignore return dataif TDCALL reports failure.
>
> should we not panic here also?
I don't think we should panic here.
Because TdVmCall is a common function (see sub-leaf of TDVMCALL). There are various errors. Some of them are not so serious that panic is needed. Caller of TdVmCall will handle the returned result.
>
> > + test rax, rax
> > + jnz .no_return_data
> > +
> > + ; Propagate TDVMCALL success/failure to return value.
> > + mov rax, r10
> > +
> > + ; Retrieve the Val pointer.
> > + mov r9, [rsp + second_variable_on_stack_offset ]
> > + test r9, r9
> > + jz .no_return_data
> > +
> > + ; On success, propagate TDVMCALL output value to output param
> > + test rax, rax
> > + jnz .no_return_data
> > + mov [r9], r11
> > +.no_return_data:
> > + tdcall_regs_postamble
> > +
> > + tdcall_pop_regs
> > +
> > + ret
> > +
> > +;------------------------------------------------------------------------------
> > +; 0 => RAX = TDCALL leaf
> > +; M => RCX = TDVMCALL register behavior
> > +; 1 => R10 = standard vs. vendor
> > +; RDI => R11 = TDVMCALL function / nr ; RSI = R12 = p1 ; RDX => R13
> > += p2 ; RCX => R14 = p3 ; R8 => R15 = p4
> > +
>
> Above comment does not match the below definition.
Thanks for reminder. It will be updated in the next version.
>
> > +; UINT64
> > +; EFIAPI
> > +; TdVmCallCpuid (
> > +; UINT64 EaxIn, // Rcx
> > +; UINT64 EcxIn, // Rdx
> > +; UINT64 *Results // R8
> > +; )
> > +global ASM_PFX(TdVmCallCpuid)
> > +ASM_PFX(TdVmCallCpuid):
> > + tdcall_push_regs
> > +
> > + mov r11, EXIT_REASON_CPUID
> > + mov r12, rcx
> > + mov r13, rdx
> > +
> > + ; Save *results pointers
> > + push r8
> > +
>
> Looks like we are leaking r14 and r15 here.
Thanks for reminder. It will be fixed in the next version.
>
> > + tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
> > +
> > + tdcall
> > +
> > + ; Panic if TDCALL reports failure.
> > + test rax, rax
> > + jnz .no_return_data
> > +
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83675): https://edk2.groups.io/g/devel/message/83675
Mute This Topic: https://groups.io/mt/86739959/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
BTW: Is this internal API?
I feel we should add ASSERT() for invalid page size as well, to catch issue earlier.
Thank you
> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Friday, November 12, 2021 10:39 AM
> To: Erdem Aktas <erdemaktas@google.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>;
> Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <Thomas.Lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: RE: [PATCH V3 02/29] MdePkg: Add TdxLib to wrap Tdx operations
>
> On November 10, 2021 6:38 PM, Erdem Aktas wrote:
> > On Mon, Nov 1, 2021 at 6:16 AM Min Xu <min.m.xu@intel.com> wrote:
> > >
> > > RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> > >
> > ....
> > > +**/
> > > +UINT32
> > > +GetGpaPageLevel (
> > > + UINT32 PageSize
> > > + )
> > > +{
> > > + UINT32 Index;
> > > +
> > > + for (Index = 0; Index < ARRAY_SIZE (mTdxAcceptPageLevelMap); Index++)
> > {
> > > + if (mTdxAcceptPageLevelMap[Index] == PageSize) {
> > > + break;
> > > + }
> > > + }
> > > +
> > > + return Index == ARRAY_SIZE (mTdxAcceptPageLevelMap) ? -1 : Index;
> >
> > Is this intentional? Returning signed integer but the function returns
> > unsigned integer.
> Ah, -1 should not be returned because the function returns unsigned integer.
> It will be fixed in the next version like this:
>
> UINT32 mTdxAcceptPageLevelMap[2] = {
> SIZE_4KB,
> SIZE_2MB
> };
> #define INVALID_ACCEPT_PAGELEVEL ARRAY_SIZE(mTdxAcceptPageLevelMap)
>
> UINT32
> GetGpaPageLevel (
> UINT32 PageSize
> )
> {
> UINT32 Index;
> for (Index = 0; Index < ARRAY_SIZE (mTdxAcceptPageLevelMap); Index++) {
> if (mTdxAcceptPageLevelMap[Index] == PageSize) {
> break;
> }
> }
> return Index;
> }
> ... ...
> GpaPageLevel = GetGpaPageLevel (PageSize);
> if (GpaPageLevel == INVALID_ACCEPT_PAGELEVEL) {
> DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page size -
> 0x%llx\n", PageSize));
> return EFI_INVALID_PARAMETER;
> }
> >
> > +/**
> > + This function accepts a pending private page, and initialize the page
> > +to
> > + all-0 using the TD ephemeral private key.
> > +
> > + @param[in] StartAddress Guest physical address of the private page
> > + to accept.
> > + @param[in] NumberOfPages Number of the pages to be accepted.
> > + @param[in] PageSize GPA page size. Accept 1G/2M/4K page size.
> >
> > The comment says that 1G is acceptable but the code only accepts 2M or 4K
> > page sizes.
> Currently 2M/4K accept page size is supported. The comments will be fixed in
> the next version.
> >
> > +
> > + @return EFI_SUCCESS
> > > +EFI_STATUS
> > > +EFIAPI
> > > +TdAcceptPages (
> > > + IN UINT64 StartAddress,
> > > + IN UINT64 NumberOfPages,
> > > + IN UINT32 PageSize
> > > + )
> > > +{
> > > + EFI_STATUS Status;
> > > + UINT64 Address;
> > > + UINT64 TdxStatus;
> > > + UINT64 Index;
> > > + UINT32 GpaPageLevel;
> > > + UINT32 PageSize2;
> > > +
> > > + Address = StartAddress;
> > > +
> > > + GpaPageLevel = GetGpaPageLevel (PageSize); if (GpaPageLevel == -1)
> > > + {
> >
> > Comparing unsigned int to signed int. I believe this should work with GCC
> > with warning messages.
> > Just checking if this is intentional?
> It will be fixed. See my first comments.
> >
> > > + DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page
> > size - 0x%llx\n", PageSize));
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > + Status = EFI_SUCCESS;
> > > + for (Index = 0; Index < NumberOfPages; Index++) {
> > > + TdxStatus = TdCall (TDCALL_TDACCEPTPAGE, Address | GpaPageLevel,
> > > + 0, 0, 0);
> >
> > Address[11:3] and [63:52] are reserved and must be 0. The code does not
> > check it, spec is not clear about the behavior but I am assuming that in that
> > case, TDX_OPERAND_INVALID will be returned as error code but should we
> > check and log it properly?
> Right. The input address should be checked with Address[11:3] and [63:52].
> Address[2:0] should be 0 too. Because Address[2:0] will be set with Accept Page
> Level.
> It will be fixed in the next version.
> >
> > > + if (TdxStatus != TDX_EXIT_REASON_SUCCESS) {
> > > + if ((TdxStatus & ~0xFFFFULL) ==
> > TDX_EXIT_REASON_PAGE_ALREADY_ACCEPTED) {
> > > + //
> > > + // Already accepted
> > > + //
> > > + mNumberOfDuplicatedAcceptedPages++;
> >
> > Is there any legit case that a page should be accepted twice? Looks like other
> > than debug printing, this information is ignored.
> Ideally a page should be accepted only once. But if a page is accepted twice, it is
> not an error (from the perspective of TdCall_Accept). In the PoC we do ran into
> such cases (it is a bug in the code). So we keep it as debug printing.
> >
> > > + DEBUG ((DEBUG_WARN, "Page at Address (0x%llx) has already been
> > accepted. - %d\n", Address, mNumberOfDuplicatedAcceptedPages));
> > > + } else if ((TdxStatus & ~0xFFFFULL) ==
> > TDX_EXIT_REASON_PAGE_SIZE_MISMATCH) {
> > > + //
> > > + // GpaPageLevel is mismatch, fall back to a smaller GpaPageLevel if
> > possible
> > > + //
> > > + DEBUG ((DEBUG_VERBOSE, "Address %llx cannot be accepted in
> > > + PageLevel of %d\n", Address, GpaPageLevel));
> > > +
> > > + if (GpaPageLevel == 0) {
> > > + //
> > > + // Cannot fall back to smaller page level
> > > + //
> >
> > Could you help me understand this? So if ,for some reason, VMM maps a
> > 2MB private page and a guest wants to accept it in 4KB page chunks, this will
> > always fail. Is it not a possible use case?
> Guest want to accept page with 2M size, but in some case, the VMM cannot do
> that with 2M page size. In this case, Guest will get a returned result
> (TDX_EXIT_REASON_PAGE_SIZE_MISMATCH) from the TdCall. So Guest falls
> back to accept the page with a smaller page size. Currently only 2M/4K accept
> page size is supported. In the future, 1G accept page size will be supported. In
> that case, there may be 2 fall backs: 1G->2M and 2M->4K. But if the accept page
> size is 4K, it cannot fall back to a smaller size.
> Guest start to accept pages, VMM just response to the accept page command
> from Guest.
> >
> > > + DEBUG ((DEBUG_ERROR, "AcceptPage cannot fallback from
> > PageLevel %d\n", GpaPageLevel));
> > > + Status = EFI_INVALID_PARAMETER;
> > > + break;
> > > + } else {
> > > + //
> > > + // Fall back to a smaller page size
> > > + //
> > > + PageSize2 = mTdxAcceptPageLevelMap [GpaPageLevel - 1];
> > > + Status = TdAcceptPages(Address, 512, PageSize2);
> > > + if (EFI_ERROR (Status)) {
> > > + break;
> > > + }
> > > + }
> > > + }else {
> > > +
> > > + //
> > > + // Other errors
> > > + //
> >
> > Why not handle the TDX_OPERAND_BUSY? It is not an error and tdaccept
> > needs to be retired, I guess, handling it like an error might cause reliability
> > issues.
> Thanks for reminder. It will be fixed in the next version.
> >
> > > + DEBUG ((DEBUG_ERROR, "Address %llx (%d) failed to be accepted.
> > Error = 0x%llx\n",
> > > + Address, Index, TdxStatus));
> > > + Status = EFI_INVALID_PARAMETER;
> > > + break;
> > > + }
> > > + }
> > > + Address += PageSize;
> > > + }
> > > + return Status;
> > > +}
> >
> > .....
> >
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +TdExtendRtmr (
> > > + IN UINT32 *Data,
> > > + IN UINT32 DataLen,
> > > + IN UINT8 Index
> > > + )
> > > +{
> > > + EFI_STATUS Status;
> > > + UINT64 TdCallStatus;
> > > + UINT8 *ExtendBuffer;
> > > +
> > > + Status = EFI_SUCCESS;
> > > +
> > > + ASSERT (Data != NULL);
> > > + ASSERT (DataLen == SHA384_DIGEST_SIZE); ASSERT (Index >= 0 &&
> > > + Index < RTMR_COUNT);
> > > +
> >
> > Because of the Asserts above , the following condition will never run, right?
> ASSERT () is for debugging purpose. It will be ignored in release mode. So the
> following condition will run in release mode.
> See
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/Debu
> gLib.h#L385-L409
> >
> > > + if (Data == NULL || DataLen != SHA384_DIGEST_SIZE || Index >=
> > RTMR_COUNT) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> >
> > ...
> >
> > > +; UINT64
> > > +; EFIAPI
> > > +; TdVmCall (
> > > +; UINT64 Leaf, // Rcx
> > > +; UINT64 P1, // Rdx
> > > +; UINT64 P2, // R8
> > > +; UINT64 P3, // R9
> > > +; UINT64 P4, // rsp + 0x28
> > > +; UINT64 *Val // rsp + 0x30
> > > +; )
> > > +global ASM_PFX(TdVmCall)
> > > +ASM_PFX(TdVmCall):
> > > + tdcall_push_regs
> > > +
> > > + mov r11, rcx
> > > + mov r12, rdx
> > > + mov r13, r8
> > > + mov r14, r9
> > > + mov r15, [rsp + first_variable_on_stack_offset ]
> > > +
> > > + tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
> > > +
> > > + tdcall
> > > +
> > > + ; ignore return dataif TDCALL reports failure.
> >
> > should we not panic here also?
> I don't think we should panic here.
> Because TdVmCall is a common function (see sub-leaf of TDVMCALL). There are
> various errors. Some of them are not so serious that panic is needed. Caller of
> TdVmCall will handle the returned result.
> >
> > > + test rax, rax
> > > + jnz .no_return_data
> > > +
> > > + ; Propagate TDVMCALL success/failure to return value.
> > > + mov rax, r10
> > > +
> > > + ; Retrieve the Val pointer.
> > > + mov r9, [rsp + second_variable_on_stack_offset ]
> > > + test r9, r9
> > > + jz .no_return_data
> > > +
> > > + ; On success, propagate TDVMCALL output value to output param
> > > + test rax, rax
> > > + jnz .no_return_data
> > > + mov [r9], r11
> > > +.no_return_data:
> > > + tdcall_regs_postamble
> > > +
> > > + tdcall_pop_regs
> > > +
> > > + ret
> > > +
> > > +;------------------------------------------------------------------------------
> > > +; 0 => RAX = TDCALL leaf
> > > +; M => RCX = TDVMCALL register behavior
> > > +; 1 => R10 = standard vs. vendor
> > > +; RDI => R11 = TDVMCALL function / nr ; RSI = R12 = p1 ; RDX => R13
> > > += p2 ; RCX => R14 = p3 ; R8 => R15 = p4
> > > +
> >
> > Above comment does not match the below definition.
> Thanks for reminder. It will be updated in the next version.
> >
> > > +; UINT64
> > > +; EFIAPI
> > > +; TdVmCallCpuid (
> > > +; UINT64 EaxIn, // Rcx
> > > +; UINT64 EcxIn, // Rdx
> > > +; UINT64 *Results // R8
> > > +; )
> > > +global ASM_PFX(TdVmCallCpuid)
> > > +ASM_PFX(TdVmCallCpuid):
> > > + tdcall_push_regs
> > > +
> > > + mov r11, EXIT_REASON_CPUID
> > > + mov r12, rcx
> > > + mov r13, rdx
> > > +
> > > + ; Save *results pointers
> > > + push r8
> > > +
> >
> > Looks like we are leaking r14 and r15 here.
> Thanks for reminder. It will be fixed in the next version.
> >
> > > + tdcall_regs_preamble TDVMCALL, TDVMCALL_EXPOSE_REGS_MASK
> > > +
> > > + tdcall
> > > +
> > > + ; Panic if TDCALL reports failure.
> > > + test rax, rax
> > > + jnz .no_return_data
> > > +
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83676): https://edk2.groups.io/g/devel/message/83676
Mute This Topic: https://groups.io/mt/86739959/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On November 12, 2021 10:42 AM, Yao Jiewen wrote: > BTW: Is this internal API? > I feel we should add ASSERT() for invalid page size as well, to catch issue > earlier. TdAcceptPages () is not an internal API. It is exposed in TdxLib. Sure, I will add more check to the input parameter in the code. Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#83682): https://edk2.groups.io/g/devel/message/83682 Mute This Topic: https://groups.io/mt/86739959/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Sorry for the confusing. My word is not accurate. When I say "internal" in this context, I really mean: it is produced and consumed by OVMF. The API will not be called by *third party*, such as OS loader, or Option ROM. If so, it is OK to add ASSERT. If you look at the BaseLib, many functions add ASSERT. It can help to check the usage of API is correctly. Thank you Yao Jiewen > -----Original Message----- > From: Xu, Min M <min.m.xu@intel.com> > Sent: Friday, November 12, 2021 1:30 PM > To: Yao, Jiewen <jiewen.yao@intel.com>; Erdem Aktas > <erdemaktas@google.com> > Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; > Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang > <zhiguang.liu@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; James > Bottomley <jejb@linux.ibm.com>; Tom Lendacky > <Thomas.Lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com> > Subject: RE: [PATCH V3 02/29] MdePkg: Add TdxLib to wrap Tdx operations > > On November 12, 2021 10:42 AM, Yao Jiewen wrote: > > BTW: Is this internal API? > > I feel we should add ASSERT() for invalid page size as well, to catch issue > > earlier. > TdAcceptPages () is not an internal API. It is exposed in TdxLib. > Sure, I will add more check to the input parameter in the code. > > Thanks > Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#83683): https://edk2.groups.io/g/devel/message/83683 Mute This Topic: https://groups.io/mt/86739959/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.