Re: [PATCH 1/3] arm64/kernel: FIQ support
> On 20 Jan 2021, at 14:16, Marc Zyngier wrote:
>
> Hi Mohamed,
>
> On 2021-01-20 11:36, Mohamed Mediouni wrote:
>> From: Stan Skowronek
>> On Apple processors, the timer is wired through FIQ.
>
> Which timer? There are at least 3, potentially 4 timers per CPU
> that can fire.
This is about the Arm architectural timers.
>> As such, add FIQ support to the kernel.
>> Signed-off-by: Stan Skowronek
>
> Missing SoB from the sender.
>
Fixed in the RFC.
>> ---
>> arch/arm64/include/asm/arch_gicv3.h | 2 +-
>> arch/arm64/include/asm/assembler.h | 8 ++--
>> arch/arm64/include/asm/daifflags.h | 4 +-
>> arch/arm64/include/asm/irq.h| 4 ++
>> arch/arm64/include/asm/irqflags.h | 6 +--
>> arch/arm64/kernel/entry.S | 74 ++---
>> arch/arm64/kernel/irq.c | 14 ++
>> arch/arm64/kernel/process.c | 2 +-
>> 8 files changed, 97 insertions(+), 17 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");
>
> If I trust the persistent rumour, this system doesn't have a GIC.
> Why this change?
>
Will ask about why GIC functions were changed too… and yeah
This exclusively has an Apple AIC interrupt controller.
>> #endif /* __ASSEMBLY__ */
>> diff --git a/arch/arm64/include/asm/assembler.h
>> b/arch/arm64/include/asm/assembler.h
>> index bf125c591116..6fe55713dfe0 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 is the lowest priority flag, unconditionally unmask the
>> rest. */
>> +.macro enable_da
>> +msr daifclr, #(8 | 4)
>
> This cannot be unconditional. This potentially changes existing behaviours,
> and I'd feel a lot safer if FIQ was only messed with on that specific HW.
>
> I have the feeling that this should be detected on the boot CPU and patched
> before any interrupt can fire.
>
Could alternatives be the proper mechanism for this?
>> .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..44de96c7fb1a 100644
>> --- a/arch/arm64/include/asm/daifflags.h
>> +++ b/arch/arm64/include/asm/daifflags.h
>> @@ -13,8 +13,8 @@
>> #include
>> #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_I_BIT | PSR_F_BIT | PSR_A_BIT)
>> #define DAIF_MASK(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
>> diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
>> index b2b0c6405eb0..2d1537d3a245 100644
>> --- a/arch/arm64/include/asm/irq.h
>> +++ b/arch/arm64/include/asm/irq.h
>> @@ -13,5 +13,9 @@ static inline int nr_legacy_irqs(void)
>> return 0;
>> }
>> +int set_handle_fiq(void (*handle_fiq)(struct pt_regs *));
>> +
>> +extern void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init;
>
> I guess this is set from the root interrupt controller, which also
> will set handle_arch_irq? Why do we need two entry points? We have
> ISR_EL1 to find out what is pending. Isn't that enough?
>
>> +
>> #endif /* !__ASSEMBLER__ */
>> #endif
>> diff --git a/arch/arm64/include/asm/irqflags.h
>> b/arch/arm64/include/asm/irqflags.h
>> index ff328e5bbb75..26d7f378113e 100644
>> --- a/arch/arm64/include/asm/irqflags.h
>> +++ b/arch/arm64/include/asm/irqflags.h
>> @@ -35,7 +35,7 @@ static inline void arch_local_irq_enable(void)
>> }
>> asm volatile(ALTERNATIVE(
>> -"msrdaifclr, #2 // arch_local_irq_enable",
>> +"msrdaifclr, #3 // arch_local_irq_enable",
>> __msr_s(SYS_ICC_PMR_EL1, "%0"),
>> ARM64_HAS_IRQ_PRIO_MASKING)
>> :
>> @@ -54,7 +54,7 @@ static inline void arch_local_irq_disable(void)
>> }
>> asm volatile(ALTERNATIVE(
>> -"msrdaifset, #2 // arch_local_irq_disable",
>> +"msrdaifset, #3 // arch_local_irq_disable",
>> __msr_s(SYS_ICC_PMR_EL1, "%0"),
>> ARM64_HAS_IRQ_PRIO_MASKING)
>> :
>> @@ -85,7
Re: [PATCH 1/3] arm64/kernel: FIQ support
Hi Mohamed,
On 2021-01-20 11:36, Mohamed Mediouni wrote:
From: Stan Skowronek
On Apple processors, the timer is wired through FIQ.
Which timer? There are at least 3, potentially 4 timers per CPU
that can fire.
As such, add FIQ support to the kernel.
Signed-off-by: Stan Skowronek
Missing SoB from the sender.
---
arch/arm64/include/asm/arch_gicv3.h | 2 +-
arch/arm64/include/asm/assembler.h | 8 ++--
arch/arm64/include/asm/daifflags.h | 4 +-
arch/arm64/include/asm/irq.h| 4 ++
arch/arm64/include/asm/irqflags.h | 6 +--
arch/arm64/kernel/entry.S | 74 ++---
arch/arm64/kernel/irq.c | 14 ++
arch/arm64/kernel/process.c | 2 +-
8 files changed, 97 insertions(+), 17 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");
If I trust the persistent rumour, this system doesn't have a GIC.
Why this change?
}
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/assembler.h
b/arch/arm64/include/asm/assembler.h
index bf125c591116..6fe55713dfe0 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 is the lowest priority flag, unconditionally unmask the
rest. */
+ .macro enable_da
+ msr daifclr, #(8 | 4)
This cannot be unconditional. This potentially changes existing
behaviours,
and I'd feel a lot safer if FIQ was only messed with on that specific
HW.
I have the feeling that this should be detected on the boot CPU and
patched
before any interrupt can fire.
.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..44de96c7fb1a 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -13,8 +13,8 @@
#include
#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_I_BIT | PSR_F_BIT | PSR_A_BIT)
#define DAIF_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
diff --git a/arch/arm64/include/asm/irq.h
b/arch/arm64/include/asm/irq.h
index b2b0c6405eb0..2d1537d3a245 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -13,5 +13,9 @@ static inline int nr_legacy_irqs(void)
return 0;
}
+int set_handle_fiq(void (*handle_fiq)(struct pt_regs *));
+
+extern void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init;
I guess this is set from the root interrupt controller, which also
will set handle_arch_irq? Why do we need two entry points? We have
ISR_EL1 to find out what is pending. Isn't that enough?
+
#endif /* !__ASSEMBLER__ */
#endif
diff --git a/arch/arm64/include/asm/irqflags.h
b/arch/arm64/include/asm/irqflags.h
index ff328e5bbb75..26d7f378113e 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -35,7 +35,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 +54,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)
:
@@ -85,7 +85,7 @@ static inline int arch_irqs_disabled_flags(unsigned
long flags)
int res;
asm volatile(ALTERNATIVE(
- "and %w0, %w1, #" __stringify(PSR_I_BIT),
+ "and %w0, %w1, #" __stringify(PSR_I_BIT | PSR_F_BIT),
"eor %w0, %w1, #" __stringify(GIC_PRIO_IRQON),
ARM64_HAS_IRQ_PRIO_MASKING)
: "=" (res)
diff --git a/arch/arm64/kernel/entry.S
[PATCH 1/3] arm64/kernel: FIQ support
From: Stan Skowronek
On Apple processors, the timer is wired through FIQ.
As such, add FIQ support to the kernel.
Signed-off-by: Stan Skowronek
---
arch/arm64/include/asm/arch_gicv3.h | 2 +-
arch/arm64/include/asm/assembler.h | 8 ++--
arch/arm64/include/asm/daifflags.h | 4 +-
arch/arm64/include/asm/irq.h| 4 ++
arch/arm64/include/asm/irqflags.h | 6 +--
arch/arm64/kernel/entry.S | 74 ++---
arch/arm64/kernel/irq.c | 14 ++
arch/arm64/kernel/process.c | 2 +-
8 files changed, 97 insertions(+), 17 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 bf125c591116..6fe55713dfe0 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 is the lowest priority flag, 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..44de96c7fb1a 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -13,8 +13,8 @@
#include
#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_I_BIT | PSR_F_BIT | PSR_A_BIT)
#define DAIF_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index b2b0c6405eb0..2d1537d3a245 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -13,5 +13,9 @@ static inline int nr_legacy_irqs(void)
return 0;
}
+int set_handle_fiq(void (*handle_fiq)(struct pt_regs *));
+
+extern void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init;
+
#endif /* !__ASSEMBLER__ */
#endif
diff --git a/arch/arm64/include/asm/irqflags.h
b/arch/arm64/include/asm/irqflags.h
index ff328e5bbb75..26d7f378113e 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -35,7 +35,7 @@ static inline void arch_local_irq_enable(void)
}
asm volatile(ALTERNATIVE(
- "msrdaifclr, #2 // arch_local_irq_enable",
+ "msrdaifclr, #3 // arch_local_irq_enable",
__msr_s(SYS_ICC_PMR_EL1, "%0"),
ARM64_HAS_IRQ_PRIO_MASKING)
:
@@ -54,7 +54,7 @@ static inline void arch_local_irq_disable(void)
}
asm volatile(ALTERNATIVE(
- "msrdaifset, #2 // arch_local_irq_disable",
+ "msrdaifset, #3 // arch_local_irq_disable",
__msr_s(SYS_ICC_PMR_EL1, "%0"),
ARM64_HAS_IRQ_PRIO_MASKING)
:
@@ -85,7 +85,7 @@ static inline int arch_irqs_disabled_flags(unsigned long
flags)
int res;
asm volatile(ALTERNATIVE(
- "and%w0, %w1, #" __stringify(PSR_I_BIT),
+ "and%w0, %w1, #" __stringify(PSR_I_BIT | PSR_F_BIT),
"eor%w0, %w1, #" __stringify(GIC_PRIO_IRQON),
ARM64_HAS_IRQ_PRIO_MASKING)
: "=" (res)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index c9bae73f2621..abcca0db0736 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -499,6 +499,14 @@ tsk.reqx28 // current thread_info
irq_stack_exit
.endm
+ .macro fiq_handler
+ ldr_l x1, handle_arch_fiq
+ mov x0, sp
+ irq_stack_entry
+ blr x1
+ irq_stack_exit
+ .endm
+
#ifdef CONFIG_ARM64_PSEUDO_NMI
/*
* Set res to 0 if irqs were unmasked in interrupted context.
@@ -547,18 +555,18 @@ SYM_CODE_START(vectors)
kernel_ventry 1, sync // Synchronous EL1h
kernel_ventry 1, irq // IRQ EL1h
-
