Re: [PATCH 6/9] KVM: arm64: PMU: Move the ID_AA64DFR0_EL1.PMUver limit to VM creation

From: Marc Zyngier <maz@kernel.org>
To: Reiji Watanabe <reijiw@google.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	kernel-team@android.com
Subject: Re: [PATCH 6/9] KVM: arm64: PMU: Move the ID_AA64DFR0_EL1.PMUver limit to VM creation
Date: Thu, 27 Oct 2022 18:24:35 +0100	[thread overview]
Message-ID: <989ec7a63aff44e5fe2d85f691a7f330@kernel.org> (raw)
In-Reply-To: <CAAeT=FwFS+oTG3Q0sDMyobfQst2TWUqyU4XQFmmELPS1rwp96w@mail.gmail.com>

On 2022-10-27 17:09, Reiji Watanabe wrote:
>> I think that with this patch both PMUVer and Perfmon values get set to
>> 0 (pmuver_to_perfmon() returns 0 for both ID_AA64DFR0_PMUVER_IMP_DEF
>> and no PMU at all). Am I missing anything here?
> 
> When pmuver_to_perfmon() returns 0 for ID_AA64DFR0_PMUVER_IMP_DEF,
> cpuid_feature_cap_perfmon_field() is called with 'cap' == 0.  Then,
> the code in cpuid_feature_cap_perfmon_field() updates the 'val' with 0
> if the given 'features' (sanitized) value is 
> ID_AA64DFR0_PMUVER_IMP_DEF.
> So, now the val(== 0) is not larger than the cap (== 0), and
> cpuid_feature_cap_perfmon_field() ends up returning the given 
> 'features'
> value as it is without updating the PERFMON field.

Ah, thanks for spelling it out for me, I was definitely looking
at the wrong side of things. You're absolutely right. The code
I have now makes sure to:

(1) preserve the IMP_DEF view of the PMU if userspace provides
     such setting

(2) directly places the emulated PMU revision in the feature
     set without calling cpuid_feature_cap_perfmon_field(),
     which indeed does the wrong thing.

Hopefully I got it right this time! ;-)

Thanks again,

         M.
-- 
Jazz is not dead. It just smells funny...