Re: [PATCH v5 9/9] irqchip/al-fic: support per-child affinity via interrupt redirection

[Radu Rendec]

On Sun, 2026-10-11 at 05:04 +0000, Eliav Farber wrote:
> The FIC multiplexes up to 32 sources into one parent GIC SPI, and a
> child's handler runs in the parent's context, so a child has no affinity
> control of its own. irq_set_affinity() therefore fails with -EINVAL, and
> drivers that treat that as fatal cannot probe when their interrupt
> arrives through a FIC group. The parent routing cannot be changed on
> behalf of one child either, because several FIC groups may share one SPI
> and all of them would move with it.
> 
> Use the interrupt redirection infrastructure. Dispatch children with
> generic_handle_demux_domain_irq() and set irq_set_affinity to
> irq_chip_redirect_set_affinity(), so a child whose affinity excludes the
> CPU taking the parent interrupt runs on its target CPU in IRQ work
> context. The interrupt is still taken on the parent's CPU; only the
> child's handler body moves.
> 
> Redirection defers the whole flow handler, so the cause bit is cleared in
> al_fic_irq_pre_redirect(), which runs in parent context on both the
> inline and the redirected path. A level configured group is masked there
> too, because its cause bit re-latches while the input stays asserted; the
> deferred handle_level_irq() masks again and unmasks when the handler is
> done. An edge configured group is only acked, since handle_edge_irq() has
> no unconditional unmask. irq_ack is a noop because irq_pre_redirect runs
> on every dispatch and the ack must not happen twice.
> 
> Signed-off-by: Eliav Farber <farbere@xxxxxxxxxx>
> ---
> v5: rewritten from scratch. Thomas Gleixner rejected the
>     IRQ_SET_MASK_OK_DONE approach below ("That's a horrible hack and just
>     pretends to solve a problem. We have the interrupt redirection
>     infrastructure to handle that correctly."). This patch instead wires
>     al-fic into that infrastructure (irq_chip_redirect_set_affinity(),
>     generic_handle_demux_domain_irq(), and an irq_pre_redirect callback
>     that moves the ack - and, for a level configured group, the mask -
>     into parent context), per Radu Rendec's confirmation of this shape on
>     the v2 thread and the dw_pci driver he and Thomas pointed at as a
>     reference. Tested on our hardware: with a child's affinity pinned
>     away from the CPU taking the parent SPI, the handler body runs on the
>     target CPU and the ack still happens on the parent's CPU, with no
>     re-fire storm on a level configured group, and no change to the
>     inline (non-redirected) path. This replaces every version of the
>     irq_set_affinity callback below; the v2-v4 history is kept for the
>     record of how the approach got here.
> 
> v4: no change. The question Radu Rendec raised on this patch - whether an
>     unconditional IRQ_SET_MASK_OK_DONE is the right contract, or whether
>     it should fail with -EINVAL - is still open.
> 
> v3: no change. The question Radu Rendec raised on this patch - whether an
>     unconditional IRQ_SET_MASK_OK_DONE is the right contract, or whether
>     it should fail with -EINVAL - is still under discussion on the v2
>     thread and is answered there.
> 
> v2: add Co-developed-by/Signed-off-by: Talel Shenhar. No code change from
>     v1.
> 
>  drivers/irqchip/irq-al-fic.c | 46 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-al-fic.c b/drivers/irqchip/irq-al-fic.c
> index 13a945a1d312..77cff6308eba 100644
> --- a/drivers/irqchip/irq-al-fic.c
> +++ b/drivers/irqchip/irq-al-fic.c
> @@ -142,7 +142,13 @@ static irqreturn_t al_fic_irq_handler(int irq, void *data)
>  	pending &= ~gc->mask_cache;
>  
>  	for_each_set_bit(hwirq, &pending, NR_FIC_IRQS)
> -		generic_handle_domain_irq(domain, hwirq);
> +		/*
> +		 * Redirect a child whose affinity excludes this CPU to its
> +		 * target CPU via irq_work. The ack moves to
> +		 * al_fic_irq_pre_redirect(), because the whole flow handler is
> +		 * deferred when that happens.
> +		 */
> +		generic_handle_demux_domain_irq(domain, hwirq);
>  
>  	return pending ? IRQ_HANDLED : IRQ_NONE;
>  }
> @@ -157,6 +163,32 @@ static int al_fic_irq_retrigger(struct irq_data *data)
>  	return 1;
>  }
>  
> +#ifdef CONFIG_SMP
> +/* The ack happens in al_fic_irq_pre_redirect() on both paths instead. */
> +static void al_fic_irq_noop(struct irq_data *data) { }
> +
> +/*
> + * Runs in the parent interrupt context on every dispatch, redirected or not.
> + * Clear the cause bit here: once the flow handler is deferred, the FIC keeps
> + * driving its output until the irq_work runs, and a level parent SPI re-fires.
> + *
> + * A level configured group is masked too, because its cause bit re-latches
> + * while the input stays asserted; the deferred handle_level_irq() unmasks
> + * when the handler is done. An edge configured group is only acked, since
> + * handle_edge_irq() never unmasks unconditionally and the mask would stay.
> + */
> +static void al_fic_irq_pre_redirect(struct irq_data *data)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
> +	struct al_fic *fic = gc->private;
> +
> +	if (fic->state == AL_FIC_CONFIGURED_LEVEL)
> +		irq_gc_mask_set_bit(data);
> +
> +	irq_gc_ack_clr_bit(data);
> +}
> +#endif /* CONFIG_SMP */
> +
>  static int al_fic_register(struct device_node *node,
>  			   struct al_fic *fic,
>  			   enum al_fic_id fic_id)
> @@ -195,9 +227,19 @@ static int al_fic_register(struct device_node *node,
>  	gc->chip_types->regs.ack = AL_FIC_CAUSE;
>  	gc->chip_types->chip.irq_mask = irq_gc_mask_set_bit;
>  	gc->chip_types->chip.irq_unmask = irq_gc_mask_clr_bit;
> -	gc->chip_types->chip.irq_ack = irq_gc_ack_clr_bit;
>  	gc->chip_types->chip.irq_set_type = al_fic_irq_set_type;
>  	gc->chip_types->chip.irq_retrigger = al_fic_irq_retrigger;
> +#ifdef CONFIG_SMP
> +	/*
> +	 * The FIC has no hardware affinity control, so give each child real
> +	 * affinity by redirecting its handler to the target CPU.
> +	 */
> +	gc->chip_types->chip.irq_ack = al_fic_irq_noop;
> +	gc->chip_types->chip.irq_pre_redirect = al_fic_irq_pre_redirect;
> +	gc->chip_types->chip.irq_set_affinity = irq_chip_redirect_set_affinity;
> +#else
> +	gc->chip_types->chip.irq_ack = irq_gc_ack_clr_bit;
> +#endif
>  	gc->chip_types->chip.flags = IRQCHIP_SKIP_SET_WAKE;
>  	gc->private = fic;
>  

Reviewed-by: Radu Rendec <radu@xxxxxxxxxx>