Re: [PATCH v8 4/7] RISC-V: Treat IPIs as normal Linux IRQs

[Marc Zyngier]

On 2022-08-26 19:48, Conor.Dooley@xxxxxxxxxxxxx wrote:
> On 20/08/2022 07:54, Anup Patel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> > the content is safe
> >
> > Currently, the RISC-V kernel provides arch specific hooks (i.e.
> > struct riscv_ipi_ops) to register IPI handling methods. The stats
> > gathering of IPIs is also arch specific in the RISC-V kernel.
> >
> > Other architectures (such as ARM, ARM64, and MIPS) have moved away
> > from custom arch specific IPI handling methods. Currently, these
> > architectures have Linux irqchip drivers providing a range of Linux
> > IRQ numbers to be used as IPIs and IPI triggering is done using
> > generic IPI APIs. This approach allows architectures to treat IPIs
> > as normal Linux IRQs and IPI stats gathering is done by the generic
> > Linux IRQ subsystem.
> >
> > We extend the RISC-V IPI handling as-per above approach so that arch
> > specific IPI handling methods (struct riscv_ipi_ops) can be removed
> > and the IPI handling is done through the Linux IRQ subsystem.
> >
> > Signed-off-by: Anup Patel <apatel@xxxxxxxxxxxxxxxx>
>
> > +void riscv_ipi_set_virq_range(int virq, int nr)
> > +{
> > +       int i, err;
> >
> > -               if (ops & (1 << IPI_IRQ_WORK)) {
> > -                       stats[IPI_IRQ_WORK]++;
> > -                       irq_work_run();
> > -               }
> > +       if (WARN_ON(ipi_virq_base))
> > +               return;
> >
> > -#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
> > -               if (ops & (1 << IPI_TIMER)) {
> > -                       stats[IPI_TIMER]++;
> > -                       tick_receive_broadcast();
> > -               }
> > -#endif
> > -               BUG_ON((ops >> IPI_MAX) != 0);
> > +       WARN_ON(nr < IPI_MAX);
> > +       nr_ipi = min(nr, IPI_MAX);
> > +       ipi_virq_base = virq;
> > +
> > +       /* Request IPIs */
> > +       for (i = 0; i < nr_ipi; i++) {
> > +               err = request_percpu_irq(ipi_virq_base + i, 
> > handle_IPI,
> > +                                        "IPI", &ipi_virq_base);
>
> FWIW, ?sparse? does not like this:
> arch/riscv/kernel/smp.c:163:50: warning: incorrect type in argument 4
> (different address spaces)
> arch/riscv/kernel/smp.c:163:50:    expected void [noderef] __percpu
> *percpu_dev_id
> arch/riscv/kernel/smp.c:163:50:    got int *

Huh, well spotted. This will totally give the wrong sort of
result, as this is used as a percpu variable from the irq
core code.

The arm64 code passes instead a pointer to the CPU number, which
is not very useful, but at least not completely wrong.

I'm sure the RISC-V code has some sort of semi-useful data to
stuff in there instead of this.

        M.
--
Jazz is not dead. It just smells funny...