linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}()
@ 2021-10-26  9:24 Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 01/17] irq: mips: avoid nested irq_enter() Mark Rutland
                   ` (17 more replies)
  0 siblings, 18 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel, maz
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will, paulmck, peterz,
	torvalds

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

Marc, on the assumption you'll take this into the irqchip tree, I've pushed
this out to my kernel.org repo, tagged as remove-handle-domain-irq-20211026,
which should be commit:

  0953fb263714e1c8 ("irq: remove handle_domain_{irq,nmi}() (2021-10-26 10:13:31 +0100)")

Since v1 [1]:
* Accumulate tags
* Simplify bcm6345_l1_irq_handle()
* Don't export generic_handle_domain_nmi()
* Unexport handle_irq_desc()

[1] https://lore.kernel.org/r/20211021180236.37428-1-mark.rutland@arm.com

The gory details are in the thread starting at:

  https://lore.kernel.org/r/20211011104729.GB1421@C02TD0UTHF1T.local

... 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 (17):
  irq: mips: avoid nested irq_enter()
  irq: mips: simplify bcm6345_l1_irq_handle()
  irq: mips: stop (ab)using handle_domain_irq()
  irq: mips: simplify do_domain_IRQ()
  irq: simplify handle_domain_{irq,nmi}()
  irq: unexport handle_irq_desc()
  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          |  6 +--
 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                | 17 ++++++-
 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/irq.h                       |  1 +
 include/linux/irqdesc.h                   |  9 +---
 kernel/irq/Kconfig                        |  3 --
 kernel/irq/handle.c                       | 18 +++++++
 kernel/irq/irqdesc.c                      | 81 +++++++------------------------
 67 files changed, 192 insertions(+), 219 deletions(-)

-- 
2.11.0


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

* [PATCH v2 01/17] irq: mips: avoid nested irq_enter()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 02/17] irq: mips: simplify bcm6345_l1_irq_handle() Mark Rutland
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

As bcm6345_l1_irq_handle() is a chained irqchip handler, it will be
invoked within the context of the root irqchip handler, which must have
entered IRQ context already.

When bcm6345_l1_irq_handle() calls arch/mips's do_IRQ() , this will nest
another call to irq_enter(), and the resulting nested increment to
`rcu_data.dynticks_nmi_nesting` will cause rcu_is_cpu_rrupt_from_idle()
to fail to identify wakeups from idle, resulting in failure to preempt,
and RCU stalls.

Chained irqchip handlers must invoke IRQ handlers by way of thee core
irqchip code, i.e. generic_handle_irq() or generic_handle_domain_irq()
and should not call do_IRQ(), which is intended only for root irqchip
handlers.

Fix bcm6345_l1_irq_handle() by calling generic_handle_irq() directly.

Fixes: c7c42ec2baa1de7a ("irqchips/bmips: Add bcm6345-l1 interrupt controller")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-bcm6345-l1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-bcm6345-l1.c b/drivers/irqchip/irq-bcm6345-l1.c
index e3483789f4df..1bd0621c4ce2 100644
--- a/drivers/irqchip/irq-bcm6345-l1.c
+++ b/drivers/irqchip/irq-bcm6345-l1.c
@@ -140,7 +140,7 @@ static void bcm6345_l1_irq_handle(struct irq_desc *desc)
 		for_each_set_bit(hwirq, &pending, IRQS_PER_WORD) {
 			irq = irq_linear_revmap(intc->domain, base + hwirq);
 			if (irq)
-				do_IRQ(irq);
+				generic_handle_irq(irq);
 			else
 				spurious_interrupt();
 		}
-- 
2.11.0


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

* [PATCH v2 02/17] irq: mips: simplify bcm6345_l1_irq_handle()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 01/17] irq: mips: avoid nested irq_enter() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 03/17] irq: mips: stop (ab)using handle_domain_irq() Mark Rutland
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

As bcm6345_l1_irq_handle() only needs to know /whether/ an IRQ was
resolved, and doesn't need to know the specific IRQ, it's simpler for it
to call generic_handle_domain_irq() directly and check the return code,
so let's do that.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-bcm6345-l1.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-bcm6345-l1.c b/drivers/irqchip/irq-bcm6345-l1.c
index 1bd0621c4ce2..fd079215c17f 100644
--- a/drivers/irqchip/irq-bcm6345-l1.c
+++ b/drivers/irqchip/irq-bcm6345-l1.c
@@ -132,16 +132,12 @@ static void bcm6345_l1_irq_handle(struct irq_desc *desc)
 		int base = idx * IRQS_PER_WORD;
 		unsigned long pending;
 		irq_hw_number_t hwirq;
-		unsigned int irq;
 
 		pending = __raw_readl(cpu->map_base + reg_status(intc, idx));
 		pending &= __raw_readl(cpu->map_base + reg_enable(intc, idx));
 
 		for_each_set_bit(hwirq, &pending, IRQS_PER_WORD) {
-			irq = irq_linear_revmap(intc->domain, base + hwirq);
-			if (irq)
-				generic_handle_irq(irq);
-			else
+			if (generic_handle_domain_irq(intc->domain, base + hwirq))
 				spurious_interrupt();
 		}
 	}
-- 
2.11.0


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

* [PATCH v2 03/17] irq: mips: stop (ab)using handle_domain_irq()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 01/17] irq: mips: avoid nested irq_enter() Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 02/17] irq: mips: simplify bcm6345_l1_irq_handle() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 04/17] irq: mips: simplify do_domain_IRQ() Mark Rutland
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

On MIPS, the only user of handle_domain_irq() is octeon_irq_ciu3_ip2(),
which is called from the platform-specific plat_irq_dispatch() function
invoked from the early assembly code.

No other irqchip relevant to arch/mips uses handle_domain_irq():

* No other plat_irq_dispatch() function transitively calls
  handle_domain_irq().

* No other vectored IRQ dispatch function registered with
  set_vi_handler() calls handle_domain_irq().

* No chained irqchip handlers call handle_domain_irq(), which makes
  sense as this is meant to only be used by root irqchip handlers.

Currently octeon_irq_ciu3_ip2() passes NULL as the `regs` argument to
handle_domain_irq(), and as handle_domain_irq() will pass this to
set_irq_regs(), any invoked IRQ handlers will erroneously see a NULL
pt_regs if they call get_pt_regs().

Fix this by calling generic_handle_domain_irq() directly, and performing
the necessary irq_{enter,exit}() logic directly in
octeon_irq_ciu3_ip2(). At the same time, deselect HANDLE_DOMAIN_IRQ,
which subsequent patches will remove.

Other than the corrected behaviour of get_pt_regs(), there should be no
functional change as a result of this patch.

Fixes: ce210d35bb93c2c5 ("MIPS: OCTEON: Add support for OCTEON III interrupt controller.")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/mips/Kconfig                    | 1 -
 arch/mips/cavium-octeon/octeon-irq.c | 5 ++++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 771ca53af06d..7b004c5bd796 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -47,7 +47,6 @@ config MIPS
 	select GENERIC_SMP_IDLE_THREAD
 	select GENERIC_TIME_VSYSCALL
 	select GUP_GET_PTE_LOW_HIGH if CPU_MIPS32 && PHYS_ADDR_T_64BIT
-	select HANDLE_DOMAIN_IRQ
 	select HAVE_ARCH_COMPILER_H
 	select HAVE_ARCH_JUMP_LABEL
 	select HAVE_ARCH_KGDB if MIPS_FP_SUPPORT
diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
index be5d4afcd30f..844f882096e6 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -2609,7 +2609,10 @@ static void octeon_irq_ciu3_ip2(void)
 		else
 			hw = intsn;
 
-		ret = handle_domain_irq(domain, hw, NULL);
+		irq_enter();
+		ret = generic_handle_domain_irq(domain, hw);
+		irq_exit();
+
 		if (ret < 0) {
 			union cvmx_ciu3_iscx_w1c isc_w1c;
 			u64 isc_w1c_addr = ciu3_addr + CIU3_ISC_W1C(intsn);
-- 
2.11.0


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

* [PATCH v2 04/17] irq: mips: simplify do_domain_IRQ()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (2 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 03/17] irq: mips: stop (ab)using handle_domain_irq() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 05/17] irq: simplify handle_domain_{irq,nmi}() Mark Rutland
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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


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

* [PATCH v2 05/17] irq: simplify handle_domain_{irq,nmi}()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (3 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 04/17] irq: mips: simplify do_domain_IRQ() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 06/17] irq: unexport handle_irq_desc() Mark Rutland
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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


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

* [PATCH v2 06/17] irq: unexport handle_irq_desc()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (4 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 05/17] irq: simplify handle_domain_{irq,nmi}() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 07/17] irq: add generic_handle_arch_irq() Mark Rutland
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

There are no modular users of handle_irq_desc(). Remove the export
before we gain any.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/irq/irqdesc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index b07d0e1552bc..e25d4bddf3d8 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -646,7 +646,6 @@ int handle_irq_desc(struct irq_desc *desc)
 	generic_handle_irq_desc(desc);
 	return 0;
 }
-EXPORT_SYMBOL_GPL(handle_irq_desc);
 
 /**
  * generic_handle_irq - Invoke the handler for a particular irq
-- 
2.11.0


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

* [PATCH v2 07/17] irq: add generic_handle_arch_irq()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (5 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 06/17] irq: unexport handle_irq_desc() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 08/17] irq: arc: avoid CONFIG_HANDLE_DOMAIN_IRQ Mark Rutland
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 common 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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Guo Ren <guoren@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irq.h |  1 +
 kernel/irq/handle.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index c8293c817646..988c225eef2d 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1261,6 +1261,7 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *));
  * top-level IRQ handler.
  */
 extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+asmlinkage void generic_handle_arch_irq(struct pt_regs *regs);
 #else
 #ifndef set_handle_irq
 #define set_handle_irq(handle_irq)		\
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


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

* [PATCH v2 08/17] irq: arc: avoid CONFIG_HANDLE_DOMAIN_IRQ
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (6 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 07/17] irq: add generic_handle_arch_irq() Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 09/17] irq: nds32: " Mark Rutland
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vineet Gupta <vgupta@kernel.org>
---
 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


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

* [PATCH v2 09/17] irq: nds32: avoid CONFIG_HANDLE_DOMAIN_IRQ
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (7 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 08/17] irq: arc: avoid CONFIG_HANDLE_DOMAIN_IRQ Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 10/17] irq: add a (temporary) CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY Mark Rutland
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Greentime Hu <green.hu@gmail.com>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vincent Chen <deanbo422@gmail.com>
---
 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


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

* [PATCH v2 10/17] irq: add a (temporary) CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (8 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 09/17] irq: nds32: " Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 11/17] irq: arm: perform irqentry in entry code Mark Rutland
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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 e25d4bddf3d8..5677a849cf1f 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -676,6 +676,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
@@ -699,6 +700,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


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

* [PATCH v2 11/17] irq: arm: perform irqentry in entry code
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (9 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 10/17] irq: add a (temporary) CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:24 ` [PATCH v2 12/17] irq: arm64: " Mark Rutland
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Tested-by: Vladimir Murzin <vladimir.murzin@arm.com> # ARMv7M
Cc: Russell King <linux@armlinux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/arm/Kconfig             |  1 -
 arch/arm/kernel/entry-armv.S |  5 +----
 arch/arm/kernel/irq.c        | 14 ++++++++------
 drivers/irqchip/irq-nvic.c   | 17 ++++++++++++++++-
 4 files changed, 25 insertions(+), 12 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..b2bd96253d40 100644
--- a/drivers/irqchip/irq-nvic.c
+++ b/drivers/irqchip/irq-nvic.c
@@ -37,10 +37,25 @@
 
 static struct irq_domain *nvic_irq_domain;
 
+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
 nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
 {
-	handle_domain_irq(nvic_irq_domain, hwirq, 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,
-- 
2.11.0


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

* [PATCH v2 12/17] irq: arm64: perform irqentry in entry code
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (10 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 11/17] irq: arm: perform irqentry in entry code Mark Rutland
@ 2021-10-26  9:24 ` Mark Rutland
  2021-10-26  9:25 ` [PATCH v2 13/17] irq: csky: " Mark Rutland
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Pingfan Liu <kernelfans@gmail.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
---
 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


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

* [PATCH v2 13/17] irq: csky: perform irqentry in entry code
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (11 preceding siblings ...)
  2021-10-26  9:24 ` [PATCH v2 12/17] irq: arm64: " Mark Rutland
@ 2021-10-26  9:25 ` Mark Rutland
  2021-10-26  9:25 ` [PATCH v2 14/17] irq: openrisc: " Mark Rutland
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Guo Ren <guoren@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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


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

* [PATCH v2 14/17] irq: openrisc: perform irqentry in entry code
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (12 preceding siblings ...)
  2021-10-26  9:25 ` [PATCH v2 13/17] irq: csky: " Mark Rutland
@ 2021-10-26  9:25 ` Mark Rutland
  2021-10-26  9:25 ` [PATCH v2 15/17] irq: riscv: " Mark Rutland
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Stafford Horne <shorne@gmail.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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


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

* [PATCH v2 15/17] irq: riscv: perform irqentry in entry code
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (13 preceding siblings ...)
  2021-10-26  9:25 ` [PATCH v2 14/17] irq: openrisc: " Mark Rutland
@ 2021-10-26  9:25 ` Mark Rutland
  2021-10-26  9:25 ` [PATCH v2 16/17] irq: remove CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY Mark Rutland
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Guo Ren <guoren@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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


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

* [PATCH v2 16/17] irq: remove CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (14 preceding siblings ...)
  2021-10-26  9:25 ` [PATCH v2 15/17] irq: riscv: " Mark Rutland
@ 2021-10-26  9:25 ` Mark Rutland
  2021-10-26  9:25 ` [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
  2021-10-26 10:12 ` [PATCH v2 00/17] " Marc Zyngier
  17 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/irq/irqdesc.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 5677a849cf1f..7041698a7bff 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -676,31 +676,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
@@ -728,7 +703,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


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

* [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (15 preceding siblings ...)
  2021-10-26  9:25 ` [PATCH v2 16/17] irq: remove CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY Mark Rutland
@ 2021-10-26  9:25 ` Mark Rutland
  2022-05-06 20:32   ` Lukas Wunner
  2021-10-26 10:12 ` [PATCH v2 00/17] " Marc Zyngier
  17 siblings, 1 reply; 32+ messages in thread
From: Mark Rutland @ 2021-10-26  9:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: aou, catalin.marinas, deanbo422, green.hu, guoren, jonas,
	kernelfans, linux-arm-kernel, linux, mark.rutland, maz, nickhu,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

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 <mark.rutland@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 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                      | 68 ++++++++-----------------------
 48 files changed, 80 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 b2bd96253d40..63bac3f78863 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 @@ 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 7041698a7bff..2267e6527db3 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -651,7 +651,11 @@ int handle_irq_desc(struct irq_desc *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));
@@ -661,77 +665,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
-#endif
 
 /* Dynamic interrupt handling */
 
-- 
2.11.0


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

* Re: [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}()
  2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
                   ` (16 preceding siblings ...)
  2021-10-26  9:25 ` [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
@ 2021-10-26 10:12 ` Marc Zyngier
  17 siblings, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2021-10-26 10:12 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-kernel, aou, catalin.marinas, deanbo422, green.hu, guoren,
	jonas, kernelfans, linux-arm-kernel, linux, nickhu, palmer,
	paul.walmsley, shorne, stefan.kristiansson, tglx, tsbogend,
	vgupta, vladimir.murzin, will, paulmck, peterz, torvalds

On Tue, 26 Oct 2021 10:24:47 +0100,
Mark Rutland <mark.rutland@arm.com> 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).
> 
> Marc, on the assumption you'll take this into the irqchip tree, I've pushed
> this out to my kernel.org repo, tagged as remove-handle-domain-irq-20211026,
> which should be commit:
> 
>   0953fb263714e1c8 ("irq: remove handle_domain_{irq,nmi}() (2021-10-26 10:13:31 +0100)")

I've pulled this into the irq/irqchip-next branch, and it will
hopefully hit -next tomorrow.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2021-10-26  9:25 ` [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
@ 2022-05-06 20:32   ` Lukas Wunner
  2022-05-09  8:54     ` Mark Rutland
  0 siblings, 1 reply; 32+ messages in thread
From: Lukas Wunner @ 2022-05-06 20:32 UTC (permalink / raw)
  To: Mark Rutland, maz, Linus Walleij, Bartosz Golaszewski,
	linux-gpio, Octavian Purdila
  Cc: linux-kernel, aou, catalin.marinas, deanbo422, green.hu, guoren,
	jonas, kernelfans, linux-arm-kernel, linux, nickhu, palmer,
	paul.walmsley, shorne, stefan.kristiansson, tglx, tsbogend,
	vgupta, vladimir.murzin, will

On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland 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.
[...]
>  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);

Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
(See handle_irq_desc() and c16816acd086.)

I believe the above change causes a regression in drivers/gpio/gpio-dln2.c
such that a gratuitous WARN splat is now emitted.

Thanks,

Lukas

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-06 20:32   ` Lukas Wunner
@ 2022-05-09  8:54     ` Mark Rutland
  2022-05-09  9:09       ` Marc Zyngier
  2022-05-10 12:13       ` Lukas Wunner
  0 siblings, 2 replies; 32+ messages in thread
From: Mark Rutland @ 2022-05-09  8:54 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: maz, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
> On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland 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.
> [...]
> >  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);
> 
> Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
> (See handle_irq_desc() and c16816acd086.)

I did this for consistency with the in_nmi() check in
generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
IRQD_HANDLE_ENFORCE_IRQCTX.

I'll have ot leave it to Marc and Thomas as to what we should do there.

> I believe the above change causes a regression in drivers/gpio/gpio-dln2.c
> such that a gratuitous WARN splat is now emitted.

Do you mean you beleive that from inspection, or are you actually seeing this
go wrong in practice?

If it's the latter, are you able to give a copy of the dmesg splat?

Thanks,
Mark.

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-09  8:54     ` Mark Rutland
@ 2022-05-09  9:09       ` Marc Zyngier
  2022-05-09 13:12         ` Thomas Gleixner
  2022-05-10 23:36         ` Thomas Gleixner
  2022-05-10 12:13       ` Lukas Wunner
  1 sibling, 2 replies; 32+ messages in thread
From: Marc Zyngier @ 2022-05-09  9:09 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Lukas Wunner, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

On Mon, 09 May 2022 09:54:21 +0100,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
> > On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland 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.
> > [...]
> > >  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);
> > 
> > Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
> > (See handle_irq_desc() and c16816acd086.)
> 
> I did this for consistency with the in_nmi() check in
> generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
> IRQD_HANDLE_ENFORCE_IRQCTX.
> 
> I'll have ot leave it to Marc and Thomas as to what we should do there.

My preference would be to not introduce things that result in
different behaviours for drivers, specially for things that are
evidently cross-architecture such as USB drivers (which seems to be
the case here).

I'd rather do something that allows these to be handled in the right
context such as a self-IPI. This would certainly work for the GIC. No
idea whether this is valid for x86, which is the other user.

Thanks,

	M.


-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-09  9:09       ` Marc Zyngier
@ 2022-05-09 13:12         ` Thomas Gleixner
  2022-05-10 23:36         ` Thomas Gleixner
  1 sibling, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2022-05-09 13:12 UTC (permalink / raw)
  To: Marc Zyngier, Mark Rutland
  Cc: Lukas Wunner, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson,
	tsbogend, vgupta, vladimir.murzin, will

On Mon, May 09 2022 at 10:09, Marc Zyngier wrote:
> On Mon, 09 May 2022 09:54:21 +0100,
> Mark Rutland <mark.rutland@arm.com> wrote:
>> 
>> On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
>> > On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland wrote:
>> > >  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);
>> > 
>> > Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
>> > (See handle_irq_desc() and c16816acd086.)
>> 
>> I did this for consistency with the in_nmi() check in
>> generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
>> IRQD_HANDLE_ENFORCE_IRQCTX.
>> 
>> I'll have ot leave it to Marc and Thomas as to what we should do there.
>
> My preference would be to not introduce things that result in
> different behaviours for drivers, specially for things that are
> evidently cross-architecture such as USB drivers (which seems to be
> the case here).
>
> I'd rather do something that allows these to be handled in the right
> context such as a self-IPI. This would certainly work for the GIC. No
> idea whether this is valid for x86, which is the other user.

We have to differentiate between interrupts which require hard interrupt
context due to constraints in the hardware, e.g. the horrors of affinity
changes on x86, and interrupts like the dln one which are synthetic. The
latter never go through the regular interrupt entry ASM muck.

Thanks,

        tglx

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-09  8:54     ` Mark Rutland
  2022-05-09  9:09       ` Marc Zyngier
@ 2022-05-10 12:13       ` Lukas Wunner
  2022-05-10 14:15         ` Mark Rutland
  1 sibling, 1 reply; 32+ messages in thread
From: Lukas Wunner @ 2022-05-10 12:13 UTC (permalink / raw)
  To: Mark Rutland
  Cc: maz, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

On Mon, May 09, 2022 at 09:54:21AM +0100, Mark Rutland wrote:
> On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
> > On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland wrote:
> > >  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);
> > 
> > Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
> > (See handle_irq_desc() and c16816acd086.)
> 
> I did this for consistency with the in_nmi() check in
> generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
> IRQD_HANDLE_ENFORCE_IRQCTX.

Actually, since you're mentioning the in_nmi() check, I suspect
there's another problem here:

generic_handle_domain_nmi() warns if !in_nmi(), then calls down 
to handle_irq_desc() which warns if !in_hardirq().  Doesn't this
cause a false-positive !in_hardirq() warning for a NMI on GIC/GICv3?

The only driver calling request_nmi() or request_percpu_nmi() is
drivers/perf/arm_pmu.c.  So that's the only one affected.
You may want to test if that driver indeed exhibits such a
false-positive warning since c16816acd086.


> > I believe the above change causes a regression in drivers/gpio/gpio-dln2.c
> > such that a gratuitous WARN splat is now emitted.
> 
> Do you mean you beleive that from inspection, or are you actually seeing this
> go wrong in practice?
> 
> If it's the latter, are you able to give a copy of the dmesg splat?

For gpio-dln2.c, I believe it from inspection.

For smsc95xx.c, I'm actually seeing it go wrong in practice,
unedited dmesg splat is included below FWIW.

That's with the following series applied on top of current net-next:
https://lore.kernel.org/netdev/cover.1651574194.git.lukas@wunner.de/

You need to remove the __irq_enter_raw() in patch [5/7] of that series
to actually see the WARN splat.  I used it to work around your
WARN_ON_ONCE() and that's what Marc took exception to.

With gpio-dln2.c, the call stack looks like this:

  dln2_rx()                         # drivers/mfd/dln2.c
    dln2_run_event_callbacks()
      dln2_gpio_event()             # drivers/gpio/gpio-dln2.c
        generic_handle_domain_irq()

That's basically the same as the callstack for smsc95xx.c below.
Interrupts are disabled in dln2_rx() via spin_lock_irqsave(),
but there isn't an __irq_enter_raw() anywhere to be seen
and that would be necessary to avoid the false-positive
WARN splat in generic_handle_domain_irq().

Thanks,

Lukas

-- >8 --

[ 1227.718928] WARNING: CPU: 3 PID: 75 at kernel/irq/irqdesc.c:702 generic_handle_domain_irq+0x88/0x94
[ 1227.718951] Modules linked in: sha256_generic cfg80211 rfkill 8021q garp stp llc ax88796b asix raspberrypi_hwmon bcm2835_codec(C) bcm2835_v4l2(C) bcm2835_isp(C) v4l2_mem2mem bcm2835_mmal_vchiq(C) snd_bcm2835(C) vc_sm_cma(C) videobuf2_dma_contig videobuf2_vmalloc snd_pcm videobuf2_memops videobuf2_v4l2 videobuf2_common snd_timer snd videodev mc micrel ks8851_spi ks8851_common uio_pdrv_genirq uio eeprom_93cx6 piControl(O) ad5446 ti_dac082s085 mcp320x iio_mux mux_gpio mux_core fixed gpio_74x164 spi_bcm2835aux spi_bcm2835 gpio_max3191x crc8 industrialio i2c_dev ip_tables x_tables ipv6
[ 1227.719076] CPU: 3 PID: 75 Comm: irq/89-dwc_otg_ Tainted: G         C O      5.17.0-rt15-v7+ #2
[ 1227.719084] Hardware name: BCM2835
[ 1227.719087] Backtrace: 
[ 1227.719091]  dump_backtrace from show_stack+0x20/0x24
[ 1227.719106]  r7:00000009 r6:00000080 r5:80d25844 r4:60000093
[ 1227.719109]  show_stack from dump_stack_lvl+0x74/0x9c
[ 1227.719120]  dump_stack_lvl from dump_stack+0x18/0x1c
[ 1227.719134]  r7:00000009 r6:801957f4 r5:000002be r4:80d1e8ac
[ 1227.719137]  dump_stack from __warn+0xdc/0x190
[ 1227.719148]  __warn from warn_slowpath_fmt+0x70/0xcc
[ 1227.719161]  r8:00000009 r7:801957f4 r6:000002be r5:80d1e8ac r4:00000000
[ 1227.719163]  warn_slowpath_fmt from generic_handle_domain_irq+0x88/0x94
[ 1227.719177]  r8:81e87600 r7:00000000 r6:81d28000 r5:00000000 r4:81d4be00
[ 1227.719179]  generic_handle_domain_irq from smsc95xx_status+0x54/0xb0
[ 1227.719195]  r7:00000000 r6:00008000 r5:81f28640 r4:60000013
[ 1227.719197]  smsc95xx_status from intr_complete+0x80/0x84
[ 1227.719213]  r9:81d40a00 r8:00000001 r7:00000000 r6:00000000 r5:81f28640 r4:81d4bd80
[ 1227.719216]  intr_complete from __usb_hcd_giveback_urb+0xa4/0x12c
[ 1227.719229]  r5:815b3000 r4:81d4bd80
[ 1227.719232]  __usb_hcd_giveback_urb from usb_hcd_giveback_urb+0x118/0x11c
[ 1227.719245]  r7:811498e0 r6:81d4bd80 r5:81682000 r4:815b3000
[ 1227.719248]  usb_hcd_giveback_urb from completion_tasklet_func+0x7c/0xc8
[ 1227.719262]  r7:811498e0 r6:81d4bd80 r5:81682000 r4:8639f800
[ 1227.719264]  completion_tasklet_func from tasklet_callback+0x20/0x24
[ 1227.719277]  r6:80f8d140 r5:b6b5c330 r4:00000000
[ 1227.719279]  tasklet_callback from tasklet_action_common.constprop.0+0x148/0x220
[ 1227.719288]  tasklet_action_common.constprop.0 from tasklet_hi_action+0x28/0x30
[ 1227.719301]  r10:81d28000 r9:00000001 r8:00000000 r7:811498e0 r6:00000000 r5:00000001
[ 1227.719304]  r4:81003080
[ 1227.719306]  tasklet_hi_action from __do_softirq+0x154/0x3e8
[ 1227.719316]  __do_softirq from __local_bh_enable_ip+0x12c/0x1a8
[ 1227.719328]  r10:801970b0 r9:80f85320 r8:00000001 r7:00000000 r6:00000001 r5:00000100
[ 1227.719332]  r4:40000013
[ 1227.719334]  __local_bh_enable_ip from irq_forced_thread_fn+0x7c/0xac
[ 1227.719347]  r9:81d40ac0 r8:814f3c00 r7:00000001 r6:00000001 r5:814f3c00 r4:81d40ac0
[ 1227.719350]  irq_forced_thread_fn from irq_thread+0x16c/0x228
[ 1227.719363]  r7:00000001 r6:81d40ae4 r5:81d28000 r4:00000000
[ 1227.719365]  irq_thread from kthread+0x100/0x140
[ 1227.719380]  r10:00000000 r9:8154fbfc r8:81d43000 r7:81d40ac0 r6:80196dcc r5:81d40b00
[ 1227.719383]  r4:81d28000
[ 1227.719386]  kthread from ret_from_fork+0x14/0x34
[ 1227.719394] Exception stack(0x81777fb0 to 0x81777ff8)
[ 1227.719401] 7fa0:                                     00000000 00000000 00000000 00000000
[ 1227.719408] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 1227.719414] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 1227.719422]  r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:8014a6c8 r4:81d40b00
[ 1227.719425] ---[ end trace 0000000000000000 ]---

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-10 12:13       ` Lukas Wunner
@ 2022-05-10 14:15         ` Mark Rutland
  2022-05-10 22:52           ` Thomas Gleixner
  2022-05-11  0:11           ` Thomas Gleixner
  0 siblings, 2 replies; 32+ messages in thread
From: Mark Rutland @ 2022-05-10 14:15 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: maz, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will

On Tue, May 10, 2022 at 02:13:20PM +0200, Lukas Wunner wrote:
> On Mon, May 09, 2022 at 09:54:21AM +0100, Mark Rutland wrote:
> > On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
> > > On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland wrote:
> > > >  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);
> > > 
> > > Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
> > > (See handle_irq_desc() and c16816acd086.)
> > 
> > I did this for consistency with the in_nmi() check in
> > generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
> > IRQD_HANDLE_ENFORCE_IRQCTX.
> 
> Actually, since you're mentioning the in_nmi() check, I suspect
> there's another problem here:
> 
> generic_handle_domain_nmi() warns if !in_nmi(), then calls down 
> to handle_irq_desc() which warns if !in_hardirq().  Doesn't this
> cause a false-positive !in_hardirq() warning for a NMI on GIC/GICv3?

I agree that doesn't look right.

> The only driver calling request_nmi() or request_percpu_nmi() is
> drivers/perf/arm_pmu.c.  So that's the only one affected.
> You may want to test if that driver indeed exhibits such a
> false-positive warning since c16816acd086.

In testing with v5.18-rc5, I can't see that going wrong.

I also hacked the following in:

-------->8--------
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 939d21cd55c38..3c85608a8779f 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -718,6 +718,7 @@ EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
 int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
 {
        WARN_ON_ONCE(!in_nmi());
+       WARN_ON_ONCE(!in_hardirq());
        return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
 }
 #endif
-------->8--------

... and that never fired, which doesn't seem right.

I also see some other problems which may or may not be related (lockdep
tracking gone wrong, which could indicate a more general failure to track irq
state correctly). I'll go dig into that...

> > > I believe the above change causes a regression in drivers/gpio/gpio-dln2.c
> > > such that a gratuitous WARN splat is now emitted.
> > 
> > Do you mean you beleive that from inspection, or are you actually seeing this
> > go wrong in practice?
> > 
> > If it's the latter, are you able to give a copy of the dmesg splat?
> 
> For gpio-dln2.c, I believe it from inspection.
> 
> For smsc95xx.c, I'm actually seeing it go wrong in practice,
> unedited dmesg splat is included below FWIW.

Thanks; having the trace makes this much easier to analyse.

Mark.

> That's with the following series applied on top of current net-next:
> https://lore.kernel.org/netdev/cover.1651574194.git.lukas@wunner.de/
> 
> You need to remove the __irq_enter_raw() in patch [5/7] of that series
> to actually see the WARN splat.  I used it to work around your
> WARN_ON_ONCE() and that's what Marc took exception to.
> 
> With gpio-dln2.c, the call stack looks like this:
> 
>   dln2_rx()                         # drivers/mfd/dln2.c
>     dln2_run_event_callbacks()
>       dln2_gpio_event()             # drivers/gpio/gpio-dln2.c
>         generic_handle_domain_irq()
> 
> That's basically the same as the callstack for smsc95xx.c below.
> Interrupts are disabled in dln2_rx() via spin_lock_irqsave(),
> but there isn't an __irq_enter_raw() anywhere to be seen
> and that would be necessary to avoid the false-positive
> WARN splat in generic_handle_domain_irq().
> 
> Thanks,
> 
> Lukas
> 
> -- >8 --
> 
> [ 1227.718928] WARNING: CPU: 3 PID: 75 at kernel/irq/irqdesc.c:702 generic_handle_domain_irq+0x88/0x94
> [ 1227.718951] Modules linked in: sha256_generic cfg80211 rfkill 8021q garp stp llc ax88796b asix raspberrypi_hwmon bcm2835_codec(C) bcm2835_v4l2(C) bcm2835_isp(C) v4l2_mem2mem bcm2835_mmal_vchiq(C) snd_bcm2835(C) vc_sm_cma(C) videobuf2_dma_contig videobuf2_vmalloc snd_pcm videobuf2_memops videobuf2_v4l2 videobuf2_common snd_timer snd videodev mc micrel ks8851_spi ks8851_common uio_pdrv_genirq uio eeprom_93cx6 piControl(O) ad5446 ti_dac082s085 mcp320x iio_mux mux_gpio mux_core fixed gpio_74x164 spi_bcm2835aux spi_bcm2835 gpio_max3191x crc8 industrialio i2c_dev ip_tables x_tables ipv6
> [ 1227.719076] CPU: 3 PID: 75 Comm: irq/89-dwc_otg_ Tainted: G         C O      5.17.0-rt15-v7+ #2
> [ 1227.719084] Hardware name: BCM2835
> [ 1227.719087] Backtrace: 
> [ 1227.719091]  dump_backtrace from show_stack+0x20/0x24
> [ 1227.719106]  r7:00000009 r6:00000080 r5:80d25844 r4:60000093
> [ 1227.719109]  show_stack from dump_stack_lvl+0x74/0x9c
> [ 1227.719120]  dump_stack_lvl from dump_stack+0x18/0x1c
> [ 1227.719134]  r7:00000009 r6:801957f4 r5:000002be r4:80d1e8ac
> [ 1227.719137]  dump_stack from __warn+0xdc/0x190
> [ 1227.719148]  __warn from warn_slowpath_fmt+0x70/0xcc
> [ 1227.719161]  r8:00000009 r7:801957f4 r6:000002be r5:80d1e8ac r4:00000000
> [ 1227.719163]  warn_slowpath_fmt from generic_handle_domain_irq+0x88/0x94
> [ 1227.719177]  r8:81e87600 r7:00000000 r6:81d28000 r5:00000000 r4:81d4be00
> [ 1227.719179]  generic_handle_domain_irq from smsc95xx_status+0x54/0xb0
> [ 1227.719195]  r7:00000000 r6:00008000 r5:81f28640 r4:60000013
> [ 1227.719197]  smsc95xx_status from intr_complete+0x80/0x84
> [ 1227.719213]  r9:81d40a00 r8:00000001 r7:00000000 r6:00000000 r5:81f28640 r4:81d4bd80
> [ 1227.719216]  intr_complete from __usb_hcd_giveback_urb+0xa4/0x12c
> [ 1227.719229]  r5:815b3000 r4:81d4bd80
> [ 1227.719232]  __usb_hcd_giveback_urb from usb_hcd_giveback_urb+0x118/0x11c
> [ 1227.719245]  r7:811498e0 r6:81d4bd80 r5:81682000 r4:815b3000
> [ 1227.719248]  usb_hcd_giveback_urb from completion_tasklet_func+0x7c/0xc8
> [ 1227.719262]  r7:811498e0 r6:81d4bd80 r5:81682000 r4:8639f800
> [ 1227.719264]  completion_tasklet_func from tasklet_callback+0x20/0x24
> [ 1227.719277]  r6:80f8d140 r5:b6b5c330 r4:00000000
> [ 1227.719279]  tasklet_callback from tasklet_action_common.constprop.0+0x148/0x220
> [ 1227.719288]  tasklet_action_common.constprop.0 from tasklet_hi_action+0x28/0x30
> [ 1227.719301]  r10:81d28000 r9:00000001 r8:00000000 r7:811498e0 r6:00000000 r5:00000001
> [ 1227.719304]  r4:81003080
> [ 1227.719306]  tasklet_hi_action from __do_softirq+0x154/0x3e8
> [ 1227.719316]  __do_softirq from __local_bh_enable_ip+0x12c/0x1a8
> [ 1227.719328]  r10:801970b0 r9:80f85320 r8:00000001 r7:00000000 r6:00000001 r5:00000100
> [ 1227.719332]  r4:40000013
> [ 1227.719334]  __local_bh_enable_ip from irq_forced_thread_fn+0x7c/0xac
> [ 1227.719347]  r9:81d40ac0 r8:814f3c00 r7:00000001 r6:00000001 r5:814f3c00 r4:81d40ac0
> [ 1227.719350]  irq_forced_thread_fn from irq_thread+0x16c/0x228
> [ 1227.719363]  r7:00000001 r6:81d40ae4 r5:81d28000 r4:00000000
> [ 1227.719365]  irq_thread from kthread+0x100/0x140
> [ 1227.719380]  r10:00000000 r9:8154fbfc r8:81d43000 r7:81d40ac0 r6:80196dcc r5:81d40b00
> [ 1227.719383]  r4:81d28000
> [ 1227.719386]  kthread from ret_from_fork+0x14/0x34
> [ 1227.719394] Exception stack(0x81777fb0 to 0x81777ff8)
> [ 1227.719401] 7fa0:                                     00000000 00000000 00000000 00000000
> [ 1227.719408] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 1227.719414] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [ 1227.719422]  r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:8014a6c8 r4:81d40b00
> [ 1227.719425] ---[ end trace 0000000000000000 ]---

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-10 14:15         ` Mark Rutland
@ 2022-05-10 22:52           ` Thomas Gleixner
  2022-05-11  8:23             ` Mark Rutland
  2022-05-11  0:11           ` Thomas Gleixner
  1 sibling, 1 reply; 32+ messages in thread
From: Thomas Gleixner @ 2022-05-10 22:52 UTC (permalink / raw)
  To: Mark Rutland, Lukas Wunner
  Cc: maz, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson,
	tsbogend, vgupta, vladimir.murzin, will, Peter Zijlstra

On Tue, May 10 2022 at 15:15, Mark Rutland wrote:
> On Tue, May 10, 2022 at 02:13:20PM +0200, Lukas Wunner wrote:
>> For gpio-dln2.c, I believe it from inspection.
>> 
>> For smsc95xx.c, I'm actually seeing it go wrong in practice,
>> unedited dmesg splat is included below FWIW.
>
> Thanks; having the trace makes this much easier to analyse.

which confirmes what I talked about before:

>> WARNING: CPU: 3 PID: 75 at kernel/irq/irqdesc.c:702 generic_handle_domain_irq+0x88/0x94
>>  generic_handle_domain_irq from smsc95xx_status+0x54/0xb0
>>  smsc95xx_status from intr_complete+0x80/0x84
>>  intr_complete from __usb_hcd_giveback_urb+0xa4/0x12c
>>  __usb_hcd_giveback_urb from usb_hcd_giveback_urb+0x118/0x11c
>>  usb_hcd_giveback_urb from completion_tasklet_func+0x7c/0xc8
>>  completion_tasklet_func from tasklet_callback+0x20/0x24
>>  tasklet_callback from tasklet_action_common.constprop.0+0x148/0x220
>>  tasklet_action_common.constprop.0 from tasklet_hi_action+0x28/0x30
>>  tasklet_hi_action from __do_softirq+0x154/0x3e8
>>  __do_softirq from __local_bh_enable_ip+0x12c/0x1a8
>>  __local_bh_enable_ip from irq_forced_thread_fn+0x7c/0xac
>>  irq_forced_thread_fn from irq_thread+0x16c/0x228
>>  irq_thread from kthread+0x100/0x140

So what happens here:

 interrupt
    -> wakeup threaded handler

 threaded handler runs
    local_bh_disable();
    ....
    schedules tasklet
    ...
    local_bh_enable()
      do_softirq()
        run_tasklet()
          urb_completion()
            smsc95xx_status()
              generic_handle_domain_irq()

That interrupt in question is an interrupt, which is not handled by the
primary CPU interrupt chips. It's a synthetic interrupt which is
generated from the received USB packet.

+	/* USB interrupts are received in softirq (tasklet) context.
+	 * Switch to hardirq context to make genirq code happy.
+	 */
+	local_irq_save(flags);
+	__irq_enter_raw();
+
 	if (intdata & INT_ENP_PHY_INT_)
-		;
+		generic_handle_domain_irq(pdata->irqdomain, PHY_HWIRQ);

This __irq_enter_raw() is really wrong. This is _not_ running in hard
interrupt context. Pretending so creates more problems than it
solves. It breaks context tracking, confuses lockdep ...

We also have demultiplexed interrupts which are nested in a threaded
interrupt handler and share the thread context. No, we are not going to
pretend that they run in hard interrupt context either.

So we need a clear distinction between interrupts which really happen in
hard interrupt context and those which are synthetic and can be invoked
from pretty much any context.

Anything else is just a recipe for disaster and endless supply of half
baken hacks.

Thanks,

        tglx

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-09  9:09       ` Marc Zyngier
  2022-05-09 13:12         ` Thomas Gleixner
@ 2022-05-10 23:36         ` Thomas Gleixner
  1 sibling, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2022-05-10 23:36 UTC (permalink / raw)
  To: Marc Zyngier, Mark Rutland
  Cc: Lukas Wunner, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson,
	tsbogend, vgupta, vladimir.murzin, will

On Mon, May 09 2022 at 10:09, Marc Zyngier wrote:
> On Mon, 09 May 2022 09:54:21 +0100,
> Mark Rutland <mark.rutland@arm.com> wrote:
>> > Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
>> > (See handle_irq_desc() and c16816acd086.)
>> 
>> I did this for consistency with the in_nmi() check in
>> generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
>> IRQD_HANDLE_ENFORCE_IRQCTX.
>> 
>> I'll have ot leave it to Marc and Thomas as to what we should do there.
>
> My preference would be to not introduce things that result in
> different behaviours for drivers, specially for things that are
> evidently cross-architecture such as USB drivers (which seems to be
> the case here).

No. USB drivers which synthesize their interrupts from a received packet
work perfectly fine on all architectures because the interrupt domain
and interrupt chip they are using are a software construct designed for
the purpose and have no hard interrupt context requirements.

The reason why this is done is to leverage the interrupt driven PHY
status changes instead of enforcing timer based polling. The charm is
that the phy code does not have to grow another wart and just uses the
offered synthetic interrupt. There are other places which do similar
things for the same reason. This also provides the beloved statistics in
/proc/interrupt, tracepoints etc. out of the box without having to add
extra muck into yet another subsystem.

> I'd rather do something that allows these to be handled in the right
> context such as a self-IPI. This would certainly work for the GIC. No
> idea whether this is valid for x86, which is the other user.

This interrupt is neither directly nor indirectly connected to GIC
or APIC. It's synthesized. So what would the self-IPI help?

And no, I don't want to create infrastructure to allocate a pseudo
device vector on x86 just to be able to self-IPI this USB synthesized
interrupt. That'd be yet another horrorshow and worse a horrorshow for
no reason and zero value.

IRQD_HANDLE_ENFORCE_IRQCTX was introduced to be able to differentiate
between interrupt chips which require a particular context and chips
which can handle it perfectly fine to have e.g. their software retrigger
handled by directly invoking the handler from an arbitrary context.

I wish this would be the case on x86. That would eliminate a boatload of
horrible code.

Thanks,

        tglx





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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-10 14:15         ` Mark Rutland
  2022-05-10 22:52           ` Thomas Gleixner
@ 2022-05-11  0:11           ` Thomas Gleixner
  2022-05-11  8:11             ` Mark Rutland
  1 sibling, 1 reply; 32+ messages in thread
From: Thomas Gleixner @ 2022-05-11  0:11 UTC (permalink / raw)
  To: Mark Rutland, Lukas Wunner
  Cc: maz, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	palmer, paul.walmsley, shorne, stefan.kristiansson, tsbogend,
	vgupta, vladimir.murzin, will

On Tue, May 10 2022 at 15:15, Mark Rutland wrote:
> On Tue, May 10, 2022 at 02:13:20PM +0200, Lukas Wunner wrote:
>> Actually, since you're mentioning the in_nmi() check, I suspect
>> there's another problem here:
>> 
>> generic_handle_domain_nmi() warns if !in_nmi(), then calls down 
>> to handle_irq_desc() which warns if !in_hardirq().  Doesn't this
>> cause a false-positive !in_hardirq() warning for a NMI on GIC/GICv3?
>
> I agree that doesn't look right.
>
>> The only driver calling request_nmi() or request_percpu_nmi() is
>> drivers/perf/arm_pmu.c.  So that's the only one affected.
>> You may want to test if that driver indeed exhibits such a
>> false-positive warning since c16816acd086.
>
> In testing with v5.18-rc5, I can't see that going wrong.
>
> I also hacked the following in:
>
> -------->8--------
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index 939d21cd55c38..3c85608a8779f 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -718,6 +718,7 @@ EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
>  int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
>  {
>         WARN_ON_ONCE(!in_nmi());
> +       WARN_ON_ONCE(!in_hardirq());
>         return handle_irq_desc(irq_resolve_mapping(domain, hwirq));

which is pointless because NMI entry code has to invoke [__]nmi_enter()
before invoking this function. [__]nmi_enter() does:

    __preempt_count_add(NMI_OFFSET + HARDIRQ_OFFSET);

So it's more than bloody obvious why there is no warning triggered for a
regular hardware induced NMI invocation.

For a software invocation from the wrong context it does not matter how
many redundant WARN_ONs you add. The existing ones are covering it
nicely already.

Thanks,

        tglx

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-11  0:11           ` Thomas Gleixner
@ 2022-05-11  8:11             ` Mark Rutland
  0 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2022-05-11  8:11 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Lukas Wunner, maz, Linus Walleij, Bartosz Golaszewski,
	linux-gpio, Octavian Purdila, linux-kernel, aou, catalin.marinas,
	deanbo422, green.hu, guoren, jonas, kernelfans, linux-arm-kernel,
	linux, palmer, paul.walmsley, shorne, stefan.kristiansson,
	tsbogend, vgupta, vladimir.murzin, will

On Wed, May 11, 2022 at 02:11:52AM +0200, Thomas Gleixner wrote:
> On Tue, May 10 2022 at 15:15, Mark Rutland wrote:
> > On Tue, May 10, 2022 at 02:13:20PM +0200, Lukas Wunner wrote:
> >> Actually, since you're mentioning the in_nmi() check, I suspect
> >> there's another problem here:
> >> 
> >> generic_handle_domain_nmi() warns if !in_nmi(), then calls down 
> >> to handle_irq_desc() which warns if !in_hardirq().  Doesn't this
> >> cause a false-positive !in_hardirq() warning for a NMI on GIC/GICv3?
> >
> > I agree that doesn't look right.
> >
> >> The only driver calling request_nmi() or request_percpu_nmi() is
> >> drivers/perf/arm_pmu.c.  So that's the only one affected.
> >> You may want to test if that driver indeed exhibits such a
> >> false-positive warning since c16816acd086.
> >
> > In testing with v5.18-rc5, I can't see that going wrong.
> >
> > I also hacked the following in:
> >
> > -------->8--------
> > diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> > index 939d21cd55c38..3c85608a8779f 100644
> > --- a/kernel/irq/irqdesc.c
> > +++ b/kernel/irq/irqdesc.c
> > @@ -718,6 +718,7 @@ EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
> >  int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
> >  {
> >         WARN_ON_ONCE(!in_nmi());
> > +       WARN_ON_ONCE(!in_hardirq());
> >         return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
> 
> which is pointless because NMI entry code has to invoke [__]nmi_enter()
> before invoking this function. [__]nmi_enter() does:
> 
>     __preempt_count_add(NMI_OFFSET + HARDIRQ_OFFSET);
> 
> So it's more than bloody obvious why there is no warning triggered for a
> regular hardware induced NMI invocation.

Ugh, yes; clearly I need new eyes and/or more sleep. I entirely missed that we
treat an NMI as *also* being a hardirq rather than something completely
independent, and that means that this is *not* a problem for NMI.

Thanks for pointing that out!

> For a software invocation from the wrong context it does not matter how
> many redundant WARN_ONs you add. The existing ones are covering it
> nicely already.

Yup; as above I was clearly not thinknig straight here.

Mark.

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-10 22:52           ` Thomas Gleixner
@ 2022-05-11  8:23             ` Mark Rutland
  2022-05-11  8:57               ` Lukas Wunner
  0 siblings, 1 reply; 32+ messages in thread
From: Mark Rutland @ 2022-05-11  8:23 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Lukas Wunner, maz, Linus Walleij, Bartosz Golaszewski,
	linux-gpio, Octavian Purdila, linux-kernel, aou, catalin.marinas,
	deanbo422, green.hu, guoren, jonas, kernelfans, linux-arm-kernel,
	linux, nickhu, palmer, paul.walmsley, shorne,
	stefan.kristiansson, tsbogend, vgupta, vladimir.murzin, will,
	Peter Zijlstra

On Wed, May 11, 2022 at 12:52:29AM +0200, Thomas Gleixner wrote:
> On Tue, May 10 2022 at 15:15, Mark Rutland wrote:
> > On Tue, May 10, 2022 at 02:13:20PM +0200, Lukas Wunner wrote:
> >> For gpio-dln2.c, I believe it from inspection.
> >> 
> >> For smsc95xx.c, I'm actually seeing it go wrong in practice,
> >> unedited dmesg splat is included below FWIW.
> >
> > Thanks; having the trace makes this much easier to analyse.
> 
> which confirmes what I talked about before:
> 
> >> WARNING: CPU: 3 PID: 75 at kernel/irq/irqdesc.c:702 generic_handle_domain_irq+0x88/0x94
> >>  generic_handle_domain_irq from smsc95xx_status+0x54/0xb0
> >>  smsc95xx_status from intr_complete+0x80/0x84
> >>  intr_complete from __usb_hcd_giveback_urb+0xa4/0x12c
> >>  __usb_hcd_giveback_urb from usb_hcd_giveback_urb+0x118/0x11c
> >>  usb_hcd_giveback_urb from completion_tasklet_func+0x7c/0xc8
> >>  completion_tasklet_func from tasklet_callback+0x20/0x24
> >>  tasklet_callback from tasklet_action_common.constprop.0+0x148/0x220
> >>  tasklet_action_common.constprop.0 from tasklet_hi_action+0x28/0x30
> >>  tasklet_hi_action from __do_softirq+0x154/0x3e8
> >>  __do_softirq from __local_bh_enable_ip+0x12c/0x1a8
> >>  __local_bh_enable_ip from irq_forced_thread_fn+0x7c/0xac
> >>  irq_forced_thread_fn from irq_thread+0x16c/0x228
> >>  irq_thread from kthread+0x100/0x140
> 
> So what happens here:
> 
>  interrupt
>     -> wakeup threaded handler
> 
>  threaded handler runs
>     local_bh_disable();
>     ....
>     schedules tasklet
>     ...
>     local_bh_enable()
>       do_softirq()
>         run_tasklet()
>           urb_completion()
>             smsc95xx_status()
>               generic_handle_domain_irq()
> 
> That interrupt in question is an interrupt, which is not handled by the
> primary CPU interrupt chips. It's a synthetic interrupt which is
> generated from the received USB packet.
> 
> +	/* USB interrupts are received in softirq (tasklet) context.
> +	 * Switch to hardirq context to make genirq code happy.
> +	 */
> +	local_irq_save(flags);
> +	__irq_enter_raw();
> +
>  	if (intdata & INT_ENP_PHY_INT_)
> -		;
> +		generic_handle_domain_irq(pdata->irqdomain, PHY_HWIRQ);
> 
> This __irq_enter_raw() is really wrong. This is _not_ running in hard
> interrupt context. Pretending so creates more problems than it
> solves. It breaks context tracking, confuses lockdep ...
> 
> We also have demultiplexed interrupts which are nested in a threaded
> interrupt handler and share the thread context. No, we are not going to
> pretend that they run in hard interrupt context either.
> 
> So we need a clear distinction between interrupts which really happen in
> hard interrupt context and those which are synthetic and can be invoked
> from pretty much any context.
> 
> Anything else is just a recipe for disaster and endless supply of half
> baken hacks.

Agreed. IIUC everyone agrees the __irq_enter_raw() usage is a hack, but what's
not clear is what we *should* do -- sorry if I'm being thick here.

I suspect that given we have generic_handle_irq_safe() for situations like this
we should add a generic_handle_domain_irq_safe(), and use that in this driver?
That way we can keep the `WARN_ON_ONCE(!in_hardirq())` in
generic_handle_domain_irq().

... or do you think we should do something else entirely?

Thanks,
Mark.

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-11  8:23             ` Mark Rutland
@ 2022-05-11  8:57               ` Lukas Wunner
  2022-05-11  9:27                 ` Mark Rutland
  0 siblings, 1 reply; 32+ messages in thread
From: Lukas Wunner @ 2022-05-11  8:57 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Thomas Gleixner, maz, Linus Walleij, Bartosz Golaszewski,
	linux-gpio, Octavian Purdila, linux-kernel, aou, catalin.marinas,
	deanbo422, green.hu, guoren, jonas, kernelfans, linux-arm-kernel,
	linux, nickhu, palmer, paul.walmsley, shorne,
	stefan.kristiansson, tsbogend, vgupta, vladimir.murzin, will,
	Peter Zijlstra

On Wed, May 11, 2022 at 09:23:56AM +0100, Mark Rutland wrote:
> On Wed, May 11, 2022 at 12:52:29AM +0200, Thomas Gleixner wrote:
> > +	/* USB interrupts are received in softirq (tasklet) context.
> > +	 * Switch to hardirq context to make genirq code happy.
> > +	 */
> > +	local_irq_save(flags);
> > +	__irq_enter_raw();
> > +
> >  	if (intdata & INT_ENP_PHY_INT_)
> > -		;
> > +		generic_handle_domain_irq(pdata->irqdomain, PHY_HWIRQ);
> 
> Agreed. IIUC everyone agrees the __irq_enter_raw() usage is a hack,
> but what's not clear is what we *should* do
> 
> I suspect that given we have generic_handle_irq_safe() for situations
> like this we should add a generic_handle_domain_irq_safe(), and use
> that in this driver?
> That way we can keep the `WARN_ON_ONCE(!in_hardirq())` in
> generic_handle_domain_irq().

Thomas applied 792ea6a074ae ("genirq: Remove WARN_ON_ONCE() in
generic_handle_domain_irq()") tonight:

http://git.kernel.org/tip/tip/c/792ea6a074ae

That allows me to drop the controversial __irq_enter_raw()
and thus unblock my smsc95xx series.

generic_handle_domain_irq_safe() would merely be a wrapper for
generic_handle_domain_irq() which disables local interrupts.
Then I wouldn't have to do that in smsc95xx.c.  IMHO that's a
cosmetic improvement, though I'll be happy to provide a patch
if desired?

Thanks,

Lukas

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

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
  2022-05-11  8:57               ` Lukas Wunner
@ 2022-05-11  9:27                 ` Mark Rutland
  0 siblings, 0 replies; 32+ messages in thread
From: Mark Rutland @ 2022-05-11  9:27 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Thomas Gleixner, maz, Linus Walleij, Bartosz Golaszewski,
	linux-gpio, Octavian Purdila, linux-kernel, aou, catalin.marinas,
	deanbo422, green.hu, guoren, jonas, kernelfans, linux-arm-kernel,
	linux, nickhu, palmer, paul.walmsley, shorne,
	stefan.kristiansson, tsbogend, vgupta, vladimir.murzin, will,
	Peter Zijlstra

On Wed, May 11, 2022 at 10:57:41AM +0200, Lukas Wunner wrote:
> On Wed, May 11, 2022 at 09:23:56AM +0100, Mark Rutland wrote:
> > On Wed, May 11, 2022 at 12:52:29AM +0200, Thomas Gleixner wrote:
> > > +	/* USB interrupts are received in softirq (tasklet) context.
> > > +	 * Switch to hardirq context to make genirq code happy.
> > > +	 */
> > > +	local_irq_save(flags);
> > > +	__irq_enter_raw();
> > > +
> > >  	if (intdata & INT_ENP_PHY_INT_)
> > > -		;
> > > +		generic_handle_domain_irq(pdata->irqdomain, PHY_HWIRQ);
> > 
> > Agreed. IIUC everyone agrees the __irq_enter_raw() usage is a hack,
> > but what's not clear is what we *should* do
> > 
> > I suspect that given we have generic_handle_irq_safe() for situations
> > like this we should add a generic_handle_domain_irq_safe(), and use
> > that in this driver?
> > That way we can keep the `WARN_ON_ONCE(!in_hardirq())` in
> > generic_handle_domain_irq().
> 
> Thomas applied 792ea6a074ae ("genirq: Remove WARN_ON_ONCE() in
> generic_handle_domain_irq()") tonight:
> 
> http://git.kernel.org/tip/tip/c/792ea6a074ae

Ah; I missed that. Sorry for the noise!

> That allows me to drop the controversial __irq_enter_raw()
> and thus unblock my smsc95xx series.
> 
> generic_handle_domain_irq_safe() would merely be a wrapper for
> generic_handle_domain_irq() which disables local interrupts.
> Then I wouldn't have to do that in smsc95xx.c.  IMHO that's a
> cosmetic improvement, though I'll be happy to provide a patch
> if desired?

I think it's a nice-to-have, but I don't have a strong feelings about it, so I
think we can forget about it for now unless Marc or Thomas want it.

Thanks,
Mark.

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

end of thread, other threads:[~2022-05-11  9:28 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26  9:24 [PATCH v2 00/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 01/17] irq: mips: avoid nested irq_enter() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 02/17] irq: mips: simplify bcm6345_l1_irq_handle() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 03/17] irq: mips: stop (ab)using handle_domain_irq() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 04/17] irq: mips: simplify do_domain_IRQ() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 05/17] irq: simplify handle_domain_{irq,nmi}() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 06/17] irq: unexport handle_irq_desc() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 07/17] irq: add generic_handle_arch_irq() Mark Rutland
2021-10-26  9:24 ` [PATCH v2 08/17] irq: arc: avoid CONFIG_HANDLE_DOMAIN_IRQ Mark Rutland
2021-10-26  9:24 ` [PATCH v2 09/17] irq: nds32: " Mark Rutland
2021-10-26  9:24 ` [PATCH v2 10/17] irq: add a (temporary) CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY Mark Rutland
2021-10-26  9:24 ` [PATCH v2 11/17] irq: arm: perform irqentry in entry code Mark Rutland
2021-10-26  9:24 ` [PATCH v2 12/17] irq: arm64: " Mark Rutland
2021-10-26  9:25 ` [PATCH v2 13/17] irq: csky: " Mark Rutland
2021-10-26  9:25 ` [PATCH v2 14/17] irq: openrisc: " Mark Rutland
2021-10-26  9:25 ` [PATCH v2 15/17] irq: riscv: " Mark Rutland
2021-10-26  9:25 ` [PATCH v2 16/17] irq: remove CONFIG_HANDLE_DOMAIN_IRQ_IRQENTRY Mark Rutland
2021-10-26  9:25 ` [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}() Mark Rutland
2022-05-06 20:32   ` Lukas Wunner
2022-05-09  8:54     ` Mark Rutland
2022-05-09  9:09       ` Marc Zyngier
2022-05-09 13:12         ` Thomas Gleixner
2022-05-10 23:36         ` Thomas Gleixner
2022-05-10 12:13       ` Lukas Wunner
2022-05-10 14:15         ` Mark Rutland
2022-05-10 22:52           ` Thomas Gleixner
2022-05-11  8:23             ` Mark Rutland
2022-05-11  8:57               ` Lukas Wunner
2022-05-11  9:27                 ` Mark Rutland
2022-05-11  0:11           ` Thomas Gleixner
2022-05-11  8:11             ` Mark Rutland
2021-10-26 10:12 ` [PATCH v2 00/17] " Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).