| Message ID | 1c3f555934c73301a9cbf10232500f3d15efe3cc.1649219184.git.kai.huang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TDX host kernel support | expand |
On 4/5/22 9:49 PM, Kai Huang wrote: > SEAMCALL leaf functions use an ABI different from the x86-64 system-v > ABI. Instead, they share the same ABI with the TDCALL leaf functions. TDCALL is a new term for this patch set. Maybe add some detail about it in ()?. > %rax is used to carry both the SEAMCALL leaf function number (input) and > the completion status code (output). Additional GPRs (%rcx, %rdx, > %r8->%r11) may be further used as both input and output operands in > individual leaf functions.
On Tue, 2022-04-19 at 07:07 -0700, Sathyanarayanan Kuppuswamy wrote: > > On 4/5/22 9:49 PM, Kai Huang wrote: > > SEAMCALL leaf functions use an ABI different from the x86-64 system-v > > ABI. Instead, they share the same ABI with the TDCALL leaf functions. > > TDCALL is a new term for this patch set. Maybe add some detail about > it in ()?. > > > TDCALL implementation is already in tip/tdx. This series will be rebased to it. I don't think we need to explain more about something that is already in the tip tree?
On 4/19/22 9:16 PM, Kai Huang wrote: > On Tue, 2022-04-19 at 07:07 -0700, Sathyanarayanan Kuppuswamy wrote: >> >> On 4/5/22 9:49 PM, Kai Huang wrote: >>> SEAMCALL leaf functions use an ABI different from the x86-64 system-v >>> ABI. Instead, they share the same ABI with the TDCALL leaf functions. >> >> TDCALL is a new term for this patch set. Maybe add some detail about >> it in ()?. >> >>> > > TDCALL implementation is already in tip/tdx. This series will be rebased to it. > I don't think we need to explain more about something that is already in the tip > tree? Since you have already expanded terms like TD,TDX and SEAM in this patch set, I thought you wanted to explain TDX terms to make it easy for new readers. So to keep it uniform, I have suggested adding some brief details about the TDCALL. But I am fine either way. > >
On Wed, 2022-04-20 at 00:29 -0700, Sathyanarayanan Kuppuswamy wrote: > > On 4/19/22 9:16 PM, Kai Huang wrote: > > On Tue, 2022-04-19 at 07:07 -0700, Sathyanarayanan Kuppuswamy wrote: > > > > > > On 4/5/22 9:49 PM, Kai Huang wrote: > > > > SEAMCALL leaf functions use an ABI different from the x86-64 system-v > > > > ABI. Instead, they share the same ABI with the TDCALL leaf functions. > > > > > > TDCALL is a new term for this patch set. Maybe add some detail about > > > it in ()?. > > > > > > > > > > > TDCALL implementation is already in tip/tdx. This series will be rebased to it. > > I don't think we need to explain more about something that is already in the tip > > tree? > > Since you have already expanded terms like TD,TDX and SEAM in this patch > set, I thought you wanted to explain TDX terms to make it easy for new > readers. So to keep it uniform, I have suggested adding some brief > details about the TDCALL. > > All right. I can add one sentence to explain it.
On 4/5/22 21:49, Kai Huang wrote: > 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 isolated from legacy VMX root > and VMX non-root mode. I feel like this is too much detail for an opening paragraph. > A CPU-attested software module (called the 'TDX module') runs in SEAM > VMX root to manage the crypto-protected VMs running in SEAM VMX non-root. > SEAM VMX root is also used to host another CPU-attested software module > (called the 'P-SEAMLDR') to load and update the TDX module. >> Host kernel transits to either the P-SEAMLDR or the TDX module via the > new SEAMCALL instruction. SEAMCALL leaf functions are host-side > interface functions defined by the P-SEAMLDR and the TDX module around > the new SEAMCALL instruction. They are similar to a hypercall, except > they are made by host kernel to the SEAM software. I think you can get rid of about half of this changelog so farand make it more clear in the process with this: TDX introduces a new CPU mode: Secure Arbitration Mode (SEAM). This mode runs only the TDX module itself or other code needed to load the TDX module. The host kernel communicates with SEAM software via a new SEAMCALL instruction. This is conceptually similar to a guest->host hypercall, except it is made from the host to SEAM software instead. This is a technical document, but you're writing too technically for my taste and focusing on the low-level details rather than the high-level concepts. What do I care that SEAM is two modes and what their names are at this juncture? Are those details necesarry to get me to understand what a SEAMCALL is or what this patch implements? > SEAMCALL leaf functions use an ABI different from the x86-64 system-v > ABI. Instead, they share the same ABI with the TDCALL leaf functions. > %rax is used to carry both the SEAMCALL leaf function number (input) and > the completion status code (output). Additional GPRs (%rcx, %rdx, > %r8->%r11) may be further used as both input and output operands in > individual leaf functions. > > Implement a C function __seamcall() Your "C function" looks a bit like assembly to me. > to do SEAMCALL leaf functions using > the assembly macro used by __tdx_module_call() (the implementation of > TDCALL leaf functions). The only exception not covered here is TDENTER > leaf function which takes all GPRs and XMM0-XMM15 as both input and > output. The caller of TDENTER should implement its own logic to call > TDENTER directly instead of using this function. I have no idea why this paragraph is here or what it is trying to tell me. > SEAMCALL instruction is essentially a VMExit from VMX root to SEAM VMX > root, and it can fail with VMfailInvalid, for instance, when the SEAM > software module is not loaded. The C function __seamcall() returns > TDX_SEAMCALL_VMFAILINVALID, which doesn't conflict with any actual error > code of SEAMCALLs, to uniquely represent this case. Again, I'm lost. Why is this detail here? I don't even see TDX_SEAMCALL_VMFAILINVALID in the patch. > diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile > index 1bd688684716..fd577619620e 100644 > --- a/arch/x86/virt/vmx/tdx/Makefile > +++ b/arch/x86/virt/vmx/tdx/Makefile > @@ -1,2 +1,2 @@ > # SPDX-License-Identifier: GPL-2.0-only > -obj-$(CONFIG_INTEL_TDX_HOST) += tdx.o > +obj-$(CONFIG_INTEL_TDX_HOST) += tdx.o seamcall.o > diff --git a/arch/x86/virt/vmx/tdx/seamcall.S b/arch/x86/virt/vmx/tdx/seamcall.S > new file mode 100644 > index 000000000000..327961b2dd5a > --- /dev/null > +++ b/arch/x86/virt/vmx/tdx/seamcall.S > @@ -0,0 +1,52 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#include <linux/linkage.h> > +#include <asm/frame.h> > + > +#include "tdxcall.S" > + > +/* > + * __seamcall() - Host-side interface functions to SEAM software module > + * (the P-SEAMLDR or the TDX module) > + * > + * Transform function call register arguments into the SEAMCALL register > + * ABI. Return TDX_SEAMCALL_VMFAILINVALID, or the completion status of > + * the SEAMCALL. Additional output operands are saved in @out (if it is > + * provided by caller). This needs to say: Returns TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself fails. > + *------------------------------------------------------------------------- > + * SEAMCALL ABI: > + *------------------------------------------------------------------------- > + * Input Registers: > + * > + * RAX - SEAMCALL Leaf number. > + * RCX,RDX,R8-R9 - SEAMCALL Leaf specific input registers. > + * > + * Output Registers: > + * > + * RAX - SEAMCALL completion status code. > + * RCX,RDX,R8-R11 - SEAMCALL Leaf specific output registers. > + * > + *------------------------------------------------------------------------- > + * > + * __seamcall() function ABI: > + * > + * @fn (RDI) - SEAMCALL Leaf number, 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 > + * used by the P-SEAMLDR or the TDX > + * module). It can be NULL. > + * > + * Return (via RAX) the completion status of the SEAMCALL, or > + * TDX_SEAMCALL_VMFAILINVALID. > + */ > +SYM_FUNC_START(__seamcall) > + FRAME_BEGIN > + TDX_MODULE_CALL host=1 > + FRAME_END > + ret > +SYM_FUNC_END(__seamcall) > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h > new file mode 100644 > index 000000000000..9d5b6f554c20 > --- /dev/null > +++ b/arch/x86/virt/vmx/tdx/tdx.h > @@ -0,0 +1,11 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _X86_VIRT_TDX_H > +#define _X86_VIRT_TDX_H > + > +#include <linux/types.h> > + > +struct tdx_module_output; > +u64 __seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9, > + struct tdx_module_output *out); > + > +#endif
On Tue, 2022-04-26 at 13:37 -0700, Dave Hansen wrote: > On 4/5/22 21:49, Kai Huang wrote: > > 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 isolated from legacy VMX root > > and VMX non-root mode. > > I feel like this is too much detail for an opening paragraph. > > > A CPU-attested software module (called the 'TDX module') runs in SEAM > > VMX root to manage the crypto-protected VMs running in SEAM VMX non-root. > > SEAM VMX root is also used to host another CPU-attested software module > > (called the 'P-SEAMLDR') to load and update the TDX module. > > > Host kernel transits to either the P-SEAMLDR or the TDX module via the > > new SEAMCALL instruction. SEAMCALL leaf functions are host-side > > interface functions defined by the P-SEAMLDR and the TDX module around > > the new SEAMCALL instruction. They are similar to a hypercall, except > > they are made by host kernel to the SEAM software. > > I think you can get rid of about half of this changelog so farand make > it more clear in the process with this: > > TDX introduces a new CPU mode: Secure Arbitration Mode (SEAM). > This mode runs only the TDX module itself or other code needed > to load the TDX module. > > The host kernel communicates with SEAM software via a new > SEAMCALL instruction. This is conceptually similar to > a guest->host hypercall, except it is made from the host to SEAM > software instead. Thank you! > > This is a technical document, but you're writing too technically for my > taste and focusing on the low-level details rather than the high-level > concepts. What do I care that SEAM is two modes and what their names > are at this juncture? Are those details necesarry to get me to > understand what a SEAMCALL is or what this patch implements? Thanks for the point. I'll revisit this series based on this in next version. > > > SEAMCALL leaf functions use an ABI different from the x86-64 system-v > > ABI. Instead, they share the same ABI with the TDCALL leaf functions. > > %rax is used to carry both the SEAMCALL leaf function number (input) and > > the completion status code (output). Additional GPRs (%rcx, %rdx, > > %r8->%r11) may be further used as both input and output operands in > > individual leaf functions. > > > > Implement a C function __seamcall() > > Your "C function" looks a bit like assembly to me. Will change to (I saw TDX guest patch used similar way): Add a generic interface to do SEAMCALL leaf functions, using the assembly macro used by __tdx_module_call(). > > > to do SEAMCALL leaf functions using > > the assembly macro used by __tdx_module_call() (the implementation of > > TDCALL leaf functions). The only exception not covered here is TDENTER > > leaf function which takes all GPRs and XMM0-XMM15 as both input and > > output. The caller of TDENTER should implement its own logic to call > > TDENTER directly instead of using this function. > > I have no idea why this paragraph is here or what it is trying to tell me. Will get rid of the rest staff. > > > SEAMCALL instruction is essentially a VMExit from VMX root to SEAM VMX > > root, and it can fail with VMfailInvalid, for instance, when the SEAM > > software module is not loaded. The C function __seamcall() returns > > TDX_SEAMCALL_VMFAILINVALID, which doesn't conflict with any actual error > > code of SEAMCALLs, to uniquely represent this case. > > Again, I'm lost. Why is this detail here? I don't even see > TDX_SEAMCALL_VMFAILINVALID in the patch. Will remove. > > > diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile > > index 1bd688684716..fd577619620e 100644 > > --- a/arch/x86/virt/vmx/tdx/Makefile > > +++ b/arch/x86/virt/vmx/tdx/Makefile > > @@ -1,2 +1,2 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > -obj-$(CONFIG_INTEL_TDX_HOST) += tdx.o > > +obj-$(CONFIG_INTEL_TDX_HOST) += tdx.o seamcall.o > > diff --git a/arch/x86/virt/vmx/tdx/seamcall.S b/arch/x86/virt/vmx/tdx/seamcall.S > > new file mode 100644 > > index 000000000000..327961b2dd5a > > --- /dev/null > > +++ b/arch/x86/virt/vmx/tdx/seamcall.S > > @@ -0,0 +1,52 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#include <linux/linkage.h> > > +#include <asm/frame.h> > > + > > +#include "tdxcall.S" > > + > > +/* > > + * __seamcall() - Host-side interface functions to SEAM software module > > + * (the P-SEAMLDR or the TDX module) > > + * > > + * Transform function call register arguments into the SEAMCALL register > > + * ABI. Return TDX_SEAMCALL_VMFAILINVALID, or the completion status of > > + * the SEAMCALL. Additional output operands are saved in @out (if it is > > + * provided by caller). > > This needs to say: > > Returns TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself fails. OK.
diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile index 1bd688684716..fd577619620e 100644 --- a/arch/x86/virt/vmx/tdx/Makefile +++ b/arch/x86/virt/vmx/tdx/Makefile @@ -1,2 +1,2 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_INTEL_TDX_HOST) += tdx.o +obj-$(CONFIG_INTEL_TDX_HOST) += tdx.o seamcall.o diff --git a/arch/x86/virt/vmx/tdx/seamcall.S b/arch/x86/virt/vmx/tdx/seamcall.S new file mode 100644 index 000000000000..327961b2dd5a --- /dev/null +++ b/arch/x86/virt/vmx/tdx/seamcall.S @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <linux/linkage.h> +#include <asm/frame.h> + +#include "tdxcall.S" + +/* + * __seamcall() - Host-side interface functions to SEAM software module + * (the P-SEAMLDR or the TDX module) + * + * Transform function call register arguments into the SEAMCALL register + * ABI. Return TDX_SEAMCALL_VMFAILINVALID, or the completion status of + * the SEAMCALL. Additional output operands are saved in @out (if it is + * provided by caller). + * + *------------------------------------------------------------------------- + * SEAMCALL ABI: + *------------------------------------------------------------------------- + * Input Registers: + * + * RAX - SEAMCALL Leaf number. + * RCX,RDX,R8-R9 - SEAMCALL Leaf specific input registers. + * + * Output Registers: + * + * RAX - SEAMCALL completion status code. + * RCX,RDX,R8-R11 - SEAMCALL Leaf specific output registers. + * + *------------------------------------------------------------------------- + * + * __seamcall() function ABI: + * + * @fn (RDI) - SEAMCALL Leaf number, 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 + * used by the P-SEAMLDR or the TDX + * module). It can be NULL. + * + * Return (via RAX) the completion status of the SEAMCALL, or + * TDX_SEAMCALL_VMFAILINVALID. + */ +SYM_FUNC_START(__seamcall) + FRAME_BEGIN + TDX_MODULE_CALL host=1 + FRAME_END + ret +SYM_FUNC_END(__seamcall) diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h new file mode 100644 index 000000000000..9d5b6f554c20 --- /dev/null +++ b/arch/x86/virt/vmx/tdx/tdx.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _X86_VIRT_TDX_H +#define _X86_VIRT_TDX_H + +#include <linux/types.h> + +struct tdx_module_output; +u64 __seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9, + struct tdx_module_output *out); + +#endif
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 isolated from legacy VMX root and VMX non-root mode. A CPU-attested software module (called the 'TDX module') runs in SEAM VMX root to manage the crypto-protected VMs running in SEAM VMX non-root. SEAM VMX root is also used to host another CPU-attested software module (called the 'P-SEAMLDR') to load and update the TDX module. Host kernel transits to either the P-SEAMLDR or the TDX module via the new SEAMCALL instruction. SEAMCALL leaf functions are host-side interface functions defined by the P-SEAMLDR and the TDX module around the new SEAMCALL instruction. They are similar to a hypercall, except they are made by host kernel to the SEAM software. SEAMCALL leaf functions use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL leaf functions. %rax is used to carry both the SEAMCALL leaf function number (input) and the completion status code (output). Additional GPRs (%rcx, %rdx, %r8->%r11) may be further used as both input and output operands in individual leaf functions. Implement a C function __seamcall() to do SEAMCALL leaf functions using the assembly macro used by __tdx_module_call() (the implementation of TDCALL leaf functions). The only exception not covered here is TDENTER leaf function which takes all GPRs and XMM0-XMM15 as both input and output. The caller of TDENTER should implement its own logic to call TDENTER directly instead of using this function. SEAMCALL instruction is essentially a VMExit from VMX root to SEAM VMX root, and it can fail with VMfailInvalid, for instance, when the SEAM software module is not loaded. The C function __seamcall() returns TDX_SEAMCALL_VMFAILINVALID, which doesn't conflict with any actual error code of SEAMCALLs, to uniquely represent this case. Signed-off-by: Kai Huang <kai.huang@intel.com> --- arch/x86/virt/vmx/tdx/Makefile | 2 +- arch/x86/virt/vmx/tdx/seamcall.S | 52 ++++++++++++++++++++++++++++++++ arch/x86/virt/vmx/tdx/tdx.h | 11 +++++++ 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 arch/x86/virt/vmx/tdx/seamcall.S create mode 100644 arch/x86/virt/vmx/tdx/tdx.h