On Mon, Jul 03, 2023 at 12:15:13PM +0000, Huang, Kai wrote: > > > > > So I think the below deals with everything and unifies __tdx_hypercall() > > and __tdx_module_call(), since both sides needs to deal with exactly the > > same trainwreck. > > Hi Peter, > > Just want to make sure I understand you correctly: > > You want to make __tdx_module_call() look like __tdx_hypercall(), but not to > unify them into one assembly (at least for now), right? Well, given the horrendous trainwreck this is all turning into, I through it prudent to have it all in a single place. The moment you go play games with callee-saved registers you're really close to what hypercall does so then they might as well be the same. > I am confused you mentioned VP.VMCALL below, which is handled by > __tdx_hypercall(). But why? It really isn't *that* special if you consider the other calls that are using callee-saved regs, yes it has the rdi/rsi extra, but meh, it really just is tdcall-0. > > *------------------------------------------------------------------------- > > * TDCALL/SEAMCALL ABI: > > *------------------------------------------------------------------------- > > * Input Registers: > > * > > * RAX - Leaf number. > > * RCX,RDX,R8-R11 - Leaf specific input registers. > > * RDI,RSI,RBX,R11-R15 - VP.VMCALL VP.ENTER > > * > > * Output Registers: > > * > > * RAX - instruction error code. > > * RCX,RDX,R8-R11 - Leaf specific output registers. > > * RDI,RSI,RBX,R12-R15 - VP.VMCALL VP.ENTER > > As mentioned above, VP.VMCALL is handled by __tdx_hypercall(). Also, VP.ENTER > will be handled by KVM's own assembly. They both are not handled in this > TDX_MODULE_CALL assembly. I don't think they should be special, they're really just yet another leaf call. Yes, they have a shit calling convention, and yes VP.ENTER is terminally broken for unconditionally clobbering BP :-( That really *must* be fixed. > > .Lcall: > > .if \host > > 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. > > */ > > jc .Lseamfail > > > > .if \saved && \ret > > /* > > * VP.ENTER clears RSI on output, use it to restore state. > > */ > > popq %rsi > > xor %edi,%edi > > movq %rdi, TDX_MODULE_rdi(%rsi) > > movq %rdi, TDX_MODULE_rsi(%rsi) > > .endif > > .else > > tdcall > > > > /* > > * RAX!=0 indicates a failure, assume no return values. > > */ > > testq %rax, %rax > > jne .Lerror > > For some SEAMCALL/TDCALL the output registers may contain additional error > information. We need to jump to a location where whether returning those > additional regs to 'struct tdx_module_args' depends on \ret. I suppose we can move this into the below conditional :-( The [DS]I register stuff requires a scratch reg to recover, AX being zero provides that. > > .if \saved && \ret > > /* > > * Since RAX==0, it can be used as a scratch register to restore state. > > * > > * [ assumes \saved implies \ret ] > > */ > > popq %rax > > movq %rdi, TDX_MODULE_rdi(%rax) > > movq %rsi, TDX_MODULE_rsi(%rax) > > movq %rax, %rsi > > xor %eax, %eax; > > .endif > > .endif // \host So the reason I want this, is that I feel very strongly that if you cannot write a single coherent wrapper for all this, its calling convention is fundamentally *too* complex / broken.
