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

[Paolo Bonzini <pbonzini@redhat.com>]

On 5/18/22 15:25, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
> 
> 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@tencent.com>
> Reviewed-by: Wanpeng Li <wanpengli@tencent.com>

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

>   arch/x86/kvm/pmu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index f189512207db..24624654e476 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -246,8 +246,9 @@ static bool check_pmu_event_filter(struct kvm_pmc *pmc)
>   	struct kvm *kvm = pmc->vcpu->kvm;
>   	bool allow_event = true;
>   	__u64 key;
> -	int idx;
> +	int idx, srcu_idx;
>   
> +	srcu_idx = srcu_read_lock(&kvm->srcu);
>   	filter = srcu_dereference(kvm->arch.pmu_event_filter, &kvm->srcu);
>   	if (!filter)
>   		goto out;
> @@ -270,6 +271,7 @@ static bool check_pmu_event_filter(struct kvm_pmc *pmc)
>   	}
>   
>   out:
> +	srcu_read_unlock(&kvm->srcu, srcu_idx);
>   	return allow_event;
>   }
>