Re: [PATCH v4 3/5] irqchip/mips-gic: Transfer interrupt mask state across clusters

[Thomas Bogendoerfer]

On Tue, Sep 29, 2026 at 02:14:10PM +0200, Benoît Monin wrote:
> When an interrupt's affinity is moved to a CPU in another cluster,
> gic_set_affinity() updates the routing (GIC_SH_MAP_VP) and trigger type
> in the destination cluster, but never touched the interrupt's mask state.
> 
> The interrupt mask is per-cluster. After such a move the interrupt may
> be left disabled in the destination cluster, so it never fires despite
> being correctly routed to its new VP.
> 
> Move the mask state along with the interrupt: disable it in the old
> cluster while clearing the route so it is no longer delivered, then
> configure the trigger type in the new cluster and re-enable it there
> if it was enabled in the old cluster.
> 
> Fixes: 322a90638768 ("irqchip/mips-gic: Multi-cluster support")
> Signed-off-by: Benoît Monin <benoit.monin@xxxxxxxxxxx>
> ---
>  drivers/irqchip/irq-mips-gic.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
> index f2ae60d39d66..be38989d8e73 100644
> --- a/drivers/irqchip/irq-mips-gic.c
> +++ b/drivers/irqchip/irq-mips-gic.c
> @@ -368,6 +368,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *cpumask,
>  	unsigned int irq = GIC_HWIRQ_TO_SHARED(d->hwirq);
>  	unsigned int cpu, cl, old_cpu, old_cl;
>  	unsigned long flags;
> +	bool enabled;
>  
>  	/*
>  	 * The GIC specifies that we can only route an interrupt to one VP(E),
> @@ -389,15 +390,20 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *cpumask,
>  	raw_spin_lock_irqsave(&gic_lock, flags);
>  
>  	/*
> -	 * If we're moving affinity between clusters, stop routing the
> -	 * interrupt to any VP(E) in the old cluster.
> +	 * If we're moving affinity between clusters, save the interrupt's
> +	 * mask state, stop routing it to any VP(E) in the old cluster and
> +	 * disable it there so it is no longer delivered.
>  	 */
>  	if (cl != old_cl) {
>  		if (gic_irq_lock_cluster(d)) {
> +			enabled = read_gic_redir_mask(irq);
>  			write_gic_redir_map_vp(irq, 0);
> +			write_gic_redir_rmask(irq);
>  			mips_cm_unlock_other();
>  		} else {
> +			enabled = read_gic_mask(irq);
>  			write_gic_map_vp(irq, 0);
> +			write_gic_rmask(irq);
>  		}
>  	}
>  
> @@ -409,10 +415,20 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *cpumask,
>  
>  	/*
>  	 * If we're moving affinity between clusters, configure the interrupt
> -	 * trigger type in the new cluster.
> +	 * trigger type and, if it was enabled in the old cluster, enable it
> +	 * in the new one.
>  	 */
> -	if (cl != old_cl)
> +	if (cl != old_cl) {
>  		gic_set_type_locked(d, irqd_get_trigger_type(d));
> +		if (enabled) {
> +			if (gic_irq_lock_cluster(d)) {
> +				write_gic_redir_smask(irq);
> +				mips_cm_unlock_other();
> +			} else {
> +				write_gic_smask(irq);
> +			}
> +		}
> +	}
>  
>  	/* Route the interrupt to its new VP(E) */
>  	if (gic_irq_lock_cluster(d)) {
> 
> -- 
> 2.55.0

Reviewed-by: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]