| Message ID | 1542023835-21446-6-git-send-email-julien.thierry@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: provide pseudo NMI with GICv3 | expand |
On Mon, Nov 12, 2018 at 11:56:56AM +0000, Julien Thierry wrote: > Mask the IRQ priority through PMR and re-enable IRQs at CPU level, > allowing only higher priority interrupts to be received during interrupt > handling. > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/include/asm/arch_gicv3.h | 17 +++++++++++++++++ > arch/arm64/include/asm/arch_gicv3.h | 17 +++++++++++++++++ > drivers/irqchip/irq-gic-v3.c | 10 ++++++++++ > 3 files changed, 44 insertions(+) > > diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h > index bef0b5d..f6f485f 100644 > --- a/arch/arm/include/asm/arch_gicv3.h > +++ b/arch/arm/include/asm/arch_gicv3.h > @@ -363,5 +363,22 @@ static inline void gits_write_vpendbaser(u64 val, void * __iomem addr) > > #define gits_read_vpendbaser(c) __gic_readq_nonatomic(c) > > +static inline bool gic_prio_masking_enabled(void) > +{ > + return false; > +} > + > +static inline void gic_pmr_mask_irqs(void) > +{ > + /* Should not get called. */ > + WARN_ON_ONCE(true); > +} > + > +static inline void gic_arch_enable_irqs(void) > +{ > + /* Should not get called. */ > + WARN_ON_ONCE(true); > +} > + > #endif /* !__ASSEMBLY__ */ > #endif /* !__ASM_ARCH_GICV3_H */ > diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h > index 37193e2..3f8d5f4 100644 > --- a/arch/arm64/include/asm/arch_gicv3.h > +++ b/arch/arm64/include/asm/arch_gicv3.h > @@ -155,5 +155,22 @@ static inline u32 gic_read_rpr(void) > #define gits_write_vpendbaser(v, c) writeq_relaxed(v, c) > #define gits_read_vpendbaser(c) readq_relaxed(c) > > +static inline bool gic_prio_masking_enabled(void) > +{ > + return system_supports_irq_prio_masking(); > +} > + > +static inline void gic_pmr_mask_irqs(void) > +{ > + /* Should not get called yet. */ > + WARN_ON_ONCE(true); > +} > + > +static inline void gic_arch_enable_irqs(void) > +{ > + /* Should not get called yet. */ > + WARN_ON_ONCE(true); > +} > + > #endif /* __ASSEMBLY__ */ > #endif /* __ASM_ARCH_GICV3_H */ > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 8f87f40..e5d8c14 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -353,6 +353,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs > if (likely(irqnr > 15 && irqnr < 1020) || irqnr >= 8192) { > int err; > > + if (gic_prio_masking_enabled()) { > + gic_pmr_mask_irqs(); > + gic_arch_enable_irqs(); > + } IIUC, if we have two pNMIs, this will allow one to preempt another, e.g. < pNMI#1 asserted > < CPU takes IRQ exception for pNMI #1> irqnr = gic_read_iar(); // pNMI #1 < pNMI#2 asserted > // masks IRQs at GIC, leaves other pNMIs unmasked gic_pmr_mask_irqs() gic_arch_enable_irqs(); ... < CPU takes IRQ exception for pNMI #2 > ... or is that not a problem? Is the NMI code re-entrant? > + > if (static_branch_likely(&supports_deactivate_key)) > gic_write_eoir(irqnr); > else > @@ -371,6 +376,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs > return; > } > if (irqnr < 16) { > + if (gic_prio_masking_enabled()) { > + gic_pmr_mask_irqs(); > + gic_arch_enable_irqs(); > + } Can we pull this above the two cases, or is there a problem with doing this for spurious IRQs? Where is the corresponding unmask of the PMR, and disable of IRQs? It's difficult to follow the logic if that's in another patch. Thanks, Mark.
On 29/11/18 18:12, Mark Rutland wrote: > On Mon, Nov 12, 2018 at 11:56:56AM +0000, Julien Thierry wrote: >> Mask the IRQ priority through PMR and re-enable IRQs at CPU level, >> allowing only higher priority interrupts to be received during interrupt >> handling. >> >> Signed-off-by: Julien Thierry <julien.thierry@arm.com> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Jason Cooper <jason@lakedaemon.net> >> Cc: Marc Zyngier <marc.zyngier@arm.com> >> --- >> arch/arm/include/asm/arch_gicv3.h | 17 +++++++++++++++++ >> arch/arm64/include/asm/arch_gicv3.h | 17 +++++++++++++++++ >> drivers/irqchip/irq-gic-v3.c | 10 ++++++++++ >> 3 files changed, 44 insertions(+) >> >> diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h >> index bef0b5d..f6f485f 100644 >> --- a/arch/arm/include/asm/arch_gicv3.h >> +++ b/arch/arm/include/asm/arch_gicv3.h >> @@ -363,5 +363,22 @@ static inline void gits_write_vpendbaser(u64 val, void * __iomem addr) >> >> #define gits_read_vpendbaser(c) __gic_readq_nonatomic(c) >> >> +static inline bool gic_prio_masking_enabled(void) >> +{ >> + return false; >> +} >> + >> +static inline void gic_pmr_mask_irqs(void) >> +{ >> + /* Should not get called. */ >> + WARN_ON_ONCE(true); >> +} >> + >> +static inline void gic_arch_enable_irqs(void) >> +{ >> + /* Should not get called. */ >> + WARN_ON_ONCE(true); >> +} >> + >> #endif /* !__ASSEMBLY__ */ >> #endif /* !__ASM_ARCH_GICV3_H */ >> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h >> index 37193e2..3f8d5f4 100644 >> --- a/arch/arm64/include/asm/arch_gicv3.h >> +++ b/arch/arm64/include/asm/arch_gicv3.h >> @@ -155,5 +155,22 @@ static inline u32 gic_read_rpr(void) >> #define gits_write_vpendbaser(v, c) writeq_relaxed(v, c) >> #define gits_read_vpendbaser(c) readq_relaxed(c) >> >> +static inline bool gic_prio_masking_enabled(void) >> +{ >> + return system_supports_irq_prio_masking(); >> +} >> + >> +static inline void gic_pmr_mask_irqs(void) >> +{ >> + /* Should not get called yet. */ >> + WARN_ON_ONCE(true); >> +} >> + >> +static inline void gic_arch_enable_irqs(void) >> +{ >> + /* Should not get called yet. */ >> + WARN_ON_ONCE(true); >> +} >> + >> #endif /* __ASSEMBLY__ */ >> #endif /* __ASM_ARCH_GICV3_H */ >> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c >> index 8f87f40..e5d8c14 100644 >> --- a/drivers/irqchip/irq-gic-v3.c >> +++ b/drivers/irqchip/irq-gic-v3.c >> @@ -353,6 +353,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs >> if (likely(irqnr > 15 && irqnr < 1020) || irqnr >= 8192) { >> int err; >> >> + if (gic_prio_masking_enabled()) { >> + gic_pmr_mask_irqs(); >> + gic_arch_enable_irqs(); >> + } > > IIUC, if we have two pNMIs, this will allow one to preempt another, e.g. > > < pNMI#1 asserted > > > < CPU takes IRQ exception for pNMI #1> > > irqnr = gic_read_iar(); // pNMI #1 > > < pNMI#2 asserted > > > // masks IRQs at GIC, leaves other pNMIs unmasked > gic_pmr_mask_irqs() > gic_arch_enable_irqs(); > > ... > > < CPU takes IRQ exception for pNMI #2 > > > ... or is that not a problem? Is the NMI code re-entrant? At this patch stage, the GICv3 handling code is not supporting pseudo-NMIs yet, only using interrupt priorities. I introduce this in patch 18: irqchip/gic-v3: Handle pseudo-NMIs. And yes, this would be an issue but only after gic_write_eoir. Once an interrupt is running, its priority becomes the running priority and only an interrupt with higher priority can preempt it. When we do the eoir, the running priority is dropped meaning any interrupt can preempt (if PSR.I is cleared and its priority is not masked by PMR). What is done in the later patch for pseudo-NMIs is that we don't switch to PMR masking and leave the I bit set since we don't want anything to preempt it. > >> + >> if (static_branch_likely(&supports_deactivate_key)) >> gic_write_eoir(irqnr); >> else >> @@ -371,6 +376,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs >> return; >> } >> if (irqnr < 16) { >> + if (gic_prio_masking_enabled()) { >> + gic_pmr_mask_irqs(); >> + gic_arch_enable_irqs(); >> + } > > Can we pull this above the two cases, or is there a problem with doing > this for spurious IRQs? > So the reason I split this is to avoid doing it for NMIs. Otherwise we would mask PMR, clear PSR.I and once we know we have an NMI set PSR.I again. An alternative to it would be to check whether we have an NMI before we know the type of interrupt we have, which would be unnecessary for IPIs and of course spurious interrupts. Maybe I can always mask PMR and clear PSR.I (after the interrupt ack) at this stage and then do the necessary modifications once pseudo-NMI handling code is added? > Where is the corresponding unmask of the PMR, and disable of IRQs? It's > difficult to follow the logic if that's in another patch. > In patch 7: arm64: Make PMR part of task context The arch/arm64 code becomes responsible for saving/restoring the value of PMR upon exception entry/return. I'm not really sure whether it makes sense to merge both patches though. Would explaining that PMR will get restored on irq return in the commit message be sufficient? Thanks,
On Mon, Nov 12, 2018 at 11:56:56AM +0000, Julien Thierry wrote: > Mask the IRQ priority through PMR and re-enable IRQs at CPU level, > allowing only higher priority interrupts to be received during interrupt > handling. > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/include/asm/arch_gicv3.h | 17 +++++++++++++++++ > arch/arm64/include/asm/arch_gicv3.h | 17 +++++++++++++++++ > drivers/irqchip/irq-gic-v3.c | 10 ++++++++++ > 3 files changed, 44 insertions(+) For the arm64 bits: Acked-by: Catalin Marinas <catalin.marinas@arm.com> (this time without the legal disclaimer ;))
diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h index bef0b5d..f6f485f 100644 --- a/arch/arm/include/asm/arch_gicv3.h +++ b/arch/arm/include/asm/arch_gicv3.h @@ -363,5 +363,22 @@ static inline void gits_write_vpendbaser(u64 val, void * __iomem addr) #define gits_read_vpendbaser(c) __gic_readq_nonatomic(c) +static inline bool gic_prio_masking_enabled(void) +{ + return false; +} + +static inline void gic_pmr_mask_irqs(void) +{ + /* Should not get called. */ + WARN_ON_ONCE(true); +} + +static inline void gic_arch_enable_irqs(void) +{ + /* Should not get called. */ + WARN_ON_ONCE(true); +} + #endif /* !__ASSEMBLY__ */ #endif /* !__ASM_ARCH_GICV3_H */ diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h index 37193e2..3f8d5f4 100644 --- a/arch/arm64/include/asm/arch_gicv3.h +++ b/arch/arm64/include/asm/arch_gicv3.h @@ -155,5 +155,22 @@ static inline u32 gic_read_rpr(void) #define gits_write_vpendbaser(v, c) writeq_relaxed(v, c) #define gits_read_vpendbaser(c) readq_relaxed(c) +static inline bool gic_prio_masking_enabled(void) +{ + return system_supports_irq_prio_masking(); +} + +static inline void gic_pmr_mask_irqs(void) +{ + /* Should not get called yet. */ + WARN_ON_ONCE(true); +} + +static inline void gic_arch_enable_irqs(void) +{ + /* Should not get called yet. */ + WARN_ON_ONCE(true); +} + #endif /* __ASSEMBLY__ */ #endif /* __ASM_ARCH_GICV3_H */ diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 8f87f40..e5d8c14 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -353,6 +353,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs if (likely(irqnr > 15 && irqnr < 1020) || irqnr >= 8192) { int err; + if (gic_prio_masking_enabled()) { + gic_pmr_mask_irqs(); + gic_arch_enable_irqs(); + } + if (static_branch_likely(&supports_deactivate_key)) gic_write_eoir(irqnr); else @@ -371,6 +376,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs return; } if (irqnr < 16) { + if (gic_prio_masking_enabled()) { + gic_pmr_mask_irqs(); + gic_arch_enable_irqs(); + } + gic_write_eoir(irqnr); if (static_branch_likely(&supports_deactivate_key)) gic_write_dir(irqnr);
Mask the IRQ priority through PMR and re-enable IRQs at CPU level, allowing only higher priority interrupts to be received during interrupt handling. Signed-off-by: Julien Thierry <julien.thierry@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Jason Cooper <jason@lakedaemon.net> Cc: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/include/asm/arch_gicv3.h | 17 +++++++++++++++++ arch/arm64/include/asm/arch_gicv3.h | 17 +++++++++++++++++ drivers/irqchip/irq-gic-v3.c | 10 ++++++++++ 3 files changed, 44 insertions(+)