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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABgObfYfAS2DBaW71iUcQgua7K3VY4nz8krGYGxyBt1+7i193A@mail.gmail.com>
Date: Wed, 13 Mar 2024 16:01:34 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Wei W Wang <wei.w.wang@...el.com>, Mingwei Zhang <mizhang@...gle.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 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.

>   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

> 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