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, Jim Mattson wrote:
> On Fri, Oct 28, 2022 at 2:51 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > 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.
> 
> The VMM is only blind because KVM_GET_MSR_INDEX_LIST poked it in the
> eye. It would be nice to have an API that the VMM could query for the
> list of supported MSRs.

That should be a fairly easy bug fix, kvm_init_msr_list() can and should omit PMU
MSRs if enable_pmu==false.