mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] MIPS: SB1250: Validate and update IRQ affinity
@ 2026-11-03 21:50 Florian Fainelli
  2026-11-05 15:44 ` Thomas Bogendoerfer
  2026-11-06  6:00 ` Maciej W. Rozycki
  0 siblings, 2 replies; 5+ messages in thread
From: Florian Fainelli @ 2026-11-03 21:50 UTC (permalink / raw)
  To: linux-mips
  Cc: macro, Florian Fainelli, Thomas Bogendoerfer, Thomas Gleixner, open list

sb1250_set_affinity() selects the first online CPU in the requested
affinity mask without checking whether such a CPU exists. If the
intersection is empty, the resulting CPU index is invalid and must
not be passed to cpu_logical_map().

Reject affinity masks containing no online CPU with -EINVAL before
accessing the CPU mapping or interrupt mapper registers.

Also update the effective affinity mask to reflect the single logical
CPU selected as the interrupt target.

Fixes: 421d1563c662 ("mips: Validate online cpus in irq_set_affinity() callbacks")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/mips/sibyte/sb1250/irq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/mips/sibyte/sb1250/irq.c b/arch/mips/sibyte/sb1250/irq.c
index 2f08ad267a11..d7a656cd7ec0 100644
--- a/arch/mips/sibyte/sb1250/irq.c
+++ b/arch/mips/sibyte/sb1250/irq.c
@@ -76,6 +76,8 @@ static int sb1250_set_affinity(struct irq_data *d, const struct cpumask *mask,
 	unsigned long flags;
 
 	i = cpumask_first_and(mask, cpu_online_mask);
+	if (i >= nr_cpu_ids)
+		return -EINVAL;
 
 	/* Convert logical CPU to physical CPU */
 	cpu = cpu_logical_map(i);
@@ -103,6 +105,7 @@ static int sb1250_set_affinity(struct irq_data *d, const struct cpumask *mask,
 		____raw_writeq(cur_ints, IOADDR(A_IMR_MAPPER(cpu) +
 					R_IMR_INTERRUPT_MASK));
 	}
+	irq_data_update_effective_affinity(d, cpumask_of(i));
 	raw_spin_unlock_irqrestore(&sb1250_imr_lock, flags);
 
 	return 0;
-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: SB1250: Validate and update IRQ affinity
  2026-11-03 21:50 [PATCH] MIPS: SB1250: Validate and update IRQ affinity Florian Fainelli
@ 2026-11-05 15:44 ` Thomas Bogendoerfer
  2026-11-06  6:00 ` Maciej W. Rozycki
  1 sibling, 0 replies; 5+ messages in thread
From: Thomas Bogendoerfer @ 2026-11-05 15:44 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: linux-mips, macro, Thomas Gleixner, open list

On Tue, Nov 03, 2026 at 01:50:40PM -0800, Florian Fainelli wrote:
> sb1250_set_affinity() selects the first online CPU in the requested
> affinity mask without checking whether such a CPU exists. If the
> intersection is empty, the resulting CPU index is invalid and must
> not be passed to cpu_logical_map().
> 
> Reject affinity masks containing no online CPU with -EINVAL before
> accessing the CPU mapping or interrupt mapper registers.
> 
> Also update the effective affinity mask to reflect the single logical
> CPU selected as the interrupt target.
> 
> Fixes: 421d1563c662 ("mips: Validate online cpus in irq_set_affinity() callbacks")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/mips/sibyte/sb1250/irq.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/mips/sibyte/sb1250/irq.c b/arch/mips/sibyte/sb1250/irq.c
> index 2f08ad267a11..d7a656cd7ec0 100644
> --- a/arch/mips/sibyte/sb1250/irq.c
> +++ b/arch/mips/sibyte/sb1250/irq.c
> @@ -76,6 +76,8 @@ static int sb1250_set_affinity(struct irq_data *d, const struct cpumask *mask,
>  	unsigned long flags;
>  
>  	i = cpumask_first_and(mask, cpu_online_mask);
> +	if (i >= nr_cpu_ids)
> +		return -EINVAL;
>  
>  	/* Convert logical CPU to physical CPU */
>  	cpu = cpu_logical_map(i);
> @@ -103,6 +105,7 @@ static int sb1250_set_affinity(struct irq_data *d, const struct cpumask *mask,
>  		____raw_writeq(cur_ints, IOADDR(A_IMR_MAPPER(cpu) +
>  					R_IMR_INTERRUPT_MASK));
>  	}
> +	irq_data_update_effective_affinity(d, cpumask_of(i));
>  	raw_spin_unlock_irqrestore(&sb1250_imr_lock, flags);
>  
>  	return 0;
> -- 
> 2.53.0

applied to mips-fixes

Thomas.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: SB1250: Validate and update IRQ affinity
  2026-11-03 21:50 [PATCH] MIPS: SB1250: Validate and update IRQ affinity Florian Fainelli
  2026-11-05 15:44 ` Thomas Bogendoerfer
@ 2026-11-06  6:00 ` Maciej W. Rozycki
  2026-11-09  6:17   ` Maciej W. Rozycki
  1 sibling, 1 reply; 5+ messages in thread
From: Maciej W. Rozycki @ 2026-11-06  6:00 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-mips, Thomas Bogendoerfer, Thomas Gleixner, open list

On Tue, 3 Nov 2026, Florian Fainelli wrote:

> sb1250_set_affinity() selects the first online CPU in the requested
> affinity mask without checking whether such a CPU exists. If the
> intersection is empty, the resulting CPU index is invalid and must
> not be passed to cpu_logical_map().
> 
> Reject affinity masks containing no online CPU with -EINVAL before
> accessing the CPU mapping or interrupt mapper registers.

 Why do you think this is needed?  The only case where an unvalidated mask 
is passed is AFAICT with:

/**
 * irq_force_affinity - Force the irq affinity of a given irq
 * @irq:	Interrupt to set affinity
 * @cpumask:	cpumask
 *
 * Same as irq_set_affinity, but without checking the mask against
 * online cpus.
 *
 * Solely for low level cpu hotplug code, where we need to make per
 * cpu interrupts affine before the cpu becomes online.
 */

-- which does not apply here (nor for arch/mips/sibyte/bcm1480/irq.c, 
which uses analogous code and should otherwise be updated accordingly) 
since all the CPUs are onchip the SoC.

 Have I missed anything?

> Also update the effective affinity mask to reflect the single logical
> CPU selected as the interrupt target.

 Likewise, why is it needed?  This code returns IRQ_SET_MASK_OK (should 
really use the enumeration constant), so the caller will do the copy.

  Maciej

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: SB1250: Validate and update IRQ affinity
  2026-11-06  6:00 ` Maciej W. Rozycki
@ 2026-11-09  6:17   ` Maciej W. Rozycki
  2026-11-09 17:43     ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: Maciej W. Rozycki @ 2026-11-09  6:17 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-mips, Thomas Bogendoerfer, Thomas Gleixner, open list

On Fri, 6 Nov 2026, Maciej W. Rozycki wrote:

> > sb1250_set_affinity() selects the first online CPU in the requested
> > affinity mask without checking whether such a CPU exists. If the
> > intersection is empty, the resulting CPU index is invalid and must
> > not be passed to cpu_logical_map().
> > 
> > Reject affinity masks containing no online CPU with -EINVAL before
> > accessing the CPU mapping or interrupt mapper registers.
> 
>  Why do you think this is needed?  The only case where an unvalidated mask 
> is passed is AFAICT with:
> 
> /**
>  * irq_force_affinity - Force the irq affinity of a given irq
>  * @irq:	Interrupt to set affinity
>  * @cpumask:	cpumask
>  *
>  * Same as irq_set_affinity, but without checking the mask against
>  * online cpus.
>  *
>  * Solely for low level cpu hotplug code, where we need to make per
>  * cpu interrupts affine before the cpu becomes online.
>  */
> 
> -- which does not apply here (nor for arch/mips/sibyte/bcm1480/irq.c, 
> which uses analogous code and should otherwise be updated accordingly) 
> since all the CPUs are onchip the SoC.
> 
>  Have I missed anything?
> 
> > Also update the effective affinity mask to reflect the single logical
> > CPU selected as the interrupt target.
> 
>  Likewise, why is it needed?  This code returns IRQ_SET_MASK_OK (should 
> really use the enumeration constant), so the caller will do the copy.

 I can see it went to Linus already.  Can we please have it reverted?

  Maciej

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: SB1250: Validate and update IRQ affinity
  2026-11-09  6:17   ` Maciej W. Rozycki
@ 2026-11-09 17:43     ` Florian Fainelli
  0 siblings, 0 replies; 5+ messages in thread
From: Florian Fainelli @ 2026-11-09 17:43 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: linux-mips, Thomas Bogendoerfer, Thomas Gleixner, open list

Hi Maciej,

On 11/8/26 22:17, Maciej W. Rozycki wrote:
> On Fri, 6 Nov 2026, Maciej W. Rozycki wrote:
> 
>>> sb1250_set_affinity() selects the first online CPU in the requested
>>> affinity mask without checking whether such a CPU exists. If the
>>> intersection is empty, the resulting CPU index is invalid and must
>>> not be passed to cpu_logical_map().
>>>
>>> Reject affinity masks containing no online CPU with -EINVAL before
>>> accessing the CPU mapping or interrupt mapper registers.
>>
>>   Why do you think this is needed?  The only case where an unvalidated mask
>> is passed is AFAICT with:
>>
>> /**
>>   * irq_force_affinity - Force the irq affinity of a given irq
>>   * @irq:	Interrupt to set affinity
>>   * @cpumask:	cpumask
>>   *
>>   * Same as irq_set_affinity, but without checking the mask against
>>   * online cpus.
>>   *
>>   * Solely for low level cpu hotplug code, where we need to make per
>>   * cpu interrupts affine before the cpu becomes online.
>>   */
>>
>> -- which does not apply here (nor for arch/mips/sibyte/bcm1480/irq.c,
>> which uses analogous code and should otherwise be updated accordingly)
>> since all the CPUs are onchip the SoC.
>>
>>   Have I missed anything?
>>
>>> Also update the effective affinity mask to reflect the single logical
>>> CPU selected as the interrupt target.
>>
>>   Likewise, why is it needed?  This code returns IRQ_SET_MASK_OK (should
>> really use the enumeration constant), so the caller will do the copy.
> 
>   I can see it went to Linus already.  Can we please have it reverted?

Apologies for the late response, I was hoping Thomas would allow us to 
discuss this before applying. The effective-affinity update addresses 
this warning observed during boot:

genirq: irq_chip SB1250-IMR did not update eff. affinity mask of irq 2

IRQ_SET_MASK_OK causes irq_do_set_affinity() to copy the requested mask 
into irq_common_data.affinity, but it does not populate the separate 
effective_affinity mask when CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK is 
enabled. irq_validate_effective_affinity() then finds that mask empty 
and emits the warning.

Since the mapper selects a single CPU, 
irq_data_update_effective_affinity(d, cpumask_of(i)) records the actual 
target and addresses that warning. I should have included this in the 
commit message.

You are right about the empty-mask check, I will remove that part, 
revise the commit message accordingly, and use IRQ_SET_MASK_OK 
explicitly. BCM1480 should receive the analogous effective-affinity 
update as well.
-- 
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-09 17:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 21:50 [PATCH] MIPS: SB1250: Validate and update IRQ affinity Florian Fainelli
2026-11-05 15:44 ` Thomas Bogendoerfer
2026-11-06  6:00 ` Maciej W. Rozycki
2026-11-09  6:17   ` Maciej W. Rozycki
2026-11-09 17:43     ` Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®