Hi,
On Fri, Jul 10, 2026 at 08:14:55PM +0900, Akihiko Odaki wrote:
> kvm_arm_set_nr_counters() updates MDCR_EL2.HPMN for every vCPU while
> holding kvm->arch.config_lock. However, KVM_SET_ONE_REG currently writes
> MDCR_EL2 through the generic sysreg path without taking the same lock.
> Concurrent PMU configuration and register restore can therefore race and
> lose updates to unrelated MDCR_EL2 bits.
Ugh, we should just stop updating MDCR_EL2.HPMN altogether. Since this
overwrites the previous value (rather than clamping it) we could discard
a legal value set by userspace.
>From the UAPI POV all we need to do is ensure the reset value is sane.
The documentation says that system registers are reset to their warm
reset values when KVM_ARM_VCPU_INIT is called. Which in this would mean
HPMN is reset to the number of implemented counters at the time of the
ioctl.
If userspace changes the number of counters afterwards, that's their
problem.
> Add explicit userspace accessors for MDCR_EL2. Serialize them with
> config_lock so whole-register userspace writes cannot race with HPMN
> rewrites, reject HPMN values above the configured PMU counter count, and
> request a PMU reload when HPME changes to match guest trap behavior.
>
> Fixes: c8823e51b534 ("KVM: arm64: Fix MDCR_EL2.HPMN reset value")
> Closes:
> https://sashiko.dev/#/patchset/20260706-hybrid-v8-0-de459617b59d%40rsg.ci.i.u-tokyo.ac.jp?part=6
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Akihiko Odaki <[email protected]>
> ---
> arch/arm64/kvm/sys_regs.c | 39 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index d217530359ba..2b2ea33159e9 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2949,6 +2949,42 @@ static bool access_mdcr(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +static int get_mdcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + u64 *val)
> +{
> + struct kvm *kvm = vcpu->kvm;
> +
> + guard(mutex)(&kvm->arch.config_lock);
Hrm... I would strongly prefer that we *not* take the config_lock for
this register since there's no way for userspace to avoid lock
contention. ID registers are special and documented as VM-scoped, so an
aware VMM could potentially set these once (avoiding the lock).
> + *val = __vcpu_sys_reg(vcpu, MDCR_EL2);
> +
> + return 0;
> +}
> +
> +static int set_mdcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + u64 val)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + u64 old, hpmn = FIELD_GET(MDCR_EL2_HPMN, val);
> +
> + guard(mutex)(&kvm->arch.config_lock);
> +
> + if (hpmn > vcpu->kvm->arch.nr_pmu_counters)
> + return -EINVAL;
KVM allows userspace to write whatever it wants right now, we can't
start rejecting values that were previously valid. The architecture also
allows anything to be written to the field, just that unimplemented
values have UNKNOWN behavior.
Thanks,
Oliver
