Re: [PATCH] irqchip/gic-v4: Use a separate lockdep class for vPE doorbell IRQs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] irqchip/gic-v4: missing IRQ_NOTHREAD in its_alloc_vcpu_irqs() causes deadlock
--

commit 3b1ba3145da23221937683b56baa88615a91fb69
Author: Colton Lewis <coltonlewis@google.com>

irqchip/gic-v4: Use a separate lockdep class for vPE doorbell IRQs

This commit assigns a dedicated lockdep class to GICv4 vPE doorbell IRQs in
its_alloc_vcpu_irqs() to avoid a circular locking dependency between
rq->__lock and the shared irq_desc_lock_class when scheduling out a KVM
vCPU.

> diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> index 8455b4a5fbb0d..68ee288f6a076 100644
> --- a/drivers/irqchip/irq-gic-v4.c
> +++ b/drivers/irqchip/irq-gic-v4.c
> @@ -157,6 +157,8 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
>  
>  int its_alloc_vcpu_irqs(struct its_vm *vm)
>  {
> +	static struct lock_class_key vpe_irq_lock_class;
> +	static struct lock_class_key vpe_irq_request_class;
>  	int vpe_base_irq, i;
>  
>  	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv4-vpe",
> @@ -183,6 +185,8 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
>  	for (i = 0; i < vm->nr_vpes; i++) {
>  		int ret;
>  		vm->vpes[i]->irq = vpe_base_irq + i;
> +		irq_set_lockdep_class(vm->vpes[i]->irq, &vpe_irq_lock_class,
> +				      &vpe_irq_request_class);

[Severity: High]
Can vpe->irq still establish a vpe_irq_lock_class -> p->pi_lock ->
rq->__lock dependency and deadlock when forced IRQ threading is active (via
the threadirqs boot parameter or CONFIG_PREEMPT_RT=y)?

Neither its_alloc_vcpu_irqs() / its_vpe_irq_domain_alloc() nor DB_IRQ_FLAGS
sets IRQ_NOTHREAD, and vgic_v4_request_vpe_irq() in
arch/arm64/kvm/vgic/vgic-v4.c requests the interrupt without IRQF_NO_THREAD:

int vgic_v4_request_vpe_irq(struct kvm_vcpu *vcpu, int irq)
{
	return request_irq(irq, vgic_v4_doorbell_handler, 0, "vcpu", vcpu);
}

Because arm64 unconditionally selects IRQ_FORCED_THREADING, when
force_irqthreads() is enabled, __setup_irq() -> irq_setup_forced_threading()
converts vgic_v4_doorbell_handler into a threaded handler and creates
action->thread for vpe->irq.

While IRQ_NO_BALANCING blocks userspace affinity changes, vgic_v4_load() in
arch/arm64/kvm/vgic/vgic-v4.c directly updates the affinity on vcpu_load():

	err = irq_set_affinity(vpe->irq, cpumask_of(smp_processor_id()));

This acquires vpe->irq's desc->lock (vpe_irq_lock_class) and calls
irq_set_affinity_locked() -> irq_try_set_affinity() -> irq_do_set_affinity().
After its_vpe_set_affinity() returns IRQ_SET_MASK_OK_DONE,
irq_do_set_affinity() invokes irq_set_thread_affinity() in
kernel/irq/manage.c while still holding desc->lock:

	for_each_action_of_desc(desc, action) {
		if (action->thread) {
			set_bit(IRQTF_AFFINITY, &action->thread_flags);
			wake_up_process(action->thread);
		}

Calling wake_up_process() under desc->lock acquires p->pi_lock and
rq->__lock under vpe_irq_lock_class (as does __irq_wake_thread() when the
doorbell interrupt fires). When a resident vCPU is later scheduled out,
__schedule() holds rq->__lock and calls kvm_sched_out() ->
kvm_arch_vcpu_put() -> vgic_v3_put() -> vgic_v4_put() ->
its_make_vpe_non_resident() -> irq_set_vcpu_affinity(), acquiring
vpe_irq_lock_class under rq->__lock.

Should vPE doorbell IRQs also be marked with IRQ_NOTHREAD (or requested with
IRQF_NO_THREAD) so they are never force-threaded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023215207.1465369-1-coltonlewis@google.com?part=1