Re: [PATCH RESEND v3 03/11] KVM: x86/pmu: Protect kvm->arch.pmu_event_filter with SRCU

[Jim Mattson]

On Fri, May 20, 2022 at 5:51 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> On 5/18/22 15:25, Like Xu wrote:
> > From: Like Xu <likexu@xxxxxxxxxxx>
> >
> > Similar to "kvm->arch.msr_filter", KVM should guarantee that vCPUs will
> > see either the previous filter or the new filter when user space calls
> > KVM_SET_PMU_EVENT_FILTER ioctl with the vCPU running so that guest
> > pmu events with identical settings in both the old and new filter have
> > deterministic behavior.
> >
> > Fixes: 66bb8a065f5a ("KVM: x86: PMU Event Filter")
> > Signed-off-by: Like Xu <likexu@xxxxxxxxxxx>
> > Reviewed-by: Wanpeng Li <wanpengli@xxxxxxxxxxx>
>
> Please always include the call trace where SRCU is not taken.  The ones
> I reconstructed always end up at a place inside srcu_read_lock/unlock:
>
> reprogram_gp_counter/reprogram_fixed_counter
>    amd_pmu_set_msr
>     kvm_set_msr_common
>      svm_set_msr
>       __kvm_set_msr
>       kvm_set_msr_ignored_check
>        kvm_set_msr_with_filter
>         kvm_emulate_wrmsr**
>         emulator_set_msr_with_filter**
>        kvm_set_msr
>         emulator_set_msr**
>        do_set_msr
>         __msr_io
>          msr_io
>           ioctl(KVM_SET_MSRS)**
>    intel_pmu_set_msr
>     kvm_set_msr_common
>      vmx_set_msr (see svm_set_msr)
>    reprogram_counter
>     global_ctrl_changed
>      intel_pmu_set_msr (see above)
>     kvm_pmu_handle_event
>      vcpu_enter_guest**
>     kvm_pmu_incr_counter
>      kvm_pmu_trigger_event
>       nested_vmx_run**
>       kvm_skip_emulated_instruction**
>       x86_emulate_instruction**
>    reprogram_fixed_counters
>     intel_pmu_set_msr (see above)
>
> Paolo

I agree with Paolo that existing usage is covered by
srcu_read_lock/unlock, but (a) it's not easy to confirm this, and (b)
this is very fragile.

Whichever way we decide to go, the userspace MSR filter and the PMU
event filter should adopt the same approach.