On Wed, Jan 31, 2024, at 10:52, Yann Sionneau wrote:
> On 22/01/2023 12:54, Krzysztof Kozlowski wrote:
>> On 20/01/2023 15:10, Yann Sionneau wrote:
>>> +
>>> +int __init kvx_ipi_ctrl_probe(irqreturn_t (*ipi_irq_handler)(int, void *))
>>> +{
>>> + struct device_node *np;
>>> + int ret;
>>> + unsigned int ipi_irq;
>>> + void __iomem *ipi_base;
>>> +
>>> + np = of_find_compatible_node(NULL, NULL, "kalray,kvx-ipi-ctrl");
>> Nope, big no.
>>
>> Drivers go to drivers, not to arch code. Use proper driver infrastructure.
> Thank you for your review.
>
> It raises questions on our side about how to handle this change.
>
> First let me describe the hardware:
>
> The coolidge ipi controller device handles IPI communication between cpus
> inside a cluster.
>
> Each cpu has 8 of its dedicated irq lines (24 to 31) hard-wired to the ipi.
> The ipi controller has 8 sets of 2 registers:
> - a 17-bit "interrupt" register
> - a 17-bit "mask" register
>
> Each couple of register is dedicated to 1 of the 8 irqlines.
> Each of the 17 bits of interrupt/mask register
> identifies a cpu (cores 0 to 15 + secure_core).
> Writing bit i in interrupt register sends an irq to cpu i, according to the mask
> in mask register.
> Writing in interrupt/mask register couple N targets irq line N of the core.
>
> - Ipi generates an interrupt to the cpu when message is ready.
> - Messages are delivered via Axi.
> - Ipi does not have any interrupt input lines.
>
>
> +---------------+ irq axi_w
> | | i |<--/--- ipi <------
> | CPU | n | x8
> | core0 | t |
> | | c | irq irq msi
> | | t |<--/--- apic <----- mbox <-------
> | | l | x4
> +---------------+
> with intctl = core-irq controller
>
>
> We analyzed how other Linux ports are handling this situation (IPI) and
> here are several possible solutions:
>
> 1/ put everything in smp.c like what longarch is doing.
> * Except that IPI in longarch seems to involve writing to a special
> purpose CPU register and not doing a memory mapped write like kvx.
>
> 2/ write a device driver in drivers/xxx/ with the content from ipi.c
> * the probe would just ioremap the reg from DT and register the irq
> using request_percpu_irq()
> * it would export a function "kvx_ipi_send()" that would directly be
> called by smp.c
> * Question : where would this driver be placed in drivers/ ?
> drivers/irqchip/ ? Even if this is not per-se an interrupt-controller
> driver?
This looks like it's close enough to the irqchip driver
that you can just have it in the same file as the 'intctl'
portion.
Top-level irqchip implementations tend to be rather architecture
specific, as does the IPI mechanism. Depending on the register
layout, I think you can have a single devicetree node for
the combination of the core-irq (for managing your
own interrupts) and the ipi (for receiving interrupts from
others), and then have a driver in drivers/irqchip to
deal with both. For the ipi mechanism, trying to abstract
it too much generally makes it too slow, so I would not
go through a nested irqchip or a mailbox driver etc.
I don't know what the 'apic' in your diagram is, so that
would be either a nested irqchip or could be part of
the same driver as well.
Arnd
