linux-kernel - Re: [PATCH] KVM: x86/pmu: Return correct value of IA32_PERF_CAPABILITIES for userspace after vCPU has run

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: <ZfHt-waLl3mg0Lbx@google.com>
Date: Wed, 13 Mar 2024 18:18:35 +0000
From: Mingwei Zhang <mizhang@...gle.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
	Wei W Wang <wei.w.wang@...el.com>, "H. Peter Anvin" <hpa@...or.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Aaron Lewis <aaronlewis@...gle.com>,
	Jim Mattson <jmattson@...gle.com>
Subject: Re: [PATCH] KVM: x86/pmu: Return correct value of
 IA32_PERF_CAPABILITIES for userspace after vCPU has run

On Wed, Mar 13, 2024, Paolo Bonzini wrote:
> On Wed, Mar 13, 2024 at 3:42 PM Sean Christopherson <seanjc@...gle.com> wrote:
> > We discussed this whole MSRs mess at PUCK this morning.  I forgot to hit RECORD,
> > but Paolo took notes and will post them soon.
> >
> > Going from memory, the plan is to:
> >
> >   1. Commit to, and document, that userspace must do KVM_SET_CPUID{,2} prior to
> >      KVM_SET_MSRS.
> 
> Correct.

This is clear to me now. Glad to have the direction settled down.

> 
> >   2. Go with roughly what I proposed in the CET thread (for unsupported MSRS,
> >      read 0 and drop writes of '0')[*].
> 
> More precisely, read a sensible default value corresponding to
> "everything disabled", which generally speaking should be 0. And
> generally speaking, commit to:
> - allowing host_initiated reads independent of CPUID
> - allowing host_initiated writes of the same value that was read
> - blocking host_initiated writes of nonzero (or nondefault) values if
> the corresponding guest CPUID bit is clear

>
> Right now some MSRs do not allow host initiated writes, for example
> MSR_KVM_* (check for calls to guest_pv_has), and the VMX MSRs.
> 
> Generally speaking we want to fix them, unless it's an unholy pain
> (for example the VMX capabilities MSRs are good candidates for pain,
> because they have some "must be 1" bits in bits 63:32).
> 
> All this should be covered by selftests.
> 
> >   3. Add a quire for PERF_CAPABILITIES, ARCH_CAPABILITIES, and PLATFORM_INFO
> >      (if quirk==enabled, keep KVM's current behavior; if quirk==disabled, zero-
> >       initialize the MSRs).
> 
> More precisely, even if quirk==enabled we will move the setting of a
> non-zero default value for the MSR from vCPU creation to
> KVM_SET_CPUID2, and only set a non-zero default value if the CPUID bit
> is set.
> 
> Another small thing in my notes was to look at the duplication between
> emulated_msrs and msr_based_features_all_except_vmx. Right now
> MSR_AMD64_DE_CFG is the only one that is not in both and, probably not
> a coincidence, it's also the only one implemented only for one vendor.
> There's probably some opportunity for both cleanups and fixes. It
> looks like svm_has_emulated_msr(MSR_AMD64_DE_CFG) should return true
> for example.
> 
> Paolo
> 

Ack. Thanks.

> > With those pieces in place, KVM can simply check X86_FEATURE_PDCM for both reads
> > and writes to PERF_CAPABILITIES, and the common userspace MSR handling will
> > convert "unsupported" to "success" as appropriate.
> >
> > [*] https://lore.kernel.org/all/ZfDdS8rtVtyEr0UR@google.com
>