From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>, Jim Mattson <jmattson@google.com>
Cc: kvm list <kvm@vger.kernel.org>,
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
Date: Thu, 18 Jun 2020 13:08:46 +0200 [thread overview]
Message-ID: <87imfohbr5.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <36ebc576-52c0-4164-1c83-e31146806b6b@redhat.com>
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 17/06/20 13:38, Vitaly Kuznetsov wrote:
>>
>> 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.
>
> Yes, this is intended. KVM_GET_MSR_INDEX_LIST is not the full list of
> supported MSRs or KVM_GET_MSRS (especially PMU MSRs are missing) but it
> certainly should be a sufficient condition for KVM_GET_MSRS support.
>
> In this case your patch is sort-of correct because AMD machines won't
> have X86_FEATURE_PDCM. However, even in that case there are two things
> we can do that are better:
>
> 1) force-set X86_FEATURE_PDCM in vmx_set_cpu_caps instead of having it
> in kvm_set_cpu_caps. The latter is incorrect because if AMD for
> whatever reason added it we'd lack the support. This would be basically
> a refined version of your patch.
>
> 2) emulate the MSR on AMD too (returning zero) if somebody for whatever
> reason enables PDCM in there too: this would include returning it in
> KVM_GET_FEATURE_MSR_INDEX_LIST, and using kvm_get_msr_feature to set a
> default value in kvm_pmu_refresh. The feature bit then would be
> force-set in kvm_set_cpu_caps. This would be nicer since we have the
> value in vcpu->arch already instead of struct vcpu_vmx.
Let's try the hard way :-) I'll send v2 implementing 2) (hope I got the
idea right), thanks!
--
Vitaly
prev parent reply other threads:[~2020-06-18 11:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-16 16:14 [PATCH] KVM: SVM: drop MSR_IA32_PERF_CAPABILITIES from emulated MSRs Vitaly Kuznetsov
2020-06-16 16:24 ` Jim Mattson
2020-06-16 16:45 ` Vitaly Kuznetsov
2020-06-16 16:52 ` Jim Mattson
2020-06-17 11:38 ` Vitaly Kuznetsov
2020-06-17 16:47 ` Jim Mattson
2020-06-17 17:17 ` Paolo Bonzini
2020-06-18 11:08 ` Vitaly Kuznetsov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87imfohbr5.fsf@vitty.brq.redhat.com \
--to=vkuznets@redhat.com \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=like.xu@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=sean.j.christopherson@intel.com \
--cc=wanpengli@tencent.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).