Re: [PATCH 2/2] irqchip/armada-370-xp: Do not allow mapping IRQ 0 and 1

[Pali Rohár]

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.