Use arm64's scheme for registering first-level IRQ handlers on RISC-V

Use arm64's scheme for registering first-level IRQ handlers on RISC-V

[Palmer Dabbelt]

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.

[PATCH 1/2] asm-generic: Add a generic set_handle_irq

[Palmer Dabbelt]

From: Palmer Dabbelt <palmer@dabbelt.com>

I've copied this from arm64, but it appears to be the same code that's
in arm and openrisc.  I wanted to use it for RISC-V's interrupt handler
as well, so despite this only being a handful of lines of code it seems
worth having a generic version -- if it existed when we created our
interrupt controller then maybe we would have just started with the
generic version.

This patch alone is just dead code, further patches actually use this
infrastructure.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
---
 include/asm-generic/handle_irq.h | 28 ++++++++++++++++++++++++++++
 lib/Kconfig                      |  3 +++
 lib/Makefile                     |  2 ++
 lib/handle_irq.c                 | 26 ++++++++++++++++++++++++++
 4 files changed, 59 insertions(+)
 create mode 100644 include/asm-generic/handle_irq.h
 create mode 100644 lib/handle_irq.c

diff --git a/include/asm-generic/handle_irq.h b/include/asm-generic/handle_irq.h
new file mode 100644
index 000000000000..2865fa12993a
--- /dev/null
+++ b/include/asm-generic/handle_irq.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 1992 Linus Torvalds
+ * Modifications for ARM processor Copyright (C) 1995-2000 Russell King.
+ * Support for Dynamic Tick Timer Copyright (C) 2004-2005 Nokia Corporation.
+ * Dynamic Tick Timer written by Tony Lindgren <tony@atomide.com> and
+ * Tuukka Tikkanen <tuukka.tikkanen@elektrobit.com>.
+ * Copyright (C) 2012 ARM Ltd.
+ * Copyright (C) 2017 SiFive, Inc
+ */
+
+#ifndef __ASM_GENERIC_IRQ_HANDLER_H
+#define __ASM_GENERIC_IRQ_HANDLER_H
+
+#include <asm/ptrace.h>
+#include <linux/module.h>
+
+/*
+ * 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.
+ */
+int __init set_handle_irq(void (*handle_irq)(struct pt_regs *));
+
+#endif
diff --git a/lib/Kconfig b/lib/Kconfig
index c5e84fbcb30b..c74469d44fdc 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -601,3 +601,6 @@ config GENERIC_CMPDI2
 
 config GENERIC_UCMPDI2
 	bool
+
+config GENERIC_HANDLE_IRQ
+	bool
diff --git a/lib/Makefile b/lib/Makefile
index d11c48ec8ffd..57ae58fde821 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -258,3 +258,5 @@ obj-$(CONFIG_GENERIC_LSHRDI3) += lshrdi3.o
 obj-$(CONFIG_GENERIC_MULDI3) += muldi3.o
 obj-$(CONFIG_GENERIC_CMPDI2) += cmpdi2.o
 obj-$(CONFIG_GENERIC_UCMPDI2) += ucmpdi2.o
+
+obj-$(CONFIG_GENERIC_HANDLE_IRQ) += handle_irq.o
diff --git a/lib/handle_irq.c b/lib/handle_irq.c
new file mode 100644
index 000000000000..e732147537f4
--- /dev/null
+++ b/lib/handle_irq.c
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 1992 Linus Torvalds
+ * Modifications for ARM processor Copyright (C) 1995-2000 Russell King.
+ * Support for Dynamic Tick Timer Copyright (C) 2004-2005 Nokia Corporation.
+ * Dynamic Tick Timer written by Tony Lindgren <tony@atomide.com> and
+ * Tuukka Tikkanen <tuukka.tikkanen@elektrobit.com>.
+ * Copyright (C) 2012 ARM Ltd.
+ * Copyright (C) 2017 SiFive, Inc
+ */
+
+#include <asm-generic/handle_irq.h>
+
+void (*handle_arch_irq)(struct pt_regs *) = NULL;
+
+int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
+{
+	if (!handle_arch_irq)
+		handle_arch_irq = handle_irq;
+
+	if (handle_arch_irq != handle_irq)
+		return -EBUSY;
+
+	return 0;
+}
+
-- 
2.13.6

Re: [PATCH 1/2] asm-generic: Add a generic set_handle_irq

[Christoph Hellwig]

I think this should not be asm-generic and lib, but kernel/irq/handle.c
and include/linux/irq.h, under the CONFIG_MULTI_IRQ_HANDLER symbol
already used by arm.

Also for completeness of the series please convert arm, arm64 and
openrisc over to the generic version.  Last but not least I think
handle_arch_irq needs to be marked as __ro_after_init as a security
hardening measure.

Re: [patches] Re: [PATCH 1/2] asm-generic: Add a generic set_handle_irq

[Palmer Dabbelt]

On Thu, 18 Jan 2018 07:45:13 PST (-0800), Christoph Hellwig wrote:
> I think this should not be asm-generic and lib, but kernel/irq/handle.c
> and include/linux/irq.h, under the CONFIG_MULTI_IRQ_HANDLER symbol
> already used by arm.
>
> Also for completeness of the series please convert arm, arm64 and
> openrisc over to the generic version.  Last but not least I think
> handle_arch_irq needs to be marked as __ro_after_init as a security
> hardening measure.

Makes sense.  I sent out a new patch set.

[PATCH 2/2] RISC-V: Move to the new asm-generic IRQ handler

[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 asm-generic 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..49faed9e8b93 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 GENERIC_HANDLE_IRQ
 
 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

Re: Use arm64's scheme for registering first-level IRQ handlers on RISC-V

[Arnd Bergmann]

On Thu, Jan 18, 2018 at 4:40 PM, Palmer Dabbelt <palmer@sifive.com> wrote:
> 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.

Yes, this sounds like a useful cleanup. I also agree with all of Christoph's
suggestions, otherwise it looks good to me.

       Arnd