On 2026/07/13 16:33, Oliver Upton wrote:
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.
One wrinkle is that KVM_ARM_VCPU_PMU_V3_SET_PMU and
KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS can only be used after
KVM_ARM_VCPU_INIT. Thus, either change necessarily occurs after the
initial MDCR_EL2.HPMN reset value has been established. Userspace can
issue KVM_ARM_VCPU_INIT again to reset HPMN from the new counter count.
This is awkward, but reflects the ordering imposed by the current UAPI.
On real hardware, the counter count is fixed before the CPU is reset,
whereas the UAPI configures it after the initial vCPU reset.
Reinitializing the vCPU is at least consistent with the documented warm
reset semantics.
I'll update the next version to stop rewriting MDCR_EL2.HPMN after vCPU
initialization. This also removes the need to serialize userspace
MDCR_EL2 access or reject HPMN values.
Regards,
Akihiko Odaki
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
