+Xin, Boris, and Dapeng
On Wed, Nov 27, 2024, Aaron Lewis wrote:
> Move the possible passthrough MSRs to kvm_x86_ops. Doing this allows
> them to be accessed from common x86 code.
...
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3e8afc82ae2fb..7e9fee4d36cc2 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1817,6 +1817,9 @@ struct kvm_x86_ops {
> int (*enable_l2_tlb_flush)(struct kvm_vcpu *vcpu);
>
> void (*migrate_timers)(struct kvm_vcpu *vcpu);
> +
> + const u32 * const possible_passthrough_msrs;
> + const u32 nr_possible_passthrough_msrs;
> void (*msr_filter_changed)(struct kvm_vcpu *vcpu);
> int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
...
> +/*
> + * List of MSRs that can be directly passed to the guest.
> + * In addition to these x2apic, PT and LBR MSRs are handled specially.
> + */
> +static const u32 vmx_possible_passthrough_msrs[] = {
> + MSR_IA32_SPEC_CTRL,
> + MSR_IA32_PRED_CMD,
> + MSR_IA32_FLUSH_CMD,
> + MSR_IA32_TSC,
> +#ifdef CONFIG_X86_64
> + MSR_FS_BASE,
> + MSR_GS_BASE,
> + MSR_KERNEL_GS_BASE,
> + MSR_IA32_XFD,
> + MSR_IA32_XFD_ERR,
> +#endif
> + MSR_IA32_SYSENTER_CS,
> + MSR_IA32_SYSENTER_ESP,
> + MSR_IA32_SYSENTER_EIP,
> + MSR_CORE_C1_RES,
> + MSR_CORE_C3_RESIDENCY,
> + MSR_CORE_C6_RESIDENCY,
> + MSR_CORE_C7_RESIDENCY,
> +};
Looking at this with fresh eyes, the "possible" passthrough MSR list, and SVM's
direct_access_msrs, are confusing and flat out stupid. VMX's list isn't the set
of "possible" passthrough MSRs, it's the set of MSRs for which KVM may disable
interceptions without dedicated handling in .msr_filter_changed(). Ditto for
SVM's list, but at least SVM's array uses a slightly less awful name.
Xin Li and Boris have been bikeshedding over the VMX array, and it's all a giant
waste of time.
In all cases, KVM *already knows* which MSRs it wants to pass-through to the
guest. In a few cases the logic isn't super intuitive, e.g. for SPEC_CTRL, but
it's always fairly easy to understand what KVM wants to do.
Rather than expose the lists to common code, I think we should pivot after
"KVM: SVM: Drop "always" flag from list of possible passthrough MSRs" and rip
them out entirely.
The attached patch is compile-tested only (the nested interactions in particular
need a bit of scrutiny) and needs to be chunked into multiple patches, but I don't
see any obvious blockers, and the diffstats speak volumes:
arch/x86/include/asm/kvm-x86-ops.h | 2 +-
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/lapic.h | 2 +
arch/x86/kvm/svm/svm.c | 310 ++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------------------------------------
arch/x86/kvm/svm/svm.h | 6 ---
arch/x86/kvm/vmx/main.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 179 ++++++++++++++++++---------------------------------------------------------
arch/x86/kvm/vmx/vmx.h | 9 ----
arch/x86/kvm/vmx/x86_ops.h | 2 +-
arch/x86/kvm/x86.c | 10 ++++-
10 files changed, 147 insertions(+), 377 deletions(-)
[*] https://lore.kernel.org/all/20241001050110.3643764-10-xin@xxxxxxxxx
