| Message ID | 20220425113706.29310-2-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] irqchip/armada-370-xp: Do not touch Performance Counter Overflow on A375, A38x, A39x | expand |
On Mon, Apr 25, 2022 at 01:37:06PM +0200, Pali Rohár wrote: > IRQs 0 and 1 cannot be mapped, they are handled internally by this driver > and this driver does not call generic_handle_domain_irq() for these IRQs. > So do not allow mapping these IRQs and correctly propagate error from the > .irq_map callback. So you are referring to this? /* Check if the interrupt is not masked on current CPU. * Test IRQ (0-1) and FIQ (8-9) mask bits. */ if (!(irqsrc & ARMADA_370_XP_INT_IRQ_FIQ_MASK(cpuid))) continue; if (irqn == 1) { armada_370_xp_handle_msi_irq(NULL, true); continue; } Should the two FIQ interrupts also return -EINVAL? Andrew
On Friday 29 April 2022 14:30:53 Andrew Lunn wrote: > On Mon, Apr 25, 2022 at 01:37:06PM +0200, Pali Rohár wrote: > > IRQs 0 and 1 cannot be mapped, they are handled internally by this driver > > and this driver does not call generic_handle_domain_irq() for these IRQs. > > So do not allow mapping these IRQs and correctly propagate error from the > > .irq_map callback. > > So you are referring to this? > > /* Check if the interrupt is not masked on current CPU. > * Test IRQ (0-1) and FIQ (8-9) mask bits. > */ > if (!(irqsrc & ARMADA_370_XP_INT_IRQ_FIQ_MASK(cpuid))) > continue; > > if (irqn == 1) { > armada_370_xp_handle_msi_irq(NULL, true); > continue; > } I'm referring to irqn, which is not handled on armada_370_xp_mpic_domain via generic_handle_domain_irq() when it is == 1. Also I'm referring to another section: if (irqnr > 1) { generic_handle_domain_irq(armada_370_xp_mpic_domain, irqnr); continue; } /* MSI handling */ if (irqnr == 1) armada_370_xp_handle_msi_irq(regs, false); #ifdef CONFIG_SMP /* IPI Handling */ if (irqnr == 0) { unsigned long ipimask; int ipi; ipimask = readl_relaxed(per_cpu_int_base + ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS) & IPI_DOORBELL_MASK; for_each_set_bit(ipi, &ipimask, IPI_DOORBELL_END) generic_handle_domain_irq(ipi_domain, ipi); } #endif First 'if (irqnr > 1)' just cause that irqnr 0 and 1 are not handled on armada_370_xp_mpic_domain via generic_handle_domain_irq(). > > Should the two FIQ interrupts also return -EINVAL? > > Andrew No. Following code irqsrc = readl_relaxed(main_int_base + ARMADA_370_XP_INT_SOURCE_CTL(irqn)); /* Check if the interrupt is not masked on current CPU. * Test IRQ (0-1) and FIQ (8-9) mask bits. */ if (!(irqsrc & ARMADA_370_XP_INT_IRQ_FIQ_MASK(cpuid))) continue; skips processing irqn interrupt if it is masked for the current CPU. Interrupt irqn is **unmasked** on CPU0 if either bit 0 or 8 is set in register ARMADA_370_XP_INT_IRQ_FIQ_MASK. And **unmasked** on CPU1 if either bit 1 or 9 is set. The reason for this check and skipping is because some per-cpu interrupts on A375, A38x and A39x can be handled directly via GIC and also via MPIC subhierarchy (it depends what you put into DTS, both options are possible and working, just some interrupts are MPIC-only). So if you unmask some interrupt on GIC and interrupt is triggered then also MPIC per-CPU cause register contains in bit for that triggered interrupt (even you have not asked MPIC to unmask interrupt). And once another MPIC interrupt is triggered then routine for handling MPIC subhierarchy see that some cause bits are set and tried to call generic_handle_domain_irq() for all of them, also for those which are masked. That is why it is needed to check if MPIC interrupt is masked on the current CPU or not prior processing it. So those bits 0-1 and 8-9 have nothing with mapping interrupts.
On Mon, 25 Apr 2022 12:37:06 +0100, Pali Rohár <pali@kernel.org> wrote: > > IRQs 0 and 1 cannot be mapped, they are handled internally by this driver > and this driver does not call generic_handle_domain_irq() for these IRQs. > So do not allow mapping these IRQs and correctly propagate error from the > .irq_map callback. > > Signed-off-by: Pali Rohár <pali@kernel.org> > Cc: stable@vger.kernel.org > --- > drivers/irqchip/irq-armada-370-xp.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c > index 1120084cba09..ebd76ea1c69b 100644 > --- a/drivers/irqchip/irq-armada-370-xp.c > +++ b/drivers/irqchip/irq-armada-370-xp.c > @@ -546,6 +546,10 @@ static struct irq_chip armada_370_xp_irq_chip = { > static int armada_370_xp_mpic_irq_map(struct irq_domain *h, > unsigned int virq, irq_hw_number_t hw) > { > + /* IRQs 0 and 1 cannot be mapped, they are handled internally */ > + if (hw <= 1) > + return -EINVAL; > + > armada_370_xp_irq_mask(irq_get_irq_data(virq)); > if (!is_percpu_irq(hw)) > writel(hw, per_cpu_int_base + > -- > 2.20.1 > > Given that this is completely academic and obviously doesn't affect anyone, I have removed the Cc: stable from the patch when applying it. M.
diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c index 1120084cba09..ebd76ea1c69b 100644 --- a/drivers/irqchip/irq-armada-370-xp.c +++ b/drivers/irqchip/irq-armada-370-xp.c @@ -546,6 +546,10 @@ static struct irq_chip armada_370_xp_irq_chip = { static int armada_370_xp_mpic_irq_map(struct irq_domain *h, unsigned int virq, irq_hw_number_t hw) { + /* IRQs 0 and 1 cannot be mapped, they are handled internally */ + if (hw <= 1) + return -EINVAL; + armada_370_xp_irq_mask(irq_get_irq_data(virq)); if (!is_percpu_irq(hw)) writel(hw, per_cpu_int_base +
IRQs 0 and 1 cannot be mapped, they are handled internally by this driver and this driver does not call generic_handle_domain_irq() for these IRQs. So do not allow mapping these IRQs and correctly propagate error from the .irq_map callback. Signed-off-by: Pali Rohár <pali@kernel.org> Cc: stable@vger.kernel.org --- drivers/irqchip/irq-armada-370-xp.c | 4 ++++ 1 file changed, 4 insertions(+)