Re: [PATCH 1/2] genirq/manage: Provide irq_set_percpu_irqchip_state()

[Marc Zyngier <maz@kernel.org>]

On Sat, 24 Oct 2026 03:24:49 +0100,
Amir Noohi <amirmnoohi@gmail.com> wrote:
> 
> irq_set_irqchip_state() acquires the bus lock and the descriptor lock of
> the interrupt. A per CPU devid interrupt has a single descriptor for all
> CPUs, while its irqchip state is per CPU, e.g. each GICv3 redistributor
> holds the active and pending state of the PPIs of its own CPU and GICv5
> keeps that state in per CPU system registers. Callers which modify that
> state have to be pinned to the CPU anyway, so the descriptor lock only
> serializes CPUs which update their own private state.
> 
> KVM on arm64 sets the active state of the host timer interrupts on every
> vCPU load, i.e. on every KVM_RUN ioctl and every time a vCPU thread is
> scheduled in. When many CPUs load vCPUs at the same time, they all spin
> on the descriptor lock of the same interrupt, even when every vCPU has
> a physical CPU of its own. A global serialization point on the same path
> was removed before in commit 64b6d1d7a845 ("genirq: Get rid of global
> lock in irq_do_set_affinity()").
> 
> Provide irq_set_percpu_irqchip_state(), which sets the irqchip state of
> a per CPU devid interrupt on the calling CPU without acquiring the
> descriptor lock. The irqchip callback is still invoked with interrupts
> disabled. The caller has to have preemption disabled and has to ensure
> that the interrupt is neither freed nor reconfigured concurrently, which
> is the case for an interrupt it has requested. Interrupt chips which
> require a bus lock are rejected, as the bus lock may sleep and cannot be
> taken with preemption disabled.
> 
> The function is not exported as its only user, KVM on arm64, cannot be
> built as a module.
> 
> Signed-off-by: Amir Noohi <amirmnoohi@gmail.com>
> ---
>  include/linux/interrupt.h |  2 ++
>  kernel/irq/manage.c       | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 3bf969ad8fe0..ab20129615d4 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -507,6 +507,8 @@ extern int irq_get_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
>  				 bool *state);
>  extern int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
>  				 bool state);
> +int irq_set_percpu_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
> +				 bool state);
>  
>  #ifdef CONFIG_IRQ_FORCED_THREADING
>  # ifdef CONFIG_PREEMPT_RT
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index bdb47837c66f..e76fe1cec9fb 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -2777,6 +2777,42 @@ int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state which, bool v
>  }
>  EXPORT_SYMBOL_GPL(irq_set_irqchip_state);
>  
> +/**
> + * irq_set_percpu_irqchip_state - Set the irqchip state of a per CPU interrupt
> + * @irq:	Linux interrupt number of a per CPU devid interrupt
> + * @which:	State to be set (one of IRQCHIP_STATE_*)
> + * @val:	Value corresponding to @which
> + *
> + * Variant of irq_set_irqchip_state() which sets the state of the calling
> + * CPU's instance of a per CPU devid interrupt without acquiring the
> + * interrupt descriptor lock. The descriptor and its lock are shared by all
> + * CPUs while the state is not, so acquiring the lock only serializes CPUs
> + * which modify their own state.
> + *
> + * Must be called with preemption disabled on an interrupt which has been
> + * requested, so that the interrupt chip hierarchy cannot change
> + * concurrently. Interrupt chips which require a bus lock are not supported.
> + *
> + * Return: 0 on success, a negative error code otherwise.
> + */
> +int irq_set_percpu_irqchip_state(unsigned int irq, enum irqchip_irq_state which, bool val)
> +{
> +	struct irq_desc *desc = irq_to_desc(irq);
> +	struct irq_chip *chip;
> +
> +	lockdep_assert_preemption_disabled();
> +
> +	if (WARN_ON_ONCE(!desc || !irq_settings_is_per_cpu_devid(desc)))
> +		return -EINVAL;
> +
> +	chip = irq_desc_get_chip(desc);
> +	if (WARN_ON_ONCE(chip->irq_bus_lock || chip->irq_bus_sync_unlock))
> +		return -EINVAL;
> +
> +	guard(irqsave)();
> +	return __irq_set_irqchip_state(desc, which, val);
> +}
> +
>  /**
>   * irq_has_action - Check whether an interrupt is requested
>   * @irq:	The linux irq number

It really feels like a leap of faith.

There is nothing that says that it is safe to call this without
holding the global lock. For all we know, drivers have been relying on
it for a good decade, and I don't feel confident letting things rip
just because it (mostly) works with the GIC.

At the very least, something exposed by the irqchip (a per-IRQ flag?)
should tell you whether it is safe to elide the lock.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.