Re: [PATCH] KVM: SVM: drop MSR_IA32_PERF_CAPABILITIES from emulated MSRs

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Jim Mattson <jmattson@google.com> writes:

> On Tue, Jun 16, 2020 at 9:45 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> Jim Mattson <jmattson@google.com> writes:
>>
>> > On Tue, Jun 16, 2020 at 9:14 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>> >>
>> >> state_test/smm_test selftests are failing on AMD with:
>> >> "Unexpected result from KVM_GET_MSRS, r: 51 (failed MSR was 0x345)"
>> >>
>> >> MSR_IA32_PERF_CAPABILITIES is an emulated MSR indeed but only on Intel,
>> >> make svm_has_emulated_msr() skip it so it is not returned by
>> >> KVM_GET_MSR_INDEX_LIST.
>> >
>> > Do we need to support this MSR under SVM for cross-vendor migration?
>> > Or, have we given up on that?
>>
>> To be honest I'm not sure about the status of cross-vendor migration in
>> general and PMU implications in particular, hope Paolo/Sean can shed
>> some light. In this particular case my shallow understanding is that
>> MSR_IA32_PERF_CAPABILITIES has only one known feature bit which unlocks
>> an MSR range with additional counters. If the feature bit is not set
>> this, I guess, can easily be migrated (basically, let's allow writing
>> '0' there on AMD and return '0' on read). But what if the feature was
>> enabled? We'll have to support the new MSR range and do something with
>> it after migration (run intel_pmu in fully emulated mode?).
>>
>> Anyway, the immediate issue I'm trying to fix here is: whatever is
>> returned by KVM_GET_MSR_INDEX_LIST can be successfully queried with
>> KVM_GET_MSRS as some userspaces count on that.
>
> That's a nice property. Is it documented somewhere?
>

Hm, good question.

Documentation/virt/kvm/api.rst says:

"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.

[...]

KVM_GET_MSR_FEATURE_INDEX_LIST returns the list of MSRs that can be passed
to the KVM_GET_MSRS system ioctl.  This lets userspace probe host capabilities
and processor features that are exposed via MSRs (e.g., VMX capabilities)."

Side note: MSR_IA32_PERF_CAPABILITIES can be returned by both
KVM_GET_MSR_INDEX_LIST and KVM_GET_MSR_FEATURE_INDEX_LIST as we have it
both as an emulated MSR filtered by kvm_x86_ops.has_emulated_msr() and
a feature msr filtered by kvm_x86_ops.get_msr_feature(). But the later
is a whitelist so MSR_IA32_PERF_CAPABILITIES won't appear on AMD and the
promise "can be passed to the KVM_GET_MSRS" is kept.

For KVM_GET_MSR_INDEX_LIST, the promise is "guest msrs that are
supported" and I'm not exactly sure what this means. Personally, I see
no point in returning MSRs which can't be read with KVM_GET_MSRS (as
this also means the guest can't read them) and KVM selftests seem to
rely on that (vcpu_save_state()) but this is not a documented feature.

> Reviewed-by: Jim Mattson <jmattson@google.com>
>

Thanks!

-- 
Vitaly