Skip to contentSkip to conversation

IPI numbering cleanup

PATCH New Marc Zyngier 3 participants 8 messages Dec 18, 2015 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

cover IPI numbering cleanup

Daniel Thompson reported[1] that IPI_CPU_BACKTRACE failed to fire on one of his platforms because SGI15 was already used by the secure firmware.

In order to move this IPI to the non-secure side, this series drops IPI_CALL_FUNC_SINGLE (which has been deprecated for a while). This allows the last non-secure SGI to be reallocated to IPI_CPU_BACKTRACE.

       M.

[1]: https://lkml.org/lkml/2015/12/16/375

Marc Zyngier (2):
ARM: Remove IPI_CALL_FUNC_SINGLE
ARM: Make IPI_CPU_BACKTRACE a "non-secure" SGI

 arch/arm/include/asm/hardirq.h |  2 +-
 arch/arm/kernel/smp.c          | 12 ++----------
 2 files changed, 3 insertions(+), 11 deletions(-)
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/2 ARM: Remove IPI_CALL_FUNC_SINGLE

Since 9a46ad6d6df3 ("smp: make smp_call_function_many() use logic similar to smp_call_function_single()"), the core IPI handling has been simplified, and generic_smp_call_function_interrupt is now the same as generic_smp_call_function_single_interrupt.

This means that one of IPI_CALL_FUNC and IPI_CALL_FUNC_SINGLE has become redundant. We can then safely drop IPI_CALL_FUNC_SINGLE, and use only IPI_CALL_FUNC.

This has the advantage of reducing the number of SGI IDs we're using (a fairly scarse resource).

Tested on a dual A7 board.

---
 arch/arm/include/asm/hardirq.h |  2 +-
 arch/arm/kernel/smp.c          | 10 +---------
 2 files changed, 2 insertions(+), 10 deletions(-)
arch/arm/include/asm/hardirq.h +1 -1
index fe3ea77..3d7351c 100644
@@ -5,7 +5,7 @@
 #include <linux/threads.h>
 #include <asm/irq.h>
 
-#define NR_IPI	8
+#define NR_IPI	7
 
 typedef struct {
 	unsigned int __softirq_pending;
arch/arm/kernel/smp.c +1 -9
index b263613..d50a77d 100644
@@ -69,7 +69,6 @@ enum ipi_msg_type {
 	IPI_TIMER,
 	IPI_RESCHEDULE,
 	IPI_CALL_FUNC,
-	IPI_CALL_FUNC_SINGLE,
 	IPI_CPU_STOP,
 	IPI_IRQ_WORK,
 	IPI_COMPLETION,
@@ -475,7 +474,6 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
 	S(IPI_TIMER, "Timer broadcast interrupts"),
 	S(IPI_RESCHEDULE, "Rescheduling interrupts"),
 	S(IPI_CALL_FUNC, "Function call interrupts"),
-	S(IPI_CALL_FUNC_SINGLE, "Single function call interrupts"),
 	S(IPI_CPU_STOP, "CPU stop interrupts"),
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
 	S(IPI_COMPLETION, "completion interrupts"),
@@ -525,7 +523,7 @@ void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
 
 void arch_send_call_function_single_ipi(int cpu)
 {
-	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC_SINGLE);
+	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
 }
 
 #ifdef CONFIG_IRQ_WORK
@@ -620,12 +618,6 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		irq_exit();
 		break;
 
-	case IPI_CALL_FUNC_SINGLE:
-		irq_enter();
-		generic_smp_call_function_single_interrupt();
-		irq_exit();
-		break;
-
 	case IPI_CPU_STOP:
 		irq_enter();
 		ipi_cpu_stop(cpu);
Signature · 1 line
2.1.4
#2 lore raw

PATCH 2/2 ARM: Make IPI_CPU_BACKTRACE a "non-secure" SGI

In reply to Marc Zyngier

Having IPI_CPU_BACKTRACE as SGI15 may not work if the kernel is running in non-secure mode and that the secure firmware has decided to follow ARM's recommendations that SGI8-15 should be reserved for secure purpose.

Now that we are "only" using SGI0-6, change IPI_CPU_BACKTRACE to use SGI7, which makes it more likely to work.

---
 arch/arm/kernel/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/kernel/smp.c +1 -1
index d50a77d..fe517f1 100644
@@ -72,7 +72,7 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_IRQ_WORK,
 	IPI_COMPLETION,
-	IPI_CPU_BACKTRACE = 15,
+	IPI_CPU_BACKTRACE,
 };
 
 static DECLARE_COMPLETION(cpu_running);
Signature · 1 line
2.1.4
#3 lore raw

PATCH 2/2 ARM: Make IPI_CPU_BACKTRACE a "non-secure" SGI

Quoted 7 lines · Marc Zyngier

Having IPI_CPU_BACKTRACE as SGI15 may not work if the kernel is running in non-secure mode and that the secure firmware has decided to follow ARM's recommendations that SGI8-15 should be reserved for secure purpose.

Now that we are "only" using SGI0-6, change IPI_CPU_BACKTRACE to use SGI7, which makes it more likely to work.

We should add a comment to the enum explaining this, so it's obvious for the future - best place is after the IPI_CPU_BACKTRACE entry. Not everyone checks the git logs for these kinds of details.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#4 lore raw

PATCH 2/2 ARM: Make IPI_CPU_BACKTRACE a "non-secure" SGI

Quoted 6 lines · Russell King - ARM Linux
Quoted 5 lines · Marc Zyngier

Having IPI_CPU_BACKTRACE as SGI15 may not work if the kernel is running in non-secure mode and that the secure firmware has decided to follow ARM's recommendations that SGI8-15 should be reserved for secure purpose.

Quoted 6 lines
Quoted 2 lines

Now that we are "only" using SGI0-6, change IPI_CPU_BACKTRACE to use SGI7, which makes it more likely to work.

We should add a comment to the enum explaining this, so it's obvious for the future - best place is after the IPI_CPU_BACKTRACE entry. Not everyone checks the git logs for these kinds of details.

Indeed. How about something like this:

arch/arm/kernel/smp.c +5 -0
index fe517f1..37312f6 100644
@@ -73,6 +73,11 @@ enum ipi_msg_type {
 	IPI_IRQ_WORK,
 	IPI_COMPLETION,
 	IPI_CPU_BACKTRACE,
+	/*
+	 * SGI8-15 can be reserved by secure firmware, and thus may
+	 * not be usable by the kernel. Please keep the above limited
+	 * to at most 8 entries.
+	 */
 };
 
 static DECLARE_COMPLETION(cpu_running);

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#5 lore raw

PATCH 2/2 ARM: Make IPI_CPU_BACKTRACE a "non-secure" SGI

Quoted 7 lines · Marc Zyngier
Quoted 6 lines · Russell King - ARM Linux
Quoted 5 lines · Marc Zyngier

Having IPI_CPU_BACKTRACE as SGI15 may not work if the kernel is running in non-secure mode and that the secure firmware has decided to follow ARM's recommendations that SGI8-15 should be reserved for secure purpose.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Now that we are "only" using SGI0-6, change IPI_CPU_BACKTRACE to use SGI7, which makes it more likely to work.

We should add a comment to the enum explaining this, so it's obvious for the future - best place is after the IPI_CPU_BACKTRACE entry. Not everyone checks the git logs for these kinds of details.

Indeed. How about something like this:

Yep.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#6 lore raw

PATCH 2/2 ARM: Make IPI_CPU_BACKTRACE a "non-secure" SGI

In reply to Marc Zyngier
Quoted 9 lines · Marc Zyngier

Having IPI_CPU_BACKTRACE as SGI15 may not work if the kernel is running in non-secure mode and that the secure firmware has decided to follow ARM's recommendations that SGI8-15 should be reserved for secure purpose.

Now that we are "only" using SGI0-6, change IPI_CPU_BACKTRACE to use SGI7, which makes it more likely to work.

Quoted 18 lines
---
  arch/arm/kernel/smp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/kernel/smp.c +1 -1
index d50a77d..fe517f1 100644
@@ -72,7 +72,7 @@ enum ipi_msg_type {
  	IPI_CPU_STOP,
  	IPI_IRQ_WORK,
  	IPI_COMPLETION,
-	IPI_CPU_BACKTRACE = 15,
+	IPI_CPU_BACKTRACE,
  };
 
  static DECLARE_COMPLETION(cpu_running);
#7 lore raw

PATCH 2/2 ARM: Make IPI_CPU_BACKTRACE a "non-secure" SGI

Quoted 6 lines · Daniel Thompson
Quoted 5 lines · Marc Zyngier

Having IPI_CPU_BACKTRACE as SGI15 may not work if the kernel is running in non-secure mode and that the secure firmware has decided to follow ARM's recommendations that SGI8-15 should be reserved for secure purpose.

Quoted 6 lines
Quoted 4 lines

Now that we are "only" using SGI0-6, change IPI_CPU_BACKTRACE to use SGI7, which makes it more likely to work.

Thanks Daniel, much appreciated.

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...