Re: [PATCH v12 20/22] x86/virt/tdx: Allow SEAMCALL to handle #UD and #GP

[Huang, Kai]

On Fri, 2023-06-30 at 12:06 +0200, Peter Zijlstra wrote:
> 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 :-(

Yes.

However those new SEAMCALLs are for TDX guest live migration support,  which is
at a year(s)-later thing from upstreaming's point of view.  My thinking is we
can defer supporting those new SEAMCALls until that phase.  Yes we need to do
some assembly change at that time, but also looks fine to me.

How does this sound?