| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 21 Dec 2021 20:11:45 +0100
From: Borislav Petkov <bp@...en8.de>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc: tglx@...utronix.de, mingo@...hat.com, dave.hansen@...el.com,
luto@...nel.org, peterz@...radead.org,
sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/26] x86/tdx: Add __tdx_module_call() and
__tdx_hypercall() helper functions
On Tue, Dec 14, 2021 at 06:02:41PM +0300, Kirill A. Shutemov wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
>
> 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) exists in the CPU to facilitate secure communication
in the CPU?!
I think you wanna say, "it is loaded like a firmware into a special CPU
mode called SEAM..." or so.
> between the host and the guest. 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
"module"
> (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).
"module". No need to capitalize every word like in CPU manuals.
>
> 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".
>
> https://www.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface-1.0-344426-002.pdf
>
> Originally-by: Sean Christopherson <seanjc@...gle.com>
Just state that in free text in the commit message:
"Based on a previous patch by Sean... "
...
> + /*
> + * 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 mcall_done
All those local label names need to be prefixed with .L so that they
don't appear in the vmlinux symbol table unnecessarily:
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)
> +
> +mcall_done:
.Lno_output_struct:
Ditto below.
> + /* 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.
^
massage_diff: Warning: Unicode char [–] (0x2013) in line: + * R8-R9, R12–R15 - Same as above.
All those other '-' are char 0x2d but this one has probably happened due
to copy-paste or whatnot. The second time I see UTF-8 chars in a patch
today.
> +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 ret_err
> +
> + /* Save callee-saved GPRs as mandated by the x86_64 ABI */
> + push %r15
> + push %r14
> + push %r13
> + push %r12
> +
> + /*
> + * Save output pointer (rax) in stack, it will be used
"... on the stack... "
> + * 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 hcall_done
> +ret_err:
> + movq $(-EINVAL), %rax
What are the brackets for?
> +hcall_done:
> + FRAME_END
> +
> + retq
> +SYM_FUNC_END(__tdx_hypercall)
> diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
> index d32d9d9946d8..1cc850fd03ff 100644
> --- a/arch/x86/kernel/tdx.c
> +++ b/arch/x86/kernel/tdx.c
> @@ -9,6 +9,30 @@
>
> 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;
> +
> + err = __tdx_hypercall(TDX_HYPERCALL_STANDARD, fn, r12, r13, r14,
> + r15, out);
> +
> + /* Non zero return value indicates buggy TDX module, so panic */
> + if (err)
> + panic("Hypercall fn %llu failed (Buggy TDX module!)\n", fn);
Use a standard formatted pattern pls:
/* 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.32.0
>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists