2018-01-25 03:11:30

by Palmer Dabbelt

[permalink] [raw]
Subject: Make set_handle_irq and handle_arch_irq generic

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.



2018-01-25 03:09:25

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH v2 1/4] arm: Make set_handle_irq and handle_arch_irq generic

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 <[email protected]>
---
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


2018-01-25 03:09:29

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler

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 <[email protected]>
---
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


2018-01-25 03:09:31

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH v2 3/4] openrisc: Use the new MULTI_IRQ_HANDLER

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 <[email protected]>
---
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


2018-01-25 03:10:14

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER

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 <[email protected]>
---
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


2018-01-25 07:46:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] arm64: Use the new MULTI_IRQ_HANDLER

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2018-01-25 07:46:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] arm: Make set_handle_irq and handle_arch_irq generic

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.

2018-01-25 07:47:55

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] openrisc: Use the new MULTI_IRQ_HANDLER

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.

2018-01-25 07:48:56

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler

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 <[email protected]>

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2018-01-25 08:33:45

by Stafford Horne

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler

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 <[email protected]>
> ---
> 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 <[email protected]>


> - 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
>

2018-01-25 09:08:04

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] arm: Make set_handle_irq and handle_arch_irq generic

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 <[email protected]>
> ---
> 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

2018-01-25 16:05:55

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [patches] Re: [PATCH v2 4/4] RISC-V: Move to the new generic IRQ handler

On Thu, 25 Jan 2018 00:31:53 PST (-0800), [email protected] 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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
>
>> - 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
>>

2018-01-27 03:14:15

by Stafford Horne

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] openrisc: Use the new MULTI_IRQ_HANDLER

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 <[email protected]>

-Stafford