Re: [PATCH v2 3/8] KVM: arm64: PMU: Preserve vCPU's PMCR_EL0.N value on vCPU reset

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

On Mon, Jan 16, 2023 at 05:35:37PM -0800, Reiji Watanabe wrote:
> The number of PMU event counters is indicated in PMCR_EL0.N.
> For a vCPU with PMUv3 configured, its value will be the same as
> the host value by default. Userspace can set PMCR_EL0.N for the
> vCPU to a lower value than the host value using KVM_SET_ONE_REG.
> However, it is practically unsupported, as reset_pmcr() resets
> PMCR_EL0.N to the host value on vCPU reset.
> 
> Change reset_pmcr() to preserve the vCPU's PMCR_EL0.N value on
> vCPU reset so that userspace can limit the number of the PMU
> event counter on the vCPU.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/kvm/pmu-emul.c | 6 ++++++
>  arch/arm64/kvm/sys_regs.c | 4 +++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 24908400e190..937a272b00a5 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -213,6 +213,12 @@ void kvm_pmu_vcpu_init(struct kvm_vcpu *vcpu)
>  
>  	for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++)
>  		pmu->pmc[i].idx = i;
> +
> +	/*
> +	 * Initialize PMCR_EL0 for the vCPU with the host value so that
> +	 * the value is available at the very first vCPU reset.
> +	 */
> +	__vcpu_sys_reg(vcpu, PMCR_EL0) = read_sysreg(pmcr_el0);

I think we need to derive a sanitised value for PMCR_EL0.N, as I believe
nothing in the architecture prevents implementers from gluing together
cores with varying numbers of PMCs. We probably haven't noticed it yet
since it would appear all Arm designs have had 6 PMCs.

>  }
>  
>  /**
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 4959658b502c..67c1bd39b478 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -637,8 +637,10 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>  	if (!kvm_arm_support_pmu_v3())
>  		return;
>  
> +	/* PMCR_EL0 for the vCPU is set to the host value at vCPU creation. */
> +

nit: I think we can do without the floating comment here.

--
Thanks,
Oliver