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. -- Thanks, -Kai
