Received-SPF: pass (google.com: domain of linux-kernel+bounces-46232-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-144-ge5821d614e-fm-20240125.002-ge5821d61
Precedence: bulk
MIME-Version: 1.0
Message-Id: <1036060c-25dd-439b-b081-893d0cb000f6@app.fastmail.com>
In-Reply-To: <269edff0-d989-4ac8-b0c3-bce31283806b@kalrayinc.com>
References: <20230120141002.2442-1-ysionneau@kalray.eu>
 <20230120141002.2442-32-ysionneau@kalray.eu>
 <995eb624-3efe-10fc-a6ed-883d52d591bb@linaro.org>
 <269edff0-d989-4ac8-b0c3-bce31283806b@kalrayinc.com>
Date: Wed, 31 Jan 2024 11:28:57 +0100
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Yann Sionneau" <ysionneau@kalrayinc.com>,
 "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
 "Yann Sionneau" <ysionneau@kalray.eu>,
 "Jonathan Corbet" <corbet@lwn.net>,
 "Thomas Gleixner" <tglx@linutronix.de>, "Marc Zyngier" <maz@kernel.org>,
 "Rob Herring" <robh+dt@kernel.org>,
 "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
 "Will Deacon" <will@kernel.org>, "Peter Zijlstra" <peterz@infradead.org>,
 "Boqun Feng" <boqun.feng@gmail.com>,
 "Mark Rutland" <mark.rutland@arm.com>,
 "Eric W. Biederman" <ebiederm@xmission.com>,
 "Kees Cook" <keescook@chromium.org>, "Oleg Nesterov" <oleg@redhat.com>,
 "Ingo Molnar" <mingo@redhat.com>, "Waiman Long" <longman@redhat.com>,
 "Aneesh Kumar" <aneesh.kumar@linux.ibm.com>,
 "Andrew Morton" <akpm@linux-foundation.org>,
 "Nicholas Piggin" <npiggin@gmail.com>,
 "Paul Moore" <paul@paul-moore.com>, "Eric Paris" <eparis@redhat.com>,
 "Christian Brauner" <brauner@kernel.org>,
 "Paul Walmsley" <paul.walmsley@sifive.com>,
 "Palmer Dabbelt" <palmer@dabbelt.com>,
 "Albert Ou" <aou@eecs.berkeley.edu>,
 "Jules Maselbas" <jmaselbas@kalray.eu>,
 "Guillaume Thouvenin" <gthouvenin@kalray.eu>,
 "Clement Leger" <clement@clement-leger.fr>,
 "Vincent Chardon" <vincent.chardon@elsys-design.com>,
 =?UTF-8?Q?Marc_Poulhi=C3=A8s?= <dkm@kataplop.net>,
 "Julian Vetter" <jvetter@kalray.eu>, "Samuel Jones" <sjones@kalray.eu>,
 "Ashley Lesdalons" <alesdalons@kalray.eu>,
 "Thomas Costis" <tcostis@kalray.eu>, "Marius Gligor" <mgligor@kalray.eu>,
 "Jonathan Borne" <jborne@kalray.eu>,
 "Julien Villette" <jvillette@kalray.eu>,
 "Luc Michel" <lmichel@kalray.eu>, "Louis Morhet" <lmorhet@kalray.eu>,
 "Julien Hascoet" <jhascoet@kalray.eu>,
 "Jean-Christophe Pince" <jcpince@gmail.com>,
 "Guillaume Missonnier" <gmissonnier@kalray.eu>,
 "Alex Michon" <amichon@kalray.eu>, "Huacai Chen" <chenhuacai@kernel.org>,
 "WANG Xuerui" <git@xen0n.name>,
 "Shaokun Zhang" <zhangshaokun@hisilicon.com>,
 "John Garry" <john.garry@huawei.com>,
 "Guangbin Huang" <huangguangbin2@huawei.com>,
 "Bharat Bhushan" <bbhushan2@marvell.com>,
 "Bibo Mao" <maobibo@loongson.cn>, "Atish Patra" <atishp@atishpatra.org>,
 "Jason A . Donenfeld" <Jason@zx2c4.com>, "Qi Liu" <liuqi115@huawei.com>,
 "Jiaxun Yang" <jiaxun.yang@flygoat.com>,
 "Catalin Marinas" <catalin.marinas@arm.com>,
 "Mark Brown" <broonie@kernel.org>,
 "Janosch Frank" <frankja@linux.ibm.com>,
 "Alexey Dobriyan" <adobriyan@gmail.com>,
 "Julian Vetter" <jvetter@kalrayinc.com>, jmaselbas@zdiv.net
Cc: "Benjamin Mugnier" <mugnier.benjamin@gmail.com>,
 linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
 devicetree@vger.kernel.org, linux-mm@kvack.org,
 Linux-Arch <linux-arch@vger.kernel.org>, linux-audit@redhat.com,
 linux-riscv@lists.infradead.org, bpf@vger.kernel.org
Subject: Re: [RFC PATCH v2 31/31] kvx: Add IPI driver
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable

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, v=
oid *))
>>> +{
>>> +	struct device_node *np;
>>> +	int ret;
>>> +	unsigned int ipi_irq;
>>> +	void __iomem *ipi_base;
>>> +
>>> +	np =3D of_find_compatible_node(NULL, NULL, "kalray,kvx-ipi-ctrl");
>> Nope, big no.
>>
>> Drivers go to drivers, not to arch code. Use proper driver infrastruc=
ture.
> 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 c=
pus
> 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 t=
o 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.
>
>
> =C2=A0 +---------------+=C2=A0=C2=A0 irq=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 axi_w
> =C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 i=C2=A0=
 |<--/--- ipi <------
> =C2=A0 | CPU=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 n=C2=A0 |=C2=A0 x8
> =C2=A0 |=C2=A0 core0=C2=A0 |=C2=A0 t=C2=A0 |
> =C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 c=C2=A0=
 |=C2=A0 irq=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 irq=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 msi
> =C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 t=C2=A0=
 |<--/--- apic <----- mbox <-------
> =C2=A0 |=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 l=C2=A0=
 |=C2=A0 x4
> =C2=A0 +---------------+
> =C2=A0 with intctl =3D core-irq controller
>=C2=A0=C2=A0 =C2=A0
>
> We analyzed how other Linux ports are handling this situation (IPI) an=
d=20
> here are several possible solutions:
>
> 1/ put everything in smp.c like what longarch is doing.
> =C2=A0 * Except that IPI in longarch seems to involve writing to a spe=
cial=20
> 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
> =C2=A0 * the probe would just ioremap the reg from DT and register the=
 irq=20
> using request_percpu_irq()
> =C2=A0 * it would export a function "kvx_ipi_send()" that would direct=
ly be=20
> called by smp.c
> =C2=A0 * Question : where would this driver be placed in drivers/ ?=20
> drivers/irqchip/ ? Even if this is not per-se an interrupt-controller=20
> 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