Re: [PATCH v2 00/32] KVM: x86: Clean up MSR interception code

[Manali Shukla <manali.shukla@amd.com>]

Hi Sean,

On 6/11/2025 4:27 AM, Sean Christopherson wrote:
> Clean up KVM's MSR interception code (especially the SVM code, which is all
> kinds of ugly).  The main goals are to:
> 
>  - Make the SVM and VMX APIs consistent (and sane; the current SVM APIs have
>    inverted polarity).
> 
>  - Eliminate the shadow bitmaps that are used to determine intercepts on
>    userspace MSR filter update.
> 
> v2:
>  - Add a patch to set MSR_IA32_SPEC_CTRL interception as appropriate. [Chao]
>  - Add a patch to cleanup {svm,vmx}_disable_intercept_for_msr() once the
>    dust has settled. [Dapeng]
>  - Return -ENOSPC if msrpm_offsets[] is full. [Chao]
>  - Free iopm_pages directly instead of bouncing through iopm_base. [Chao]
>  - Check for "offset == MSR_INVALID" before using offset. [Chao]
>  - Temporarily keep MSR_IA32_DEBUGCTLMSR in the nested list. [Chao]
>  - Add a comment to explain nested_svm_msrpm_merge_offsets. [Chao]
>  - Add a patch to shift the IOPM allocation to avoid having to unwind it.
>  - Init nested_svm_msrpm_merge_offsets iff nested=1. [Chao]
>  - Add a helper to dedup alloc+init of MSRPM and IOPM.
>  - Tag merge_msrs as "static" and "__initconst". [Paolo]
>  - Rework helpers to use fewer macros. [Paolo]
>  - Account for each MSRPM byte covering 4 MSRs. [Paolo]
>  - Opportunistically use cpu_feature_enabled(). [Xin]
>  - Fully remove MAX_DIRECT_ACCESS_MSRS, MSRPM_OFFSETS, and msrpm_offsets.
>    [Francesco]
>  - Fix typos. [Dapeng, Chao]
>  - Collect reviews. [Chao, Dapeng, Xin]
> 
> v1: https://lore.kernel.org/all/20250529234013.3826933-1-seanjc@google.com
> 
> v0: https://lore.kernel.org/kvm/20241127201929.4005605-1-aaronlewis@google.com
> 
> Sean Christopherson (32):
>   KVM: SVM: Disable interception of SPEC_CTRL iff the MSR exists for the
>     guest
>   KVM: SVM: Allocate IOPM pages after initial setup in
>     svm_hardware_setup()
>   KVM: SVM: Don't BUG if setting up the MSR intercept bitmaps fails
>   KVM: SVM: Tag MSR bitmap initialization helpers with __init
>   KVM: SVM: Use ARRAY_SIZE() to iterate over direct_access_msrs
>   KVM: SVM: Kill the VM instead of the host if MSR interception is buggy
>   KVM: x86: Use non-atomic bit ops to manipulate "shadow" MSR intercepts
>   KVM: SVM: Massage name and param of helper that merges vmcb01 and
>     vmcb12 MSRPMs
>   KVM: SVM: Clean up macros related to architectural MSRPM definitions
>   KVM: nSVM: Use dedicated array of MSRPM offsets to merge L0 and L1
>     bitmaps
>   KVM: nSVM: Omit SEV-ES specific passthrough MSRs from L0+L1 bitmap
>     merge
>   KVM: nSVM: Don't initialize vmcb02 MSRPM with vmcb01's "always
>     passthrough"
>   KVM: SVM: Add helpers for accessing MSR bitmap that don't rely on
>     offsets
>   KVM: SVM: Implement and adopt VMX style MSR intercepts APIs
>   KVM: SVM: Pass through GHCB MSR if and only if VM is an SEV-ES guest
>   KVM: SVM: Drop "always" flag from list of possible passthrough MSRs
>   KVM: x86: Move definition of X2APIC_MSR() to lapic.h
>   KVM: VMX: Manually recalc all MSR intercepts on userspace MSR filter
>     change
>   KVM: SVM: Manually recalc all MSR intercepts on userspace MSR filter
>     change
>   KVM: x86: Rename msr_filter_changed() => recalc_msr_intercepts()
>   KVM: SVM: Rename init_vmcb_after_set_cpuid() to make it intercepts
>     specific
>   KVM: SVM: Fold svm_vcpu_init_msrpm() into its sole caller
>   KVM: SVM: Merge "after set CPUID" intercept recalc helpers
>   KVM: SVM: Drop explicit check on MSRPM offset when emulating SEV-ES
>     accesses
>   KVM: SVM: Move svm_msrpm_offset() to nested.c
>   KVM: SVM: Store MSRPM pointer as "void *" instead of "u32 *"
>   KVM: nSVM: Access MSRPM in 4-byte chunks only for merging L0 and L1
>     bitmaps
>   KVM: SVM: Return -EINVAL instead of MSR_INVALID to signal out-of-range
>     MSR
>   KVM: nSVM: Merge MSRPM in 64-bit chunks on 64-bit kernels
>   KVM: SVM: Add a helper to allocate and initialize permissions bitmaps
>   KVM: x86: Simplify userspace filter logic when disabling MSR
>     interception
>   KVM: selftests: Verify KVM disable interception (for userspace) on
>     filter change
> 
>  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/nested.c                     | 126 +++--
>  arch/x86/kvm/svm/sev.c                        |  29 +-
>  arch/x86/kvm/svm/svm.c                        | 490 ++++++------------
>  arch/x86/kvm/svm/svm.h                        | 102 +++-
>  arch/x86/kvm/vmx/main.c                       |   6 +-
>  arch/x86/kvm/vmx/vmx.c                        | 202 ++------
>  arch/x86/kvm/vmx/vmx.h                        |   9 -
>  arch/x86/kvm/vmx/x86_ops.h                    |   2 +-
>  arch/x86/kvm/x86.c                            |   8 +-
>  .../kvm/x86/userspace_msr_exit_test.c         |   8 +
>  13 files changed, 426 insertions(+), 562 deletions(-)
> 
> 
> base-commit: 61374cc145f4a56377eaf87c7409a97ec7a34041


I’ve tested this patch series using the `msr` tests from kvm-unit-tests and didn’t
observe any unexpected results.

Additionally, I rebased the mediated PMU v4 patches on top of this series and ran
PMU-related tests from kvm-unit-tests with the following configurations:

  -cpu host
  -cpu host,-perfctr-core
  -cpu host,-perfmon-v2

I don't see any unexpected results.
Testing was performed on a Turin machine (AMD EPYC 9745 128-Core Processor).

I understand the patches are already merged, but just wanted to share this for reference.  

Feel free to add:
Tested-by: Manali Shukla <Manali.Shukla@amd.com>

-Manali