Hector's M1 support series [1] shows that some platforms have critical
interrupts wired to FIQ, and to support these platforms we need to support
handling FIQ exceptions. Other contemporary platforms don't use FIQ (since e.g.
this is usually routed to EL3), and as we never expect to take an FIQ, we have
the FIQ vector cause a panic.
Since the use of FIQ is a platform integration detail (which can differ across
bare-metal and virtualized environments), we need be able to explicitly opt-in
to handling FIQs while retaining the existing behaviour otherwise. This series
adds a new set_handle_fiq() hook so that the FIQ controller can do so, and
where no controller is registered the default handler will panic(). For
consistency the set_handle_irq() code is made to do the same.
The first four patches move arm64 over to a local set_handle_irq()
implementation, which is written to share code with a set_handle_fiq() function
in the last two patches. This adds a default handler which will directly
panic() rather than branching to NULL if an IRQ is taken unexpectedly, and the
boot-time panic in the absence of a handler is removed (for consistently with
FIQ support added later).
The penultimate patch reworks arm64's IRQ masking to always keep DAIF.[IF] in
sync, so that we can treat IRQ and FIQ as equals. This is cherry-picked from
Hector's reply [2] to the first version of this series.
The final patch adds the low-level FIQ exception handling and registration
mechanism atop the prior rework.
I'm hoping this is ready to be merged into the arm64 tree, given the
preparatory cleanup made it into v5.12-rc3. I've pushed the series out to my
arm64/fiq branch [3] on kernel.org, also tagged as arm64-fiq-20210315, atop
v5.12-rc3.
Since v1 [4]:
* Rebase to v5.12-rc1
* Pick up Hector's latest DAIF.[IF] patch
* Use "root {IRQ,FIQ} handler" rather than "{IRQ,FIQ} controller"
* Remove existing panic per Marc's comments
* Log registered root handlers
* Make default root handlers static
* Remove redundant el0_fiq_invalid_compat, per Joey's comments
Since v2 [5]:
* Fold in Hector's Tested-by tags
* Rebase to v5.12-rc3
* Drop patches merged in v5.12-rc3
[1] https://http://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/fiq
[4] https://lore.kernel.org/r/[email protected]
[5] https://lore.kernel.org/r/[email protected]
Thanks,
Mark.
Hector Martin (1):
arm64: Always keep DAIF.[IF] in sync
Marc Zyngier (3):
genirq: Allow architectures to override set_handle_irq() fallback
arm64: don't use GENERIC_IRQ_MULTI_HANDLER
arm64: entry: factor irq triage logic into macros
Mark Rutland (2):
arm64: irq: rework root IRQ handler registration
arm64: irq: allow FIQs to be handled
arch/arm64/Kconfig | 1 -
arch/arm64/include/asm/arch_gicv3.h | 2 +-
arch/arm64/include/asm/assembler.h | 8 +--
arch/arm64/include/asm/daifflags.h | 10 ++--
arch/arm64/include/asm/irq.h | 4 ++
arch/arm64/include/asm/irqflags.h | 16 +++--
arch/arm64/kernel/entry.S | 114 +++++++++++++++++++++---------------
arch/arm64/kernel/irq.c | 35 ++++++++++-
arch/arm64/kernel/process.c | 2 +-
arch/arm64/kernel/smp.c | 1 +
include/linux/irq.h | 2 +
11 files changed, 125 insertions(+), 70 deletions(-)
--
2.11.0
From: Marc Zyngier <[email protected]>
Some architectures want to provide the generic set_handle_irq() API, but
for structural reasons need to provide their own implementation. For
example, arm64 needs to do this to provide uniform set_handle_irq() and
set_handle_fiq() registration functions.
Make this possible by allowing architectures to provide their own
implementation of set_handle_irq when CONFIG_GENERIC_IRQ_MULTI_HANDLER
is not selected.
Signed-off-by: Marc Zyngier <[email protected]>
[Mark: expand commit message]
Signed-off-by: Mark Rutland <[email protected]>
Tested-by: Hector Martin <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
---
include/linux/irq.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 2efde6a79b7e..9890180b84fd 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1258,11 +1258,13 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *));
*/
extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
#else
+#ifndef set_handle_irq
#define set_handle_irq(handle_irq) \
do { \
(void)handle_irq; \
WARN_ON(1); \
} while (0)
#endif
+#endif
#endif /* _LINUX_IRQ_H */
--
2.11.0
If we accidentally unmask IRQs before we've registered a root IRQ
handler, handle_arch_irq will be NULL, and the IRQ exception handler
will branch to a bogus address.
To make this easier to debug, this patch initialises handle_arch_irq to
a default handler which will panic(), making such problems easier to
debug. When we add support for FIQ handlers, we can follow the same
approach.
When we add support for a root FIQ handler, it's possible to have root
IRQ handler without an root FIQ handler, and in theory the inverse is
also possible. To permit this, and to keep the IRQ/FIQ registration
logic similar, this patch removes the panic in the absence of a root IRQ
controller. Instead, set_handle_irq() logs when a handler is registered,
which is sufficient for debug purposes.
Signed-off-by: Mark Rutland <[email protected]>
Tested-by: Hector Martin <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/kernel/irq.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index ad63bd50fa7b..2fe0b535de30 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -71,14 +71,20 @@ static void init_irq_stacks(void)
}
#endif
-void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+static void default_handle_irq(struct pt_regs *regs)
+{
+ panic("IRQ taken without a root IRQ handler\n");
+}
+
+void (*handle_arch_irq)(struct pt_regs *) __ro_after_init = default_handle_irq;
int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
{
- if (handle_arch_irq)
+ if (handle_arch_irq != default_handle_irq)
return -EBUSY;
handle_arch_irq = handle_irq;
+ pr_info("Root IRQ handler: %ps\n", handle_irq);
return 0;
}
@@ -87,8 +93,6 @@ void __init init_IRQ(void)
init_irq_stacks();
init_irq_scs();
irqchip_init();
- if (!handle_arch_irq)
- panic("No interrupt controller found.");
if (system_uses_irq_prio_masking()) {
/*
--
2.11.0
From: Hector Martin <[email protected]>
Apple SoCs (A11 and newer) have some interrupt sources hardwired to the
FIQ line. We implement support for this by simply treating IRQs and FIQs
the same way in the interrupt vectors.
To support these systems, the FIQ mask bit needs to be kept in sync with
the IRQ mask bit, so both kinds of exceptions are masked together. No
other platforms should be delivering FIQ exceptions right now, and we
already unmask FIQ in normal process context, so this should not have an
effect on other systems - if spurious FIQs were arriving, they would
already panic the kernel.
Signed-off-by: Hector Martin <[email protected]>
Signed-off-by: Mark Rutland <[email protected]>
Tested-by: Hector Martin <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/arch_gicv3.h | 2 +-
arch/arm64/include/asm/assembler.h | 8 ++++----
arch/arm64/include/asm/daifflags.h | 10 +++++-----
arch/arm64/include/asm/irqflags.h | 16 +++++++---------
arch/arm64/kernel/entry.S | 12 +++++++-----
arch/arm64/kernel/process.c | 2 +-
arch/arm64/kernel/smp.c | 1 +
7 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index 880b9054d75c..934b9be582d2 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -173,7 +173,7 @@ static inline void gic_pmr_mask_irqs(void)
static inline void gic_arch_enable_irqs(void)
{
- asm volatile ("msr daifclr, #2" : : : "memory");
+ asm volatile ("msr daifclr, #3" : : : "memory");
}
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index ca31594d3d6c..b76a71e84b7c 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -40,9 +40,9 @@
msr daif, \flags
.endm
- /* IRQ is the lowest priority flag, unconditionally unmask the rest. */
- .macro enable_da_f
- msr daifclr, #(8 | 4 | 1)
+ /* IRQ/FIQ are the lowest priority flags, unconditionally unmask the rest. */
+ .macro enable_da
+ msr daifclr, #(8 | 4)
.endm
/*
@@ -50,7 +50,7 @@
*/
.macro save_and_disable_irq, flags
mrs \flags, daif
- msr daifset, #2
+ msr daifset, #3
.endm
.macro restore_irq, flags
diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
index 1c26d7baa67f..5eb7af9c4557 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -13,8 +13,8 @@
#include <asm/ptrace.h>
#define DAIF_PROCCTX 0
-#define DAIF_PROCCTX_NOIRQ PSR_I_BIT
-#define DAIF_ERRCTX (PSR_I_BIT | PSR_A_BIT)
+#define DAIF_PROCCTX_NOIRQ (PSR_I_BIT | PSR_F_BIT)
+#define DAIF_ERRCTX (PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
#define DAIF_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
@@ -47,7 +47,7 @@ static inline unsigned long local_daif_save_flags(void)
if (system_uses_irq_prio_masking()) {
/* If IRQs are masked with PMR, reflect it in the flags */
if (read_sysreg_s(SYS_ICC_PMR_EL1) != GIC_PRIO_IRQON)
- flags |= PSR_I_BIT;
+ flags |= PSR_I_BIT | PSR_F_BIT;
}
return flags;
@@ -69,7 +69,7 @@ static inline void local_daif_restore(unsigned long flags)
bool irq_disabled = flags & PSR_I_BIT;
WARN_ON(system_has_prio_mask_debugging() &&
- !(read_sysreg(daif) & PSR_I_BIT));
+ (read_sysreg(daif) & (PSR_I_BIT | PSR_F_BIT)) != (PSR_I_BIT | PSR_F_BIT));
if (!irq_disabled) {
trace_hardirqs_on();
@@ -86,7 +86,7 @@ static inline void local_daif_restore(unsigned long flags)
* If interrupts are disabled but we can take
* asynchronous errors, we can take NMIs
*/
- flags &= ~PSR_I_BIT;
+ flags &= ~(PSR_I_BIT | PSR_F_BIT);
pmr = GIC_PRIO_IRQOFF;
} else {
pmr = GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET;
diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
index ff328e5bbb75..b57b9b1e4344 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -12,15 +12,13 @@
/*
* Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
- * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'dai'
+ * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'daif'
* order:
* Masking debug exceptions causes all other exceptions to be masked too/
- * Masking SError masks irq, but not debug exceptions. Masking irqs has no
- * side effects for other flags. Keeping to this order makes it easier for
- * entry.S to know which exceptions should be unmasked.
- *
- * FIQ is never expected, but we mask it when we disable debug exceptions, and
- * unmask it at all other times.
+ * Masking SError masks IRQ/FIQ, but not debug exceptions. IRQ and FIQ are
+ * always masked and unmasked together, and have no side effects for other
+ * flags. Keeping to this order makes it easier for entry.S to know which
+ * exceptions should be unmasked.
*/
/*
@@ -35,7 +33,7 @@ static inline void arch_local_irq_enable(void)
}
asm volatile(ALTERNATIVE(
- "msr daifclr, #2 // arch_local_irq_enable",
+ "msr daifclr, #3 // arch_local_irq_enable",
__msr_s(SYS_ICC_PMR_EL1, "%0"),
ARM64_HAS_IRQ_PRIO_MASKING)
:
@@ -54,7 +52,7 @@ static inline void arch_local_irq_disable(void)
}
asm volatile(ALTERNATIVE(
- "msr daifset, #2 // arch_local_irq_disable",
+ "msr daifset, #3 // arch_local_irq_disable",
__msr_s(SYS_ICC_PMR_EL1, "%0"),
ARM64_HAS_IRQ_PRIO_MASKING)
:
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index e235b0e4e468..ce8d4dc416fb 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -533,7 +533,7 @@ alternative_endif
.macro el1_interrupt_handler, handler:req
gic_prio_irq_setup pmr=x20, tmp=x1
- enable_da_f
+ enable_da
mov x0, sp
bl enter_el1_irq_or_nmi
@@ -544,8 +544,10 @@ alternative_endif
ldr x24, [tsk, #TSK_TI_PREEMPT] // get preempt count
alternative_if ARM64_HAS_IRQ_PRIO_MASKING
/*
- * DA_F were cleared at start of handling. If anything is set in DAIF,
- * we come back from an NMI, so skip preemption
+ * DA were cleared at start of handling, and IF are cleared by
+ * the GIC irqchip driver using gic_arch_enable_irqs() for
+ * normal IRQs. If anything is set, it means we come back from
+ * an NMI instead of a normal IRQ, so skip preemption
*/
mrs x0, daif
orr x24, x24, x0
@@ -562,7 +564,7 @@ alternative_else_nop_endif
.macro el0_interrupt_handler, handler:req
gic_prio_irq_setup pmr=x20, tmp=x0
user_exit_irqoff
- enable_da_f
+ enable_da
tbz x22, #55, 1f
bl do_el0_irq_bp_hardening
@@ -763,7 +765,7 @@ el0_error_naked:
mov x0, sp
mov x1, x25
bl do_serror
- enable_da_f
+ enable_da
b ret_to_user
SYM_CODE_END(el0_error)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 325c83b1a24d..a29028d3d46e 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -84,7 +84,7 @@ static void noinstr __cpu_do_idle_irqprio(void)
unsigned long daif_bits;
daif_bits = read_sysreg(daif);
- write_sysreg(daif_bits | PSR_I_BIT, daif);
+ write_sysreg(daif_bits | PSR_I_BIT | PSR_F_BIT, daif);
/*
* Unmask PMR before going idle to make sure interrupts can
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 357590beaabb..dcd7041b2b07 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -188,6 +188,7 @@ static void init_gic_priority_masking(void)
cpuflags = read_sysreg(daif);
WARN_ON(!(cpuflags & PSR_I_BIT));
+ WARN_ON(!(cpuflags & PSR_F_BIT));
gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET);
}
--
2.11.0
From: Marc Zyngier <[email protected]>
In subsequent patches we want to allow irqchip drivers to register as
FIQ handlers, with a set_handle_fiq() function. To keep the IRQ/FIQ
paths similar, we want arm64 to provide both set_handle_irq() and
set_handle_fiq(), rather than using GENERIC_IRQ_MULTI_HANDLER for the
former.
This patch adds an arm64-specific implementation of set_handle_irq().
There should be no functional change as a result of this patch.
Signed-off-by: Marc Zyngier <[email protected]>
[Mark: use a single handler pointer]
Signed-off-by: Mark Rutland <[email protected]>
Tested-by: Hector Martin <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/Kconfig | 1 -
arch/arm64/include/asm/irq.h | 3 +++
arch/arm64/kernel/irq.c | 11 +++++++++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5656e7aacd69..e7d2405be71f 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -110,7 +110,6 @@ config ARM64
select GENERIC_EARLY_IOREMAP
select GENERIC_IDLE_POLL_SETUP
select GENERIC_IRQ_IPI
- select GENERIC_IRQ_MULTI_HANDLER
select GENERIC_IRQ_PROBE
select GENERIC_IRQ_SHOW
select GENERIC_IRQ_SHOW_LEVEL
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index b2b0c6405eb0..8391c6f6f746 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -8,6 +8,9 @@
struct pt_regs;
+int set_handle_irq(void (*handle_irq)(struct pt_regs *));
+#define set_handle_irq set_handle_irq
+
static inline int nr_legacy_irqs(void)
{
return 0;
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index dfb1feab867d..ad63bd50fa7b 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -71,6 +71,17 @@ static void init_irq_stacks(void)
}
#endif
+void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+
+int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
+{
+ if (handle_arch_irq)
+ return -EBUSY;
+
+ handle_arch_irq = handle_irq;
+ return 0;
+}
+
void __init init_IRQ(void)
{
init_irq_stacks();
--
2.11.0
From: Marc Zyngier <[email protected]>
In subsequent patches we'll allow an FIQ handler to be registered, and
FIQ exceptions will need to be triaged very similarly to IRQ exceptions.
So that we can reuse the existing logic, this patch factors the IRQ
triage logic out into macros that can be reused for FIQ.
The macros are named to follow the elX_foo_handler scheme used by the C
exception handlers. For consistency with other top-level exception
handlers, the kernel_entry/kernel_exit logic is not moved into the
macros. As FIQ will use a different C handler, this handler name is
provided as an argument to the macros.
There should be no functional change as a result of this patch.
Signed-off-by: Marc Zyngier <[email protected]>
[Mark: rework macros, commit message, rebase before DAIF rework]
Signed-off-by: Mark Rutland <[email protected]>
Tested-by: Hector Martin <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/kernel/entry.S | 80 +++++++++++++++++++++++++----------------------
1 file changed, 43 insertions(+), 37 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index a31a0a713c85..e235b0e4e468 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -491,8 +491,8 @@ tsk .req x28 // current thread_info
/*
* Interrupt handling.
*/
- .macro irq_handler
- ldr_l x1, handle_arch_irq
+ .macro irq_handler, handler:req
+ ldr_l x1, \handler
mov x0, sp
irq_stack_entry
blr x1
@@ -531,6 +531,45 @@ alternative_endif
#endif
.endm
+ .macro el1_interrupt_handler, handler:req
+ gic_prio_irq_setup pmr=x20, tmp=x1
+ enable_da_f
+
+ mov x0, sp
+ bl enter_el1_irq_or_nmi
+
+ irq_handler \handler
+
+#ifdef CONFIG_PREEMPTION
+ ldr x24, [tsk, #TSK_TI_PREEMPT] // get preempt count
+alternative_if ARM64_HAS_IRQ_PRIO_MASKING
+ /*
+ * DA_F were cleared at start of handling. If anything is set in DAIF,
+ * we come back from an NMI, so skip preemption
+ */
+ mrs x0, daif
+ orr x24, x24, x0
+alternative_else_nop_endif
+ cbnz x24, 1f // preempt count != 0 || NMI return path
+ bl arm64_preempt_schedule_irq // irq en/disable is done inside
+1:
+#endif
+
+ mov x0, sp
+ bl exit_el1_irq_or_nmi
+ .endm
+
+ .macro el0_interrupt_handler, handler:req
+ gic_prio_irq_setup pmr=x20, tmp=x0
+ user_exit_irqoff
+ enable_da_f
+
+ tbz x22, #55, 1f
+ bl do_el0_irq_bp_hardening
+1:
+ irq_handler \handler
+ .endm
+
.text
/*
@@ -660,32 +699,7 @@ SYM_CODE_END(el1_sync)
.align 6
SYM_CODE_START_LOCAL_NOALIGN(el1_irq)
kernel_entry 1
- gic_prio_irq_setup pmr=x20, tmp=x1
- enable_da_f
-
- mov x0, sp
- bl enter_el1_irq_or_nmi
-
- irq_handler
-
-#ifdef CONFIG_PREEMPTION
- ldr x24, [tsk, #TSK_TI_PREEMPT] // get preempt count
-alternative_if ARM64_HAS_IRQ_PRIO_MASKING
- /*
- * DA_F were cleared at start of handling. If anything is set in DAIF,
- * we come back from an NMI, so skip preemption
- */
- mrs x0, daif
- orr x24, x24, x0
-alternative_else_nop_endif
- cbnz x24, 1f // preempt count != 0 || NMI return path
- bl arm64_preempt_schedule_irq // irq en/disable is done inside
-1:
-#endif
-
- mov x0, sp
- bl exit_el1_irq_or_nmi
-
+ el1_interrupt_handler handle_arch_irq
kernel_exit 1
SYM_CODE_END(el1_irq)
@@ -725,15 +739,7 @@ SYM_CODE_END(el0_error_compat)
SYM_CODE_START_LOCAL_NOALIGN(el0_irq)
kernel_entry 0
el0_irq_naked:
- gic_prio_irq_setup pmr=x20, tmp=x0
- user_exit_irqoff
- enable_da_f
-
- tbz x22, #55, 1f
- bl do_el0_irq_bp_hardening
-1:
- irq_handler
-
+ el0_interrupt_handler handle_arch_irq
b ret_to_user
SYM_CODE_END(el0_irq)
--
2.11.0
On contemporary platforms we don't use FIQ, and treat any stray FIQ as a
fatal event. However, some platforms have an interrupt controller wired
to FIQ, and need to handle FIQ as part of regular operation.
So that we can support both cases dynamically, this patch updates the
FIQ exception handling code to operate the same way as the IRQ handling
code, with its own handle_arch_fiq handler. Where a root FIQ handler is
not registered, an unexpected FIQ exception will trigger the default FIQ
handler, which will panic() as today. Where a root FIQ handler is
registered, handling of the FIQ is deferred to that handler.
As el0_fiq_invalid_compat is supplanted by el0_fiq, the former is
removed. For !CONFIG_COMPAT builds we never expect to take an exception
from AArch32 EL0, so we keep the common el0_fiq_invalid handler.
Signed-off-by: Mark Rutland <[email protected]>
Tested-by: Hector Martin <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/irq.h | 1 +
arch/arm64/kernel/entry.S | 30 +++++++++++++++++++++---------
arch/arm64/kernel/irq.c | 16 ++++++++++++++++
3 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index 8391c6f6f746..fac08e18bcd5 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -10,6 +10,7 @@ struct pt_regs;
int set_handle_irq(void (*handle_irq)(struct pt_regs *));
#define set_handle_irq set_handle_irq
+int set_handle_fiq(void (*handle_fiq)(struct pt_regs *));
static inline int nr_legacy_irqs(void)
{
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index ce8d4dc416fb..a86f50de2c7b 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -588,18 +588,18 @@ SYM_CODE_START(vectors)
kernel_ventry 1, sync // Synchronous EL1h
kernel_ventry 1, irq // IRQ EL1h
- kernel_ventry 1, fiq_invalid // FIQ EL1h
+ kernel_ventry 1, fiq // FIQ EL1h
kernel_ventry 1, error // Error EL1h
kernel_ventry 0, sync // Synchronous 64-bit EL0
kernel_ventry 0, irq // IRQ 64-bit EL0
- kernel_ventry 0, fiq_invalid // FIQ 64-bit EL0
+ kernel_ventry 0, fiq // FIQ 64-bit EL0
kernel_ventry 0, error // Error 64-bit EL0
#ifdef CONFIG_COMPAT
kernel_ventry 0, sync_compat, 32 // Synchronous 32-bit EL0
kernel_ventry 0, irq_compat, 32 // IRQ 32-bit EL0
- kernel_ventry 0, fiq_invalid_compat, 32 // FIQ 32-bit EL0
+ kernel_ventry 0, fiq_compat, 32 // FIQ 32-bit EL0
kernel_ventry 0, error_compat, 32 // Error 32-bit EL0
#else
kernel_ventry 0, sync_invalid, 32 // Synchronous 32-bit EL0
@@ -665,12 +665,6 @@ SYM_CODE_START_LOCAL(el0_error_invalid)
inv_entry 0, BAD_ERROR
SYM_CODE_END(el0_error_invalid)
-#ifdef CONFIG_COMPAT
-SYM_CODE_START_LOCAL(el0_fiq_invalid_compat)
- inv_entry 0, BAD_FIQ, 32
-SYM_CODE_END(el0_fiq_invalid_compat)
-#endif
-
SYM_CODE_START_LOCAL(el1_sync_invalid)
inv_entry 1, BAD_SYNC
SYM_CODE_END(el1_sync_invalid)
@@ -705,6 +699,12 @@ SYM_CODE_START_LOCAL_NOALIGN(el1_irq)
kernel_exit 1
SYM_CODE_END(el1_irq)
+SYM_CODE_START_LOCAL_NOALIGN(el1_fiq)
+ kernel_entry 1
+ el1_interrupt_handler handle_arch_fiq
+ kernel_exit 1
+SYM_CODE_END(el1_fiq)
+
/*
* EL0 mode handlers.
*/
@@ -731,6 +731,11 @@ SYM_CODE_START_LOCAL_NOALIGN(el0_irq_compat)
b el0_irq_naked
SYM_CODE_END(el0_irq_compat)
+SYM_CODE_START_LOCAL_NOALIGN(el0_fiq_compat)
+ kernel_entry 0, 32
+ b el0_fiq_naked
+SYM_CODE_END(el0_fiq_compat)
+
SYM_CODE_START_LOCAL_NOALIGN(el0_error_compat)
kernel_entry 0, 32
b el0_error_naked
@@ -745,6 +750,13 @@ el0_irq_naked:
b ret_to_user
SYM_CODE_END(el0_irq)
+SYM_CODE_START_LOCAL_NOALIGN(el0_fiq)
+ kernel_entry 0
+el0_fiq_naked:
+ el0_interrupt_handler handle_arch_fiq
+ b ret_to_user
+SYM_CODE_END(el0_fiq)
+
SYM_CODE_START_LOCAL(el1_error)
kernel_entry 1
mrs x1, esr_el1
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 2fe0b535de30..bda49430c9ea 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -76,7 +76,13 @@ static void default_handle_irq(struct pt_regs *regs)
panic("IRQ taken without a root IRQ handler\n");
}
+static void default_handle_fiq(struct pt_regs *regs)
+{
+ panic("FIQ taken without a root FIQ handler\n");
+}
+
void (*handle_arch_irq)(struct pt_regs *) __ro_after_init = default_handle_irq;
+void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init = default_handle_fiq;
int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
{
@@ -88,6 +94,16 @@ int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
return 0;
}
+int __init set_handle_fiq(void (*handle_fiq)(struct pt_regs *))
+{
+ if (handle_arch_fiq != default_handle_fiq)
+ return -EBUSY;
+
+ handle_arch_fiq = handle_fiq;
+ pr_info("Root FIQ handler: %ps\n", handle_fiq);
+ return 0;
+}
+
void __init init_IRQ(void)
{
init_irq_stacks();
--
2.11.0
On Mon, Mar 15, 2021 at 11:56:25AM +0000, Mark Rutland wrote:
> From: Marc Zyngier <[email protected]>
>
> In subsequent patches we want to allow irqchip drivers to register as
> FIQ handlers, with a set_handle_fiq() function. To keep the IRQ/FIQ
> paths similar, we want arm64 to provide both set_handle_irq() and
> set_handle_fiq(), rather than using GENERIC_IRQ_MULTI_HANDLER for the
> former.
Having looked through the series I do not understand this rationale
at all. You've only added the default_handle_irq logic, which seems
perfectly suitable and desirable for the generic version. Please
don't fork off generic code for no good reason.
Hi Christoph,
On Mon, Mar 15, 2021 at 07:28:03PM +0000, Christoph Hellwig wrote:
> On Mon, Mar 15, 2021 at 11:56:25AM +0000, Mark Rutland wrote:
> > From: Marc Zyngier <[email protected]>
> >
> > In subsequent patches we want to allow irqchip drivers to register as
> > FIQ handlers, with a set_handle_fiq() function. To keep the IRQ/FIQ
> > paths similar, we want arm64 to provide both set_handle_irq() and
> > set_handle_fiq(), rather than using GENERIC_IRQ_MULTI_HANDLER for the
> > former.
>
> Having looked through the series I do not understand this rationale
> at all. You've only added the default_handle_irq logic, which seems
> perfectly suitable and desirable for the generic version.
The default_handle_irq thing isn't the point of the series, that part is
all preparatory work. I agree that probably makes sense for the generic
code, and I'm happy to update core code with this.
The big thing here is that (unlike most architectures), with arm64 a CPU
has two interrupt pins, IRQ and FIQ, and we need separate root handlers
for these. That's what this series aims to do, and patches 1-5 are all
preparatory work with that appearing in patch 6.
Our initial stab at this did try to add that support to core code, but
that was more painful to deal with, since you either add abstractions to
make this look generic that make the code more complex for bot hthe
genreic code and arch code, or you place arch-specific assumptions in
the core code. See Marc's eariler stab at this, where in effect we had
to duplicate the logic in the core code so that we didn't adversely
affect existing entry assembly on other architectures due to the way the
function pointers were stored.
> Please don't fork off generic code for no good reason.
I appreciate that this runs counter to the general goal of making things
generic wherever possible, but I do think in this case we have good
reasons, and the duplication is better than adding single-user
abstractions in the generic code that complicate the generic code and
arch code.
Thanks,
Mark.
On Mon, Mar 15, 2021 at 11:56:26AM +0000, Mark Rutland wrote:
> If we accidentally unmask IRQs before we've registered a root IRQ
> handler, handle_arch_irq will be NULL, and the IRQ exception handler
> will branch to a bogus address.
>
> To make this easier to debug, this patch initialises handle_arch_irq to
> a default handler which will panic(), making such problems easier to
> debug. When we add support for FIQ handlers, we can follow the same
> approach.
>
> When we add support for a root FIQ handler, it's possible to have root
> IRQ handler without an root FIQ handler, and in theory the inverse is
> also possible. To permit this, and to keep the IRQ/FIQ registration
> logic similar, this patch removes the panic in the absence of a root IRQ
> controller. Instead, set_handle_irq() logs when a handler is registered,
> which is sufficient for debug purposes.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Tested-by: Hector Martin <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: James Morse <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> arch/arm64/kernel/irq.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
Acked-by: Will Deacon <[email protected]>
Will
On Mon, Mar 15, 2021 at 11:56:27AM +0000, Mark Rutland wrote:
> From: Marc Zyngier <[email protected]>
>
> In subsequent patches we'll allow an FIQ handler to be registered, and
> FIQ exceptions will need to be triaged very similarly to IRQ exceptions.
> So that we can reuse the existing logic, this patch factors the IRQ
> triage logic out into macros that can be reused for FIQ.
>
> The macros are named to follow the elX_foo_handler scheme used by the C
> exception handlers. For consistency with other top-level exception
> handlers, the kernel_entry/kernel_exit logic is not moved into the
> macros. As FIQ will use a different C handler, this handler name is
> provided as an argument to the macros.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> [Mark: rework macros, commit message, rebase before DAIF rework]
> Signed-off-by: Mark Rutland <[email protected]>
> Tested-by: Hector Martin <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: James Morse <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> arch/arm64/kernel/entry.S | 80 +++++++++++++++++++++++++----------------------
> 1 file changed, 43 insertions(+), 37 deletions(-)
Acked-by: Will Deacon <[email protected]>
Will
On Mon, Mar 15, 2021 at 11:56:28AM +0000, Mark Rutland wrote:
> From: Hector Martin <[email protected]>
>
> Apple SoCs (A11 and newer) have some interrupt sources hardwired to the
> FIQ line. We implement support for this by simply treating IRQs and FIQs
> the same way in the interrupt vectors.
>
> To support these systems, the FIQ mask bit needs to be kept in sync with
> the IRQ mask bit, so both kinds of exceptions are masked together. No
> other platforms should be delivering FIQ exceptions right now, and we
> already unmask FIQ in normal process context, so this should not have an
> effect on other systems - if spurious FIQs were arriving, they would
> already panic the kernel.
>
> Signed-off-by: Hector Martin <[email protected]>
> Signed-off-by: Mark Rutland <[email protected]>
> Tested-by: Hector Martin <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: James Morse <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> arch/arm64/include/asm/arch_gicv3.h | 2 +-
> arch/arm64/include/asm/assembler.h | 8 ++++----
> arch/arm64/include/asm/daifflags.h | 10 +++++-----
> arch/arm64/include/asm/irqflags.h | 16 +++++++---------
> arch/arm64/kernel/entry.S | 12 +++++++-----
> arch/arm64/kernel/process.c | 2 +-
> arch/arm64/kernel/smp.c | 1 +
> 7 files changed, 26 insertions(+), 25 deletions(-)
Acked-by: Will Deacon <[email protected]>
Will
On Mon, Mar 15, 2021 at 11:56:29AM +0000, Mark Rutland wrote:
> On contemporary platforms we don't use FIQ, and treat any stray FIQ as a
> fatal event. However, some platforms have an interrupt controller wired
> to FIQ, and need to handle FIQ as part of regular operation.
>
> So that we can support both cases dynamically, this patch updates the
> FIQ exception handling code to operate the same way as the IRQ handling
> code, with its own handle_arch_fiq handler. Where a root FIQ handler is
> not registered, an unexpected FIQ exception will trigger the default FIQ
> handler, which will panic() as today. Where a root FIQ handler is
> registered, handling of the FIQ is deferred to that handler.
>
> As el0_fiq_invalid_compat is supplanted by el0_fiq, the former is
> removed. For !CONFIG_COMPAT builds we never expect to take an exception
> from AArch32 EL0, so we keep the common el0_fiq_invalid handler.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Tested-by: Hector Martin <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: James Morse <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> arch/arm64/include/asm/irq.h | 1 +
> arch/arm64/kernel/entry.S | 30 +++++++++++++++++++++---------
> arch/arm64/kernel/irq.c | 16 ++++++++++++++++
> 3 files changed, 38 insertions(+), 9 deletions(-)
Acked-by: Will Deacon <[email protected]>
Will
On Mon, 15 Mar 2021 11:56:23 +0000, Mark Rutland wrote:
> Hector's M1 support series [1] shows that some platforms have critical
> interrupts wired to FIQ, and to support these platforms we need to support
> handling FIQ exceptions. Other contemporary platforms don't use FIQ (since e.g.
> this is usually routed to EL3), and as we never expect to take an FIQ, we have
> the FIQ vector cause a panic.
>
> Since the use of FIQ is a platform integration detail (which can differ across
> bare-metal and virtualized environments), we need be able to explicitly opt-in
> to handling FIQs while retaining the existing behaviour otherwise. This series
> adds a new set_handle_fiq() hook so that the FIQ controller can do so, and
> where no controller is registered the default handler will panic(). For
> consistency the set_handle_irq() code is made to do the same.
>
> [...]
Applied to arm64 (for-next/fiq), thanks!
[1/6] genirq: Allow architectures to override set_handle_irq() fallback
https://git.kernel.org/arm64/c/b0b8b689d78c
[2/6] arm64: don't use GENERIC_IRQ_MULTI_HANDLER
https://git.kernel.org/arm64/c/338a743640e9
[3/6] arm64: irq: rework root IRQ handler registration
https://git.kernel.org/arm64/c/8ff443cebffa
[4/6] arm64: entry: factor irq triage logic into macros
https://git.kernel.org/arm64/c/9eb563cdabe1
[5/6] arm64: Always keep DAIF.[IF] in sync
https://git.kernel.org/arm64/c/f0098155d337
[6/6] arm64: irq: allow FIQs to be handled
https://git.kernel.org/arm64/c/3889ba70102e
--
Catalin