From: Kuppuswamy Sathyanarayanan <[email protected]>
Guests communicate with VMMs with hypercalls. Historically, these
are implemented using instructions that are known to cause VMEXITs
like VMCALL, VMLAUNCH, etc. However, with TDX, VMEXITs no longer
expose the guest state to the host. This prevents the old hypercall
mechanisms from working. So, to communicate with VMM, TDX
specification defines a new instruction called TDCALL.
In a TDX based VM, since the VMM is an untrusted entity, an intermediary
layer -- TDX module -- facilitates secure communication between the host
and the guest. TDX module is loaded like a firmware into a special CPU
mode called SEAM. TDX guests communicate with the TDX module using the
TDCALL instruction.
A guest uses TDCALL to communicate with both the TDX module and VMM.
The value of the RAX register when executing the TDCALL instruction is
used to determine the TDCALL type. A variant of TDCALL used to communicate
with the VMM is called TDVMCALL.
Add generic interfaces to communicate with the TDX module and VMM
(using the TDCALL instruction).
__tdx_hypercall() - Used by the guest to request services from the
VMM (via TDVMCALL).
__tdx_module_call() - Used to communicate with the TDX module (via
TDCALL).
Also define an additional wrapper _tdx_hypercall(), which adds error
handling support for the TDCALL failure.
The __tdx_module_call() and __tdx_hypercall() helper functions are
implemented in assembly in a .S file. The TDCALL ABI requires
shuffling arguments in and out of registers, which proved to be
awkward with inline assembly.
Just like syscalls, not all TDVMCALL use cases need to use the same
number of argument registers. The implementation here picks the current
worst-case scenario for TDCALL (4 registers). For TDCALLs with fewer
than 4 arguments, there will end up being a few superfluous (cheap)
instructions. But, this approach maximizes code reuse.
For registers used by the TDCALL instruction, please check TDX GHCI
specification, the section titled "TDCALL instruction" and "TDG.VP.VMCALL
Interface".
Based on previous patch by Sean Christopherson.
Reviewed-by: Tony Luck <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/include/asm/tdx.h | 40 +++++
arch/x86/kernel/Makefile | 2 +-
arch/x86/kernel/asm-offsets.c | 20 +++
arch/x86/kernel/tdcall.S | 269 ++++++++++++++++++++++++++++++++++
arch/x86/kernel/tdx.c | 23 +++
5 files changed, 353 insertions(+), 1 deletion(-)
create mode 100644 arch/x86/kernel/tdcall.S
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index e375a950a033..5107a4d9ba8f 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -8,11 +8,51 @@
#define TDX_CPUID_LEAF_ID 0x21
#define TDX_IDENT "IntelTDX "
+#define TDX_HYPERCALL_STANDARD 0
+
+/*
+ * Used in __tdx_module_call() to gather the output registers'
+ * values of the TDCALL instruction when requesting services from
+ * the TDX module. This is a software only structure and not part
+ * of the TDX module/VMM ABI
+ */
+struct tdx_module_output {
+ u64 rcx;
+ u64 rdx;
+ u64 r8;
+ u64 r9;
+ u64 r10;
+ u64 r11;
+};
+
+/*
+ * Used in __tdx_hypercall() to gather the output registers' values
+ * of the TDCALL instruction when requesting services from the VMM.
+ * This is a software only structure and not part of the TDX
+ * module/VMM ABI.
+ */
+struct tdx_hypercall_output {
+ u64 r10;
+ u64 r11;
+ u64 r12;
+ u64 r13;
+ u64 r14;
+ u64 r15;
+};
+
#ifdef CONFIG_INTEL_TDX_GUEST
void __init tdx_early_init(void);
bool is_tdx_guest(void);
+/* Used to communicate with the TDX module */
+u64 __tdx_module_call(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
+ struct tdx_module_output *out);
+
+/* Used to request services from the VMM */
+u64 __tdx_hypercall(u64 type, u64 fn, u64 r12, u64 r13, u64 r14,
+ u64 r15, struct tdx_hypercall_output *out);
+
#else
static inline void tdx_early_init(void) { };
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 67415037c33c..ce3e044f7f12 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -133,7 +133,7 @@ obj-$(CONFIG_PARAVIRT_CLOCK) += pvclock.o
obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o
obj-$(CONFIG_JAILHOUSE_GUEST) += jailhouse.o
-obj-$(CONFIG_INTEL_TDX_GUEST) += tdx.o
+obj-$(CONFIG_INTEL_TDX_GUEST) += tdcall.o tdx.o
obj-$(CONFIG_EISA) += eisa.o
obj-$(CONFIG_PCSPKR_PLATFORM) += pcspeaker.o
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 9fb0a2f8b62a..8a3c6b34be7d 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -18,6 +18,7 @@
#include <asm/bootparam.h>
#include <asm/suspend.h>
#include <asm/tlbflush.h>
+#include <asm/tdx.h>
#ifdef CONFIG_XEN
#include <xen/interface/xen.h>
@@ -65,6 +66,25 @@ static void __used common(void)
OFFSET(XEN_vcpu_info_arch_cr2, vcpu_info, arch.cr2);
#endif
+#ifdef CONFIG_INTEL_TDX_GUEST
+ BLANK();
+ /* Offset for fields in tdx_module_output */
+ OFFSET(TDX_MODULE_rcx, tdx_module_output, rcx);
+ OFFSET(TDX_MODULE_rdx, tdx_module_output, rdx);
+ OFFSET(TDX_MODULE_r8, tdx_module_output, r8);
+ OFFSET(TDX_MODULE_r9, tdx_module_output, r9);
+ OFFSET(TDX_MODULE_r10, tdx_module_output, r10);
+ OFFSET(TDX_MODULE_r11, tdx_module_output, r11);
+
+ /* Offset for fields in tdx_hypercall_output */
+ OFFSET(TDX_HYPERCALL_r10, tdx_hypercall_output, r10);
+ OFFSET(TDX_HYPERCALL_r11, tdx_hypercall_output, r11);
+ OFFSET(TDX_HYPERCALL_r12, tdx_hypercall_output, r12);
+ OFFSET(TDX_HYPERCALL_r13, tdx_hypercall_output, r13);
+ OFFSET(TDX_HYPERCALL_r14, tdx_hypercall_output, r14);
+ OFFSET(TDX_HYPERCALL_r15, tdx_hypercall_output, r15);
+#endif
+
BLANK();
OFFSET(BP_scratch, boot_params, scratch);
OFFSET(BP_secure_boot, boot_params, secure_boot);
diff --git a/arch/x86/kernel/tdcall.S b/arch/x86/kernel/tdcall.S
new file mode 100644
index 000000000000..46a49a96cf6c
--- /dev/null
+++ b/arch/x86/kernel/tdcall.S
@@ -0,0 +1,269 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <asm/asm-offsets.h>
+#include <asm/asm.h>
+#include <asm/frame.h>
+#include <asm/unwind_hints.h>
+
+#include <linux/linkage.h>
+#include <linux/bits.h>
+#include <linux/errno.h>
+
+/*
+ * Bitmasks of exposed registers (with VMM).
+ */
+#define TDX_R10 BIT(10)
+#define TDX_R11 BIT(11)
+#define TDX_R12 BIT(12)
+#define TDX_R13 BIT(13)
+#define TDX_R14 BIT(14)
+#define TDX_R15 BIT(15)
+
+/* Frame offset + 8 (for arg1) */
+#define ARG7_SP_OFFSET (FRAME_OFFSET + 0x08)
+
+/*
+ * These registers are clobbered to hold arguments for each
+ * TDVMCALL. They are safe to expose to the VMM.
+ * Each bit in this mask represents a register ID. Bit field
+ * details can be found in TDX GHCI specification, section
+ * titled "TDCALL [TDG.VP.VMCALL] leaf".
+ */
+#define TDVMCALL_EXPOSE_REGS_MASK ( TDX_R10 | TDX_R11 | \
+ TDX_R12 | TDX_R13 | \
+ TDX_R14 | TDX_R15 )
+
+/*
+ * TDX guests use the TDCALL instruction to make requests to the
+ * TDX module and hypercalls to the VMM. It is supported in
+ * Binutils >= 2.36.
+ */
+#define tdcall .byte 0x66,0x0f,0x01,0xcc
+
+/*
+ * __tdx_module_call() - Used by TDX guests to request services from
+ * the TDX module (does not include VMM services).
+ *
+ * Transforms function call register arguments into the TDCALL
+ * register ABI. After TDCALL operation, TDX module output is saved
+ * in @out (if it is provided by the user)
+ *
+ *-------------------------------------------------------------------------
+ * TDCALL ABI:
+ *-------------------------------------------------------------------------
+ * Input Registers:
+ *
+ * RAX - TDCALL Leaf number.
+ * RCX,RDX,R8-R9 - TDCALL Leaf specific input registers.
+ *
+ * Output Registers:
+ *
+ * RAX - TDCALL instruction error code.
+ * RCX,RDX,R8-R11 - TDCALL Leaf specific output registers.
+ *
+ *-------------------------------------------------------------------------
+ *
+ * __tdx_module_call() function ABI:
+ *
+ * @fn (RDI) - TDCALL Leaf ID, moved to RAX
+ * @rcx (RSI) - Input parameter 1, moved to RCX
+ * @rdx (RDX) - Input parameter 2, moved to RDX
+ * @r8 (RCX) - Input parameter 3, moved to R8
+ * @r9 (R8) - Input parameter 4, moved to R9
+ *
+ * @out (R9) - struct tdx_module_output pointer
+ * stored temporarily in R12 (not
+ * shared with the TDX module). It
+ * can be NULL.
+ *
+ * Return status of TDCALL via RAX.
+ */
+SYM_FUNC_START(__tdx_module_call)
+ FRAME_BEGIN
+
+ /*
+ * R12 will be used as temporary storage for
+ * struct tdx_module_output pointer. Since R12-R15
+ * registers are not used by TDCALL services supported
+ * by this function, it can be reused.
+ */
+
+ /* Callee saved, so preserve it */
+ push %r12
+
+ /*
+ * Push output pointer to stack.
+ * After the TDCALL operation, it will be fetched
+ * into R12 register.
+ */
+ push %r9
+
+ /* Mangle function call ABI into TDCALL ABI: */
+ /* Move TDCALL Leaf ID to RAX */
+ mov %rdi, %rax
+ /* Move input 4 to R9 */
+ mov %r8, %r9
+ /* Move input 3 to R8 */
+ mov %rcx, %r8
+ /* Move input 1 to RCX */
+ mov %rsi, %rcx
+ /* Leave input param 2 in RDX */
+
+ tdcall
+
+ /*
+ * Fetch output pointer from stack to R12 (It is used
+ * as temporary storage)
+ */
+ pop %r12
+
+ /* Check for TDCALL success: 0 - Successful, otherwise failed */
+ test %rax, %rax
+ jnz .Lno_output_struct
+
+ /*
+ * Since this function can be initiated without an output pointer,
+ * check if caller provided an output struct before storing
+ * output registers.
+ */
+ test %r12, %r12
+ jz .Lno_output_struct
+
+ /* Copy TDCALL result registers to output struct: */
+ movq %rcx, TDX_MODULE_rcx(%r12)
+ movq %rdx, TDX_MODULE_rdx(%r12)
+ movq %r8, TDX_MODULE_r8(%r12)
+ movq %r9, TDX_MODULE_r9(%r12)
+ movq %r10, TDX_MODULE_r10(%r12)
+ movq %r11, TDX_MODULE_r11(%r12)
+
+.Lno_output_struct:
+ /* Restore the state of R12 register */
+ pop %r12
+
+ FRAME_END
+ ret
+SYM_FUNC_END(__tdx_module_call)
+
+/*
+ * __tdx_hypercall() - Make hypercalls to a TDX VMM.
+ *
+ * Transforms function call register arguments into the TDCALL
+ * register ABI. After TDCALL operation, VMM output is saved in @out.
+ *
+ *-------------------------------------------------------------------------
+ * TD VMCALL ABI:
+ *-------------------------------------------------------------------------
+ *
+ * Input Registers:
+ *
+ * RAX - TDCALL instruction leaf number (0 - TDG.VP.VMCALL)
+ * RCX - BITMAP which controls which part of TD Guest GPR
+ * is passed as-is to the VMM and back.
+ * R10 - Set 0 to indicate TDCALL follows standard TDX ABI
+ * specification. Non zero value indicates vendor
+ * specific ABI.
+ * R11 - VMCALL sub function number
+ * RBX, RBP, RDI, RSI - Used to pass VMCALL sub function specific arguments.
+ * R8-R9, R12-R15 - Same as above.
+ *
+ * Output Registers:
+ *
+ * RAX - TDCALL instruction status (Not related to hypercall
+ * output).
+ * R10 - Hypercall output error code.
+ * R11-R15 - Hypercall sub function specific output values.
+ *
+ *-------------------------------------------------------------------------
+ *
+ * __tdx_hypercall() function ABI:
+ *
+ * @type (RDI) - TD VMCALL type, moved to R10
+ * @fn (RSI) - TD VMCALL sub function, moved to R11
+ * @r12 (RDX) - Input parameter 1, moved to R12
+ * @r13 (RCX) - Input parameter 2, moved to R13
+ * @r14 (R8) - Input parameter 3, moved to R14
+ * @r15 (R9) - Input parameter 4, moved to R15
+ *
+ * @out (stack) - struct tdx_hypercall_output pointer (cannot be NULL)
+ *
+ * On successful completion, return TDCALL status or -EINVAL for invalid
+ * inputs.
+ */
+SYM_FUNC_START(__tdx_hypercall)
+ FRAME_BEGIN
+
+ /* Move argument 7 from caller stack to RAX */
+ movq ARG7_SP_OFFSET(%rsp), %rax
+
+ /* Check if caller provided an output struct */
+ test %rax, %rax
+ /* If out pointer is NULL, return -EINVAL */
+ jz .Lret_err
+
+ /* Save callee-saved GPRs as mandated by the x86_64 ABI */
+ push %r15
+ push %r14
+ push %r13
+ push %r12
+
+ /*
+ * Save output pointer (rax) on the stack, it will be used again
+ * when storing the output registers after the TDCALL operation.
+ */
+ push %rax
+
+ /* Mangle function call ABI into TDCALL ABI: */
+ /* Set TDCALL leaf ID (TDVMCALL (0)) in RAX */
+ xor %eax, %eax
+ /* Move TDVMCALL type (standard vs vendor) in R10 */
+ mov %rdi, %r10
+ /* Move TDVMCALL sub function id to R11 */
+ mov %rsi, %r11
+ /* Move input 1 to R12 */
+ mov %rdx, %r12
+ /* Move input 2 to R13 */
+ mov %rcx, %r13
+ /* Move input 3 to R14 */
+ mov %r8, %r14
+ /* Move input 4 to R15 */
+ mov %r9, %r15
+
+ movl $TDVMCALL_EXPOSE_REGS_MASK, %ecx
+
+ tdcall
+
+ /* Restore output pointer to R9 */
+ pop %r9
+
+ /* Copy hypercall result registers to output struct: */
+ movq %r10, TDX_HYPERCALL_r10(%r9)
+ movq %r11, TDX_HYPERCALL_r11(%r9)
+ movq %r12, TDX_HYPERCALL_r12(%r9)
+ movq %r13, TDX_HYPERCALL_r13(%r9)
+ movq %r14, TDX_HYPERCALL_r14(%r9)
+ movq %r15, TDX_HYPERCALL_r15(%r9)
+
+ /*
+ * Zero out registers exposed to the VMM to avoid
+ * speculative execution with VMM-controlled values.
+ * This needs to include all registers present in
+ * TDVMCALL_EXPOSE_REGS_MASK (except R12-R15).
+ * R12-R15 context will be restored.
+ */
+ xor %r10d, %r10d
+ xor %r11d, %r11d
+
+ /* Restore callee-saved GPRs as mandated by the x86_64 ABI */
+ pop %r12
+ pop %r13
+ pop %r14
+ pop %r15
+
+ jmp .Lhcall_done
+.Lret_err:
+ movq $-EINVAL, %rax
+.Lhcall_done:
+ FRAME_END
+
+ retq
+SYM_FUNC_END(__tdx_hypercall)
diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
index 1ef6979a6434..d40b6df51e26 100644
--- a/arch/x86/kernel/tdx.c
+++ b/arch/x86/kernel/tdx.c
@@ -9,6 +9,29 @@
static bool tdx_guest_detected __ro_after_init;
+/*
+ * Wrapper for standard use of __tdx_hypercall with panic report
+ * for TDCALL error.
+ */
+static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14,
+ u64 r15, struct tdx_hypercall_output *out)
+{
+ struct tdx_hypercall_output dummy_out;
+ u64 err;
+
+ /* __tdx_hypercall() does not accept NULL output pointer */
+ if (!out)
+ out = &dummy_out;
+
+ /* Non zero return value indicates buggy TDX module, so panic */
+ err = __tdx_hypercall(TDX_HYPERCALL_STANDARD, fn, r12, r13, r14,
+ r15, out);
+ if (err)
+ panic("Hypercall fn %llu failed (Buggy TDX module!)\n", fn);
+
+ return out->r10;
+}
+
bool is_tdx_guest(void)
{
return tdx_guest_detected;
--
2.34.1
Kirill,
On Mon, Jan 24 2022 at 18:01, Kirill A. Shutemov wrote:
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -8,11 +8,51 @@
> #define TDX_CPUID_LEAF_ID 0x21
> #define TDX_IDENT "IntelTDX "
>
> +#define TDX_HYPERCALL_STANDARD 0
> +
> +/*
> + * Used in __tdx_module_call() to gather the output registers'
> + * values of the TDCALL instruction when requesting services from
> + * the TDX module. This is a software only structure and not part
> + * of the TDX module/VMM ABI
> + */
> +struct tdx_module_output {
> + u64 rcx;
> + u64 rdx;
> + u64 r8;
> + u64 r9;
> + u64 r10;
> + u64 r11;
> +};
I've seen exactly the same struct named seamcall_regs_out in the TDX
host series. I assume that's not coincidence which begs the question why
this is required twice with different names.
> +#ifdef CONFIG_INTEL_TDX_GUEST
> + BLANK();
> + /* Offset for fields in tdx_module_output */
> + OFFSET(TDX_MODULE_rcx, tdx_module_output, rcx);
> + OFFSET(TDX_MODULE_rdx, tdx_module_output, rdx);
> + OFFSET(TDX_MODULE_r8, tdx_module_output, r8);
> + OFFSET(TDX_MODULE_r9, tdx_module_output, r9);
> + OFFSET(TDX_MODULE_r10, tdx_module_output, r10);
> + OFFSET(TDX_MODULE_r11, tdx_module_output, r11);
Which obviously duplicates the above part as well.
> + *-------------------------------------------------------------------------
> + * TDCALL ABI:
> + *-------------------------------------------------------------------------
> + * Input Registers:
> + *
> + * RAX - TDCALL Leaf number.
> + * RCX,RDX,R8-R9 - TDCALL Leaf specific input registers.
> + *
> + * Output Registers:
> + *
> + * RAX - TDCALL instruction error code.
> + * RCX,RDX,R8-R11 - TDCALL Leaf specific output registers.
> + *
> + *-------------------------------------------------------------------------
> + *
> + * __tdx_module_call() function ABI:
> + *
> + * @fn (RDI) - TDCALL Leaf ID, moved to RAX
> + * @rcx (RSI) - Input parameter 1, moved to RCX
> + * @rdx (RDX) - Input parameter 2, moved to RDX
> + * @r8 (RCX) - Input parameter 3, moved to R8
> + * @r9 (R8) - Input parameter 4, moved to R9
> + *
> + * @out (R9) - struct tdx_module_output pointer
> + * stored temporarily in R12 (not
> + * shared with the TDX module). It
> + * can be NULL.
> + *
> + * Return status of TDCALL via RAX.
> + */
And unsurprisingly this function and __seamcall of the other patch set
are very similar aside of the calling convention (__seamcall has a
struct for the input parameters) and the obvious difference that one
issues TDCALL and the other SEAMCALL.
So can we please have _one_ implementation and the same struct(s) for
the module call which is exactly the same for host and guest except for
the instruction used.
IOW, this begs a macro implementation
.macro TDX_MODULE_CALL host:req
....
.if \host
seamcall
.else
tdcall
.endif
....
So the actual functions become:
SYM_FUNC_START(__tdx_module_call)
FRAME_BEGIN
TDX_MODULE_CALL host=0
FRAME_END
ret
SYM_FUNC_END(__tdx_module_call)
SYM_FUNC_START(__tdx_seam_call)
FRAME_BEGIN
TDX_MODULE_CALL host=1
FRAME_END
ret
SYM_FUNC_END(__tdx_seam_call)
Hmm?
> +/*
> + * Wrapper for standard use of __tdx_hypercall with panic report
> + * for TDCALL error.
> + */
> +static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14,
> + u64 r15, struct tdx_hypercall_output
> *out)
This begs the question whether having a struct hypercall_input similar
to the way how seamcall input parameters are implemented makes more
sense than 7 function arguments. Hmm?
Thanks,
tglx
> /*
> diff --git a/arch/x86/kernel/tdxcall.S b/arch/x86/kernel/tdxcall.S
> new file mode 100644
> index 000000000000..ba5e8e35de36
> --- /dev/null
> +++ b/arch/x86/kernel/tdxcall.S
> @@ -0,0 +1,76 @@
> +#include <asm/asm-offsets.h>
> +
> +/*
> + * TDX guests use the TDCALL instruction to make requests to the
> + * TDX module and hypercalls to the VMM.
> + *
> + * TDX host user SEAMCALL instruction to make requests to TDX module.
> + *
> + * They are supported in Binutils >= 2.36.
> + */
> +#define tdcall .byte 0x66,0x0f,0x01,0xcc
> +#define seamcall .byte 0x66,0x0f,0x01,0xcf
> +
> +.macro TDX_MODULE_CALL host:req
> + /*
> + * R12 will be used as temporary storage for struct tdx_module_output
> + * pointer. Since R12-R15 registers are not used by TDCALL/SEAMCALL
> + * services supported by this function, it can be reused.
> + */
> +
> + /* Callee saved, so preserve it */
> + push %r12
> +
> + /*
> + * Push output pointer to stack.
> + * After the operation, it will be fetched into R12 register.
> + */
> + push %r9
> +
> + /* Mangle function call ABI into TDCALL/SEAMCALL ABI: */
> + /* Move Leaf ID to RAX */
> + mov %rdi, %rax
> + /* Move input 4 to R9 */
> + mov %r8, %r9
> + /* Move input 3 to R8 */
> + mov %rcx, %r8
> + /* Move input 1 to RCX */
> + mov %rsi, %rcx
> + /* Leave input param 2 in RDX */
Currently host seamcall also uses a data structure which has all possible input
registers as input argument, rather than having one argument for each register:
struct seamcall_regs_in {
u64 rcx;
u64 rdx;
u64 r8;
u64 r9;
};
Which way is better (above struct name can be changed of course)?
Or should we rename tdx_module_output to tdx_module_regs, and use it as both
input and output (similar to __tdx_hypercall())?
> +
> + .if \host
> + seamcall
> + .else
> + tdcall
> + .endif
> +
> + /*
> + * Fetch output pointer from stack to R12 (It is used
> + * as temporary storage)
> + */
> + pop %r12
For host SEAMCALL, one additional check against VMfailInvalid needs to be
done after here. This is because at host side, both P-SEAMLDR and TDX module
are expected to be loaded by UEFI (i.e. UEFI shell tool) before booting to the
kernel, therefore host kernel needs to detect whether them have been loaded, by
issuing SEAMCALL.
When SEAM software (P-SEAMLDR or TDX module) is not loaded, the SEAMCALL fails
with VMfailInvalid. VMfailInvalid is indicated via a combination of RFLAGS
rather than using %rax. In practice, RFLAGS.CF=1 can be used to check whether
VMfailInvalid happened, and we need to have something like below:
/*
* SEAMCALL instruction is essentially a VMExit from VMX root
* mode to SEAM VMX root mode. VMfailInvalid (CF=1) indicates
* that the targeted SEAM firmware is not loaded or disabled,
* or P-SEAMLDR is busy with another SEAMCALL. %rax is not
* changed in this case.
*
* Set %rax to VMFAILINVALID for VMfailInvalid. This value
* will never be used as actual SEAMCALL error code.
*/
jnb .Lno_vmfailinvalid
mov $(VMFAILINVALID), %rax
jmp .Lno_output_struct
.Lno_vmfailinvalid:
> +
> + /* Check for success: 0 - Successful, otherwise failed */
> + test %rax, %rax
> + jnz .Lno_output_struct
> +
> + /*
> + * Since this function can be initiated without an output pointer,
> + * check if caller provided an output struct before storing
> + * output registers.
> + */
> + test %r12, %r12
> + jz .Lno_output_struct
> +
> + /* Copy result registers to output struct: */
> + movq %rcx, TDX_MODULE_rcx(%r12)
> + movq %rdx, TDX_MODULE_rdx(%r12)
> + movq %r8, TDX_MODULE_r8(%r12)
> + movq %r9, TDX_MODULE_r9(%r12)
> + movq %r10, TDX_MODULE_r10(%r12)
> + movq %r11, TDX_MODULE_r11(%r12)
> +
> +.Lno_output_struct:
> + /* Restore the state of R12 register */
> + pop %r12
> +.endm
> --
> Kirill A. Shutemov
On Tue, Feb 01, 2022 at 08:58:59PM +0100, Thomas Gleixner wrote:
> And unsurprisingly this function and __seamcall of the other patch set
> are very similar aside of the calling convention (__seamcall has a
> struct for the input parameters) and the obvious difference that one
> issues TDCALL and the other SEAMCALL.
>
> So can we please have _one_ implementation and the same struct(s) for
> the module call which is exactly the same for host and guest except for
> the instruction used.
>
> IOW, this begs a macro implementation
>
> .macro TDX_MODULE_CALL host:req
>
> ....
>
> .if \host
> seamcall
> .else
> tdcall
> .endif
>
> ....
>
> So the actual functions become:
>
> SYM_FUNC_START(__tdx_module_call)
> FRAME_BEGIN
> TDX_MODULE_CALL host=0
> FRAME_END
> ret
> SYM_FUNC_END(__tdx_module_call)
>
> SYM_FUNC_START(__tdx_seam_call)
> FRAME_BEGIN
> TDX_MODULE_CALL host=1
> FRAME_END
> ret
> SYM_FUNC_END(__tdx_seam_call)
>
> Hmm?
>
> > +/*
> > + * Wrapper for standard use of __tdx_hypercall with panic report
> > + * for TDCALL error.
> > + */
> > +static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14,
> > + u64 r15, struct tdx_hypercall_output
> > *out)
>
> This begs the question whether having a struct hypercall_input similar
> to the way how seamcall input parameters are implemented makes more
> sense than 7 function arguments. Hmm?
Okay, below is my take on addressing feedback for both __tdx_module_call()
and __tdx_hypercall().
It is fixup for whole patchset. It has to be folded accordingly. I wanted
to check if it works and see if I understand your request correctly.
__tdx_module_call() is now implemented by including tdxcall.S can using
the macro defined there. Host side of TDX can do the same on their side.
TDX_MODULE_* offsets are now outside of CONFIG_INTEL_TDX_GUEST and can be
used by both host can guest.
I changed __tdx_hypercall() to take single argument with struct pointer
that used for both input and output.
Is it the right direction? Or did I misunderstand something?
diff --git a/arch/x86/boot/compressed/tdx.c b/arch/x86/boot/compressed/tdx.c
index f2e1449c74cd..3ff676379947 100644
--- a/arch/x86/boot/compressed/tdx.c
+++ b/arch/x86/boot/compressed/tdx.c
@@ -21,20 +21,31 @@ bool early_is_tdx_guest(void)
static inline unsigned int tdx_io_in(int size, int port)
{
- struct tdx_hypercall_output out;
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_IO_INSTRUCTION,
+ .r12 = size,
+ .r13 = 0,
+ .r14 = port,
+ };
- __tdx_hypercall(TDX_HYPERCALL_STANDARD, EXIT_REASON_IO_INSTRUCTION,
- size, 0, port, 0, &out);
+ __tdx_hypercall(&args);
- return out.r10 ? UINT_MAX : out.r11;
+ return args.r10 ? UINT_MAX : args.r11;
}
static inline void tdx_io_out(int size, int port, u64 value)
{
- struct tdx_hypercall_output out;
-
- __tdx_hypercall(TDX_HYPERCALL_STANDARD, EXIT_REASON_IO_INSTRUCTION,
- size, 1, port, value, &out);
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_IO_INSTRUCTION,
+ .r12 = size,
+ .r13 = 1,
+ .r14 = port,
+ .r15 = value,
+ };
+
+ __tdx_hypercall(&args);
}
static inline unsigned char tdx_inb(int port)
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index 4a0218bedc75..ce06002346a3 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -9,7 +9,7 @@
* This is a software only structure and not part of the TDX
* module/VMM ABI.
*/
-struct tdx_hypercall_output {
+struct tdx_hypercall_args {
u64 r10;
u64 r11;
u64 r12;
@@ -24,7 +24,6 @@ struct tdx_hypercall_output {
#define TDX_IDENT "IntelTDX "
/* Used to request services from the VMM */
-u64 __tdx_hypercall(u64 type, u64 fn, u64 r12, u64 r13, u64 r14,
- u64 r15, struct tdx_hypercall_output *out);
+u64 __tdx_hypercall(struct tdx_hypercall_args *out);
#endif
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 8a3c6b34be7d..0b465e7d0a2f 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -66,9 +66,7 @@ static void __used common(void)
OFFSET(XEN_vcpu_info_arch_cr2, vcpu_info, arch.cr2);
#endif
-#ifdef CONFIG_INTEL_TDX_GUEST
BLANK();
- /* Offset for fields in tdx_module_output */
OFFSET(TDX_MODULE_rcx, tdx_module_output, rcx);
OFFSET(TDX_MODULE_rdx, tdx_module_output, rdx);
OFFSET(TDX_MODULE_r8, tdx_module_output, r8);
@@ -76,13 +74,14 @@ static void __used common(void)
OFFSET(TDX_MODULE_r10, tdx_module_output, r10);
OFFSET(TDX_MODULE_r11, tdx_module_output, r11);
- /* Offset for fields in tdx_hypercall_output */
- OFFSET(TDX_HYPERCALL_r10, tdx_hypercall_output, r10);
- OFFSET(TDX_HYPERCALL_r11, tdx_hypercall_output, r11);
- OFFSET(TDX_HYPERCALL_r12, tdx_hypercall_output, r12);
- OFFSET(TDX_HYPERCALL_r13, tdx_hypercall_output, r13);
- OFFSET(TDX_HYPERCALL_r14, tdx_hypercall_output, r14);
- OFFSET(TDX_HYPERCALL_r15, tdx_hypercall_output, r15);
+#ifdef CONFIG_INTEL_TDX_GUEST
+ BLANK();
+ OFFSET(TDX_HYPERCALL_r10, tdx_hypercall_args, r10);
+ OFFSET(TDX_HYPERCALL_r11, tdx_hypercall_args, r11);
+ OFFSET(TDX_HYPERCALL_r12, tdx_hypercall_args, r12);
+ OFFSET(TDX_HYPERCALL_r13, tdx_hypercall_args, r13);
+ OFFSET(TDX_HYPERCALL_r14, tdx_hypercall_args, r14);
+ OFFSET(TDX_HYPERCALL_r15, tdx_hypercall_args, r15);
#endif
BLANK();
diff --git a/arch/x86/kernel/tdcall.S b/arch/x86/kernel/tdcall.S
index ae74da33ccc6..4db79fbbb857 100644
--- a/arch/x86/kernel/tdcall.S
+++ b/arch/x86/kernel/tdcall.S
@@ -9,6 +9,8 @@
#include <linux/bits.h>
#include <linux/errno.h>
+#include "tdxcall.S"
+
/*
* Bitmasks of exposed registers (with VMM).
*/
@@ -19,9 +21,6 @@
#define TDX_R14 BIT(14)
#define TDX_R15 BIT(15)
-/* Frame offset + 8 (for arg1) */
-#define ARG7_SP_OFFSET (FRAME_OFFSET + 0x08)
-
/*
* These registers are clobbered to hold arguments for each
* TDVMCALL. They are safe to expose to the VMM.
@@ -33,13 +32,6 @@
TDX_R12 | TDX_R13 | \
TDX_R14 | TDX_R15 )
-/*
- * TDX guests use the TDCALL instruction to make requests to the
- * TDX module and hypercalls to the VMM. It is supported in
- * Binutils >= 2.36.
- */
-#define tdcall .byte 0x66,0x0f,0x01,0xcc
-
/*
* Used in __tdx_hypercall() to determine whether to enable interrupts
* before issuing TDCALL for the EXIT_REASON_HLT case.
@@ -86,67 +78,7 @@
*/
SYM_FUNC_START(__tdx_module_call)
FRAME_BEGIN
-
- /*
- * R12 will be used as temporary storage for
- * struct tdx_module_output pointer. Since R12-R15
- * registers are not used by TDCALL services supported
- * by this function, it can be reused.
- */
-
- /* Callee saved, so preserve it */
- push %r12
-
- /*
- * Push output pointer to stack.
- * After the TDCALL operation, it will be fetched
- * into R12 register.
- */
- push %r9
-
- /* Mangle function call ABI into TDCALL ABI: */
- /* Move TDCALL Leaf ID to RAX */
- mov %rdi, %rax
- /* Move input 4 to R9 */
- mov %r8, %r9
- /* Move input 3 to R8 */
- mov %rcx, %r8
- /* Move input 1 to RCX */
- mov %rsi, %rcx
- /* Leave input param 2 in RDX */
-
- tdcall
-
- /*
- * Fetch output pointer from stack to R12 (It is used
- * as temporary storage)
- */
- pop %r12
-
- /* Check for TDCALL success: 0 - Successful, otherwise failed */
- test %rax, %rax
- jnz .Lno_output_struct
-
- /*
- * Since this function can be initiated without an output pointer,
- * check if caller provided an output struct before storing
- * output registers.
- */
- test %r12, %r12
- jz .Lno_output_struct
-
- /* Copy TDCALL result registers to output struct: */
- movq %rcx, TDX_MODULE_rcx(%r12)
- movq %rdx, TDX_MODULE_rdx(%r12)
- movq %r8, TDX_MODULE_r8(%r12)
- movq %r9, TDX_MODULE_r9(%r12)
- movq %r10, TDX_MODULE_r10(%r12)
- movq %r11, TDX_MODULE_r11(%r12)
-
-.Lno_output_struct:
- /* Restore the state of R12 register */
- pop %r12
-
+ TDX_MODULE_CALL host=0
FRAME_END
ret
SYM_FUNC_END(__tdx_module_call)
@@ -184,14 +116,7 @@ SYM_FUNC_END(__tdx_module_call)
*
* __tdx_hypercall() function ABI:
*
- * @type (RDI) - TD VMCALL type, moved to R10
- * @fn (RSI) - TD VMCALL sub function, moved to R11
- * @r12 (RDX) - Input parameter 1, moved to R12
- * @r13 (RCX) - Input parameter 2, moved to R13
- * @r14 (R8) - Input parameter 3, moved to R14
- * @r15 (R9) - Input parameter 4, moved to R15
- *
- * @out (stack) - struct tdx_hypercall_output pointer (cannot be NULL)
+ * @args (RDI) - struct tdx_hypercall_output args
*
* On successful completion, return TDCALL status or -EINVAL for invalid
* inputs.
@@ -199,41 +124,23 @@ SYM_FUNC_END(__tdx_module_call)
SYM_FUNC_START(__tdx_hypercall)
FRAME_BEGIN
- /* Move argument 7 from caller stack to RAX */
- movq ARG7_SP_OFFSET(%rsp), %rax
-
- /* Check if caller provided an output struct */
- test %rax, %rax
- /* If out pointer is NULL, return -EINVAL */
- jz .Lret_err
-
/* Save callee-saved GPRs as mandated by the x86_64 ABI */
push %r15
push %r14
push %r13
push %r12
- /*
- * Save output pointer (rax) on the stack, it will be used again
- * when storing the output registers after the TDCALL operation.
- */
- push %rax
-
/* Mangle function call ABI into TDCALL ABI: */
/* Set TDCALL leaf ID (TDVMCALL (0)) in RAX */
xor %eax, %eax
- /* Move TDVMCALL type (standard vs vendor) in R10 */
- mov %rdi, %r10
- /* Move TDVMCALL sub function id to R11 */
- mov %rsi, %r11
- /* Move input 1 to R12 */
- mov %rdx, %r12
- /* Move input 2 to R13 */
- mov %rcx, %r13
- /* Move input 3 to R14 */
- mov %r8, %r14
- /* Move input 4 to R15 */
- mov %r9, %r15
+
+ /* Copy hypercall registers from arg struct: */
+ movq TDX_HYPERCALL_r10(%rdi), %r10
+ movq TDX_HYPERCALL_r11(%rdi), %r11
+ movq TDX_HYPERCALL_r12(%rdi), %r12
+ movq TDX_HYPERCALL_r13(%rdi), %r13
+ movq TDX_HYPERCALL_r14(%rdi), %r14
+ movq TDX_HYPERCALL_r15(%rdi), %r15
movl $TDVMCALL_EXPOSE_REGS_MASK, %ecx
@@ -263,16 +170,13 @@ SYM_FUNC_START(__tdx_hypercall)
.Lskip_sti:
tdcall
- /* Restore output pointer to R9 */
- pop %r9
-
- /* Copy hypercall result registers to output struct: */
- movq %r10, TDX_HYPERCALL_r10(%r9)
- movq %r11, TDX_HYPERCALL_r11(%r9)
- movq %r12, TDX_HYPERCALL_r12(%r9)
- movq %r13, TDX_HYPERCALL_r13(%r9)
- movq %r14, TDX_HYPERCALL_r14(%r9)
- movq %r15, TDX_HYPERCALL_r15(%r9)
+ /* Copy hypercall result registers to arg struct: */
+ movq %r10, TDX_HYPERCALL_r10(%rdi)
+ movq %r11, TDX_HYPERCALL_r11(%rdi)
+ movq %r12, TDX_HYPERCALL_r12(%rdi)
+ movq %r13, TDX_HYPERCALL_r13(%rdi)
+ movq %r14, TDX_HYPERCALL_r14(%rdi)
+ movq %r15, TDX_HYPERCALL_r15(%rdi)
/*
* Zero out registers exposed to the VMM to avoid
@@ -290,10 +194,6 @@ SYM_FUNC_START(__tdx_hypercall)
pop %r14
pop %r15
- jmp .Lhcall_done
-.Lret_err:
- movq $-EINVAL, %rax
-.Lhcall_done:
FRAME_END
retq
diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
index be5465cb81c3..4924c7a1a002 100644
--- a/arch/x86/kernel/tdx.c
+++ b/arch/x86/kernel/tdx.c
@@ -35,36 +35,39 @@ static struct {
* Wrapper for standard use of __tdx_hypercall with panic report
* for TDCALL error.
*/
-static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14,
- u64 r15, struct tdx_hypercall_output *out)
+static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15)
{
- struct tdx_hypercall_output dummy_out;
- u64 err;
-
- /* __tdx_hypercall() does not accept NULL output pointer */
- if (!out)
- out = &dummy_out;
-
- /* Non zero return value indicates buggy TDX module, so panic */
- err = __tdx_hypercall(TDX_HYPERCALL_STANDARD, fn, r12, r13, r14,
- r15, out);
- if (err)
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = fn,
+ .r12 = r12,
+ .r13 = r13,
+ .r14 = r14,
+ .r15 = r15,
+ };
+
+ if (__tdx_hypercall(&args))
panic("Hypercall fn %llu failed (Buggy TDX module!)\n", fn);
- return out->r10;
+ return args.r10;
}
#ifdef CONFIG_KVM_GUEST
long tdx_kvm_hypercall(unsigned int nr, unsigned long p1, unsigned long p2,
unsigned long p3, unsigned long p4)
{
- struct tdx_hypercall_output out;
-
- /* Non zero return value indicates buggy TDX module, so panic */
- if (__tdx_hypercall(nr, p1, p2, p3, p4, 0, &out))
+ struct tdx_hypercall_args args = {
+ .r10 = nr,
+ .r11 = p1,
+ .r12 = p2,
+ .r13 = p3,
+ .r14 = p4,
+ };
+
+ if (__tdx_hypercall(&args))
panic("KVM hypercall %u failed. Buggy TDX module?\n", nr);
- return out.r10;
+ return args.r10;
}
EXPORT_SYMBOL_GPL(tdx_kvm_hypercall);
#endif
@@ -146,7 +149,7 @@ int tdx_hcall_request_gpa_type(phys_addr_t start, phys_addr_t end, bool enc)
* can be found in TDX Guest-Host-Communication Interface (GHCI),
* sec "TDG.VP.VMCALL<MapGPA>"
*/
- ret = _tdx_hypercall(TDVMCALL_MAP_GPA, start, end - start, 0, 0, NULL);
+ ret = _tdx_hypercall(TDVMCALL_MAP_GPA, start, end - start, 0, 0);
if (ret)
ret = -EIO;
@@ -192,8 +195,7 @@ static u64 __cpuidle _tdx_halt(const bool irq_disabled, const bool do_sti)
* whether to call the STI instruction before executing the
* TDCALL instruction.
*/
- return _tdx_hypercall(EXIT_REASON_HLT, irq_disabled, 0, 0,
- do_sti, NULL);
+ return _tdx_hypercall(EXIT_REASON_HLT, irq_disabled, 0, 0, do_sti);
}
static bool tdx_halt(void)
@@ -231,47 +233,65 @@ void __cpuidle tdx_safe_halt(void)
static bool tdx_read_msr(unsigned int msr, u64 *val)
{
- struct tdx_hypercall_output out;
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_MSR_READ,
+ .r12 = msr,
+ };
/*
* Emulate the MSR read via hypercall. More info about ABI
* can be found in TDX Guest-Host-Communication Interface
* (GHCI), sec titled "TDG.VP.VMCALL<Instruction.RDMSR>".
*/
- if (_tdx_hypercall(EXIT_REASON_MSR_READ, msr, 0, 0, 0, &out))
- return false;
+ if (__tdx_hypercall(&args))
+ panic("Hypercall failed (Buggy TDX module!)\n");
- *val = out.r11;
+ if (args.r10)
+ return false;
+ *val = args.r11;
return true;
}
static bool tdx_write_msr(unsigned int msr, unsigned int low,
unsigned int high)
{
- u64 ret;
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_MSR_WRITE,
+ .r12 = msr,
+ .r13 = (u64)high << 32 | low,
+ };
/*
* Emulate the MSR write via hypercall. More info about ABI
* can be found in TDX Guest-Host-Communication Interface
* (GHCI) sec titled "TDG.VP.VMCALL<Instruction.WRMSR>".
*/
- ret = _tdx_hypercall(EXIT_REASON_MSR_WRITE, msr, (u64)high << 32 | low,
- 0, 0, NULL);
+ if (__tdx_hypercall(&args))
+ panic("Hypercall failed (Buggy TDX module!)\n");
- return ret ? false : true;
+ return args.r10 ? false : true;
}
static bool tdx_handle_cpuid(struct pt_regs *regs)
{
- struct tdx_hypercall_output out;
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_CPUID,
+ .r12 = regs->ax,
+ .r13 = regs->cx,
+ };
/*
* Emulate the CPUID instruction via a hypercall. More info about
* ABI can be found in TDX Guest-Host-Communication Interface
* (GHCI), section titled "VP.VMCALL<Instruction.CPUID>".
*/
- if (_tdx_hypercall(EXIT_REASON_CPUID, regs->ax, regs->cx, 0, 0, &out))
+ if (__tdx_hypercall(&args))
+ panic("Hypercall failed (Buggy TDX module!)\n");
+ if (args.r10)
return false;
/*
@@ -279,10 +299,10 @@ static bool tdx_handle_cpuid(struct pt_regs *regs)
* EAX, EBX, ECX, EDX registers after the CPUID instruction execution.
* So copy the register contents back to pt_regs.
*/
- regs->ax = out.r12;
- regs->bx = out.r13;
- regs->cx = out.r14;
- regs->dx = out.r15;
+ regs->ax = args.r12;
+ regs->bx = args.r13;
+ regs->cx = args.r14;
+ regs->dx = args.r15;
return true;
}
@@ -290,15 +310,21 @@ static bool tdx_handle_cpuid(struct pt_regs *regs)
static int tdx_mmio(int size, bool write, unsigned long addr,
unsigned long *val)
{
- struct tdx_hypercall_output out;
- u64 err;
-
- err = _tdx_hypercall(EXIT_REASON_EPT_VIOLATION, size, write,
- addr, *val, &out);
- if (err)
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_EPT_VIOLATION,
+ .r12 = size,
+ .r13 = write,
+ .r14 = addr,
+ .r15 = *val,
+ };
+
+ if (__tdx_hypercall(&args))
+ panic("Hypercall failed (Buggy TDX module!)\n");
+ if (args.r10)
return -EFAULT;
- *val = out.r11;
+ *val = args.r11;
return 0;
}
@@ -402,8 +428,11 @@ static int tdx_handle_mmio(struct pt_regs *regs, struct ve_info *ve)
*/
static bool tdx_handle_io(struct pt_regs *regs, u32 exit_qual)
{
- struct tdx_hypercall_output out;
- int size, port, ret;
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = EXIT_REASON_IO_INSTRUCTION,
+ };
+ int size, port;
u64 mask;
bool in;
@@ -415,20 +444,25 @@ static bool tdx_handle_io(struct pt_regs *regs, u32 exit_qual)
port = VE_GET_PORT_NUM(exit_qual);
mask = GENMASK(BITS_PER_BYTE * size, 0);
+ args.r12 = size;
+ args.r13 = !in;
+ args.r14 = port;
+ args.r15 = in ? 0 : regs->ax;
+
/*
* Emulate the I/O read/write via hypercall. More info about
* ABI can be found in TDX Guest-Host-Communication Interface
* (GHCI) sec titled "TDG.VP.VMCALL<Instruction.IO>".
*/
- ret = _tdx_hypercall(EXIT_REASON_IO_INSTRUCTION, size, !in, port,
- in ? 0 : regs->ax, &out);
+ if (__tdx_hypercall(&args))
+ panic("Hypercall failed (Buggy TDX module!)\n");
if (!in)
- return !ret;
+ return !args.r10;
regs->ax &= ~mask;
- regs->ax |= ret ? UINT_MAX : out.r11 & mask;
+ regs->ax |= args.r10 ? UINT_MAX : args.r11 & mask;
- return !ret;
+ return !args.r10;
}
/*
diff --git a/arch/x86/kernel/tdxcall.S b/arch/x86/kernel/tdxcall.S
new file mode 100644
index 000000000000..ba5e8e35de36
--- /dev/null
+++ b/arch/x86/kernel/tdxcall.S
@@ -0,0 +1,76 @@
+#include <asm/asm-offsets.h>
+
+/*
+ * TDX guests use the TDCALL instruction to make requests to the
+ * TDX module and hypercalls to the VMM.
+ *
+ * TDX host user SEAMCALL instruction to make requests to TDX module.
+ *
+ * They are supported in Binutils >= 2.36.
+ */
+#define tdcall .byte 0x66,0x0f,0x01,0xcc
+#define seamcall .byte 0x66,0x0f,0x01,0xcf
+
+.macro TDX_MODULE_CALL host:req
+ /*
+ * R12 will be used as temporary storage for struct tdx_module_output
+ * pointer. Since R12-R15 registers are not used by TDCALL/SEAMCALL
+ * services supported by this function, it can be reused.
+ */
+
+ /* Callee saved, so preserve it */
+ push %r12
+
+ /*
+ * Push output pointer to stack.
+ * After the operation, it will be fetched into R12 register.
+ */
+ push %r9
+
+ /* Mangle function call ABI into TDCALL/SEAMCALL ABI: */
+ /* Move Leaf ID to RAX */
+ mov %rdi, %rax
+ /* Move input 4 to R9 */
+ mov %r8, %r9
+ /* Move input 3 to R8 */
+ mov %rcx, %r8
+ /* Move input 1 to RCX */
+ mov %rsi, %rcx
+ /* Leave input param 2 in RDX */
+
+ .if \host
+ seamcall
+ .else
+ tdcall
+ .endif
+
+ /*
+ * Fetch output pointer from stack to R12 (It is used
+ * as temporary storage)
+ */
+ pop %r12
+
+ /* Check for success: 0 - Successful, otherwise failed */
+ test %rax, %rax
+ jnz .Lno_output_struct
+
+ /*
+ * Since this function can be initiated without an output pointer,
+ * check if caller provided an output struct before storing
+ * output registers.
+ */
+ test %r12, %r12
+ jz .Lno_output_struct
+
+ /* Copy result registers to output struct: */
+ movq %rcx, TDX_MODULE_rcx(%r12)
+ movq %rdx, TDX_MODULE_rdx(%r12)
+ movq %r8, TDX_MODULE_r8(%r12)
+ movq %r9, TDX_MODULE_r9(%r12)
+ movq %r10, TDX_MODULE_r10(%r12)
+ movq %r11, TDX_MODULE_r11(%r12)
+
+.Lno_output_struct:
+ /* Restore the state of R12 register */
+ pop %r12
+.endm
--
Kirill A. Shutemov
Kirill,
On Wed, Feb 02 2022 at 05:55, Kirill A. Shutemov wrote:
> On Tue, Feb 01, 2022 at 08:58:59PM +0100, Thomas Gleixner wrote:
> Okay, below is my take on addressing feedback for both __tdx_module_call()
> and __tdx_hypercall().
>
> It is fixup for whole patchset. It has to be folded accordingly. I wanted
> to check if it works and see if I understand your request correctly.
>
> __tdx_module_call() is now implemented by including tdxcall.S can using
> the macro defined there. Host side of TDX can do the same on their side.
> TDX_MODULE_* offsets are now outside of CONFIG_INTEL_TDX_GUEST and can be
> used by both host can guest.
>
> I changed __tdx_hypercall() to take single argument with struct pointer
> that used for both input and output.
>
> Is it the right direction? Or did I misunderstand something?
Looks good. Nice consolidation and I like the idea of using one
structure for in and out.
Thanks,
tglx
On Wed, Feb 02, 2022 at 11:59:10PM +1300, Kai Huang wrote:
>
> > /*
> > diff --git a/arch/x86/kernel/tdxcall.S b/arch/x86/kernel/tdxcall.S
> > new file mode 100644
> > index 000000000000..ba5e8e35de36
> > --- /dev/null
> > +++ b/arch/x86/kernel/tdxcall.S
> > @@ -0,0 +1,76 @@
> > +#include <asm/asm-offsets.h>
> > +
> > +/*
> > + * TDX guests use the TDCALL instruction to make requests to the
> > + * TDX module and hypercalls to the VMM.
> > + *
> > + * TDX host user SEAMCALL instruction to make requests to TDX module.
> > + *
> > + * They are supported in Binutils >= 2.36.
> > + */
> > +#define tdcall .byte 0x66,0x0f,0x01,0xcc
> > +#define seamcall .byte 0x66,0x0f,0x01,0xcf
> > +
> > +.macro TDX_MODULE_CALL host:req
> > + /*
> > + * R12 will be used as temporary storage for struct tdx_module_output
> > + * pointer. Since R12-R15 registers are not used by TDCALL/SEAMCALL
> > + * services supported by this function, it can be reused.
> > + */
> > +
> > + /* Callee saved, so preserve it */
> > + push %r12
> > +
> > + /*
> > + * Push output pointer to stack.
> > + * After the operation, it will be fetched into R12 register.
> > + */
> > + push %r9
> > +
> > + /* Mangle function call ABI into TDCALL/SEAMCALL ABI: */
> > + /* Move Leaf ID to RAX */
> > + mov %rdi, %rax
> > + /* Move input 4 to R9 */
> > + mov %r8, %r9
> > + /* Move input 3 to R8 */
> > + mov %rcx, %r8
> > + /* Move input 1 to RCX */
> > + mov %rsi, %rcx
> > + /* Leave input param 2 in RDX */
>
> Currently host seamcall also uses a data structure which has all possible input
> registers as input argument, rather than having one argument for each register:
>
> struct seamcall_regs_in {
> u64 rcx;
> u64 rdx;
> u64 r8;
> u64 r9;
> };
>
> Which way is better (above struct name can be changed of course)?
>
> Or should we rename tdx_module_output to tdx_module_regs, and use it as both
> input and output (similar to __tdx_hypercall())?
Unlike hypercall case, here we have more managable number of arguments.
I would rather keep input arguments outside of any structs. It is easier
for callers, IMO.
Any objections?
> > +
> > + .if \host
> > + seamcall
> > + .else
> > + tdcall
> > + .endif
> > +
> > + /*
> > + * Fetch output pointer from stack to R12 (It is used
> > + * as temporary storage)
> > + */
> > + pop %r12
>
> For host SEAMCALL, one additional check against VMfailInvalid needs to be
> done after here. This is because at host side, both P-SEAMLDR and TDX module
> are expected to be loaded by UEFI (i.e. UEFI shell tool) before booting to the
> kernel, therefore host kernel needs to detect whether them have been loaded, by
> issuing SEAMCALL.
>
> When SEAM software (P-SEAMLDR or TDX module) is not loaded, the SEAMCALL fails
> with VMfailInvalid. VMfailInvalid is indicated via a combination of RFLAGS
> rather than using %rax. In practice, RFLAGS.CF=1 can be used to check whether
> VMfailInvalid happened, and we need to have something like below:
>
> /*
> * SEAMCALL instruction is essentially a VMExit from VMX root
> * mode to SEAM VMX root mode. VMfailInvalid (CF=1) indicates
> * that the targeted SEAM firmware is not loaded or disabled,
> * or P-SEAMLDR is busy with another SEAMCALL. %rax is not
> * changed in this case.
> *
> * Set %rax to VMFAILINVALID for VMfailInvalid. This value
> * will never be used as actual SEAMCALL error code.
> */
> jnb .Lno_vmfailinvalid
> mov $(VMFAILINVALID), %rax
> jmp .Lno_output_struct
>
> .Lno_vmfailinvalid:
Okay, I will add it under .if \host.
But maybe use JNC instead of JNB? Since we check for CF flag,
Jump-if-Not-Carry sounds more reasonable than Jump-if-Not-Below.
Not-Below of what?
--
Kirill A. Shutemov
On Thu, Feb 03, 2022 at 05:44:03PM +0300, Kirill A. Shutemov wrote:
> Any objections?
Below is proper patch of the idea. It can be used to implement both
SEAMCALL and TDCALL wrappers.
It works for TDCALL. Kai, could you check if it is fine for SEAMCALL?
----------------------------------8<-----------------------------------
From: "Kirill A. Shutemov" <[email protected]>
Date: Fri, 4 Feb 2022 02:03:21 +0300
Subject: [PATCH] x86/tdx: Provide common base for SEAMCALL and TDCALL C
wrappers
Secure Arbitration Mode (SEAM) is an extension of VMX architecture. It
defines a new VMX root operation (SEAM VMX root) and a new VMX non-root
operation (SEAM VMX non-root) which are both isolated from the legacy
VMX operation where the host kernel runs.
A CPU-attested software module (called 'TDX module') runs in SEAM VMX
root to manage and protect VMs running in SEAM VMX non-root. SEAM VMX
root is also used to host another CPU-attested software module (called
'P-SEAMLDR') to load and update the TDX module.
Host kernel transits to either P-SEAMLDR or TDX module via the new
SEAMCALL instruction, which is essentially a VMExit from VMX root mode
to SEAM VMX root mode. SEAMCALLs are leaf functions defined by
P-SEAMLDR and TDX module around the new SEAMCALL instruction.
A guest kernel can also communicate with TDX module via TDCALL
instruction.
TDCALLs and SEAMCALLs use an ABI different from the x86-64 system-v ABI.
RAX is used to carry both the SEAMCALL leaf function number (input) and
the completion status (output). Additional GPRs (RCX, RDX, R8-R11) may
be further used as both input and output operands in individual leaf.
TDCALL and SEAMCALL share the same ABI and require the largely same
code to pass down arguments and retrieve results.
Define an assembly macro that can be used to implement C wrapper for
both TDCALL and SEAMCALL.
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/include/asm/tdx.h | 20 ++++++++
arch/x86/kernel/asm-offsets.c | 9 ++++
arch/x86/kernel/tdxcall.S | 91 +++++++++++++++++++++++++++++++++++
3 files changed, 120 insertions(+)
create mode 100644 arch/x86/kernel/tdxcall.S
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index ba8042ce61c2..2f8cb1e53e77 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -8,6 +8,25 @@
#define TDX_CPUID_LEAF_ID 0x21
#define TDX_IDENT "IntelTDX "
+#define TDX_SEAMCALL_VMFAILINVALID 0x8000FF00FFFF0000ULL
+
+#ifndef __ASSEMBLY__
+
+/*
+ * Used to gather the output registers values of the TDCALL and SEAMCALL
+ * instructions when requesting services from the TDX module.
+ *
+ * This is a software only structure and not part of the TDX module/VMM ABI.
+ */
+struct tdx_module_output {
+ u64 rcx;
+ u64 rdx;
+ u64 r8;
+ u64 r9;
+ u64 r10;
+ u64 r11;
+};
+
#ifdef CONFIG_INTEL_TDX_GUEST
void __init tdx_early_init(void);
@@ -18,4 +37,5 @@ static inline void tdx_early_init(void) { };
#endif /* CONFIG_INTEL_TDX_GUEST */
+#endif /* !__ASSEMBLY__ */
#endif /* _ASM_X86_TDX_H */
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 9fb0a2f8b62a..7dca52f5cfc6 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -18,6 +18,7 @@
#include <asm/bootparam.h>
#include <asm/suspend.h>
#include <asm/tlbflush.h>
+#include <asm/tdx.h>
#ifdef CONFIG_XEN
#include <xen/interface/xen.h>
@@ -65,6 +66,14 @@ static void __used common(void)
OFFSET(XEN_vcpu_info_arch_cr2, vcpu_info, arch.cr2);
#endif
+ BLANK();
+ OFFSET(TDX_MODULE_rcx, tdx_module_output, rcx);
+ OFFSET(TDX_MODULE_rdx, tdx_module_output, rdx);
+ OFFSET(TDX_MODULE_r8, tdx_module_output, r8);
+ OFFSET(TDX_MODULE_r9, tdx_module_output, r9);
+ OFFSET(TDX_MODULE_r10, tdx_module_output, r10);
+ OFFSET(TDX_MODULE_r11, tdx_module_output, r11);
+
BLANK();
OFFSET(BP_scratch, boot_params, scratch);
OFFSET(BP_secure_boot, boot_params, secure_boot);
diff --git a/arch/x86/kernel/tdxcall.S b/arch/x86/kernel/tdxcall.S
new file mode 100644
index 000000000000..27d6fcc8e44c
--- /dev/null
+++ b/arch/x86/kernel/tdxcall.S
@@ -0,0 +1,91 @@
+#include <asm/asm-offsets.h>
+#include <asm/tdx.h>
+
+/*
+ * TDX guests use the TDCALL instruction to make requests to the
+ * TDX module and hypercalls to the VMM.
+ *
+ * TDX host user SEAMCALL instruction to make requests to TDX module.
+ *
+ * They are supported in Binutils >= 2.36.
+ */
+#define tdcall .byte 0x66,0x0f,0x01,0xcc
+#define seamcall .byte 0x66,0x0f,0x01,0xcf
+
+.macro TDX_MODULE_CALL host:req
+ /*
+ * R12 will be used as temporary storage for struct tdx_module_output
+ * pointer. Since R12-R15 registers are not used by TDCALL/SEAMCALL
+ * services supported by this function, it can be reused.
+ */
+
+ /* Callee saved, so preserve it */
+ push %r12
+
+ /*
+ * Push output pointer to stack.
+ * After the operation, it will be fetched into R12 register.
+ */
+ push %r9
+
+ /* Mangle function call ABI into TDCALL/SEAMCALL ABI: */
+ /* Move Leaf ID to RAX */
+ mov %rdi, %rax
+ /* Move input 4 to R9 */
+ mov %r8, %r9
+ /* Move input 3 to R8 */
+ mov %rcx, %r8
+ /* Move input 1 to RCX */
+ mov %rsi, %rcx
+ /* Leave input param 2 in RDX */
+
+ .if \host
+ seamcall
+ /*
+ * SEAMCALL instruction is essentially a VMExit from VMX root
+ * mode to SEAM VMX root mode. VMfailInvalid (CF=1) indicates
+ * that the targeted SEAM firmware is not loaded or disabled,
+ * or P-SEAMLDR is busy with another SEAMCALL. %rax is not
+ * changed in this case.
+ *
+ * Set %rax to TDX_SEAMCALL_VMFAILINVALID for VMfailInvalid.
+ * This value will never be used as actual SEAMCALL error code.
+ */
+ jnc .Lno_vmfailinvalid
+ mov $TDX_SEAMCALL_VMFAILINVALID, %rax
+ jmp .Lno_output_struct
+.Lno_vmfailinvalid:
+ .else
+ tdcall
+ .endif
+
+ /*
+ * Fetch output pointer from stack to R12 (It is used
+ * as temporary storage)
+ */
+ pop %r12
+
+ /* Check for success: 0 - Successful, otherwise failed */
+ test %rax, %rax
+ jnz .Lno_output_struct
+
+ /*
+ * Since this function can be initiated without an output pointer,
+ * check if caller provided an output struct before storing
+ * output registers.
+ */
+ test %r12, %r12
+ jz .Lno_output_struct
+
+ /* Copy result registers to output struct: */
+ movq %rcx, TDX_MODULE_rcx(%r12)
+ movq %rdx, TDX_MODULE_rdx(%r12)
+ movq %r8, TDX_MODULE_r8(%r12)
+ movq %r9, TDX_MODULE_r9(%r12)
+ movq %r10, TDX_MODULE_r10(%r12)
+ movq %r11, TDX_MODULE_r11(%r12)
+
+.Lno_output_struct:
+ /* Restore the state of R12 register */
+ pop %r12
+.endm
--
Kirill A. Shutemov
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -8,6 +8,25 @@
> #define TDX_CPUID_LEAF_ID 0x21
> #define TDX_IDENT "IntelTDX "
Seems above two are not required by assembly file? If so also move them to
within #ifndef __ASSEMBLY__?
>
> +#define TDX_SEAMCALL_VMFAILINVALID 0x8000FF00FFFF0000ULL
> +
> +#ifndef __ASSEMBLY__
> +
> +/*
> + * Used to gather the output registers values of the TDCALL and SEAMCALL
> + * instructions when requesting services from the TDX module.
> + *
> + * This is a software only structure and not part of the TDX module/VMM ABI.
> + */
> +struct tdx_module_output {
> + u64 rcx;
> + u64 rdx;
> + u64 r8;
> + u64 r9;
> + u64 r10;
> + u64 r11;
> +};
> +
Is declaration of __tdx_module_call() outside of CONFIG_INTEL_TDX_GUEST?
> #ifdef CONFIG_INTEL_TDX_GUEST
>
> void __init tdx_early_init(void);
> @@ -18,4 +37,5 @@ static inline void tdx_early_init(void) { };
>
> #endif /* CONFIG_INTEL_TDX_GUEST */
>
> +#endif /* !__ASSEMBLY__ */
> #endif /* _ASM_X86_TDX_H */
> > Is declaration of __tdx_module_call() outside of CONFIG_INTEL_TDX_GUEST?
>
> No, it is defined within CONFIG_INTEL_TDX_GUEST. Why? Host side has to
> build their helper anyway.
>
Right. Fine to me.
On Fri, Feb 04, 2022 at 10:51:38PM +1300, Kai Huang wrote:
> > + .if \host
> > + seamcall
> > + /*
> > + * SEAMCALL instruction is essentially a VMExit from VMX root
> > + * mode to SEAM VMX root mode. VMfailInvalid (CF=1) indicates
> > + * that the targeted SEAM firmware is not loaded or disabled,
> > + * or P-SEAMLDR is busy with another SEAMCALL. %rax is not
> > + * changed in this case.
> > + *
> > + * Set %rax to TDX_SEAMCALL_VMFAILINVALID for VMfailInvalid.
> > + * This value will never be used as actual SEAMCALL error code.
> > + */
> > + jnc .Lno_vmfailinvalid
> > + mov $TDX_SEAMCALL_VMFAILINVALID, %rax
> > + jmp .Lno_output_struct
>
> If I read correctly, in case of VMfailInvalid, another "pop %r12" is needed
> before jmp to .Lno_output_struct, otherwise it doesn't match the stack (pushed
> twice).
Oopsie. Thanks for catching it.
> However, since "test %rax, %rax" will also catch TDX_SEAMCALL_VMFAILINVALID, it
> seems we can just delete above "jmp .Lno_output_struct"?
Good point. Will do it this way.
--
Kirill A. Shutemov
On Fri, Feb 04, 2022 at 11:12:39PM +1300, Kai Huang wrote:
>
> > --- a/arch/x86/include/asm/tdx.h
> > +++ b/arch/x86/include/asm/tdx.h
> > @@ -8,6 +8,25 @@
> > #define TDX_CPUID_LEAF_ID 0x21
> > #define TDX_IDENT "IntelTDX "
>
> Seems above two are not required by assembly file? If so also move them to
> within #ifndef __ASSEMBLY__?
Why? It is harmless.
> >
> > +#define TDX_SEAMCALL_VMFAILINVALID 0x8000FF00FFFF0000ULL
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +/*
> > + * Used to gather the output registers values of the TDCALL and SEAMCALL
> > + * instructions when requesting services from the TDX module.
> > + *
> > + * This is a software only structure and not part of the TDX module/VMM ABI.
> > + */
> > +struct tdx_module_output {
> > + u64 rcx;
> > + u64 rdx;
> > + u64 r8;
> > + u64 r9;
> > + u64 r10;
> > + u64 r11;
> > +};
> > +
>
> Is declaration of __tdx_module_call() outside of CONFIG_INTEL_TDX_GUEST?
No, it is defined within CONFIG_INTEL_TDX_GUEST. Why? Host side has to
build their helper anyway.
> > #ifdef CONFIG_INTEL_TDX_GUEST
> >
> > void __init tdx_early_init(void);
> > @@ -18,4 +37,5 @@ static inline void tdx_early_init(void) { };
> >
> > #endif /* CONFIG_INTEL_TDX_GUEST */
> >
> > +#endif /* !__ASSEMBLY__ */
> > #endif /* _ASM_X86_TDX_H */
--
Kirill A. Shutemov
> +
> +.macro TDX_MODULE_CALL host:req
> + /*
> + * R12 will be used as temporary storage for struct tdx_module_output
> + * pointer. Since R12-R15 registers are not used by TDCALL/SEAMCALL
> + * services supported by this function, it can be reused.
> + */
> +
> + /* Callee saved, so preserve it */
> + push %r12
> +
> + /*
> + * Push output pointer to stack.
> + * After the operation, it will be fetched into R12 register.
> + */
> + push %r9
> +
> + /* Mangle function call ABI into TDCALL/SEAMCALL ABI: */
> + /* Move Leaf ID to RAX */
> + mov %rdi, %rax
> + /* Move input 4 to R9 */
> + mov %r8, %r9
> + /* Move input 3 to R8 */
> + mov %rcx, %r8
> + /* Move input 1 to RCX */
> + mov %rsi, %rcx
> + /* Leave input param 2 in RDX */
> +
> + .if \host
> + seamcall
> + /*
> + * SEAMCALL instruction is essentially a VMExit from VMX root
> + * mode to SEAM VMX root mode. VMfailInvalid (CF=1) indicates
> + * that the targeted SEAM firmware is not loaded or disabled,
> + * or P-SEAMLDR is busy with another SEAMCALL. %rax is not
> + * changed in this case.
> + *
> + * Set %rax to TDX_SEAMCALL_VMFAILINVALID for VMfailInvalid.
> + * This value will never be used as actual SEAMCALL error code.
> + */
> + jnc .Lno_vmfailinvalid
> + mov $TDX_SEAMCALL_VMFAILINVALID, %rax
> + jmp .Lno_output_struct
If I read correctly, in case of VMfailInvalid, another "pop %r12" is needed
before jmp to .Lno_output_struct, otherwise it doesn't match the stack (pushed
twice).
However, since "test %rax, %rax" will also catch TDX_SEAMCALL_VMFAILINVALID, it
seems we can just delete above "jmp .Lno_output_struct"?
> +.Lno_vmfailinvalid:
> + .else
> + tdcall
> + .endif
> +
> + /*
> + * Fetch output pointer from stack to R12 (It is used
> + * as temporary storage)
> + */
> + pop %r12
> +
> + /* Check for success: 0 - Successful, otherwise failed */
> + test %rax, %rax
> + jnz .Lno_output_struct
> +
> + /*
> + * Since this function can be initiated without an output pointer,
> + * check if caller provided an output struct before storing
> + * output registers.
> + */
> + test %r12, %r12
> + jz .Lno_output_struct
> +
> + /* Copy result registers to output struct: */
> + movq %rcx, TDX_MODULE_rcx(%r12)
> + movq %rdx, TDX_MODULE_rdx(%r12)
> + movq %r8, TDX_MODULE_r8(%r12)
> + movq %r9, TDX_MODULE_r9(%r12)
> + movq %r10, TDX_MODULE_r10(%r12)
> + movq %r11, TDX_MODULE_r11(%r12)
> +
> +.Lno_output_struct:
> + /* Restore the state of R12 register */
> + pop %r12
> +.endm
> --
> Kirill A. Shutemov