Re: [PATCH v15 13/37] KVM: arm64: CCA: Support timers in realm RECs

On Wed, 15 Jul 2026 15:28:15 +0100,
Steven Price <[email protected]> wrote:
> 
> The RMM keeps track of the timer while the realm REC is running, but on
> exit to the normal world KVM is responsible for handling the timers.
> 
> A later patch adds the support for propagating the timer values from the
> exit data structure and calling kvm_realm_timers_update().
> 
> Signed-off-by: Steven Price <[email protected]>
> ---
> Changes since v14:
>  * Special case in kvm_timer_vcpu_load()/kvm_timer_vcpu_put() the timer
>    handling.
> Changes since v12:
>  * Adapt to upstream changes.
> Changes since v11:
>  * Drop the kvm_is_realm() check from timer_set_offset(). We already
>    ensure that the offset is 0 when calling the function.
> Changes since v10:
>  * KVM_CAP_COUNTER_OFFSET is now already hidden by a previous patch.
> Changes since v9:
>  * No need to move the call to kvm_timer_unblocking() in
>    kvm_timer_vcpu_load().
> Changes since v7:
>  * Hide KVM_CAP_COUNTER_OFFSET for realm guests.
> ---
>  arch/arm64/kvm/arch_timer.c  | 38 +++++++++++++++++++++++++++++++++---
>  include/kvm/arm_arch_timer.h |  2 ++
>  2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 4155fe89b58a..fdd68f1f5b7b 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -482,6 +482,20 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
>  			    timer_ctx);
>  }
>  
> +void kvm_realm_timers_update(struct kvm_vcpu *vcpu)
> +{
> +	struct arch_timer_cpu *arch_timer = &vcpu->arch.timer_cpu;
> +	int i;
> +
> +	for (i = 0; i < NR_KVM_EL0_TIMERS; i++) {
> +		struct arch_timer_context *timer = &arch_timer->timers[i];
> +		bool status = timer_get_ctl(timer) & ARCH_TIMER_CTRL_IT_STAT;
> +		bool level = kvm_timer_enabled(timer) && status;
> +
> +		kvm_timer_update_irq(vcpu, level, timer);
> +	}
> +}
> +

Why do we need this? What is so special about CCA that it cannot use
the existing timer flow?

>  /* Only called for a fully emulated timer */
>  static void timer_emulate(struct arch_timer_context *ctx)
>  {
> @@ -888,6 +902,11 @@ void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)
>  	if (unlikely(!timer->enabled))
>  		return;
>  
> +	if (vcpu_is_rec(vcpu)) {
> +		kvm_timer_unblocking(vcpu);
> +		return;
> +	}
> +
>  	get_timer_map(vcpu, &map);
>  
>  	if (static_branch_likely(&has_gic_active_state)) {
> @@ -923,6 +942,12 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
>  	if (unlikely(!timer->enabled))
>  		return;
>  
> +	if (vcpu_is_rec(vcpu)) {
> +		if (kvm_vcpu_is_blocking(vcpu))
> +			kvm_timer_blocking(vcpu);
> +		return;
> +	}
> +
>  	get_timer_map(vcpu, &map);
>  
>  	timer_save_state(map.direct_vtimer);
> @@ -1073,7 +1098,7 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)
>  
>  	ctxt->timer_id = timerid;
>  
> -	if (!kvm_vm_is_protected(vcpu->kvm)) {
> +	if (!kvm_vm_is_protected(vcpu->kvm) && !vcpu_is_rec(vcpu)) {
>  		if (timerid == TIMER_VTIMER)
>  			ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset;
>  		else
> @@ -1104,7 +1129,7 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
>  		timer_context_init(vcpu, i);
>  
>  	/* Synchronize offsets across timers of a VM if not already provided */
> -	if (!vcpu_is_protected(vcpu) &&
> +	if (!vcpu_is_protected(vcpu) && !vcpu_is_rec(vcpu) &&
>  	    !test_bit(KVM_ARCH_FLAG_VM_COUNTER_OFFSET, &vcpu->kvm->arch.flags)) {
>  		timer_set_offset(vcpu_vtimer(vcpu), kvm_phys_timer_read());
>  		timer_set_offset(vcpu_ptimer(vcpu), 0);
> @@ -1600,6 +1625,13 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * We don't use mapped IRQs for Realms because the RMI doesn't allow
> +	 * us setting the LR.HW bit in the VGIC.
> +	 */
> +	if (vcpu_is_rec(vcpu))
> +		return 0;
> +

If you can't set the HW bit, then use the existing infrastructure for
similarly challenged systems. But I find this restriction hard to
swallow. What is so special about the HW bit?

As it stands, I don't think this is acceptable, and I really want to
know why you need to special-case the timer code.

	M.

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