Message ID | cedda3dbe8597356374ef64de26ecef0d8cd7a62.1646422845.git.isaku.yamahata@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM TDX basic feature support | expand |
On Fri, Mar 4, 2022 at 11:50 AM <isaku.yamahata@intel.com> wrote: > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > This patch implements running TDX vcpu. Once vcpu runs on the logical > processor (LP), the TDX vcpu is associated with it. When the TDX vcpu > moves to another LP, the TDX vcpu needs to flush its status on the LP. > When destroying TDX vcpu, it needs to complete flush and flush cpu memory > cache. Track which LP the TDX vcpu run and flush it as necessary. > > Do nothing on sched_in event as TDX doesn't support pause loop. > > TDX vcpu execution requires restoring PMU debug store after returning back > to KVM because the TDX module unconditionally resets the value. To reuse > the existing code, export perf_restore_debug_store. > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > --- > arch/x86/kvm/vmx/main.c | 10 +++++++++- > arch/x86/kvm/vmx/tdx.c | 34 ++++++++++++++++++++++++++++++++++ > arch/x86/kvm/vmx/tdx.h | 33 +++++++++++++++++++++++++++++++++ > arch/x86/kvm/vmx/x86_ops.h | 2 ++ > arch/x86/kvm/x86.c | 1 + > 5 files changed, 79 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c > index f571b07c2aae..2e5a7a72d560 100644 > --- a/arch/x86/kvm/vmx/main.c > +++ b/arch/x86/kvm/vmx/main.c > @@ -89,6 +89,14 @@ static void vt_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > return vmx_vcpu_reset(vcpu, init_event); > } > > +static fastpath_t vt_vcpu_run(struct kvm_vcpu *vcpu) > +{ > + if (is_td_vcpu(vcpu)) > + return tdx_vcpu_run(vcpu); > + > + return vmx_vcpu_run(vcpu); > +} > + > static void vt_flush_tlb_all(struct kvm_vcpu *vcpu) > { > if (is_td_vcpu(vcpu)) > @@ -200,7 +208,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { > .tlb_flush_guest = vt_flush_tlb_guest, > > .vcpu_pre_run = vmx_vcpu_pre_run, > - .run = vmx_vcpu_run, > + .run = vt_vcpu_run, > .handle_exit = vmx_handle_exit, > .skip_emulated_instruction = vmx_skip_emulated_instruction, > .update_emulated_instruction = vmx_update_emulated_instruction, > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c > index 85d5f961d97e..ebe4f9bf19e7 100644 > --- a/arch/x86/kvm/vmx/tdx.c > +++ b/arch/x86/kvm/vmx/tdx.c > @@ -10,6 +10,9 @@ > #include "vmx.h" > #include "x86.h" > > +#include <trace/events/kvm.h> > +#include "trace.h" > + > #undef pr_fmt > #define pr_fmt(fmt) "tdx: " fmt > > @@ -509,6 +512,37 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > vcpu->kvm->vm_bugged = true; > } > > +u64 __tdx_vcpu_run(hpa_t tdvpr, void *regs, u32 regs_mask); > + > +static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu, > + struct vcpu_tdx *tdx) > +{ > + guest_enter_irqoff(); > + tdx->exit_reason.full = __tdx_vcpu_run(tdx->tdvpr.pa, vcpu->arch.regs, 0); > + guest_exit_irqoff(); > +} > + > +fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_tdx *tdx = to_tdx(vcpu); > + > + if (unlikely(vcpu->kvm->vm_bugged)) { > + tdx->exit_reason.full = TDX_NON_RECOVERABLE_VCPU; > + return EXIT_FASTPATH_NONE; > + } > + > + trace_kvm_entry(vcpu); > + > + tdx_vcpu_enter_exit(vcpu, tdx); > + > + vcpu->arch.regs_avail &= ~VMX_REGS_LAZY_LOAD_SET; > + trace_kvm_exit(vcpu, KVM_ISA_VMX); > + > + if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable) > + return EXIT_FASTPATH_NONE; Looks like the above if statement has no effect. Just checking if this is intentional. > + return EXIT_FASTPATH_NONE; > +} > + > void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int pgd_level) > { > td_vmcs_write64(to_tdx(vcpu), SHARED_EPT_POINTER, root_hpa & PAGE_MASK); > diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h > index bf9865a88991..e950404ce5de 100644 > --- a/arch/x86/kvm/vmx/tdx.h > +++ b/arch/x86/kvm/vmx/tdx.h > @@ -44,12 +44,45 @@ struct kvm_tdx { > spinlock_t seamcall_lock; > }; > > +union tdx_exit_reason { > + struct { > + /* 31:0 mirror the VMX Exit Reason format */ > + u64 basic : 16; > + u64 reserved16 : 1; > + u64 reserved17 : 1; > + u64 reserved18 : 1; > + u64 reserved19 : 1; > + u64 reserved20 : 1; > + u64 reserved21 : 1; > + u64 reserved22 : 1; > + u64 reserved23 : 1; > + u64 reserved24 : 1; > + u64 reserved25 : 1; > + u64 bus_lock_detected : 1; > + u64 enclave_mode : 1; > + u64 smi_pending_mtf : 1; > + u64 smi_from_vmx_root : 1; > + u64 reserved30 : 1; > + u64 failed_vmentry : 1; > + > + /* 63:32 are TDX specific */ > + u64 details_l1 : 8; > + u64 class : 8; > + u64 reserved61_48 : 14; > + u64 non_recoverable : 1; > + u64 error : 1; > + }; > + u64 full; > +}; > + > struct vcpu_tdx { > struct kvm_vcpu vcpu; > > struct tdx_td_page tdvpr; > struct tdx_td_page *tdvpx; > > + union tdx_exit_reason exit_reason; > + > bool initialized; > }; > > diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h > index 922a3799336e..44404dd25737 100644 > --- a/arch/x86/kvm/vmx/x86_ops.h > +++ b/arch/x86/kvm/vmx/x86_ops.h > @@ -140,6 +140,7 @@ void tdx_vm_free(struct kvm *kvm); > int tdx_vcpu_create(struct kvm_vcpu *vcpu); > void tdx_vcpu_free(struct kvm_vcpu *vcpu); > void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event); > +fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu); > > int tdx_vm_ioctl(struct kvm *kvm, void __user *argp); > int tdx_vcpu_ioctl(struct kvm_vcpu *vcpu, void __user *argp); > @@ -160,6 +161,7 @@ static inline void tdx_vm_free(struct kvm *kvm) {} > static inline int tdx_vcpu_create(struct kvm_vcpu *vcpu) { return -EOPNOTSUPP; } > static inline void tdx_vcpu_free(struct kvm_vcpu *vcpu) {} > static inline void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) {} > +static inline fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu) { return EXIT_FASTPATH_NONE; } > > static inline int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) { return -EOPNOTSUPP; } > static inline int tdx_vcpu_ioctl(struct kvm_vcpu *vcpu, void __user *argp) { return -EOPNOTSUPP; } > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index da411bcd8cbc..66400810d54f 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -300,6 +300,7 @@ const struct kvm_stats_header kvm_vcpu_stats_header = { > }; > > u64 __read_mostly host_xcr0; > +EXPORT_SYMBOL_GPL(host_xcr0); > u64 __read_mostly supported_xcr0; > EXPORT_SYMBOL_GPL(supported_xcr0); > > -- > 2.25.1 >
On Tue, Mar 22, 2022 at 10:28:42AM -0700, Erdem Aktas <erdemaktas@google.com> wrote: > On Fri, Mar 4, 2022 at 11:50 AM <isaku.yamahata@intel.com> wrote: > > @@ -509,6 +512,37 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > > vcpu->kvm->vm_bugged = true; > > } > > > > +u64 __tdx_vcpu_run(hpa_t tdvpr, void *regs, u32 regs_mask); > > + > > +static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu, > > + struct vcpu_tdx *tdx) > > +{ > > + guest_enter_irqoff(); > > + tdx->exit_reason.full = __tdx_vcpu_run(tdx->tdvpr.pa, vcpu->arch.regs, 0); > > + guest_exit_irqoff(); > > +} > > + > > +fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu) > > +{ > > + struct vcpu_tdx *tdx = to_tdx(vcpu); > > + > > + if (unlikely(vcpu->kvm->vm_bugged)) { > > + tdx->exit_reason.full = TDX_NON_RECOVERABLE_VCPU; > > + return EXIT_FASTPATH_NONE; > > + } > > + > > + trace_kvm_entry(vcpu); > > + > > + tdx_vcpu_enter_exit(vcpu, tdx); > > + > > + vcpu->arch.regs_avail &= ~VMX_REGS_LAZY_LOAD_SET; > > + trace_kvm_exit(vcpu, KVM_ISA_VMX); > > + > > + if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable) > > + return EXIT_FASTPATH_NONE; > > Looks like the above if statement has no effect. Just checking if this > is intentional. I'm not sure if I get your point. tdx->exit_reason is updated by the above tdx_cpu_enter_exit(). So it makes sense to check .error or .non_recoverable.
On Wed, Mar 23, 2022 at 10:55 AM Isaku Yamahata <isaku.yamahata@gmail.com> wrote: > > On Tue, Mar 22, 2022 at 10:28:42AM -0700, > Erdem Aktas <erdemaktas@google.com> wrote: > > > On Fri, Mar 4, 2022 at 11:50 AM <isaku.yamahata@intel.com> wrote: > > > @@ -509,6 +512,37 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > > > vcpu->kvm->vm_bugged = true; > > > } > > > > > > +u64 __tdx_vcpu_run(hpa_t tdvpr, void *regs, u32 regs_mask); > > > + > > > +static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu, > > > + struct vcpu_tdx *tdx) > > > +{ > > > + guest_enter_irqoff(); > > > + tdx->exit_reason.full = __tdx_vcpu_run(tdx->tdvpr.pa, vcpu->arch.regs, 0); > > > + guest_exit_irqoff(); > > > +} > > > + > > > +fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu) > > > +{ > > > + struct vcpu_tdx *tdx = to_tdx(vcpu); > > > + > > > + if (unlikely(vcpu->kvm->vm_bugged)) { > > > + tdx->exit_reason.full = TDX_NON_RECOVERABLE_VCPU; > > > + return EXIT_FASTPATH_NONE; > > > + } > > > + > > > + trace_kvm_entry(vcpu); > > > + > > > + tdx_vcpu_enter_exit(vcpu, tdx); > > > + > > > + vcpu->arch.regs_avail &= ~VMX_REGS_LAZY_LOAD_SET; > > > + trace_kvm_exit(vcpu, KVM_ISA_VMX); > > > + > > > + if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable) > > > + return EXIT_FASTPATH_NONE; > > > > Looks like the above if statement has no effect. Just checking if this > > is intentional. > > I'm not sure if I get your point. tdx->exit_reason is updated by the above > tdx_cpu_enter_exit(). So it makes sense to check .error or .non_recoverable. > -- > Isaku Yamahata <isaku.yamahata@gmail.com> What I mean is, if there is an error, it returns EXIT_FASTPATH_NONE but if there is no error, it still returns EXIT_FASTPATH_NONE. The code is like below, the if-statement might be there as a placeholder to check errors but it has no impact on what is returned from this function. if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable) return EXIT_FASTPATH_NONE; return EXIT_FASTPATH_NONE;
On Wed, Mar 23, 2022 at 01:05:27PM -0700, Erdem Aktas <erdemaktas@google.com> wrote: > On Wed, Mar 23, 2022 at 10:55 AM Isaku Yamahata > <isaku.yamahata@gmail.com> wrote: > > > > On Tue, Mar 22, 2022 at 10:28:42AM -0700, > > Erdem Aktas <erdemaktas@google.com> wrote: > > > > > On Fri, Mar 4, 2022 at 11:50 AM <isaku.yamahata@intel.com> wrote: > > > > @@ -509,6 +512,37 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > > > > vcpu->kvm->vm_bugged = true; > > > > } > > > > > > > > +u64 __tdx_vcpu_run(hpa_t tdvpr, void *regs, u32 regs_mask); > > > > + > > > > +static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu, > > > > + struct vcpu_tdx *tdx) > > > > +{ > > > > + guest_enter_irqoff(); > > > > + tdx->exit_reason.full = __tdx_vcpu_run(tdx->tdvpr.pa, vcpu->arch.regs, 0); > > > > + guest_exit_irqoff(); > > > > +} > > > > + > > > > +fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu) > > > > +{ > > > > + struct vcpu_tdx *tdx = to_tdx(vcpu); > > > > + > > > > + if (unlikely(vcpu->kvm->vm_bugged)) { > > > > + tdx->exit_reason.full = TDX_NON_RECOVERABLE_VCPU; > > > > + return EXIT_FASTPATH_NONE; > > > > + } > > > > + > > > > + trace_kvm_entry(vcpu); > > > > + > > > > + tdx_vcpu_enter_exit(vcpu, tdx); > > > > + > > > > + vcpu->arch.regs_avail &= ~VMX_REGS_LAZY_LOAD_SET; > > > > + trace_kvm_exit(vcpu, KVM_ISA_VMX); > > > > + > > > > + if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable) > > > > + return EXIT_FASTPATH_NONE; > > > > > > Looks like the above if statement has no effect. Just checking if this > > > is intentional. > > > > I'm not sure if I get your point. tdx->exit_reason is updated by the above > > tdx_cpu_enter_exit(). So it makes sense to check .error or .non_recoverable. > > -- > > Isaku Yamahata <isaku.yamahata@gmail.com> > > What I mean is, if there is an error, it returns EXIT_FASTPATH_NONE > but if there is no error, it still returns EXIT_FASTPATH_NONE. > > The code is like below, the if-statement might be there as a > placeholder to check errors but it has no impact on what is returned > from this function. > > if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable) > return EXIT_FASTPATH_NONE; > return EXIT_FASTPATH_NONE; Got it. It doesn't make sense. I'll fix it with the next respin.
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index f571b07c2aae..2e5a7a72d560 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -89,6 +89,14 @@ static void vt_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) return vmx_vcpu_reset(vcpu, init_event); } +static fastpath_t vt_vcpu_run(struct kvm_vcpu *vcpu) +{ + if (is_td_vcpu(vcpu)) + return tdx_vcpu_run(vcpu); + + return vmx_vcpu_run(vcpu); +} + static void vt_flush_tlb_all(struct kvm_vcpu *vcpu) { if (is_td_vcpu(vcpu)) @@ -200,7 +208,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .tlb_flush_guest = vt_flush_tlb_guest, .vcpu_pre_run = vmx_vcpu_pre_run, - .run = vmx_vcpu_run, + .run = vt_vcpu_run, .handle_exit = vmx_handle_exit, .skip_emulated_instruction = vmx_skip_emulated_instruction, .update_emulated_instruction = vmx_update_emulated_instruction, diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c index 85d5f961d97e..ebe4f9bf19e7 100644 --- a/arch/x86/kvm/vmx/tdx.c +++ b/arch/x86/kvm/vmx/tdx.c @@ -10,6 +10,9 @@ #include "vmx.h" #include "x86.h" +#include <trace/events/kvm.h> +#include "trace.h" + #undef pr_fmt #define pr_fmt(fmt) "tdx: " fmt @@ -509,6 +512,37 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) vcpu->kvm->vm_bugged = true; } +u64 __tdx_vcpu_run(hpa_t tdvpr, void *regs, u32 regs_mask); + +static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu, + struct vcpu_tdx *tdx) +{ + guest_enter_irqoff(); + tdx->exit_reason.full = __tdx_vcpu_run(tdx->tdvpr.pa, vcpu->arch.regs, 0); + guest_exit_irqoff(); +} + +fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu) +{ + struct vcpu_tdx *tdx = to_tdx(vcpu); + + if (unlikely(vcpu->kvm->vm_bugged)) { + tdx->exit_reason.full = TDX_NON_RECOVERABLE_VCPU; + return EXIT_FASTPATH_NONE; + } + + trace_kvm_entry(vcpu); + + tdx_vcpu_enter_exit(vcpu, tdx); + + vcpu->arch.regs_avail &= ~VMX_REGS_LAZY_LOAD_SET; + trace_kvm_exit(vcpu, KVM_ISA_VMX); + + if (tdx->exit_reason.error || tdx->exit_reason.non_recoverable) + return EXIT_FASTPATH_NONE; + return EXIT_FASTPATH_NONE; +} + void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int pgd_level) { td_vmcs_write64(to_tdx(vcpu), SHARED_EPT_POINTER, root_hpa & PAGE_MASK); diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h index bf9865a88991..e950404ce5de 100644 --- a/arch/x86/kvm/vmx/tdx.h +++ b/arch/x86/kvm/vmx/tdx.h @@ -44,12 +44,45 @@ struct kvm_tdx { spinlock_t seamcall_lock; }; +union tdx_exit_reason { + struct { + /* 31:0 mirror the VMX Exit Reason format */ + u64 basic : 16; + u64 reserved16 : 1; + u64 reserved17 : 1; + u64 reserved18 : 1; + u64 reserved19 : 1; + u64 reserved20 : 1; + u64 reserved21 : 1; + u64 reserved22 : 1; + u64 reserved23 : 1; + u64 reserved24 : 1; + u64 reserved25 : 1; + u64 bus_lock_detected : 1; + u64 enclave_mode : 1; + u64 smi_pending_mtf : 1; + u64 smi_from_vmx_root : 1; + u64 reserved30 : 1; + u64 failed_vmentry : 1; + + /* 63:32 are TDX specific */ + u64 details_l1 : 8; + u64 class : 8; + u64 reserved61_48 : 14; + u64 non_recoverable : 1; + u64 error : 1; + }; + u64 full; +}; + struct vcpu_tdx { struct kvm_vcpu vcpu; struct tdx_td_page tdvpr; struct tdx_td_page *tdvpx; + union tdx_exit_reason exit_reason; + bool initialized; }; diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index 922a3799336e..44404dd25737 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -140,6 +140,7 @@ void tdx_vm_free(struct kvm *kvm); int tdx_vcpu_create(struct kvm_vcpu *vcpu); void tdx_vcpu_free(struct kvm_vcpu *vcpu); void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event); +fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu); int tdx_vm_ioctl(struct kvm *kvm, void __user *argp); int tdx_vcpu_ioctl(struct kvm_vcpu *vcpu, void __user *argp); @@ -160,6 +161,7 @@ static inline void tdx_vm_free(struct kvm *kvm) {} static inline int tdx_vcpu_create(struct kvm_vcpu *vcpu) { return -EOPNOTSUPP; } static inline void tdx_vcpu_free(struct kvm_vcpu *vcpu) {} static inline void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) {} +static inline fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu) { return EXIT_FASTPATH_NONE; } static inline int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) { return -EOPNOTSUPP; } static inline int tdx_vcpu_ioctl(struct kvm_vcpu *vcpu, void __user *argp) { return -EOPNOTSUPP; } diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index da411bcd8cbc..66400810d54f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -300,6 +300,7 @@ const struct kvm_stats_header kvm_vcpu_stats_header = { }; u64 __read_mostly host_xcr0; +EXPORT_SYMBOL_GPL(host_xcr0); u64 __read_mostly supported_xcr0; EXPORT_SYMBOL_GPL(supported_xcr0);