mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Huang, Kai" <kai.huang@intel.com>
To: "kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>
Cc: "Hansen, Dave" <dave.hansen@intel.com>, "Christopherson,,
	Sean" <seanjc@google.com>, "x86@kernel.org" <x86@kernel.org>,
	"bp@alien8.de" <bp@alien8.de>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"Yamahata, Isaku" <isaku.yamahata@intel.com>,
	"sathyanarayanan.kuppuswamy@linux.intel.com" 
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	"n.borisov.lkml@gmail.com" <n.borisov.lkml@gmail.com>
Subject: Re: [PATCH v3 11/12] x86/virt/tdx: Allow SEAMCALL to handle #UD and #GP
Date: Mon, 7 Aug 2023 02:14:37 +0000	[thread overview]
Message-ID: <b15b45ff5dc2fcfa08dfb3171c269d9ab0349088.camel@intel.com> (raw)
In-Reply-To: <20230806114131.2ilofgmxhdaa2u6b@box.shutemov.name>

On Sun, 2023-08-06 at 14:41 +0300, kirill.shutemov@linux.intel.com wrote:
> On Wed, Jul 26, 2023 at 11:25:13PM +1200, Kai Huang wrote:
> > @@ -20,6 +21,9 @@
> >  #define TDX_SW_ERROR			(TDX_ERROR | GENMASK_ULL(47, 40))
> >  #define TDX_SEAMCALL_VMFAILINVALID	(TDX_SW_ERROR | _UL(0xFFFF0000))
> >  
> > +#define TDX_SEAMCALL_GP			(TDX_SW_ERROR | X86_TRAP_GP)
> > +#define TDX_SEAMCALL_UD			(TDX_SW_ERROR | X86_TRAP_UD)
> 
> Is there any explantion how these error codes got chosen? Looks very
> arbitrary and may collide with other error codes in the future.
> 

Any error code has TDX_SW_ERROR is reserved to software use so the TDX module
can never return any error code which conflicts with those software ones.

For why to choose these two, I believe XOR the TRAP number to TDX_SW_ERROR is
the simplest way to achieve: 1) costing minimal assembly code; 2)
opportunistically handling #GP too, allowing caller to distinguish the two
errors.

I can add this to the changelog.

Btw, as I chatted to you I believe we have another justification to handle
#UD/#GP in the assembly: emergency virtualization disable.  Thus we can even get
rid of the erratum staff in the changelog.

How does below look like?

commit d3ff21da1083a525eb2cac6576490045e22f6f5d
Author: Kai Huang <kai.huang@intel.com>
Date:   Mon Jun 26 16:04:08 2023 +1200

    x86/virt/tdx: Allow SEAMCALL to handle #UD and #GP
    
    SEAMCALL instruction causes #UD if the CPU isn't in VMX operation.
    Currently the TDX_MODULE_CALL assembly doesn't handle #UD, thus making
    SEAMCALL when VMX is disabled would cause Oops.
    
    Unfortunately, there are legal cases that SEAMCALL can be made when VMX
    is disabled.  For instance, VMX can be disabled due to emergency reboot
    while there are still TDX guest is running.
    
    Extend the TDX_MODULE_CALL assembly to return an error code for #UD to
    handle this case gracefully, e.g., KVM can then quitely eat all SEAMCALL
    errors caused by emergency reboot.
    
    SEAMCALL instruction also causes #GP when TDX isn't enabled by the BIOS.
    Use _ASM_EXTABLE_FAULT() to catch both exceptions with the trap number
    recorded, and define two new error codes by XORing the trap number to
    the TDX_SW_ERROR.  This opportunistically handles #GP too while using
    the same simple assembly code.
    
    A bonus is when kernel mistakenly calls SEAMCALL when CPU isn't in VMX
    operation, or when TDX isn't enabled by the BIOS, or when the BIOS is
    buggy, the kernel can get a nicer error code rather than a less
    understandable Oops.
    
    This is basically based on Peter's code.
    
    Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
    Cc: Dave Hansen <dave.hansen@linux.intel.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Suggested-by: Peter Zijlstra <peterz@infradead.org>
    Signed-off-by: Kai Huang <kai.huang@intel.com>

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 603e6d1e9d4a..6b8547dc40fd 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -8,6 +8,7 @@
 
 #include <asm/errno.h>
 #include <asm/ptrace.h>
+#include <asm/trapnr.h>
 #include <asm/shared/tdx.h>
 
 /*
@@ -20,6 +21,9 @@
 #define TDX_SW_ERROR                   (TDX_ERROR | GENMASK_ULL(47, 40))
 #define TDX_SEAMCALL_VMFAILINVALID     (TDX_SW_ERROR | _UL(0xFFFF0000))
 
+#define TDX_SEAMCALL_GP                        (TDX_SW_ERROR | X86_TRAP_GP)
+#define TDX_SEAMCALL_UD                        (TDX_SW_ERROR | X86_TRAP_UD)
+
 #ifndef __ASSEMBLY__
 
 /*
diff --git a/arch/x86/virt/vmx/tdx/tdxcall.S b/arch/x86/virt/vmx/tdx/tdxcall.S
index 3f0b83a9977e..016a2a1ec1d6 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/frame.h>
+#include <asm/asm.h>
 #include <asm/tdx.h>
 
 /*
@@ -85,6 +86,7 @@
 .endif /* \saved */
 
 .if \host
+.Lseamcall\@:
        seamcall
        /*
         * SEAMCALL instruction is essentially a VMExit from VMX root
@@ -191,11 +193,28 @@
 .if \host
 .Lseamcall_vmfailinvalid\@:
        mov $TDX_SEAMCALL_VMFAILINVALID, %rax
+       jmp .Lseamcall_fail\@
+
+.Lseamcall_trap\@:
+       /*
+        * SEAMCALL caused #GP or #UD.  By reaching here RAX contains
+        * the trap number.  Convert the trap number to the TDX error
+        * code by setting TDX_SW_ERROR to the high 32-bits of RAX.
+        *
+        * Note cannot OR TDX_SW_ERROR directly to RAX as OR instruction
+        * only accepts 32-bit immediate at most.
+        */
+       movq $TDX_SW_ERROR, %rdi
+       orq  %rdi, %rax
+
+.Lseamcall_fail\@:
 .if \ret && \saved
        /* pop the unused structure pointer back to RSI */
        popq %rsi
 .endif
        jmp .Lout\@
+
+       _ASM_EXTABLE_FAULT(.Lseamcall\@, .Lseamcall_trap\@)
 .endif /* \host */
 
 .endm

  reply	other threads:[~2023-08-07  2:14 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-26 11:25 [PATCH v3 00/12] Unify TDCALL/SEAMCALL and TDVMCALL assembly Kai Huang
2023-07-26 11:25 ` [PATCH v3 01/12] x86/tdx: Zero out the missing RSI in TDX_HYPERCALL macro Kai Huang
2023-07-27 12:48   ` kirill.shutemov
2023-07-26 11:25 ` [PATCH v3 02/12] x86/tdx: Skip saving output regs when SEAMCALL fails with VMFailInvalid Kai Huang
2023-07-27 12:52   ` kirill.shutemov
2023-07-27 22:55     ` Huang, Kai
2023-07-26 11:25 ` [PATCH v3 03/12] x86/tdx: Make macros of TDCALLs consistent with the spec Kai Huang
2023-07-27 13:00   ` kirill.shutemov
2023-07-28  1:54   ` Sathyanarayanan Kuppuswamy
2023-07-28  2:45     ` Huang, Kai
2023-07-26 11:25 ` [PATCH v3 04/12] x86/tdx: Rename __tdx_module_call() to __tdcall() Kai Huang
2023-07-27 13:02   ` kirill.shutemov
2023-07-28 15:33   ` Sathyanarayanan Kuppuswamy
2023-07-26 11:25 ` [PATCH v3 05/12] x86/tdx: Pass TDCALL/SEAMCALL input/output registers via a structure Kai Huang
2023-07-27 16:36   ` kirill.shutemov
2023-07-27 22:54     ` Huang, Kai
2023-08-03 10:58       ` kirill.shutemov
2023-08-03 11:35         ` Huang, Kai
2023-08-03 11:47           ` kirill.shutemov
2023-07-26 11:25 ` [PATCH v3 06/12] x86/tdx: Extend TDX_MODULE_CALL to support more TDCALL/SEAMCALL leafs Kai Huang
2023-07-27 16:50   ` kirill.shutemov
2023-07-27 22:58     ` Huang, Kai
2023-07-26 11:25 ` [PATCH v3 07/12] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL Kai Huang
2023-07-27 17:10   ` kirill.shutemov
2023-07-27 23:05     ` Huang, Kai
2023-08-03 11:45       ` kirill.shutemov
2023-08-03 11:56         ` Huang, Kai
2023-08-03 12:12           ` kirill.shutemov
2023-08-03 12:41             ` Huang, Kai
2023-08-03 13:47               ` kirill.shutemov
2023-08-03 22:41                 ` Huang, Kai
2023-07-26 11:25 ` [PATCH v3 08/12] x86/tdx: Reimplement __tdx_hypercall() using TDX_MODULE_CALL asm Kai Huang
2023-08-06 11:25   ` kirill.shutemov
2023-07-26 11:25 ` [PATCH v3 09/12] x86/tdx: Remove 'struct tdx_hypercall_args' Kai Huang
2023-08-06 11:29   ` kirill.shutemov
2023-07-26 11:25 ` [PATCH v3 10/12] x86/virt/tdx: Wire up basic SEAMCALL functions Kai Huang
2023-08-06 11:36   ` kirill.shutemov
2023-08-07  1:40     ` Huang, Kai
2023-08-07 14:30       ` Sean Christopherson
2023-08-07 23:51         ` Huang, Kai
2023-07-26 11:25 ` [PATCH v3 11/12] x86/virt/tdx: Allow SEAMCALL to handle #UD and #GP Kai Huang
2023-08-06 11:41   ` kirill.shutemov
2023-08-07  2:14     ` Huang, Kai [this message]
2023-08-07  9:53       ` kirill.shutemov
2023-08-07 12:41         ` Huang, Kai
2023-08-07 14:27           ` kirill.shutemov
2023-08-07 14:46     ` Dave Hansen
2023-07-26 11:25 ` [PATCH v3 12/12] x86/virt/tdx: Adjust 'struct tdx_module_args' to use x86 "register index" layout Kai Huang
2023-08-02 21:32   ` Isaku Yamahata
2023-08-02 23:20     ` Huang, Kai
2023-08-02 21:39   ` Isaku Yamahata
2023-08-06 11:50   ` kirill.shutemov
2023-08-07  2:16     ` Huang, Kai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b15b45ff5dc2fcfa08dfb3171c269d9ab0349088.camel@intel.com \
    --to=kai.huang@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=isaku.yamahata@intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=n.borisov.lkml@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome