TDX memory has integrity and confidentiality protections. Violations of
this integrity protection are supposed to only affect TDX operations and
are never supposed to affect the host kernel itself. In other words,
the host kernel should never, itself, see machine checks induced by the
TDX integrity hardware.
Alas, the first few generations of TDX hardware have an erratum. A
partial write to a TDX private memory cacheline will silently "poison"
the line. Subsequent reads will consume the poison and generate a
machine check. According to the TDX hardware spec, neither of these
things should have happened.
On the platform with this erratum, it would be nice if the #MC handler
could print additional information to show the #MC was TDX private
memory error due to possible kernel bug.
To do that, the machine check handler needs to use SEAMCALL to query
page type of the error memory from the TDX module, because there's no
existing infrastructure to track TDX private pages.
SEAMCALL instruction causes #UD if CPU isn't in VMX operation. In #MC
handler, it is legal that CPU isn't in VMX operation when making this
SEAMCALL. Extend the TDX_MODULE_CALL macro to handle #UD so the
SEAMCALL can return error code instead of Oops in the #MC handler.
Opportunistically handles #GP too since they share the same code.
A bonus is when kernel mistakenly calls SEAMCALL when CPU isn't in VMX
operation, or when TDX isn't enabled by the BIOS, or when the BIOS is
buggy, the kernel can get a nicer error message rather than a less
understandable Oops.
This is basically based on Peter's code.
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---
v2 -> v3:
- Added more material to the changelog to explain the TDX erratum
(hoping it can be merged together with this series).
- Changed to use %rdi to hold $TDX_SW_ERROR instead of %r12 as the
latter is callee-saved.
v1 -> v2:
- Skip saving output registers when SEAMCALL #UD/#GP
---
arch/x86/include/asm/tdx.h | 4 ++++
arch/x86/virt/vmx/tdx/tdxcall.S | 19 +++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index a69bb7d3061b..adcbe3f1de30 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -8,6 +8,7 @@
#include <asm/errno.h>
#include <asm/ptrace.h>
+#include <asm/trapnr.h>
#include <asm/shared/tdx.h>
/*
@@ -20,6 +21,9 @@
#define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40))
#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000))
+#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP)
+#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)
+
#ifndef __ASSEMBLY__
/*
diff --git a/arch/x86/virt/vmx/tdx/tdxcall.S b/arch/x86/virt/vmx/tdx/tdxcall.S
index 3ed6d8b8d2a9..ccb7ff08078a 100644
--- a/arch/x86/virt/vmx/tdx/tdxcall.S
+++ b/arch/x86/virt/vmx/tdx/tdxcall.S
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
#include <asm/asm-offsets.h>
#include <asm/frame.h>
+#include <asm/asm.h>
#include <asm/tdx.h>
/*
@@ -85,6 +86,7 @@
.endif /* \saved */
.if \host
+.Lseamcall\@:
seamcall
/*
* SEAMCALL instruction is essentially a VMExit from VMX root
@@ -192,11 +194,28 @@
.if \host
.Lseamcall_vmfailinvalid\@:
mov $TDX_SEAMCALL_VMFAILINVALID, %rax
+ jmp .Lseamcall_fail\@
+
+.Lseamcall_trap\@:
+ /*
+ * SEAMCALL caused #GP or #UD. By reaching here %eax contains
+ * the trap number. Convert the trap number to the TDX error
+ * code by setting TDX_SW_ERROR to the high 32-bits of %rax.
+ *
+ * Note cannot OR TDX_SW_ERROR directly to %rax as OR instruction
+ * only accepts 32-bit immediate at most.
+ */
+ movq $TDX_SW_ERROR, %rdi
+ orq %rdi, %rax
+
+.Lseamcall_fail\@:
.if \ret && \saved
/* pop the unused structure pointer back to %rsi */
popq %rsi
.endif
jmp .Lout\@
+
+ _ASM_EXTABLE_FAULT(.Lseamcall\@, .Lseamcall_trap\@)
.endif /* \host */
.endm
--
2.41.0On Wed, Jul 26, 2023 at 11:25:13PM +1200, Kai Huang wrote: > @@ -20,6 +21,9 @@ > #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40)) > #define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000)) > > +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) > +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) Is there any explantion how these error codes got chosen? Looks very arbitrary and may collide with other error codes in the future. -- Kiryl Shutsemau / Kirill A. Shutemov
On 8/6/23 04:41, kirill.shutemov@linux.intel.com wrote: > On Wed, Jul 26, 2023 at 11:25:13PM +1200, Kai Huang wrote: >> @@ -20,6 +21,9 @@ >> #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40)) >> #define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000)) >> >> +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) >> +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) > Is there any explantion how these error codes got chosen? Looks very > arbitrary and may collide with other error codes in the future. If they collide, we can just fix it then. So, please, do comment what the limitations are and what must be avoided in the future, but I don't think we need to go mucking with this at all.
On Sun, 2023-08-06 at 14:41 +0300, kirill.shutemov@linux.intel.com wrote:
> On Wed, Jul 26, 2023 at 11:25:13PM +1200, Kai Huang wrote:
> > @@ -20,6 +21,9 @@
> > #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40))
> > #define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000))
> >
> > +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP)
> > +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)
>
> Is there any explantion how these error codes got chosen? Looks very
> arbitrary and may collide with other error codes in the future.
>
Any error code has TDX_SW_ERROR is reserved to software use so the TDX module
can never return any error code which conflicts with those software ones.
For why to choose these two, I believe XOR the TRAP number to TDX_SW_ERROR is
the simplest way to achieve: 1) costing minimal assembly code; 2)
opportunistically handling #GP too, allowing caller to distinguish the two
errors.
I can add this to the changelog.
Btw, as I chatted to you I believe we have another justification to handle
#UD/#GP in the assembly: emergency virtualization disable. Thus we can even get
rid of the erratum staff in the changelog.
How does below look like?
commit d3ff21da1083a525eb2cac6576490045e22f6f5d
Author: Kai Huang <kai.huang@intel.com>
Date: Mon Jun 26 16:04:08 2023 +1200
x86/virt/tdx: Allow SEAMCALL to handle #UD and #GP
SEAMCALL instruction causes #UD if the CPU isn't in VMX operation.
Currently the TDX_MODULE_CALL assembly doesn't handle #UD, thus making
SEAMCALL when VMX is disabled would cause Oops.
Unfortunately, there are legal cases that SEAMCALL can be made when VMX
is disabled. For instance, VMX can be disabled due to emergency reboot
while there are still TDX guest is running.
Extend the TDX_MODULE_CALL assembly to return an error code for #UD to
handle this case gracefully, e.g., KVM can then quitely eat all SEAMCALL
errors caused by emergency reboot.
SEAMCALL instruction also causes #GP when TDX isn't enabled by the BIOS.
Use _ASM_EXTABLE_FAULT() to catch both exceptions with the trap number
recorded, and define two new error codes by XORing the trap number to
the TDX_SW_ERROR. This opportunistically handles #GP too while using
the same simple assembly code.
A bonus is when kernel mistakenly calls SEAMCALL when CPU isn't in VMX
operation, or when TDX isn't enabled by the BIOS, or when the BIOS is
buggy, the kernel can get a nicer error code rather than a less
understandable Oops.
This is basically based on Peter's code.
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Kai Huang <kai.huang@intel.com>
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 603e6d1e9d4a..6b8547dc40fd 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -8,6 +8,7 @@
#include <asm/errno.h>
#include <asm/ptrace.h>
+#include <asm/trapnr.h>
#include <asm/shared/tdx.h>
/*
@@ -20,6 +21,9 @@
#define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40))
#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000))
+#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP)
+#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)
+
#ifndef __ASSEMBLY__
/*
diff --git a/arch/x86/virt/vmx/tdx/tdxcall.S b/arch/x86/virt/vmx/tdx/tdxcall.S
index 3f0b83a9977e..016a2a1ec1d6 100644
--- a/arch/x86/virt/vmx/tdx/tdxcall.S
+++ b/arch/x86/virt/vmx/tdx/tdxcall.S
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
#include <asm/asm-offsets.h>
#include <asm/frame.h>
+#include <asm/asm.h>
#include <asm/tdx.h>
/*
@@ -85,6 +86,7 @@
.endif /* \saved */
.if \host
+.Lseamcall\@:
seamcall
/*
* SEAMCALL instruction is essentially a VMExit from VMX root
@@ -191,11 +193,28 @@
.if \host
.Lseamcall_vmfailinvalid\@:
mov $TDX_SEAMCALL_VMFAILINVALID, %rax
+ jmp .Lseamcall_fail\@
+
+.Lseamcall_trap\@:
+ /*
+ * SEAMCALL caused #GP or #UD. By reaching here RAX contains
+ * the trap number. Convert the trap number to the TDX error
+ * code by setting TDX_SW_ERROR to the high 32-bits of RAX.
+ *
+ * Note cannot OR TDX_SW_ERROR directly to RAX as OR instruction
+ * only accepts 32-bit immediate at most.
+ */
+ movq $TDX_SW_ERROR, %rdi
+ orq %rdi, %rax
+
+.Lseamcall_fail\@:
.if \ret && \saved
/* pop the unused structure pointer back to RSI */
popq %rsi
.endif
jmp .Lout\@
+
+ _ASM_EXTABLE_FAULT(.Lseamcall\@, .Lseamcall_trap\@)
.endif /* \host */
.endm
On Mon, Aug 07, 2023 at 02:14:37AM +0000, Huang, Kai wrote: > On Sun, 2023-08-06 at 14:41 +0300, kirill.shutemov@linux.intel.com wrote: > > On Wed, Jul 26, 2023 at 11:25:13PM +1200, Kai Huang wrote: > > > @@ -20,6 +21,9 @@ > > > #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40)) > > > #define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000)) > > > > > > +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) > > > +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) > > > > Is there any explantion how these error codes got chosen? Looks very > > arbitrary and may collide with other error codes in the future. > > > > Any error code has TDX_SW_ERROR is reserved to software use so the TDX module > can never return any error code which conflicts with those software ones. > > For why to choose these two, I believe XOR the TRAP number to TDX_SW_ERROR is > the simplest way to achieve: 1) costing minimal assembly code; 2) > opportunistically handling #GP too, allowing caller to distinguish the two > errors. My problem is that it is going to conflict with errno-based errors if we going to take this path in the future. Like these errors are the same as (TDX_SW_ERROR | EACCES) and (TDX_SW_ERROR | ENXIO) respectively. -- Kiryl Shutsemau / Kirill A. Shutemov
On Mon, 2023-08-07 at 12:53 +0300, kirill.shutemov@linux.intel.com wrote: > On Mon, Aug 07, 2023 at 02:14:37AM +0000, Huang, Kai wrote: > > On Sun, 2023-08-06 at 14:41 +0300, kirill.shutemov@linux.intel.com wrote: > > > On Wed, Jul 26, 2023 at 11:25:13PM +1200, Kai Huang wrote: > > > > @@ -20,6 +21,9 @@ > > > > #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40)) > > > > #define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000)) > > > > > > > > +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) > > > > +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) > > > > > > Is there any explantion how these error codes got chosen? Looks very > > > arbitrary and may collide with other error codes in the future. > > > > > > > Any error code has TDX_SW_ERROR is reserved to software use so the TDX module > > can never return any error code which conflicts with those software ones. > > > > For why to choose these two, I believe XOR the TRAP number to TDX_SW_ERROR is > > the simplest way to achieve: 1) costing minimal assembly code; 2) > > opportunistically handling #GP too, allowing caller to distinguish the two > > errors. > > My problem is that it is going to conflict with errno-based errors if we > going to take this path in the future. Like these errors are the same as > (TDX_SW_ERROR | EACCES) and (TDX_SW_ERROR | ENXIO) respectively. > Is there any use case that we need those definitions? Even we have such requirement in the future, we still have many bits available after taking out the bits of TDX_SW_ERROR thus I assume we can do some bit shift when this really happens??
On Mon, Aug 07, 2023 at 12:41:13PM +0000, Huang, Kai wrote: > On Mon, 2023-08-07 at 12:53 +0300, kirill.shutemov@linux.intel.com wrote: > > On Mon, Aug 07, 2023 at 02:14:37AM +0000, Huang, Kai wrote: > > > On Sun, 2023-08-06 at 14:41 +0300, kirill.shutemov@linux.intel.com wrote: > > > > On Wed, Jul 26, 2023 at 11:25:13PM +1200, Kai Huang wrote: > > > > > @@ -20,6 +21,9 @@ > > > > > #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40)) > > > > > #define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000)) > > > > > > > > > > +#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) > > > > > +#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) > > > > > > > > Is there any explantion how these error codes got chosen? Looks very > > > > arbitrary and may collide with other error codes in the future. > > > > > > > > > > Any error code has TDX_SW_ERROR is reserved to software use so the TDX module > > > can never return any error code which conflicts with those software ones. > > > > > > For why to choose these two, I believe XOR the TRAP number to TDX_SW_ERROR is > > > the simplest way to achieve: 1) costing minimal assembly code; 2) > > > opportunistically handling #GP too, allowing caller to distinguish the two > > > errors. > > > > My problem is that it is going to conflict with errno-based errors if we > > going to take this path in the future. Like these errors are the same as > > (TDX_SW_ERROR | EACCES) and (TDX_SW_ERROR | ENXIO) respectively. > > > > Is there any use case that we need those definitions? > > Even we have such requirement in the future, we still have many bits available > after taking out the bits of TDX_SW_ERROR thus I assume we can do some bit shift > when this really happens?? Okay, fair enough. -- Kiryl Shutsemau / Kirill A. Shutemov
© 2016 - 2026 Red Hat, Inc.