On Fri, Sep 27, 2019 at 8:55 AM Jim Mattson <jmattson@xxxxxxxxxx> wrote: > > On Fri, Sep 27, 2019 at 8:47 AM Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: > > > > Sean Christopherson <sean.j.christopherson@xxxxxxxxx> writes: > > > > > On Fri, Sep 27, 2019 at 05:19:25PM +0200, Paolo Bonzini wrote: > > >> On 27/09/19 16:40, Vitaly Kuznetsov wrote: > > >> > Paolo Bonzini <pbonzini@xxxxxxxxxx> writes: > > >> > > > >> >> On 27/09/19 15:53, Vitaly Kuznetsov wrote: > > >> >>> Paolo Bonzini <pbonzini@xxxxxxxxxx> 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) > > > > > > That "failed at %x" print line should really be updated to make it clear > > > that it's printing hex... > > > > > > > Yea, I also wasn't sure and had to look in the code. Will send a patch > > if no one beats me to it. > > > > >> >>> 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. > > > > > > My vote would be to programmatically generate the MSRs using CPUID and the > > > base MSR, as opposed to dumping them into the list and cross-referencing > > > them against CPUID. E.g. there should also be some form of check that the > > > architectural PMUs are even supported. > > > > Yes. The problem appears to be that msrs_to_save[] and emulated_msrs[] > > are global and for the MSRs in question we check > > kvm_find_cpuid_entry(vcpu, 0xa, ) to find out how many of them are > > available so this can be different for different VMs (and even vCPUs :-) > > However, > > > > "KVM_GET_MSR_INDEX_LIST returns the guest msrs that are supported. The list > > varies by kvm version and host processor, but does not change otherwise." > > > > So it seems that PMU MSRs just can't be there. Revert? > > The API design is unfortunate, but I would argue that any MSR that a > guest *might* support has to be in this list for live migration to > work with the vPMU enabled. I don't know about qemu, but Google's > userspace will only save/restore MSRs that are in this list Having said that, please revert the offending commit, and I'll take another shot at it (unless someone else wants to do it).
