Re: [PATCH v2 07/23] perf: arm_pmuv3: Introduce method to partition the PMU

[Oliver Upton <oliver.upton@linux.dev>]

On Fri, Jun 20, 2025 at 10:13:07PM +0000, Colton Lewis wrote:
> For PMUv3, the register field MDCR_EL2.HPMN partitiones the PMU
> counters into two ranges where counters 0..HPMN-1 are accessible by
> EL1 and, if allowed, EL0 while counters HPMN..N are only accessible by
> EL2.
> 
> Create module parameters partition_pmu and reserved_guest_counters to
> reserve a number of counters for the guest. These numbers are set at
> boot because the perf subsystem assumes the number of counters will
> not change after the PMU is probed.
> 
> Introduce the function armv8pmu_partition() to modify the PMU driver's
> cntr_mask of available counters to exclude the counters being reserved
> for the guest and record reserved_guest_counters as the maximum
> allowable value for HPMN.
> 
> Due to the difficulty this feature would create for the driver running
> at EL1 on the host, partitioning is only allowed in VHE mode. Working
> on nVHE mode would require a hypercall for every counter access in the
> driver because the counters reserved for the host by HPMN are only
> accessible to EL2.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  arch/arm/include/asm/arm_pmuv3.h   | 10 ++++
>  arch/arm64/include/asm/arm_pmuv3.h |  5 ++
>  drivers/perf/arm_pmuv3.c           | 95 +++++++++++++++++++++++++++++-
>  include/linux/perf/arm_pmu.h       |  1 +
>  4 files changed, 109 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
> index 2ec0e5e83fc9..9dc43242538c 100644
> --- a/arch/arm/include/asm/arm_pmuv3.h
> +++ b/arch/arm/include/asm/arm_pmuv3.h
> @@ -228,6 +228,11 @@ static inline bool kvm_set_pmuserenr(u64 val)
>  
>  static inline void kvm_vcpu_pmu_resync_el0(void) {}
>  
> +static inline bool has_vhe(void)
> +{
> +	return false;
> +}
> +

This has nothing to do with PMUv3, I'm a bit surprised to see you're
touching 32-bit ARM. Can you just gate the whole partitioning thing on
arm64?

> +static bool partition_pmu __read_mostly;
> +static u8 reserved_guest_counters __read_mostly;
> +
> +module_param(partition_pmu, bool, 0);
> +MODULE_PARM_DESC(partition_pmu,
> +		 "Partition the PMU into host and guest VM counters [y/n]");
> +
> +module_param(reserved_guest_counters, byte, 0);
> +MODULE_PARM_DESC(reserved_guest_counters,
> +		 "How many counters to reserve for guest VMs [0-$NR_COUNTERS]");
> +

This is confusing and not what we discussed offline.

Please use a single parameter that describes the number of counters used
by the *host*. This affects the *host* PMU driver, KVM can discover (and
use) the leftovers.

If the single module parameter goes unspecified the user did not ask for
PMU partitioning.

> +/**
> + * armv8pmu_reservation_is_valid() - Determine if reservation is allowed
> + * @guest_counters: Number of host counters to reserve
> + *
> + * Determine if the number of host counters in the argument is
> + * allowed. It is allowed if it will produce a valid value for
> + * register field MDCR_EL2.HPMN.
> + *
> + * Return: True if reservation allowed, false otherwise
> + */
> +static bool armv8pmu_reservation_is_valid(u8 guest_counters)
> +{
> +	return guest_counters <= armv8pmu_pmcr_n_read();
> +}
> +
> +/**
> + * armv8pmu_partition_supported() - Determine if partitioning is possible
> + *
> + * Partitioning is only supported in VHE mode (with PMUv3, assumed
> + * since we are in the PMUv3 driver)
> + *
> + * Return: True if partitioning is possible, false otherwise
> + */
> +static bool armv8pmu_partition_supported(void)
> +{
> +	return has_vhe();
> +}
> +
> +/**
> + * armv8pmu_partition() - Partition the PMU
> + * @pmu: Pointer to pmu being partitioned
> + * @guest_counters: Number of host counters to reserve
> + *
> + * Partition the given PMU by taking a number of host counters to
> + * reserve and, if it is a valid reservation, recording the
> + * corresponding HPMN value in the hpmn field of the PMU and clearing
> + * the guest-reserved counters from the counter mask.
> + *
> + * Passing 0 for @guest_counters has the effect of disabling partitioning.
> + *
> + * Return: 0 on success, -ERROR otherwise
> + */
> +static int armv8pmu_partition(struct arm_pmu *pmu, u8 guest_counters)
> +{
> +	u8 nr_counters;
> +	u8 hpmn;
> +
> +	if (!armv8pmu_reservation_is_valid(guest_counters))
> +		return -EINVAL;
> +
> +	nr_counters = armv8pmu_pmcr_n_read();
> +	hpmn = guest_counters;
> +
> +	pmu->hpmn_max = hpmn;

I'm not sure the host driver needs this for anything, KVM just needs to
know what's potentially in use by the host.

> +	/* Inform host driver of available counters */

... said the driver to itself :)

Thanks,
Oliver