[patch 1/5] ia64: Validate online cpus in irq_set_affinity() callbacks

[patch 1/5] ia64: Validate online cpus in irq_set_affinity() callbacks

[Thomas Gleixner]

[-- Attachment #1: ia64-filter-offline-cpus-in-irq-set-affinity.patch --]
[-- Type: text/plain, Size: 2350 bytes --]

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: ia64 <linux-ia64@vger.kernel.org>
---
 arch/ia64/kernel/msi_ia64.c  |    5 +----
 arch/ia64/sn/kernel/irq.c    |    4 ++--
 arch/ia64/sn/kernel/msi_sn.c |    2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

Index: tip/arch/ia64/kernel/msi_ia64.c
===================================================================
--- tip.orig/arch/ia64/kernel/msi_ia64.c
+++ tip/arch/ia64/kernel/msi_ia64.c
@@ -17,12 +17,9 @@ static int ia64_set_msi_irq_affinity(str
 {
 	struct msi_msg msg;
 	u32 addr, data;
-	int cpu = first_cpu(*cpu_mask);
+	int cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	unsigned int irq = idata->irq;
 
-	if (!cpu_online(cpu))
-		return -1;
-
 	if (irq_prepare_move(irq, cpu))
 		return -1;
 
Index: tip/arch/ia64/sn/kernel/irq.c
===================================================================
--- tip.orig/arch/ia64/sn/kernel/irq.c
+++ tip/arch/ia64/sn/kernel/irq.c
@@ -209,8 +209,8 @@ static int sn_set_affinity_irq(struct ir
 	nasid_t nasid;
 	int slice;
 
-	nasid = cpuid_to_nasid(cpumask_first(mask));
-	slice = cpuid_to_slice(cpumask_first(mask));
+	nasid = cpuid_to_nasid(cpumask_first_and(mask, cpu_online_mask));
+	slice = cpuid_to_slice(cpumask_first_and(mask, cpu_online_mask));
 
 	list_for_each_entry_safe(sn_irq_info, sn_irq_info_safe,
 				 sn_irq_lh[irq], list)
Index: tip/arch/ia64/sn/kernel/msi_sn.c
===================================================================
--- tip.orig/arch/ia64/sn/kernel/msi_sn.c
+++ tip/arch/ia64/sn/kernel/msi_sn.c
@@ -166,7 +166,7 @@ static int sn_set_msi_irq_affinity(struc
 	struct sn_pcibus_provider *provider;
 	unsigned int cpu, irq = data->irq;
 
-	cpu = cpumask_first(cpu_mask);
+	cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	sn_irq_info = sn_msi_info[irq].sn_irq_info;
 	if (sn_irq_info == NULL || sn_irq_info->irq_int_bit >= 0)
 		return -1;

[patch 0/5] genirq: Sanitize irq_set_affinity callbacks

[Thomas Gleixner]

In my last tree wide sweep of irq controller related oddities I
discovered that some controllers have an broken implementation versus
selecting a single target cpu from the given cpu mask.

The core merily guarantees that at least one of the cpus in the mask
is online.

If the controller callback does not restrict the search for a single
target to the online cpus it might run into the following situation:

     Assume 4 cores online: online mask = 0x0f

     Offline Core 1 	    online mask = 0x0d

     Set affinity to 0xe    is valid due to online mask 0xd

     cpumask_first(0xe)     picks core 1, which is offline

The following series takes care of this. The patches should be tagged
stable if there are no objections.

I'm pondering to extend the core functionality to select a target cpu
before calling into irq_set_affinity() to prevent future copy and
paste failures and get rid of all the duplicated code.

Thanks,

	tglx

[patch 1/5] ia64: Validate online cpus in irq_set_affinity() callbacks

[Thomas Gleixner]

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: ia64 <linux-ia64@vger.kernel.org>
---
 arch/ia64/kernel/msi_ia64.c  |    5 +----
 arch/ia64/sn/kernel/irq.c    |    4 ++--
 arch/ia64/sn/kernel/msi_sn.c |    2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

Index: tip/arch/ia64/kernel/msi_ia64.c
=================================--- tip.orig/arch/ia64/kernel/msi_ia64.c
+++ tip/arch/ia64/kernel/msi_ia64.c
@@ -17,12 +17,9 @@ static int ia64_set_msi_irq_affinity(str
 {
 	struct msi_msg msg;
 	u32 addr, data;
-	int cpu = first_cpu(*cpu_mask);
+	int cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	unsigned int irq = idata->irq;
 
-	if (!cpu_online(cpu))
-		return -1;
-
 	if (irq_prepare_move(irq, cpu))
 		return -1;
 
Index: tip/arch/ia64/sn/kernel/irq.c
=================================--- tip.orig/arch/ia64/sn/kernel/irq.c
+++ tip/arch/ia64/sn/kernel/irq.c
@@ -209,8 +209,8 @@ static int sn_set_affinity_irq(struct ir
 	nasid_t nasid;
 	int slice;
 
-	nasid = cpuid_to_nasid(cpumask_first(mask));
-	slice = cpuid_to_slice(cpumask_first(mask));
+	nasid = cpuid_to_nasid(cpumask_first_and(mask, cpu_online_mask));
+	slice = cpuid_to_slice(cpumask_first_and(mask, cpu_online_mask));
 
 	list_for_each_entry_safe(sn_irq_info, sn_irq_info_safe,
 				 sn_irq_lh[irq], list)
Index: tip/arch/ia64/sn/kernel/msi_sn.c
=================================--- tip.orig/arch/ia64/sn/kernel/msi_sn.c
+++ tip/arch/ia64/sn/kernel/msi_sn.c
@@ -166,7 +166,7 @@ static int sn_set_msi_irq_affinity(struc
 	struct sn_pcibus_provider *provider;
 	unsigned int cpu, irq = data->irq;
 
-	cpu = cpumask_first(cpu_mask);
+	cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	sn_irq_info = sn_msi_info[irq].sn_irq_info;
 	if (sn_irq_info = NULL || sn_irq_info->irq_int_bit >= 0)
 		return -1;

[patch 2/5] mips: Validate online cpus in irq_set_affinity() callbacks

[Thomas Gleixner]

[-- Attachment #1: mips-validate-online-cpus-in-irq-set-affinity-callback.patch --]
[-- Type: text/plain, Size: 1521 bytes --]

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/sibyte/bcm1480/irq.c |    2 +-
 arch/mips/sibyte/sb1250/irq.c  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: tip/arch/mips/sibyte/bcm1480/irq.c
===================================================================
--- tip.orig/arch/mips/sibyte/bcm1480/irq.c
+++ tip/arch/mips/sibyte/bcm1480/irq.c
@@ -95,7 +95,7 @@ static int bcm1480_set_affinity(struct i
 	u64 cur_ints;
 	unsigned long flags;
 
-	i = cpumask_first(mask);
+	i = cpumask_first_and(mask< cpu_online_mask);
 
 	/* Convert logical CPU to physical CPU */
 	cpu = cpu_logical_map(i);
Index: tip/arch/mips/sibyte/sb1250/irq.c
===================================================================
--- tip.orig/arch/mips/sibyte/sb1250/irq.c
+++ tip/arch/mips/sibyte/sb1250/irq.c
@@ -88,7 +88,7 @@ static int sb1250_set_affinity(struct ir
 	u64 cur_ints;
 	unsigned long flags;
 
-	i = cpumask_first(mask);
+	i = cpumask_first_and(mask< cpu_online_mask);
 
 	/* Convert logical CPU to physical CPU */
 	cpu = cpu_logical_map(i);

[patch 4/5] xen: Validate online cpus in set_affinity

[Thomas Gleixner]

[-- Attachment #1: xen-validate-online-cpus-in-set-affinity.patch --]
[-- Type: text/plain, Size: 1177 bytes --]

The user space interface does not filter out offline cpus. It merily
verifies that the mask contains at least one online cpu. So the
selector in the irq chip implementation needs to make sure to pick
only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe
     Selector will pick first set bit, i.e. core 1

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Xen <xen-devel@lists.xenproject.org>
---
 drivers/xen/events/events_base.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: tip/drivers/xen/events/events_base.c
===================================================================
--- tip.orig/drivers/xen/events/events_base.c
+++ tip/drivers/xen/events/events_base.c
@@ -1324,7 +1324,7 @@ static int rebind_irq_to_cpu(unsigned ir
 static int set_affinity_irq(struct irq_data *data, const struct cpumask *dest,
 			    bool force)
 {
-	unsigned tcpu = cpumask_first(dest);
+	unsigned tcpu = cpumask_first_and(dest, cpu_online_mask);
 
 	return rebind_irq_to_cpu(data->irq, tcpu);
 }

[patch 3/5] parisc: Validate online cpus in irq_set_affinity() callbacks

[Thomas Gleixner]

[-- Attachment #1: parisc-validate-online-cpus-in-irq-set-affinity-callback.patch --]
[-- Type: text/plain, Size: 1058 bytes --]

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "James E.J. Bottomley" <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
---
 arch/parisc/kernel/irq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: tip/arch/parisc/kernel/irq.c
===================================================================
--- tip.orig/arch/parisc/kernel/irq.c
+++ tip/arch/parisc/kernel/irq.c
@@ -117,7 +117,7 @@ int cpu_check_affinity(struct irq_data *
 		return -EINVAL;
 
 	/* whatever mask they set, we just allow one CPU */
-	cpu_dest = first_cpu(*dest);
+	cpu_dest = cpumask_first_and(dest, cpu_online_mask);
 
 	return cpu_dest;
 }

[patch 4/5] xen: Validate online cpus in set_affinity

[Thomas Gleixner]

[-- Attachment #1: xen-validate-online-cpus-in-set-affinity.patch --]
[-- Type: text/plain, Size: 1175 bytes --]

The user space interface does not filter out offline cpus. It merily
verifies that the mask contains at least one online cpu. So the
selector in the irq chip implementation needs to make sure to pick
only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe
     Selector will pick first set bit, i.e. core 1

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Xen <xen-devel@lists.xenproject.org>
---
 drivers/xen/events/events_base.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: tip/drivers/xen/events/events_base.c
===================================================================
--- tip.orig/drivers/xen/events/events_base.c
+++ tip/drivers/xen/events/events_base.c
@@ -1324,7 +1324,7 @@ static int rebind_irq_to_cpu(unsigned ir
 static int set_affinity_irq(struct irq_data *data, const struct cpumask *dest,
 			    bool force)
 {
-	unsigned tcpu = cpumask_first(dest);
+	unsigned tcpu = cpumask_first_and(dest, cpu_online_mask);
 
 	return rebind_irq_to_cpu(data->irq, tcpu);
 }

[patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Thomas Gleixner]

[-- Attachment #1: irqchip-armanda-sanitize-irq-affinity-logic.patch --]
[-- Type: text/plain, Size: 2980 bytes --]

The set_irq_affinity() function has two issues:

1) It has no protection against selecting an offline cpu from the
   given mask.

2) It pointlessly restricts the affinity masks to have a single cpu
   set. This collides with the irq migration code of arm.

   irq affinity is set to core 3
   core 3 goes offline

   migration code sets mask to cpu_online_mask and calls the
   irq_set_affinity() callback of the irq_chip which fails due to bit
   0,1,2 set.

So instead of doing silly for_each_cpu() loops just pick any bit of
the mask which intersects with the online mask.

The read back of the routing register is pointless as well. We can
simply write the new mask as the bits in this register reflect one
core.

Get rid of fiddling with the default_irq_affinity as well.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: Jason Cooper <jason@lakedaemon.net>

---
 drivers/irqchip/irq-armada-370-xp.c |   38 ++++--------------------------------
 1 file changed, 5 insertions(+), 33 deletions(-)

Index: tip/drivers/irqchip/irq-armada-370-xp.c
===================================================================
--- tip.orig/drivers/irqchip/irq-armada-370-xp.c
+++ tip/drivers/irqchip/irq-armada-370-xp.c
@@ -244,35 +244,16 @@ static DEFINE_RAW_SPINLOCK(irq_controlle
 static int armada_xp_set_affinity(struct irq_data *d,
 				  const struct cpumask *mask_val, bool force)
 {
-	unsigned long reg;
-	unsigned long new_mask = 0;
-	unsigned long online_mask = 0;
-	unsigned long count = 0;
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	unsigned long mask;
 	int cpu;
 
-	for_each_cpu(cpu, mask_val) {
-		new_mask |= 1 << cpu_logical_map(cpu);
-		count++;
-	}
-
-	/*
-	 * Forbid mutlicore interrupt affinity
-	 * This is required since the MPIC HW doesn't limit
-	 * several CPUs from acknowledging the same interrupt.
-	 */
-	if (count > 1)
-		return -EINVAL;
-
-	for_each_cpu(cpu, cpu_online_mask)
-		online_mask |= 1 << cpu_logical_map(cpu);
+	/* Select a single core from the affinity mask which is online */
+	cpu = cpumask_any_and(mask_val, cpu_online_mask);
+	mask = 1UL << cpu_logical_map(cpu);
 
 	raw_spin_lock(&irq_controller_lock);
-
-	reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
-	reg = (reg & (~online_mask)) | new_mask;
-	writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
-
+	writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
 	raw_spin_unlock(&irq_controller_lock);
 
 	return 0;
@@ -494,15 +475,6 @@ static int __init armada_370_xp_mpic_of_
 
 #ifdef CONFIG_SMP
 	armada_xp_mpic_smp_cpu_init();
-
-	/*
-	 * Set the default affinity from all CPUs to the boot cpu.
-	 * This is required since the MPIC doesn't limit several CPUs
-	 * from acknowledging the same interrupt.
-	 */
-	cpumask_clear(irq_default_affinity);
-	cpumask_set_cpu(smp_processor_id(), irq_default_affinity);
-
 #endif
 
 	armada_370_xp_msi_init(node, main_int_res.start);

Re: [patch 4/5] xen: Validate online cpus in set_affinity

[David Vrabel]

On 04/03/14 20:43, Thomas Gleixner wrote:
> The user space interface does not filter out offline cpus. It merily
> verifies that the mask contains at least one online cpu. So the
> selector in the irq chip implementation needs to make sure to pick
> only an online cpu because otherwise:
> 
>      Offline Core 1
>      Set affinity to 0xe
>      Selector will pick first set bit, i.e. core 1

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

Thanks.

David

Re: [patch 4/5] xen: Validate online cpus in set_affinity

[David Vrabel]

On 04/03/14 20:43, Thomas Gleixner wrote:
> The user space interface does not filter out offline cpus. It merily
> verifies that the mask contains at least one online cpu. So the
> selector in the irq chip implementation needs to make sure to pick
> only an online cpu because otherwise:
> 
>      Offline Core 1
>      Set affinity to 0xe
>      Selector will pick first set bit, i.e. core 1

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

Thanks.

David

Re: [patch 2/5] mips: Validate online cpus in irq_set_affinity() callbacks

[Peter Zijlstra]

On Tue, Mar 04, 2014 at 08:43:39PM -0000, Thomas Gleixner wrote:
> +	i = cpumask_first_and(mask< cpu_online_mask);
> +	i = cpumask_first_and(mask< cpu_online_mask);

s/</,/ ?

Re: [patch 2/5] mips: Validate online cpus in irq_set_affinity() callbacks

[Thomas Gleixner]

On Wed, 5 Mar 2014, Peter Zijlstra wrote:

> On Tue, Mar 04, 2014 at 08:43:39PM -0000, Thomas Gleixner wrote:
> > +	i = cpumask_first_and(mask< cpu_online_mask);
> > +	i = cpumask_first_and(mask< cpu_online_mask);
> 
> s/</,/ ?

Duh, yes. 

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Jason Cooper]

Thomas,

nit: s/armanda/armada/ in the patch subject.

Gregory,

Mind providing an Ack on this?

thx,

Jason.

On Tue, Mar 04, 2014 at 08:43:41PM -0000, Thomas Gleixner wrote:
> The set_irq_affinity() function has two issues:
> 
> 1) It has no protection against selecting an offline cpu from the
>    given mask.
> 
> 2) It pointlessly restricts the affinity masks to have a single cpu
>    set. This collides with the irq migration code of arm.
> 
>    irq affinity is set to core 3
>    core 3 goes offline
> 
>    migration code sets mask to cpu_online_mask and calls the
>    irq_set_affinity() callback of the irq_chip which fails due to bit
>    0,1,2 set.
> 
> So instead of doing silly for_each_cpu() loops just pick any bit of
> the mask which intersects with the online mask.
> 
> The read back of the routing register is pointless as well. We can
> simply write the new mask as the bits in this register reflect one
> core.
> 
> Get rid of fiddling with the default_irq_affinity as well.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> 
> ---
>  drivers/irqchip/irq-armada-370-xp.c |   38 ++++--------------------------------
>  1 file changed, 5 insertions(+), 33 deletions(-)
> 
> Index: tip/drivers/irqchip/irq-armada-370-xp.c
> ===================================================================
> --- tip.orig/drivers/irqchip/irq-armada-370-xp.c
> +++ tip/drivers/irqchip/irq-armada-370-xp.c
> @@ -244,35 +244,16 @@ static DEFINE_RAW_SPINLOCK(irq_controlle
>  static int armada_xp_set_affinity(struct irq_data *d,
>  				  const struct cpumask *mask_val, bool force)
>  {
> -	unsigned long reg;
> -	unsigned long new_mask = 0;
> -	unsigned long online_mask = 0;
> -	unsigned long count = 0;
>  	irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +	unsigned long mask;
>  	int cpu;
>  
> -	for_each_cpu(cpu, mask_val) {
> -		new_mask |= 1 << cpu_logical_map(cpu);
> -		count++;
> -	}
> -
> -	/*
> -	 * Forbid mutlicore interrupt affinity
> -	 * This is required since the MPIC HW doesn't limit
> -	 * several CPUs from acknowledging the same interrupt.
> -	 */
> -	if (count > 1)
> -		return -EINVAL;
> -
> -	for_each_cpu(cpu, cpu_online_mask)
> -		online_mask |= 1 << cpu_logical_map(cpu);
> +	/* Select a single core from the affinity mask which is online */
> +	cpu = cpumask_any_and(mask_val, cpu_online_mask);
> +	mask = 1UL << cpu_logical_map(cpu);
>  
>  	raw_spin_lock(&irq_controller_lock);
> -
> -	reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> -	reg = (reg & (~online_mask)) | new_mask;
> -	writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> -
> +	writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>  	raw_spin_unlock(&irq_controller_lock);
>  
>  	return 0;
> @@ -494,15 +475,6 @@ static int __init armada_370_xp_mpic_of_
>  
>  #ifdef CONFIG_SMP
>  	armada_xp_mpic_smp_cpu_init();
> -
> -	/*
> -	 * Set the default affinity from all CPUs to the boot cpu.
> -	 * This is required since the MPIC doesn't limit several CPUs
> -	 * from acknowledging the same interrupt.
> -	 */
> -	cpumask_clear(irq_default_affinity);
> -	cpumask_set_cpu(smp_processor_id(), irq_default_affinity);
> -
>  #endif
>  
>  	armada_370_xp_msi_init(node, main_int_res.start);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Gregory CLEMENT]

On 06/03/2014 20:05, Jason Cooper wrote:
> Thomas,
> 
> nit: s/armanda/armada/ in the patch subject.
> 
> Gregory,
> 
> Mind providing an Ack on this?

Well sorry but with this patch the kernel doesn't
work anymore.

I am investigating to find if some part could be kept.

> 
> thx,
> 
> Jason.
> 
> On Tue, Mar 04, 2014 at 08:43:41PM -0000, Thomas Gleixner wrote:
>> The set_irq_affinity() function has two issues:
>>
>> 1) It has no protection against selecting an offline cpu from the
>>    given mask.
>>
>> 2) It pointlessly restricts the affinity masks to have a single cpu
>>    set. This collides with the irq migration code of arm.
>>
>>    irq affinity is set to core 3
>>    core 3 goes offline
>>
>>    migration code sets mask to cpu_online_mask and calls the
>>    irq_set_affinity() callback of the irq_chip which fails due to bit
>>    0,1,2 set.
>>
>> So instead of doing silly for_each_cpu() loops just pick any bit of
>> the mask which intersects with the online mask.
>>
>> The read back of the routing register is pointless as well. We can
>> simply write the new mask as the bits in this register reflect one
>> core.
>>
>> Get rid of fiddling with the default_irq_affinity as well.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Cc: Jason Cooper <jason@lakedaemon.net>
>>
>> ---
>>  drivers/irqchip/irq-armada-370-xp.c |   38 ++++--------------------------------
>>  1 file changed, 5 insertions(+), 33 deletions(-)
>>
>> Index: tip/drivers/irqchip/irq-armada-370-xp.c
>> ===================================================================
>> --- tip.orig/drivers/irqchip/irq-armada-370-xp.c
>> +++ tip/drivers/irqchip/irq-armada-370-xp.c
>> @@ -244,35 +244,16 @@ static DEFINE_RAW_SPINLOCK(irq_controlle
>>  static int armada_xp_set_affinity(struct irq_data *d,
>>  				  const struct cpumask *mask_val, bool force)
>>  {
>> -	unsigned long reg;
>> -	unsigned long new_mask = 0;
>> -	unsigned long online_mask = 0;
>> -	unsigned long count = 0;
>>  	irq_hw_number_t hwirq = irqd_to_hwirq(d);
>> +	unsigned long mask;
>>  	int cpu;
>>  
>> -	for_each_cpu(cpu, mask_val) {
>> -		new_mask |= 1 << cpu_logical_map(cpu);
>> -		count++;
>> -	}
>> -
>> -	/*
>> -	 * Forbid mutlicore interrupt affinity
>> -	 * This is required since the MPIC HW doesn't limit
>> -	 * several CPUs from acknowledging the same interrupt.
>> -	 */
>> -	if (count > 1)
>> -		return -EINVAL;
>> -
>> -	for_each_cpu(cpu, cpu_online_mask)
>> -		online_mask |= 1 << cpu_logical_map(cpu);
>> +	/* Select a single core from the affinity mask which is online */
>> +	cpu = cpumask_any_and(mask_val, cpu_online_mask);
>> +	mask = 1UL << cpu_logical_map(cpu);
>>  
>>  	raw_spin_lock(&irq_controller_lock);
>> -
>> -	reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>> -	reg = (reg & (~online_mask)) | new_mask;
>> -	writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>> -
>> +	writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>>  	raw_spin_unlock(&irq_controller_lock);
>>  
>>  	return 0;
>> @@ -494,15 +475,6 @@ static int __init armada_370_xp_mpic_of_
>>  
>>  #ifdef CONFIG_SMP
>>  	armada_xp_mpic_smp_cpu_init();
>> -
>> -	/*
>> -	 * Set the default affinity from all CPUs to the boot cpu.
>> -	 * This is required since the MPIC doesn't limit several CPUs
>> -	 * from acknowledging the same interrupt.
>> -	 */
>> -	cpumask_clear(irq_default_affinity);
>> -	cpumask_set_cpu(smp_processor_id(), irq_default_affinity);
>> -
>>  #endif
>>  
>>  	armada_370_xp_msi_init(node, main_int_res.start);
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Thomas Gleixner]

On Fri, 7 Mar 2014, Gregory CLEMENT wrote:

> On 06/03/2014 20:05, Jason Cooper wrote:
> > Thomas,
> > 
> > nit: s/armanda/armada/ in the patch subject.
> > 
> > Gregory,
> > 
> > Mind providing an Ack on this?
> 
> Well sorry but with this patch the kernel doesn't
> work anymore.
> 
> I am investigating to find if some part could be kept.

It might be the readback of the routing register. I don't have the
datasheet of this.
 
Thanks,

	tglx

[tip:irq/core] ia64: Validate online cpus in irq_set_affinity() callbacks

[tip-bot for Thomas Gleixner]

Commit-ID:  785aebd0cfff52e735ad4fd188d3726b5affc8e5
Gitweb:     http://git.kernel.org/tip/785aebd0cfff52e735ad4fd188d3726b5affc8e5
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 4 Mar 2014 20:43:38 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 12 Mar 2014 13:07:40 +0100

ia64: Validate online cpus in irq_set_affinity() callbacks

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: ia64 <linux-ia64@vger.kernel.org>
Link: http://lkml.kernel.org/r/20140304203100.650414633@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/ia64/kernel/msi_ia64.c  | 10 ++--------
 arch/ia64/sn/kernel/irq.c    |  4 ++--
 arch/ia64/sn/kernel/msi_sn.c |  2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index fb2f1e6..c430f91 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -17,12 +17,9 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
 {
 	struct msi_msg msg;
 	u32 addr, data;
-	int cpu = first_cpu(*cpu_mask);
+	int cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	unsigned int irq = idata->irq;
 
-	if (!cpu_online(cpu))
-		return -1;
-
 	if (irq_prepare_move(irq, cpu))
 		return -1;
 
@@ -139,10 +136,7 @@ static int dmar_msi_set_affinity(struct irq_data *data,
 	unsigned int irq = data->irq;
 	struct irq_cfg *cfg = irq_cfg + irq;
 	struct msi_msg msg;
-	int cpu = cpumask_first(mask);
-
-	if (!cpu_online(cpu))
-		return -1;
+	int cpu = cpumask_first_and(mask, cpu_online_mask);
 
 	if (irq_prepare_move(irq, cpu))
 		return -1;
diff --git a/arch/ia64/sn/kernel/irq.c b/arch/ia64/sn/kernel/irq.c
index 62cf4dd..85d0951 100644
--- a/arch/ia64/sn/kernel/irq.c
+++ b/arch/ia64/sn/kernel/irq.c
@@ -209,8 +209,8 @@ static int sn_set_affinity_irq(struct irq_data *data,
 	nasid_t nasid;
 	int slice;
 
-	nasid = cpuid_to_nasid(cpumask_first(mask));
-	slice = cpuid_to_slice(cpumask_first(mask));
+	nasid = cpuid_to_nasid(cpumask_first_and(mask, cpu_online_mask));
+	slice = cpuid_to_slice(cpumask_first_and(mask, cpu_online_mask));
 
 	list_for_each_entry_safe(sn_irq_info, sn_irq_info_safe,
 				 sn_irq_lh[irq], list)
diff --git a/arch/ia64/sn/kernel/msi_sn.c b/arch/ia64/sn/kernel/msi_sn.c
index 2b98b9e..afc58d2 100644
--- a/arch/ia64/sn/kernel/msi_sn.c
+++ b/arch/ia64/sn/kernel/msi_sn.c
@@ -166,7 +166,7 @@ static int sn_set_msi_irq_affinity(struct irq_data *data,
 	struct sn_pcibus_provider *provider;
 	unsigned int cpu, irq = data->irq;
 
-	cpu = cpumask_first(cpu_mask);
+	cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	sn_irq_info = sn_msi_info[irq].sn_irq_info;
 	if (sn_irq_info == NULL || sn_irq_info->irq_int_bit >= 0)
 		return -1;

[tip:irq/core] ia64: Validate online cpus in irq_set_affinity() callbacks

[tip-bot for Thomas Gleixner]

Commit-ID:  785aebd0cfff52e735ad4fd188d3726b5affc8e5
Gitweb:     http://git.kernel.org/tip/785aebd0cfff52e735ad4fd188d3726b5affc8e5
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 4 Mar 2014 20:43:38 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 12 Mar 2014 13:07:40 +0100

ia64: Validate online cpus in irq_set_affinity() callbacks

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: ia64 <linux-ia64@vger.kernel.org>
Link: http://lkml.kernel.org/r/20140304203100.650414633@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/ia64/kernel/msi_ia64.c  | 10 ++--------
 arch/ia64/sn/kernel/irq.c    |  4 ++--
 arch/ia64/sn/kernel/msi_sn.c |  2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index fb2f1e6..c430f91 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -17,12 +17,9 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
 {
 	struct msi_msg msg;
 	u32 addr, data;
-	int cpu = first_cpu(*cpu_mask);
+	int cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	unsigned int irq = idata->irq;
 
-	if (!cpu_online(cpu))
-		return -1;
-
 	if (irq_prepare_move(irq, cpu))
 		return -1;
 
@@ -139,10 +136,7 @@ static int dmar_msi_set_affinity(struct irq_data *data,
 	unsigned int irq = data->irq;
 	struct irq_cfg *cfg = irq_cfg + irq;
 	struct msi_msg msg;
-	int cpu = cpumask_first(mask);
-
-	if (!cpu_online(cpu))
-		return -1;
+	int cpu = cpumask_first_and(mask, cpu_online_mask);
 
 	if (irq_prepare_move(irq, cpu))
 		return -1;
diff --git a/arch/ia64/sn/kernel/irq.c b/arch/ia64/sn/kernel/irq.c
index 62cf4dd..85d0951 100644
--- a/arch/ia64/sn/kernel/irq.c
+++ b/arch/ia64/sn/kernel/irq.c
@@ -209,8 +209,8 @@ static int sn_set_affinity_irq(struct irq_data *data,
 	nasid_t nasid;
 	int slice;
 
-	nasid = cpuid_to_nasid(cpumask_first(mask));
-	slice = cpuid_to_slice(cpumask_first(mask));
+	nasid = cpuid_to_nasid(cpumask_first_and(mask, cpu_online_mask));
+	slice = cpuid_to_slice(cpumask_first_and(mask, cpu_online_mask));
 
 	list_for_each_entry_safe(sn_irq_info, sn_irq_info_safe,
 				 sn_irq_lh[irq], list)
diff --git a/arch/ia64/sn/kernel/msi_sn.c b/arch/ia64/sn/kernel/msi_sn.c
index 2b98b9e..afc58d2 100644
--- a/arch/ia64/sn/kernel/msi_sn.c
+++ b/arch/ia64/sn/kernel/msi_sn.c
@@ -166,7 +166,7 @@ static int sn_set_msi_irq_affinity(struct irq_data *data,
 	struct sn_pcibus_provider *provider;
 	unsigned int cpu, irq = data->irq;
 
-	cpu = cpumask_first(cpu_mask);
+	cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
 	sn_irq_info = sn_msi_info[irq].sn_irq_info;
 	if (sn_irq_info = NULL || sn_irq_info->irq_int_bit >= 0)
 		return -1;

[tip:irq/core] mips: Validate online cpus in irq_set_affinity() callbacks

[tip-bot for Thomas Gleixner]

Commit-ID:  421d1563c6620423d23e394711e3f209e585c161
Gitweb:     http://git.kernel.org/tip/421d1563c6620423d23e394711e3f209e585c161
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 4 Mar 2014 20:43:39 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 12 Mar 2014 13:07:40 +0100

mips: Validate online cpus in irq_set_affinity() callbacks

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Link: http://lkml.kernel.org/r/20140304203100.744800502@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/mips/sibyte/bcm1480/irq.c | 2 +-
 arch/mips/sibyte/sb1250/irq.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/sibyte/bcm1480/irq.c b/arch/mips/sibyte/bcm1480/irq.c
index 09d6e16..59cfe26 100644
--- a/arch/mips/sibyte/bcm1480/irq.c
+++ b/arch/mips/sibyte/bcm1480/irq.c
@@ -95,7 +95,7 @@ static int bcm1480_set_affinity(struct irq_data *d, const struct cpumask *mask,
 	u64 cur_ints;
 	unsigned long flags;
 
-	i = cpumask_first(mask);
+	i = cpumask_first_and(mask, cpu_online_mask);
 
 	/* Convert logical CPU to physical CPU */
 	cpu = cpu_logical_map(i);
diff --git a/arch/mips/sibyte/sb1250/irq.c b/arch/mips/sibyte/sb1250/irq.c
index fca0cdb..6d8dba5 100644
--- a/arch/mips/sibyte/sb1250/irq.c
+++ b/arch/mips/sibyte/sb1250/irq.c
@@ -88,7 +88,7 @@ static int sb1250_set_affinity(struct irq_data *d, const struct cpumask *mask,
 	u64 cur_ints;
 	unsigned long flags;
 
-	i = cpumask_first(mask);
+	i = cpumask_first_and(mask, cpu_online_mask);
 
 	/* Convert logical CPU to physical CPU */
 	cpu = cpu_logical_map(i);

[tip:irq/core] xen: Validate online cpus in set_affinity

[tip-bot for Thomas Gleixner]

Commit-ID:  753fbd23f5e59ea9dc0cabe0a684d32100a4af02
Gitweb:     http://git.kernel.org/tip/753fbd23f5e59ea9dc0cabe0a684d32100a4af02
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 4 Mar 2014 20:43:40 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 12 Mar 2014 13:07:41 +0100

xen: Validate online cpus in set_affinity

The user space interface does not filter out offline cpus. It merily
verifies that the mask contains at least one online cpu. So the
selector in the irq chip implementation needs to make sure to pick
only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe
     Selector will pick first set bit, i.e. core 1

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Xen <xen-devel@lists.xenproject.org>
Link: http://lkml.kernel.org/r/20140304203100.978031089@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/xen/events/events_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 8b91c256..c3458f5 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1324,7 +1324,7 @@ static int rebind_irq_to_cpu(unsigned irq, unsigned tcpu)
 static int set_affinity_irq(struct irq_data *data, const struct cpumask *dest,
 			    bool force)
 {
-	unsigned tcpu = cpumask_first(dest);
+	unsigned tcpu = cpumask_first_and(dest, cpu_online_mask);
 
 	return rebind_irq_to_cpu(data->irq, tcpu);
 }

[tip:irq/core] xen: Validate online cpus in set_affinity

[tip-bot for Thomas Gleixner]

Commit-ID:  753fbd23f5e59ea9dc0cabe0a684d32100a4af02
Gitweb:     http://git.kernel.org/tip/753fbd23f5e59ea9dc0cabe0a684d32100a4af02
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 4 Mar 2014 20:43:40 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 12 Mar 2014 13:07:41 +0100

xen: Validate online cpus in set_affinity

The user space interface does not filter out offline cpus. It merily
verifies that the mask contains at least one online cpu. So the
selector in the irq chip implementation needs to make sure to pick
only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe
     Selector will pick first set bit, i.e. core 1

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Xen <xen-devel@lists.xenproject.org>
Link: http://lkml.kernel.org/r/20140304203100.978031089@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/xen/events/events_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 8b91c256..c3458f5 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1324,7 +1324,7 @@ static int rebind_irq_to_cpu(unsigned irq, unsigned tcpu)
 static int set_affinity_irq(struct irq_data *data, const struct cpumask *dest,
 			    bool force)
 {
-	unsigned tcpu = cpumask_first(dest);
+	unsigned tcpu = cpumask_first_and(dest, cpu_online_mask);
 
 	return rebind_irq_to_cpu(data->irq, tcpu);
 }

[tip:irq/core] parisc: Validate online cpus in irq_set_affinity() callbacks

[tip-bot for Thomas Gleixner]

Commit-ID:  1ed71e59bca79e866c4bebbe1efc0bc18245119d
Gitweb:     http://git.kernel.org/tip/1ed71e59bca79e866c4bebbe1efc0bc18245119d
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 4 Mar 2014 20:43:40 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 12 Mar 2014 13:07:41 +0100

parisc: Validate online cpus in irq_set_affinity() callbacks

The [user space] interface does not filter out offline cpus. It merily
guarantees that the mask contains at least one online cpu.

So the selector in the irq chip implementation needs to make sure to
pick only an online cpu because otherwise:

     Offline Core 1
     Set affinity to 0xe (is valid due to online mask 0xd)
     cpumask_first will pick core 1, which is offline

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
Link: http://lkml.kernel.org/r/20140304203100.859489993@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/parisc/kernel/irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index 8ceac47..cfe056f 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -117,7 +117,7 @@ int cpu_check_affinity(struct irq_data *d, const struct cpumask *dest)
 		return -EINVAL;
 
 	/* whatever mask they set, we just allow one CPU */
-	cpu_dest = first_cpu(*dest);
+	cpu_dest = cpumask_first_and(dest, cpu_online_mask);
 
 	return cpu_dest;
 }

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Gregory CLEMENT]

Hi Thomas,

On 07/03/2014 18:17, Thomas Gleixner wrote:
> On Fri, 7 Mar 2014, Gregory CLEMENT wrote:
> 
>> On 06/03/2014 20:05, Jason Cooper wrote:
>>> Thomas,
>>>
>>> nit: s/armanda/armada/ in the patch subject.
>>>
>>> Gregory,
>>>
>>> Mind providing an Ack on this?
>>
>> Well sorry but with this patch the kernel doesn't
>> work anymore.
>>
>> I am investigating to find if some part could be kept.
> 
> It might be the readback of the routing register. I don't have the
> datasheet of this.

Sorry for the delay, I was on vacation without the hardware to test it.

Indeed it was the readback of the routing register. (Unfortunately the
datasheet was not yet publicly available :(  ). In your patch by replacing
the line:

writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));

by the following ones:

reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
reg = (reg & (~ARMADA_370_XP_INT_SOURCE_CPU_MASK)) | mask;
writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));

with
#define ARMADA_370_XP_INT_SOURCE_CPU_MASK	0xF

Then it worked.

About masking with the online CPU in the original code, the purpose
was to allow sharing the SoC with an other OS by doing AMP. This feature
is part of the Marvell version of the kernel.
The idea was to bring this latter in the vanilla kernel, but I am not sure
that all the part needed for AMP are acceptable for mainline. So I can add
it back later when we will need it.


Thanks,

Gregory


>  
> Thanks,
> 
> 	tglx
> 



-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Thomas Gleixner]

On Tue, 18 Mar 2014, Gregory CLEMENT wrote:
> On 07/03/2014 18:17, Thomas Gleixner wrote:
> > It might be the readback of the routing register. I don't have the
> > datasheet of this.
> 
> Sorry for the delay, I was on vacation without the hardware to test it.
> 
> Indeed it was the readback of the routing register. (Unfortunately the
> datasheet was not yet publicly available :(  ). In your patch by replacing
> the line:
> 
> writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> 
> by the following ones:
> 
> reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> reg = (reg & (~ARMADA_370_XP_INT_SOURCE_CPU_MASK)) | mask;
> writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> 
> with
> #define ARMADA_370_XP_INT_SOURCE_CPU_MASK	0xF
> 
> Then it worked.
> 
> About masking with the online CPU in the original code, the purpose
> was to allow sharing the SoC with an other OS by doing AMP. This feature
> is part of the Marvell version of the kernel.
> The idea was to bring this latter in the vanilla kernel, but I am not sure
> that all the part needed for AMP are acceptable for mainline. So I can add
> it back later when we will need it.

Right.

So are you ok with that patch (including your change) ? If you send me
tested/acked-by i'll route it for 3.15

Thanks,

	tglx

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Gregory CLEMENT]

On 18/03/2014 21:55, Thomas Gleixner wrote:
> On Tue, 18 Mar 2014, Gregory CLEMENT wrote:
>> On 07/03/2014 18:17, Thomas Gleixner wrote:
>>> It might be the readback of the routing register. I don't have the
>>> datasheet of this.
>>
>> Sorry for the delay, I was on vacation without the hardware to test it.
>>
>> Indeed it was the readback of the routing register. (Unfortunately the
>> datasheet was not yet publicly available :(  ). In your patch by replacing
>> the line:
>>
>> writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>>
>> by the following ones:
>>
>> reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>> reg = (reg & (~ARMADA_370_XP_INT_SOURCE_CPU_MASK)) | mask;
>> writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>>
>> with
>> #define ARMADA_370_XP_INT_SOURCE_CPU_MASK	0xF
>>
>> Then it worked.
>>
>> About masking with the online CPU in the original code, the purpose
>> was to allow sharing the SoC with an other OS by doing AMP. This feature
>> is part of the Marvell version of the kernel.
>> The idea was to bring this latter in the vanilla kernel, but I am not sure
>> that all the part needed for AMP are acceptable for mainline. So I can add
>> it back later when we will need it.
> 
> Right.
> 
> So are you ok with that patch (including your change) ? If you send me
> tested/acked-by i'll route it for 3.15

With the change I included you can add my

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


Thanks,

Gregory


> 
> Thanks,
> 
> 	tglx
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Gregory CLEMENT]

Hi Thomas,

On 18/03/2014 22:04, Gregory CLEMENT wrote:
> On 18/03/2014 21:55, Thomas Gleixner wrote:
>> On Tue, 18 Mar 2014, Gregory CLEMENT wrote:
>>> On 07/03/2014 18:17, Thomas Gleixner wrote:
>>>> It might be the readback of the routing register. I don't have the
>>>> datasheet of this.
>>>
>>> Sorry for the delay, I was on vacation without the hardware to test it.
>>>
>>> Indeed it was the readback of the routing register. (Unfortunately the
>>> datasheet was not yet publicly available :(  ). In your patch by replacing
>>> the line:
>>>
>>> writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>>>
>>> by the following ones:
>>>
>>> reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>>> reg = (reg & (~ARMADA_370_XP_INT_SOURCE_CPU_MASK)) | mask;
>>> writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
>>>
>>> with
>>> #define ARMADA_370_XP_INT_SOURCE_CPU_MASK	0xF
>>>
>>> Then it worked.
>>>
>>> About masking with the online CPU in the original code, the purpose
>>> was to allow sharing the SoC with an other OS by doing AMP. This feature
>>> is part of the Marvell version of the kernel.
>>> The idea was to bring this latter in the vanilla kernel, but I am not sure
>>> that all the part needed for AMP are acceptable for mainline. So I can add
>>> it back later when we will need it.
>>
>> Right.
>>
>> So are you ok with that patch (including your change) ? If you send me
>> tested/acked-by i'll route it for 3.15
> 
> With the change I included you can add my
> 
> Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 

I have just noticed that this patch was not merged in 3.15.
Actually it is not in the core/irq branch of tip.git with
the other patches of the same series.
Is there any reason for this?

Thanks,

Gregory


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Jason Cooper]

Thomas,

On Thu, Apr 24, 2014 at 06:04:41PM +0200, Gregory CLEMENT wrote:
> Hi Thomas,
> 
> On 18/03/2014 22:04, Gregory CLEMENT wrote:
> > On 18/03/2014 21:55, Thomas Gleixner wrote:
> >> On Tue, 18 Mar 2014, Gregory CLEMENT wrote:
> >>> On 07/03/2014 18:17, Thomas Gleixner wrote:
> >>>> It might be the readback of the routing register. I don't have the
> >>>> datasheet of this.
> >>>
> >>> Sorry for the delay, I was on vacation without the hardware to test it.
> >>>
> >>> Indeed it was the readback of the routing register. (Unfortunately the
> >>> datasheet was not yet publicly available :(  ). In your patch by replacing
> >>> the line:
> >>>
> >>> writel(mask, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> >>>
> >>> by the following ones:
> >>>
> >>> reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> >>> reg = (reg & (~ARMADA_370_XP_INT_SOURCE_CPU_MASK)) | mask;
> >>> writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
> >>>
> >>> with
> >>> #define ARMADA_370_XP_INT_SOURCE_CPU_MASK	0xF
> >>>
> >>> Then it worked.
> >>>
> >>> About masking with the online CPU in the original code, the purpose
> >>> was to allow sharing the SoC with an other OS by doing AMP. This feature
> >>> is part of the Marvell version of the kernel.
> >>> The idea was to bring this latter in the vanilla kernel, but I am not sure
> >>> that all the part needed for AMP are acceptable for mainline. So I can add
> >>> it back later when we will need it.
> >>
> >> Right.
> >>
> >> So are you ok with that patch (including your change) ? If you send me
> >> tested/acked-by i'll route it for 3.15
> > 
> > With the change I included you can add my
> > 
> > Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > 
> 
> I have just noticed that this patch was not merged in 3.15.
> Actually it is not in the core/irq branch of tip.git with
> the other patches of the same series.
> Is there any reason for this?

I know your mail filters are pretty aggressive and I just wanted to make
sure you saw this.  I'm also not sure what happened here.

thx,

Jason.

Re: [patch 5/5] irqchip: armanda: Sanitize set_irq_affinity()

[Thomas Gleixner]

On Fri, 25 Apr 2014, Jason Cooper wrote:
> On Thu, Apr 24, 2014 at 06:04:41PM +0200, Gregory CLEMENT wrote:
> > 
> > I have just noticed that this patch was not merged in 3.15.
> > Actually it is not in the core/irq branch of tip.git with
> > the other patches of the same series.
> > Is there any reason for this?
> 
> I know your mail filters are pretty aggressive and I just wanted to make
> sure you saw this.  I'm also not sure what happened here.

Dunno, must have fallen through the cracks. Lemme find it again.

[tip:irq/urgent] irqchip: armanda: Sanitize set_irq_affinity()

[tip-bot for Thomas Gleixner]

Commit-ID:  8cc3cfc5ccf1680b7c88f874912b6bec2797b76b
Gitweb:     http://git.kernel.org/tip/8cc3cfc5ccf1680b7c88f874912b6bec2797b76b
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 4 Mar 2014 20:43:41 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 28 Apr 2014 21:27:15 +0200

irqchip: armanda: Sanitize set_irq_affinity()

The set_irq_affinity() function has two issues:

1) It has no protection against selecting an offline cpu from the
   given mask.

2) It pointlessly restricts the affinity masks to have a single cpu
   set. This collides with the irq migration code of arm.

   irq affinity is set to core 3
   core 3 goes offline

   migration code sets mask to cpu_online_mask and calls the
   irq_set_affinity() callback of the irq_chip which fails due to bit
   0,1,2 set.

So instead of doing silly for_each_cpu() loops just pick any bit of
the mask which intersects with the online mask.

Get rid of fiddling with the default_irq_affinity as well.

[ Gregory: Fixed the access to the routing register ]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@elte.hu>
Link: http://lkml.kernel.org/r/20140304203101.088889302@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 drivers/irqchip/irq-armada-370-xp.c | 37 ++++++-------------------------------
 1 file changed, 6 insertions(+), 31 deletions(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 41be897..304a20d 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -41,6 +41,7 @@
 #define ARMADA_370_XP_INT_SET_ENABLE_OFFS	(0x30)
 #define ARMADA_370_XP_INT_CLEAR_ENABLE_OFFS	(0x34)
 #define ARMADA_370_XP_INT_SOURCE_CTL(irq)	(0x100 + irq*4)
+#define ARMADA_370_XP_INT_SOURCE_CPU_MASK	0xF
 
 #define ARMADA_370_XP_CPU_INTACK_OFFS		(0x44)
 #define ARMADA_375_PPI_CAUSE			(0x10)
@@ -244,35 +245,18 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 static int armada_xp_set_affinity(struct irq_data *d,
 				  const struct cpumask *mask_val, bool force)
 {
-	unsigned long reg;
-	unsigned long new_mask = 0;
-	unsigned long online_mask = 0;
-	unsigned long count = 0;
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	unsigned long reg, mask;
 	int cpu;
 
-	for_each_cpu(cpu, mask_val) {
-		new_mask |= 1 << cpu_logical_map(cpu);
-		count++;
-	}
-
-	/*
-	 * Forbid mutlicore interrupt affinity
-	 * This is required since the MPIC HW doesn't limit
-	 * several CPUs from acknowledging the same interrupt.
-	 */
-	if (count > 1)
-		return -EINVAL;
-
-	for_each_cpu(cpu, cpu_online_mask)
-		online_mask |= 1 << cpu_logical_map(cpu);
+	/* Select a single core from the affinity mask which is online */
+	cpu = cpumask_any_and(mask_val, cpu_online_mask);
+	mask = 1UL << cpu_logical_map(cpu);
 
 	raw_spin_lock(&irq_controller_lock);
-
 	reg = readl(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
-	reg = (reg & (~online_mask)) | new_mask;
+	reg = (reg & (~ARMADA_370_XP_INT_SOURCE_CPU_MASK)) | mask;
 	writel(reg, main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(hwirq));
-
 	raw_spin_unlock(&irq_controller_lock);
 
 	return 0;
@@ -494,15 +478,6 @@ static int __init armada_370_xp_mpic_of_init(struct device_node *node,
 
 #ifdef CONFIG_SMP
 	armada_xp_mpic_smp_cpu_init();
-
-	/*
-	 * Set the default affinity from all CPUs to the boot cpu.
-	 * This is required since the MPIC doesn't limit several CPUs
-	 * from acknowledging the same interrupt.
-	 */
-	cpumask_clear(irq_default_affinity);
-	cpumask_set_cpu(smp_processor_id(), irq_default_affinity);
-
 #endif
 
 	armada_370_xp_msi_init(node, main_int_res.start);