On Tue, Oct 1, 2019 at 7:24 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > On 01/10/19 16:07, Jim Mattson wrote: > > On Tue, Oct 1, 2019 at 6:29 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > >> > >> On 01/10/19 13:32, Vitaly Kuznetsov wrote: > >>> Jim Mattson <jmattson@xxxxxxxxxx> writes: > >>> > >>>> KVM can only virtualize as many PMCs as the host supports. > >>>> > >>>> Limit the number of generic counters and fixed counters to the number > >>>> of corresponding counters supported on the host, rather than to > >>>> INTEL_PMC_MAX_GENERIC and INTEL_PMC_MAX_FIXED, respectively. > >>>> > >>>> Note that INTEL_PMC_MAX_GENERIC is currently 32, which exceeds the 18 > >>>> contiguous MSR indices reserved by Intel for event selectors. Since > >>>> the existing code relies on a contiguous range of MSR indices for > >>>> event selectors, it can't possibly work for more than 18 general > >>>> purpose counters. > >>> > >>> Should we also trim msrs_to_save[] by removing impossible entries > >>> (18-31) then? > >> > >> Yes, I'll send a patch in a second. > > > > I thought you were going to revert that msrs_to_save patch. I've been > > working on a replacement. > > We can use a little more time to think more about it and discuss it. > > For example, trimming is enough for the basic usage of passing > KVM_SET_SUPPORTED_CPUID output to KVM_SET_CPUID2 and then retrieving all > MSRs in the list. If that is also okay for Google's userspace, we might > actually leave everything that way and retroactively decide that you > need to filter the MSRs but only if you pass your own CPUID. > > Paolo If just trimming the static list, remember to trim to even less than 18, since Intel has used one of the reserved MSRs following the event selectors for something else. I was going to follow Sean's suggestion and specifically enumerate all of the PMU MSRs based on CPUID 0AH.
