Received-SPF: pass (google.com: domain of linux-kernel+bounces-40495-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 26 Jan 2024 10:33:41 -0800
In-Reply-To: <ZbGn8lAj4XxiecFn@google.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240124003858.3954822-1-mizhang@google.com> <20240124003858.3954822-2-mizhang@google.com>
 <ZbExcMMl-IAzJrfx@google.com> <CAAAPnDFAvJBuETUsBScX6WqSbf_j=5h_CpWwrPHwXdBxDg_LFQ@mail.gmail.com>
 <ZbGAXpFUso9JzIjo@google.com> <ZbGOK9m6UKkQ38bK@google.com>
 <ZbGUfmn-ZAe4lkiN@google.com> <ZbGn8lAj4XxiecFn@google.com>
Message-ID: <ZbP7BTvdZ1-b3MmE@google.com>
Subject: Re: [PATCH 1/2] KVM: x86/pmu: Reset perf_capabilities in vcpu to 0 if
 PDCM is disabled
From: Sean Christopherson <seanjc@google.com>
To: Mingwei Zhang <mizhang@google.com>
Cc: Aaron Lewis <aaronlewis@google.com>, Paolo Bonzini <pbonzini@redhat.com>, 
	"H. Peter Anvin" <hpa@zytor.com>, kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="us-ascii"

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 :-(