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. > > Or did I misunderstand your comment? That makes sense. Reviewed-by: Aaron Lewis <aaronlewis@xxxxxxxxxx>
