| Message ID | 0596db2913da40660e87d5005167c623cee14765.1646422845.git.isaku.yamahata@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM TDX basic feature support | expand |
On Fri, 2022-03-04 at 11:48 -0800, isaku.yamahata@intel.com wrote: > From: Isaku Yamahata <isaku.yamahata@intel.com> > > As first step TDX VM support, return that TDX VM type supported to device > model, e.g. qemu. The callback to create guest TD is vm_init callback for > KVM_CREATE_VM. Add a place holder function and call a function to > initialize TDX module on demand because in that callback VMX is enabled by > hardware_enable callback (vmx_hardware_enable). Should we put this patch at the end of series until all changes required to run TD are introduced? This patch essentially tells userspace KVM is ready to support a TD but actually it's not ready. And this might also cause bisect issue I suppose?
On Mon, Mar 14, 2022 at 12:08:59PM +1300, Kai Huang <kai.huang@intel.com> wrote: > On Fri, 2022-03-04 at 11:48 -0800, isaku.yamahata@intel.com wrote: > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > As first step TDX VM support, return that TDX VM type supported to device > > model, e.g. qemu. The callback to create guest TD is vm_init callback for > > KVM_CREATE_VM. Add a place holder function and call a function to > > initialize TDX module on demand because in that callback VMX is enabled by > > hardware_enable callback (vmx_hardware_enable). > > Should we put this patch at the end of series until all changes required to run > TD are introduced? This patch essentially tells userspace KVM is ready to > support a TD but actually it's not ready. And this might also cause bisect > issue I suppose? The intention is that developers can exercise the new code step-by-step even if the TDX KVM isn't complete. How about introducing new config and remove it at the last of the patch series? diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index 2b1548da00eb..a3287440aa9e 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -98,6 +98,20 @@ config X86_SGX_KVM If unsure, say N. +config X86_TDX_KVM_EXPERIMENTAL + bool "EXPERIMENTAL Trust Domian Extensions (TDX) KVM support" + default n + depends on INTEL_TDX_HOST + depends on KVM_INTEL + help + Enable experimental TDX KVM support. TDX KVM needs many patches and + the patches will be merged step by step, not at once. Even if TDX KVM + support is incomplete, enable TDX KVM support so that developper can + exercise TDX KVM code. TODO: Remove this configuration once the + (first step of) TDX KVM support is complete. + + If unsure, say N. + config KVM_AMD tristate "KVM for AMD processors support" depends on KVM diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c index b16e2ed3b204..e31d6902e49c 100644 --- a/arch/x86/kvm/vmx/tdx.c +++ b/arch/x86/kvm/vmx/tdx.c @@ -170,7 +170,11 @@ int tdx_module_setup(void) bool tdx_is_vm_type_supported(unsigned long type) { +#ifdef CONFIG_X86_TDX_KVM_EXPERIMENTAL return type == KVM_X86_TDX_VM && READ_ONCE(enable_tdx); +#else + return false; +#endif } static int __init __tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
On Tue, 2022-03-15 at 14:03 -0700, Isaku Yamahata wrote: > On Mon, Mar 14, 2022 at 12:08:59PM +1300, > Kai Huang <kai.huang@intel.com> wrote: > > > On Fri, 2022-03-04 at 11:48 -0800, isaku.yamahata@intel.com wrote: > > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > > > As first step TDX VM support, return that TDX VM type supported to device > > > model, e.g. qemu. The callback to create guest TD is vm_init callback for > > > KVM_CREATE_VM. Add a place holder function and call a function to > > > initialize TDX module on demand because in that callback VMX is enabled by > > > hardware_enable callback (vmx_hardware_enable). > > > > Should we put this patch at the end of series until all changes required to run > > TD are introduced? This patch essentially tells userspace KVM is ready to > > support a TD but actually it's not ready. And this might also cause bisect > > issue I suppose? > > The intention is that developers can exercise the new code step-by-step even if > the TDX KVM isn't complete. What is the purpose/value to allow developers to exercise the new code step-by- step? Userspace cannot create TD successfully anyway until all patches are ready.
On 3/15/22 22:47, Kai Huang wrote: >> The intention is that developers can exercise the new code step-by-step even if >> the TDX KVM isn't complete. > What is the purpose/value to allow developers to exercise the new code step-by- > step? Userspace cannot create TD successfully anyway until all patches are > ready. We can move this to the end when the patch is committed, but I think there is value in showing that the series works (for partial definitions of "work") at every step of the enablement process. Paolo
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index 77da926ee505..8103d1c32cc9 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -5,6 +5,12 @@ #include "vmx.h" #include "nested.h" #include "pmu.h" +#include "tdx.h" + +static bool vt_is_vm_type_supported(unsigned long type) +{ + return type == KVM_X86_DEFAULT_VM || tdx_is_vm_type_supported(type); +} static __init int vt_hardware_setup(void) { @@ -19,6 +25,20 @@ static __init int vt_hardware_setup(void) return 0; } +static int vt_vm_init(struct kvm *kvm) +{ + int ret; + + if (is_td(kvm)) { + ret = tdx_module_setup(); + if (ret) + return ret; + return -EOPNOTSUPP; /* Not ready to create guest TD yet. */ + } + + return vmx_vm_init(kvm); +} + struct kvm_x86_ops vt_x86_ops __initdata = { .name = "kvm_intel", @@ -29,9 +49,9 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .cpu_has_accelerated_tpr = report_flexpriority, .has_emulated_msr = vmx_has_emulated_msr, - .is_vm_type_supported = vmx_is_vm_type_supported, + .is_vm_type_supported = vt_is_vm_type_supported, .vm_size = sizeof(struct kvm_vmx), - .vm_init = vmx_vm_init, + .vm_init = vt_vm_init, .vcpu_create = vmx_vcpu_create, .vcpu_free = vmx_vcpu_free, diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c index 8adc87ad1807..e8d293a3c11c 100644 --- a/arch/x86/kvm/vmx/tdx.c +++ b/arch/x86/kvm/vmx/tdx.c @@ -105,6 +105,11 @@ int tdx_module_setup(void) return ret; } +bool tdx_is_vm_type_supported(unsigned long type) +{ + return type == KVM_X86_TDX_VM && READ_ONCE(enable_tdx); +} + static int __init __tdx_hardware_setup(struct kvm_x86_ops *x86_ops) { u32 max_pa; diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 3c7b3f245fee..7838cd177f0e 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7079,11 +7079,6 @@ int vmx_vcpu_create(struct kvm_vcpu *vcpu) return err; } -bool vmx_is_vm_type_supported(unsigned long type) -{ - return type == KVM_X86_DEFAULT_VM; -} - #define L1TF_MSG_SMT "L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n" #define L1TF_MSG_L1D "L1TF CPU bug present and virtualization mitigation disabled, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for details.\n" diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index f7327bc73be0..78331dbc29f7 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -25,7 +25,6 @@ void vmx_hardware_unsetup(void); int vmx_hardware_enable(void); void vmx_hardware_disable(void); bool report_flexpriority(void); -bool vmx_is_vm_type_supported(unsigned long type); int vmx_vm_init(struct kvm *kvm); int vmx_vcpu_create(struct kvm_vcpu *vcpu); int vmx_vcpu_pre_run(struct kvm_vcpu *vcpu); @@ -130,10 +129,12 @@ void vmx_setup_mce(struct kvm_vcpu *vcpu); #ifdef CONFIG_INTEL_TDX_HOST void __init tdx_pre_kvm_init(unsigned int *vcpu_size, unsigned int *vcpu_align, unsigned int *vm_size); +bool tdx_is_vm_type_supported(unsigned long type); void __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops); #else static inline void tdx_pre_kvm_init( unsigned int *vcpu_size, unsigned int *vcpu_align, unsigned int *vm_size) {} +static inline bool tdx_is_vm_type_supported(unsigned long type) { return false; } static inline void tdx_hardware_setup(struct kvm_x86_ops *x86_ops) {} #endif