Re: [PATCH v4 18/26] arm64: smp: Support non-SGIs for IPIs

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

On Wed, May 28, 2025 at 01:17:44PM +0100, Jonathan Cameron wrote:
> On Tue, 13 May 2025 19:48:11 +0200
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> 
> > From: Marc Zyngier <maz@kernel.org>
> > 
> > The arm64 arch has relied so far on GIC architectural software
> > generated interrupt (SGIs) to handle IPIs. Those are per-cpu
> > software generated interrupts.
> > 
> > arm64 architecture code that allocates the IPIs virtual IRQs and
> > IRQ descriptors was written accordingly.
> > 
> > On GICv5 systems, IPIs are implemented using LPIs that are not
> > per-cpu interrupts - they are just normal routable IRQs.
> > 
> > Add arch code to set-up IPIs on systems where they are handled
> > using normal routable IRQs.
> > 
> > For those systems, force the IRQ affinity (and make it immutable)
> > to the cpu a given IRQ was assigned to.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > [timothy.hayes@arm.com: fixed ipi/irq conversion, irq flags]
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > [lpieralisi: changed affinity set-up, log]
> > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> Hi Lorenzo,
> 
> A few trivial comments inline.
> 
> > +
> > +static int ipi_to_irq(int ipi, int cpu)
> 
> Maybe this naming needs a breadcrumb to indicate this only
> applies only to lpi case as it's directly computed in the old ppi code?
> A comment might do the job.

Maybe rename it to ipi_to_irq_percpu() (similar to what we did for
set_smp_ipi_range()) and then

static int ipi_to_irq(int ipi)
{
	ipi_to_irq_percpu(ipi, 0);
}

and use ipi_to_irq() in ppi code ?

Likely overkill, not a big deal anyway.

> > +{
> > +	return ipi_irq_base + (cpu * nr_ipi) + ipi;
> > +}
> > +
> > +static int irq_to_ipi(int irq)
> > +{
> > +	return (irq - ipi_irq_base) % nr_ipi;
> > +}
> 
> 
> > +static void ipi_setup_lpi(int ipi, int ncpus)
> > +{
> > +	for (int cpu = 0; cpu < ncpus; cpu++) {
> > +		int err, irq;
> > +
> > +		irq = ipi_to_irq(ipi, cpu);
> > +
> > +		err = irq_force_affinity(irq, cpumask_of(cpu));
> > +
> Trivial local consistency thing but maybe no blank line here or...
> > +		WARN(err, "Could not force affinity IRQ %d, err=%d\n", irq, err);
> > +
> > +		err = request_irq(irq, ipi_handler, IRQF_NO_AUTOEN, "IPI",
> > +				  &irq_stat);
> > +
> here to match the style in ipi_setup_ppi()

Done.

Thanks,
Lorenzo

> > +		WARN(err, "Could not request IRQ %d, err=%d\n", irq, err);
> > +
> > +		irq_set_status_flags(irq, (IRQ_HIDDEN | IRQ_NO_BALANCING_MASK));
> > +
> > +		get_ipi_desc(cpu, ipi) = irq_to_desc(irq);
> > +	}
> > +}
>