linux-kernel - Re: [PATCH] KVM: SVM: drop MSR_IA32_PERF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87imfohbr5.fsf@vitty.brq.redhat.com>
Date:   Thu, 18 Jun 2020 13:08:46 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Jim Mattson <jmattson@...gle.com>
Cc:     kvm list <kvm@...r.kernel.org>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Like Xu <like.xu@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM: SVM: drop MSR_IA32_PERF_CAPABILITIES from emulated MSRs

Paolo Bonzini <pbonzini@...hat.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