linux-kernel - Re: [PATCH 1/2] KVM: x86/pmu: Reset perf_capabilities in vcpu to 0 if PDCM is disabled

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: <ZbP7BTvdZ1-b3MmE@google.com>
Date: Fri, 26 Jan 2024 10:33:41 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Mingwei Zhang <mizhang@...gle.com>
Cc: Aaron Lewis <aaronlewis@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>, 
	"H. Peter Anvin" <hpa@...or.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] KVM: x86/pmu: Reset perf_capabilities in vcpu to 0 if
 PDCM is disabled

On Thu, Jan 25, 2024, Mingwei Zhang wrote:
> On Wed, Jan 24, 2024, Sean Christopherson wrote:
> > On Wed, Jan 24, 2024, Mingwei Zhang wrote:
> > > I think this makes a lot of confusions on migration where VMM on the source
> > > believes that a non-zero value from KVM_GET_MSRS is valid and the VMM on the
> > > target will find it not true.
> > 
> > Yes, but seeing a non-zero value is a KVM bug that should be fixed.
> > 
> How about adding an entry in vmx_get_msr() for
> MSR_IA32_PERF_CAPABILITIES and check pmu_version? This basically pairs
> with the implementation in vmx_set_msr() for MSR_IA32_PERF_CAPABILITIES.
> Doing so allows KVM_GET_MSRS return 0 for the MSR instead of returning
> the initial permitted value.

Hrm, I don't hate it as a stopgap.  But if we are the only people that are affected,
because again I'm pretty sure QEMU is fine, I would rather we just fix things in
our VMM and/or internal kernel.

Long term, I want some form of fix for the initialization code, even if that means
adding a quirk to let userspace opt out of KVM setting default values for platform
MSRs.

Side topic, vmx_set_msr() should check X86_FEATURE_PDCM, not just the PMU version.

> The benefit is that it is not enforcing the VMM to explicitly set the
> value. In fact, there are several platform MSRs which has initial value
> that VMM may rely on instead of explicitly setting.
> MSR_IA32_PERF_CAPABILITIES is only one of them.

Yeah, and all of those are broken.  AFAICT, the bad behavior got introduced for
MSR_PLATFORM_INFO, and then people kept copy+pasting that broken pattern :-(