Re: [PATCH v2 2/5] KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems

From: Oliver Upton <oliver.upton@linux.dev>
To: Reiji Watanabe <reijiw@google.com>
Cc: Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.linux.dev, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>
Subject: Re: [PATCH v2 2/5] KVM: arm64: PMU: Disallow vPMU on non-uniform PMUVer systems
Date: Fri, 28 Jul 2023 19:52:50 +0000	[thread overview]
Message-ID: <ZMQckrDB7tg9gPfw@linux.dev> (raw)
In-Reply-To: <20230728181907.1759513-3-reijiw@google.com>

On Fri, Jul 28, 2023 at 11:19:04AM -0700, Reiji Watanabe wrote:
> Disallow userspace from configuring vPMU for guests on systems
> where the PMUVer is not uniform across all PEs.
> KVM has not been advertising PMUv3 to the guests with vPMU on
> such systems anyway, and such systems would be extremely
> uncommon and unlikely to even use KVM.

This doesn't actually disallow userspace from configuring a vPMU, it
only hides the KVM cap. kvm_host_pmu_init() will still insert the host
PMU instance in the list of valid PMUs, and there doesn't appear to be
any check against the static key anywhere on that path.

FWIW, this static key is actually responsible for indicating whether KVM
supports context switching the PMU between host/guest. While vPMU obviously
depends on that, the perf subsystem also allows the host to program events
to count while the guest is running.

I actually prefer where we flip the static key, as PMU context switching
depends on both KVM support as well as the PMU driver coming up successfully.
Instead, you could hoist the check against the sanitised PMU version into
kvm_host_pmu_init(), maybe something like:

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 560650972478..f6a0e558472f 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -672,8 +672,11 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
 {
 	struct arm_pmu_entry *entry;
 
-	if (pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_NI ||
-	    pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF)
+	/*
+	 * Check the sanitised PMU version for the system, as KVM does not
+	 * support implementations where PMUv3 exists on a subset of CPUs.
+	 */
+	if (!pmuv3_implemented(kvm_arm_pmu_get_pmuver_limit()))
 		return;
 
 	mutex_lock(&arm_pmus_lock);

-- 
Thanks,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
