lkml.org 
[lkml]   [2022]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 08/22] x86/virt/tdx: Shut down TDX module in case of error
From
On 6/27/22 15:34, Kai Huang wrote:
> On Mon, 2022-06-27 at 13:46 -0700, Dave Hansen wrote:
> I think I can just use __always_unused for this purpose?
>
> So I think we put seamcall() implementation to the patch which implements
> __seamcall(). And we can inline for seamcall() and put it in either tdx.h or
> tdx.c, or we can use __always_unused (or the one you prefer) to get rid of the
> warning.
>
> What's your opinion?

A temporary __always_unused seems fine to me.

>>> Alternatively, we can always add EXTABLE to TDX_MODULE_CALL macro to handle #UD
>>> and #GP by returning dedicated error codes (please also see my reply to previous
>>> patch for the code needed to handle), in which case we don't need such check
>>> here.
>>>
>>> Always handling #UD in TDX_MODULE_CALL macro also has another advantage: there
>>> will be no Oops for #UD regardless the issue that "there's no way to check
>>> whether VMXON has been done" in the above comment.
>>>
>>> What's your opinion?
>>
>> I think you should explore using the EXTABLE. Let's see how it looks.
>
> I tried to wrote the code before. I didn't test but it should look like to
> something below. Any comments?
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 4b75c930fa1b..4a97ca8eb14c 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -8,6 +8,7 @@
> #include <asm/ptrace.h>
> #include <asm/shared/tdx.h>
>
> +#ifdef CONFIG_INTEL_TDX_HOST
> /*
> * SW-defined error codes.
> *
> @@ -18,6 +19,21 @@
> #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40))
> #define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000))
>
> +/*
> + * Special error codes to indicate SEAMCALL #GP and #UD.
> + *
> + * SEAMCALL causes #GP when SEAMRR is not properly enabled by BIOS, and
> + * causes #UD when CPU is not in VMX operation. Define two separate
> + * error codes to distinguish the two cases so caller can be aware of
> + * what caused the SEAMCALL to fail.
> + *
> + * Bits 61:48 are reserved bits which will never be set by the TDX
> + * module. Borrow 2 reserved bits to represent #GP and #UD.
> + */
> +#define TDX_SEAMCALL_GP (TDX_ERROR | GENMASK_ULL(48, 48))
> +#define TDX_SEAMCALL_UD (TDX_ERROR | GENMASK_ULL(49, 49))
> +#endif
> +
> #ifndef __ASSEMBLY__
>
> /*
> diff --git a/arch/x86/virt/vmx/tdx/tdxcall.S b/arch/x86/virt/vmx/tdx/tdxcall.S
> index 49a54356ae99..7431c47258d9 100644
> --- a/arch/x86/virt/vmx/tdx/tdxcall.S
> +++ b/arch/x86/virt/vmx/tdx/tdxcall.S
> @@ -1,6 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> #include <asm/asm-offsets.h>
> #include <asm/tdx.h>
> +#include <asm/asm.h>
>
> /*
> * TDCALL and SEAMCALL are supported in Binutils >= 2.36.
> @@ -45,6 +46,7 @@
> /* Leave input param 2 in RDX */
>
> .if \host
> +1:
> seamcall
> /*
> * SEAMCALL instruction is essentially a VMExit from VMX root
> @@ -57,9 +59,25 @@
> * This value will never be used as actual SEAMCALL error code as
> * it is from the Reserved status code class.
> */
> - jnc .Lno_vmfailinvalid
> + jnc .Lseamcall_out
> mov $TDX_SEAMCALL_VMFAILINVALID, %rax
> -.Lno_vmfailinvalid:
> + jmp .Lseamcall_out
> +2:
> + /*
> + * SEAMCALL caused #GP or #UD. By reaching here %eax contains
> + * the trap number. Check the trap number and set up the return
> + * value to %rax.
> + */
> + cmp $X86_TRAP_GP, %eax
> + je .Lseamcall_gp
> + mov $TDX_SEAMCALL_UD, %rax
> + jmp .Lseamcall_out
> +.Lseamcall_gp:
> + mov $TDX_SEAMCALL_GP, %rax
> + jmp .Lseamcall_out
> +
> + _ASM_EXTABLE_FAULT(1b, 2b)
> +.Lseamcall_out

Not too bad, although the end of that is a bit ugly. It would be nicer
if you could just return the %rax value in the exception section instead
of having to do the transform there. Maybe have a TDX_ERROR code with
enough bits to hold any X86_TRAP_FOO.

It'd be nice if Peter Z or Andy L has a sec to look at this. Seems like
the kind of thing they'd have good ideas about.

\
 
 \ /
  Last update: 2022-06-28 00:58    [W:0.217 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site