| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230411154938.000059a9.zhi.wang.linux@gmail.com>
Date: Tue, 11 Apr 2023 15:49:38 +0300
From: Zhi Wang <zhi.wang.linux@...il.com>
To: isaku.yamahata@...el.com
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
isaku.yamahata@...il.com, Paolo Bonzini <pbonzini@...hat.com>,
erdemaktas@...gle.com, Sean Christopherson <seanjc@...gle.com>,
Sagi Shahar <sagis@...gle.com>,
David Matlack <dmatlack@...gle.com>,
Kai Huang <kai.huang@...el.com>
Subject: Re: [PATCH v13 064/113] KVM: TDX: Implement TDX vcpu enter/exit
path
On Sun, 12 Mar 2023 10:56:28 -0700
isaku.yamahata@...el.com wrote:
> From: Isaku Yamahata <isaku.yamahata@...el.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@...el.com>
> ---
> arch/x86/kvm/vmx/main.c | 21 +++++++++++++++++++--
> arch/x86/kvm/vmx/tdx.c | 32 ++++++++++++++++++++++++++++++++
> arch/x86/kvm/vmx/tdx.h | 33 +++++++++++++++++++++++++++++++++
> arch/x86/kvm/vmx/x86_ops.h | 2 ++
> arch/x86/kvm/x86.c | 1 +
> 5 files changed, 87 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
> index 55001b34e1f0..2fd6c954590d 100644
> --- a/arch/x86/kvm/vmx/main.c
> +++ b/arch/x86/kvm/vmx/main.c
> @@ -170,6 +170,23 @@ static void vt_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> vmx_vcpu_reset(vcpu, init_event);
> }
>
> +static int vt_vcpu_pre_run(struct kvm_vcpu *vcpu)
> +{
> + if (is_td_vcpu(vcpu))
> + /* Unconditionally continue to vcpu_run(). */
> + return 1;
> +
> + return vmx_vcpu_pre_run(vcpu);
> +}
> +
> +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)) {
> @@ -323,8 +340,8 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
> .flush_tlb_gva = vt_flush_tlb_gva,
> .flush_tlb_guest = vt_flush_tlb_guest,
>
> - .vcpu_pre_run = vmx_vcpu_pre_run,
> - .vcpu_run = vmx_vcpu_run,
> + .vcpu_pre_run = vt_vcpu_pre_run,
> + .vcpu_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 d5a2f769a58d..28a19b14cbbc 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -11,6 +11,9 @@
> #include "x86.h"
> #include "mmu.h"
>
> +#include <trace/events/kvm.h>
> +#include "trace.h"
> +
> #undef pr_fmt
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> @@ -439,6 +442,35 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> */
> }
>
> +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;
> + }
> +
Maybe check if a TD vCPU is initialized here or in the vcpu_pre_run? Bascially
I am thinking what if a TD vCPU is not initialized by KVM_TDX_INIT_VCPU (TDVPR
does not even exist) and now userspace wants to run it. What would be the
consequence?
> + 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);
> +
> + 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 9d8445324841..af29e1d89657 100644
> --- a/arch/x86/kvm/vmx/tdx.h
> +++ b/arch/x86/kvm/vmx/tdx.h
> @@ -25,12 +25,45 @@ struct kvm_tdx {
> u64 tsc_offset;
> };
>
> +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;
>
> unsigned long tdvpr_pa;
> unsigned long *tdvpx_pa;
>
> + 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 eba10dabc45f..c939a9d4d927 100644
> --- a/arch/x86/kvm/vmx/x86_ops.h
> +++ b/arch/x86/kvm/vmx/x86_ops.h
> @@ -155,6 +155,7 @@ int tdx_vm_ioctl(struct kvm *kvm, void __user *argp);
> 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);
> u8 tdx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
>
> int tdx_vcpu_ioctl(struct kvm_vcpu *vcpu, void __user *argp);
> @@ -181,6 +182,7 @@ static inline int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) { return -EOP
> 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 u8 tdx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio) { return 0; }
>
> 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 6d7ca694e1c9..41af9a943d49 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -309,6 +309,7 @@ const struct kvm_stats_header kvm_vcpu_stats_header = {
> };
>
> u64 __read_mostly host_xcr0;
> +EXPORT_SYMBOL_GPL(host_xcr0);
>
> static struct kmem_cache *x86_emulator_cache;
>
Powered by blists - more mailing lists