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

[Mingwei Zhang <mizhang@google.com>]

On Wed, Mar 13, 2024, Paolo Bonzini wrote:
> On Wed, Mar 13, 2024 at 3:42 PM Sean Christopherson <seanjc@google.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
>