The handle_domain_{irq,nmi}() functions were oringally intended as a
convenience, but recent rework to entry code across the kernel tree has
demonstrated that they cause more pain than they're worth and prevent
architectures from being able to write robust entry code.
This series reworks the irq code to remove them, handling the necessary
entry work consistently in entry code (be it architectural or generic).
The gory details are in the thread starting at:
https://lore.kernel.org/r/[email protected]
... which can be summarized as:
* At entry time, entry code needs to poke lockdep, RCU, and irqflag
tracing in a specific order due to interdependencies, before most
other C code can run. Part of this involves calling rcu_irq_enter().
Currently, arm64 is the only architecture which uses
handle_domain_irq() and performs the nominally correct sequence
(which is aligned with the generic entry code we aim to move to in
future).
* RCU relies on rcu_irq_enter() being called precisely once per IRQ
exception, or rcu_is_cpu_rrupt_from_idle() may fail to identify
wakeups from idle, and RCU may not trigger a reschedule when
necessary, leading to RCU stalls.
* The handle_domain_irq() helper, which is called from irqchip code
between the entry code and interrupt handlers, calls rcu_irq_enter(),
consequently causing problems for RCU on arm64.
In the thread Linus decreed:
I really think that if the rule is "we can't do accounting in
handle_domain_irq(), because it's too late for arm64", then the fix
really should be to just not do that.
Move the irq_enter()/irq_exit() to the callers - quite possibly far up
the call chain to the root of it all, and just say "architecture code
needs to do this in the low-level code before calling
handle_arch_irq".
This series does so, making entry code responsible for the IRQ entry
work, and removing the handle_domain_{irq,nmi}() functions entirely so
we're not tempted to reintroduce similar problems in future.
The rework fixes a couple of latent bugs for MIPS, fixes the problem
originally diagnosed for arm64, and should make it easier for the other
architectures using handle_domain_irq() to move to nominally correct
entry sequencing (e.g. by moving to the generic entry code).
I've added a couple of checks to generic_handle_irq() and
generic_handle_domain_irq() to verify that architectures are correctly
performing the required entry work (which fingers crossed, shouldn't
fire). Other than the above, there should be no functional change as
result of these changes (except that previously erroneous usage of RCU
in irqchip code will have become correct by virtue of RCU starting to
watch earlier).
I've given the series some light build and boot testing so far.
Thanks,
Mark.
Mark Rutland (15):
irq: mips: avoid nested irq_enter()
irq: mips: stop (ab)using handle_domain_irq()
irq: mips: simplify do_domain_IRQ()
irq: simplify handle_domain_{irq,nmi}()
irq: add generic_handle_arch_irq()
irq: arc: avoid CONFIG_HANDLE_DOMAIN_IRQ
irq: nds32: avoid CONFIG_HANDLE_DOMAIN_IRQ
irq: add a (temporary) CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY
irq: arm: perform irqentry in entry code
irq: arm64: perform irqentry in entry code
irq: csky: perform irqentry in entry code
irq: openrisc: perform irqentry in entry code
irq: riscv: perform irqentry in entry code
irq: remove CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY
irq: remove handle_domain_{irq,nmi}()
Documentation/core-api/irq/irq-domain.rst | 3 --
arch/arc/Kconfig | 1 -
arch/arc/kernel/irq.c | 10 +++-
arch/arm/Kconfig | 1 -
arch/arm/kernel/entry-armv.S | 5 +-
arch/arm/kernel/irq.c | 14 +++---
arch/arm/mach-imx/avic.c | 2 +-
arch/arm/mach-imx/tzic.c | 2 +-
arch/arm/mach-omap1/irq.c | 2 +-
arch/arm/mach-s3c/irq-s3c24xx.c | 2 +-
arch/arm64/Kconfig | 1 -
arch/arm64/kernel/entry-common.c | 52 ++++++++++++--------
arch/csky/Kconfig | 1 -
arch/csky/kernel/entry.S | 2 +-
arch/csky/kernel/irq.c | 5 --
arch/mips/Kconfig | 1 -
arch/mips/cavium-octeon/octeon-irq.c | 5 +-
arch/mips/kernel/irq.c | 6 +--
arch/nds32/Kconfig | 1 -
arch/openrisc/Kconfig | 1 -
arch/openrisc/kernel/entry.S | 4 +-
arch/openrisc/kernel/irq.c | 5 --
arch/riscv/Kconfig | 1 -
arch/riscv/kernel/entry.S | 3 +-
arch/riscv/kernel/smp.c | 9 +---
drivers/irqchip/irq-apple-aic.c | 20 ++++----
drivers/irqchip/irq-armada-370-xp.c | 13 ++---
drivers/irqchip/irq-aspeed-vic.c | 2 +-
drivers/irqchip/irq-ativic32.c | 22 ++++++++-
drivers/irqchip/irq-atmel-aic.c | 2 +-
drivers/irqchip/irq-atmel-aic5.c | 2 +-
drivers/irqchip/irq-bcm2835.c | 2 +-
drivers/irqchip/irq-bcm2836.c | 2 +-
drivers/irqchip/irq-bcm6345-l1.c | 2 +-
drivers/irqchip/irq-clps711x.c | 8 +--
drivers/irqchip/irq-csky-apb-intc.c | 2 +-
drivers/irqchip/irq-csky-mpintc.c | 4 +-
drivers/irqchip/irq-davinci-aintc.c | 2 +-
drivers/irqchip/irq-davinci-cp-intc.c | 2 +-
drivers/irqchip/irq-digicolor.c | 2 +-
drivers/irqchip/irq-dw-apb-ictl.c | 2 +-
drivers/irqchip/irq-ftintc010.c | 2 +-
drivers/irqchip/irq-gic-v3.c | 4 +-
drivers/irqchip/irq-gic.c | 2 +-
drivers/irqchip/irq-hip04.c | 2 +-
drivers/irqchip/irq-ixp4xx.c | 4 +-
drivers/irqchip/irq-lpc32xx.c | 2 +-
drivers/irqchip/irq-mmp.c | 4 +-
drivers/irqchip/irq-mxs.c | 2 +-
drivers/irqchip/irq-nvic.c | 19 +++++++-
drivers/irqchip/irq-omap-intc.c | 2 +-
drivers/irqchip/irq-or1k-pic.c | 2 +-
drivers/irqchip/irq-orion.c | 4 +-
drivers/irqchip/irq-rda-intc.c | 2 +-
drivers/irqchip/irq-riscv-intc.c | 2 +-
drivers/irqchip/irq-sa11x0.c | 4 +-
drivers/irqchip/irq-sun4i.c | 2 +-
drivers/irqchip/irq-versatile-fpga.c | 2 +-
drivers/irqchip/irq-vic.c | 2 +-
drivers/irqchip/irq-vt8500.c | 2 +-
drivers/irqchip/irq-wpcm450-aic.c | 2 +-
drivers/irqchip/irq-zevio.c | 2 +-
include/linux/irqdesc.h | 9 +---
kernel/irq/Kconfig | 3 --
kernel/irq/handle.c | 18 +++++++
kernel/irq/irqdesc.c | 81 +++++++------------------------
66 files changed, 193 insertions(+), 215 deletions(-)
--
2.11.0
There's no need fpr arch/mips's do_domain_IRQ() to open-code the NULL
check performed by handle_irq_desc(), nor the resolution of the desc
performed by generic_handle_domain_irq().
Use generic_handle_domain_irq() directly, as this is functioanlly
equivalent and clearer.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
arch/mips/kernel/irq.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c
index d20e002b3246..1fee96ef8059 100644
--- a/arch/mips/kernel/irq.c
+++ b/arch/mips/kernel/irq.c
@@ -115,11 +115,7 @@ void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq)
irq_enter();
check_stack_overflow();
-
- desc = irq_resolve_mapping(domain, hwirq);
- if (likely(desc))
- handle_irq_desc(desc);
-
+ generic_handle_domain_irq(domain, hwirq);
irq_exit();
}
#endif
--
2.11.0
There's no need for handle_domain_{irq,nmi}() to open-code the NULL
check performed by handle_irq_desc(), nor the resolution of the desc
performed by generic_handle_domain_irq().
Use generic_handle_domain_irq() directly, as this is functioanlly
equivalent and clearer. At the same time, delete the stale comments,
which are no longer helpful.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
kernel/irq/irqdesc.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 4e3c29bb603c..b07d0e1552bc 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -690,17 +690,11 @@ int handle_domain_irq(struct irq_domain *domain,
unsigned int hwirq, struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);
- struct irq_desc *desc;
- int ret = 0;
+ int ret;
irq_enter();
- /* The irqdomain code provides boundary checks */
- desc = irq_resolve_mapping(domain, hwirq);
- if (likely(desc))
- handle_irq_desc(desc);
- else
- ret = -EINVAL;
+ ret = generic_handle_domain_irq(domain, hwirq);
irq_exit();
set_irq_regs(old_regs);
@@ -721,24 +715,14 @@ int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);
- struct irq_desc *desc;
- int ret = 0;
+ int ret;
/*
* NMI context needs to be setup earlier in order to deal with tracing.
*/
WARN_ON(!in_nmi());
- desc = irq_resolve_mapping(domain, hwirq);
-
- /*
- * ack_bad_irq is not NMI-safe, just report
- * an invalid interrupt.
- */
- if (likely(desc))
- handle_irq_desc(desc);
- else
- ret = -EINVAL;
+ ret = generic_handle_domain_irq(domain, hwirq);
set_irq_regs(old_regs);
return ret;
--
2.11.0
Several architectures select GENERIC_IRQ_MULTI_HANDLER and branch to
handle_arch_irq() without performing any entry accounting.
Add a generic wrapper to handle the commoon irqentry work when invoking
handle_arch_irq(). Where an architecture needs to perform some entry
accounting itself, it will need to invoke handle_arch_irq() itself.
In subsequent patches it will become the responsibilty of the entry code
to set the irq regs when entering an IRQ (rather than deferring this to
an irqchip handler), so generic_handle_arch_irq() is made to set the irq
regs now. This can be redundant in some cases, but is never harmful as
saving/restoring the old regs nests safely.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
kernel/irq/handle.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 221d80c31e94..27182003b879 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -14,6 +14,8 @@
#include <linux/interrupt.h>
#include <linux/kernel_stat.h>
+#include <asm/irq_regs.h>
+
#include <trace/events/irq.h>
#include "internals.h"
@@ -226,4 +228,20 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
handle_arch_irq = handle_irq;
return 0;
}
+
+/**
+ * generic_handle_arch_irq - root irq handler for architectures which do no
+ * entry accounting themselves
+ * @regs: Register file coming from the low-level handling code
+ */
+asmlinkage void noinstr generic_handle_arch_irq(struct pt_regs *regs)
+{
+ struct pt_regs *old_regs;
+
+ irq_enter();
+ old_regs = set_irq_regs(regs);
+ handle_arch_irq(regs);
+ set_irq_regs(old_regs);
+ irq_exit();
+}
#endif
--
2.11.0
In preparation for removing HANDLE_DOMAIN_IRQ, have arch/arc perform all
the necessary IRQ entry accounting in its entry code.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vineet Gupta <[email protected]>
---
arch/arc/Kconfig | 1 -
arch/arc/kernel/irq.c | 10 +++++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 3a5a80f302e1..b4ae6058902a 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -40,7 +40,6 @@ config ARC
select HAVE_KRETPROBES
select HAVE_MOD_ARCH_SPECIFIC
select HAVE_PERF_EVENTS
- select HANDLE_DOMAIN_IRQ
select IRQ_DOMAIN
select MODULES_USE_ELF_RELA
select OF
diff --git a/arch/arc/kernel/irq.c b/arch/arc/kernel/irq.c
index ef909dd4b40c..dd09b58ff82d 100644
--- a/arch/arc/kernel/irq.c
+++ b/arch/arc/kernel/irq.c
@@ -6,6 +6,8 @@
#include <linux/interrupt.h>
#include <linux/irqchip.h>
#include <asm/mach_desc.h>
+
+#include <asm/irq_regs.h>
#include <asm/smp.h>
/*
@@ -39,5 +41,11 @@ void __init init_IRQ(void)
*/
void arch_do_IRQ(unsigned int hwirq, struct pt_regs *regs)
{
- handle_domain_irq(NULL, hwirq, regs);
+ struct pt_regs *old_regs;
+
+ irq_enter();
+ old_regs = set_irq_regs(regs);
+ generic_handle_domain_irq(NULL, hwirq);
+ set_irq_regs(old_regs);
+ irq_exit();
}
--
2.11.0
In preparation for removing HANDLE_DOMAIN_IRQ, have arch/nds32 perform
all the necessary IRQ entry accounting in its entry code.
Currently arch/nds32 is tightly coupled with the ativic32 irqchip, and
while the entry code should logically live under arch/nds32/, moving the
entry logic there makes things more convoluted. So for now, place the
entry logic in the ativic32 irqchip, but separated into a separate
function to make the split of responsibility clear.
In future this should probably use GENERIC_IRQ_MULTI_HANDLER to cleanly
decouple this.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Greentime Hu <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Nick Hu <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vincent Chen <[email protected]>
---
arch/nds32/Kconfig | 1 -
drivers/irqchip/irq-ativic32.c | 22 ++++++++++++++++++++--
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
index aea26e739543..4d1421b18734 100644
--- a/arch/nds32/Kconfig
+++ b/arch/nds32/Kconfig
@@ -27,7 +27,6 @@ config NDS32
select GENERIC_LIB_MULDI3
select GENERIC_LIB_UCMPDI2
select GENERIC_TIME_VSYSCALL
- select HANDLE_DOMAIN_IRQ
select HAVE_ARCH_TRACEHOOK
select HAVE_DEBUG_KMEMLEAK
select HAVE_EXIT_THREAD
diff --git a/drivers/irqchip/irq-ativic32.c b/drivers/irqchip/irq-ativic32.c
index 476d6024aaf2..223dd2f97d28 100644
--- a/drivers/irqchip/irq-ativic32.c
+++ b/drivers/irqchip/irq-ativic32.c
@@ -5,11 +5,14 @@
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
+#include <linux/hardirq.h>
#include <linux/interrupt.h>
#include <linux/irqdomain.h>
#include <linux/irqchip.h>
#include <nds32_intrinsic.h>
+#include <asm/irq_regs.h>
+
unsigned long wake_mask;
static void ativic32_ack_irq(struct irq_data *data)
@@ -103,10 +106,25 @@ static irq_hw_number_t get_intr_src(void)
- NDS32_VECTOR_offINTERRUPT;
}
-asmlinkage void asm_do_IRQ(struct pt_regs *regs)
+static void ativic32_handle_irq(struct pt_regs *regs)
{
irq_hw_number_t hwirq = get_intr_src();
- handle_domain_irq(root_domain, hwirq, regs);
+ generic_handle_domain_irq(root_domain, hwirq);
+}
+
+/*
+ * TODO: convert nds32 to GENERIC_IRQ_MULTI_HANDLER so that this entry logic
+ * can live in arch code.
+ */
+asmlinkage void asm_do_IRQ(struct pt_regs *regs)
+{
+ struct pt_regs *old_regs;
+
+ irq_enter();
+ old_regs = set_irq_regs(regs);
+ ativic32_handle_irq(regs);
+ set_irq_regs(old_regs);
+ irq_exit();
}
int __init ativic32_init_irq(struct device_node *node, struct device_node *parent)
--
2.11.0
Now that all users of CONFIG_HANDLE_DOMAIN_IRQ perform the irq entry
work themselves, we can remove the legacy
CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY behaviour.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
kernel/irq/irqdesc.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index c8bc856c5e9f..acfb163b9f36 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -677,31 +677,6 @@ int generic_handle_domain_irq(struct irq_domain *domain, unsigned int hwirq)
EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
#ifdef CONFIG_HANDLE_DOMAIN_IRQ
-#ifdef CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY
-/**
- * handle_domain_irq - Invoke the handler for a HW irq belonging to a domain,
- * usually for a root interrupt controller
- * @domain: The domain where to perform the lookup
- * @hwirq: The HW irq number to convert to a logical one
- * @regs: Register file coming from the low-level handling code
- *
- * Returns: 0 on success, or -EINVAL if conversion has failed
- */
-int handle_domain_irq(struct irq_domain *domain,
- unsigned int hwirq, struct pt_regs *regs)
-{
- struct pt_regs *old_regs = set_irq_regs(regs);
- int ret;
-
- irq_enter();
-
- ret = generic_handle_domain_irq(domain, hwirq);
-
- irq_exit();
- set_irq_regs(old_regs);
- return ret;
-}
-#else
/**
* handle_domain_irq - Invoke the handler for a HW irq belonging to a domain,
* usually for a root interrupt controller
@@ -729,7 +704,6 @@ int handle_domain_irq(struct irq_domain *domain,
set_irq_regs(old_regs);
return ret;
}
-#endif
/**
* handle_domain_nmi - Invoke the handler for a HW irq belonging to a domain
--
2.11.0
In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/csky
perform all the irqentry accounting in its entry code. As arch/csky uses
GENERIC_IRQ_MULTI_HANDLER, we can use generic_handle_arch_irq() to do
so.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Guo Ren <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
arch/csky/Kconfig | 1 -
arch/csky/kernel/entry.S | 2 +-
arch/csky/kernel/irq.c | 5 -----
3 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index 45f03f674a61..9d4d898df76b 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -18,7 +18,6 @@ config CSKY
select DMA_DIRECT_REMAP
select IRQ_DOMAIN
select HANDLE_DOMAIN_IRQ
- select HANDLE_DOMAIN_IRQ_IRQENTRY
select DW_APB_TIMER_OF
select GENERIC_IOREMAP
select GENERIC_LIB_ASHLDI3
diff --git a/arch/csky/kernel/entry.S b/arch/csky/kernel/entry.S
index 00e3c8ebf9b8..a4ababf25e24 100644
--- a/arch/csky/kernel/entry.S
+++ b/arch/csky/kernel/entry.S
@@ -249,7 +249,7 @@ ENTRY(csky_irq)
mov a0, sp
- jbsr csky_do_IRQ
+ jbsr generic_handle_arch_irq
jmpi ret_from_exception
diff --git a/arch/csky/kernel/irq.c b/arch/csky/kernel/irq.c
index 03a1930f1cbb..fcdaf3156286 100644
--- a/arch/csky/kernel/irq.c
+++ b/arch/csky/kernel/irq.c
@@ -15,8 +15,3 @@ void __init init_IRQ(void)
setup_smp_ipi();
#endif
}
-
-asmlinkage void __irq_entry csky_do_IRQ(struct pt_regs *regs)
-{
- handle_arch_irq(regs);
-}
--
2.11.0
In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/arm64
perform all the irqentry accounting in its entry code.
As arch/arm64 already performs portions of the irqentry logic in
enter_from_kernel_mode() and exit_to_kernel_mode(), including
rcu_irq_{enter,exit}(), the only additional calls that need to be made
are to irq_{enter,exit}_rcu(). Removing the calls to
rcu_irq_{enter,exit}() from handle_domain_irq() ensures that we inform
RCU once per IRQ entry and will correctly identify quiescent periods.
Since we should not call irq_{enter,exit}_rcu() when entering a
pseudo-NMI, el1_interrupt() is reworked to have separate __el1_irq() and
__el1_pnmi() paths for regular IRQ and psuedo-NMI entry, with
irq_{enter,exit}_irq() only called for the former.
In preparation for removing HANDLE_DOMAIN_IRQ, the irq regs are managed
in do_interrupt_handler() for both regular IRQ and pseudo-NMI. This is
currently redundant, but not harmful.
For clarity the preemption logic is moved into __el1_irq(). We should
never preempt within a pseudo-NMI, and arm64_enter_nmi() already
enforces this by incrementing the preempt_count, but it's clearer if we
never invoke the preemption logic when entering a pseudo-NMI.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/Kconfig | 1 -
arch/arm64/kernel/entry-common.c | 52 ++++++++++++++++++++++++----------------
2 files changed, 31 insertions(+), 22 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 553239a5a5f7..5c7ae4c3954b 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -134,7 +134,6 @@ config ARM64
select GENERIC_GETTIMEOFDAY
select GENERIC_VDSO_TIME_NS
select HANDLE_DOMAIN_IRQ
- select HANDLE_DOMAIN_IRQ_IRQENTRY
select HARDIRQS_SW_RESEND
select HAVE_MOVE_PMD
select HAVE_MOVE_PUD
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 32f9796c4ffe..f7408edf8571 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -17,6 +17,7 @@
#include <asm/daifflags.h>
#include <asm/esr.h>
#include <asm/exception.h>
+#include <asm/irq_regs.h>
#include <asm/kprobes.h>
#include <asm/mmu.h>
#include <asm/processor.h>
@@ -219,22 +220,6 @@ static void noinstr arm64_exit_el1_dbg(struct pt_regs *regs)
lockdep_hardirqs_on(CALLER_ADDR0);
}
-static void noinstr enter_el1_irq_or_nmi(struct pt_regs *regs)
-{
- if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
- arm64_enter_nmi(regs);
- else
- enter_from_kernel_mode(regs);
-}
-
-static void noinstr exit_el1_irq_or_nmi(struct pt_regs *regs)
-{
- if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
- arm64_exit_nmi(regs);
- else
- exit_to_kernel_mode(regs);
-}
-
static void __sched arm64_preempt_schedule_irq(void)
{
lockdep_assert_irqs_disabled();
@@ -263,10 +248,14 @@ static void __sched arm64_preempt_schedule_irq(void)
static void do_interrupt_handler(struct pt_regs *regs,
void (*handler)(struct pt_regs *))
{
+ struct pt_regs *old_regs = set_irq_regs(regs);
+
if (on_thread_stack())
call_on_irq_stack(regs, handler);
else
handler(regs);
+
+ set_irq_regs(old_regs);
}
extern void (*handle_arch_irq)(struct pt_regs *);
@@ -432,13 +421,22 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
}
}
-static void noinstr el1_interrupt(struct pt_regs *regs,
- void (*handler)(struct pt_regs *))
+static __always_inline void __el1_pnmi(struct pt_regs *regs,
+ void (*handler)(struct pt_regs *))
{
- write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
+ arm64_enter_nmi(regs);
+ do_interrupt_handler(regs, handler);
+ arm64_exit_nmi(regs);
+}
+
+static __always_inline void __el1_irq(struct pt_regs *regs,
+ void (*handler)(struct pt_regs *))
+{
+ enter_from_kernel_mode(regs);
- enter_el1_irq_or_nmi(regs);
+ irq_enter_rcu();
do_interrupt_handler(regs, handler);
+ irq_exit_rcu();
/*
* Note: thread_info::preempt_count includes both thread_info::count
@@ -449,7 +447,17 @@ static void noinstr el1_interrupt(struct pt_regs *regs,
READ_ONCE(current_thread_info()->preempt_count) == 0)
arm64_preempt_schedule_irq();
- exit_el1_irq_or_nmi(regs);
+ exit_to_kernel_mode(regs);
+}
+static void noinstr el1_interrupt(struct pt_regs *regs,
+ void (*handler)(struct pt_regs *))
+{
+ write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
+
+ if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
+ __el1_pnmi(regs, handler);
+ else
+ __el1_irq(regs, handler);
}
asmlinkage void noinstr el1h_64_irq_handler(struct pt_regs *regs)
@@ -667,7 +675,9 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
if (regs->pc & BIT(55))
arm64_apply_bp_hardening();
+ irq_enter_rcu();
do_interrupt_handler(regs, handler);
+ irq_exit_rcu();
exit_to_user_mode(regs);
}
--
2.11.0
In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/arm
perform all the irqentry accounting in its entry code.
For configurations with CONFIG_GENERIC_IRQ_MULTI_HANDLER, we can use
generic_handle_arch_irq(). Other than asm_do_IRQ(), all C calls to
handle_IRQ() are from irqchip handlers which will be called from
generic_handle_arch_irq(), so to avoid double accounting IRQ entry, the
entry logic is moved from handle_IRQ() into asm_do_IRQ().
For ARMv7M the entry assembly is tightly coupled with the NVIC irqchip, and
while the entry code should logically live under arch/arm/, moving the
entry logic there makes things more convoluted. So for now, place the
entry logic in the NVIC irqchip, but separated into a separate
function to make the split of responsibility clear.
For all other configurations without CONFIG_GENERIC_IRQ_MULTI_HANDLER,
IRQ entry is already handled in arch code, and requires no changes.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Russell King <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
arch/arm/Kconfig | 1 -
arch/arm/kernel/entry-armv.S | 5 +----
arch/arm/kernel/irq.c | 14 ++++++++------
drivers/irqchip/irq-nvic.c | 19 +++++++++++++++++--
4 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f18aff82c27b..fc196421b2ce 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -65,7 +65,6 @@ config ARM
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
select HANDLE_DOMAIN_IRQ
- select HANDLE_DOMAIN_IRQ_IRQENTRY
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL if AEABI && !OABI_COMPAT
select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 241b73d64df7..3d0b6169ab86 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -38,14 +38,11 @@
*/
.macro irq_handler
#ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
- ldr r1, =handle_arch_irq
mov r0, sp
- badr lr, 9997f
- ldr pc, [r1]
+ bl generic_handle_arch_irq
#else
arch_irq_handler_default
#endif
-9997:
.endm
.macro pabt_helper
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 20ab1e607522..b79975bd988c 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -63,11 +63,8 @@ int arch_show_interrupts(struct seq_file *p, int prec)
*/
void handle_IRQ(unsigned int irq, struct pt_regs *regs)
{
- struct pt_regs *old_regs = set_irq_regs(regs);
struct irq_desc *desc;
- irq_enter();
-
/*
* Some hardware gives randomly wrong interrupts. Rather
* than crashing, do something sensible.
@@ -81,9 +78,6 @@ void handle_IRQ(unsigned int irq, struct pt_regs *regs)
handle_irq_desc(desc);
else
ack_bad_irq(irq);
-
- irq_exit();
- set_irq_regs(old_regs);
}
/*
@@ -92,7 +86,15 @@ void handle_IRQ(unsigned int irq, struct pt_regs *regs)
asmlinkage void __exception_irq_entry
asm_do_IRQ(unsigned int irq, struct pt_regs *regs)
{
+ struct pt_regs *old_regs;
+
+ irq_enter();
+ old_regs = set_irq_regs(regs);
+
handle_IRQ(irq, regs);
+
+ set_irq_regs(old_regs);
+ irq_exit();
}
void __init init_IRQ(void)
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
index b31c4cff4d3a..ce330880665e 100644
--- a/drivers/irqchip/irq-nvic.c
+++ b/drivers/irqchip/irq-nvic.c
@@ -37,12 +37,27 @@
static struct irq_domain *nvic_irq_domain;
-asmlinkage void __exception_irq_entry
-nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
+static void __nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
{
handle_domain_irq(nvic_irq_domain, hwirq, regs);
}
+/*
+ * TODO: restructure the ARMv7M entry logic so that this entry logic can live
+ * in arch code.
+ */
+asmlinkage void __exception_irq_entry
+static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
+{
+ struct pt_regs *old_regs;
+
+ irq_enter();
+ old_regs = set_irq_regs(regs);
+ __nvic_handle_irq(hwirq, regs);
+ set_irq_regs(old_regs);
+ irq_exit();
+}
+
static int nvic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
unsigned int nr_irqs, void *arg)
{
--
2.11.0
In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have
arch/openrisc perform all the irqentry accounting in its entry code. As
arch/openrisc uses GENERIC_IRQ_MULTI_HANDLER, we can use
generic_handle_arch_irq() to do so.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Jonas Bonn <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Stafford Horne <[email protected]>
Cc: Stefan Kristiansson <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
arch/openrisc/Kconfig | 1 -
arch/openrisc/kernel/entry.S | 4 ++--
arch/openrisc/kernel/irq.c | 5 -----
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index ed783a67065e..e804026b4797 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -14,7 +14,6 @@ config OPENRISC
select OF_EARLY_FLATTREE
select IRQ_DOMAIN
select HANDLE_DOMAIN_IRQ
- select HANDLE_DOMAIN_IRQ_IRQENTRY
select GPIOLIB
select HAVE_ARCH_TRACEHOOK
select SPARSE_IRQ
diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index edaa775a648e..59c6d3aa7081 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -569,8 +569,8 @@ EXCEPTION_ENTRY(_external_irq_handler)
#endif
CLEAR_LWA_FLAG(r3)
l.addi r3,r1,0
- l.movhi r8,hi(do_IRQ)
- l.ori r8,r8,lo(do_IRQ)
+ l.movhi r8,hi(generic_handle_arch_irq)
+ l.ori r8,r8,lo(generic_handle_arch_irq)
l.jalr r8
l.nop
l.j _ret_from_intr
diff --git a/arch/openrisc/kernel/irq.c b/arch/openrisc/kernel/irq.c
index c38fa863afa8..f38e10962a84 100644
--- a/arch/openrisc/kernel/irq.c
+++ b/arch/openrisc/kernel/irq.c
@@ -36,8 +36,3 @@ void __init init_IRQ(void)
{
irqchip_init();
}
-
-void __irq_entry do_IRQ(struct pt_regs *regs)
-{
- handle_arch_irq(regs);
-}
--
2.11.0
In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/riscv
perform all the irqentry accounting in its entry code. As arch/riscv
uses GENERIC_IRQ_MULTI_HANDLER, we can use generic_handle_arch_irq() to
do so.
Since generic_handle_arch_irq() handles the irq entry and setting the
irq regs, and happens before the irqchip code calls handle_IPI(), we can
remove the redundant irq entry and irq regs manipulation from
handle_IPI().
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Albert Ou <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
arch/riscv/Kconfig | 1 -
arch/riscv/kernel/entry.S | 3 +--
arch/riscv/kernel/smp.c | 9 +--------
3 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 740653063a56..301a54233c7e 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -63,7 +63,6 @@ config RISCV
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL if MMU && 64BIT
select HANDLE_DOMAIN_IRQ
- select HANDLE_DOMAIN_IRQ_IRQENTRY
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 98f502654edd..64236f7efde5 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -130,8 +130,7 @@ skip_context_tracking:
/* Handle interrupts */
move a0, sp /* pt_regs */
- la a1, handle_arch_irq
- REG_L a1, (a1)
+ la a1, generic_handle_arch_irq
jr a1
1:
/*
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 921d9d7df400..2f6da845c9ae 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -140,12 +140,9 @@ void arch_irq_work_raise(void)
void handle_IPI(struct pt_regs *regs)
{
- struct pt_regs *old_regs = set_irq_regs(regs);
unsigned long *pending_ipis = &ipi_data[smp_processor_id()].bits;
unsigned long *stats = ipi_data[smp_processor_id()].stats;
- irq_enter();
-
riscv_clear_ipi();
while (true) {
@@ -156,7 +153,7 @@ void handle_IPI(struct pt_regs *regs)
ops = xchg(pending_ipis, 0);
if (ops == 0)
- goto done;
+ return;
if (ops & (1 << IPI_RESCHEDULE)) {
stats[IPI_RESCHEDULE]++;
@@ -189,10 +186,6 @@ void handle_IPI(struct pt_regs *regs)
/* Order data access and bit testing. */
mb();
}
-
-done:
- irq_exit();
- set_irq_regs(old_regs);
}
static const char * const ipi_names[] = {
--
2.11.0
Going forward we want architecture/entry code to perform all the
necessary work to enter/exit IRQ context, with irqchip code merely
handling the mapping of the interrupt to any handler(s). Among other
reasons, this is necessary to consistently fix some longstanding issues
with the ordering of lockdep/RCU/tracing instrumentation which many
architectures get wrong today in their entry code.
Importantly, rcu_irq_{enter,exit}() must be called precisely once per
IRQ exception, so that rcu_is_cpu_rrupt_from_idle() can correctly
identify when an interrupt was taken from an idle context which must be
explicitly preempted. Currently handle_domain_irq() calls
rcu_irq_{enter,exit}() via irq_{enter,exit}(), but entry code needs to
be able to call rcu_irq_{enter,exit}() earlier for correct ordering
across lockdep/RCU/tracing updates for sequences such as:
lockdep_hardirqs_off(CALLER_ADDR0);
rcu_irq_enter();
trace_hardirqs_off_finish();
To permit each architecture to be converted to the new style in turn,
this patch adds a new CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY selected by all
current users of HANDLE_DOMAIN_IRQ, which gates the existing behaviour.
When CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY is not selected,
handle_domain_irq() requires entry code to perform the
irq_{enter,exit}() work, with an explicit check for this matching the
style of handle_domain_nmi().
Subsequent patches will:
1) Add the necessary IRQ entry accounting to each architecture in turn,
dropping CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY from that architecture's
Kconfig.
2) Remove CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY once it is no longer
selected.
3) Convert irqchip drivers to consistently use
generic_handle_domain_irq() rather than handle_domain_irq().
4) Remove handle_domain_irq() and CONFIG_HANDLE_DOMAIN_IRQ.
... which should leave us with a clear split of responsiblity across the
entry and irqchip code, making it possible to perform additional
cleanups and fixes for the aforementioned longstanding issues with entry
code.
There should be no functional change as a result of this patch.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm64/Kconfig | 1 +
arch/csky/Kconfig | 1 +
arch/openrisc/Kconfig | 1 +
arch/riscv/Kconfig | 1 +
kernel/irq/Kconfig | 4 ++++
kernel/irq/irqdesc.c | 30 ++++++++++++++++++++++++++++++
7 files changed, 39 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index fc196421b2ce..f18aff82c27b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -65,6 +65,7 @@ config ARM
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
select HANDLE_DOMAIN_IRQ
+ select HANDLE_DOMAIN_IRQ_IRQENTRY
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL if AEABI && !OABI_COMPAT
select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5c7ae4c3954b..553239a5a5f7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -134,6 +134,7 @@ config ARM64
select GENERIC_GETTIMEOFDAY
select GENERIC_VDSO_TIME_NS
select HANDLE_DOMAIN_IRQ
+ select HANDLE_DOMAIN_IRQ_IRQENTRY
select HARDIRQS_SW_RESEND
select HAVE_MOVE_PMD
select HAVE_MOVE_PUD
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index 9d4d898df76b..45f03f674a61 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -18,6 +18,7 @@ config CSKY
select DMA_DIRECT_REMAP
select IRQ_DOMAIN
select HANDLE_DOMAIN_IRQ
+ select HANDLE_DOMAIN_IRQ_IRQENTRY
select DW_APB_TIMER_OF
select GENERIC_IOREMAP
select GENERIC_LIB_ASHLDI3
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index e804026b4797..ed783a67065e 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -14,6 +14,7 @@ config OPENRISC
select OF_EARLY_FLATTREE
select IRQ_DOMAIN
select HANDLE_DOMAIN_IRQ
+ select HANDLE_DOMAIN_IRQ_IRQENTRY
select GPIOLIB
select HAVE_ARCH_TRACEHOOK
select SPARSE_IRQ
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 301a54233c7e..740653063a56 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -63,6 +63,7 @@ config RISCV
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL if MMU && 64BIT
select HANDLE_DOMAIN_IRQ
+ select HANDLE_DOMAIN_IRQ_IRQENTRY
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index fbc54c2a7f23..897dfc552bb0 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -100,6 +100,10 @@ config IRQ_MSI_IOMMU
config HANDLE_DOMAIN_IRQ
bool
+# Legacy behaviour; architectures should call irq_{enter,exit}() themselves
+config HANDLE_DOMAIN_IRQ_IRQENTRY
+ bool
+
config IRQ_TIMINGS
bool
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index b07d0e1552bc..c8bc856c5e9f 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -677,6 +677,7 @@ int generic_handle_domain_irq(struct irq_domain *domain, unsigned int hwirq)
EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
#ifdef CONFIG_HANDLE_DOMAIN_IRQ
+#ifdef CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY
/**
* handle_domain_irq - Invoke the handler for a HW irq belonging to a domain,
* usually for a root interrupt controller
@@ -700,6 +701,35 @@ int handle_domain_irq(struct irq_domain *domain,
set_irq_regs(old_regs);
return ret;
}
+#else
+/**
+ * handle_domain_irq - Invoke the handler for a HW irq belonging to a domain,
+ * usually for a root interrupt controller
+ * @domain: The domain where to perform the lookup
+ * @hwirq: The HW irq number to convert to a logical one
+ * @regs: Register file coming from the low-level handling code
+ *
+ * This function must be called from an IRQ context.
+ *
+ * Returns: 0 on success, or -EINVAL if conversion has failed
+ */
+int handle_domain_irq(struct irq_domain *domain,
+ unsigned int hwirq, struct pt_regs *regs)
+{
+ struct pt_regs *old_regs = set_irq_regs(regs);
+ int ret;
+
+ /*
+ * IRQ context needs to be setup earlier.
+ */
+ WARN_ON(!in_irq());
+
+ ret = generic_handle_domain_irq(domain, hwirq);
+
+ set_irq_regs(old_regs);
+ return ret;
+}
+#endif
/**
* handle_domain_nmi - Invoke the handler for a HW irq belonging to a domain
--
2.11.0
Now that entry code handles IRQ entry (including setting the IRQ regs)
before calling irqchip code, irqchip code can safely call
generic_handle_domain_irq(), and there's no functional reason for it to
call handle_domain_irq().
Let's cement this split of responsibility and remove handle_domain_irq()
entirely, updating irqchip drivers to call generic_handle_domain_irq().
For consistency, handle_domain_nmi() is similarly removed and replaced
with a generic_handle_domain_nmi() function which also does not perform
any entry logic.
Previously handle_domain_{irq,nmi}() had a WARN_ON() which would fire
when they were called in an inappropriate context. So that we can
identify similar issues going forward, similar WARN_ON_ONCE() logic is
added to the generic_handle_*() functions, and comments are updated for
clarity and consistency.
Signed-off-by: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
Documentation/core-api/irq/irq-domain.rst | 3 --
arch/arm/Kconfig | 1 -
arch/arm/mach-imx/avic.c | 2 +-
arch/arm/mach-imx/tzic.c | 2 +-
arch/arm/mach-omap1/irq.c | 2 +-
arch/arm/mach-s3c/irq-s3c24xx.c | 2 +-
arch/arm64/Kconfig | 1 -
arch/csky/Kconfig | 1 -
arch/openrisc/Kconfig | 1 -
arch/riscv/Kconfig | 1 -
drivers/irqchip/irq-apple-aic.c | 20 ++++-----
drivers/irqchip/irq-armada-370-xp.c | 13 ++----
drivers/irqchip/irq-aspeed-vic.c | 2 +-
drivers/irqchip/irq-atmel-aic.c | 2 +-
drivers/irqchip/irq-atmel-aic5.c | 2 +-
drivers/irqchip/irq-bcm2835.c | 2 +-
drivers/irqchip/irq-bcm2836.c | 2 +-
drivers/irqchip/irq-clps711x.c | 8 ++--
drivers/irqchip/irq-csky-apb-intc.c | 2 +-
drivers/irqchip/irq-csky-mpintc.c | 4 +-
drivers/irqchip/irq-davinci-aintc.c | 2 +-
drivers/irqchip/irq-davinci-cp-intc.c | 2 +-
drivers/irqchip/irq-digicolor.c | 2 +-
drivers/irqchip/irq-dw-apb-ictl.c | 2 +-
drivers/irqchip/irq-ftintc010.c | 2 +-
drivers/irqchip/irq-gic-v3.c | 4 +-
drivers/irqchip/irq-gic.c | 2 +-
drivers/irqchip/irq-hip04.c | 2 +-
drivers/irqchip/irq-ixp4xx.c | 4 +-
drivers/irqchip/irq-lpc32xx.c | 2 +-
drivers/irqchip/irq-mmp.c | 4 +-
drivers/irqchip/irq-mxs.c | 2 +-
drivers/irqchip/irq-nvic.c | 6 +--
drivers/irqchip/irq-omap-intc.c | 2 +-
drivers/irqchip/irq-or1k-pic.c | 2 +-
drivers/irqchip/irq-orion.c | 4 +-
drivers/irqchip/irq-rda-intc.c | 2 +-
drivers/irqchip/irq-riscv-intc.c | 2 +-
drivers/irqchip/irq-sa11x0.c | 4 +-
drivers/irqchip/irq-sun4i.c | 2 +-
drivers/irqchip/irq-versatile-fpga.c | 2 +-
drivers/irqchip/irq-vic.c | 2 +-
drivers/irqchip/irq-vt8500.c | 2 +-
drivers/irqchip/irq-wpcm450-aic.c | 2 +-
drivers/irqchip/irq-zevio.c | 2 +-
include/linux/irqdesc.h | 9 +---
kernel/irq/Kconfig | 7 ----
kernel/irq/irqdesc.c | 69 ++++++++-----------------------
48 files changed, 81 insertions(+), 141 deletions(-)
diff --git a/Documentation/core-api/irq/irq-domain.rst b/Documentation/core-api/irq/irq-domain.rst
index 9c0e8758037a..d30b4d0a9769 100644
--- a/Documentation/core-api/irq/irq-domain.rst
+++ b/Documentation/core-api/irq/irq-domain.rst
@@ -67,9 +67,6 @@ variety of methods:
deprecated
- generic_handle_domain_irq() handles an interrupt described by a
domain and a hwirq number
-- handle_domain_irq() does the same thing for root interrupt
- controllers and deals with the set_irq_reg()/irq_enter() sequences
- that most architecture requires
Note that irq domain lookups must happen in contexts that are
compatible with a RCU read-side critical section.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index fc196421b2ce..3361a6c29ee9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -64,7 +64,6 @@ config ARM
select GENERIC_PCI_IOMAP
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
- select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL if AEABI && !OABI_COMPAT
select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
diff --git a/arch/arm/mach-imx/avic.c b/arch/arm/mach-imx/avic.c
index 21bce4049cec..cf6546ddc7a3 100644
--- a/arch/arm/mach-imx/avic.c
+++ b/arch/arm/mach-imx/avic.c
@@ -154,7 +154,7 @@ static void __exception_irq_entry avic_handle_irq(struct pt_regs *regs)
if (nivector == 0xffff)
break;
- handle_domain_irq(domain, nivector, regs);
+ generic_handle_domain_irq(domain, nivector);
} while (1);
}
diff --git a/arch/arm/mach-imx/tzic.c b/arch/arm/mach-imx/tzic.c
index 479a01bdac56..8b3d98d288d9 100644
--- a/arch/arm/mach-imx/tzic.c
+++ b/arch/arm/mach-imx/tzic.c
@@ -134,7 +134,7 @@ static void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs)
while (stat) {
handled = 1;
irqofs = fls(stat) - 1;
- handle_domain_irq(domain, irqofs + i * 32, regs);
+ generic_handle_domain_irq(domain, irqofs + i * 32);
stat &= ~(1 << irqofs);
}
}
diff --git a/arch/arm/mach-omap1/irq.c b/arch/arm/mach-omap1/irq.c
index b11edc8a46f0..ee6a93083154 100644
--- a/arch/arm/mach-omap1/irq.c
+++ b/arch/arm/mach-omap1/irq.c
@@ -165,7 +165,7 @@ asmlinkage void __exception_irq_entry omap1_handle_irq(struct pt_regs *regs)
}
irq:
if (irqnr)
- handle_domain_irq(domain, irqnr, regs);
+ generic_handle_domain_irq(domain, irqnr);
else
break;
} while (irqnr);
diff --git a/arch/arm/mach-s3c/irq-s3c24xx.c b/arch/arm/mach-s3c/irq-s3c24xx.c
index 3edc5f614eef..45dfd546e6fa 100644
--- a/arch/arm/mach-s3c/irq-s3c24xx.c
+++ b/arch/arm/mach-s3c/irq-s3c24xx.c
@@ -354,7 +354,7 @@ static inline int s3c24xx_handle_intc(struct s3c_irq_intc *intc,
if (!(pnd & (1 << offset)))
offset = __ffs(pnd);
- handle_domain_irq(intc->domain, intc_offset + offset, regs);
+ generic_handle_domain_irq(intc->domain, intc_offset + offset);
return true;
}
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5c7ae4c3954b..e6593a03ea27 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -133,7 +133,6 @@ config ARM64
select GENERIC_TIME_VSYSCALL
select GENERIC_GETTIMEOFDAY
select GENERIC_VDSO_TIME_NS
- select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_MOVE_PMD
select HAVE_MOVE_PUD
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index 9d4d898df76b..e0bd71b9e23f 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -17,7 +17,6 @@ config CSKY
select CSKY_APB_INTC
select DMA_DIRECT_REMAP
select IRQ_DOMAIN
- select HANDLE_DOMAIN_IRQ
select DW_APB_TIMER_OF
select GENERIC_IOREMAP
select GENERIC_LIB_ASHLDI3
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index e804026b4797..c2491b295d60 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -13,7 +13,6 @@ config OPENRISC
select OF
select OF_EARLY_FLATTREE
select IRQ_DOMAIN
- select HANDLE_DOMAIN_IRQ
select GPIOLIB
select HAVE_ARCH_TRACEHOOK
select SPARSE_IRQ
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 301a54233c7e..353e28f5f849 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -62,7 +62,6 @@ config RISCV
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL if MMU && 64BIT
- select HANDLE_DOMAIN_IRQ
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index 6fc145aacaf0..3759dc36cc8f 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -245,7 +245,7 @@ static void __exception_irq_entry aic_handle_irq(struct pt_regs *regs)
irq = FIELD_GET(AIC_EVENT_NUM, event);
if (type == AIC_EVENT_TYPE_HW)
- handle_domain_irq(aic_irqc->hw_domain, irq, regs);
+ generic_handle_domain_irq(aic_irqc->hw_domain, irq);
else if (type == AIC_EVENT_TYPE_IPI && irq == 1)
aic_handle_ipi(regs);
else if (event != 0)
@@ -392,25 +392,25 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs)
}
if (TIMER_FIRING(read_sysreg(cntp_ctl_el0)))
- handle_domain_irq(aic_irqc->hw_domain,
- aic_irqc->nr_hw + AIC_TMR_EL0_PHYS, regs);
+ generic_handle_domain_irq(aic_irqc->hw_domain,
+ aic_irqc->nr_hw + AIC_TMR_EL0_PHYS);
if (TIMER_FIRING(read_sysreg(cntv_ctl_el0)))
- handle_domain_irq(aic_irqc->hw_domain,
- aic_irqc->nr_hw + AIC_TMR_EL0_VIRT, regs);
+ generic_handle_domain_irq(aic_irqc->hw_domain,
+ aic_irqc->nr_hw + AIC_TMR_EL0_VIRT);
if (is_kernel_in_hyp_mode()) {
uint64_t enabled = read_sysreg_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2);
if ((enabled & VM_TMR_FIQ_ENABLE_P) &&
TIMER_FIRING(read_sysreg_s(SYS_CNTP_CTL_EL02)))
- handle_domain_irq(aic_irqc->hw_domain,
- aic_irqc->nr_hw + AIC_TMR_EL02_PHYS, regs);
+ generic_handle_domain_irq(aic_irqc->hw_domain,
+ aic_irqc->nr_hw + AIC_TMR_EL02_PHYS);
if ((enabled & VM_TMR_FIQ_ENABLE_V) &&
TIMER_FIRING(read_sysreg_s(SYS_CNTV_CTL_EL02)))
- handle_domain_irq(aic_irqc->hw_domain,
- aic_irqc->nr_hw + AIC_TMR_EL02_VIRT, regs);
+ generic_handle_domain_irq(aic_irqc->hw_domain,
+ aic_irqc->nr_hw + AIC_TMR_EL02_VIRT);
}
if ((read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) & (PMCR0_IMODE | PMCR0_IACT)) ==
@@ -674,7 +674,7 @@ static void aic_handle_ipi(struct pt_regs *regs)
firing = atomic_fetch_andnot(enabled, this_cpu_ptr(&aic_vipi_flag)) & enabled;
for_each_set_bit(i, &firing, AIC_NR_SWIPI)
- handle_domain_irq(aic_irqc->ipi_domain, i, regs);
+ generic_handle_domain_irq(aic_irqc->ipi_domain, i);
/*
* No ordering needed here; at worst this just changes the timing of
diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 53e0fb0562c1..80906bfec845 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -589,12 +589,7 @@ static void armada_370_xp_handle_msi_irq(struct pt_regs *regs, bool is_chained)
irq = msinr - PCI_MSI_DOORBELL_START;
- if (is_chained)
- generic_handle_domain_irq(armada_370_xp_msi_inner_domain,
- irq);
- else
- handle_domain_irq(armada_370_xp_msi_inner_domain,
- irq, regs);
+ generic_handle_domain_irq(armada_370_xp_msi_inner_domain, irq);
}
}
#else
@@ -646,8 +641,8 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
break;
if (irqnr > 1) {
- handle_domain_irq(armada_370_xp_mpic_domain,
- irqnr, regs);
+ generic_handle_domain_irq(armada_370_xp_mpic_domain,
+ irqnr);
continue;
}
@@ -666,7 +661,7 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
& IPI_DOORBELL_MASK;
for_each_set_bit(ipi, &ipimask, IPI_DOORBELL_END)
- handle_domain_irq(ipi_domain, ipi, regs);
+ generic_handle_domain_irq(ipi_domain, ipi);
}
#endif
diff --git a/drivers/irqchip/irq-aspeed-vic.c b/drivers/irqchip/irq-aspeed-vic.c
index 58717cd44f99..62ccf2c0c414 100644
--- a/drivers/irqchip/irq-aspeed-vic.c
+++ b/drivers/irqchip/irq-aspeed-vic.c
@@ -100,7 +100,7 @@ static void __exception_irq_entry avic_handle_irq(struct pt_regs *regs)
if (stat == 0)
break;
irq += ffs(stat) - 1;
- handle_domain_irq(vic->dom, irq, regs);
+ generic_handle_domain_irq(vic->dom, irq);
}
}
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c
index 2c999dc310c1..4631f6847953 100644
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -71,7 +71,7 @@ aic_handle(struct pt_regs *regs)
if (!irqstat)
irq_reg_writel(gc, 0, AT91_AIC_EOICR);
else
- handle_domain_irq(aic_domain, irqnr, regs);
+ generic_handle_domain_irq(aic_domain, irqnr);
}
static int aic_retrigger(struct irq_data *d)
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index fb4ad2aaa727..145535bd7560 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -80,7 +80,7 @@ aic5_handle(struct pt_regs *regs)
if (!irqstat)
irq_reg_writel(bgc, 0, AT91_AIC5_EOICR);
else
- handle_domain_irq(aic5_domain, irqnr, regs);
+ generic_handle_domain_irq(aic5_domain, irqnr);
}
static void aic5_mask(struct irq_data *d)
diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index adc1556ed332..e94e2882286c 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -246,7 +246,7 @@ static void __exception_irq_entry bcm2835_handle_irq(
u32 hwirq;
while ((hwirq = get_next_armctrl_hwirq()) != ~0)
- handle_domain_irq(intc.domain, hwirq, regs);
+ generic_handle_domain_irq(intc.domain, hwirq);
}
static void bcm2836_chained_handle_irq(struct irq_desc *desc)
diff --git a/drivers/irqchip/irq-bcm2836.c b/drivers/irqchip/irq-bcm2836.c
index 501facdb4570..51491c3c6fdd 100644
--- a/drivers/irqchip/irq-bcm2836.c
+++ b/drivers/irqchip/irq-bcm2836.c
@@ -143,7 +143,7 @@ __exception_irq_entry bcm2836_arm_irqchip_handle_irq(struct pt_regs *regs)
if (stat) {
u32 hwirq = ffs(stat) - 1;
- handle_domain_irq(intc.domain, hwirq, regs);
+ generic_handle_domain_irq(intc.domain, hwirq);
}
}
diff --git a/drivers/irqchip/irq-clps711x.c b/drivers/irqchip/irq-clps711x.c
index d0da29aeedc8..77ebe7e47e0e 100644
--- a/drivers/irqchip/irq-clps711x.c
+++ b/drivers/irqchip/irq-clps711x.c
@@ -77,14 +77,14 @@ static asmlinkage void __exception_irq_entry clps711x_irqh(struct pt_regs *regs)
irqstat = readw_relaxed(clps711x_intc->intmr[0]) &
readw_relaxed(clps711x_intc->intsr[0]);
if (irqstat)
- handle_domain_irq(clps711x_intc->domain,
- fls(irqstat) - 1, regs);
+ generic_handle_domain_irq(clps711x_intc->domain,
+ fls(irqstat) - 1);
irqstat = readw_relaxed(clps711x_intc->intmr[1]) &
readw_relaxed(clps711x_intc->intsr[1]);
if (irqstat)
- handle_domain_irq(clps711x_intc->domain,
- fls(irqstat) - 1 + 16, regs);
+ generic_handle_domain_irq(clps711x_intc->domain,
+ fls(irqstat) - 1 + 16);
} while (irqstat);
}
diff --git a/drivers/irqchip/irq-csky-apb-intc.c b/drivers/irqchip/irq-csky-apb-intc.c
index ab91afa86755..d36f536506ba 100644
--- a/drivers/irqchip/irq-csky-apb-intc.c
+++ b/drivers/irqchip/irq-csky-apb-intc.c
@@ -138,7 +138,7 @@ static inline bool handle_irq_perbit(struct pt_regs *regs, u32 hwirq,
if (hwirq == 0)
return 0;
- handle_domain_irq(root_domain, irq_base + __fls(hwirq), regs);
+ generic_handle_domain_irq(root_domain, irq_base + __fls(hwirq));
return 1;
}
diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index a1534edef7fa..cb403c960ac0 100644
--- a/drivers/irqchip/irq-csky-mpintc.c
+++ b/drivers/irqchip/irq-csky-mpintc.c
@@ -74,8 +74,8 @@ static void csky_mpintc_handler(struct pt_regs *regs)
{
void __iomem *reg_base = this_cpu_read(intcl_reg);
- handle_domain_irq(root_domain,
- readl_relaxed(reg_base + INTCL_RDYIR), regs);
+ generic_handle_domain_irq(root_domain,
+ readl_relaxed(reg_base + INTCL_RDYIR));
}
static void csky_mpintc_enable(struct irq_data *d)
diff --git a/drivers/irqchip/irq-davinci-aintc.c b/drivers/irqchip/irq-davinci-aintc.c
index 810ccc4fe476..123eb7bfc117 100644
--- a/drivers/irqchip/irq-davinci-aintc.c
+++ b/drivers/irqchip/irq-davinci-aintc.c
@@ -73,7 +73,7 @@ davinci_aintc_handle_irq(struct pt_regs *regs)
irqnr >>= 2;
irqnr -= 1;
- handle_domain_irq(davinci_aintc_irq_domain, irqnr, regs);
+ generic_handle_domain_irq(davinci_aintc_irq_domain, irqnr);
}
/* ARM Interrupt Controller Initialization */
diff --git a/drivers/irqchip/irq-davinci-cp-intc.c b/drivers/irqchip/irq-davinci-cp-intc.c
index 276da2772e7f..7482c8ed34b2 100644
--- a/drivers/irqchip/irq-davinci-cp-intc.c
+++ b/drivers/irqchip/irq-davinci-cp-intc.c
@@ -135,7 +135,7 @@ davinci_cp_intc_handle_irq(struct pt_regs *regs)
return;
}
- handle_domain_irq(davinci_cp_intc_irq_domain, irqnr, regs);
+ generic_handle_domain_irq(davinci_cp_intc_irq_domain, irqnr);
}
static int davinci_cp_intc_host_map(struct irq_domain *h, unsigned int virq,
diff --git a/drivers/irqchip/irq-digicolor.c b/drivers/irqchip/irq-digicolor.c
index fc38d2da11b9..3b0d78aac13b 100644
--- a/drivers/irqchip/irq-digicolor.c
+++ b/drivers/irqchip/irq-digicolor.c
@@ -50,7 +50,7 @@ static void __exception_irq_entry digicolor_handle_irq(struct pt_regs *regs)
return;
}
- handle_domain_irq(digicolor_irq_domain, hwirq, regs);
+ generic_handle_domain_irq(digicolor_irq_domain, hwirq);
} while (1);
}
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index a67266e44491..d5c1c750c8d2 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -42,7 +42,7 @@ static void __irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs)
while (stat) {
u32 hwirq = ffs(stat) - 1;
- handle_domain_irq(d, hwirq, regs);
+ generic_handle_domain_irq(d, hwirq);
stat &= ~BIT(hwirq);
}
}
diff --git a/drivers/irqchip/irq-ftintc010.c b/drivers/irqchip/irq-ftintc010.c
index 0bf98425dca5..5cc268880f8e 100644
--- a/drivers/irqchip/irq-ftintc010.c
+++ b/drivers/irqchip/irq-ftintc010.c
@@ -134,7 +134,7 @@ asmlinkage void __exception_irq_entry ft010_irqchip_handle_irq(struct pt_regs *r
while ((status = readl(FT010_IRQ_STATUS(f->base)))) {
irq = ffs(status) - 1;
- handle_domain_irq(f->domain, irq, regs);
+ generic_handle_domain_irq(f->domain, irq);
}
}
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index fd4e9a37fea6..daec3309b014 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -660,7 +660,7 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
* PSR.I will be restored when we ERET to the
* interrupted context.
*/
- err = handle_domain_nmi(gic_data.domain, irqnr, regs);
+ err = generic_handle_domain_nmi(gic_data.domain, irqnr);
if (err)
gic_deactivate_unhandled(irqnr);
@@ -728,7 +728,7 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
else
isb();
- if (handle_domain_irq(gic_data.domain, irqnr, regs)) {
+ if (generic_handle_domain_irq(gic_data.domain, irqnr)) {
WARN_ONCE(true, "Unexpected interrupt received!\n");
gic_deactivate_unhandled(irqnr);
}
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 5f22c9d65e57..b8bb46c65a97 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -369,7 +369,7 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
this_cpu_write(sgi_intid, irqstat);
}
- handle_domain_irq(gic->domain, irqnr, regs);
+ generic_handle_domain_irq(gic->domain, irqnr);
} while (1);
}
diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
index 058ebaebe2c4..46161f6ff289 100644
--- a/drivers/irqchip/irq-hip04.c
+++ b/drivers/irqchip/irq-hip04.c
@@ -206,7 +206,7 @@ static void __exception_irq_entry hip04_handle_irq(struct pt_regs *regs)
irqnr = irqstat & GICC_IAR_INT_ID_MASK;
if (irqnr <= HIP04_MAX_IRQS)
- handle_domain_irq(hip04_data.domain, irqnr, regs);
+ generic_handle_domain_irq(hip04_data.domain, irqnr);
} while (irqnr > HIP04_MAX_IRQS);
}
diff --git a/drivers/irqchip/irq-ixp4xx.c b/drivers/irqchip/irq-ixp4xx.c
index 37e0749215c7..fb68f8c59fbb 100644
--- a/drivers/irqchip/irq-ixp4xx.c
+++ b/drivers/irqchip/irq-ixp4xx.c
@@ -114,7 +114,7 @@ asmlinkage void __exception_irq_entry ixp4xx_handle_irq(struct pt_regs *regs)
status = __raw_readl(ixi->irqbase + IXP4XX_ICIP);
for_each_set_bit(i, &status, 32)
- handle_domain_irq(ixi->domain, i, regs);
+ generic_handle_domain_irq(ixi->domain, i);
/*
* IXP465/IXP435 has an upper IRQ status register
@@ -122,7 +122,7 @@ asmlinkage void __exception_irq_entry ixp4xx_handle_irq(struct pt_regs *regs)
if (ixi->is_356) {
status = __raw_readl(ixi->irqbase + IXP4XX_ICIP2);
for_each_set_bit(i, &status, 32)
- handle_domain_irq(ixi->domain, i + 32, regs);
+ generic_handle_domain_irq(ixi->domain, i + 32);
}
}
diff --git a/drivers/irqchip/irq-lpc32xx.c b/drivers/irqchip/irq-lpc32xx.c
index 5e6f6e25f2ae..a29357f39450 100644
--- a/drivers/irqchip/irq-lpc32xx.c
+++ b/drivers/irqchip/irq-lpc32xx.c
@@ -126,7 +126,7 @@ static void __exception_irq_entry lpc32xx_handle_irq(struct pt_regs *regs)
while (hwirq) {
irq = __ffs(hwirq);
hwirq &= ~BIT(irq);
- handle_domain_irq(lpc32xx_mic_irqc->domain, irq, regs);
+ generic_handle_domain_irq(lpc32xx_mic_irqc->domain, irq);
}
}
diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
index 4a74ac7b7c42..83455ca72439 100644
--- a/drivers/irqchip/irq-mmp.c
+++ b/drivers/irqchip/irq-mmp.c
@@ -230,7 +230,7 @@ static void __exception_irq_entry mmp_handle_irq(struct pt_regs *regs)
if (!(hwirq & SEL_INT_PENDING))
return;
hwirq &= SEL_INT_NUM_MASK;
- handle_domain_irq(icu_data[0].domain, hwirq, regs);
+ generic_handle_domain_irq(icu_data[0].domain, hwirq);
}
static void __exception_irq_entry mmp2_handle_irq(struct pt_regs *regs)
@@ -241,7 +241,7 @@ static void __exception_irq_entry mmp2_handle_irq(struct pt_regs *regs)
if (!(hwirq & SEL_INT_PENDING))
return;
hwirq &= SEL_INT_NUM_MASK;
- handle_domain_irq(icu_data[0].domain, hwirq, regs);
+ generic_handle_domain_irq(icu_data[0].domain, hwirq);
}
/* MMP (ARMv5) */
diff --git a/drivers/irqchip/irq-mxs.c b/drivers/irqchip/irq-mxs.c
index d1f5740cd575..55cb6b5a686e 100644
--- a/drivers/irqchip/irq-mxs.c
+++ b/drivers/irqchip/irq-mxs.c
@@ -136,7 +136,7 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
irqnr = __raw_readl(icoll_priv.stat);
__raw_writel(irqnr, icoll_priv.vector);
- handle_domain_irq(icoll_domain, irqnr, regs);
+ generic_handle_domain_irq(icoll_domain, irqnr);
}
static int icoll_irq_domain_map(struct irq_domain *d, unsigned int virq,
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
index ce330880665e..9eaec816d22f 100644
--- a/drivers/irqchip/irq-nvic.c
+++ b/drivers/irqchip/irq-nvic.c
@@ -37,9 +37,9 @@
static struct irq_domain *nvic_irq_domain;
-static void __nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
+static void __nvic_handle_irq(irq_hw_number_t hwirq)
{
- handle_domain_irq(nvic_irq_domain, hwirq, regs);
+ generic_handle_domain_irq(nvic_irq_domain, hwirq);
}
/*
@@ -53,7 +53,7 @@ static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
irq_enter();
old_regs = set_irq_regs(regs);
- __nvic_handle_irq(hwirq, regs);
+ __nvic_handle_irq(hwirq);
set_irq_regs(old_regs);
irq_exit();
}
diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
index d360a6eddd6d..dc82162ba763 100644
--- a/drivers/irqchip/irq-omap-intc.c
+++ b/drivers/irqchip/irq-omap-intc.c
@@ -357,7 +357,7 @@ omap_intc_handle_irq(struct pt_regs *regs)
}
irqnr &= ACTIVEIRQ_MASK;
- handle_domain_irq(domain, irqnr, regs);
+ generic_handle_domain_irq(domain, irqnr);
}
static int __init intc_of_init(struct device_node *node,
diff --git a/drivers/irqchip/irq-or1k-pic.c b/drivers/irqchip/irq-or1k-pic.c
index 03d2366118dd..49b47e787644 100644
--- a/drivers/irqchip/irq-or1k-pic.c
+++ b/drivers/irqchip/irq-or1k-pic.c
@@ -116,7 +116,7 @@ static void or1k_pic_handle_irq(struct pt_regs *regs)
int irq = -1;
while ((irq = pic_get_irq(irq + 1)) != NO_IRQ)
- handle_domain_irq(root_domain, irq, regs);
+ generic_handle_domain_irq(root_domain, irq);
}
static int or1k_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
index b6868f7b805a..17c2c7a07f10 100644
--- a/drivers/irqchip/irq-orion.c
+++ b/drivers/irqchip/irq-orion.c
@@ -42,8 +42,8 @@ __exception_irq_entry orion_handle_irq(struct pt_regs *regs)
gc->mask_cache;
while (stat) {
u32 hwirq = __fls(stat);
- handle_domain_irq(orion_irq_domain,
- gc->irq_base + hwirq, regs);
+ generic_handle_domain_irq(orion_irq_domain,
+ gc->irq_base + hwirq);
stat &= ~(1 << hwirq);
}
}
diff --git a/drivers/irqchip/irq-rda-intc.c b/drivers/irqchip/irq-rda-intc.c
index 960168303b73..9f0144a73777 100644
--- a/drivers/irqchip/irq-rda-intc.c
+++ b/drivers/irqchip/irq-rda-intc.c
@@ -53,7 +53,7 @@ static void __exception_irq_entry rda_handle_irq(struct pt_regs *regs)
while (stat) {
hwirq = __fls(stat);
- handle_domain_irq(rda_irq_domain, hwirq, regs);
+ generic_handle_domain_irq(rda_irq_domain, hwirq);
stat &= ~BIT(hwirq);
}
}
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 8017f6d32d52..b65bd8878d4f 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -37,7 +37,7 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
break;
#endif
default:
- handle_domain_irq(intc_domain, cause, regs);
+ generic_handle_domain_irq(intc_domain, cause);
break;
}
}
diff --git a/drivers/irqchip/irq-sa11x0.c b/drivers/irqchip/irq-sa11x0.c
index dbccc7dafbf8..31c202a1ae62 100644
--- a/drivers/irqchip/irq-sa11x0.c
+++ b/drivers/irqchip/irq-sa11x0.c
@@ -140,8 +140,8 @@ sa1100_handle_irq(struct pt_regs *regs)
if (mask == 0)
break;
- handle_domain_irq(sa1100_normal_irqdomain,
- ffs(mask) - 1, regs);
+ generic_handle_domain_irq(sa1100_normal_irqdomain,
+ ffs(mask) - 1);
} while (1);
}
diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c
index 8a315d6a3399..dd506ebfdacb 100644
--- a/drivers/irqchip/irq-sun4i.c
+++ b/drivers/irqchip/irq-sun4i.c
@@ -195,7 +195,7 @@ static void __exception_irq_entry sun4i_handle_irq(struct pt_regs *regs)
return;
do {
- handle_domain_irq(irq_ic_data->irq_domain, hwirq, regs);
+ generic_handle_domain_irq(irq_ic_data->irq_domain, hwirq);
hwirq = readl(irq_ic_data->irq_base +
SUN4I_IRQ_VECTOR_REG) >> 2;
} while (hwirq != 0);
diff --git a/drivers/irqchip/irq-versatile-fpga.c b/drivers/irqchip/irq-versatile-fpga.c
index 75be350cf82f..f2757b6aecc8 100644
--- a/drivers/irqchip/irq-versatile-fpga.c
+++ b/drivers/irqchip/irq-versatile-fpga.c
@@ -105,7 +105,7 @@ static int handle_one_fpga(struct fpga_irq_data *f, struct pt_regs *regs)
while ((status = readl(f->base + IRQ_STATUS))) {
irq = ffs(status) - 1;
- handle_domain_irq(f->domain, irq, regs);
+ generic_handle_domain_irq(f->domain, irq);
handled = 1;
}
diff --git a/drivers/irqchip/irq-vic.c b/drivers/irqchip/irq-vic.c
index 1e1f2d115257..9e3d5561e04e 100644
--- a/drivers/irqchip/irq-vic.c
+++ b/drivers/irqchip/irq-vic.c
@@ -208,7 +208,7 @@ static int handle_one_vic(struct vic_device *vic, struct pt_regs *regs)
while ((stat = readl_relaxed(vic->base + VIC_IRQ_STATUS))) {
irq = ffs(stat) - 1;
- handle_domain_irq(vic->domain, irq, regs);
+ generic_handle_domain_irq(vic->domain, irq);
handled = 1;
}
diff --git a/drivers/irqchip/irq-vt8500.c b/drivers/irqchip/irq-vt8500.c
index 5bce936af5d9..e17dd3a8c2d5 100644
--- a/drivers/irqchip/irq-vt8500.c
+++ b/drivers/irqchip/irq-vt8500.c
@@ -183,7 +183,7 @@ static void __exception_irq_entry vt8500_handle_irq(struct pt_regs *regs)
continue;
}
- handle_domain_irq(intc[i].domain, irqnr, regs);
+ generic_handle_domain_irq(intc[i].domain, irqnr);
}
}
diff --git a/drivers/irqchip/irq-wpcm450-aic.c b/drivers/irqchip/irq-wpcm450-aic.c
index f3ac392d5bc8..0dcbeb1a05a1 100644
--- a/drivers/irqchip/irq-wpcm450-aic.c
+++ b/drivers/irqchip/irq-wpcm450-aic.c
@@ -69,7 +69,7 @@ static void __exception_irq_entry wpcm450_aic_handle_irq(struct pt_regs *regs)
/* Read IPER to signal that nIRQ can be de-asserted */
hwirq = readl(aic->regs + AIC_IPER) / 4;
- handle_domain_irq(aic->domain, hwirq, regs);
+ generic_handle_domain_irq(aic->domain, hwirq);
}
static void wpcm450_aic_eoi(struct irq_data *d)
diff --git a/drivers/irqchip/irq-zevio.c b/drivers/irqchip/irq-zevio.c
index 84163f1ebfcf..7a72620fc478 100644
--- a/drivers/irqchip/irq-zevio.c
+++ b/drivers/irqchip/irq-zevio.c
@@ -50,7 +50,7 @@ static void __exception_irq_entry zevio_handle_irq(struct pt_regs *regs)
while (readl(zevio_irq_io + IO_STATUS)) {
irqnr = readl(zevio_irq_io + IO_CURRENT);
- handle_domain_irq(zevio_irq_domain, irqnr, regs);
+ generic_handle_domain_irq(zevio_irq_domain, irqnr);
}
}
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 59aea39785bf..93d270ca0c56 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -168,14 +168,7 @@ int generic_handle_irq(unsigned int irq);
* conversion failed.
*/
int generic_handle_domain_irq(struct irq_domain *domain, unsigned int hwirq);
-
-#ifdef CONFIG_HANDLE_DOMAIN_IRQ
-int handle_domain_irq(struct irq_domain *domain,
- unsigned int hwirq, struct pt_regs *regs);
-
-int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
- struct pt_regs *regs);
-#endif
+int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq);
#endif
/* Test to see if a driver has successfully requested an irq */
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 897dfc552bb0..1b41078222f3 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -97,13 +97,6 @@ config GENERIC_MSI_IRQ_DOMAIN
config IRQ_MSI_IOMMU
bool
-config HANDLE_DOMAIN_IRQ
- bool
-
-# Legacy behaviour; architectures should call irq_{enter,exit}() themselves
-config HANDLE_DOMAIN_IRQ_IRQENTRY
- bool
-
config IRQ_TIMINGS
bool
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index acfb163b9f36..83d5001c0091 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -652,7 +652,11 @@ EXPORT_SYMBOL_GPL(handle_irq_desc);
* generic_handle_irq - Invoke the handler for a particular irq
* @irq: The irq number to handle
*
- */
+ * Returns: 0 on success, or -EINVAL if conversion has failed
+ *
+ * This function must be called from an IRQ context with irq regs
+ * initialized.
+ */
int generic_handle_irq(unsigned int irq)
{
return handle_irq_desc(irq_to_desc(irq));
@@ -662,76 +666,39 @@ EXPORT_SYMBOL_GPL(generic_handle_irq);
#ifdef CONFIG_IRQ_DOMAIN
/**
* generic_handle_domain_irq - Invoke the handler for a HW irq belonging
- * to a domain, usually for a non-root interrupt
- * controller
+ * to a domain.
* @domain: The domain where to perform the lookup
* @hwirq: The HW irq number to convert to a logical one
*
* Returns: 0 on success, or -EINVAL if conversion has failed
*
+ * This function must be called from an IRQ context with irq regs
+ * initialized.
*/
int generic_handle_domain_irq(struct irq_domain *domain, unsigned int hwirq)
{
+ WARN_ON_ONCE(!in_irq());
return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
}
EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
-#ifdef CONFIG_HANDLE_DOMAIN_IRQ
/**
- * handle_domain_irq - Invoke the handler for a HW irq belonging to a domain,
- * usually for a root interrupt controller
+ * generic_handle_domain_nmi - Invoke the handler for a HW nmi belonging
+ * to a domain.
* @domain: The domain where to perform the lookup
* @hwirq: The HW irq number to convert to a logical one
- * @regs: Register file coming from the low-level handling code
- *
- * This function must be called from an IRQ context.
*
* Returns: 0 on success, or -EINVAL if conversion has failed
- */
-int handle_domain_irq(struct irq_domain *domain,
- unsigned int hwirq, struct pt_regs *regs)
-{
- struct pt_regs *old_regs = set_irq_regs(regs);
- int ret;
-
- /*
- * IRQ context needs to be setup earlier.
- */
- WARN_ON(!in_irq());
-
- ret = generic_handle_domain_irq(domain, hwirq);
-
- set_irq_regs(old_regs);
- return ret;
-}
-
-/**
- * handle_domain_nmi - Invoke the handler for a HW irq belonging to a domain
- * @domain: The domain where to perform the lookup
- * @hwirq: The HW irq number to convert to a logical one
- * @regs: Register file coming from the low-level handling code
- *
- * This function must be called from an NMI context.
*
- * Returns: 0 on success, or -EINVAL if conversion has failed
- */
-int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
- struct pt_regs *regs)
+ * This function must be called from an NMI context with irq regs
+ * initialized.
+ **/
+int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
{
- struct pt_regs *old_regs = set_irq_regs(regs);
- int ret;
-
- /*
- * NMI context needs to be setup earlier in order to deal with tracing.
- */
- WARN_ON(!in_nmi());
-
- ret = generic_handle_domain_irq(domain, hwirq);
-
- set_irq_regs(old_regs);
- return ret;
+ WARN_ON_ONCE(!in_nmi());
+ return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
}
-#endif
+EXPORT_SYMBOL_GPL(generic_handle_domain_nmi);
#endif
/* Dynamic interrupt handling */
--
2.11.0
On Thu, Oct 21, 2021 at 8:02 AM Mark Rutland <[email protected]> wrote:
>
> This series reworks the irq code to remove them, handling the necessary
> entry work consistently in entry code (be it architectural or generic).
Thanks for doing this.
This is obviously a much bigger patch than the original, but this
really feels like it's the proper fix, and maintainable going
forwards.
Obviously the big issue is testing this all, but I like the patches
from scanning them.
Linus
On Thu, Oct 21, 2021 at 07:02:31PM +0100, Mark Rutland wrote:
> In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/arm64
> perform all the irqentry accounting in its entry code.
>
> As arch/arm64 already performs portions of the irqentry logic in
> enter_from_kernel_mode() and exit_to_kernel_mode(), including
> rcu_irq_{enter,exit}(), the only additional calls that need to be made
> are to irq_{enter,exit}_rcu(). Removing the calls to
> rcu_irq_{enter,exit}() from handle_domain_irq() ensures that we inform
> RCU once per IRQ entry and will correctly identify quiescent periods.
>
> Since we should not call irq_{enter,exit}_rcu() when entering a
> pseudo-NMI, el1_interrupt() is reworked to have separate __el1_irq() and
> __el1_pnmi() paths for regular IRQ and psuedo-NMI entry, with
> irq_{enter,exit}_irq() only called for the former.
>
> In preparation for removing HANDLE_DOMAIN_IRQ, the irq regs are managed
> in do_interrupt_handler() for both regular IRQ and pseudo-NMI. This is
> currently redundant, but not harmful.
>
> For clarity the preemption logic is moved into __el1_irq(). We should
> never preempt within a pseudo-NMI, and arm64_enter_nmi() already
> enforces this by incrementing the preempt_count, but it's clearer if we
> never invoke the preemption logic when entering a pseudo-NMI.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> arch/arm64/Kconfig | 1 -
> arch/arm64/kernel/entry-common.c | 52 ++++++++++++++++++++++++----------------
> 2 files changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 553239a5a5f7..5c7ae4c3954b 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -134,7 +134,6 @@ config ARM64
> select GENERIC_GETTIMEOFDAY
> select GENERIC_VDSO_TIME_NS
> select HANDLE_DOMAIN_IRQ
> - select HANDLE_DOMAIN_IRQ_IRQENTRY
> select HARDIRQS_SW_RESEND
> select HAVE_MOVE_PMD
> select HAVE_MOVE_PUD
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index 32f9796c4ffe..f7408edf8571 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -17,6 +17,7 @@
> #include <asm/daifflags.h>
> #include <asm/esr.h>
> #include <asm/exception.h>
> +#include <asm/irq_regs.h>
> #include <asm/kprobes.h>
> #include <asm/mmu.h>
> #include <asm/processor.h>
> @@ -219,22 +220,6 @@ static void noinstr arm64_exit_el1_dbg(struct pt_regs *regs)
> lockdep_hardirqs_on(CALLER_ADDR0);
> }
>
> -static void noinstr enter_el1_irq_or_nmi(struct pt_regs *regs)
> -{
> - if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
> - arm64_enter_nmi(regs);
> - else
> - enter_from_kernel_mode(regs);
> -}
> -
> -static void noinstr exit_el1_irq_or_nmi(struct pt_regs *regs)
> -{
> - if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
> - arm64_exit_nmi(regs);
> - else
> - exit_to_kernel_mode(regs);
> -}
> -
> static void __sched arm64_preempt_schedule_irq(void)
> {
> lockdep_assert_irqs_disabled();
> @@ -263,10 +248,14 @@ static void __sched arm64_preempt_schedule_irq(void)
> static void do_interrupt_handler(struct pt_regs *regs,
> void (*handler)(struct pt_regs *))
> {
> + struct pt_regs *old_regs = set_irq_regs(regs);
> +
> if (on_thread_stack())
> call_on_irq_stack(regs, handler);
> else
> handler(regs);
> +
> + set_irq_regs(old_regs);
> }
>
> extern void (*handle_arch_irq)(struct pt_regs *);
> @@ -432,13 +421,22 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
> }
> }
>
> -static void noinstr el1_interrupt(struct pt_regs *regs,
> - void (*handler)(struct pt_regs *))
> +static __always_inline void __el1_pnmi(struct pt_regs *regs,
> + void (*handler)(struct pt_regs *))
> {
> - write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
> + arm64_enter_nmi(regs);
> + do_interrupt_handler(regs, handler);
> + arm64_exit_nmi(regs);
> +}
> +
> +static __always_inline void __el1_irq(struct pt_regs *regs,
> + void (*handler)(struct pt_regs *))
> +{
> + enter_from_kernel_mode(regs);
>
> - enter_el1_irq_or_nmi(regs);
> + irq_enter_rcu();
> do_interrupt_handler(regs, handler);
> + irq_exit_rcu();
>
> /*
> * Note: thread_info::preempt_count includes both thread_info::count
> @@ -449,7 +447,17 @@ static void noinstr el1_interrupt(struct pt_regs *regs,
> READ_ONCE(current_thread_info()->preempt_count) == 0)
> arm64_preempt_schedule_irq();
>
> - exit_el1_irq_or_nmi(regs);
> + exit_to_kernel_mode(regs);
> +}
> +static void noinstr el1_interrupt(struct pt_regs *regs,
> + void (*handler)(struct pt_regs *))
> +{
> + write_sysreg(DAIF_PROCCTX_NOIRQ, daif);
> +
> + if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs))
> + __el1_pnmi(regs, handler);
> + else
> + __el1_irq(regs, handler);
> }
>
> asmlinkage void noinstr el1h_64_irq_handler(struct pt_regs *regs)
> @@ -667,7 +675,9 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
> if (regs->pc & BIT(55))
> arm64_apply_bp_hardening();
>
> + irq_enter_rcu();
> do_interrupt_handler(regs, handler);
> + irq_exit_rcu();
>
> exit_to_user_mode(regs);
> }
Reviewed-by: Pingfan Liu <[email protected]>
Reviewed-by: Guo Ren <[email protected]>
On Fri, Oct 22, 2021 at 2:03 AM Mark Rutland <[email protected]> wrote:
>
> In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/riscv
> perform all the irqentry accounting in its entry code. As arch/riscv
> uses GENERIC_IRQ_MULTI_HANDLER, we can use generic_handle_arch_irq() to
> do so.
>
> Since generic_handle_arch_irq() handles the irq entry and setting the
> irq regs, and happens before the irqchip code calls handle_IPI(), we can
> remove the redundant irq entry and irq regs manipulation from
> handle_IPI().
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Albert Ou <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Paul Walmsley <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> arch/riscv/Kconfig | 1 -
> arch/riscv/kernel/entry.S | 3 +--
> arch/riscv/kernel/smp.c | 9 +--------
> 3 files changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 740653063a56..301a54233c7e 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -63,7 +63,6 @@ config RISCV
> select GENERIC_SMP_IDLE_THREAD
> select GENERIC_TIME_VSYSCALL if MMU && 64BIT
> select HANDLE_DOMAIN_IRQ
> - select HANDLE_DOMAIN_IRQ_IRQENTRY
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
> select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 98f502654edd..64236f7efde5 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -130,8 +130,7 @@ skip_context_tracking:
>
> /* Handle interrupts */
> move a0, sp /* pt_regs */
> - la a1, handle_arch_irq
> - REG_L a1, (a1)
> + la a1, generic_handle_arch_irq
> jr a1
> 1:
> /*
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 921d9d7df400..2f6da845c9ae 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -140,12 +140,9 @@ void arch_irq_work_raise(void)
>
> void handle_IPI(struct pt_regs *regs)
> {
> - struct pt_regs *old_regs = set_irq_regs(regs);
> unsigned long *pending_ipis = &ipi_data[smp_processor_id()].bits;
> unsigned long *stats = ipi_data[smp_processor_id()].stats;
>
> - irq_enter();
> -
> riscv_clear_ipi();
>
> while (true) {
> @@ -156,7 +153,7 @@ void handle_IPI(struct pt_regs *regs)
>
> ops = xchg(pending_ipis, 0);
> if (ops == 0)
> - goto done;
> + return;
>
> if (ops & (1 << IPI_RESCHEDULE)) {
> stats[IPI_RESCHEDULE]++;
> @@ -189,10 +186,6 @@ void handle_IPI(struct pt_regs *regs)
> /* Order data access and bit testing. */
> mb();
> }
> -
> -done:
> - irq_exit();
> - set_irq_regs(old_regs);
> }
>
> static const char * const ipi_names[] = {
> --
> 2.11.0
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
On Thu, Oct 21, 2021 at 07:02:26PM +0100, Mark Rutland wrote:
> Several architectures select GENERIC_IRQ_MULTI_HANDLER and branch to
> handle_arch_irq() without performing any entry accounting.
>
> Add a generic wrapper to handle the commoon irqentry work when invoking
> handle_arch_irq(). Where an architecture needs to perform some entry
> accounting itself, it will need to invoke handle_arch_irq() itself.
>
> In subsequent patches it will become the responsibilty of the entry code
> to set the irq regs when entering an IRQ (rather than deferring this to
> an irqchip handler), so generic_handle_arch_irq() is made to set the irq
> regs now. This can be redundant in some cases, but is never harmful as
> saving/restoring the old regs nests safely.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> kernel/irq/handle.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
> index 221d80c31e94..27182003b879 100644
> --- a/kernel/irq/handle.c
> +++ b/kernel/irq/handle.c
> @@ -14,6 +14,8 @@
> #include <linux/interrupt.h>
> #include <linux/kernel_stat.h>
>
> +#include <asm/irq_regs.h>
> +
> #include <trace/events/irq.h>
>
> #include "internals.h"
> @@ -226,4 +228,20 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
> handle_arch_irq = handle_irq;
> return 0;
> }
> +
> +/**
> + * generic_handle_arch_irq - root irq handler for architectures which do no
> + * entry accounting themselves
> + * @regs: Register file coming from the low-level handling code
> + */
> +asmlinkage void noinstr generic_handle_arch_irq(struct pt_regs *regs)
> +{
> + struct pt_regs *old_regs;
> +
> + irq_enter();
> + old_regs = set_irq_regs(regs);
> + handle_arch_irq(regs);
After all involved arches call generic_handle_arch_irq(), can
handle_arch_irq be encapsulated by declaring as static?
Two places need to be fixed for this purpose:
-1. absorb the logic about handle_arch_irq in arm64/kernel/irq.c
-2. In arm, setup_arch(),
#ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
handle_arch_irq = mdesc->handle_irq;
#endif
Thanks,
Pingfan
On Fri, Oct 22, 2021 at 2:03 AM Mark Rutland <[email protected]> wrote:
>
> In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/csky
> perform all the irqentry accounting in its entry code. As arch/csky uses
> GENERIC_IRQ_MULTI_HANDLER, we can use generic_handle_arch_irq() to do
> so.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Guo Ren <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> arch/csky/Kconfig | 1 -
> arch/csky/kernel/entry.S | 2 +-
> arch/csky/kernel/irq.c | 5 -----
> 3 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
> index 45f03f674a61..9d4d898df76b 100644
> --- a/arch/csky/Kconfig
> +++ b/arch/csky/Kconfig
> @@ -18,7 +18,6 @@ config CSKY
> select DMA_DIRECT_REMAP
> select IRQ_DOMAIN
> select HANDLE_DOMAIN_IRQ
> - select HANDLE_DOMAIN_IRQ_IRQENTRY
> select DW_APB_TIMER_OF
> select GENERIC_IOREMAP
> select GENERIC_LIB_ASHLDI3
> diff --git a/arch/csky/kernel/entry.S b/arch/csky/kernel/entry.S
> index 00e3c8ebf9b8..a4ababf25e24 100644
> --- a/arch/csky/kernel/entry.S
> +++ b/arch/csky/kernel/entry.S
> @@ -249,7 +249,7 @@ ENTRY(csky_irq)
>
>
> mov a0, sp
> - jbsr csky_do_IRQ
> + jbsr generic_handle_arch_irq
>
> jmpi ret_from_exception
>
> diff --git a/arch/csky/kernel/irq.c b/arch/csky/kernel/irq.c
> index 03a1930f1cbb..fcdaf3156286 100644
> --- a/arch/csky/kernel/irq.c
> +++ b/arch/csky/kernel/irq.c
> @@ -15,8 +15,3 @@ void __init init_IRQ(void)
> setup_smp_ipi();
> #endif
> }
> -
> -asmlinkage void __irq_entry csky_do_IRQ(struct pt_regs *regs)
> -{
> - handle_arch_irq(regs);
> -}
Seems the previous code has lost old_regs save/restore?
+ struct pt_regs *old_regs;
+
+ irq_enter();
+ old_regs = set_irq_regs(regs);
+ handle_arch_irq(regs);
+ set_irq_regs(old_regs);
+ irq_exit();
> --
> 2.11.0
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
On Fri, Oct 22, 2021 at 10:19 AM Guo Ren <[email protected]> wrote:
>
> On Fri, Oct 22, 2021 at 2:03 AM Mark Rutland <[email protected]> wrote:
> >
> > In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/csky
> > perform all the irqentry accounting in its entry code. As arch/csky uses
> > GENERIC_IRQ_MULTI_HANDLER, we can use generic_handle_arch_irq() to do
> > so.
> >
> > There should be no functional change as a result of this patch.
> >
> > Signed-off-by: Mark Rutland <[email protected]>
> > Cc: Guo Ren <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > ---
> > arch/csky/Kconfig | 1 -
> > arch/csky/kernel/entry.S | 2 +-
> > arch/csky/kernel/irq.c | 5 -----
> > 3 files changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
> > index 45f03f674a61..9d4d898df76b 100644
> > --- a/arch/csky/Kconfig
> > +++ b/arch/csky/Kconfig
> > @@ -18,7 +18,6 @@ config CSKY
> > select DMA_DIRECT_REMAP
> > select IRQ_DOMAIN
> > select HANDLE_DOMAIN_IRQ
> > - select HANDLE_DOMAIN_IRQ_IRQENTRY
> > select DW_APB_TIMER_OF
> > select GENERIC_IOREMAP
> > select GENERIC_LIB_ASHLDI3
> > diff --git a/arch/csky/kernel/entry.S b/arch/csky/kernel/entry.S
> > index 00e3c8ebf9b8..a4ababf25e24 100644
> > --- a/arch/csky/kernel/entry.S
> > +++ b/arch/csky/kernel/entry.S
> > @@ -249,7 +249,7 @@ ENTRY(csky_irq)
> >
> >
> > mov a0, sp
> > - jbsr csky_do_IRQ
> > + jbsr generic_handle_arch_irq
> >
> > jmpi ret_from_exception
> >
> > diff --git a/arch/csky/kernel/irq.c b/arch/csky/kernel/irq.c
> > index 03a1930f1cbb..fcdaf3156286 100644
> > --- a/arch/csky/kernel/irq.c
> > +++ b/arch/csky/kernel/irq.c
> > @@ -15,8 +15,3 @@ void __init init_IRQ(void)
> > setup_smp_ipi();
> > #endif
> > }
> > -
> > -asmlinkage void __irq_entry csky_do_IRQ(struct pt_regs *regs)
> > -{
> > - handle_arch_irq(regs);
> > -}
>
> Seems the previous code has lost old_regs save/restore?
>
> + struct pt_regs *old_regs;
> +
> + irq_enter();
> + old_regs = set_irq_regs(regs);
> + handle_arch_irq(regs);
> + set_irq_regs(old_regs);
> + irq_exit();
Sorry, handle_domain_irq has done it, and C-SKY's IPI is in the
handle_domain_irq.
Reviewed-by: Guo Ren <[email protected]>
>
> > --
> > 2.11.0
> >
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
On Fri, Oct 22, 2021 at 2:03 AM Mark Rutland <[email protected]> wrote:
>
> Several architectures select GENERIC_IRQ_MULTI_HANDLER and branch to
> handle_arch_irq() without performing any entry accounting.
>
> Add a generic wrapper to handle the commoon irqentry work when invoking
> handle_arch_irq(). Where an architecture needs to perform some entry
> accounting itself, it will need to invoke handle_arch_irq() itself.
>
> In subsequent patches it will become the responsibilty of the entry code
> to set the irq regs when entering an IRQ (rather than deferring this to
> an irqchip handler), so generic_handle_arch_irq() is made to set the irq
> regs now. This can be redundant in some cases, but is never harmful as
> saving/restoring the old regs nests safely.
Shall we remove old_regs save/restore in handle_domain_irq? It's duplicated.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> kernel/irq/handle.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
> index 221d80c31e94..27182003b879 100644
> --- a/kernel/irq/handle.c
> +++ b/kernel/irq/handle.c
> @@ -14,6 +14,8 @@
> #include <linux/interrupt.h>
> #include <linux/kernel_stat.h>
>
> +#include <asm/irq_regs.h>
> +
> #include <trace/events/irq.h>
>
> #include "internals.h"
> @@ -226,4 +228,20 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
> handle_arch_irq = handle_irq;
> return 0;
> }
> +
> +/**
> + * generic_handle_arch_irq - root irq handler for architectures which do no
> + * entry accounting themselves
> + * @regs: Register file coming from the low-level handling code
> + */
> +asmlinkage void noinstr generic_handle_arch_irq(struct pt_regs *regs)
> +{
> + struct pt_regs *old_regs;
> +
> + irq_enter();
> + old_regs = set_irq_regs(regs);
> + handle_arch_irq(regs);
> + set_irq_regs(old_regs);
> + irq_exit();
> +}
> #endif
> --
> 2.11.0
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
Mark Rutland <[email protected]> 於 2021年10月22日 週五 上午2:03寫道：
>
> In preparation for removing HANDLE_DOMAIN_IRQ, have arch/nds32 perform
> all the necessary IRQ entry accounting in its entry code.
>
> Currently arch/nds32 is tightly coupled with the ativic32 irqchip, and
> while the entry code should logically live under arch/nds32/, moving the
> entry logic there makes things more convoluted. So for now, place the
> entry logic in the ativic32 irqchip, but separated into a separate
> function to make the split of responsibility clear.
>
> In future this should probably use GENERIC_IRQ_MULTI_HANDLER to cleanly
> decouple this.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Greentime Hu <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Nick Hu <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Vincent Chen <[email protected]>
> ---
> arch/nds32/Kconfig | 1 -
> drivers/irqchip/irq-ativic32.c | 22 ++++++++++++++++++++--
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
> index aea26e739543..4d1421b18734 100644
> --- a/arch/nds32/Kconfig
> +++ b/arch/nds32/Kconfig
> @@ -27,7 +27,6 @@ config NDS32
> select GENERIC_LIB_MULDI3
> select GENERIC_LIB_UCMPDI2
> select GENERIC_TIME_VSYSCALL
> - select HANDLE_DOMAIN_IRQ
> select HAVE_ARCH_TRACEHOOK
> select HAVE_DEBUG_KMEMLEAK
> select HAVE_EXIT_THREAD
> diff --git a/drivers/irqchip/irq-ativic32.c b/drivers/irqchip/irq-ativic32.c
> index 476d6024aaf2..223dd2f97d28 100644
> --- a/drivers/irqchip/irq-ativic32.c
> +++ b/drivers/irqchip/irq-ativic32.c
> @@ -5,11 +5,14 @@
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <linux/of_address.h>
> +#include <linux/hardirq.h>
> #include <linux/interrupt.h>
> #include <linux/irqdomain.h>
> #include <linux/irqchip.h>
> #include <nds32_intrinsic.h>
>
> +#include <asm/irq_regs.h>
> +
> unsigned long wake_mask;
>
> static void ativic32_ack_irq(struct irq_data *data)
> @@ -103,10 +106,25 @@ static irq_hw_number_t get_intr_src(void)
> - NDS32_VECTOR_offINTERRUPT;
> }
>
> -asmlinkage void asm_do_IRQ(struct pt_regs *regs)
> +static void ativic32_handle_irq(struct pt_regs *regs)
> {
> irq_hw_number_t hwirq = get_intr_src();
> - handle_domain_irq(root_domain, hwirq, regs);
> + generic_handle_domain_irq(root_domain, hwirq);
> +}
> +
> +/*
> + * TODO: convert nds32 to GENERIC_IRQ_MULTI_HANDLER so that this entry logic
> + * can live in arch code.
> + */
> +asmlinkage void asm_do_IRQ(struct pt_regs *regs)
> +{
> + struct pt_regs *old_regs;
> +
> + irq_enter();
> + old_regs = set_irq_regs(regs);
> + ativic32_handle_irq(regs);
> + set_irq_regs(old_regs);
> + irq_exit();
> }
>
> int __init ativic32_init_irq(struct device_node *node, struct device_node *parent)
> --
> 2.11.0
>
Loop in Alan and KC.
On Fri, Oct 22, 2021 at 10:33:53AM +0800, Guo Ren wrote:
> On Fri, Oct 22, 2021 at 2:03 AM Mark Rutland <[email protected]> wrote:
> >
> > Several architectures select GENERIC_IRQ_MULTI_HANDLER and branch to
> > handle_arch_irq() without performing any entry accounting.
> >
> > Add a generic wrapper to handle the commoon irqentry work when invoking
Ah, I'd typo'd 'common' there; I'll go fix that...
> > handle_arch_irq(). Where an architecture needs to perform some entry
> > accounting itself, it will need to invoke handle_arch_irq() itself.
> >
> > In subsequent patches it will become the responsibilty of the entry code
> > to set the irq regs when entering an IRQ (rather than deferring this to
> > an irqchip handler), so generic_handle_arch_irq() is made to set the irq
> > regs now. This can be redundant in some cases, but is never harmful as
> > saving/restoring the old regs nests safely.
> Shall we remove old_regs save/restore in handle_domain_irq? It's duplicated.
We do, in patch 15 when handle_domain_irq() is removed entirely. :)
Removing it immediately in this patch would require more ifdeffery and/or
another copy of handle_domain_irq(), and since the duplication doesn't cause a
functional problem, I think it's better to live with the temporary duplication
for the next few patches than to try to optimize the intermediate steps at the
cost of legibility.
Thanks,
Mark.
> > Signed-off-by: Mark Rutland <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > ---
> > kernel/irq/handle.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
> > index 221d80c31e94..27182003b879 100644
> > --- a/kernel/irq/handle.c
> > +++ b/kernel/irq/handle.c
> > @@ -14,6 +14,8 @@
> > #include <linux/interrupt.h>
> > #include <linux/kernel_stat.h>
> >
> > +#include <asm/irq_regs.h>
> > +
> > #include <trace/events/irq.h>
> >
> > #include "internals.h"
> > @@ -226,4 +228,20 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
> > handle_arch_irq = handle_irq;
> > return 0;
> > }
> > +
> > +/**
> > + * generic_handle_arch_irq - root irq handler for architectures which do no
> > + * entry accounting themselves
> > + * @regs: Register file coming from the low-level handling code
> > + */
> > +asmlinkage void noinstr generic_handle_arch_irq(struct pt_regs *regs)
> > +{
> > + struct pt_regs *old_regs;
> > +
> > + irq_enter();
> > + old_regs = set_irq_regs(regs);
> > + handle_arch_irq(regs);
> > + set_irq_regs(old_regs);
> > + irq_exit();
> > +}
> > #endif
> > --
> > 2.11.0
> >
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
On Fri, Oct 22, 2021 at 10:10:26AM +0800, Pingfan Liu wrote:
> On Thu, Oct 21, 2021 at 07:02:26PM +0100, Mark Rutland wrote:
> > Several architectures select GENERIC_IRQ_MULTI_HANDLER and branch to
> > handle_arch_irq() without performing any entry accounting.
> >
> > Add a generic wrapper to handle the commoon irqentry work when invoking
> > handle_arch_irq(). Where an architecture needs to perform some entry
> > accounting itself, it will need to invoke handle_arch_irq() itself.
> >
> > In subsequent patches it will become the responsibilty of the entry code
> > to set the irq regs when entering an IRQ (rather than deferring this to
> > an irqchip handler), so generic_handle_arch_irq() is made to set the irq
> > regs now. This can be redundant in some cases, but is never harmful as
> > saving/restoring the old regs nests safely.
> >
> > Signed-off-by: Mark Rutland <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > ---
> > kernel/irq/handle.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
> > index 221d80c31e94..27182003b879 100644
> > --- a/kernel/irq/handle.c
> > +++ b/kernel/irq/handle.c
> > @@ -14,6 +14,8 @@
> > #include <linux/interrupt.h>
> > #include <linux/kernel_stat.h>
> >
> > +#include <asm/irq_regs.h>
> > +
> > #include <trace/events/irq.h>
> >
> > #include "internals.h"
> > @@ -226,4 +228,20 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
> > handle_arch_irq = handle_irq;
> > return 0;
> > }
> > +
> > +/**
> > + * generic_handle_arch_irq - root irq handler for architectures which do no
> > + * entry accounting themselves
> > + * @regs: Register file coming from the low-level handling code
> > + */
> > +asmlinkage void noinstr generic_handle_arch_irq(struct pt_regs *regs)
> > +{
> > + struct pt_regs *old_regs;
> > +
> > + irq_enter();
> > + old_regs = set_irq_regs(regs);
> > + handle_arch_irq(regs);
>
> After all involved arches call generic_handle_arch_irq(), can
> handle_arch_irq be encapsulated by declaring as static?
>
> Two places need to be fixed for this purpose:
> -1. absorb the logic about handle_arch_irq in arm64/kernel/irq.c
> -2. In arm, setup_arch(),
> #ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
> handle_arch_irq = mdesc->handle_irq;
> #endif
That arm bit would need to be set_handle_irq(mdesc->handle_irq); anywhere it
uses handle_arch_irq it's depending on the CONFIG_GENERIC_IRQ_MULTI_HANDLER
definition.
While I agree it would seem nice to encapsulate this, in future we want
architectures to move to the more correct entry sequencing described in the
cover letter, and when that happens they should invoke handle_arch_irq()
directly, so I think this is best to leave as-is.
We have custom logic on arm64 because we want to handle IRQ and FIQ
consistently, and there wasn't a neat way to bodge that into the generic code,
but that issue doesn't apply to the other users of
CONFIG_GENERIC_IRQ_MULTI_HANDLER.
Thanks,
Mark.
On Thu, 21 Oct 2021 19:02:36 +0100,
Mark Rutland <[email protected]> wrote:
>
> Now that entry code handles IRQ entry (including setting the IRQ regs)
> before calling irqchip code, irqchip code can safely call
> generic_handle_domain_irq(), and there's no functional reason for it to
> call handle_domain_irq().
>
> Let's cement this split of responsibility and remove handle_domain_irq()
> entirely, updating irqchip drivers to call generic_handle_domain_irq().
>
> For consistency, handle_domain_nmi() is similarly removed and replaced
> with a generic_handle_domain_nmi() function which also does not perform
> any entry logic.
>
> Previously handle_domain_{irq,nmi}() had a WARN_ON() which would fire
> when they were called in an inappropriate context. So that we can
> identify similar issues going forward, similar WARN_ON_ONCE() logic is
> added to the generic_handle_*() functions, and comments are updated for
> clarity and consistency.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
[...]
> -/**
> - * handle_domain_nmi - Invoke the handler for a HW irq belonging to a domain
> - * @domain: The domain where to perform the lookup
> - * @hwirq: The HW irq number to convert to a logical one
> - * @regs: Register file coming from the low-level handling code
> - *
> - * This function must be called from an NMI context.
> *
> - * Returns: 0 on success, or -EINVAL if conversion has failed
> - */
> -int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
> - struct pt_regs *regs)
> + * This function must be called from an NMI context with irq regs
> + * initialized.
> + **/
> +int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
> {
> - struct pt_regs *old_regs = set_irq_regs(regs);
> - int ret;
> -
> - /*
> - * NMI context needs to be setup earlier in order to deal with tracing.
> - */
> - WARN_ON(!in_nmi());
> -
> - ret = generic_handle_domain_irq(domain, hwirq);
> -
> - set_irq_regs(old_regs);
> - return ret;
> + WARN_ON_ONCE(!in_nmi());
> + return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
> }
> -#endif
> +EXPORT_SYMBOL_GPL(generic_handle_domain_nmi);
nit: we don't need this export (only a root controller can handle
NMIs), and that's the sort of thing I would really want to avoid
exposing to modules.
M.
--
Without deviation from the norm, progress is not possible.
On Thu, 21 Oct 2021 19:02:25 +0100,
Mark Rutland <[email protected]> wrote:
>
> There's no need for handle_domain_{irq,nmi}() to open-code the NULL
> check performed by handle_irq_desc(), nor the resolution of the desc
> performed by generic_handle_domain_irq().
>
> Use generic_handle_domain_irq() directly, as this is functioanlly
> equivalent and clearer. At the same time, delete the stale comments,
> which are no longer helpful.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> kernel/irq/irqdesc.c | 24 ++++--------------------
> 1 file changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index 4e3c29bb603c..b07d0e1552bc 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -690,17 +690,11 @@ int handle_domain_irq(struct irq_domain *domain,
> unsigned int hwirq, struct pt_regs *regs)
> {
> struct pt_regs *old_regs = set_irq_regs(regs);
> - struct irq_desc *desc;
> - int ret = 0;
> + int ret;
>
> irq_enter();
>
> - /* The irqdomain code provides boundary checks */
> - desc = irq_resolve_mapping(domain, hwirq);
> - if (likely(desc))
> - handle_irq_desc(desc);
> - else
> - ret = -EINVAL;
> + ret = generic_handle_domain_irq(domain, hwirq);
>
> irq_exit();
> set_irq_regs(old_regs);
> @@ -721,24 +715,14 @@ int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
> struct pt_regs *regs)
> {
> struct pt_regs *old_regs = set_irq_regs(regs);
> - struct irq_desc *desc;
> - int ret = 0;
> + int ret;
>
> /*
> * NMI context needs to be setup earlier in order to deal with tracing.
> */
> WARN_ON(!in_nmi());
>
> - desc = irq_resolve_mapping(domain, hwirq);
> -
> - /*
> - * ack_bad_irq is not NMI-safe, just report
> - * an invalid interrupt.
> - */
> - if (likely(desc))
> - handle_irq_desc(desc);
> - else
> - ret = -EINVAL;
> + ret = generic_handle_domain_irq(domain, hwirq);
>
> set_irq_regs(old_regs);
> return ret;
Yup, that's really neat. I somehow missed that when I moved some of
the legacy stuff to be ARM specific.
On a vaguely related note, I think you can drop the EXPORT_SYMBOL_GPL
on handle_irq_desc() now.
M.
--
Without deviation from the norm, progress is not possible.
Hi Mark,
On Thu, 21 Oct 2021 19:02:21 +0100,
Mark Rutland <[email protected]> wrote:
>
> The handle_domain_{irq,nmi}() functions were oringally intended as a
> convenience, but recent rework to entry code across the kernel tree has
> demonstrated that they cause more pain than they're worth and prevent
> architectures from being able to write robust entry code.
>
> This series reworks the irq code to remove them, handling the necessary
> entry work consistently in entry code (be it architectural or generic).
[...]
Thanks for going through the pain of putting this together. The
couple of nits I mentioned notwithstanding:
Reviewed-by: Marc Zyngier <[email protected]>
It'd be good to work out a merging strategy once this has seen a bit
of testing.
M.
--
Without deviation from the norm, progress is not possible.
On Fri, Oct 22, 2021 at 11:52:28AM +0100, Marc Zyngier wrote:
> On Thu, 21 Oct 2021 19:02:25 +0100,
> Mark Rutland <[email protected]> wrote:
> >
> > There's no need for handle_domain_{irq,nmi}() to open-code the NULL
> > check performed by handle_irq_desc(), nor the resolution of the desc
> > performed by generic_handle_domain_irq().
> >
> > Use generic_handle_domain_irq() directly, as this is functioanlly
> > equivalent and clearer. At the same time, delete the stale comments,
> > which are no longer helpful.
> >
> > There should be no functional change as a result of this patch.
> >
> > Signed-off-by: Mark Rutland <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > ---
> > kernel/irq/irqdesc.c | 24 ++++--------------------
> > 1 file changed, 4 insertions(+), 20 deletions(-)
> Yup, that's really neat. I somehow missed that when I moved some of
> the legacy stuff to be ARM specific.
>
> On a vaguely related note, I think you can drop the EXPORT_SYMBOL_GPL
> on handle_irq_desc() now.
Seems so:
[mark@lakrids:~/src/linux]% git grep -w handle_irq_desc
arch/arm/kernel/irq.c: handle_irq_desc(desc);
include/linux/irqdesc.h:int handle_irq_desc(struct irq_desc *desc);
kernel/irq/irqdesc.c:int handle_irq_desc(struct irq_desc *desc)
kernel/irq/irqdesc.c: return handle_irq_desc(irq_to_desc(irq));
kernel/irq/irqdesc.c: return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
I'll add a patch to clean that up.
Thanks,
Mark
On Fri, Oct 22, 2021 at 11:05:29AM +0100, Marc Zyngier wrote:
> On Thu, 21 Oct 2021 19:02:36 +0100,
> Mark Rutland <[email protected]> wrote:
> >
> > Now that entry code handles IRQ entry (including setting the IRQ regs)
> > before calling irqchip code, irqchip code can safely call
> > generic_handle_domain_irq(), and there's no functional reason for it to
> > call handle_domain_irq().
> >
> > Let's cement this split of responsibility and remove handle_domain_irq()
> > entirely, updating irqchip drivers to call generic_handle_domain_irq().
> >
> > For consistency, handle_domain_nmi() is similarly removed and replaced
> > with a generic_handle_domain_nmi() function which also does not perform
> > any entry logic.
> >
> > Previously handle_domain_{irq,nmi}() had a WARN_ON() which would fire
> > when they were called in an inappropriate context. So that we can
> > identify similar issues going forward, similar WARN_ON_ONCE() logic is
> > added to the generic_handle_*() functions, and comments are updated for
> > clarity and consistency.
> >
> > Signed-off-by: Mark Rutland <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
>
> [...]
>
> > -/**
> > - * handle_domain_nmi - Invoke the handler for a HW irq belonging to a domain
> > - * @domain: The domain where to perform the lookup
> > - * @hwirq: The HW irq number to convert to a logical one
> > - * @regs: Register file coming from the low-level handling code
> > - *
> > - * This function must be called from an NMI context.
> > *
> > - * Returns: 0 on success, or -EINVAL if conversion has failed
> > - */
> > -int handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq,
> > - struct pt_regs *regs)
> > + * This function must be called from an NMI context with irq regs
> > + * initialized.
> > + **/
> > +int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
> > {
> > - struct pt_regs *old_regs = set_irq_regs(regs);
> > - int ret;
> > -
> > - /*
> > - * NMI context needs to be setup earlier in order to deal with tracing.
> > - */
> > - WARN_ON(!in_nmi());
> > -
> > - ret = generic_handle_domain_irq(domain, hwirq);
> > -
> > - set_irq_regs(old_regs);
> > - return ret;
> > + WARN_ON_ONCE(!in_nmi());
> > + return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
> > }
> > -#endif
> > +EXPORT_SYMBOL_GPL(generic_handle_domain_nmi);
>
> nit: we don't need this export (only a root controller can handle
> NMIs), and that's the sort of thing I would really want to avoid
> exposing to modules.
Sure, I'll drop that; I'd only added that for symmetry with
generic_handle_domain_irq(), and I don't need it to be exported.
Thanks,
Mark.
On Fri, Oct 22, 2021 at 12:20:31PM +0100, Marc Zyngier wrote:
> Hi Mark,
>
> On Thu, 21 Oct 2021 19:02:21 +0100,
> Mark Rutland <[email protected]> wrote:
> >
> > The handle_domain_{irq,nmi}() functions were oringally intended as a
> > convenience, but recent rework to entry code across the kernel tree has
> > demonstrated that they cause more pain than they're worth and prevent
> > architectures from being able to write robust entry code.
> >
> > This series reworks the irq code to remove them, handling the necessary
> > entry work consistently in entry code (be it architectural or generic).
>
> [...]
>
> Thanks for going through the pain of putting this together. The
> couple of nits I mentioned notwithstanding:
>
> Reviewed-by: Marc Zyngier <[email protected]>
Thanks!
I've pushed out an updated version to my irq/handle-domain-irq branch
on kernel.org:
git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
That has two new patches you suggested:
* irq: mips: simplify bcm6345_l1_irq_handle()
* irq: unexport handle_irq_desc()
... which I did not add your Reviewed-by to in case the commit messages
are garbage or something like that.
> It'd be good to work out a merging strategy once this has seen a bit
> of testing.
Conflict-wise, this merges near perfectly against next-20212022 aside
from a trivial conflict against arch/riscv/Kconfig:
| [mark@lakrids:~/src/linux]% git merge irq/handle-domain-irq
| Auto-merging arch/riscv/kernel/entry.S
| Auto-merging arch/riscv/Kconfig
| CONFLICT (content): Merge conflict in arch/riscv/Kconfig
| Auto-merging arch/nds32/Kconfig
| Auto-merging arch/mips/Kconfig
| Auto-merging arch/csky/Kconfig
| Auto-merging arch/arm64/Kconfig
| Auto-merging arch/arm/mach-s3c/irq-s3c24xx.c
| Auto-merging arch/arm/kernel/entry-armv.S
| Auto-merging arch/arm/Kconfig
| Auto-merging arch/arc/Kconfig
| Automatic merge failed; fix conflicts and then commit the result.
| [mark@lakrids:~/src/linux]% git diff
| diff --cc arch/riscv/Kconfig
| index 77a088d0a7e9,353e28f5f849..000000000000
| --- a/arch/riscv/Kconfig
| +++ b/arch/riscv/Kconfig
| @@@ -62,8 -62,6 +62,11 @@@ config RISC
| select GENERIC_SCHED_CLOCK
| select GENERIC_SMP_IDLE_THREAD
| select GENERIC_TIME_VSYSCALL if MMU && 64BIT
| ++<<<<<<< HEAD
| + select GENERIC_VDSO_TIME_NS if HAVE_GENERIC_VDSO
| + select HANDLE_DOMAIN_IRQ
| ++=======
| ++>>>>>>> irq/handle-domain-irq
| select HAVE_ARCH_AUDITSYSCALL
| select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
| select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
... where the resolution is:
| diff --cc arch/riscv/Kconfig
| index 77a088d0a7e9,353e28f5f849..000000000000
| --- a/arch/riscv/Kconfig
| +++ b/arch/riscv/Kconfig
| @@@ -62,8 -62,6 +62,7 @@@ config RISC
| select GENERIC_SCHED_CLOCK
| select GENERIC_SMP_IDLE_THREAD
| select GENERIC_TIME_VSYSCALL if MMU && 64BIT
| + select GENERIC_VDSO_TIME_NS if HAVE_GENERIC_VDSO
| - select HANDLE_DOMAIN_IRQ
| select HAVE_ARCH_AUDITSYSCALL
| select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
| select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
... so I reckon we're not set for major pain there unless something new
appears in arch code in the next few days.
If we can get this onto a branch for linux-next ASAP, and if Linus is
happy with this having come together a little late, maybe we could queue
this in tip for v5.16, perhaps after -rc1 to let this soak, or waiting
to apply the final patch to make it easier to revert the arch changes if
needed?
I'd like to avoid sitting on this for an entire cycle if possible.
Thanks,
Mark.
Hi Mark,
On 10/21/21 7:02 PM, Mark Rutland wrote:
> +/*
> + * TODO: restructure the ARMv7M entry logic so that this entry logic can live
> + * in arch code.
> + */
> +asmlinkage void __exception_irq_entry
> +static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
I'm seeing build time failure...
drivers/irqchip/irq-nvic.c:50:8: error: two or more data types in declaration specifiers
static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
^~~~
drivers/irqchip/irq-nvic.c:50:13: warning: 'nvic_handle_irq' defined but not used [-Wunused-function]
static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
I've fixed that locally and planing to give it a go...
Cheers
Vladimir
On Fri, Oct 22, 2021 at 04:18:18PM +0100, Vladimir Murzin wrote:
> Hi Mark,
>
> On 10/21/21 7:02 PM, Mark Rutland wrote:
> > +/*
> > + * TODO: restructure the ARMv7M entry logic so that this entry logic can live
> > + * in arch code.
> > + */
> > +asmlinkage void __exception_irq_entry
> > +static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>
> I'm seeing build time failure...
>
> drivers/irqchip/irq-nvic.c:50:8: error: two or more data types in declaration specifiers
> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> ^~~~
> drivers/irqchip/irq-nvic.c:50:13: warning: 'nvic_handle_irq' defined but not used [-Wunused-function]
> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>
> I've fixed that locally and planing to give it a go...
Ah, whoops. I've removed the extraneous `static void` from
nvic_handle_irq() and build tested that as part of stm32_defconfig.
The updated version is in my irq/handle-domain-irq branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
Thanks,
Mark.
On 10/22/21 4:36 PM, Mark Rutland wrote:
> On Fri, Oct 22, 2021 at 04:18:18PM +0100, Vladimir Murzin wrote:
>> Hi Mark,
>>
>> On 10/21/21 7:02 PM, Mark Rutland wrote:
>>> +/*
>>> + * TODO: restructure the ARMv7M entry logic so that this entry logic can live
>>> + * in arch code.
>>> + */
>>> +asmlinkage void __exception_irq_entry
>>> +static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>>
>> I'm seeing build time failure...
>>
>> drivers/irqchip/irq-nvic.c:50:8: error: two or more data types in declaration specifiers
>> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>> ^~~~
>> drivers/irqchip/irq-nvic.c:50:13: warning: 'nvic_handle_irq' defined but not used [-Wunused-function]
>> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>>
>> I've fixed that locally and planing to give it a go...
>
> Ah, whoops. I've removed the extraneous `static void` from
> nvic_handle_irq() and build tested that as part of stm32_defconfig.
>
> The updated version is in my irq/handle-domain-irq branch at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
>
$ cat /proc/interrupts
CPU0
16: 24 nvic_irq 4 Edge mps2-clkevt
17: 0 nvic_irq 32 Edge mps2-uart-rx
18: 6 nvic_irq 33 Edge mps2-uart-tx
19: 0 nvic_irq 47 Edge mps2-uart-overrun
Err: 0
So if it helps feel free to add my
Tested-by: Vladimir Murzin <[email protected]> # ARMv7M
As for TODO, is [1] look something you have been thinking of? IIUC,
the show stopper is that hwirq is being passed from exception entry
which retrieved via xPSR (IPSR to be precise). OTOH hwirq also available
via Interrupt Controller Status Register (ICSR) thus can be used in
driver itself... I gave [1] a go and it runs fine, yet I admit I might
be missing something...
[1]
---
arch/arm/include/asm/v7m.h | 3 ++-
arch/arm/kernel/entry-v7m.S | 10 +++-------
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/irq-nvic.c | 21 +++++----------------
4 files changed, 11 insertions(+), 24 deletions(-)
diff --git a/arch/arm/include/asm/v7m.h b/arch/arm/include/asm/v7m.h
index b1bad30b15d2..f047629887e7 100644
--- a/arch/arm/include/asm/v7m.h
+++ b/arch/arm/include/asm/v7m.h
@@ -13,6 +13,7 @@
#define V7M_SCB_ICSR_PENDSVSET (1 << 28)
#define V7M_SCB_ICSR_PENDSVCLR (1 << 27)
#define V7M_SCB_ICSR_RETTOBASE (1 << 11)
+#define V7M_SCB_ICSR_VECTACTIVE 0x000001ff
#define V7M_SCB_VTOR 0x08
@@ -38,7 +39,7 @@
#define V7M_SCB_SHCSR_MEMFAULTENA (1 << 16)
#define V7M_xPSR_FRAMEPTRALIGN 0x00000200
-#define V7M_xPSR_EXCEPTIONNO 0x000001ff
+#define V7M_xPSR_EXCEPTIONNO V7M_SCB_ICSR_VECTACTIVE
/*
* When branching to an address that has bits [31:28] == 0xf an exception return
diff --git a/arch/arm/kernel/entry-v7m.S b/arch/arm/kernel/entry-v7m.S
index 2e872a248e31..901c7cd1b1ce 100644
--- a/arch/arm/kernel/entry-v7m.S
+++ b/arch/arm/kernel/entry-v7m.S
@@ -72,14 +72,10 @@ __irq_entry:
@
@ Invoke the IRQ handler
@
- mrs r0, ipsr
- ldr r1, =V7M_xPSR_EXCEPTIONNO
- and r0, r1
- sub r0, #16
- mov r1, sp
+ mov r0, sp
stmdb sp!, {lr}
- @ routine called with r0 = irq number, r1 = struct pt_regs *
- bl nvic_handle_irq
+ @ routine called with r0 = struct pt_regs *
+ bl generic_handle_arch_irq
pop {lr}
@
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index aca7b595c4c7..b59a0bc0cd80 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -58,6 +58,7 @@ config ARM_NVIC
bool
select IRQ_DOMAIN_HIERARCHY
select GENERIC_IRQ_CHIP
+ select GENERIC_IRQ_MULTI_HANDLER
config ARM_VIC
bool
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
index 63bac3f78863..52ff0ed19f2f 100644
--- a/drivers/irqchip/irq-nvic.c
+++ b/drivers/irqchip/irq-nvic.c
@@ -37,25 +37,13 @@
static struct irq_domain *nvic_irq_domain;
-static void __nvic_handle_irq(irq_hw_number_t hwirq)
+static void __irq_entry nvic_handle_irq(struct pt_regs *regs)
{
- generic_handle_domain_irq(nvic_irq_domain, hwirq);
-}
+ unsigned long icsr = readl_relaxed(BASEADDR_V7M_SCB + V7M_SCB_ICSR);
+ irq_hw_number_t hwirq = (icsr & V7M_SCB_ICSR_VECTACTIVE) - 16;
-/*
- * TODO: restructure the ARMv7M entry logic so that this entry logic can live
- * in arch code.
- */
-asmlinkage void __exception_irq_entry
-nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
-{
- struct pt_regs *old_regs;
- irq_enter();
- old_regs = set_irq_regs(regs);
- __nvic_handle_irq(hwirq);
- set_irq_regs(old_regs);
- irq_exit();
+ generic_handle_domain_irq(nvic_irq_domain, hwirq);
}
static int nvic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
@@ -141,6 +129,7 @@ static int __init nvic_of_init(struct device_node *node,
for (i = 0; i < irqs; i += 4)
writel_relaxed(0, nvic_base + NVIC_IPR + i);
+ set_handle_irq(nvic_handle_irq);
return 0;
}
IRQCHIP_DECLARE(armv7m_nvic, "arm,armv7m-nvic", nvic_of_init);
> Thanks,
> Mark.
>
On Fri, Oct 22, 2021 at 05:34:20PM +0100, Vladimir Murzin wrote:
> On 10/22/21 4:36 PM, Mark Rutland wrote:
> > On Fri, Oct 22, 2021 at 04:18:18PM +0100, Vladimir Murzin wrote:
> >> Hi Mark,
> >>
> >> On 10/21/21 7:02 PM, Mark Rutland wrote:
> >>> +/*
> >>> + * TODO: restructure the ARMv7M entry logic so that this entry logic can live
> >>> + * in arch code.
> >>> + */
> >>> +asmlinkage void __exception_irq_entry
> >>> +static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> >>
> >> I'm seeing build time failure...
> >>
> >> drivers/irqchip/irq-nvic.c:50:8: error: two or more data types in declaration specifiers
> >> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> >> ^~~~
> >> drivers/irqchip/irq-nvic.c:50:13: warning: 'nvic_handle_irq' defined but not used [-Wunused-function]
> >> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> >>
> >> I've fixed that locally and planing to give it a go...
> >
> > Ah, whoops. I've removed the extraneous `static void` from
> > nvic_handle_irq() and build tested that as part of stm32_defconfig.
> >
> > The updated version is in my irq/handle-domain-irq branch at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
> >
>
> $ cat /proc/interrupts
> CPU0
> 16: 24 nvic_irq 4 Edge mps2-clkevt
> 17: 0 nvic_irq 32 Edge mps2-uart-rx
> 18: 6 nvic_irq 33 Edge mps2-uart-tx
> 19: 0 nvic_irq 47 Edge mps2-uart-overrun
> Err: 0
>
> So if it helps feel free to add my
>
> Tested-by: Vladimir Murzin <[email protected]> # ARMv7M
Thanks!
I've folded that in and uppdated the branch.
> As for TODO, is [1] look something you have been thinking of? IIUC,
> the show stopper is that hwirq is being passed from exception entry
> which retrieved via xPSR (IPSR to be precise). OTOH hwirq also available
> via Interrupt Controller Status Register (ICSR) thus can be used in
> driver itself... I gave [1] a go and it runs fine, yet I admit I might
> be missing something...
I hadn't thought about it in much detail, but that looks good!
I was wondering if we needed something like a
handle_arch_vectored_irq(), but if we can rely on the ICSR that seems
simpler overall. I'm not at all familiar with M-class, so I'm not sure
if there are pitfalls in this area.
Thanks,
Mark.
>
> [1]
>
> ---
> arch/arm/include/asm/v7m.h | 3 ++-
> arch/arm/kernel/entry-v7m.S | 10 +++-------
> drivers/irqchip/Kconfig | 1 +
> drivers/irqchip/irq-nvic.c | 21 +++++----------------
> 4 files changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/include/asm/v7m.h b/arch/arm/include/asm/v7m.h
> index b1bad30b15d2..f047629887e7 100644
> --- a/arch/arm/include/asm/v7m.h
> +++ b/arch/arm/include/asm/v7m.h
> @@ -13,6 +13,7 @@
> #define V7M_SCB_ICSR_PENDSVSET (1 << 28)
> #define V7M_SCB_ICSR_PENDSVCLR (1 << 27)
> #define V7M_SCB_ICSR_RETTOBASE (1 << 11)
> +#define V7M_SCB_ICSR_VECTACTIVE 0x000001ff
>
> #define V7M_SCB_VTOR 0x08
>
> @@ -38,7 +39,7 @@
> #define V7M_SCB_SHCSR_MEMFAULTENA (1 << 16)
>
> #define V7M_xPSR_FRAMEPTRALIGN 0x00000200
> -#define V7M_xPSR_EXCEPTIONNO 0x000001ff
> +#define V7M_xPSR_EXCEPTIONNO V7M_SCB_ICSR_VECTACTIVE
>
> /*
> * When branching to an address that has bits [31:28] == 0xf an exception return
> diff --git a/arch/arm/kernel/entry-v7m.S b/arch/arm/kernel/entry-v7m.S
> index 2e872a248e31..901c7cd1b1ce 100644
> --- a/arch/arm/kernel/entry-v7m.S
> +++ b/arch/arm/kernel/entry-v7m.S
> @@ -72,14 +72,10 @@ __irq_entry:
> @
> @ Invoke the IRQ handler
> @
> - mrs r0, ipsr
> - ldr r1, =V7M_xPSR_EXCEPTIONNO
> - and r0, r1
> - sub r0, #16
> - mov r1, sp
> + mov r0, sp
> stmdb sp!, {lr}
> - @ routine called with r0 = irq number, r1 = struct pt_regs *
> - bl nvic_handle_irq
> + @ routine called with r0 = struct pt_regs *
> + bl generic_handle_arch_irq
>
> pop {lr}
> @
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index aca7b595c4c7..b59a0bc0cd80 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -58,6 +58,7 @@ config ARM_NVIC
> bool
> select IRQ_DOMAIN_HIERARCHY
> select GENERIC_IRQ_CHIP
> + select GENERIC_IRQ_MULTI_HANDLER
>
> config ARM_VIC
> bool
> diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
> index 63bac3f78863..52ff0ed19f2f 100644
> --- a/drivers/irqchip/irq-nvic.c
> +++ b/drivers/irqchip/irq-nvic.c
> @@ -37,25 +37,13 @@
>
> static struct irq_domain *nvic_irq_domain;
>
> -static void __nvic_handle_irq(irq_hw_number_t hwirq)
> +static void __irq_entry nvic_handle_irq(struct pt_regs *regs)
> {
> - generic_handle_domain_irq(nvic_irq_domain, hwirq);
> -}
> + unsigned long icsr = readl_relaxed(BASEADDR_V7M_SCB + V7M_SCB_ICSR);
> + irq_hw_number_t hwirq = (icsr & V7M_SCB_ICSR_VECTACTIVE) - 16;
>
> -/*
> - * TODO: restructure the ARMv7M entry logic so that this entry logic can live
> - * in arch code.
> - */
> -asmlinkage void __exception_irq_entry
> -nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> -{
> - struct pt_regs *old_regs;
>
> - irq_enter();
> - old_regs = set_irq_regs(regs);
> - __nvic_handle_irq(hwirq);
> - set_irq_regs(old_regs);
> - irq_exit();
> + generic_handle_domain_irq(nvic_irq_domain, hwirq);
> }
>
> static int nvic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> @@ -141,6 +129,7 @@ static int __init nvic_of_init(struct device_node *node,
> for (i = 0; i < irqs; i += 4)
> writel_relaxed(0, nvic_base + NVIC_IPR + i);
>
> + set_handle_irq(nvic_handle_irq);
> return 0;
> }
> IRQCHIP_DECLARE(armv7m_nvic, "arm,armv7m-nvic", nvic_of_init);
>
> > Thanks,
> > Mark.
> >
>
On Fri, 22 Oct 2021 18:58:54 +0100,
Mark Rutland <[email protected]> wrote:
>
> On Fri, Oct 22, 2021 at 05:34:20PM +0100, Vladimir Murzin wrote:
> > On 10/22/21 4:36 PM, Mark Rutland wrote:
> > > On Fri, Oct 22, 2021 at 04:18:18PM +0100, Vladimir Murzin wrote:
> > >> Hi Mark,
> > >>
> > >> On 10/21/21 7:02 PM, Mark Rutland wrote:
> > >>> +/*
> > >>> + * TODO: restructure the ARMv7M entry logic so that this entry logic can live
> > >>> + * in arch code.
> > >>> + */
> > >>> +asmlinkage void __exception_irq_entry
> > >>> +static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> > >>
> > >> I'm seeing build time failure...
> > >>
> > >> drivers/irqchip/irq-nvic.c:50:8: error: two or more data types in declaration specifiers
> > >> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> > >> ^~~~
> > >> drivers/irqchip/irq-nvic.c:50:13: warning: 'nvic_handle_irq' defined but not used [-Wunused-function]
> > >> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> > >>
> > >> I've fixed that locally and planing to give it a go...
> > >
> > > Ah, whoops. I've removed the extraneous `static void` from
> > > nvic_handle_irq() and build tested that as part of stm32_defconfig.
> > >
> > > The updated version is in my irq/handle-domain-irq branch at:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
> > >
> >
> > $ cat /proc/interrupts
> > CPU0
> > 16: 24 nvic_irq 4 Edge mps2-clkevt
> > 17: 0 nvic_irq 32 Edge mps2-uart-rx
> > 18: 6 nvic_irq 33 Edge mps2-uart-tx
> > 19: 0 nvic_irq 47 Edge mps2-uart-overrun
> > Err: 0
> >
> > So if it helps feel free to add my
> >
> > Tested-by: Vladimir Murzin <[email protected]> # ARMv7M
>
> Thanks!
>
> I've folded that in and uppdated the branch.
>
> > As for TODO, is [1] look something you have been thinking of? IIUC,
> > the show stopper is that hwirq is being passed from exception entry
> > which retrieved via xPSR (IPSR to be precise). OTOH hwirq also available
> > via Interrupt Controller Status Register (ICSR) thus can be used in
> > driver itself... I gave [1] a go and it runs fine, yet I admit I might
> > be missing something...
>
> I hadn't thought about it in much detail, but that looks good!
>
> I was wondering if we needed something like a
> handle_arch_vectored_irq(), but if we can rely on the ICSR that seems
> simpler overall. I'm not at all familiar with M-class, so I'm not sure
> if there are pitfalls in this area.
Why can't we just use IPSR instead from the C code? It has the
potential of being of lower latency then a MMIO read (though I have no
idea whether it makes a material difference on M-class) and from what
I can see in the arch spec, they are strictly equivalent.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Thu, Oct 21, 2021 at 07:02:33PM +0100, Mark Rutland wrote:
> In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have
> arch/openrisc perform all the irqentry accounting in its entry code. As
> arch/openrisc uses GENERIC_IRQ_MULTI_HANDLER, we can use
> generic_handle_arch_irq() to do so.
>
> There should be no functional change as a result of this patch.
Reviewed-by: Stafford Horne <[email protected]>
On 10/22/21 7:43 PM, Marc Zyngier wrote:
> On Fri, 22 Oct 2021 18:58:54 +0100,
> Mark Rutland <[email protected]> wrote:
>>
>> On Fri, Oct 22, 2021 at 05:34:20PM +0100, Vladimir Murzin wrote:
>>> On 10/22/21 4:36 PM, Mark Rutland wrote:
>>>> On Fri, Oct 22, 2021 at 04:18:18PM +0100, Vladimir Murzin wrote:
>>>>> Hi Mark,
>>>>>
>>>>> On 10/21/21 7:02 PM, Mark Rutland wrote:
>>>>>> +/*
>>>>>> + * TODO: restructure the ARMv7M entry logic so that this entry logic can live
>>>>>> + * in arch code.
>>>>>> + */
>>>>>> +asmlinkage void __exception_irq_entry
>>>>>> +static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>>>>>
>>>>> I'm seeing build time failure...
>>>>>
>>>>> drivers/irqchip/irq-nvic.c:50:8: error: two or more data types in declaration specifiers
>>>>> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>>>>> ^~~~
>>>>> drivers/irqchip/irq-nvic.c:50:13: warning: 'nvic_handle_irq' defined but not used [-Wunused-function]
>>>>> static void nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
>>>>>
>>>>> I've fixed that locally and planing to give it a go...
>>>>
>>>> Ah, whoops. I've removed the extraneous `static void` from
>>>> nvic_handle_irq() and build tested that as part of stm32_defconfig.
>>>>
>>>> The updated version is in my irq/handle-domain-irq branch at:
>>>>
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
>>>>
>>>
>>> $ cat /proc/interrupts
>>> CPU0
>>> 16: 24 nvic_irq 4 Edge mps2-clkevt
>>> 17: 0 nvic_irq 32 Edge mps2-uart-rx
>>> 18: 6 nvic_irq 33 Edge mps2-uart-tx
>>> 19: 0 nvic_irq 47 Edge mps2-uart-overrun
>>> Err: 0
>>>
>>> So if it helps feel free to add my
>>>
>>> Tested-by: Vladimir Murzin <[email protected]> # ARMv7M
>>
>> Thanks!
>>
>> I've folded that in and uppdated the branch.
>>
>>> As for TODO, is [1] look something you have been thinking of? IIUC,
>>> the show stopper is that hwirq is being passed from exception entry
>>> which retrieved via xPSR (IPSR to be precise). OTOH hwirq also available
>>> via Interrupt Controller Status Register (ICSR) thus can be used in
>>> driver itself... I gave [1] a go and it runs fine, yet I admit I might
>>> be missing something...
>>
>> I hadn't thought about it in much detail, but that looks good!
>>
>> I was wondering if we needed something like a
>> handle_arch_vectored_irq(), but if we can rely on the ICSR that seems
>> simpler overall. I'm not at all familiar with M-class, so I'm not sure
>> if there are pitfalls in this area.
>
> Why can't we just use IPSR instead from the C code? It has the
> potential of being of lower latency then a MMIO read (though I have no
> idea whether it makes a material difference on M-class) and from what
> I can see in the arch spec, they are strictly equivalent.
Hmmm, less arch specific asm(s) in driver code, no?
Cheers
Vladimir
>
> Thanks,
>
> M.
>
On Sat, 23 Oct 2021 13:06:25 +0100,
Vladimir Murzin <[email protected]> wrote:
>
> On 10/22/21 7:43 PM, Marc Zyngier wrote:
> > On Fri, 22 Oct 2021 18:58:54 +0100,
> > Mark Rutland <[email protected]> wrote:
> >>
> >> On Fri, Oct 22, 2021 at 05:34:20PM +0100, Vladimir Murzin wrote:
[...]
> >>> As for TODO, is [1] look something you have been thinking of? IIUC,
> >>> the show stopper is that hwirq is being passed from exception entry
> >>> which retrieved via xPSR (IPSR to be precise). OTOH hwirq also available
> >>> via Interrupt Controller Status Register (ICSR) thus can be used in
> >>> driver itself... I gave [1] a go and it runs fine, yet I admit I might
> >>> be missing something...
> >>
> >> I hadn't thought about it in much detail, but that looks good!
> >>
> >> I was wondering if we needed something like a
> >> handle_arch_vectored_irq(), but if we can rely on the ICSR that seems
> >> simpler overall. I'm not at all familiar with M-class, so I'm not sure
> >> if there are pitfalls in this area.
> >
> > Why can't we just use IPSR instead from the C code? It has the
> > potential of being of lower latency then a MMIO read (though I have no
> > idea whether it makes a material difference on M-class) and from what
> > I can see in the arch spec, they are strictly equivalent.
>
> Hmmm, less arch specific asm(s) in driver code, no?
Well, it isn't like this driver is going to be useful on anything
else, is it?
If there is no overhead in reading from MMIO compared to the
architected register, then I agree that ICSR is the way to
go. Is there any chance you could measure it on a HW platform? Or
maybe in emulation?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On 10/23/21 2:18 PM, Marc Zyngier wrote:
> On Sat, 23 Oct 2021 13:06:25 +0100,
> Vladimir Murzin <[email protected]> wrote:
>>
>> On 10/22/21 7:43 PM, Marc Zyngier wrote:
>>> On Fri, 22 Oct 2021 18:58:54 +0100,
>>> Mark Rutland <[email protected]> wrote:
>>>>
>>>> On Fri, Oct 22, 2021 at 05:34:20PM +0100, Vladimir Murzin wrote:
>
> [...]
>
>>>>> As for TODO, is [1] look something you have been thinking of? IIUC,
>>>>> the show stopper is that hwirq is being passed from exception entry
>>>>> which retrieved via xPSR (IPSR to be precise). OTOH hwirq also available
>>>>> via Interrupt Controller Status Register (ICSR) thus can be used in
>>>>> driver itself... I gave [1] a go and it runs fine, yet I admit I might
>>>>> be missing something...
>>>>
>>>> I hadn't thought about it in much detail, but that looks good!
>>>>
>>>> I was wondering if we needed something like a
>>>> handle_arch_vectored_irq(), but if we can rely on the ICSR that seems
>>>> simpler overall. I'm not at all familiar with M-class, so I'm not sure
>>>> if there are pitfalls in this area.
>>>
>>> Why can't we just use IPSR instead from the C code? It has the
>>> potential of being of lower latency then a MMIO read (though I have no
>>> idea whether it makes a material difference on M-class) and from what
>>> I can see in the arch spec, they are strictly equivalent.
>>
>> Hmmm, less arch specific asm(s) in driver code, no?
>
> Well, it isn't like this driver is going to be useful on anything
> else, is it?
>
Well, with some work to unwire it from arch/arm it can be COMPILE_TEST :)
> If there is no overhead in reading from MMIO compared to the
> architected register, then I agree that ICSR is the way to
> go. Is there any chance you could measure it on a HW platform? Or
> maybe in emulation?
My MPS{2,3} boards left in office and I'm on holiday next week... OTOH, I
have no strong opinion on ICSR vs IPSR, I just wanted to check how much
work it'd be to close TODO per my (quite limited) understanding :)
Cheers
Vladimir
>
> Thanks,
>
> M.
>
On Fri, 22 Oct 2021 16:10:07 +0100,
Mark Rutland <[email protected]> wrote:
>
> On Fri, Oct 22, 2021 at 12:20:31PM +0100, Marc Zyngier wrote:
> > Hi Mark,
> >
> > On Thu, 21 Oct 2021 19:02:21 +0100,
> > Mark Rutland <[email protected]> wrote:
> > >
> > > The handle_domain_{irq,nmi}() functions were oringally intended as a
> > > convenience, but recent rework to entry code across the kernel tree has
> > > demonstrated that they cause more pain than they're worth and prevent
> > > architectures from being able to write robust entry code.
> > >
> > > This series reworks the irq code to remove them, handling the necessary
> > > entry work consistently in entry code (be it architectural or generic).
> >
> > [...]
> >
> > Thanks for going through the pain of putting this together. The
> > couple of nits I mentioned notwithstanding:
> >
> > Reviewed-by: Marc Zyngier <[email protected]>
>
> Thanks!
>
> I've pushed out an updated version to my irq/handle-domain-irq branch
> on kernel.org:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
> https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
>
> That has two new patches you suggested:
>
> * irq: mips: simplify bcm6345_l1_irq_handle()
> * irq: unexport handle_irq_desc()
>
> ... which I did not add your Reviewed-by to in case the commit messages
> are garbage or something like that.
I quickly eyeballed the patches, and they look OK to me. Feel free to
add my RB tag to them.
>
> > It'd be good to work out a merging strategy once this has seen a bit
> > of testing.
>
> Conflict-wise, this merges near perfectly against next-20212022 aside
> from a trivial conflict against arch/riscv/Kconfig:
>
> | [mark@lakrids:~/src/linux]% git merge irq/handle-domain-irq
> | Auto-merging arch/riscv/kernel/entry.S
> | Auto-merging arch/riscv/Kconfig
> | CONFLICT (content): Merge conflict in arch/riscv/Kconfig
> | Auto-merging arch/nds32/Kconfig
> | Auto-merging arch/mips/Kconfig
> | Auto-merging arch/csky/Kconfig
> | Auto-merging arch/arm64/Kconfig
> | Auto-merging arch/arm/mach-s3c/irq-s3c24xx.c
> | Auto-merging arch/arm/kernel/entry-armv.S
> | Auto-merging arch/arm/Kconfig
> | Auto-merging arch/arc/Kconfig
> | Automatic merge failed; fix conflicts and then commit the result.
> | [mark@lakrids:~/src/linux]% git diff
> | diff --cc arch/riscv/Kconfig
> | index 77a088d0a7e9,353e28f5f849..000000000000
> | --- a/arch/riscv/Kconfig
> | +++ b/arch/riscv/Kconfig
> | @@@ -62,8 -62,6 +62,11 @@@ config RISC
> | select GENERIC_SCHED_CLOCK
> | select GENERIC_SMP_IDLE_THREAD
> | select GENERIC_TIME_VSYSCALL if MMU && 64BIT
> | ++<<<<<<< HEAD
> | + select GENERIC_VDSO_TIME_NS if HAVE_GENERIC_VDSO
> | + select HANDLE_DOMAIN_IRQ
> | ++=======
> | ++>>>>>>> irq/handle-domain-irq
> | select HAVE_ARCH_AUDITSYSCALL
> | select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
> | select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
>
> ... where the resolution is:
>
> | diff --cc arch/riscv/Kconfig
> | index 77a088d0a7e9,353e28f5f849..000000000000
> | --- a/arch/riscv/Kconfig
> | +++ b/arch/riscv/Kconfig
> | @@@ -62,8 -62,6 +62,7 @@@ config RISC
> | select GENERIC_SCHED_CLOCK
> | select GENERIC_SMP_IDLE_THREAD
> | select GENERIC_TIME_VSYSCALL if MMU && 64BIT
> | + select GENERIC_VDSO_TIME_NS if HAVE_GENERIC_VDSO
> | - select HANDLE_DOMAIN_IRQ
> | select HAVE_ARCH_AUDITSYSCALL
> | select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
> | select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
>
> ... so I reckon we're not set for major pain there unless something new
> appears in arch code in the next few days.
>
> If we can get this onto a branch for linux-next ASAP, and if Linus is
> happy with this having come together a little late, maybe we could queue
> this in tip for v5.16, perhaps after -rc1 to let this soak, or waiting
> to apply the final patch to make it easier to revert the arch changes if
> needed?
I'm happy to route it via the irqchip tree (and ultimately tip) if
nobody objects (which also means getting Acks from the arch maintainers).
The branch would thus see a bit of -next before being sent to Linus.
> I'd like to avoid sitting on this for an entire cycle if possible.
+1.
M.
--
Without deviation from the norm, progress is not possible.
On Fri, Oct 22, 2021 at 4:52 PM Mark Rutland <[email protected]> wrote:
>
> On Fri, Oct 22, 2021 at 10:33:53AM +0800, Guo Ren wrote:
> > On Fri, Oct 22, 2021 at 2:03 AM Mark Rutland <[email protected]> wrote:
> > >
> > > Several architectures select GENERIC_IRQ_MULTI_HANDLER and branch to
> > > handle_arch_irq() without performing any entry accounting.
> > >
> > > Add a generic wrapper to handle the commoon irqentry work when invoking
>
> Ah, I'd typo'd 'common' there; I'll go fix that...
>
> > > handle_arch_irq(). Where an architecture needs to perform some entry
> > > accounting itself, it will need to invoke handle_arch_irq() itself.
> > >
> > > In subsequent patches it will become the responsibilty of the entry code
> > > to set the irq regs when entering an IRQ (rather than deferring this to
> > > an irqchip handler), so generic_handle_arch_irq() is made to set the irq
> > > regs now. This can be redundant in some cases, but is never harmful as
> > > saving/restoring the old regs nests safely.
> > Shall we remove old_regs save/restore in handle_domain_irq? It's duplicated.
>
> We do, in patch 15 when handle_domain_irq() is removed entirely. :)
I miss that patch. Yes, in generic_handle_domain_nmi.
>
> Removing it immediately in this patch would require more ifdeffery and/or
> another copy of handle_domain_irq(), and since the duplication doesn't cause a
> functional problem, I think it's better to live with the temporary duplication
> for the next few patches than to try to optimize the intermediate steps at the
> cost of legibility.
Thx for explaining.
Reviewed-by: Guo Ren <[email protected]>
>
> Thanks,
> Mark.
>
> > > Signed-off-by: Mark Rutland <[email protected]>
> > > Cc: Marc Zyngier <[email protected]>
> > > Cc: Thomas Gleixner <[email protected]>
> > > ---
> > > kernel/irq/handle.c | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
> > > index 221d80c31e94..27182003b879 100644
> > > --- a/kernel/irq/handle.c
> > > +++ b/kernel/irq/handle.c
> > > @@ -14,6 +14,8 @@
> > > #include <linux/interrupt.h>
> > > #include <linux/kernel_stat.h>
> > >
> > > +#include <asm/irq_regs.h>
> > > +
> > > #include <trace/events/irq.h>
> > >
> > > #include "internals.h"
> > > @@ -226,4 +228,20 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
> > > handle_arch_irq = handle_irq;
> > > return 0;
> > > }
> > > +
> > > +/**
> > > + * generic_handle_arch_irq - root irq handler for architectures which do no
> > > + * entry accounting themselves
> > > + * @regs: Register file coming from the low-level handling code
> > > + */
> > > +asmlinkage void noinstr generic_handle_arch_irq(struct pt_regs *regs)
> > > +{
> > > + struct pt_regs *old_regs;
> > > +
> > > + irq_enter();
> > > + old_regs = set_irq_regs(regs);
> > > + handle_arch_irq(regs);
> > > + set_irq_regs(old_regs);
> > > + irq_exit();
> > > +}
> > > #endif
> > > --
> > > 2.11.0
> > >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
On Thu, Oct 21, 2021 at 07:02:24PM +0100, Mark Rutland wrote:
> There's no need fpr arch/mips's do_domain_IRQ() to open-code the NULL
> check performed by handle_irq_desc(), nor the resolution of the desc
> performed by generic_handle_domain_irq().
>
> Use generic_handle_domain_irq() directly, as this is functioanlly
> equivalent and clearer.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> arch/mips/kernel/irq.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c
> index d20e002b3246..1fee96ef8059 100644
> --- a/arch/mips/kernel/irq.c
> +++ b/arch/mips/kernel/irq.c
> @@ -115,11 +115,7 @@ void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq)
>
> irq_enter();
> check_stack_overflow();
> -
> - desc = irq_resolve_mapping(domain, hwirq);
> - if (likely(desc))
> - handle_irq_desc(desc);
> -
> + generic_handle_domain_irq(domain, hwirq);
> irq_exit();
> }
> #endif
> --
> 2.11.0
Acked-by: Thomas Bogendoerfer <[email protected]>
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
On Thu, Oct 21, 2021 at 07:02:31PM +0100, Mark Rutland wrote:
> In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/arm64
> perform all the irqentry accounting in its entry code.
>
> As arch/arm64 already performs portions of the irqentry logic in
> enter_from_kernel_mode() and exit_to_kernel_mode(), including
> rcu_irq_{enter,exit}(), the only additional calls that need to be made
> are to irq_{enter,exit}_rcu(). Removing the calls to
> rcu_irq_{enter,exit}() from handle_domain_irq() ensures that we inform
> RCU once per IRQ entry and will correctly identify quiescent periods.
>
> Since we should not call irq_{enter,exit}_rcu() when entering a
> pseudo-NMI, el1_interrupt() is reworked to have separate __el1_irq() and
> __el1_pnmi() paths for regular IRQ and psuedo-NMI entry, with
> irq_{enter,exit}_irq() only called for the former.
>
> In preparation for removing HANDLE_DOMAIN_IRQ, the irq regs are managed
> in do_interrupt_handler() for both regular IRQ and pseudo-NMI. This is
> currently redundant, but not harmful.
>
> For clarity the preemption logic is moved into __el1_irq(). We should
> never preempt within a pseudo-NMI, and arm64_enter_nmi() already
> enforces this by incrementing the preempt_count, but it's clearer if we
> never invoke the preemption logic when entering a pseudo-NMI.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
On Thu, 21 Oct 2021 11:02:34 PDT (-0700), [email protected] wrote:
> In preparation for removing HANDLE_DOMAIN_IRQ_IRQENTRY, have arch/riscv
> perform all the irqentry accounting in its entry code. As arch/riscv
> uses GENERIC_IRQ_MULTI_HANDLER, we can use generic_handle_arch_irq() to
> do so.
>
> Since generic_handle_arch_irq() handles the irq entry and setting the
> irq regs, and happens before the irqchip code calls handle_IPI(), we can
> remove the redundant irq entry and irq regs manipulation from
> handle_IPI().
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Albert Ou <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Paul Walmsley <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> arch/riscv/Kconfig | 1 -
> arch/riscv/kernel/entry.S | 3 +--
> arch/riscv/kernel/smp.c | 9 +--------
> 3 files changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 740653063a56..301a54233c7e 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -63,7 +63,6 @@ config RISCV
> select GENERIC_SMP_IDLE_THREAD
> select GENERIC_TIME_VSYSCALL if MMU && 64BIT
> select HANDLE_DOMAIN_IRQ
> - select HANDLE_DOMAIN_IRQ_IRQENTRY
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
> select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 98f502654edd..64236f7efde5 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -130,8 +130,7 @@ skip_context_tracking:
>
> /* Handle interrupts */
> move a0, sp /* pt_regs */
> - la a1, handle_arch_irq
> - REG_L a1, (a1)
> + la a1, generic_handle_arch_irq
> jr a1
> 1:
> /*
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 921d9d7df400..2f6da845c9ae 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -140,12 +140,9 @@ void arch_irq_work_raise(void)
>
> void handle_IPI(struct pt_regs *regs)
> {
> - struct pt_regs *old_regs = set_irq_regs(regs);
> unsigned long *pending_ipis = &ipi_data[smp_processor_id()].bits;
> unsigned long *stats = ipi_data[smp_processor_id()].stats;
>
> - irq_enter();
> -
> riscv_clear_ipi();
>
> while (true) {
> @@ -156,7 +153,7 @@ void handle_IPI(struct pt_regs *regs)
>
> ops = xchg(pending_ipis, 0);
> if (ops == 0)
> - goto done;
> + return;
>
> if (ops & (1 << IPI_RESCHEDULE)) {
> stats[IPI_RESCHEDULE]++;
> @@ -189,10 +186,6 @@ void handle_IPI(struct pt_regs *regs)
> /* Order data access and bit testing. */
> mb();
> }
> -
> -done:
> - irq_exit();
> - set_irq_regs(old_regs);
> }
>
> static const char * const ipi_names[] = {
Acked-by: Palmer Dabbelt <[email protected]>
I'm assuming you want to keep these togther.
On Thu, Oct 21, 2021 at 07:02:24PM +0100, Mark Rutland wrote:
> There's no need fpr arch/mips's do_domain_IRQ() to open-code the NULL
> check performed by handle_irq_desc(), nor the resolution of the desc
> performed by generic_handle_domain_irq().
>
> Use generic_handle_domain_irq() directly, as this is functioanlly
> equivalent and clearer.
>
> There should be no functional change as a result of this patch.
>
Except for this compile error:
arch/mips/kernel/irq.c: In function 'do_domain_IRQ':
arch/mips/kernel/irq.c:114:26: error: unused variable 'desc' [-Werror=unused-variable]
114 | struct irq_desc *desc;
Guenter
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> arch/mips/kernel/irq.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c
> index d20e002b3246..1fee96ef8059 100644
> --- a/arch/mips/kernel/irq.c
> +++ b/arch/mips/kernel/irq.c
> @@ -115,11 +115,7 @@ void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq)
>
> irq_enter();
> check_stack_overflow();
> -
> - desc = irq_resolve_mapping(domain, hwirq);
> - if (likely(desc))
> - handle_irq_desc(desc);
> -
> + generic_handle_domain_irq(domain, hwirq);
> irq_exit();
> }
> #endif
> --
> 2.11.0
>
On Thu, Oct 28, 2021 at 10:07:32AM -0700, Guenter Roeck wrote:
> On Thu, Oct 21, 2021 at 07:02:24PM +0100, Mark Rutland wrote:
> > There's no need fpr arch/mips's do_domain_IRQ() to open-code the NULL
> > check performed by handle_irq_desc(), nor the resolution of the desc
> > performed by generic_handle_domain_irq().
> >
> > Use generic_handle_domain_irq() directly, as this is functioanlly
> > equivalent and clearer.
> >
> > There should be no functional change as a result of this patch.
> >
>
> Except for this compile error:
>
> arch/mips/kernel/irq.c: In function 'do_domain_IRQ':
> arch/mips/kernel/irq.c:114:26: error: unused variable 'desc' [-Werror=unused-variable]
> 114 | struct irq_desc *desc;
>
> Guenter
Sorry for that; this has been fixed by:
https://lore.kernel.org/r/[email protected]
... which is queued up:
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/remove-handle-domain-irq-20211026&id=34fca8947b2743e6a3a9a8a3a44962e625993533
Thanks,
Mark.
>
> > Signed-off-by: Mark Rutland <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Thomas Bogendoerfer <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > ---
> > arch/mips/kernel/irq.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c
> > index d20e002b3246..1fee96ef8059 100644
> > --- a/arch/mips/kernel/irq.c
> > +++ b/arch/mips/kernel/irq.c
> > @@ -115,11 +115,7 @@ void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq)
> >
> > irq_enter();
> > check_stack_overflow();
> > -
> > - desc = irq_resolve_mapping(domain, hwirq);
> > - if (likely(desc))
> > - handle_irq_desc(desc);
> > -
> > + generic_handle_domain_irq(domain, hwirq);
> > irq_exit();
> > }
> > #endif
> > --
> > 2.11.0
> >
On 10/23/21 2:36 PM, Vladimir Murzin wrote:
> On 10/23/21 2:18 PM, Marc Zyngier wrote:
>> On Sat, 23 Oct 2021 13:06:25 +0100,
>> Vladimir Murzin <[email protected]> wrote:
>>>
>>> On 10/22/21 7:43 PM, Marc Zyngier wrote:
>>>> On Fri, 22 Oct 2021 18:58:54 +0100,
>>>> Mark Rutland <[email protected]> wrote:
>>>>>
>>>>> On Fri, Oct 22, 2021 at 05:34:20PM +0100, Vladimir Murzin wrote:
>>
>> [...]
>>
>>>>>> As for TODO, is [1] look something you have been thinking of? IIUC,
>>>>>> the show stopper is that hwirq is being passed from exception entry
>>>>>> which retrieved via xPSR (IPSR to be precise). OTOH hwirq also available
>>>>>> via Interrupt Controller Status Register (ICSR) thus can be used in
>>>>>> driver itself... I gave [1] a go and it runs fine, yet I admit I might
>>>>>> be missing something...
>>>>>
>>>>> I hadn't thought about it in much detail, but that looks good!
>>>>>
>>>>> I was wondering if we needed something like a
>>>>> handle_arch_vectored_irq(), but if we can rely on the ICSR that seems
>>>>> simpler overall. I'm not at all familiar with M-class, so I'm not sure
>>>>> if there are pitfalls in this area.
>>>>
>>>> Why can't we just use IPSR instead from the C code? It has the
>>>> potential of being of lower latency then a MMIO read (though I have no
>>>> idea whether it makes a material difference on M-class) and from what
>>>> I can see in the arch spec, they are strictly equivalent.
>>>
>>> Hmmm, less arch specific asm(s) in driver code, no?
>>
>> Well, it isn't like this driver is going to be useful on anything
>> else, is it?
>>
>
> Well, with some work to unwire it from arch/arm it can be COMPILE_TEST :)
>
>> If there is no overhead in reading from MMIO compared to the
>> architected register, then I agree that ICSR is the way to
>> go. Is there any chance you could measure it on a HW platform? Or
>> maybe in emulation?
>
> My MPS{2,3} boards left in office and I'm on holiday next week... OTOH, I
> have no strong opinion on ICSR vs IPSR, I just wanted to check how much
> work it'd be to close TODO per my (quite limited) understanding :)
One month and a week later...
I observe that in terms of performance
MRS r0, ipsr
is equivalent to readl_relaxed(BASEADDR_V7M_SCB + V7M_SCB_ICSR)
MOV.W r3, #3758153728
LDR.W r0, [r3, #3332]
Old compilers can produce less performant sequence like
LDR r3,0xbcc0
ADD.W r3,r3,#0xaf00
LDR r0,[r3,#0]
So, what would be your preference?
Cheers
Vladimir
>
> Cheers
> Vladimir
>
>
>>
>> Thanks,
>>
>> M.
>>
>
On 2021-11-30 08:49, Vladimir Murzin wrote:
> One month and a week later...
>
> I observe that in terms of performance
>
> MRS r0, ipsr
>
> is equivalent to readl_relaxed(BASEADDR_V7M_SCB + V7M_SCB_ICSR)
>
> MOV.W r3, #3758153728
> LDR.W r0, [r3, #3332]
>
> Old compilers can produce less performant sequence like
>
> LDR r3,0xbcc0
> ADD.W r3,r3,#0xaf00
> LDR r0,[r3,#0]
>
> So, what would be your preference?
If there is no significant overhead to reading the MMIO register
and that you see a benefit in enabling COMPILE_TEST, then this
probably is the way to go.
Thanks,
M.
--
Jazz is not dead. It just smells funny...