public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Make set_handle_irq and handle_arch_irq generic
@ 2018-01-25  3:07 Palmer Dabbelt
  2018-01-25  3:07 ` [PATCH v2 1/4] arm: " Palmer Dabbelt
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Palmer Dabbelt @ 2018-01-25  3:07 UTC (permalink / raw)
  To: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann
  Cc: linux-riscv, linux-kernel, patches

This is the second version of a patch set titled "Use arm64's scheme for
registering first-level IRQ handlers on RISC-V".  That patch set's cover letter
is still the best way to describe what's going on, so I'm just copying it here:

    This patch set has been sitting around for a while, but it got a bit lost
    in the shuffle.  In RISC-V land we currently couple do_IRQ (the C entry
    point for interrupt handling) to our first-level interrupt controller.
    While this isn't completely crazy (as the first-level interrupt controller
    is specified by the ISA), it is a bit awkward.
    
    This patch set decouples our trap handler from our first-level IRQ chip
    driver by copying what a handful of other architectures are doing.  This
    does add an additional load to the interrupt handling path, but there's a
    handful of performance problems in there that I've been meaning to look at
    so I don't mind adding another one for now.  The advantage is that our
    irqchip driver is decoupled from our arch port, at least at compile time.

I've build tested this on arm, arm64, and openrisc but haven't run on any of
those systems.  The goal was to make no functional changes, but the
__ro_after_init addition does actaully change behavior.

Changes since v1:

* I based this on arm instead of arm64, which means we guard the selection of
  these routines with CONFIG_MULTI_IRQ_HANDLER.
* The changes are in kernel/irq/handle.c and include/linux/irq.h instead of
  lib.
* I've converted the arm, arm64, and openrisc ports to use the generic versions
  of these routines.

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

* [PATCH v2 1/4] arm: Make set_handle_irq and handle_arch_irq generic
  2018-01-25  3:07 Make set_handle_irq and handle_arch_irq generic Palmer Dabbelt
@ 2018-01-25  3:07 ` Palmer Dabbelt
  2018-01-25  7:45   ` Christoph Hellwig
  2018-01-25  9:05   ` Thomas Gleixner
  2018-01-25  3:07 ` [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER Palmer Dabbelt
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Palmer Dabbelt @ 2018-01-25  3:07 UTC (permalink / raw)
  To: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann
  Cc: linux-riscv, linux-kernel, patches, Palmer Dabbelt

It looks like this same irqchip registration mechanism has been copied
into a handful of ports, including aarch64 and openrisc.  I want to use
this in the RISC-V port, so I thought it would be good to make this
generic instead.

This patch simply moves set_handle_irq and handle_arch_irq from arch/arm
to kernel/irq/handle.c.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 arch/arm/Kconfig             |  5 -----
 arch/arm/include/asm/irq.h   |  5 -----
 arch/arm/kernel/entry-armv.S |  6 ------
 arch/arm/kernel/irq.c        | 10 ----------
 include/linux/irq.h          | 18 ++++++++++++++++++
 kernel/irq/Kconfig           |  5 +++++
 kernel/irq/handle.c          | 10 ++++++++++
 7 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 51c8df561077..e51f907668f6 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -917,11 +917,6 @@ config IWMMXT
 	  Enable support for iWMMXt context switching at run time if
 	  running on a CPU that supports it.
 
-config MULTI_IRQ_HANDLER
-	bool
-	help
-	  Allow each machine to specify it's own IRQ handler at run time.
-
 if !MMU
 source "arch/arm/Kconfig-nommu"
 endif
diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index b6f319606e30..c883fcbe93b6 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -31,11 +31,6 @@ extern void asm_do_IRQ(unsigned int, struct pt_regs *);
 void handle_IRQ(unsigned int, struct pt_regs *);
 void init_IRQ(void);
 
-#ifdef CONFIG_MULTI_IRQ_HANDLER
-extern void (*handle_arch_irq)(struct pt_regs *);
-extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
-#endif
-
 #ifdef CONFIG_SMP
 extern void arch_trigger_cpumask_backtrace(const cpumask_t *mask,
 					   bool exclude_self);
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index fbc707626b3e..2d31cec2f4cb 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1230,9 +1230,3 @@ vector_addrexcptn:
 	.globl	cr_alignment
 cr_alignment:
 	.space	4
-
-#ifdef CONFIG_MULTI_IRQ_HANDLER
-	.globl	handle_arch_irq
-handle_arch_irq:
-	.space	4
-#endif
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index ece04a457486..9908dacf9229 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -102,16 +102,6 @@ void __init init_IRQ(void)
 	uniphier_cache_init();
 }
 
-#ifdef CONFIG_MULTI_IRQ_HANDLER
-void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
-{
-	if (handle_arch_irq)
-		return;
-
-	handle_arch_irq = handle_irq;
-}
-#endif
-
 #ifdef CONFIG_SPARSE_IRQ
 int __init arch_probe_nr_irqs(void)
 {
diff --git a/include/linux/irq.h b/include/linux/irq.h
index a0231e96a578..4e7ca0216fb9 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1170,4 +1170,22 @@ int __ipi_send_mask(struct irq_desc *desc, const struct cpumask *dest);
 int ipi_send_single(unsigned int virq, unsigned int cpu);
 int ipi_send_mask(unsigned int virq, const struct cpumask *dest);
 
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+/*
+ * Registers a generic IRQ handling function as the top-level IRQ handler in
+ * the system, which is generally the first C code called from an assembly
+ * architecture-specific interrupt handler.
+ *
+ * Returns 0 on success, or -EBUSY if an IRQ handler has already been
+ * registered.
+ */
+void __init set_handle_irq(void (*handle_irq)(struct pt_regs *));
+
+/*
+ * Allows interrupt handlers to find the irqchip that's been registered as the
+ * top-level IRQ handler.
+ */
+extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+#endif
+
 #endif /* _LINUX_IRQ_H */
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 89e355866450..1b84dccd1a03 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -142,3 +142,8 @@ config GENERIC_IRQ_DEBUGFS
 	  If you don't know what to do here, say N.
 
 endmenu
+
+config MULTI_IRQ_HANDLER
+	bool
+	help
+	  Allow each machine to specify it's own IRQ handler at run time.
diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 79f987b942b8..eb55650a2abc 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -20,6 +20,8 @@
 
 #include "internals.h"
 
+void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+
 /**
  * handle_bad_irq - handle spurious and unhandled irqs
  * @desc:      description of the interrupt
@@ -207,3 +209,11 @@ irqreturn_t handle_irq_event(struct irq_desc *desc)
 	irqd_clear(&desc->irq_data, IRQD_IRQ_INPROGRESS);
 	return ret;
 }
+
+void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
+{
+	if (handle_arch_irq)
+		return;
+
+	handle_arch_irq = handle_irq;
+}
-- 
2.13.6

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

* [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER
  2018-01-25  3:07 Make set_handle_irq and handle_arch_irq generic Palmer Dabbelt
  2018-01-25  3:07 ` [PATCH v2 1/4] arm: " Palmer Dabbelt
@ 2018-01-25  3:07 ` Palmer Dabbelt
  2018-01-25  7:45   ` Christoph Hellwig
  2018-01-25  3:07 ` [PATCH v2 3/4] openrisc: " Palmer Dabbelt
  2018-01-25  3:07 ` [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler Palmer Dabbelt
  3 siblings, 1 reply; 13+ messages in thread
From: Palmer Dabbelt @ 2018-01-25  3:07 UTC (permalink / raw)
  To: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann
  Cc: linux-riscv, linux-kernel, patches, Palmer Dabbelt

It appears arm64 copied arm's MULTI_IRQ_HANDLER code, but made it
unconditional.  I wanted to make this generic so it could be used by the
RISC-V port.  This patch converts the arm64 code to use the new generic
code, which simply consists of deleting the arm64 code and setting
MULTI_IRQ_HANDLER instead.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 arch/arm64/Kconfig           |  1 +
 arch/arm64/include/asm/irq.h |  2 --
 arch/arm64/kernel/irq.c      | 10 ----------
 3 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c9a7e9e1414f..effb04a80520 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -129,6 +129,7 @@ config ARM64
 	select IRQ_DOMAIN
 	select IRQ_FORCED_THREADING
 	select MODULES_USE_ELF_RELA
+	select MULTI_IRQ_HANDLER
 	select NO_BOOTMEM
 	select OF
 	select OF_EARLY_FLATTREE
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index a0fee6985e6a..b2b0c6405eb0 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -8,8 +8,6 @@
 
 struct pt_regs;
 
-extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
-
 static inline int nr_legacy_irqs(void)
 {
 	return 0;
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 713561e5bcab..67b781270402 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -41,16 +41,6 @@ int arch_show_interrupts(struct seq_file *p, int prec)
 	return 0;
 }
 
-void (*handle_arch_irq)(struct pt_regs *) = NULL;
-
-void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
-{
-	if (handle_arch_irq)
-		return;
-
-	handle_arch_irq = handle_irq;
-}
-
 #ifdef CONFIG_VMAP_STACK
 static void init_irq_stacks(void)
 {
-- 
2.13.6

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

* [PATCH v2 3/4] openrisc: Use the new MULTI_IRQ_HANDLER
  2018-01-25  3:07 Make set_handle_irq and handle_arch_irq generic Palmer Dabbelt
  2018-01-25  3:07 ` [PATCH v2 1/4] arm: " Palmer Dabbelt
  2018-01-25  3:07 ` [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER Palmer Dabbelt
@ 2018-01-25  3:07 ` Palmer Dabbelt
  2018-01-25  7:46   ` Christoph Hellwig
  2018-01-25  3:07 ` [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler Palmer Dabbelt
  3 siblings, 1 reply; 13+ messages in thread
From: Palmer Dabbelt @ 2018-01-25  3:07 UTC (permalink / raw)
  To: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann
  Cc: linux-riscv, linux-kernel, patches, Palmer Dabbelt

It appears that openrisc copied arm64's MULTI_IRQ_HANDLER code (which
came from arm).  I wanted to make this generic so I could use it in the
RISC-V port.  This patch converts the openrisc code to use the generic
version.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 arch/openrisc/Kconfig      | 1 +
 arch/openrisc/kernel/irq.c | 7 -------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 339df7324e9c..77a9d45ac11e 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -35,6 +35,7 @@ config OPENRISC
 	select ARCH_USE_QUEUED_RWLOCKS
 	select OMPIC if SMP
 	select ARCH_WANT_FRAME_POINTERS
+	select MULTI_IRQ_HANDLER
 
 config CPU_BIG_ENDIAN
 	def_bool y
diff --git a/arch/openrisc/kernel/irq.c b/arch/openrisc/kernel/irq.c
index 35e478a93116..5f9445effaf8 100644
--- a/arch/openrisc/kernel/irq.c
+++ b/arch/openrisc/kernel/irq.c
@@ -41,13 +41,6 @@ void __init init_IRQ(void)
 	irqchip_init();
 }
 
-static void (*handle_arch_irq)(struct pt_regs *);
-
-void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
-{
-	handle_arch_irq = handle_irq;
-}
-
 void __irq_entry do_IRQ(struct pt_regs *regs)
 {
 	handle_arch_irq(regs);
-- 
2.13.6

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

* [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler
  2018-01-25  3:07 Make set_handle_irq and handle_arch_irq generic Palmer Dabbelt
                   ` (2 preceding siblings ...)
  2018-01-25  3:07 ` [PATCH v2 3/4] openrisc: " Palmer Dabbelt
@ 2018-01-25  3:07 ` Palmer Dabbelt
  2018-01-25  7:46   ` Christoph Hellwig
  2018-01-25  8:31   ` Stafford Horne
  3 siblings, 2 replies; 13+ messages in thread
From: Palmer Dabbelt @ 2018-01-25  3:07 UTC (permalink / raw)
  To: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann
  Cc: linux-riscv, linux-kernel, patches, Palmer Dabbelt

The old mechanism for handling IRQs on RISC-V was pretty ugly: the arch
code looked at the Kconfig entry for our first-level irqchip driver and
called into it directly.

This patch uses the new 0generic IRQ handling infastructure, which
essentially just deletes a bunch of code.  This does add an additional
load to the interrupt latency, but there's a lot of tuning left to be
done there on RISC-V so I think it's OK for now.

Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
---
 arch/riscv/Kconfig            |  1 +
 arch/riscv/include/asm/Kbuild |  1 +
 arch/riscv/kernel/entry.S     |  5 +++--
 arch/riscv/kernel/irq.c       | 13 -------------
 4 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2c6adf12713a..e67f42178059 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -35,6 +35,7 @@ config RISCV
 	select THREAD_INFO_IN_TASK
 	select RISCV_IRQ_INTC
 	select RISCV_TIMER
+	select MULTI_IRQ_HANDLER
 
 config MMU
 	def_bool y
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index 970460a0b492..e0d0fbe43ca2 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -16,6 +16,7 @@ generic-y += ftrace.h
 generic-y += futex.h
 generic-y += hardirq.h
 generic-y += hash.h
+generic-y += handle_irq.h
 generic-y += hw_irq.h
 generic-y += ioctl.h
 generic-y += ioctls.h
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 7404ec222406..a79869151aea 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -166,8 +166,9 @@ ENTRY(handle_exception)
 	/* Handle interrupts */
 	slli a0, s4, 1
 	srli a0, a0, 1
-	move a1, sp /* pt_regs */
-	tail do_IRQ
+	move a0, sp /* pt_regs */
+	REG_L a1, handle_arch_irq
+	jr a1
 1:
 	/* Handle syscalls */
 	li t0, EXC_SYSCALL
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 328718e8026e..b74cbfbce2d0 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -24,16 +24,3 @@ void __init init_IRQ(void)
 {
 	irqchip_init();
 }
-
-asmlinkage void __irq_entry do_IRQ(unsigned int cause, struct pt_regs *regs)
-{
-#ifdef CONFIG_RISCV_INTC
-	/*
-	 * FIXME: We don't want a direct call to riscv_intc_irq here.  The plan
-	 * is to put an IRQ domain here and let the interrupt controller
-	 * register with that, but I poked around the arm64 code a bit and
-	 * there might be a better way to do it (ie, something fully generic).
-	 */
-	riscv_intc_irq(cause, regs);
-#endif
-}
-- 
2.13.6

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

* Re: [PATCH v2 1/4] arm: Make set_handle_irq and handle_arch_irq generic
  2018-01-25  3:07 ` [PATCH v2 1/4] arm: " Palmer Dabbelt
@ 2018-01-25  7:45   ` Christoph Hellwig
  2018-01-25  9:05   ` Thomas Gleixner
  1 sibling, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2018-01-25  7:45 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann, linux-riscv,
	linux-kernel, patches

On Wed, Jan 24, 2018 at 07:07:53PM -0800, Palmer Dabbelt wrote:
> It looks like this same irqchip registration mechanism has been copied
> into a handful of ports, including aarch64 and openrisc.  I want to use
> this in the RISC-V port, so I thought it would be good to make this
> generic instead.
> 
> This patch simply moves set_handle_irq and handle_arch_irq from arch/arm
> to kernel/irq/handle.c.

The two important changes here are that:

 a) the handle_arch_irq defintion is moved from assembly to C code
 b) it is now marked __ro_after_init

Those should be prominently mentioned in the changelog, and for a
we probably need an explicit ACK from the ARM folks.

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

* Re: [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER
  2018-01-25  3:07 ` [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER Palmer Dabbelt
@ 2018-01-25  7:45   ` Christoph Hellwig
  0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2018-01-25  7:45 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann, linux-riscv,
	linux-kernel, patches

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v2 3/4] openrisc: Use the new MULTI_IRQ_HANDLER
  2018-01-25  3:07 ` [PATCH v2 3/4] openrisc: " Palmer Dabbelt
@ 2018-01-25  7:46   ` Christoph Hellwig
  2018-01-27  3:13     ` Stafford Horne
  0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2018-01-25  7:46 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann, linux-riscv,
	linux-kernel, patches

On Wed, Jan 24, 2018 at 07:07:55PM -0800, Palmer Dabbelt wrote:
> It appears that openrisc copied arm64's MULTI_IRQ_HANDLER code (which
> came from arm).  I wanted to make this generic so I could use it in the
> RISC-V port.  This patch converts the openrisc code to use the generic
> version.

Note that openriscv overrides previous handle_arch_irq assignments.
We'll need to know from the openrisc folks if that was intentional.

Otherwise this looks fine to me.

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

* Re: [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler
  2018-01-25  3:07 ` [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler Palmer Dabbelt
@ 2018-01-25  7:46   ` Christoph Hellwig
  2018-01-25  8:31   ` Stafford Horne
  1 sibling, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2018-01-25  7:46 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, tglx, Christoph Hellwig, Arnd Bergmann, linux-riscv,
	linux-kernel, patches

On Wed, Jan 24, 2018 at 07:07:56PM -0800, Palmer Dabbelt wrote:
> The old mechanism for handling IRQs on RISC-V was pretty ugly: the arch
> code looked at the Kconfig entry for our first-level irqchip driver and
> called into it directly.
> 
> This patch uses the new 0generic IRQ handling infastructure, which
> essentially just deletes a bunch of code.  This does add an additional
> load to the interrupt latency, but there's a lot of tuning left to be
> done there on RISC-V so I think it's OK for now.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler
  2018-01-25  3:07 ` [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler Palmer Dabbelt
  2018-01-25  7:46   ` Christoph Hellwig
@ 2018-01-25  8:31   ` Stafford Horne
  2018-01-25 16:04     ` [patches] " Palmer Dabbelt
  1 sibling, 1 reply; 13+ messages in thread
From: Stafford Horne @ 2018-01-25  8:31 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	tglx, Christoph Hellwig, Arnd Bergmann, linux-riscv, linux-kernel,
	patches

On Wed, Jan 24, 2018 at 07:07:56PM -0800, Palmer Dabbelt wrote:
> The old mechanism for handling IRQs on RISC-V was pretty ugly: the arch
> code looked at the Kconfig entry for our first-level irqchip driver and
> called into it directly.
> 
> This patch uses the new 0generic IRQ handling infastructure, which
> essentially just deletes a bunch of code.  This does add an additional
> load to the interrupt latency, but there's a lot of tuning left to be
> done there on RISC-V so I think it's OK for now.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> ---
>  arch/riscv/Kconfig            |  1 +
>  arch/riscv/include/asm/Kbuild |  1 +
>  arch/riscv/kernel/entry.S     |  5 +++--
>  arch/riscv/kernel/irq.c       | 13 -------------
>  4 files changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 2c6adf12713a..e67f42178059 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -35,6 +35,7 @@ config RISCV
>  	select THREAD_INFO_IN_TASK
>  	select RISCV_IRQ_INTC
>  	select RISCV_TIMER
> +	select MULTI_IRQ_HANDLER
>  
>  config MMU
>  	def_bool y
> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
> index 970460a0b492..e0d0fbe43ca2 100644
> --- a/arch/riscv/include/asm/Kbuild
> +++ b/arch/riscv/include/asm/Kbuild
> @@ -16,6 +16,7 @@ generic-y += ftrace.h
>  generic-y += futex.h
>  generic-y += hardirq.h
>  generic-y += hash.h
> +generic-y += handle_irq.h
>  generic-y += hw_irq.h
>  generic-y += ioctl.h
>  generic-y += ioctls.h
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 7404ec222406..a79869151aea 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -166,8 +166,9 @@ ENTRY(handle_exception)
>  	/* Handle interrupts */
>  	slli a0, s4, 1
>  	srli a0, a0, 1

Hi Palmer,

Do we need these shifts into a0?  I guess these were used when this was an arg
to  do_IRQ, but no longer needed since you put pt_regs into a0 in the next
instruction.

Other than that it looks good, and thanks for looking at OpenRISC too.

Acked-by: Stafford Horne <shorne@gmail.com>


> -	move a1, sp /* pt_regs */
> -	tail do_IRQ
> +	move a0, sp /* pt_regs */
> +	REG_L a1, handle_arch_irq
> +	jr a1
>  1:
>  	/* Handle syscalls */
>  	li t0, EXC_SYSCALL
> diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
> index 328718e8026e..b74cbfbce2d0 100644
> --- a/arch/riscv/kernel/irq.c
> +++ b/arch/riscv/kernel/irq.c
> @@ -24,16 +24,3 @@ void __init init_IRQ(void)
>  {
>  	irqchip_init();
>  }
> -
> -asmlinkage void __irq_entry do_IRQ(unsigned int cause, struct pt_regs *regs)
> -{
> -#ifdef CONFIG_RISCV_INTC
> -	/*
> -	 * FIXME: We don't want a direct call to riscv_intc_irq here.  The plan
> -	 * is to put an IRQ domain here and let the interrupt controller
> -	 * register with that, but I poked around the arm64 code a bit and
> -	 * there might be a better way to do it (ie, something fully generic).
> -	 */
> -	riscv_intc_irq(cause, regs);
> -#endif
> -}
> -- 
> 2.13.6
> 

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

* Re: [PATCH v2 1/4] arm: Make set_handle_irq and handle_arch_irq generic
  2018-01-25  3:07 ` [PATCH v2 1/4] arm: " Palmer Dabbelt
  2018-01-25  7:45   ` Christoph Hellwig
@ 2018-01-25  9:05   ` Thomas Gleixner
  1 sibling, 0 replies; 13+ messages in thread
From: Thomas Gleixner @ 2018-01-25  9:05 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	shorne, Christoph Hellwig, Arnd Bergmann, linux-riscv,
	linux-kernel, patches

On Wed, 24 Jan 2018, Palmer Dabbelt wrote:

> It looks like this same irqchip registration mechanism has been copied
> into a handful of ports, including aarch64 and openrisc.  I want to use
> this in the RISC-V port, so I thought it would be good to make this
> generic instead.
> 
> This patch simply moves set_handle_irq and handle_arch_irq from arch/arm
> to kernel/irq/handle.c.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> ---
>  arch/arm/Kconfig             |  5 -----
>  arch/arm/include/asm/irq.h   |  5 -----
>  arch/arm/kernel/entry-armv.S |  6 ------
>  arch/arm/kernel/irq.c        | 10 ----------
>  include/linux/irq.h          | 18 ++++++++++++++++++
>  kernel/irq/Kconfig           |  5 +++++
>  kernel/irq/handle.c          | 10 ++++++++++

Please split the patches into two pieces:

1) Add the infrastructure to the generic code and have a new Kconfig symbol

   GENERIC_IRQ_MULTI_HANDLER

   or something which fits in the GENERIC_IRQ_ name space.

   That also makes sure that the build does not fail after patch 1 for the
   non converted arch. With your change this breaks arm64 build with only
   patch 1 applied because the Kconfig symbol is defined twice and the code
   is defined twice. See drivers/irqchip/Kconfig ARM_GIC_V3 ....
   
2) Convert both arm and arm64 over along with the select statements in the
   arch and irqchip Kconfigs

Thanks,

	tglx

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

* Re: [patches] Re: [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler
  2018-01-25  8:31   ` Stafford Horne
@ 2018-01-25 16:04     ` Palmer Dabbelt
  0 siblings, 0 replies; 13+ messages in thread
From: Palmer Dabbelt @ 2018-01-25 16:04 UTC (permalink / raw)
  To: shorne
  Cc: linux, catalin.marinas, Will Deacon, jonas, stefan.kristiansson,
	tglx, Christoph Hellwig, Arnd Bergmann, linux-riscv, linux-kernel,
	patches

On Thu, 25 Jan 2018 00:31:53 PST (-0800), shorne@gmail.com wrote:
> On Wed, Jan 24, 2018 at 07:07:56PM -0800, Palmer Dabbelt wrote:
>> The old mechanism for handling IRQs on RISC-V was pretty ugly: the arch
>> code looked at the Kconfig entry for our first-level irqchip driver and
>> called into it directly.
>>
>> This patch uses the new 0generic IRQ handling infastructure, which
>> essentially just deletes a bunch of code.  This does add an additional
>> load to the interrupt latency, but there's a lot of tuning left to be
>> done there on RISC-V so I think it's OK for now.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
>> ---
>>  arch/riscv/Kconfig            |  1 +
>>  arch/riscv/include/asm/Kbuild |  1 +
>>  arch/riscv/kernel/entry.S     |  5 +++--
>>  arch/riscv/kernel/irq.c       | 13 -------------
>>  4 files changed, 5 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 2c6adf12713a..e67f42178059 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -35,6 +35,7 @@ config RISCV
>>  	select THREAD_INFO_IN_TASK
>>  	select RISCV_IRQ_INTC
>>  	select RISCV_TIMER
>> +	select MULTI_IRQ_HANDLER
>>
>>  config MMU
>>  	def_bool y
>> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
>> index 970460a0b492..e0d0fbe43ca2 100644
>> --- a/arch/riscv/include/asm/Kbuild
>> +++ b/arch/riscv/include/asm/Kbuild
>> @@ -16,6 +16,7 @@ generic-y += ftrace.h
>>  generic-y += futex.h
>>  generic-y += hardirq.h
>>  generic-y += hash.h
>> +generic-y += handle_irq.h
>>  generic-y += hw_irq.h
>>  generic-y += ioctl.h
>>  generic-y += ioctls.h
>> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
>> index 7404ec222406..a79869151aea 100644
>> --- a/arch/riscv/kernel/entry.S
>> +++ b/arch/riscv/kernel/entry.S
>> @@ -166,8 +166,9 @@ ENTRY(handle_exception)
>>  	/* Handle interrupts */
>>  	slli a0, s4, 1
>>  	srli a0, a0, 1
>
> Hi Palmer,
>
> Do we need these shifts into a0?  I guess these were used when this was an arg
> to  do_IRQ, but no longer needed since you put pt_regs into a0 in the next
> instruction.

Thanks!

> Other than that it looks good, and thanks for looking at OpenRISC too.
>
> Acked-by: Stafford Horne <shorne@gmail.com>
>
>
>> -	move a1, sp /* pt_regs */
>> -	tail do_IRQ
>> +	move a0, sp /* pt_regs */
>> +	REG_L a1, handle_arch_irq
>> +	jr a1
>>  1:
>>  	/* Handle syscalls */
>>  	li t0, EXC_SYSCALL
>> diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
>> index 328718e8026e..b74cbfbce2d0 100644
>> --- a/arch/riscv/kernel/irq.c
>> +++ b/arch/riscv/kernel/irq.c
>> @@ -24,16 +24,3 @@ void __init init_IRQ(void)
>>  {
>>  	irqchip_init();
>>  }
>> -
>> -asmlinkage void __irq_entry do_IRQ(unsigned int cause, struct pt_regs *regs)
>> -{
>> -#ifdef CONFIG_RISCV_INTC
>> -	/*
>> -	 * FIXME: We don't want a direct call to riscv_intc_irq here.  The plan
>> -	 * is to put an IRQ domain here and let the interrupt controller
>> -	 * register with that, but I poked around the arm64 code a bit and
>> -	 * there might be a better way to do it (ie, something fully generic).
>> -	 */
>> -	riscv_intc_irq(cause, regs);
>> -#endif
>> -}
>> --
>> 2.13.6
>>

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

* Re: [PATCH v2 3/4] openrisc: Use the new MULTI_IRQ_HANDLER
  2018-01-25  7:46   ` Christoph Hellwig
@ 2018-01-27  3:13     ` Stafford Horne
  0 siblings, 0 replies; 13+ messages in thread
From: Stafford Horne @ 2018-01-27  3:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Palmer Dabbelt, linux, catalin.marinas, Will Deacon, jonas,
	stefan.kristiansson, tglx, Arnd Bergmann, linux-riscv,
	linux-kernel, patches

On Wed, Jan 24, 2018 at 11:46:19PM -0800, Christoph Hellwig wrote:
> On Wed, Jan 24, 2018 at 07:07:55PM -0800, Palmer Dabbelt wrote:
> > It appears that openrisc copied arm64's MULTI_IRQ_HANDLER code (which
> > came from arm).  I wanted to make this generic so I could use it in the
> > RISC-V port.  This patch converts the openrisc code to use the generic
> > version.
> 
> Note that openriscv overrides previous handle_arch_irq assignments.
> We'll need to know from the openrisc folks if that was intentional.
> 
> Otherwise this looks fine to me.

This looks fine to me too.  We only assign the handle_arch_irq one time.  I
think not protecting against overrides was an oversight in the original
implementation.

Acked-by: Stafford Horne <shorne@gmail.com>

-Stafford

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

end of thread, other threads:[~2018-01-27  3:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-25  3:07 Make set_handle_irq and handle_arch_irq generic Palmer Dabbelt
2018-01-25  3:07 ` [PATCH v2 1/4] arm: " Palmer Dabbelt
2018-01-25  7:45   ` Christoph Hellwig
2018-01-25  9:05   ` Thomas Gleixner
2018-01-25  3:07 ` [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER Palmer Dabbelt
2018-01-25  7:45   ` Christoph Hellwig
2018-01-25  3:07 ` [PATCH v2 3/4] openrisc: " Palmer Dabbelt
2018-01-25  7:46   ` Christoph Hellwig
2018-01-27  3:13     ` Stafford Horne
2018-01-25  3:07 ` [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler Palmer Dabbelt
2018-01-25  7:46   ` Christoph Hellwig
2018-01-25  8:31   ` Stafford Horne
2018-01-25 16:04     ` [patches] " Palmer Dabbelt

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