iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

irqchip/gic-v3: Refactor ISB + EOIR at ack time

Browse Source 
There are cases where a context synchronization event is necessary
between an IRQ being raised and being handled, and there are races such
that we cannot rely upon the exception entry being subsequent to the
interrupt being raised. To fix this, we place an ISB between a read of
IAR and the subsequent invocation of an IRQ handler.

When EOI mode 1 is in use, we need to EOI an interrupt prior to invoking
its handler, and we have a write to EOIR for this. As this write to EOIR
requires an ISB, and this is provided by the gic_write_eoir() helper, we
omit the usual ISB in this case, with the logic being:

|	if (static_branch_likely(&supports_deactivate_key))
|		gic_write_eoir(irqnr);
|	else
|		isb();

This is somewhat opaque, and it would be a little clearer if there were
an unconditional ISB, with only the write to EOIR being conditional,
e.g.

|	if (static_branch_likely(&supports_deactivate_key))
|		write_gicreg(irqnr, ICC_EOIR1_EL1);
|
|	isb();

This patch rewrites the code that way, with this logic factored into a
new helper function with comments explaining what the ISB is for, as
were originally laid out in commit:

  39a06b67c2 ("irqchip/gic: Ensure we have an ISB between ack and ->handle_irq")

Note that since then, we removed the IAR polling in commit:

  342677d70a ("irqchip/gic-v3: Remove acknowledge loop")

... which removed one of the two race conditions.

For consistency, other portions of the driver are made to manipulate
EOIR using write_gicreg() and explcit ISBs, and the gic_write_eoir()
helper function is removed.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20220513133038.226182-3-mark.rutland@arm.com
This commit is contained in:
Mark Rutland 2022-05-13 14:30:37 +01:00 committed by Marc Zyngier
parent adf14453d2
commit 6efb509237
3 changed files with 34 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
arch/arm/include/asm/arch_gicv3.h
View File

@ -48,6 +48,7 @@ static inline u32 read_ ## a64(void) \
return read_sysreg(a32); \
} \
CPUIF_MAP(ICC_EOIR1, ICC_EOIR1_EL1)
CPUIF_MAP(ICC_PMR, ICC_PMR_EL1)
CPUIF_MAP(ICC_AP0R0, ICC_AP0R0_EL1)
CPUIF_MAP(ICC_AP0R1, ICC_AP0R1_EL1)
@ -63,12 +64,6 @@ CPUIF_MAP(ICC_AP1R3, ICC_AP1R3_EL1)
/* Low-level accessors */
static inline void gic_write_eoir(u32 irq)
{
write_sysreg(irq, ICC_EOIR1);
isb();
}
static inline void gic_write_dir(u32 val)
{
write_sysreg(val, ICC_DIR);

6
arch/arm64/include/asm/arch_gicv3.h
View File

@ -26,12 +26,6 @@
* sets the GP register's most significant bits to 0 with an explicit cast.
*/
static inline void gic_write_eoir(u32 irq)
{
write_sysreg_s(irq, SYS_ICC_EOIR1_EL1);
isb();
}
static __always_inline void gic_write_dir(u32 irq)
{
write_sysreg_s(irq, SYS_ICC_DIR_EL1);

43
drivers/irqchip/irq-gic-v3.c
View File

@ -556,7 +556,8 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
static void gic_eoi_irq(struct irq_data *d)
{
gic_write_eoir(gic_irq(d));
write_gicreg(gic_irq(d), ICC_EOIR1_EL1);
isb();
}
static void gic_eoimode1_eoi_irq(struct irq_data *d)
@ -640,10 +641,38 @@ static void gic_deactivate_unhandled(u32 irqnr)
if (irqnr < 8192)
gic_write_dir(irqnr);
} else {
gic_write_eoir(irqnr);
write_gicreg(irqnr, ICC_EOIR1_EL1);
isb();
}
}
/*
* Follow a read of the IAR with any HW maintenance that needs to happen prior
* to invoking the relevant IRQ handler. We must do two things:
*
* (1) Ensure instruction ordering between a read of IAR and subsequent
* instructions in the IRQ handler using an ISB.
*
* It is possible for the IAR to report an IRQ which was signalled *after*
* the CPU took an IRQ exception as multiple interrupts can race to be
* recognized by the GIC, earlier interrupts could be withdrawn, and/or
* later interrupts could be prioritized by the GIC.
*
* For devices which are tightly coupled to the CPU, such as PMUs, a
* context synchronization event is necessary to ensure that system
* register state is not stale, as these may have been indirectly written
* *after* exception entry.
*
* (2) Deactivate the interrupt when EOI mode 1 is in use.
*/
static inline void gic_complete_ack(u32 irqnr)
{
if (static_branch_likely(&supports_deactivate_key))
write_gicreg(irqnr, ICC_EOIR1_EL1);
isb();
}
static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
{
bool irqs_enabled = interrupts_enabled(regs);
@ -652,10 +681,7 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
if (irqs_enabled)
nmi_enter();
if (static_branch_likely(&supports_deactivate_key))
gic_write_eoir(irqnr);
else
isb()
gic_complete_ack(irqnr);
/*
* Leave the PSR.I bit set to prevent other NMIs to be
@ -726,10 +752,7 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
gic_arch_enable_irqs();
}
if (static_branch_likely(&supports_deactivate_key))
gic_write_eoir(irqnr);
else
isb();
gic_complete_ack(irqnr);
if (generic_handle_domain_irq(gic_data.domain, irqnr)) {
WARN_ONCE(true, "Unexpected interrupt received!\n");