Re: [PATCH v6 29/39] KVM: arm64: gic-v5: Enlighten arch timer for GICv5

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Guest-driven 100% host CPU soft lockup due to unconditional timer polling bypassing VGIC mask state on WFI.
- [High] `KVM_SET_DEVICE_ATTR` for timer IRQs unconditionally fails for GICv5 due to a mismatch between raw and internal bit-packed INTIDs, breaking VM initialization.
--

--- Patch [29]: [PATCH v6 29/39] KVM: arm64: gic-v5: Enlighten arch timer for GICv5 ---
commit f6d424766abc019a2ae7837f0e736e6af4c5961c
Author: Sascha Bischoff <[email protected]>

KVM: arm64: gic-v5: Enlighten arch timer for GICv5

This commit updates the KVM architectural timer to support GICv5 features,
including direct virtual interrupt (DVI) injection for mapped timers and
mandatory architected PPI assignments. It modifies how interrupts are queued,
masked, and validated to align with GICv5 requirements.

> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 53312b88c342d..4575c36cae537 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c

[ ... ]

> @@ -396,7 +406,11 @@ static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx)
>  
>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>  {
> -	return vcpu_has_wfit_active(vcpu) && wfit_delay_ns(vcpu) == 0;
> +	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
> +	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +
> +	return kvm_timer_should_fire(vtimer) || kvm_timer_should_fire(ptimer) ||
> +	       (vcpu_has_wfit_active(vcpu) && wfit_delay_ns(vcpu) == 0);
>  }

[Severity: High]
Can this cause a host soft lockup if the guest executes a WFI?

If a guest masks the timer interrupt in the VGIC but leaves it enabled in the
timer's control register, kvm_timer_should_fire() will still return true since
it only checks the timer's internal state. When the guest then executes WFI,
kvm_vcpu_block() will see a pending timer via kvm_cpu_has_pending_timer() and
refuse to block, immediately resuming the guest. The guest will then re-execute
WFI, resulting in an infinite tight loop.

[ ... ]

> @@ -1634,10 +1700,16 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>  		idx = TIMER_HPTIMER;
>  		break;
>  	default:
> -		ret = -ENXIO;
> -		goto out;
> +		return -ENXIO;
>  	}
>  
> +	/*
> +	 * The PPIs for the Arch Timers are architecturally defined for
> +	 * GICv5. Reject anything that changes them from the specified value.
> +	 */
> +	if (vgic_is_v5(vcpu->kvm) && vcpu->kvm->arch.timer_data.ppi[idx] != irq)
> +		return -EINVAL;
> +

[Severity: High]
Does this unconditionally fail for GICv5 guests?

Userspace passes a standard raw PPI ID (e.g., 27) for the irq value.
However, for GICv5, kvm_timer_init_vm() initializes
vcpu->kvm->arch.timer_data.ppi[idx] using get_vgic_ppi(), which packs
GICV5_HWIRQ_TYPE_PPI into the upper bits of the integer. Comparing this
bit-packed internal representation directly against the raw userspace
irq value will always result in a mismatch and return -EINVAL, breaking
VM initialization.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=29