On Thu, Jun 29, 2023 at 10:33:38AM +0000, Huang, Kai wrote: > On Wed, 2023-06-28 at 22:38 +0200, Peter Zijlstra wrote: > > On Wed, Jun 28, 2023 at 05:29:01PM +0200, Peter Zijlstra wrote: > > > On Tue, Jun 27, 2023 at 02:12:50AM +1200, Kai Huang wrote: > > > > diff --git a/arch/x86/virt/vmx/tdx/tdxcall.S b/arch/x86/virt/vmx/tdx/tdxcall.S > > > > index 49a54356ae99..757b0c34be10 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 > > > > > > So what registers are actually clobbered by SEAMCALL ? There's a > > > distinct lack of it in SDM Vol.2 instruction list :-( > > > > With the exception of the abomination that is TDH.VP.ENTER all SEAMCALLs > > seem to be limited to the set presented here (c,d,8,9,10,11) and all > > other registers should be available. > > RAX is also used as SEAMCALL return code. > > Looking at the later versions of TDX spec (with TD live migration, etc), it > seems they are already using R12-R13 as SEAMCALL output: > > https://cdrdv2.intel.com/v1/dl/getContent/733579 Urgh.. I think I read an older versio because I got bleeding eyes from all this colour coded crap. All this red is unreadable :-( Have they been told about the glories of TeX and diff ? > E.g., 6.3.15. NEW: TDH.IMPORT.MEM Leaf > > It uses R12 and R13 as input. 12 and 14. They skipped 13 for some mysterious raisin. But also, 10,11 are frequently used as input with this new stuff, which already suggests the setup from your patches is not tenable. > > Can we please make that a hard requirement, SEAMCALL must not use > > registers outside this? We can hardly program to random future > > extentions; we need hard ABI guarantees here. > > > I believe all other GPRs are just saved/restored in SEAMCALL/SEAMRET, so in > practice all other GPRs not used as input/output should not be clobbered. But I > will confirm with TDX module guys. And even it's true in practice it's better > to document it. > > But I think we also want to ask them to stop adding more registers as > input/output. > > I'll talk to TDX module team on this. Please, because 12,14 are callee-saved, which means we need to go add push/pop to preserve them :-( Then you end up with something like this... /* * TDX_MODULE_CALL - common helper macro for both * TDCALL and SEAMCALL instructions. * * TDCALL - used by TDX guests to make requests to the * TDX module and hypercalls to the VMM. * SEAMCALL - used by TDX hosts to make requests to the * TDX module. * *------------------------------------------------------------------------- * TDCALL/SEAMCALL ABI: *------------------------------------------------------------------------- * Input Registers: * * RAX - TDCALL Leaf number. * RCX,RDX,R8-R11 - TDCALL Leaf specific input registers. * * Output Registers: * * RAX - TDCALL instruction error code. * RCX,RDX,R8-R11 - TDCALL Leaf specific output registers. * R12-R14 - extra output registers * *------------------------------------------------------------------------- * * __tdx_module_call() function ABI: * * @fn (RDI) - TDCALL Leaf ID, moved to RAX * @regs (RSI) - struct tdx_regs pointer * * Return status of TDCALL via RAX. */ .macro TDX_MODULE_CALL host:req ret:req extra:0 FRAME_BEGIN movq %rdi, %rax movq $TDX_SEAMCALL_VMFAILINVALID, %rdi movq TDX_MODULE_rcx(%rsi), %rcx movq TDX_MODULE_rdx(%rsi), %rdx movq TDX_MODULE_r8(%rsi), %r8 movq TDX_MODULE_r9(%rsi), %r9 movq TDX_MODULE_r10(%rsi), %r10 movq TDX_MODULE_r11(%rsi), %r11 .if \extra pushq r12 pushq r13 pushq r14 // movq TDX_MODULE_r12(%rsi), %r12 // movq TDX_MODULE_r13(%rsi), %r13 // movq TDX_MODULE_r14(%rsi), %r14 .endif .if \host 1: seamcall /* * SEAMCALL instruction is essentially a VMExit from VMX root * mode to SEAM VMX root mode. VMfailInvalid (CF=1) indicates * that the targeted SEAM firmware is not loaded or disabled, * or P-SEAMLDR is busy with another SEAMCALL. %rax is not * changed in this case. * * Set %rax to TDX_SEAMCALL_VMFAILINVALID for VMfailInvalid. * This value will never be used as actual SEAMCALL error code as * it is from the Reserved status code class. */ cmovc %rdi, %rax 2: .else tdcall .endif .if \ret movq %rcx, TDX_MODULE_rcx(%rsi) movq %rdx, TDX_MODULE_rdx(%rsi) movq %r8, TDX_MODULE_r8(%rsi) movq %r9, TDX_MODULE_r9(%rsi) movq %r10, TDX_MODULE_r10(%rsi) movq %r11, TDX_MODULE_r11(%rsi) .endif .if \extra movq %r12, TDX_MODULE_r12(%rsi) movq %r13, TDX_MODULE_r13(%rsi) movq %r14, TDX_MODULE_r14(%rsi) popq %r14 popq %r13 popq %r12 .endif FRAME_END RET .if \host 3: mov $TDX_SW_ERROR, %rdi or %rdi, %rax jmp 2b _ASM_EXTABLE_FAULT(1b, 3b) .endif .endm
