Hi Peter, Kirill,

This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
Now all of them use one singe TDX_MODULE_CALL asm macro.  More
information please see cover letter of v2 (see link below).

Tested by booting TDX guest, initializing TDX module, and running TDX
guest successfully, all with this series applied.

------- Histroy --------

v3 -> v4:

 - Addressed Kirill's minor comments
 - Collected Reviewed-by tags from Kirill/Sathy.
 - Collected Isaku's Tested-by tag.
 - Removed unneeded header file inclusion in tdcall.S in the relevant
   two patches.
 - Rebased to latest tip/x86/tdx (v6.5-rc5).

v3: https://lore.kernel.org/lkml/20230806114131.2ilofgmxhdaa2u6b@box.shutemov.name/T/

v2 -> v3:

 - New patch (patch 12) to adjust 'struct tdx_module_args' layout to
   match KVM's "vcpu::regs[]" for VP.ENTER. (Peter)
 - Added __seamcall_saved_ret() wrapper to support VP.ENTER (merged to
   patch 10).
 - Fixed a 'noinstr' check build regression found by LKP (patch 7).
 - Rebased to latest Linus's tree (6.5-rc3 + 2 commits).

v2: https://lore.kernel.org/lkml/a23ce8fd289141cea3a1b4f3dace221dca847238.camel@intel.com/T/

v1 -> v2:
 - Rebased to 6.5-rc2.
 - Fixed comments from Peter and others.
 - Split patch "x86/tdx: Unify TDX_HYPERCALL and TDX_MODULE_CALL assembly"
   into three smaller patches for better review.
 - A new patch to skip saving output registers when SEAMCALL fails due to
   VMFailInvalid.  
 - Removed patch "x86/tdx: Use cmovc to save a label in TDX_MODULE_CALL asm"
 - Merged patch "x86/tdx: Move FRAME_BEGIN/END to TDX_MODULE_CALL asm macro"
   to the new patch mentioned above.

v1: https://lore.kernel.org/lkml/b95c4169-88c8-219e-87b7-6c4e058c246a@suse.com/T/



Kai Huang (12):
  x86/tdx: Zero out the missing RSI in TDX_HYPERCALL macro
  x86/tdx: Skip saving output regs when SEAMCALL fails with
    VMFailInvalid
  x86/tdx: Make macros of TDCALLs consistent with the spec
  x86/tdx: Rename __tdx_module_call() to __tdcall()
  x86/tdx: Pass TDCALL/SEAMCALL input/output registers via a structure
  x86/tdx: Extend TDX_MODULE_CALL to support more TDCALL/SEAMCALL leafs
  x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
  x86/tdx: Reimplement __tdx_hypercall() using TDX_MODULE_CALL asm
  x86/tdx: Remove 'struct tdx_hypercall_args'
  x86/virt/tdx: Wire up basic SEAMCALL functions
  x86/virt/tdx: Make TDX_MODULE_CALL handle SEAMCALL #UD and #GP
  x86/virt/tdx: Adjust 'struct tdx_module_args' to use x86 "register
    index" layout

 arch/x86/Kconfig                  |  12 ++
 arch/x86/Makefile                 |   2 +
 arch/x86/boot/compressed/tdx.c    |   6 +-
 arch/x86/coco/tdx/tdcall.S        | 234 ++++--------------------------
 arch/x86/coco/tdx/tdx-shared.c    |  28 +++-
 arch/x86/coco/tdx/tdx.c           |  71 +++++----
 arch/x86/include/asm/shared/tdx.h |  92 +++++++-----
 arch/x86/include/asm/tdx.h        |  11 ++
 arch/x86/kernel/asm-offsets.c     |  33 ++---
 arch/x86/virt/Makefile            |   2 +
 arch/x86/virt/vmx/Makefile        |   2 +
 arch/x86/virt/vmx/tdx/Makefile    |   2 +
 arch/x86/virt/vmx/tdx/seamcall.S  |  61 ++++++++
 arch/x86/virt/vmx/tdx/tdxcall.S   | 230 ++++++++++++++++++++++-------
 14 files changed, 435 insertions(+), 351 deletions(-)
 create mode 100644 arch/x86/virt/Makefile
 create mode 100644 arch/x86/virt/vmx/Makefile
 create mode 100644 arch/x86/virt/vmx/tdx/Makefile
 create mode 100644 arch/x86/virt/vmx/tdx/seamcall.S


base-commit: 52a93d39b17dc7eb98b6aa3edb93943248e03b2f
-- 
2.41.0

On 8/15/23 13:01, Kai Huang wrote:
> Hi Peter, Kirill,
> 
> This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> information please see cover letter of v2 (see link below).
> 
> Tested by booting TDX guest, initializing TDX module, and running TDX
> guest successfully, all with this series applied.

Not sure it if's new in this version but I'm seeing

     WARNING: modpost: EXPORT symbol "__seamcall_ret" [vmlinux] version ...
     Is "__seamcall_ret" prototyped in <asm/asm-prototypes.h>?
     WARNING: modpost: EXPORT symbol "__seamcall" [vmlinux] version ...
     Is "__seamcall" prototyped in <asm/asm-prototypes.h>?
     WARNING: modpost: EXPORT symbol "__seamcall_saved_ret" [vmlinux] version ...
     Is "__seamcall_saved_ret" prototyped in <asm/asm-prototypes.h>?

which is fixed by

diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h
index 8f80de627c60..eff5252e9c86 100644
--- a/arch/x86/include/asm/asm-prototypes.h
+++ b/arch/x86/include/asm/asm-prototypes.h
@@ -6,6 +6,7 @@
  #include <asm/page.h>
  #include <asm/checksum.h>
  #include <asm/mce.h>
+#include <asm/tdx.h>
  
  #include <asm-generic/asm-prototypes.h>
  

Paolo

> ------- Histroy --------
> 
> v3 -> v4:
> 
>   - Addressed Kirill's minor comments
>   - Collected Reviewed-by tags from Kirill/Sathy.
>   - Collected Isaku's Tested-by tag.
>   - Removed unneeded header file inclusion in tdcall.S in the relevant
>     two patches.
>   - Rebased to latest tip/x86/tdx (v6.5-rc5).
> 
> v3: https://lore.kernel.org/lkml/20230806114131.2ilofgmxhdaa2u6b@box.shutemov.name/T/
> 
> v2 -> v3:
> 
>   - New patch (patch 12) to adjust 'struct tdx_module_args' layout to
>     match KVM's "vcpu::regs[]" for VP.ENTER. (Peter)
>   - Added __seamcall_saved_ret() wrapper to support VP.ENTER (merged to
>     patch 10).
>   - Fixed a 'noinstr' check build regression found by LKP (patch 7).
>   - Rebased to latest Linus's tree (6.5-rc3 + 2 commits).
> 
> v2: https://lore.kernel.org/lkml/a23ce8fd289141cea3a1b4f3dace221dca847238.camel@intel.com/T/
> 
> v1 -> v2:
>   - Rebased to 6.5-rc2.
>   - Fixed comments from Peter and others.
>   - Split patch "x86/tdx: Unify TDX_HYPERCALL and TDX_MODULE_CALL assembly"
>     into three smaller patches for better review.
>   - A new patch to skip saving output registers when SEAMCALL fails due to
>     VMFailInvalid.
>   - Removed patch "x86/tdx: Use cmovc to save a label in TDX_MODULE_CALL asm"
>   - Merged patch "x86/tdx: Move FRAME_BEGIN/END to TDX_MODULE_CALL asm macro"
>     to the new patch mentioned above.
> 
> v1: https://lore.kernel.org/lkml/b95c4169-88c8-219e-87b7-6c4e058c246a@suse.com/T/
> 
> 
> 
> Kai Huang (12):
>    x86/tdx: Zero out the missing RSI in TDX_HYPERCALL macro
>    x86/tdx: Skip saving output regs when SEAMCALL fails with
>      VMFailInvalid
>    x86/tdx: Make macros of TDCALLs consistent with the spec
>    x86/tdx: Rename __tdx_module_call() to __tdcall()
>    x86/tdx: Pass TDCALL/SEAMCALL input/output registers via a structure
>    x86/tdx: Extend TDX_MODULE_CALL to support more TDCALL/SEAMCALL leafs
>    x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL
>    x86/tdx: Reimplement __tdx_hypercall() using TDX_MODULE_CALL asm
>    x86/tdx: Remove 'struct tdx_hypercall_args'
>    x86/virt/tdx: Wire up basic SEAMCALL functions
>    x86/virt/tdx: Make TDX_MODULE_CALL handle SEAMCALL #UD and #GP
>    x86/virt/tdx: Adjust 'struct tdx_module_args' to use x86 "register
>      index" layout
> 
>   arch/x86/Kconfig                  |  12 ++
>   arch/x86/Makefile                 |   2 +
>   arch/x86/boot/compressed/tdx.c    |   6 +-
>   arch/x86/coco/tdx/tdcall.S        | 234 ++++--------------------------
>   arch/x86/coco/tdx/tdx-shared.c    |  28 +++-
>   arch/x86/coco/tdx/tdx.c           |  71 +++++----
>   arch/x86/include/asm/shared/tdx.h |  92 +++++++-----
>   arch/x86/include/asm/tdx.h        |  11 ++
>   arch/x86/kernel/asm-offsets.c     |  33 ++---
>   arch/x86/virt/Makefile            |   2 +
>   arch/x86/virt/vmx/Makefile        |   2 +
>   arch/x86/virt/vmx/tdx/Makefile    |   2 +
>   arch/x86/virt/vmx/tdx/seamcall.S  |  61 ++++++++
>   arch/x86/virt/vmx/tdx/tdxcall.S   | 230 ++++++++++++++++++++++-------
>   14 files changed, 435 insertions(+), 351 deletions(-)
>   create mode 100644 arch/x86/virt/Makefile
>   create mode 100644 arch/x86/virt/vmx/Makefile
>   create mode 100644 arch/x86/virt/vmx/tdx/Makefile
>   create mode 100644 arch/x86/virt/vmx/tdx/seamcall.S
> 
> 
> base-commit: 52a93d39b17dc7eb98b6aa3edb93943248e03b2f

On Mon, 2023-09-11 at 19:08 +0200, Paolo Bonzini wrote:
> On 8/15/23 13:01, Kai Huang wrote:
> > Hi Peter, Kirill,
> > 
> > This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> > Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> > information please see cover letter of v2 (see link below).
> > 
> > Tested by booting TDX guest, initializing TDX module, and running TDX
> > guest successfully, all with this series applied.
> 
> Not sure it if's new in this version but I'm seeing
> 
>      WARNING: modpost: EXPORT symbol "__seamcall_ret" [vmlinux] version ...
>      Is "__seamcall_ret" prototyped in <asm/asm-prototypes.h>?
>      WARNING: modpost: EXPORT symbol "__seamcall" [vmlinux] version ...
>      Is "__seamcall" prototyped in <asm/asm-prototypes.h>?
>      WARNING: modpost: EXPORT symbol "__seamcall_saved_ret" [vmlinux] version ...
>      Is "__seamcall_saved_ret" prototyped in <asm/asm-prototypes.h>?
> 
> which is fixed by
> 
> diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h
> index 8f80de627c60..eff5252e9c86 100644
> --- a/arch/x86/include/asm/asm-prototypes.h
> +++ b/arch/x86/include/asm/asm-prototypes.h
> @@ -6,6 +6,7 @@
>   #include <asm/page.h>
>   #include <asm/checksum.h>
>   #include <asm/mce.h>
> +#include <asm/tdx.h>
>   
>   #include <asm-generic/asm-prototypes.h>
>   

Thanks Paolo!

I never met this issue, not even from LKP.  Do you happen to know which Kconfig
option can trigger this?  Or would you mind sharing your .config?

> -----Original Message-----
> From: Huang, Kai <kai.huang@intel.com>
> Sent: Tuesday, September 12, 2023 10:30 AM
> To: kirill.shutemov@linux.intel.com; peterz@infradead.org;
> pbonzini@redhat.com; linux-kernel@vger.kernel.org
> Cc: Hansen, Dave <dave.hansen@intel.com>; Christopherson,, Sean
> <seanjc@google.com>; bp@alien8.de; x86@kernel.org; hpa@zytor.com;
> mingo@redhat.com; tglx@linutronix.de;
> sathyanarayanan.kuppuswamy@linux.intel.com; Yamahata, Isaku
> <isaku.yamahata@intel.com>; n.borisov.lkml@gmail.com
> Subject: Re: [PATCH v4 00/12] Unify TDCALL/SEAMCALL and TDVMCALL
> assembly
> 
> On Mon, 2023-09-11 at 19:08 +0200, Paolo Bonzini wrote:
> > On 8/15/23 13:01, Kai Huang wrote:
> > > Hi Peter, Kirill,
> > >
> > > This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> > > Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> > > information please see cover letter of v2 (see link below).
> > >
> > > Tested by booting TDX guest, initializing TDX module, and running
> > > TDX guest successfully, all with this series applied.
> >
> > Not sure it if's new in this version but I'm seeing
> >
> >      WARNING: modpost: EXPORT symbol "__seamcall_ret" [vmlinux] version ...
> >      Is "__seamcall_ret" prototyped in <asm/asm-prototypes.h>?
> >      WARNING: modpost: EXPORT symbol "__seamcall" [vmlinux] version ...
> >      Is "__seamcall" prototyped in <asm/asm-prototypes.h>?
> >      WARNING: modpost: EXPORT symbol "__seamcall_saved_ret" [vmlinux]
> version ...
> >      Is "__seamcall_saved_ret" prototyped in <asm/asm-prototypes.h>?
> >
> > which is fixed by
> >
> > diff --git a/arch/x86/include/asm/asm-prototypes.h
> > b/arch/x86/include/asm/asm-prototypes.h
> > index 8f80de627c60..eff5252e9c86 100644
> > --- a/arch/x86/include/asm/asm-prototypes.h
> > +++ b/arch/x86/include/asm/asm-prototypes.h
> > @@ -6,6 +6,7 @@
> >   #include <asm/page.h>
> >   #include <asm/checksum.h>
> >   #include <asm/mce.h>
> > +#include <asm/tdx.h>
> >
> >   #include <asm-generic/asm-prototypes.h>
> >
> 
> Thanks Paolo!
> 
> I never met this issue, not even from LKP.  Do you happen to know which Kconfig
> option can trigger this?  Or would you mind sharing your .config?

(replying via outlook due to some networking issue of my home network)

After digging, this is due to exporting __seamcall*() assembly functions while MODVERSIONS is enabled.  

This series itself doesn't cause this warning.

I didn't export them in this series because I would like to leave to KVM patches when we are sure we need to export them.  

E.g., I have seamcall*() wrappers to retry running out of entropy, and we may have concern exporting __seamcall*() low level functions directly and may wish to export higher level wrappers.

So I think the fix should be with the patch that truly exports those __seamcall*().  I'll talk to Isaku on this to get his idea.

Thanks!

On 9/12/23 02:57, Huang, Kai wrote:
>>> +#include <asm/tdx.h>
>>> 
>>> #include <asm-generic/asm-prototypes.h>
>>> 
>> Thanks Paolo!
>> 
>> I never met this issue, not even from LKP.  Do you happen to know
>> which Kconfig option can trigger this?  Or would you mind sharing
>> your .config?
> (replying via outlook due to some networking issue of my home
> network)
> 
> After digging, this is due to exporting __seamcall*() assembly
> functions while MODVERSIONS is enabled.
> 
> This series itself doesn't cause this warning.
> 
> I didn't export them in this series because I would like to leave to
> KVM patches when we are sure we need to export them.
> 
> E.g., I have seamcall*() wrappers to retry running out of entropy,
> and we may have concern exporting __seamcall*() low level functions
> directly and may wish to export higher level wrappers.
> 
> So I think the fix should be with the patch that truly exports those
> __seamcall*().  I'll talk to Isaku on this to get his idea.

Ah indeed it's not caused by your patches, but rather by the Isaku's KVM 
TDX support ("KVM: TDX: Add C wrapper functions for SEAMCALLs to the TDX 
module").  The fix is just to squash the #include into his patch.

Paolo

On Tue, Sep 12, 2023 at 03:37:49PM +0200,
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 9/12/23 02:57, Huang, Kai wrote:
> > > > +#include <asm/tdx.h>
> > > > 
> > > > #include <asm-generic/asm-prototypes.h>
> > > > 
> > > Thanks Paolo!
> > > 
> > > I never met this issue, not even from LKP.  Do you happen to know
> > > which Kconfig option can trigger this?  Or would you mind sharing
> > > your .config?
> > (replying via outlook due to some networking issue of my home
> > network)
> > 
> > After digging, this is due to exporting __seamcall*() assembly
> > functions while MODVERSIONS is enabled.
> > 
> > This series itself doesn't cause this warning.
> > 
> > I didn't export them in this series because I would like to leave to
> > KVM patches when we are sure we need to export them.
> > 
> > E.g., I have seamcall*() wrappers to retry running out of entropy,
> > and we may have concern exporting __seamcall*() low level functions
> > directly and may wish to export higher level wrappers.
> > 
> > So I think the fix should be with the patch that truly exports those
> > __seamcall*().  I'll talk to Isaku on this to get his idea.
> 
> Ah indeed it's not caused by your patches, but rather by the Isaku's KVM TDX
> support ("KVM: TDX: Add C wrapper functions for SEAMCALLs to the TDX
> module").  The fix is just to squash the #include into his patch.

Yes, I'll squash the fix into the patch.
-- 
Isaku Yamahata <isaku.yamahata@linux.intel.com>

On Tue, 2023-08-15 at 23:01 +1200, Kai Huang wrote:
> Hi Peter, Kirill,
> 
> This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> information please see cover letter of v2 (see link below).
> 
> Tested by booting TDX guest, initializing TDX module, and running TDX
> guest successfully, all with this series applied.
> 
> 

Hi Peter,

Sorry to ping, but could you take a look whether you are OK with this series?

Basically Kirill has provided his Reviewed-by for all the patches except the
last one (Add unused registers to 'struct tdx_module_args' to optimize VP.ENTER
for KVM), which he didn't like.  But it is pretty much a standalone optimization
patch we either can have or drop when applying, so I'll leave to you.

Also feel free to let me know if I can help you on anything to offload part of
your load? :-)

On Tue, Sep 05, 2023 at 11:01:28PM +0000,
"Huang, Kai" <kai.huang@intel.com> wrote:

> On Tue, 2023-08-15 at 23:01 +1200, Kai Huang wrote:
> > Hi Peter, Kirill,
> > 
> > This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> > Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> > information please see cover letter of v2 (see link below).
> > 
> > Tested by booting TDX guest, initializing TDX module, and running TDX
> > guest successfully, all with this series applied.
> > 
> > 
> 
> Hi Peter,
> 
> Sorry to ping, but could you take a look whether you are OK with this series?
> 
> Basically Kirill has provided his Reviewed-by for all the patches except the
> last one (Add unused registers to 'struct tdx_module_args' to optimize VP.ENTER
> for KVM), which he didn't like.  But it is pretty much a standalone optimization
> patch we either can have or drop when applying, so I'll leave to you.
> 
> Also feel free to let me know if I can help you on anything to offload part of
> your load? :-)

Because the optimization patch was dropped in tip x86/tdx tree, TDX KVM can't
use __seamcall_saved_ret() because the argument doesn't match.  There is no
user of __seamcall_saved_ret().
-- 
Isaku Yamahata <isaku.yamahata@linux.intel.com>

On Wed, 2023-09-13 at 17:22 -0700, Isaku Yamahata wrote:
> On Tue, Sep 05, 2023 at 11:01:28PM +0000,
> "Huang, Kai" <kai.huang@intel.com> wrote:
> 
> > On Tue, 2023-08-15 at 23:01 +1200, Kai Huang wrote:
> > > Hi Peter, Kirill,
> > > 
> > > This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> > > Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> > > information please see cover letter of v2 (see link below).
> > > 
> > > Tested by booting TDX guest, initializing TDX module, and running TDX
> > > guest successfully, all with this series applied.
> > > 
> > > 
> > 
> > Hi Peter,
> > 
> > Sorry to ping, but could you take a look whether you are OK with this series?
> > 
> > Basically Kirill has provided his Reviewed-by for all the patches except the
> > last one (Add unused registers to 'struct tdx_module_args' to optimize VP.ENTER
> > for KVM), which he didn't like.  But it is pretty much a standalone optimization
> > patch we either can have or drop when applying, so I'll leave to you.
> > 
> > Also feel free to let me know if I can help you on anything to offload part of
> > your load? :-)
> 
> Because the optimization patch was dropped in tip x86/tdx tree, TDX KVM can't
> use __seamcall_saved_ret() because the argument doesn't match.  There is no
> user of __seamcall_saved_ret().
> 

I agree sometimes "optimization" is better to have, but functionally we
certainly don't depend on "optimization" to work, so sure KVM TDX can use it (of
course I will leave to maintainers though).

On Tue, Sep 05, 2023 at 11:01:28PM +0000, Huang, Kai wrote:
> On Tue, 2023-08-15 at 23:01 +1200, Kai Huang wrote:
> > Hi Peter, Kirill,
> > 
> > This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> > Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> > information please see cover letter of v2 (see link below).
> > 
> > Tested by booting TDX guest, initializing TDX module, and running TDX
> > guest successfully, all with this series applied.
> > 
> > 
> 
> Hi Peter,
> 
> Sorry to ping, but could you take a look whether you are OK with this series?
> 
> Basically Kirill has provided his Reviewed-by for all the patches except the
> last one (Add unused registers to 'struct tdx_module_args' to optimize VP.ENTER
> for KVM), which he didn't like.  But it is pretty much a standalone optimization
> patch we either can have or drop when applying, so I'll leave to you.
> 
> Also feel free to let me know if I can help you on anything to offload part of
> your load? :-)

This looks good, thanks!

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

On Wed, 2023-09-06 at 13:57 +0200, Peter Zijlstra wrote:
> On Tue, Sep 05, 2023 at 11:01:28PM +0000, Huang, Kai wrote:
> > On Tue, 2023-08-15 at 23:01 +1200, Kai Huang wrote:
> > > Hi Peter, Kirill,
> > > 
> > > This series unifies the assembly code for TDCALL/SEAMCALL and TDVMCALL.
> > > Now all of them use one singe TDX_MODULE_CALL asm macro.  More
> > > information please see cover letter of v2 (see link below).
> > > 
> > > Tested by booting TDX guest, initializing TDX module, and running TDX
> > > guest successfully, all with this series applied.
> > > 
> > > 
> > 
> > Hi Peter,
> > 
> > Sorry to ping, but could you take a look whether you are OK with this series?
> > 
> > Basically Kirill has provided his Reviewed-by for all the patches except the
> > last one (Add unused registers to 'struct tdx_module_args' to optimize VP.ENTER
> > for KVM), which he didn't like.  But it is pretty much a standalone optimization
> > patch we either can have or drop when applying, so I'll leave to you.
> > 
> > Also feel free to let me know if I can help you on anything to offload part of
> > your load? :-)
> 
> This looks good, thanks!
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Thanks!