Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Date:   Fri, 26 Aug 2022 19:57:22 +0100
From:   Marc Zyngier <maz@kernel.org>
To:     Conor.Dooley@microchip.com
Cc:     apatel@ventanamicro.com, palmer@dabbelt.com,
        paul.walmsley@sifive.com, tglx@linutronix.de,
        daniel.lezcano@linaro.org, atishp@atishpatra.org,
        Alistair.Francis@wdc.com, anup@brainfault.org,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 4/7] RISC-V: Treat IPIs as normal Linux IRQs
In-Reply-To: <8f4ae429-0f12-2096-c07b-fe43b3abb4fe@microchip.com>
References: <20220820065446.389788-1-apatel@ventanamicro.com>
 <20220820065446.389788-5-apatel@ventanamicro.com>
 <8f4ae429-0f12-2096-c07b-fe43b3abb4fe@microchip.com>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <69e58f4dc2b74415a32a97998e862479@kernel.org>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022-08-26 19:48, Conor.Dooley@microchip.com 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@ventanamicro.com>
> 
>> +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...