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
