On Fri, Oct 28, 2022, Jim Mattson wrote: > On Fri, Oct 28, 2022 at 2:51 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > On Fri, Oct 28, 2022, Aaron Lewis wrote: > > > On Fri, Oct 28, 2022 at 4:26 PM Sean Christopherson <seanjc@xxxxxxxxxx> 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.
