Received-SPF: pass (google.com: domain of linux-kernel+bounces-101654-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240313003739.3365845-1-mizhang@google.com> <DS0PR11MB63731F54EA26D14CF7D6A3FDDC2A2@DS0PR11MB6373.namprd11.prod.outlook.com>
 <ZfG7SgyqTTtqF3cw@google.com>
In-Reply-To: <ZfG7SgyqTTtqF3cw@google.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed, 13 Mar 2024 16:01:34 +0100
Message-ID: <CABgObfYfAS2DBaW71iUcQgua7K3VY4nz8krGYGxyBt1+7i193A@mail.gmail.com>
Subject: Re: [PATCH] KVM: x86/pmu: Return correct value of IA32_PERF_CAPABILITIES
 for userspace after vCPU has run
To: Sean Christopherson <seanjc@google.com>
Cc: Wei W Wang <wei.w.wang@intel.com>, Mingwei Zhang <mizhang@google.com>, 
	"H. Peter Anvin" <hpa@zytor.com>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Aaron Lewis <aaronlewis@google.com>, 
	Jim Mattson <jmattson@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 13, 2024 at 3:42=E2=80=AFPM 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} pr=
ior to
>      KVM_SET_MSRS.

Correct.

>   2. Go with roughly what I proposed in the CET thread (for unsupported M=
SRS,
>      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_I=
NFO
>      (if quirk=3D=3Denabled, keep KVM's current behavior; if quirk=3D=3Dd=
isabled, zero-
>       initialize the MSRs).

More precisely, even if quirk=3D=3Denabled 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 bot=
h reads
> and writes to PERF_CAPABILITIES, and the common userspace MSR handling wi=
ll
> convert "unsupported" to "success" as appropriate.
>
> [*] https://lore.kernel.org/all/ZfDdS8rtVtyEr0UR@google.com