Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Vitaly Kuznetsov <vkuznets@redhat.com>
To:     Jim Mattson <jmattson@google.com>
Cc:     kvm list <kvm@vger.kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Like Xu <like.xu@linux.intel.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KVM: SVM: drop MSR_IA32_PERF_CAPABILITIES from emulated MSRs
In-Reply-To: <CALMp9eQ1qe4w5FojzgsUHKpD=zXqen_D6bBg4-vfHa03BdomGA@mail.gmail.com>
References: <20200616161427.375651-1-vkuznets@redhat.com> <CALMp9eSWXGQkOOzSrALfZDMj5JHSH=CsK1wKfdj2x2jtV4XJsw@mail.gmail.com> <87366vhscx.fsf@vitty.brq.redhat.com> <CALMp9eQ1qe4w5FojzgsUHKpD=zXqen_D6bBg4-vfHa03BdomGA@mail.gmail.com>
Date:   Wed, 17 Jun 2020 13:38:01 +0200
Message-ID: <87wo45hqhy.fsf@vitty.brq.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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