On Thu, Sep 10, 2020 at 06:42:43PM +0200, Alexander Graf wrote:
> We allow user space to set the PMCR register to any value. However,
> when time comes for a vcpu reset (for example on PSCI online), PMCR
> is reset to the hardware capabilities.
>
> I would like to explicitly expose different PMU capabilities (number
> of supported event counters) to the guest than hardware supports.
> Ideally across vcpu resets.
>
> So this patch adopts the reset path to only populate the immutable
> PMCR register bits from hardware when they were not initialized
> previously. This effectively means that on a normal reset, only the
> guest settable fields are reset, while on vcpu creation the register
> gets populated from hardware like before.
>
> With this in place and a change in user space to invoke SET_ONE_REG
> on the PMCR for every vcpu, I can reliably set the PMU event counter
> number to arbitrary values.
>
> Signed-off-by: Alexander Graf <graf@xxxxxxxxxx>
> ---
> arch/arm64/kvm/sys_regs.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 20ab2a7d37ca..28f67550db7f 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -663,7 +663,14 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> {
> u64 pmcr, val;
>
> - pmcr = read_sysreg(pmcr_el0);
> + /*
> + * If we already received PMCR from a previous ONE_REG call,
> + * maintain its immutable flags
> + */
> + pmcr = __vcpu_sys_reg(vcpu, r->reg);
> + if (!__vcpu_sys_reg(vcpu, r->reg))
> + pmcr = read_sysreg(pmcr_el0);
> +
> /*
> * Writable bits of PMCR_EL0 (ARMV8_PMU_PMCR_MASK) are reset to UNKNOWN
> * except PMCR.E resetting to zero.
> --
> 2.16.4
>
Aha, a much simpler patch than I expected. With this approach we don't
need a get_user() function, or to use 'val', but don't we still want to
add sanity checks with a set_user() function? At least to ensure immutable
flags match and that PMCR_EL0.N isn't too big?
Silently changing the user's input, which I see we also do for e.g. MPIDR,
isn't super user friendly.
Thanks,
drew
