lkml.org 
[lkml]   [2022]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 03/21] x86/virt/tdx: Implement the SEAMCALL base function
    From
    Date
    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


    \
     
     \ /
      Last update: 2022-04-27 01:31    [W:5.090 / U:0.248 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site