| Message ID | 41b7e5503a3e6057dc168b3c5a9693651c501d22.1689151537.git.kai.huang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Unify TDCALL/SEAMCALL and TDVMCALL assembly | expand |
On Wed, Jul 12, 2023 at 08:55:23PM +1200, Kai Huang <kai.huang@intel.com> wrote: > Intel Trust Domain Extensions (TDX) protects guest VMs from malicious > host and certain physical attacks. A CPU-attested software module > called 'the TDX module' runs inside a new isolated memory range as a > trusted hypervisor to manage and run protected VMs. > > TDX introduces a new CPU mode: Secure Arbitration Mode (SEAM). This > mode runs only the TDX module itself or other code to load the TDX > module. > > The host kernel communicates with SEAM software via a new SEAMCALL > instruction. This is conceptually similar to a guest->host hypercall, > except it is made from the host to SEAM software instead. The TDX > module establishes a new SEAMCALL ABI which allows the host to > initialize the module and to manage VMs. > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > the basic TDX support: __seamcall(), __seamcall_ret() and > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For KVM TDH.VP.ENTER case, those arguments are already in unsigned long kvm_vcpu_arch::regs[]. It's silly to move those values twice. From kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the change with TDX KVM patch series. Thanks, > To start to support TDX, create a new arch/x86/virt/vmx/tdx/tdx.c for > TDX host kernel support. Add a new Kconfig option CONFIG_INTEL_TDX_HOST > to opt-in TDX host kernel support (to distinguish with TDX guest kernel > support). So far only KVM uses TDX. Make the new config option depend > on KVM_INTEL. > > Signed-off-by: Kai Huang <kai.huang@intel.com> > --- > arch/x86/Kconfig | 12 +++++++ > arch/x86/Makefile | 2 ++ > arch/x86/include/asm/tdx.h | 7 +++++ > 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 | 54 ++++++++++++++++++++++++++++++++ > 7 files changed, 81 insertions(+) > 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 > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 53bab123a8ee..191587f75810 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -1952,6 +1952,18 @@ config X86_SGX > > If unsure, say N. > > +config INTEL_TDX_HOST > + bool "Intel Trust Domain Extensions (TDX) host support" > + depends on CPU_SUP_INTEL > + depends on X86_64 > + depends on KVM_INTEL > + help > + Intel Trust Domain Extensions (TDX) protects guest VMs from malicious > + host and certain physical attacks. This option enables necessary TDX > + support in the host kernel to run confidential VMs. > + > + If unsure, say N. > + > config EFI > bool "EFI runtime service support" > depends on ACPI > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > index b39975977c03..ec0e71d8fa30 100644 > --- a/arch/x86/Makefile > +++ b/arch/x86/Makefile > @@ -252,6 +252,8 @@ archheaders: > > libs-y += arch/x86/lib/ > > +core-y += arch/x86/virt/ > + > # drivers-y are linked after core-y > drivers-$(CONFIG_MATH_EMULATION) += arch/x86/math-emu/ > drivers-$(CONFIG_PCI) += arch/x86/pci/ > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h > index 9b0ad0176e58..a82e5249d079 100644 > --- a/arch/x86/include/asm/tdx.h > +++ b/arch/x86/include/asm/tdx.h > @@ -74,5 +74,12 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1, > return -ENODEV; > } > #endif /* CONFIG_INTEL_TDX_GUEST && CONFIG_KVM_GUEST */ > + > +#ifdef CONFIG_INTEL_TDX_HOST > +u64 __seamcall(u64 fn, struct tdx_module_args *args); > +u64 __seamcall_ret(u64 fn, struct tdx_module_args *args); > +u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args); > +#endif /* CONFIG_INTEL_TDX_HOST */ > + > #endif /* !__ASSEMBLY__ */ > #endif /* _ASM_X86_TDX_H */ > diff --git a/arch/x86/virt/Makefile b/arch/x86/virt/Makefile > new file mode 100644 > index 000000000000..1e36502cd738 > --- /dev/null > +++ b/arch/x86/virt/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-y += vmx/ > diff --git a/arch/x86/virt/vmx/Makefile b/arch/x86/virt/vmx/Makefile > new file mode 100644 > index 000000000000..feebda21d793 > --- /dev/null > +++ b/arch/x86/virt/vmx/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_INTEL_TDX_HOST) += tdx/ > diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile > new file mode 100644 > index 000000000000..46ef8f73aebb > --- /dev/null > +++ b/arch/x86/virt/vmx/tdx/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-y += seamcall.o > diff --git a/arch/x86/virt/vmx/tdx/seamcall.S b/arch/x86/virt/vmx/tdx/seamcall.S > new file mode 100644 > index 000000000000..650a40843afe > --- /dev/null > +++ b/arch/x86/virt/vmx/tdx/seamcall.S > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#include <linux/linkage.h> > +#include <asm/frame.h> > + > +#include "tdxcall.S" > + > +/* > + * __seamcall() - Host-side interface functions to SEAM software > + * (the P-SEAMLDR or the TDX module). > + * > + * __seamcall() function ABI: > + * > + * @fn (RDI) - SEAMCALL Leaf number, moved to RAX > + * @args (RSI) - struct tdx_module_args for input > + * > + * Return (via RAX) TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself > + * fails, or the completion status of the SEAMCALL leaf function. > + */ > +SYM_FUNC_START(__seamcall) > + TDX_MODULE_CALL host=1 ret=0 saved=0 > +SYM_FUNC_END(__seamcall) > + > +/* > + * __seamcall_ret() - Host-side interface functions to SEAM software > + * (the P-SEAMLDR or the TDX module). > + * > + * __seamcall_ret() function ABI: > + * > + * @fn (RDI) - SEAMCALL Leaf number, moved to RAX > + * @args (RSI) - struct tdx_module_args for input and output > + * > + * 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=0 > +SYM_FUNC_END(__seamcall_ret) > + > +/* > + * __seamcall_saved_ret() - Host-side interface functions to SEAM software > + * (the P-SEAMLDR or the TDX module) with extra > + * "callee-saved" registers as input/output. > + * > + * __seamcall_saved_ret() function ABI: > + * > + * @fn (RDI) - SEAMCALL Leaf number, moved to RAX > + * @args (RSI) - struct tdx_module_args for input and output > + * > + * Return (via RAX) TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself > + * fails, or the completion status of the SEAMCALL leaf function. > + */ > +SYM_FUNC_START(__seamcall_saved_ret) > + TDX_MODULE_CALL host=1 ret=1 saved=0 > +SYM_FUNC_END(__seamcall_saved_ret) > -- > 2.41.0 >
On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > the basic TDX support: __seamcall(), __seamcall_ret() and > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > change with TDX KVM patch series. The assembly code assumes the second argument is a pointer to 'struct tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to match 'struct tdx_module_args''s layout and manually convert part of "regs" to the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. This was one concern that I mentioned VP.ENTER can be implemented by KVM in its own assembly in the TDX host v12 discussion. I kinda agree we should leverage KVM's existing kvm_vcpu_arch::regs[NR_CPU_REGS] infrastructure to minimize the code change to the KVM's common infrastructure. If so, I guess we have to carry this memory copy burden between two structures. Btw, I do find KVM's VP.ENTER code is a little bit redundant to the common SEAMCALL assembly, which is a good reason for KVM to use __seamcall() variants for TDH.VP.ENTER. So it's a tradeoff I think. On the other hand, given CoCo VMs normally don't expose all GPRs to VMM, it's also debatable whether we should invent another infrastructure to the KVM code to handle register access of CoCo VMs too, e.g., we can catch bugs easily when KVM tries to access the registers that it shouldn't access. It's better KVM maintainer can provide some input here. :)
On Thu, Jul 13, 2023 at 03:46:52AM +0000, Huang, Kai wrote: > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > > change with TDX KVM patch series. > > The assembly code assumes the second argument is a pointer to 'struct > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. I suspect the kvm_vcpu_arch::regs layout is given by hardware; so the only option would be to make tdx_module_args match that. It's a slightly unfortunate layout, but meh. Then you can simply do: __seamcall_saved_ret(leaf, (struct tdx_module_args *)vcpu->arch->regs);
On Thu, 2023-07-13 at 09:42 +0200, Peter Zijlstra wrote: > On Thu, Jul 13, 2023 at 03:46:52AM +0000, Huang, Kai wrote: > > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > > > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > > > change with TDX KVM patch series. > > > > The assembly code assumes the second argument is a pointer to 'struct > > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > I suspect the kvm_vcpu_arch::regs layout is given by hardware; so the > only option would be to make tdx_module_args match that. It's a slightly > unfortunate layout, but meh. > > Then you can simply do: > > __seamcall_saved_ret(leaf, (struct tdx_module_args *)vcpu->arch->regs); > > I don't think the layout matches hardware, especially I think there's no "hardware layout" for GPRs that are concerned here. They are just for KVM itself to save guest's registers when the guest exits to KVM, so that KVM can restore them when returning back to the guest.
On Thu, Jul 13, 2023 at 08:18:09AM +0000, Huang, Kai wrote: > On Thu, 2023-07-13 at 09:42 +0200, Peter Zijlstra wrote: > > On Thu, Jul 13, 2023 at 03:46:52AM +0000, Huang, Kai wrote: > > > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > > > > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > > > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > > > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > > > > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > > > > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > > > > change with TDX KVM patch series. > > > > > > The assembly code assumes the second argument is a pointer to 'struct > > > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > > > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > > > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > > > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > > > I suspect the kvm_vcpu_arch::regs layout is given by hardware; so the > > only option would be to make tdx_module_args match that. It's a slightly > > unfortunate layout, but meh. > > > > Then you can simply do: > > > > __seamcall_saved_ret(leaf, (struct tdx_module_args *)vcpu->arch->regs); > > > > > > I don't think the layout matches hardware, especially I think there's no > "hardware layout" for GPRs that are concerned here. They are just for KVM > itself to save guest's registers when the guest exits to KVM, so that KVM can > restore them when returning back to the guest. Either way around it should be possible to make them match I suppose. Ideally we get the callee-clobbered regs first, but if not, I don't think that's too big of a problem.
On Thu, 2023-07-13 at 11:03 +0200, Peter Zijlstra wrote: > On Thu, Jul 13, 2023 at 08:18:09AM +0000, Huang, Kai wrote: > > On Thu, 2023-07-13 at 09:42 +0200, Peter Zijlstra wrote: > > > On Thu, Jul 13, 2023 at 03:46:52AM +0000, Huang, Kai wrote: > > > > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > > > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > > > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > > > > > > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > > > > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > > > > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > > > > > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > > > > > > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > > > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > > > > > change with TDX KVM patch series. > > > > > > > > The assembly code assumes the second argument is a pointer to 'struct > > > > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > > > > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > > > > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > > > > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > > > > > I suspect the kvm_vcpu_arch::regs layout is given by hardware; so the > > > only option would be to make tdx_module_args match that. It's a slightly > > > unfortunate layout, but meh. > > > > > > Then you can simply do: > > > > > > __seamcall_saved_ret(leaf, (struct tdx_module_args *)vcpu->arch->regs); > > > > > > > > > > I don't think the layout matches hardware, especially I think there's no > > "hardware layout" for GPRs that are concerned here. They are just for KVM > > itself to save guest's registers when the guest exits to KVM, so that KVM can > > restore them when returning back to the guest. > > Either way around it should be possible to make them match I suppose. > Ideally we get the callee-clobbered regs first, but if not, I don't > think that's too big of a problem. > Yeah we can. I'll leave this to KVM TDX patches.
On Thu, Jul 13, 2023, Kai Huang wrote: > On Thu, 2023-07-13 at 09:42 +0200, Peter Zijlstra wrote: > > On Thu, Jul 13, 2023 at 03:46:52AM +0000, Huang, Kai wrote: > > > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > > > TDCALL infrastructure.� Wire up basic functions to make SEAMCALLs for > > > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > > > > > Hi.� __seamcall_saved_ret() uses struct tdx_module_arg as input and output.� For > > > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > > > kvm_vcpu_arch::regs[].� It's silly to move those values twice.� From > > > > kvm_vcpu_arch::regs to tdx_module_args.� From tdx_module_args to real registers. > > > > > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]?� Maybe I can make the > > > > change with TDX KVM patch series. > > > > > > The assembly code assumes the second argument is a pointer to 'struct > > > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > > > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > > > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > > > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > > > I suspect the kvm_vcpu_arch::regs layout is given by hardware; so the > > only option would be to make tdx_module_args match that. It's a slightly > > unfortunate layout, but meh. > > > > Then you can simply do: > > > > __seamcall_saved_ret(leaf, (struct tdx_module_args *)vcpu->arch->regs); > > > > > > I don't think the layout matches hardware, especially I think there's no > "hardware layout" for GPRs that are concerned here. They are just for KVM > itself to save guest's registers when the guest exits to KVM, so that KVM can > restore them when returning back to the guest. kvm_vcpu_arch::regs does follow the hardware-defined indices, and that's required for myriad emulation flows, e.g. saving GPRs into SMRAM, anywhere KVM gets a GPR index from an instruction opcode or vmcs.VMX_INSTRUCTION_INFO, etc.
On Thu, Jul 13, 2023 at 03:46:52AM +0000, "Huang, Kai" <kai.huang@intel.com> wrote: > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > > change with TDX KVM patch series. > > The assembly code assumes the second argument is a pointer to 'struct > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > This was one concern that I mentioned VP.ENTER can be implemented by KVM in its > own assembly in the TDX host v12 discussion. I kinda agree we should leverage > KVM's existing kvm_vcpu_arch::regs[NR_CPU_REGS] infrastructure to minimize the > code change to the KVM's common infrastructure. If so, I guess we have to carry > this memory copy burden between two structures. > > Btw, I do find KVM's VP.ENTER code is a little bit redundant to the common > SEAMCALL assembly, which is a good reason for KVM to use __seamcall() variants > for TDH.VP.ENTER. > > So it's a tradeoff I think. > > On the other hand, given CoCo VMs normally don't expose all GPRs to VMM, it's > also debatable whether we should invent another infrastructure to the KVM code > to handle register access of CoCo VMs too, e.g., we can catch bugs easily when > KVM tries to access the registers that it shouldn't access. Yes, we'd like to save/restore GPRs only for TDVMCALL. Otherwise skip save/restore.
On Thu, 2023-07-13 at 07:51 -0700, Sean Christopherson wrote: > On Thu, Jul 13, 2023, Kai Huang wrote: > > On Thu, 2023-07-13 at 09:42 +0200, Peter Zijlstra wrote: > > > On Thu, Jul 13, 2023 at 03:46:52AM +0000, Huang, Kai wrote: > > > > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > > > > TDCALL infrastructure.� Wire up basic functions to make SEAMCALLs for > > > > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > > > > > > > Hi.� __seamcall_saved_ret() uses struct tdx_module_arg as input and output.� For > > > > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > > > > kvm_vcpu_arch::regs[].� It's silly to move those values twice.� From > > > > > kvm_vcpu_arch::regs to tdx_module_args.� From tdx_module_args to real registers. > > > > > > > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > > > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]?� Maybe I can make the > > > > > change with TDX KVM patch series. > > > > > > > > The assembly code assumes the second argument is a pointer to 'struct > > > > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > > > > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > > > > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > > > > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > > > > > I suspect the kvm_vcpu_arch::regs layout is given by hardware; so the > > > only option would be to make tdx_module_args match that. It's a slightly > > > unfortunate layout, but meh. > > > > > > Then you can simply do: > > > > > > __seamcall_saved_ret(leaf, (struct tdx_module_args *)vcpu->arch->regs); > > > > > > > > > > I don't think the layout matches hardware, especially I think there's no > > "hardware layout" for GPRs that are concerned here. They are just for KVM > > itself to save guest's registers when the guest exits to KVM, so that KVM can > > restore them when returning back to the guest. > > kvm_vcpu_arch::regs does follow the hardware-defined indices, and that's required > for myriad emulation flows, e.g. saving GPRs into SMRAM, anywhere KVM gets a GPR > index from an instruction opcode or vmcs.VMX_INSTRUCTION_INFO, etc. Yes. Sorry I missed this. I forgot x86 uses "index register" and does have a "hardware layout". IndexReg: 0 = RAX 1 = RCX 2 = RDX 3 = RBX 4 = RSP 5 = RBP 6 = RSI 7 = RDI 8–15 represent R8–R15, respectively...
On Thu, Jul 13, 2023 at 11:44:34AM -0700, Isaku Yamahata wrote: > On Thu, Jul 13, 2023 at 03:46:52AM +0000, > "Huang, Kai" <kai.huang@intel.com> wrote: > > > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > > > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > > > change with TDX KVM patch series. > > > > The assembly code assumes the second argument is a pointer to 'struct > > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > > > This was one concern that I mentioned VP.ENTER can be implemented by KVM in its > > own assembly in the TDX host v12 discussion. I kinda agree we should leverage > > KVM's existing kvm_vcpu_arch::regs[NR_CPU_REGS] infrastructure to minimize the > > code change to the KVM's common infrastructure. If so, I guess we have to carry > > this memory copy burden between two structures. > > > > Btw, I do find KVM's VP.ENTER code is a little bit redundant to the common > > SEAMCALL assembly, which is a good reason for KVM to use __seamcall() variants > > for TDH.VP.ENTER. > > > > So it's a tradeoff I think. > > > > On the other hand, given CoCo VMs normally don't expose all GPRs to VMM, it's > > also debatable whether we should invent another infrastructure to the KVM code > > to handle register access of CoCo VMs too, e.g., we can catch bugs easily when > > KVM tries to access the registers that it shouldn't access. > > Yes, we'd like to save/restore GPRs only for TDVMCALL. Otherwise skip > save/restore. And another case to save/restore GPRs: supports DEBUG TD, which is type of TD guest allows VMM to change its register context, for debugging purpose. > > -- > Isaku Yamahata <isaku.yamahata@gmail.com>
On Tue, Aug 08, 2023 at 05:16:06PM +0800, Yuan Yao <yuan.yao@linux.intel.com> wrote: > On Thu, Jul 13, 2023 at 11:44:34AM -0700, Isaku Yamahata wrote: > > On Thu, Jul 13, 2023 at 03:46:52AM +0000, > > "Huang, Kai" <kai.huang@intel.com> wrote: > > > > > On Wed, 2023-07-12 at 15:15 -0700, Isaku Yamahata wrote: > > > > > The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much > > > > > TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for > > > > > the basic TDX support: __seamcall(), __seamcall_ret() and > > > > > __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. > > > > > > > > Hi. __seamcall_saved_ret() uses struct tdx_module_arg as input and output. For > > > > KVM TDH.VP.ENTER case, those arguments are already in unsigned long > > > > kvm_vcpu_arch::regs[]. It's silly to move those values twice. From > > > > kvm_vcpu_arch::regs to tdx_module_args. From tdx_module_args to real registers. > > > > > > > > If TDH.VP.ENTER is the only user of __seamcall_saved_ret(), can we make it to > > > > take unsigned long kvm_vcpu_argh::regs[NR_VCPU_REGS]? Maybe I can make the > > > > change with TDX KVM patch series. > > > > > > The assembly code assumes the second argument is a pointer to 'struct > > > tdx_module_args'. I don't know how can we change __seamcall_saved_ret() to > > > achieve what you said. We might change the kvm_vcpu_argh::regs[NR_VCPU_REGS] to > > > match 'struct tdx_module_args''s layout and manually convert part of "regs" to > > > the structure and pass to __seamcall_saved_ret(), but it's too hacky I suppose. > > > > > > This was one concern that I mentioned VP.ENTER can be implemented by KVM in its > > > own assembly in the TDX host v12 discussion. I kinda agree we should leverage > > > KVM's existing kvm_vcpu_arch::regs[NR_CPU_REGS] infrastructure to minimize the > > > code change to the KVM's common infrastructure. If so, I guess we have to carry > > > this memory copy burden between two structures. > > > > > > Btw, I do find KVM's VP.ENTER code is a little bit redundant to the common > > > SEAMCALL assembly, which is a good reason for KVM to use __seamcall() variants > > > for TDH.VP.ENTER. > > > > > > So it's a tradeoff I think. > > > > > > On the other hand, given CoCo VMs normally don't expose all GPRs to VMM, it's > > > also debatable whether we should invent another infrastructure to the KVM code > > > to handle register access of CoCo VMs too, e.g., we can catch bugs easily when > > > KVM tries to access the registers that it shouldn't access. > > > > Yes, we'd like to save/restore GPRs only for TDVMCALL. Otherwise skip > > save/restore. > > And another case to save/restore GPRs: supports DEBUG TD, > which is type of TD guest allows VMM to change its register > context, for debugging purpose. For Debug TD case, we can use TDH.VP.RD(general purpose register) specifically. We don't need to optimize for Debug TD case.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 53bab123a8ee..191587f75810 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1952,6 +1952,18 @@ config X86_SGX If unsure, say N. +config INTEL_TDX_HOST + bool "Intel Trust Domain Extensions (TDX) host support" + depends on CPU_SUP_INTEL + depends on X86_64 + depends on KVM_INTEL + help + Intel Trust Domain Extensions (TDX) protects guest VMs from malicious + host and certain physical attacks. This option enables necessary TDX + support in the host kernel to run confidential VMs. + + If unsure, say N. + config EFI bool "EFI runtime service support" depends on ACPI diff --git a/arch/x86/Makefile b/arch/x86/Makefile index b39975977c03..ec0e71d8fa30 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -252,6 +252,8 @@ archheaders: libs-y += arch/x86/lib/ +core-y += arch/x86/virt/ + # drivers-y are linked after core-y drivers-$(CONFIG_MATH_EMULATION) += arch/x86/math-emu/ drivers-$(CONFIG_PCI) += arch/x86/pci/ diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h index 9b0ad0176e58..a82e5249d079 100644 --- a/arch/x86/include/asm/tdx.h +++ b/arch/x86/include/asm/tdx.h @@ -74,5 +74,12 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1, return -ENODEV; } #endif /* CONFIG_INTEL_TDX_GUEST && CONFIG_KVM_GUEST */ + +#ifdef CONFIG_INTEL_TDX_HOST +u64 __seamcall(u64 fn, struct tdx_module_args *args); +u64 __seamcall_ret(u64 fn, struct tdx_module_args *args); +u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args); +#endif /* CONFIG_INTEL_TDX_HOST */ + #endif /* !__ASSEMBLY__ */ #endif /* _ASM_X86_TDX_H */ diff --git a/arch/x86/virt/Makefile b/arch/x86/virt/Makefile new file mode 100644 index 000000000000..1e36502cd738 --- /dev/null +++ b/arch/x86/virt/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-y += vmx/ diff --git a/arch/x86/virt/vmx/Makefile b/arch/x86/virt/vmx/Makefile new file mode 100644 index 000000000000..feebda21d793 --- /dev/null +++ b/arch/x86/virt/vmx/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-$(CONFIG_INTEL_TDX_HOST) += tdx/ diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile new file mode 100644 index 000000000000..46ef8f73aebb --- /dev/null +++ b/arch/x86/virt/vmx/tdx/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-y += seamcall.o diff --git a/arch/x86/virt/vmx/tdx/seamcall.S b/arch/x86/virt/vmx/tdx/seamcall.S new file mode 100644 index 000000000000..650a40843afe --- /dev/null +++ b/arch/x86/virt/vmx/tdx/seamcall.S @@ -0,0 +1,54 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <linux/linkage.h> +#include <asm/frame.h> + +#include "tdxcall.S" + +/* + * __seamcall() - Host-side interface functions to SEAM software + * (the P-SEAMLDR or the TDX module). + * + * __seamcall() function ABI: + * + * @fn (RDI) - SEAMCALL Leaf number, moved to RAX + * @args (RSI) - struct tdx_module_args for input + * + * Return (via RAX) TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself + * fails, or the completion status of the SEAMCALL leaf function. + */ +SYM_FUNC_START(__seamcall) + TDX_MODULE_CALL host=1 ret=0 saved=0 +SYM_FUNC_END(__seamcall) + +/* + * __seamcall_ret() - Host-side interface functions to SEAM software + * (the P-SEAMLDR or the TDX module). + * + * __seamcall_ret() function ABI: + * + * @fn (RDI) - SEAMCALL Leaf number, moved to RAX + * @args (RSI) - struct tdx_module_args for input and output + * + * 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=0 +SYM_FUNC_END(__seamcall_ret) + +/* + * __seamcall_saved_ret() - Host-side interface functions to SEAM software + * (the P-SEAMLDR or the TDX module) with extra + * "callee-saved" registers as input/output. + * + * __seamcall_saved_ret() function ABI: + * + * @fn (RDI) - SEAMCALL Leaf number, moved to RAX + * @args (RSI) - struct tdx_module_args for input and output + * + * Return (via RAX) TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself + * fails, or the completion status of the SEAMCALL leaf function. + */ +SYM_FUNC_START(__seamcall_saved_ret) + TDX_MODULE_CALL host=1 ret=1 saved=0 +SYM_FUNC_END(__seamcall_saved_ret)
Intel Trust Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. A CPU-attested software module called 'the TDX module' runs inside a new isolated memory range as a trusted hypervisor to manage and run protected VMs. TDX introduces a new CPU mode: Secure Arbitration Mode (SEAM). This mode runs only the TDX module itself or other code to load the TDX module. The host kernel communicates with SEAM software via a new SEAMCALL instruction. This is conceptually similar to a guest->host hypercall, except it is made from the host to SEAM software instead. The TDX module establishes a new SEAMCALL ABI which allows the host to initialize the module and to manage VMs. The SEAMCALL ABI is very similar to the TDCALL ABI and leverages much TDCALL infrastructure. Wire up basic functions to make SEAMCALLs for the basic TDX support: __seamcall(), __seamcall_ret() and __seamcall_saved_ret() which is for TDH.VP.ENTER leaf function. To start to support TDX, create a new arch/x86/virt/vmx/tdx/tdx.c for TDX host kernel support. Add a new Kconfig option CONFIG_INTEL_TDX_HOST to opt-in TDX host kernel support (to distinguish with TDX guest kernel support). So far only KVM uses TDX. Make the new config option depend on KVM_INTEL. Signed-off-by: Kai Huang <kai.huang@intel.com> --- arch/x86/Kconfig | 12 +++++++ arch/x86/Makefile | 2 ++ arch/x86/include/asm/tdx.h | 7 +++++ 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 | 54 ++++++++++++++++++++++++++++++++ 7 files changed, 81 insertions(+) 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