Re: [PATCH] KVM: x86: Fix a stall when KVM_SET_MSRS is called on the pmu counters

[Sean Christopherson <seanjc@google.com>]

On Fri, Oct 28, 2022, Aaron Lewis wrote:
> On Fri, Oct 28, 2022 at 4:26 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, Oct 28, 2022, Aaron Lewis wrote:
> > @@ -3778,16 +3775,13 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >
> >     case MSR_K7_PERFCTR0 ... MSR_K7_PERFCTR3:
> >     case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1:
> > -        pr = true;
> > -        fallthrough;
> >     case MSR_K7_EVNTSEL0 ... MSR_K7_EVNTSEL3:
> >     case MSR_P6_EVNTSEL0 ... MSR_P6_EVNTSEL1:
> >         if (kvm_pmu_is_valid_msr(vcpu, msr))
> >             return kvm_pmu_set_msr(vcpu, msr_info);
> >
> > -        if (pr || data != 0)
> > -            vcpu_unimpl(vcpu, "disabled perfctr wrmsr: "
> > -                  "0x%x data 0x%llx\n", msr, data);
> > +        if (data)
> > +            kvm_pr_unimpl_wrmsr(vcpu, msr, data);
> 
> Any reason to keep the check for 'data' around?  Now that it's
> checking for 'report_ignored_msrs' maybe we don't need that check as
> well.  I'm not sure what the harm is in removing it, and with this
> change we are additionally restricting pmu counter == 0 from printing.

Checking 'dat' doesn't restrict counter 0, it skips printing if the guest (or host)
is writing '0', e.g. it would also skip the case you encountered where the host is
blindly "restoring" unused MSRs.

Or did I misunderstand your comment?