[PATCH 00/13] KVM: arm64: De-specialise the timer UAPI

[PATCH 00/13] KVM: arm64: De-specialise the timer UAPI

[Marc Zyngier]

Since the beginning of the KVM/arm64 port, the timer registers were
handled out of the normal sysreg flow when it came to userspace
access, leading to extra complexity and a bit of code duplication.

When NV was introduced, the decision was made early to handle the new
timer registers as part of the generic infrastructure. However, the
EL0 timers were left behind until someone could be bothered to
entangle that mess.

Said mess is more complicated than it looks, due to a nasty bug
documented in 290a6bb06de9e ("arm64: KVM: Add UAPI notes for swapped
registers"), where it was realised that CNTV_CVAL_EL0 and CNTVCT_EL0
have had their encoding swapped at the user interface level. Handling
of this issue is spread all over the place instead of being contained
in a single location, and it needs to be contained.

Finally, it was noticed that we expose the CNTHV_*_EL2 registers to
userspace for nVHE guest, while the architecture is clear that they do
not exist in that configuration.

This series aims at fixing all of the above, moving the handling of
the timer sysregs to sys_regs.c, fix a corner case with WFxT, handle
the nVHE issue described above, and finally improve the testing by
introducing an E2H==0 configuration.

If excluding the selftests, this is a net deletion of code. What's not
to like?

Marc Zyngier (13):
  KVM: arm64: Hide CNTHV_*_EL2 from userspace for nVHE guests
  KVM: arm64: Introduce timer_context_to_vcpu() helper
  KVM: arm64: Replace timer context vcpu pointer with timer_id
  KVM: arm64: Make timer_set_offset() generally accessible
  KVM: arm64: Add timer UAPI workaround to sysreg infrastructure
  KVM: arm64: Move CNT*_CTL_EL0 userspace accessors to generic
    infrastructure
  KVM: arm64: Move CNT*_CVAL_EL0 userspace accessors to generic
    infrastructure
  KVM: arm64: Move CNT*CT_EL0 userspace accessors to generic
    infrastructure
  KVM: arm64: Fix WFxT handling of nested virt
  KVM: arm64: Kill leftovers of ad-hoc timer userspace access
  KVM: arm64: selftests: Make dependencies on VHE-specific registers
    explicit
  KVM: arm64: selftests: Add an E2H=0-specific configuration to
    get_reg_list
  KVM: arm64: selftest: Fix misleading comment about virtual timer
    encoding

 arch/arm64/kvm/arch_timer.c                   | 105 ++-------------
 arch/arm64/kvm/guest.c                        |  70 ----------
 arch/arm64/kvm/handle_exit.c                  |   7 +-
 arch/arm64/kvm/sys_regs.c                     | 123 +++++++++++++++---
 arch/arm64/kvm/sys_regs.h                     |   6 +
 include/kvm/arm_arch_timer.h                  |  24 ++--
 .../selftests/kvm/arm64/get-reg-list.c        |  99 +++++++++++++-
 7 files changed, 240 insertions(+), 194 deletions(-)

-- 
2.47.3

[PATCH 01/13] KVM: arm64: Hide CNTHV_*_EL2 from userspace for nVHE guests

[Marc Zyngier]

Although we correctly UNDEF any CNTHV_*_EL2 access from the guest
when E2H==0, we still expose these registers to userspace, which
is a bad idea.

Drop the ad-hoc UNDEF injection and switch to a .visibility()
callback which will also hide the register from userspace.

Fixes: 0e45981028550 ("KVM: arm64: timer: Don't adjust the EL2 virtual timer offset")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index ee8a7033c85bf..9f2f4e0b042e8 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1594,16 +1594,6 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
 	return true;
 }
 
-static bool access_hv_timer(struct kvm_vcpu *vcpu,
-			    struct sys_reg_params *p,
-			    const struct sys_reg_desc *r)
-{
-	if (!vcpu_el2_e2h_is_set(vcpu))
-		return undef_access(vcpu, p, r);
-
-	return access_arch_timer(vcpu, p, r);
-}
-
 static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
 				    s64 new, s64 cur)
 {
@@ -2831,6 +2821,16 @@ static unsigned int s1pie_el2_visibility(const struct kvm_vcpu *vcpu,
 	return __el2_visibility(vcpu, rd, s1pie_visibility);
 }
 
+static unsigned int cnthv_visibility(const struct kvm_vcpu *vcpu,
+				     const struct sys_reg_desc *rd)
+{
+	if (vcpu_has_nv(vcpu) &&
+	    !vcpu_has_feature(vcpu, KVM_ARM_VCPU_HAS_EL2_E2H0))
+		return 0;
+
+	return REG_HIDDEN;
+}
+
 static bool access_mdcr(struct kvm_vcpu *vcpu,
 			struct sys_reg_params *p,
 			const struct sys_reg_desc *r)
@@ -3691,9 +3691,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(CNTHP_CTL_EL2, access_arch_timer, reset_val, 0),
 	EL2_REG(CNTHP_CVAL_EL2, access_arch_timer, reset_val, 0),
 
-	{ SYS_DESC(SYS_CNTHV_TVAL_EL2), access_hv_timer },
-	EL2_REG(CNTHV_CTL_EL2, access_hv_timer, reset_val, 0),
-	EL2_REG(CNTHV_CVAL_EL2, access_hv_timer, reset_val, 0),
+	{ SYS_DESC(SYS_CNTHV_TVAL_EL2), access_arch_timer, .visibility = cnthv_visibility },
+	EL2_REG_FILTERED(CNTHV_CTL_EL2, access_arch_timer, reset_val, 0, cnthv_visibility),
+	EL2_REG_FILTERED(CNTHV_CVAL_EL2, access_arch_timer, reset_val, 0, cnthv_visibility),
 
 	{ SYS_DESC(SYS_CNTKCTL_EL12), access_cntkctl_el12 },
 
-- 
2.47.3

[PATCH 02/13] KVM: arm64: Introduce timer_context_to_vcpu() helper

[Marc Zyngier]

We currently have a vcpu pointer nested into each timer context.

As we are about to remove this pointer, introduce a helper (aptly
named timer_context_to_vcpu()) that returns this pointer, at least
until we repaint the data structure.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/arch_timer.c  | 25 +++++++++++++------------
 include/kvm/arm_arch_timer.h |  2 +-
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index dbd74e4885e24..e5a25e743f5be 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -66,7 +66,7 @@ static int nr_timers(struct kvm_vcpu *vcpu)
 
 u32 timer_get_ctl(struct arch_timer_context *ctxt)
 {
-	struct kvm_vcpu *vcpu = ctxt->vcpu;
+	struct kvm_vcpu *vcpu = timer_context_to_vcpu(ctxt);
 
 	switch(arch_timer_ctx_index(ctxt)) {
 	case TIMER_VTIMER:
@@ -85,7 +85,7 @@ u32 timer_get_ctl(struct arch_timer_context *ctxt)
 
 u64 timer_get_cval(struct arch_timer_context *ctxt)
 {
-	struct kvm_vcpu *vcpu = ctxt->vcpu;
+	struct kvm_vcpu *vcpu = timer_context_to_vcpu(ctxt);
 
 	switch(arch_timer_ctx_index(ctxt)) {
 	case TIMER_VTIMER:
@@ -104,7 +104,7 @@ u64 timer_get_cval(struct arch_timer_context *ctxt)
 
 static void timer_set_ctl(struct arch_timer_context *ctxt, u32 ctl)
 {
-	struct kvm_vcpu *vcpu = ctxt->vcpu;
+	struct kvm_vcpu *vcpu = timer_context_to_vcpu(ctxt);
 
 	switch(arch_timer_ctx_index(ctxt)) {
 	case TIMER_VTIMER:
@@ -126,7 +126,7 @@ static void timer_set_ctl(struct arch_timer_context *ctxt, u32 ctl)
 
 static void timer_set_cval(struct arch_timer_context *ctxt, u64 cval)
 {
-	struct kvm_vcpu *vcpu = ctxt->vcpu;
+	struct kvm_vcpu *vcpu = timer_context_to_vcpu(ctxt);
 
 	switch(arch_timer_ctx_index(ctxt)) {
 	case TIMER_VTIMER:
@@ -343,7 +343,7 @@ static enum hrtimer_restart kvm_hrtimer_expire(struct hrtimer *hrt)
 	u64 ns;
 
 	ctx = container_of(hrt, struct arch_timer_context, hrtimer);
-	vcpu = ctx->vcpu;
+	vcpu = timer_context_to_vcpu(ctx);
 
 	trace_kvm_timer_hrtimer_expire(ctx);
 
@@ -436,8 +436,9 @@ static void kvm_timer_update_status(struct arch_timer_context *ctx, bool level)
 	 *
 	 * But hey, it's fast, right?
 	 */
-	if (is_hyp_ctxt(ctx->vcpu) &&
-	    (ctx == vcpu_vtimer(ctx->vcpu) || ctx == vcpu_ptimer(ctx->vcpu))) {
+	struct kvm_vcpu *vcpu = timer_context_to_vcpu(ctx);
+	if (is_hyp_ctxt(vcpu) &&
+	    (ctx == vcpu_vtimer(vcpu) || ctx == vcpu_ptimer(vcpu))) {
 		unsigned long val = timer_get_ctl(ctx);
 		__assign_bit(__ffs(ARCH_TIMER_CTRL_IT_STAT), &val, level);
 		timer_set_ctl(ctx, val);
@@ -470,7 +471,7 @@ static void timer_emulate(struct arch_timer_context *ctx)
 	trace_kvm_timer_emulate(ctx, should_fire);
 
 	if (should_fire != ctx->irq.level)
-		kvm_timer_update_irq(ctx->vcpu, should_fire, ctx);
+		kvm_timer_update_irq(timer_context_to_vcpu(ctx), should_fire, ctx);
 
 	kvm_timer_update_status(ctx, should_fire);
 
@@ -498,7 +499,7 @@ static void set_cntpoff(u64 cntpoff)
 
 static void timer_save_state(struct arch_timer_context *ctx)
 {
-	struct arch_timer_cpu *timer = vcpu_timer(ctx->vcpu);
+	struct arch_timer_cpu *timer = vcpu_timer(timer_context_to_vcpu(ctx));
 	enum kvm_arch_timers index = arch_timer_ctx_index(ctx);
 	unsigned long flags;
 
@@ -609,7 +610,7 @@ static void kvm_timer_unblocking(struct kvm_vcpu *vcpu)
 
 static void timer_restore_state(struct arch_timer_context *ctx)
 {
-	struct arch_timer_cpu *timer = vcpu_timer(ctx->vcpu);
+	struct arch_timer_cpu *timer = vcpu_timer(timer_context_to_vcpu(ctx));
 	enum kvm_arch_timers index = arch_timer_ctx_index(ctx);
 	unsigned long flags;
 
@@ -668,7 +669,7 @@ static inline void set_timer_irq_phys_active(struct arch_timer_context *ctx, boo
 
 static void kvm_timer_vcpu_load_gic(struct arch_timer_context *ctx)
 {
-	struct kvm_vcpu *vcpu = ctx->vcpu;
+	struct kvm_vcpu *vcpu = timer_context_to_vcpu(ctx);
 	bool phys_active = false;
 
 	/*
@@ -677,7 +678,7 @@ static void kvm_timer_vcpu_load_gic(struct arch_timer_context *ctx)
 	 * this point and the register restoration, we'll take the
 	 * interrupt anyway.
 	 */
-	kvm_timer_update_irq(ctx->vcpu, kvm_timer_should_fire(ctx), ctx);
+	kvm_timer_update_irq(vcpu, kvm_timer_should_fire(ctx), ctx);
 
 	if (irqchip_in_kernel(vcpu->kvm))
 		phys_active = kvm_vgic_map_is_active(vcpu, timer_irq(ctx));
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index 681cf0c8b9df4..d188c716d03cb 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -128,7 +128,7 @@ void kvm_timer_init_vhe(void);
 #define vcpu_hptimer(v)	(&(v)->arch.timer_cpu.timers[TIMER_HPTIMER])
 
 #define arch_timer_ctx_index(ctx)	((ctx) - vcpu_timer((ctx)->vcpu)->timers)
-
+#define timer_context_to_vcpu(ctx)	((ctx)->vcpu)
 #define timer_vm_data(ctx)		(&(ctx)->vcpu->kvm->arch.timer_data)
 #define timer_irq(ctx)			(timer_vm_data(ctx)->ppi[arch_timer_ctx_index(ctx)])
 
-- 
2.47.3

[PATCH 03/13] KVM: arm64: Replace timer context vcpu pointer with timer_id

[Marc Zyngier]

Having to follow a pointer to a vcpu is pretty dumb, when the timers
are are a fixed offset in the vcpu structure itself.

Trade the vcpu pointer for a timer_id, which can then be used to
compute the vcpu address as needed.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/arch_timer.c  |  4 ++--
 include/kvm/arm_arch_timer.h | 11 ++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index e5a25e743f5be..c832c293676a3 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -149,7 +149,7 @@ static void timer_set_cval(struct arch_timer_context *ctxt, u64 cval)
 static void timer_set_offset(struct arch_timer_context *ctxt, u64 offset)
 {
 	if (!ctxt->offset.vm_offset) {
-		WARN(offset, "timer %ld\n", arch_timer_ctx_index(ctxt));
+		WARN(offset, "timer %d\n", arch_timer_ctx_index(ctxt));
 		return;
 	}
 
@@ -1064,7 +1064,7 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)
 	struct arch_timer_context *ctxt = vcpu_get_timer(vcpu, timerid);
 	struct kvm *kvm = vcpu->kvm;
 
-	ctxt->vcpu = vcpu;
+	ctxt->timer_id = timerid;
 
 	if (timerid == TIMER_VTIMER)
 		ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset;
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index d188c716d03cb..d8e400cb2bfff 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -51,8 +51,6 @@ struct arch_timer_vm_data {
 };
 
 struct arch_timer_context {
-	struct kvm_vcpu			*vcpu;
-
 	/* Emulated Timer (may be unused) */
 	struct hrtimer			hrtimer;
 	u64				ns_frac;
@@ -71,6 +69,9 @@ struct arch_timer_context {
 		bool			level;
 	} irq;
 
+	/* Who am I? */
+	enum kvm_arch_timers		timer_id;
+
 	/* Duplicated state from arch_timer.c for convenience */
 	u32				host_timer_irq;
 };
@@ -127,9 +128,9 @@ void kvm_timer_init_vhe(void);
 #define vcpu_hvtimer(v)	(&(v)->arch.timer_cpu.timers[TIMER_HVTIMER])
 #define vcpu_hptimer(v)	(&(v)->arch.timer_cpu.timers[TIMER_HPTIMER])
 
-#define arch_timer_ctx_index(ctx)	((ctx) - vcpu_timer((ctx)->vcpu)->timers)
-#define timer_context_to_vcpu(ctx)	((ctx)->vcpu)
-#define timer_vm_data(ctx)		(&(ctx)->vcpu->kvm->arch.timer_data)
+#define arch_timer_ctx_index(ctx)	((ctx)->timer_id)
+#define timer_context_to_vcpu(ctx)	container_of((ctx), struct kvm_vcpu, arch.timer_cpu.timers[(ctx)->timer_id])
+#define timer_vm_data(ctx)		(&(timer_context_to_vcpu(ctx)->kvm->arch.timer_data))
 #define timer_irq(ctx)			(timer_vm_data(ctx)->ppi[arch_timer_ctx_index(ctx)])
 
 u64 kvm_arm_timer_read_sysreg(struct kvm_vcpu *vcpu,
-- 
2.47.3

[PATCH 04/13] KVM: arm64: Make timer_set_offset() generally accessible

[Marc Zyngier]

Move the timer_set_offset() helper to arm_arch_timer.h, so that it
is next to timer_get_offset(), and accessible by the rest of KVM.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/arch_timer.c  | 10 ----------
 include/kvm/arm_arch_timer.h | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index c832c293676a3..27662a3a3043e 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -146,16 +146,6 @@ static void timer_set_cval(struct arch_timer_context *ctxt, u64 cval)
 	}
 }
 
-static void timer_set_offset(struct arch_timer_context *ctxt, u64 offset)
-{
-	if (!ctxt->offset.vm_offset) {
-		WARN(offset, "timer %d\n", arch_timer_ctx_index(ctxt));
-		return;
-	}
-
-	WRITE_ONCE(*ctxt->offset.vm_offset, offset);
-}
-
 u64 kvm_phys_timer_read(void)
 {
 	return timecounter->cc->read(timecounter->cc);
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index d8e400cb2bfff..5f7f2ed8817c5 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -179,4 +179,14 @@ static inline u64 timer_get_offset(struct arch_timer_context *ctxt)
 	return offset;
 }
 
+static inline void timer_set_offset(struct arch_timer_context *ctxt, u64 offset)
+{
+	if (!ctxt->offset.vm_offset) {
+		WARN(offset, "timer %d\n", arch_timer_ctx_index(ctxt));
+		return;
+	}
+
+	WRITE_ONCE(*ctxt->offset.vm_offset, offset);
+}
+
 #endif
-- 
2.47.3

[PATCH 05/13] KVM: arm64: Add timer UAPI workaround to sysreg infrastructure

[Marc Zyngier]

Amongst the numerous bugs that plague the KVM/arm64 UAPI, one of
the most annoying thing is that the userspace view of the virtual
timer has its CVAL and CNT encodings swapped.

In order to reduce the amount of code that has to know about this,
start by adding handling for this bug in the sys_reg code.

Nothing is making use of it yet, as the code responsible for userspace
interaction is catching the accesses early.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 33 ++++++++++++++++++++++++++++++---
 arch/arm64/kvm/sys_regs.h |  6 ++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 9f2f4e0b042e8..8e6f50f54b4bf 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -5231,15 +5231,28 @@ static int demux_c15_set(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
 	}
 }
 
+static u64 kvm_one_reg_to_id(const struct kvm_one_reg *reg)
+{
+	switch(reg->id) {
+	case KVM_REG_ARM_TIMER_CVAL:
+		return TO_ARM64_SYS_REG(CNTV_CVAL_EL0);
+	case KVM_REG_ARM_TIMER_CNT:
+		return TO_ARM64_SYS_REG(CNTVCT_EL0);
+	default:
+		return reg->id;
+	}
+}
+
 int kvm_sys_reg_get_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,
 			 const struct sys_reg_desc table[], unsigned int num)
 {
 	u64 __user *uaddr = (u64 __user *)(unsigned long)reg->addr;
 	const struct sys_reg_desc *r;
+	u64 id = kvm_one_reg_to_id(reg);
 	u64 val;
 	int ret;
 
-	r = id_to_sys_reg_desc(vcpu, reg->id, table, num);
+	r = id_to_sys_reg_desc(vcpu, id, table, num);
 	if (!r || sysreg_hidden(vcpu, r))
 		return -ENOENT;
 
@@ -5272,13 +5285,14 @@ int kvm_sys_reg_set_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,
 {
 	u64 __user *uaddr = (u64 __user *)(unsigned long)reg->addr;
 	const struct sys_reg_desc *r;
+	u64 id = kvm_one_reg_to_id(reg);
 	u64 val;
 	int ret;
 
 	if (get_user(val, uaddr))
 		return -EFAULT;
 
-	r = id_to_sys_reg_desc(vcpu, reg->id, table, num);
+	r = id_to_sys_reg_desc(vcpu, id, table, num);
 	if (!r || sysreg_hidden(vcpu, r))
 		return -ENOENT;
 
@@ -5338,10 +5352,23 @@ static u64 sys_reg_to_index(const struct sys_reg_desc *reg)
 
 static bool copy_reg_to_user(const struct sys_reg_desc *reg, u64 __user **uind)
 {
+	u64 idx;
+
 	if (!*uind)
 		return true;
 
-	if (put_user(sys_reg_to_index(reg), *uind))
+	switch (reg_to_encoding(reg)) {
+	case SYS_CNTV_CVAL_EL0:
+		idx = KVM_REG_ARM_TIMER_CVAL;
+		break;
+	case SYS_CNTVCT_EL0:
+		idx = KVM_REG_ARM_TIMER_CNT;
+		break;
+	default:
+		idx = sys_reg_to_index(reg);
+	}
+
+	if (put_user(idx, *uind))
 		return false;
 
 	(*uind)++;
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index 317abc490368d..b3f904472fac5 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -257,4 +257,10 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu);
 	(val);								       \
 })
 
+#define TO_ARM64_SYS_REG(r)	ARM64_SYS_REG(sys_reg_Op0(SYS_ ## r),	\
+					      sys_reg_Op1(SYS_ ## r),	\
+					      sys_reg_CRn(SYS_ ## r),	\
+					      sys_reg_CRm(SYS_ ## r),	\
+					      sys_reg_Op2(SYS_ ## r))
+
 #endif /* __ARM64_KVM_SYS_REGS_LOCAL_H__ */
-- 
2.47.3

[PATCH 06/13] KVM: arm64: Move CNT*_CTL_EL0 userspace accessors to generic infrastructure

[Marc Zyngier]

Remove the handling of CNT*_CTL_EL0 from guest.c, and move it to
sys_regs.c, using a new TIMER_REG() definition to encapsulate it.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/guest.c    |  4 ----
 arch/arm64/kvm/sys_regs.c | 36 +++++++++++++++++++++++++++++++-----
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 16ba5e9ac86c3..dea648706fd52 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -592,10 +592,8 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
 }
 
 static const u64 timer_reg_list[] = {
-	KVM_REG_ARM_TIMER_CTL,
 	KVM_REG_ARM_TIMER_CNT,
 	KVM_REG_ARM_TIMER_CVAL,
-	KVM_REG_ARM_PTIMER_CTL,
 	KVM_REG_ARM_PTIMER_CNT,
 	KVM_REG_ARM_PTIMER_CVAL,
 };
@@ -605,10 +603,8 @@ static const u64 timer_reg_list[] = {
 static bool is_timer_reg(u64 index)
 {
 	switch (index) {
-	case KVM_REG_ARM_TIMER_CTL:
 	case KVM_REG_ARM_TIMER_CNT:
 	case KVM_REG_ARM_TIMER_CVAL:
-	case KVM_REG_ARM_PTIMER_CTL:
 	case KVM_REG_ARM_PTIMER_CNT:
 	case KVM_REG_ARM_PTIMER_CVAL:
 		return true;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 8e6f50f54b4bf..d97aacf4c1dc9 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1594,6 +1594,23 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+static int arch_timer_set_user(struct kvm_vcpu *vcpu,
+			       const struct sys_reg_desc *rd,
+			       u64 val)
+{
+	switch (reg_to_encoding(rd)) {
+	case SYS_CNTV_CTL_EL0:
+	case SYS_CNTP_CTL_EL0:
+	case SYS_CNTHV_CTL_EL2:
+	case SYS_CNTHP_CTL_EL2:
+		val &= ~ARCH_TIMER_CTRL_IT_STAT;
+		break;
+	}
+
+	__vcpu_assign_sys_reg(vcpu, rd->reg, val);
+	return 0;
+}
+
 static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
 				    s64 new, s64 cur)
 {
@@ -2496,15 +2513,20 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
 			"trap of EL2 register redirected to EL1");
 }
 
-#define EL2_REG_FILTERED(name, acc, rst, v, filter) {	\
+#define SYS_REG_USER_FILTER(name, acc, rst, v, gu, su, filter) { \
 	SYS_DESC(SYS_##name),			\
 	.access = acc,				\
 	.reset = rst,				\
 	.reg = name,				\
+	.get_user = gu,				\
+	.set_user = su,				\
 	.visibility = filter,			\
 	.val = v,				\
 }
 
+#define EL2_REG_FILTERED(name, acc, rst, v, filter)	\
+	SYS_REG_USER_FILTER(name, acc, rst, v, NULL, NULL, filter)
+
 #define EL2_REG(name, acc, rst, v)			\
 	EL2_REG_FILTERED(name, acc, rst, v, el2_visibility)
 
@@ -2515,6 +2537,10 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
 	EL2_REG_VNCR_FILT(name, hidden_visibility)
 #define EL2_REG_REDIR(name, rst, v)	EL2_REG(name, bad_redir_trap, rst, v)
 
+#define TIMER_REG(name, vis)					   \
+	SYS_REG_USER_FILTER(name, access_arch_timer, reset_val, 0, \
+			    NULL, arch_timer_set_user, vis)
+
 /*
  * Since reset() callback and field val are not used for idregs, they will be
  * used for specific purposes for idregs.
@@ -3485,11 +3511,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_CNTPCTSS_EL0), access_arch_timer },
 	{ SYS_DESC(SYS_CNTVCTSS_EL0), access_arch_timer },
 	{ SYS_DESC(SYS_CNTP_TVAL_EL0), access_arch_timer },
-	{ SYS_DESC(SYS_CNTP_CTL_EL0), access_arch_timer },
+	TIMER_REG(CNTP_CTL_EL0, NULL),
 	{ SYS_DESC(SYS_CNTP_CVAL_EL0), access_arch_timer },
 
 	{ SYS_DESC(SYS_CNTV_TVAL_EL0), access_arch_timer },
-	{ SYS_DESC(SYS_CNTV_CTL_EL0), access_arch_timer },
+	TIMER_REG(CNTV_CTL_EL0, NULL),
 	{ SYS_DESC(SYS_CNTV_CVAL_EL0), access_arch_timer },
 
 	/* PMEVCNTRn_EL0 */
@@ -3688,11 +3714,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG_VNCR(CNTVOFF_EL2, reset_val, 0),
 	EL2_REG(CNTHCTL_EL2, access_rw, reset_val, 0),
 	{ SYS_DESC(SYS_CNTHP_TVAL_EL2), access_arch_timer },
-	EL2_REG(CNTHP_CTL_EL2, access_arch_timer, reset_val, 0),
+	TIMER_REG(CNTHP_CTL_EL2, el2_visibility),
 	EL2_REG(CNTHP_CVAL_EL2, access_arch_timer, reset_val, 0),
 
 	{ SYS_DESC(SYS_CNTHV_TVAL_EL2), access_arch_timer, .visibility = cnthv_visibility },
-	EL2_REG_FILTERED(CNTHV_CTL_EL2, access_arch_timer, reset_val, 0, cnthv_visibility),
+	TIMER_REG(CNTHV_CTL_EL2, cnthv_visibility),
 	EL2_REG_FILTERED(CNTHV_CVAL_EL2, access_arch_timer, reset_val, 0, cnthv_visibility),
 
 	{ SYS_DESC(SYS_CNTKCTL_EL12), access_cntkctl_el12 },
-- 
2.47.3

[PATCH 07/13] KVM: arm64: Move CNT*_CVAL_EL0 userspace accessors to generic infrastructure

[Marc Zyngier]

As for the control registers, move the comparator registers to
the common infrastructure.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/guest.c    | 4 ----
 arch/arm64/kvm/sys_regs.c | 8 ++++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index dea648706fd52..c23ec9be4ce27 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -593,9 +593,7 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
 
 static const u64 timer_reg_list[] = {
 	KVM_REG_ARM_TIMER_CNT,
-	KVM_REG_ARM_TIMER_CVAL,
 	KVM_REG_ARM_PTIMER_CNT,
-	KVM_REG_ARM_PTIMER_CVAL,
 };
 
 #define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)
@@ -604,9 +602,7 @@ static bool is_timer_reg(u64 index)
 {
 	switch (index) {
 	case KVM_REG_ARM_TIMER_CNT:
-	case KVM_REG_ARM_TIMER_CVAL:
 	case KVM_REG_ARM_PTIMER_CNT:
-	case KVM_REG_ARM_PTIMER_CVAL:
 		return true;
 	}
 	return false;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d97aacf4c1dc9..68e88d5c0dfb5 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3512,11 +3512,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_CNTVCTSS_EL0), access_arch_timer },
 	{ SYS_DESC(SYS_CNTP_TVAL_EL0), access_arch_timer },
 	TIMER_REG(CNTP_CTL_EL0, NULL),
-	{ SYS_DESC(SYS_CNTP_CVAL_EL0), access_arch_timer },
+	TIMER_REG(CNTP_CVAL_EL0, NULL),
 
 	{ SYS_DESC(SYS_CNTV_TVAL_EL0), access_arch_timer },
 	TIMER_REG(CNTV_CTL_EL0, NULL),
-	{ SYS_DESC(SYS_CNTV_CVAL_EL0), access_arch_timer },
+	TIMER_REG(CNTV_CVAL_EL0, NULL),
 
 	/* PMEVCNTRn_EL0 */
 	PMU_PMEVCNTR_EL0(0),
@@ -3715,11 +3715,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(CNTHCTL_EL2, access_rw, reset_val, 0),
 	{ SYS_DESC(SYS_CNTHP_TVAL_EL2), access_arch_timer },
 	TIMER_REG(CNTHP_CTL_EL2, el2_visibility),
-	EL2_REG(CNTHP_CVAL_EL2, access_arch_timer, reset_val, 0),
+	TIMER_REG(CNTHP_CVAL_EL2, el2_visibility),
 
 	{ SYS_DESC(SYS_CNTHV_TVAL_EL2), access_arch_timer, .visibility = cnthv_visibility },
 	TIMER_REG(CNTHV_CTL_EL2, cnthv_visibility),
-	EL2_REG_FILTERED(CNTHV_CVAL_EL2, access_arch_timer, reset_val, 0, cnthv_visibility),
+	TIMER_REG(CNTHV_CVAL_EL2, cnthv_visibility),
 
 	{ SYS_DESC(SYS_CNTKCTL_EL12), access_cntkctl_el12 },
 
-- 
2.47.3

[PATCH 08/13] KVM: arm64: Move CNT*CT_EL0 userspace accessors to generic infrastructure

[Marc Zyngier]

Moving the counter registers is a bit more involved than for the control
and comparator (there is no shadow data for the counter), but still
pretty manageable.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/guest.c    |  7 -------
 arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index c23ec9be4ce27..138e5e2dc10c8 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -592,19 +592,12 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
 }
 
 static const u64 timer_reg_list[] = {
-	KVM_REG_ARM_TIMER_CNT,
-	KVM_REG_ARM_PTIMER_CNT,
 };
 
 #define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)
 
 static bool is_timer_reg(u64 index)
 {
-	switch (index) {
-	case KVM_REG_ARM_TIMER_CNT:
-	case KVM_REG_ARM_PTIMER_CNT:
-		return true;
-	}
 	return false;
 }
 
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 68e88d5c0dfb5..e67eb39ddc118 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1605,12 +1605,38 @@ static int arch_timer_set_user(struct kvm_vcpu *vcpu,
 	case SYS_CNTHP_CTL_EL2:
 		val &= ~ARCH_TIMER_CTRL_IT_STAT;
 		break;
+	case SYS_CNTVCT_EL0:
+		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
+			timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read() - val);
+		return 0;
+	case SYS_CNTPCT_EL0:
+		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
+			timer_set_offset(vcpu_ptimer(vcpu), kvm_phys_timer_read() - val);
+		return 0;
 	}
 
 	__vcpu_assign_sys_reg(vcpu, rd->reg, val);
 	return 0;
 }
 
+static int arch_timer_get_user(struct kvm_vcpu *vcpu,
+			       const struct sys_reg_desc *rd,
+			       u64 *val)
+{
+	switch (reg_to_encoding(rd)) {
+	case SYS_CNTVCT_EL0:
+		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_vtimer(vcpu));
+		break;
+	case SYS_CNTPCT_EL0:
+		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_ptimer(vcpu));
+		break;
+	default:
+		*val = __vcpu_sys_reg(vcpu, rd->reg);
+	}
+
+	return 0;
+}
+
 static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
 				    s64 new, s64 cur)
 {
@@ -2539,7 +2565,7 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
 
 #define TIMER_REG(name, vis)					   \
 	SYS_REG_USER_FILTER(name, access_arch_timer, reset_val, 0, \
-			    NULL, arch_timer_set_user, vis)
+			    arch_timer_get_user, arch_timer_set_user, vis)
 
 /*
  * Since reset() callback and field val are not used for idregs, they will be
@@ -3506,8 +3532,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	AMU_AMEVTYPER1_EL0(14),
 	AMU_AMEVTYPER1_EL0(15),
 
-	{ SYS_DESC(SYS_CNTPCT_EL0), access_arch_timer },
-	{ SYS_DESC(SYS_CNTVCT_EL0), access_arch_timer },
+	{ SYS_DESC(SYS_CNTPCT_EL0), .access = access_arch_timer,
+	  .get_user = arch_timer_get_user, .set_user = arch_timer_set_user },
+	{ SYS_DESC(SYS_CNTVCT_EL0), .access = access_arch_timer,
+	  .get_user = arch_timer_get_user, .set_user = arch_timer_set_user },
 	{ SYS_DESC(SYS_CNTPCTSS_EL0), access_arch_timer },
 	{ SYS_DESC(SYS_CNTVCTSS_EL0), access_arch_timer },
 	{ SYS_DESC(SYS_CNTP_TVAL_EL0), access_arch_timer },
-- 
2.47.3

[PATCH 09/13] KVM: arm64: Fix WFxT handling of nested virt

[Marc Zyngier]

The spec for WFxT indicates that the parameter to the WFxT instruction
is relative to the reading of CNTVCT_EL0. This means that the implementation
needs to take the execution context into account, as CNTVOFF_EL2
does not always affect readings of CNTVCT_EL0 (such as when HCR_EL2.E2H
is 1 and that we're in host context).

This also rids us of the last instance of KVM_REG_ARM_TIMER_CNT
outside of the userspace interaction code.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/handle_exit.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index d449e15680e46..415f91ee8bcbf 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -147,7 +147,12 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu)
 		if (esr & ESR_ELx_WFx_ISS_RV) {
 			u64 val, now;
 
-			now = kvm_arm_timer_get_reg(vcpu, KVM_REG_ARM_TIMER_CNT);
+			now = kvm_phys_timer_read();
+			if (is_hyp_ctxt(vcpu) && vcpu_el2_e2h_is_set(vcpu))
+				now -= timer_get_offset(vcpu_hvtimer(vcpu));
+			else
+				now -= timer_get_offset(vcpu_vtimer(vcpu));
+
 			val = vcpu_get_reg(vcpu, kvm_vcpu_sys_get_rt(vcpu));
 
 			if (now >= val)
-- 
2.47.3

[PATCH 10/13] KVM: arm64: Kill leftovers of ad-hoc timer userspace access

[Marc Zyngier]

Now that the whole timer infrastructure is handled as system register
accesses, get rid of the now unused ad-hoc infrastructure.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/arch_timer.c  | 68 ------------------------------------
 arch/arm64/kvm/guest.c       | 55 -----------------------------
 include/kvm/arm_arch_timer.h |  3 --
 3 files changed, 126 deletions(-)

diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index 27662a3a3043e..3f675875abea2 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -1112,49 +1112,6 @@ void kvm_timer_cpu_down(void)
 		disable_percpu_irq(host_ptimer_irq);
 }
 
-int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value)
-{
-	struct arch_timer_context *timer;
-
-	switch (regid) {
-	case KVM_REG_ARM_TIMER_CTL:
-		timer = vcpu_vtimer(vcpu);
-		kvm_arm_timer_write(vcpu, timer, TIMER_REG_CTL, value);
-		break;
-	case KVM_REG_ARM_TIMER_CNT:
-		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET,
-			      &vcpu->kvm->arch.flags)) {
-			timer = vcpu_vtimer(vcpu);
-			timer_set_offset(timer, kvm_phys_timer_read() - value);
-		}
-		break;
-	case KVM_REG_ARM_TIMER_CVAL:
-		timer = vcpu_vtimer(vcpu);
-		kvm_arm_timer_write(vcpu, timer, TIMER_REG_CVAL, value);
-		break;
-	case KVM_REG_ARM_PTIMER_CTL:
-		timer = vcpu_ptimer(vcpu);
-		kvm_arm_timer_write(vcpu, timer, TIMER_REG_CTL, value);
-		break;
-	case KVM_REG_ARM_PTIMER_CNT:
-		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET,
-			      &vcpu->kvm->arch.flags)) {
-			timer = vcpu_ptimer(vcpu);
-			timer_set_offset(timer, kvm_phys_timer_read() - value);
-		}
-		break;
-	case KVM_REG_ARM_PTIMER_CVAL:
-		timer = vcpu_ptimer(vcpu);
-		kvm_arm_timer_write(vcpu, timer, TIMER_REG_CVAL, value);
-		break;
-
-	default:
-		return -1;
-	}
-
-	return 0;
-}
-
 static u64 read_timer_ctl(struct arch_timer_context *timer)
 {
 	/*
@@ -1171,31 +1128,6 @@ static u64 read_timer_ctl(struct arch_timer_context *timer)
 	return ctl;
 }
 
-u64 kvm_arm_timer_get_reg(struct kvm_vcpu *vcpu, u64 regid)
-{
-	switch (regid) {
-	case KVM_REG_ARM_TIMER_CTL:
-		return kvm_arm_timer_read(vcpu,
-					  vcpu_vtimer(vcpu), TIMER_REG_CTL);
-	case KVM_REG_ARM_TIMER_CNT:
-		return kvm_arm_timer_read(vcpu,
-					  vcpu_vtimer(vcpu), TIMER_REG_CNT);
-	case KVM_REG_ARM_TIMER_CVAL:
-		return kvm_arm_timer_read(vcpu,
-					  vcpu_vtimer(vcpu), TIMER_REG_CVAL);
-	case KVM_REG_ARM_PTIMER_CTL:
-		return kvm_arm_timer_read(vcpu,
-					  vcpu_ptimer(vcpu), TIMER_REG_CTL);
-	case KVM_REG_ARM_PTIMER_CNT:
-		return kvm_arm_timer_read(vcpu,
-					  vcpu_ptimer(vcpu), TIMER_REG_CNT);
-	case KVM_REG_ARM_PTIMER_CVAL:
-		return kvm_arm_timer_read(vcpu,
-					  vcpu_ptimer(vcpu), TIMER_REG_CVAL);
-	}
-	return (u64)-1;
-}
-
 static u64 kvm_arm_timer_read(struct kvm_vcpu *vcpu,
 			      struct arch_timer_context *timer,
 			      enum kvm_arch_timer_regs treg)
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 138e5e2dc10c8..1c87699fd886e 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -591,49 +591,6 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
 	return copy_core_reg_indices(vcpu, NULL);
 }
 
-static const u64 timer_reg_list[] = {
-};
-
-#define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)
-
-static bool is_timer_reg(u64 index)
-{
-	return false;
-}
-
-static int copy_timer_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
-{
-	for (int i = 0; i < NUM_TIMER_REGS; i++) {
-		if (put_user(timer_reg_list[i], uindices))
-			return -EFAULT;
-		uindices++;
-	}
-
-	return 0;
-}
-
-static int set_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
-{
-	void __user *uaddr = (void __user *)(long)reg->addr;
-	u64 val;
-	int ret;
-
-	ret = copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id));
-	if (ret != 0)
-		return -EFAULT;
-
-	return kvm_arm_timer_set_reg(vcpu, reg->id, val);
-}
-
-static int get_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
-{
-	void __user *uaddr = (void __user *)(long)reg->addr;
-	u64 val;
-
-	val = kvm_arm_timer_get_reg(vcpu, reg->id);
-	return copy_to_user(uaddr, &val, KVM_REG_SIZE(reg->id)) ? -EFAULT : 0;
-}
-
 static unsigned long num_sve_regs(const struct kvm_vcpu *vcpu)
 {
 	const unsigned int slices = vcpu_sve_slices(vcpu);
@@ -709,7 +666,6 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu)
 	res += num_sve_regs(vcpu);
 	res += kvm_arm_num_sys_reg_descs(vcpu);
 	res += kvm_arm_get_fw_num_regs(vcpu);
-	res += NUM_TIMER_REGS;
 
 	return res;
 }
@@ -740,11 +696,6 @@ int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 		return ret;
 	uindices += kvm_arm_get_fw_num_regs(vcpu);
 
-	ret = copy_timer_indices(vcpu, uindices);
-	if (ret < 0)
-		return ret;
-	uindices += NUM_TIMER_REGS;
-
 	return kvm_arm_copy_sys_reg_indices(vcpu, uindices);
 }
 
@@ -762,9 +713,6 @@ int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 	case KVM_REG_ARM64_SVE:	return get_sve_reg(vcpu, reg);
 	}
 
-	if (is_timer_reg(reg->id))
-		return get_timer_reg(vcpu, reg);
-
 	return kvm_arm_sys_reg_get_reg(vcpu, reg);
 }
 
@@ -782,9 +730,6 @@ int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 	case KVM_REG_ARM64_SVE:	return set_sve_reg(vcpu, reg);
 	}
 
-	if (is_timer_reg(reg->id))
-		return set_timer_reg(vcpu, reg);
-
 	return kvm_arm_sys_reg_set_reg(vcpu, reg);
 }
 
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index 5f7f2ed8817c5..7310841f45121 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -107,9 +107,6 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu);
 
 void kvm_timer_init_vm(struct kvm *kvm);
 
-u64 kvm_arm_timer_get_reg(struct kvm_vcpu *, u64 regid);
-int kvm_arm_timer_set_reg(struct kvm_vcpu *, u64 regid, u64 value);
-
 int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
 int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
 int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
-- 
2.47.3

[PATCH 11/13] KVM: arm64: selftests: Make dependencies on VHE-specific registers explicit

[Marc Zyngier]

The hyp virtual timer registers only exist when VHE is present,
Similarly, VNCR_EL2 only exists when NV2 is present.

Make these dependencies explicit.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 tools/testing/selftests/kvm/arm64/get-reg-list.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/kvm/arm64/get-reg-list.c b/tools/testing/selftests/kvm/arm64/get-reg-list.c
index 011fad95dd021..0a4cfb368512a 100644
--- a/tools/testing/selftests/kvm/arm64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/arm64/get-reg-list.c
@@ -65,6 +65,9 @@ static struct feature_id_reg feat_id_regs[] = {
 	REG_FEAT(SCTLR2_EL1,	ID_AA64MMFR3_EL1, SCTLRX, IMP),
 	REG_FEAT(VDISR_EL2,	ID_AA64PFR0_EL1, RAS, IMP),
 	REG_FEAT(VSESR_EL2,	ID_AA64PFR0_EL1, RAS, IMP),
+	REG_FEAT(VNCR_EL2,	ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY),
+	REG_FEAT(CNTHV_CTL_EL2, ID_AA64MMFR1_EL1, VH, IMP),
+	REG_FEAT(CNTHV_CVAL_EL2,ID_AA64MMFR1_EL1, VH, IMP),
 };
 
 bool filter_reg(__u64 reg)
-- 
2.47.3

[PATCH 12/13] KVM: arm64: selftests: Add an E2H=0-specific configuration to get_reg_list

[Marc Zyngier]

Add yet another configuration, this time dealing E2H=0.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 .../selftests/kvm/arm64/get-reg-list.c        | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/tools/testing/selftests/kvm/arm64/get-reg-list.c b/tools/testing/selftests/kvm/arm64/get-reg-list.c
index 0a4cfb368512a..7a238755f0728 100644
--- a/tools/testing/selftests/kvm/arm64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/arm64/get-reg-list.c
@@ -758,6 +758,10 @@ static __u64 el2_regs[] = {
 	SYS_REG(VSESR_EL2),
 };
 
+static __u64 el2_e2h0_regs[] = {
+	/* Empty */
+};
+
 #define BASE_SUBLIST \
 	{ "base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), }
 #define VREGS_SUBLIST \
@@ -792,6 +796,15 @@ static __u64 el2_regs[] = {
 		.regs		= el2_regs,			\
 		.regs_n		= ARRAY_SIZE(el2_regs),		\
 	}
+#define EL2_E2H0_SUBLIST					\
+	EL2_SUBLIST,						\
+	{							\
+		.name 		= "EL2 E2H0",			\
+		.capability	= KVM_CAP_ARM_EL2_E2H0,		\
+		.feature	= KVM_ARM_VCPU_HAS_EL2_E2H0,	\
+		.regs		= el2_e2h0_regs,		\
+		.regs_n		= ARRAY_SIZE(el2_e2h0_regs),	\
+	}
 
 static struct vcpu_reg_list vregs_config = {
 	.sublists = {
@@ -900,6 +913,65 @@ static struct vcpu_reg_list el2_pauth_pmu_config = {
 	},
 };
 
+static struct vcpu_reg_list el2_e2h0_vregs_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	EL2_E2H0_SUBLIST,
+	VREGS_SUBLIST,
+	{0},
+	},
+};
+
+static struct vcpu_reg_list el2_e2h0_vregs_pmu_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	EL2_E2H0_SUBLIST,
+	VREGS_SUBLIST,
+	PMU_SUBLIST,
+	{0},
+	},
+};
+
+static struct vcpu_reg_list el2_e2h0_sve_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	EL2_E2H0_SUBLIST,
+	SVE_SUBLIST,
+	{0},
+	},
+};
+
+static struct vcpu_reg_list el2_e2h0_sve_pmu_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	EL2_E2H0_SUBLIST,
+	SVE_SUBLIST,
+	PMU_SUBLIST,
+	{0},
+	},
+};
+
+static struct vcpu_reg_list el2_e2h0_pauth_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	EL2_E2H0_SUBLIST,
+	VREGS_SUBLIST,
+	PAUTH_SUBLIST,
+	{0},
+	},
+};
+
+static struct vcpu_reg_list el2_e2h0_pauth_pmu_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	EL2_E2H0_SUBLIST,
+	VREGS_SUBLIST,
+	PAUTH_SUBLIST,
+	PMU_SUBLIST,
+	{0},
+	},
+};
+
 struct vcpu_reg_list *vcpu_configs[] = {
 	&vregs_config,
 	&vregs_pmu_config,
@@ -914,5 +986,12 @@ struct vcpu_reg_list *vcpu_configs[] = {
 	&el2_sve_pmu_config,
 	&el2_pauth_config,
 	&el2_pauth_pmu_config,
+
+	&el2_e2h0_vregs_config,
+	&el2_e2h0_vregs_pmu_config,
+	&el2_e2h0_sve_config,
+	&el2_e2h0_sve_pmu_config,
+	&el2_e2h0_pauth_config,
+	&el2_e2h0_pauth_pmu_config,
 };
 int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
-- 
2.47.3

[PATCH 13/13] KVM: arm64: selftest: Fix misleading comment about virtual timer encoding

[Marc Zyngier]

The userspace-visible encoding for CNTV_CVAL_EL0 and CNTVCNT_EL0
have been swapped for as long as usersapce has had access to the
registers. This is documented in arch/arm64/include/uapi/asm/kvm.h.

Despite that, the get_reg_list test has unhelpful comments indicating
the wrong register for the encoding.

Replace this with definitions exposed in the include file, and
a comment explaining again the brokenness.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 .../testing/selftests/kvm/arm64/get-reg-list.c  | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/arm64/get-reg-list.c b/tools/testing/selftests/kvm/arm64/get-reg-list.c
index 7a238755f0728..c9b84eeaab6b2 100644
--- a/tools/testing/selftests/kvm/arm64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/arm64/get-reg-list.c
@@ -348,9 +348,20 @@ static __u64 base_regs[] = {
 	KVM_REG_ARM_FW_FEAT_BMAP_REG(1),	/* KVM_REG_ARM_STD_HYP_BMAP */
 	KVM_REG_ARM_FW_FEAT_BMAP_REG(2),	/* KVM_REG_ARM_VENDOR_HYP_BMAP */
 	KVM_REG_ARM_FW_FEAT_BMAP_REG(3),	/* KVM_REG_ARM_VENDOR_HYP_BMAP_2 */
-	ARM64_SYS_REG(3, 3, 14, 3, 1),	/* CNTV_CTL_EL0 */
-	ARM64_SYS_REG(3, 3, 14, 3, 2),	/* CNTV_CVAL_EL0 */
-	ARM64_SYS_REG(3, 3, 14, 0, 2),
+
+	/*
+	 * EL0 Virtual Timer Registers
+	 *
+	 * WARNING:
+	 * KVM_REG_ARM_TIMER_CVAL and KVM_REG_ARM_TIMER_CNT are not defined
+	 * with the appropriate register encodings.  Their values have been
+	 * accidentally swapped.  As this is set API, the definitions here
+	 * must be used, rather than ones derived from the encodings.
+	 */
+	KVM_ARM64_SYS_REG(SYS_CNTV_CTL_EL0),
+	KVM_REG_ARM_TIMER_CVAL,
+	KVM_REG_ARM_TIMER_CNT,
+
 	ARM64_SYS_REG(3, 0, 0, 0, 0),	/* MIDR_EL1 */
 	ARM64_SYS_REG(3, 0, 0, 0, 6),	/* REVIDR_EL1 */
 	ARM64_SYS_REG(3, 1, 0, 0, 1),	/* CLIDR_EL1 */
-- 
2.47.3

Re: [PATCH 01/13] KVM: arm64: Hide CNTHV_*_EL2 from userspace for nVHE guests

[Oliver Upton]

Hey,

On Mon, Sep 29, 2025 at 05:04:45PM +0100, Marc Zyngier wrote:
> Although we correctly UNDEF any CNTHV_*_EL2 access from the guest
> when E2H==0, we still expose these registers to userspace, which
> is a bad idea.
> 
> Drop the ad-hoc UNDEF injection and switch to a .visibility()
> callback which will also hide the register from userspace.
> 
> Fixes: 0e45981028550 ("KVM: arm64: timer: Don't adjust the EL2 virtual timer offset")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index ee8a7033c85bf..9f2f4e0b042e8 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1594,16 +1594,6 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
>  	return true;
>  }
>  
> -static bool access_hv_timer(struct kvm_vcpu *vcpu,
> -			    struct sys_reg_params *p,
> -			    const struct sys_reg_desc *r)
> -{
> -	if (!vcpu_el2_e2h_is_set(vcpu))
> -		return undef_access(vcpu, p, r);
> -
> -	return access_arch_timer(vcpu, p, r);
> -}
> -
>  static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
>  				    s64 new, s64 cur)
>  {
> @@ -2831,6 +2821,16 @@ static unsigned int s1pie_el2_visibility(const struct kvm_vcpu *vcpu,
>  	return __el2_visibility(vcpu, rd, s1pie_visibility);
>  }
>  
> +static unsigned int cnthv_visibility(const struct kvm_vcpu *vcpu,
> +				     const struct sys_reg_desc *rd)
> +{
> +	if (vcpu_has_nv(vcpu) &&
> +	    !vcpu_has_feature(vcpu, KVM_ARM_VCPU_HAS_EL2_E2H0))
> +		return 0;
> +
> +	return REG_HIDDEN;
> +}

Hmm. We've already exposed these to userspace at this point, we just
conveniently last the get-reg-list test to assert the accessibility of
these (broken) exposures.

Given the amount of UAPI mishaps we've had with registers in the past I
don't have much appetite for taking away something we already
advertised.

What about making these RAZ/WI from userspace?

Thanks,
Oliver

Re: [PATCH 05/13] KVM: arm64: Add timer UAPI workaround to sysreg infrastructure

[Oliver Upton]

On Mon, Sep 29, 2025 at 05:04:49PM +0100, Marc Zyngier wrote:
> Amongst the numerous bugs that plague the KVM/arm64 UAPI, one of
> the most annoying thing is that the userspace view of the virtual
> timer has its CVAL and CNT encodings swapped.
> 
> In order to reduce the amount of code that has to know about this,
> start by adding handling for this bug in the sys_reg code.
> 
> Nothing is making use of it yet, as the code responsible for userspace
> interaction is catching the accesses early.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 33 ++++++++++++++++++++++++++++++---
>  arch/arm64/kvm/sys_regs.h |  6 ++++++
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 9f2f4e0b042e8..8e6f50f54b4bf 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -5231,15 +5231,28 @@ static int demux_c15_set(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
>  	}
>  }
>  
> +static u64 kvm_one_reg_to_id(const struct kvm_one_reg *reg)
> +{
> +	switch(reg->id) {
> +	case KVM_REG_ARM_TIMER_CVAL:
> +		return TO_ARM64_SYS_REG(CNTV_CVAL_EL0);
> +	case KVM_REG_ARM_TIMER_CNT:
> +		return TO_ARM64_SYS_REG(CNTVCT_EL0);
> +	default:
> +		return reg->id;
> +	}
> +}
> +

Seems like a good spot to name n' blame the commit that introduced this
bug as a comment.

Thanks,
Oliver

Re: [PATCH 01/13] KVM: arm64: Hide CNTHV_*_EL2 from userspace for nVHE guests

[Marc Zyngier]

On Tue, 30 Sep 2025 01:35:07 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Hey,
> 
> On Mon, Sep 29, 2025 at 05:04:45PM +0100, Marc Zyngier wrote:
> > Although we correctly UNDEF any CNTHV_*_EL2 access from the guest
> > when E2H==0, we still expose these registers to userspace, which
> > is a bad idea.
> > 
> > Drop the ad-hoc UNDEF injection and switch to a .visibility()
> > callback which will also hide the register from userspace.
> > 
> > Fixes: 0e45981028550 ("KVM: arm64: timer: Don't adjust the EL2 virtual timer offset")
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index ee8a7033c85bf..9f2f4e0b042e8 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1594,16 +1594,6 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
> >  	return true;
> >  }
> >  
> > -static bool access_hv_timer(struct kvm_vcpu *vcpu,
> > -			    struct sys_reg_params *p,
> > -			    const struct sys_reg_desc *r)
> > -{
> > -	if (!vcpu_el2_e2h_is_set(vcpu))
> > -		return undef_access(vcpu, p, r);
> > -
> > -	return access_arch_timer(vcpu, p, r);
> > -}
> > -
> >  static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
> >  				    s64 new, s64 cur)
> >  {
> > @@ -2831,6 +2821,16 @@ static unsigned int s1pie_el2_visibility(const struct kvm_vcpu *vcpu,
> >  	return __el2_visibility(vcpu, rd, s1pie_visibility);
> >  }
> >  
> > +static unsigned int cnthv_visibility(const struct kvm_vcpu *vcpu,
> > +				     const struct sys_reg_desc *rd)
> > +{
> > +	if (vcpu_has_nv(vcpu) &&
> > +	    !vcpu_has_feature(vcpu, KVM_ARM_VCPU_HAS_EL2_E2H0))
> > +		return 0;
> > +
> > +	return REG_HIDDEN;
> > +}
> 
> Hmm. We've already exposed these to userspace at this point, we just
> conveniently last the get-reg-list test to assert the accessibility of
> these (broken) exposures.
> 
> Given the amount of UAPI mishaps we've had with registers in the past I
> don't have much appetite for taking away something we already
> advertised.
> 
> What about making these RAZ/WI from userspace?

Honestly, I don't think we should bother.

The only VMM supporting NV is QEMU, and it explicitly isn't able to
select E2H0. I'm happy to Cc stable on this, but worrying about nVHE
save/restore at this stage seems like an overreaction -- I'm pretty
sure NV save/restore is generally broken in many more ways.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 05/13] KVM: arm64: Add timer UAPI workaround to sysreg infrastructure

[Marc Zyngier]

On Tue, 30 Sep 2025 01:41:01 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Mon, Sep 29, 2025 at 05:04:49PM +0100, Marc Zyngier wrote:
> > Amongst the numerous bugs that plague the KVM/arm64 UAPI, one of
> > the most annoying thing is that the userspace view of the virtual
> > timer has its CVAL and CNT encodings swapped.
> > 
> > In order to reduce the amount of code that has to know about this,
> > start by adding handling for this bug in the sys_reg code.
> > 
> > Nothing is making use of it yet, as the code responsible for userspace
> > interaction is catching the accesses early.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 33 ++++++++++++++++++++++++++++++---
> >  arch/arm64/kvm/sys_regs.h |  6 ++++++
> >  2 files changed, 36 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 9f2f4e0b042e8..8e6f50f54b4bf 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -5231,15 +5231,28 @@ static int demux_c15_set(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
> >  	}
> >  }
> >  
> > +static u64 kvm_one_reg_to_id(const struct kvm_one_reg *reg)
> > +{
> > +	switch(reg->id) {
> > +	case KVM_REG_ARM_TIMER_CVAL:
> > +		return TO_ARM64_SYS_REG(CNTV_CVAL_EL0);
> > +	case KVM_REG_ARM_TIMER_CNT:
> > +		return TO_ARM64_SYS_REG(CNTVCT_EL0);
> > +	default:
> > +		return reg->id;
> > +	}
> > +}
> > +
> 
> Seems like a good spot to name n' blame the commit that introduced this
> bug as a comment.

Sure. That'd be 39735a3a39043 ("ARM/KVM: save and restore generic
timer registers"), but that's also the first time save/restore was
implemented at all, and there wasn't a sane version before that.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 03/13] KVM: arm64: Replace timer context vcpu pointer with timer_id

[Joey Gouly]

On Mon, Sep 29, 2025 at 05:04:47PM +0100, Marc Zyngier wrote:
> Having to follow a pointer to a vcpu is pretty dumb, when the timers
> are are a fixed offset in the vcpu structure itself.
> 
> Trade the vcpu pointer for a timer_id, which can then be used to
> compute the vcpu address as needed.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Joey Gouly <joey.gouly@arm.com>

> ---
>  arch/arm64/kvm/arch_timer.c  |  4 ++--
>  include/kvm/arm_arch_timer.h | 11 ++++++-----
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index e5a25e743f5be..c832c293676a3 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -149,7 +149,7 @@ static void timer_set_cval(struct arch_timer_context *ctxt, u64 cval)
>  static void timer_set_offset(struct arch_timer_context *ctxt, u64 offset)
>  {
>  	if (!ctxt->offset.vm_offset) {
> -		WARN(offset, "timer %ld\n", arch_timer_ctx_index(ctxt));
> +		WARN(offset, "timer %d\n", arch_timer_ctx_index(ctxt));
>  		return;
>  	}
>  
> @@ -1064,7 +1064,7 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)
>  	struct arch_timer_context *ctxt = vcpu_get_timer(vcpu, timerid);
>  	struct kvm *kvm = vcpu->kvm;
>  
> -	ctxt->vcpu = vcpu;
> +	ctxt->timer_id = timerid;
>  
>  	if (timerid == TIMER_VTIMER)
>  		ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset;
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index d188c716d03cb..d8e400cb2bfff 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -51,8 +51,6 @@ struct arch_timer_vm_data {
>  };
>  
>  struct arch_timer_context {
> -	struct kvm_vcpu			*vcpu;
> -
>  	/* Emulated Timer (may be unused) */
>  	struct hrtimer			hrtimer;
>  	u64				ns_frac;
> @@ -71,6 +69,9 @@ struct arch_timer_context {
>  		bool			level;
>  	} irq;
>  
> +	/* Who am I? */
> +	enum kvm_arch_timers		timer_id;
> +
>  	/* Duplicated state from arch_timer.c for convenience */
>  	u32				host_timer_irq;
>  };
> @@ -127,9 +128,9 @@ void kvm_timer_init_vhe(void);
>  #define vcpu_hvtimer(v)	(&(v)->arch.timer_cpu.timers[TIMER_HVTIMER])
>  #define vcpu_hptimer(v)	(&(v)->arch.timer_cpu.timers[TIMER_HPTIMER])
>  
> -#define arch_timer_ctx_index(ctx)	((ctx) - vcpu_timer((ctx)->vcpu)->timers)
> -#define timer_context_to_vcpu(ctx)	((ctx)->vcpu)
> -#define timer_vm_data(ctx)		(&(ctx)->vcpu->kvm->arch.timer_data)
> +#define arch_timer_ctx_index(ctx)	((ctx)->timer_id)
> +#define timer_context_to_vcpu(ctx)	container_of((ctx), struct kvm_vcpu, arch.timer_cpu.timers[(ctx)->timer_id])
> +#define timer_vm_data(ctx)		(&(timer_context_to_vcpu(ctx)->kvm->arch.timer_data))
>  #define timer_irq(ctx)			(timer_vm_data(ctx)->ppi[arch_timer_ctx_index(ctx)])
>  
>  u64 kvm_arm_timer_read_sysreg(struct kvm_vcpu *vcpu,
> -- 
> 2.47.3
> 

Re: [PATCH 08/13] KVM: arm64: Move CNT*CT_EL0 userspace accessors to generic infrastructure

[Joey Gouly]

Observation below

  |
  v

On Mon, Sep 29, 2025 at 05:04:52PM +0100, Marc Zyngier wrote:
> Moving the counter registers is a bit more involved than for the control
> and comparator (there is no shadow data for the counter), but still
> pretty manageable.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/guest.c    |  7 -------
>  arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++++++++++++---
>  2 files changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index c23ec9be4ce27..138e5e2dc10c8 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -592,19 +592,12 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
>  }
>  
>  static const u64 timer_reg_list[] = {
> -	KVM_REG_ARM_TIMER_CNT,
> -	KVM_REG_ARM_PTIMER_CNT,
>  };
>  
>  #define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)
>  
>  static bool is_timer_reg(u64 index)
>  {
> -	switch (index) {
> -	case KVM_REG_ARM_TIMER_CNT:
> -	case KVM_REG_ARM_PTIMER_CNT:
> -		return true;
> -	}
>  	return false;
>  }
>  
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 68e88d5c0dfb5..e67eb39ddc118 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1605,12 +1605,38 @@ static int arch_timer_set_user(struct kvm_vcpu *vcpu,
>  	case SYS_CNTHP_CTL_EL2:
>  		val &= ~ARCH_TIMER_CTRL_IT_STAT;
>  		break;
> +	case SYS_CNTVCT_EL0:
> +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> +			timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read() - val);
> +		return 0;
> +	case SYS_CNTPCT_EL0:
> +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> +			timer_set_offset(vcpu_ptimer(vcpu), kvm_phys_timer_read() - val);
> +		return 0;
>  	}
>  
>  	__vcpu_assign_sys_reg(vcpu, rd->reg, val);
>  	return 0;
>  }
>  
> +static int arch_timer_get_user(struct kvm_vcpu *vcpu,
> +			       const struct sys_reg_desc *rd,
> +			       u64 *val)
> +{
> +	switch (reg_to_encoding(rd)) {
> +	case SYS_CNTVCT_EL0:
> +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_vtimer(vcpu));
> +		break;
> +	case SYS_CNTPCT_EL0:
> +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_ptimer(vcpu));
> +		break;
> +	default:
> +		*val = __vcpu_sys_reg(vcpu, rd->reg);

Unsure if this is actually an issue but for the _CTL registers, via
access_arch_timer() (kvm_arm_timer_read_sysreg() -> .. -> read_timer_ctl()),
the ARCH_TIMER_CTRL_IT_STAT bit will be set if the timer expired, but that's
not done here.

> +	}
> +
> +	return 0;
> +}
> +
>  static s64 kvm_arm64_ftr_safe_value(u32 id, const struct arm64_ftr_bits *ftrp,
>  				    s64 new, s64 cur)
>  {
> @@ -2539,7 +2565,7 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
>  
>  #define TIMER_REG(name, vis)					   \
>  	SYS_REG_USER_FILTER(name, access_arch_timer, reset_val, 0, \
> -			    NULL, arch_timer_set_user, vis)
> +			    arch_timer_get_user, arch_timer_set_user, vis)
>  
>  /*
>   * Since reset() callback and field val are not used for idregs, they will be
> @@ -3506,8 +3532,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	AMU_AMEVTYPER1_EL0(14),
>  	AMU_AMEVTYPER1_EL0(15),
>  
> -	{ SYS_DESC(SYS_CNTPCT_EL0), access_arch_timer },
> -	{ SYS_DESC(SYS_CNTVCT_EL0), access_arch_timer },
> +	{ SYS_DESC(SYS_CNTPCT_EL0), .access = access_arch_timer,
> +	  .get_user = arch_timer_get_user, .set_user = arch_timer_set_user },
> +	{ SYS_DESC(SYS_CNTVCT_EL0), .access = access_arch_timer,
> +	  .get_user = arch_timer_get_user, .set_user = arch_timer_set_user },
>  	{ SYS_DESC(SYS_CNTPCTSS_EL0), access_arch_timer },
>  	{ SYS_DESC(SYS_CNTVCTSS_EL0), access_arch_timer },
>  	{ SYS_DESC(SYS_CNTP_TVAL_EL0), access_arch_timer },

Thanks,
Joey

Re: [PATCH 08/13] KVM: arm64: Move CNT*CT_EL0 userspace accessors to generic infrastructure

[Marc Zyngier]

On Tue, 30 Sep 2025 11:45:52 +0100,
Joey Gouly <joey.gouly@arm.com> wrote:
> 
> Observation below
> 
>   |
>   v
> 
> On Mon, Sep 29, 2025 at 05:04:52PM +0100, Marc Zyngier wrote:
> > Moving the counter registers is a bit more involved than for the control
> > and comparator (there is no shadow data for the counter), but still
> > pretty manageable.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/guest.c    |  7 -------
> >  arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++++++++++++---
> >  2 files changed, 31 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index c23ec9be4ce27..138e5e2dc10c8 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -592,19 +592,12 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
> >  }
> >  
> >  static const u64 timer_reg_list[] = {
> > -	KVM_REG_ARM_TIMER_CNT,
> > -	KVM_REG_ARM_PTIMER_CNT,
> >  };
> >  
> >  #define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)
> >  
> >  static bool is_timer_reg(u64 index)
> >  {
> > -	switch (index) {
> > -	case KVM_REG_ARM_TIMER_CNT:
> > -	case KVM_REG_ARM_PTIMER_CNT:
> > -		return true;
> > -	}
> >  	return false;
> >  }
> >  
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 68e88d5c0dfb5..e67eb39ddc118 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1605,12 +1605,38 @@ static int arch_timer_set_user(struct kvm_vcpu *vcpu,
> >  	case SYS_CNTHP_CTL_EL2:
> >  		val &= ~ARCH_TIMER_CTRL_IT_STAT;
> >  		break;
> > +	case SYS_CNTVCT_EL0:
> > +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> > +			timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read() - val);
> > +		return 0;
> > +	case SYS_CNTPCT_EL0:
> > +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> > +			timer_set_offset(vcpu_ptimer(vcpu), kvm_phys_timer_read() - val);
> > +		return 0;
> >  	}
> >  
> >  	__vcpu_assign_sys_reg(vcpu, rd->reg, val);
> >  	return 0;
> >  }
> >  
> > +static int arch_timer_get_user(struct kvm_vcpu *vcpu,
> > +			       const struct sys_reg_desc *rd,
> > +			       u64 *val)
> > +{
> > +	switch (reg_to_encoding(rd)) {
> > +	case SYS_CNTVCT_EL0:
> > +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_vtimer(vcpu));
> > +		break;
> > +	case SYS_CNTPCT_EL0:
> > +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_ptimer(vcpu));
> > +		break;
> > +	default:
> > +		*val = __vcpu_sys_reg(vcpu, rd->reg);
> 
> Unsure if this is actually an issue but for the _CTL registers, via
> access_arch_timer() (kvm_arm_timer_read_sysreg() -> .. -> read_timer_ctl()),
> the ARCH_TIMER_CTRL_IT_STAT bit will be set if the timer expired, but that's
> not done here.

Indeed, but I don't think this really matters, at least not for
save/restore. We always clear the ISTATUS bit on restore, and
snapshoting CTL is always a racy process.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 08/13] KVM: arm64: Move CNT*CT_EL0 userspace accessors to generic infrastructure

[Joey Gouly]

On Tue, Sep 30, 2025 at 01:05:05PM +0100, Marc Zyngier wrote:
> On Tue, 30 Sep 2025 11:45:52 +0100,
> Joey Gouly <joey.gouly@arm.com> wrote:
> > 
> > Observation below
> > 
> >   |
> >   v
> > 
> > On Mon, Sep 29, 2025 at 05:04:52PM +0100, Marc Zyngier wrote:
> > > Moving the counter registers is a bit more involved than for the control
> > > and comparator (there is no shadow data for the counter), but still
> > > pretty manageable.
> > > 
> > > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > > ---
> > >  arch/arm64/kvm/guest.c    |  7 -------
> > >  arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++++++++++++---
> > >  2 files changed, 31 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > index c23ec9be4ce27..138e5e2dc10c8 100644
> > > --- a/arch/arm64/kvm/guest.c
> > > +++ b/arch/arm64/kvm/guest.c
> > > @@ -592,19 +592,12 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)
> > >  }
> > >  
> > >  static const u64 timer_reg_list[] = {
> > > -	KVM_REG_ARM_TIMER_CNT,
> > > -	KVM_REG_ARM_PTIMER_CNT,
> > >  };
> > >  
> > >  #define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)
> > >  
> > >  static bool is_timer_reg(u64 index)
> > >  {
> > > -	switch (index) {
> > > -	case KVM_REG_ARM_TIMER_CNT:
> > > -	case KVM_REG_ARM_PTIMER_CNT:
> > > -		return true;
> > > -	}
> > >  	return false;
> > >  }
> > >  
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 68e88d5c0dfb5..e67eb39ddc118 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -1605,12 +1605,38 @@ static int arch_timer_set_user(struct kvm_vcpu *vcpu,
> > >  	case SYS_CNTHP_CTL_EL2:
> > >  		val &= ~ARCH_TIMER_CTRL_IT_STAT;
> > >  		break;
> > > +	case SYS_CNTVCT_EL0:
> > > +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> > > +			timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read() - val);
> > > +		return 0;
> > > +	case SYS_CNTPCT_EL0:
> > > +		if (!test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags))
> > > +			timer_set_offset(vcpu_ptimer(vcpu), kvm_phys_timer_read() - val);
> > > +		return 0;
> > >  	}
> > >  
> > >  	__vcpu_assign_sys_reg(vcpu, rd->reg, val);
> > >  	return 0;
> > >  }
> > >  
> > > +static int arch_timer_get_user(struct kvm_vcpu *vcpu,
> > > +			       const struct sys_reg_desc *rd,
> > > +			       u64 *val)
> > > +{
> > > +	switch (reg_to_encoding(rd)) {
> > > +	case SYS_CNTVCT_EL0:
> > > +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_vtimer(vcpu));
> > > +		break;
> > > +	case SYS_CNTPCT_EL0:
> > > +		*val = kvm_phys_timer_read() - timer_get_offset(vcpu_ptimer(vcpu));
> > > +		break;
> > > +	default:
> > > +		*val = __vcpu_sys_reg(vcpu, rd->reg);
> > 
> > Unsure if this is actually an issue but for the _CTL registers, via
> > access_arch_timer() (kvm_arm_timer_read_sysreg() -> .. -> read_timer_ctl()),
> > the ARCH_TIMER_CTRL_IT_STAT bit will be set if the timer expired, but that's
> > not done here.
> 
> Indeed, but I don't think this really matters, at least not for
> save/restore. We always clear the ISTATUS bit on restore, and
> snapshoting CTL is always a racy process.

Makes sense, so:

Reviewed-by: Joey Gouly <joey.gouly@arm.com>

Thanks,
Joey

> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

Re: [PATCH 00/13] KVM: arm64: De-specialise the timer UAPI

[Marc Zyngier]

On Mon, 29 Sep 2025 17:04:44 +0100, Marc Zyngier wrote:
> Since the beginning of the KVM/arm64 port, the timer registers were
> handled out of the normal sysreg flow when it came to userspace
> access, leading to extra complexity and a bit of code duplication.
> 
> When NV was introduced, the decision was made early to handle the new
> timer registers as part of the generic infrastructure. However, the
> EL0 timers were left behind until someone could be bothered to
> entangle that mess.
> 
> [...]

Applied to fixes, thanks!

[01/13] KVM: arm64: Hide CNTHV_*_EL2 from userspace for nVHE guests
        commit: 4cab5c857d1f92b4b322e30349fdc5e2e38e7a2f
[02/13] KVM: arm64: Introduce timer_context_to_vcpu() helper
        commit: aa68975c973ed3b0bd4ff513113495588afb855c
[03/13] KVM: arm64: Replace timer context vcpu pointer with timer_id
        commit: 8625a670afb05f1e1d69d50a74dbcc9d1b855efe
[04/13] KVM: arm64: Make timer_set_offset() generally accessible
        commit: a92d552266890f83126fdef4f777a985cc1302bd
[05/13] KVM: arm64: Add timer UAPI workaround to sysreg infrastructure
        commit: 77a0c42eaf03c66936429d190bb2ea1a214bd528
[06/13] KVM: arm64: Move CNT*_CTL_EL0 userspace accessors to generic infrastructure
        commit: 09424d5d7d4e8b427ee4a737fb7765103789e08a
[07/13] KVM: arm64: Move CNT*_CVAL_EL0 userspace accessors to generic infrastructure
        commit: 8af198980eff2ed2a5df3d2ee39f8c9d61f40559
[08/13] KVM: arm64: Move CNT*CT_EL0 userspace accessors to generic infrastructure
        commit: c3be3a48fb18f9d243fac452e0be41469bb246b4
[09/13] KVM: arm64: Fix WFxT handling of nested virt
        commit: 892f7c38ba3b7de19b3dffb8e148d5fbf1228f20
[10/13] KVM: arm64: Kill leftovers of ad-hoc timer userspace access
        commit: 386aac77da112651a5cdadc4a6b29181592f5aa0
[11/13] KVM: arm64: selftests: Make dependencies on VHE-specific registers explicit
        commit: 6418330c8478735f625398bc4e96d3ac6ce1e055
[12/13] KVM: arm64: selftests: Add an E2H=0-specific configuration to get_reg_list
        commit: 4da5a9af78b74fb771a4d25dc794296d10e170b1
[13/13] KVM: arm64: selftest: Fix misleading comment about virtual timer encoding
        commit: 5c7cf1e44e94a5408b1b5277810502b0f82b77fe

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.