Re: [PATCH] kvm: x86: Add Intel PMU MSRs to msrs_to_save[]

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Fri, Sep 27, 2019 at 10:36:14AM -0700, Jim Mattson wrote:
> On Fri, Sep 27, 2019 at 8:19 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 27/09/19 16:40, Vitaly Kuznetsov wrote:
> > > Paolo Bonzini <pbonzini@redhat.com> writes:
> > >
> > >> On 27/09/19 15:53, Vitaly Kuznetsov wrote:
> > >>> Paolo Bonzini <pbonzini@redhat.com> writes:
> > >>>
> > >>>> Queued, thanks.
> > >>>
> > >>> I'm sorry for late feedback but this commit seems to be causing
> > >>> selftests failures for me, e.g.:
> > >>>
> > >>> # ./x86_64/state_test
> > >>> Testing guest mode: PA-bits:ANY, VA-bits:48,  4K pages
> > >>> Guest physical address width detected: 46
> > >>> ==== Test Assertion Failure ====
> > >>>   lib/x86_64/processor.c:1089: r == nmsrs
> > >>>   pid=14431 tid=14431 - Argument list too long
> > >>>      1      0x000000000040a55f: vcpu_save_state at processor.c:1088 (discriminator 3)
> > >>>      2      0x00000000004010e3: main at state_test.c:171 (discriminator 4)
> > >>>      3      0x00007f881eb453d4: ?? ??:0
> > >>>      4      0x0000000000401287: _start at ??:?
> > >>>   Unexpected result from KVM_GET_MSRS, r: 36 (failed at 194)
> > >>>
> > >>> Is this something known already or should I investigate?
> > >>
> > >> No, I didn't know about it, it works here.
> > >>
> > >
> > > Ok, this is a bit weird :-) '194' is 'MSR_ARCH_PERFMON_EVENTSEL0 +
> > > 14'. In intel_pmu_refresh() nr_arch_gp_counters is set to '8', however,
> > > rdmsr_safe() for this MSR passes in kvm_init_msr_list() (but it fails
> > > for 0x18e..0x193!) so it stay in the list. get_gp_pmc(), however, checks
> > > it against nr_arch_gp_counters and returns a failure.
> >
> > Huh, 194h apparently is a "FLEX_RATIO" MSR.  I agree that PMU MSRs need
> > to be checked against CPUID before allowing them.
> 
> According to the SDM, volume 4, table 2-2, IA-32 Architectural MSRs,
> 194H is reserved. Sounds like your CPU is mis-architected. :-)

It's definitely supposed to be reserved, the footnote in that table even
doubles down:

  Starting with Intel Core Duo processors, MSR addresses 180H-185H,
  188H-197H are reserved.

But looking at some internal definitions for MSR layouts, 194H is used for
IA32_FLEX_RATIO MSR on quite literally every processor that is supposed to
reserve 188H-197H.