| 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
| ||
|
Message-ID: <2c1fba82f9425fa569cdf5c2aa22a7b1159cdafb.camel@intel.com>
Date: Fri, 21 Jul 2023 11:02:50 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "peterz@...radead.org" <peterz@...radead.org>
CC: "Hansen, Dave" <dave.hansen@...el.com>,
"Christopherson,, Sean" <seanjc@...gle.com>,
"bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
"hpa@...or.com" <hpa@...or.com>,
"mingo@...hat.com" <mingo@...hat.com>,
"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"pbonzini@...hat.com" <pbonzini@...hat.com>,
"Yamahata, Isaku" <isaku.yamahata@...el.com>,
"sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
"n.borisov.lkml@...il.com" <n.borisov.lkml@...il.com>
Subject: Re: [PATCH v2 00/11] Unify TDCALL/SEAMCALL and TDVMCALL assembly
On Fri, 2023-07-21 at 10:06 +0200, Peter Zijlstra wrote:
> On Fri, Jul 21, 2023 at 12:18:23AM +0000, Huang, Kai wrote:
>
> > Unfortunately I don't think it's feasible. Sean pointed out that
> > kvm_vcpu_arch::regs[] do follow the "register index" hardware layout in x86 (for
> > which I missed sorry), so we cannot re-order KVM part.
> >
> > And unfortunately RBP (5) is in middle of those registers:
> >
> > 0 = RAX
> > 1 = RCX
> > 2 = RDX
> > 3 = RBX
> > 4 = RSP
> > 5 = RBP
> > 6 = RSI
> > 7 = RDI
> > 8–15 represent R8–R15, respectively...
> >
> > Thus unless we add RBP to 'struct tdx_module_args', it's impossible to re-order
> > the structure to match KVM's layout.
>
> Adding RBP to the struct shouldn't be a problem, we'll just not use it.
> Same as RSP, nobody sane would expect that to be used. If you worry
> about it you can give them funny names like:
>
> struct tdx_module_args {
> unsigned long ax;
> unsigned long cx;
> unsigned long dx;
> unsigned long bx;
> unsigned long __sp_unused;
> unsigned long __bp_unused;
> unsigned long si;
> unsigned long di;
> ...
> };
>
> I mean, at this point the whole thing is just 128 bytes of data anyway.
Sure I can do.
How about adding an additional patch on top of this series?
For instance, below patch (compile only):
From c63d01bf91fe23f1e2d2e085644326bdee114d49 Mon Sep 17 00:00:00 2001
From: Kai Huang <kai.huang@...el.com>
Date: Fri, 21 Jul 2023 22:06:31 +1200
Subject: [PATCH] x86/tdx: Add __seamcall_saved_ret() for TDH.VP.ENTER
For TDX guest, KVM uses TDH.VP.ENTER SEAMCALL leaf to enter the guest.
When TDH.VP.ENTER returns to KVM due to TDG.VP.VMCALL from the TDX
guest, all RCX/RDX/RBX/RDI/RSI/R8-R15 are valid output registers, and
the following TDH.VP.ENTER also uses all those registers as input.
Introduce __seamcall_saved_ret(), which uses all above registers as
both input/output, for KVM to support TDH.VP.ENTER.
Also, KVM caches guest's GPRs in 'kvm_vcpu_arch::regs[]', which follows
follows the "register index" hardware layout of x86 GPRs. However the
__seamcall_saved_ret() takes the pointer of 'struct tdx_module_args' as
argument, thus there's a mismatch here.
One option is KVM to copy input registers from 'vcpu::regs[]' to a
'struct tdx_module_args' on the stack and use that to make the SEAMCALL,
but such memory copy isn't desired and should be avoided if possible.
It's not feasible to change KVM's 'vcpu::regs[]' layout due to various
reasons (e.g., emulation code uses decoded register index as array index
to access the register). Therefore, adjust 'struct tdx_module_args' to
match KVM's 'vcpu::regs[]' so that KVM can simply do below:
seamcall_saved_ret(TDH.VP.ENTER,
(struct tdx_module_args *)vcpu->arch->regs);
Note RAX/RSP/RBP are not used by the TDX_MODULE_CALL assembly, but they
are necessary in order match the layout of 'struct tdx_module_args' to
KVM's 'vcpu::regs[]'. Thus add them to the structure, but name them as
*_unused. Also don't include them to asm-offset.c so that any misuse of
them in the assembly would result in build error.
Signed-off-by: Kai Huang <kai.huang@...el.com>
---
arch/x86/include/asm/shared/tdx.h | 19 +++++++++++++------
arch/x86/virt/vmx/tdx/seamcall.S | 19 +++++++++++++++++++
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/shared/tdx.h
b/arch/x86/include/asm/shared/tdx.h
index 74fc466dfdcd..8d1427562c63 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -58,24 +58,31 @@
* Used in __tdcall*() to gather the input/output registers' values of the
* TDCALL instruction when requesting services from the TDX module. This is a
* software only structure and not part of the TDX module/VMM ABI
+ *
+ * Note those *_unused are not used by the TDX_MODULE_CALL assembly.
+ * The layout of this structure also matches KVM's kvm_vcpu_arch::regs[]
+ * layout, which follows the "register index" order of x86 GPRs. KVM
+ * then can simply type cast kvm_vcpu_arch::regs[] to this structure to
+ * avoid the extra memory copy between two structures when making
+ * TDH.VP.ENTER SEAMCALL.
*/
struct tdx_module_args {
- /* callee-clobbered */
+ u64 rax_unused;
u64 rcx;
u64 rdx;
+ u64 rbx;
+ u64 rsp_unused;
+ u64 rbp_unused;
+ u64 rsi;
+ u64 rdi;
u64 r8;
u64 r9;
- /* extra callee-clobbered */
u64 r10;
u64 r11;
- /* callee-saved + rdi/rsi */
u64 r12;
u64 r13;
u64 r14;
u64 r15;
- u64 rbx;
- u64 rdi;
- u64 rsi;
};
/* Used to communicate with the TDX module */
diff --git a/arch/x86/virt/vmx/tdx/seamcall.S b/arch/x86/virt/vmx/tdx/seamcall.S
index b32934837f16..f251ebadb014 100644
--- a/arch/x86/virt/vmx/tdx/seamcall.S
+++ b/arch/x86/virt/vmx/tdx/seamcall.S
@@ -40,3 +40,22 @@ SYM_FUNC_END(__seamcall)
SYM_FUNC_START(__seamcall_ret)
TDX_MODULE_CALL host=1 ret=1
SYM_FUNC_END(__seamcall_ret)
+
+/*
+ * __seamcall_saved_ret() - Host-side interface functions to SEAM software
+ * (the P-SEAMLDR or the TDX module), with saving output registers to the
+ * 'struct tdx_module_args' used as input.
+ *
+ * __seamcall_saved_ret() function ABI:
+ *
+ * @fn (RDI) - SEAMCALL Leaf number, moved to RAX
+ * @args (RSI) - struct tdx_module_args for input and output
+ *
+ * All registers in @args are used as input/output registers.
+ *
+ * Return (via RAX) TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself
+ * fails, or the completion status of the SEAMCALL leaf function.
+ */
+SYM_FUNC_START(__seamcall_ret)
+ TDX_MODULE_CALL host=1 ret=1 saved=1
+SYM_FUNC_END(__seamcall_ret)
--
2.41.0
Powered by blists - more mailing lists