Received-SPF: pass (google.com: domain of linux-kernel+bounces-69287-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Thomas Gleixner <tglx@linutronix.de>
To: Anup Patel <anup@brainfault.org>
Cc: Anup Patel <apatel@ventanamicro.com>, Palmer Dabbelt
 <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Rob
 Herring <robh+dt@kernel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@linaro.org>, Frank Rowand
 <frowand.list@gmail.com>, Conor Dooley <conor+dt@kernel.org>, Marc Zyngier
 <maz@kernel.org>, =?utf-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>,
 Atish Patra
 <atishp@atishpatra.org>, Andrew Jones <ajones@ventanamicro.com>, Sunil V L
 <sunilvl@ventanamicro.com>, Saravana Kannan <saravanak@google.com>,
 linux-riscv@lists.infradead.org, linux-arm-kernel@lists.infradead.org,
 linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v12 14/25] irqchip/sifive-plic: Convert PLIC driver into
 a platform driver
In-Reply-To: <CAAhSdy2aeyJBcMVre12jGwU52oP9Z=1emB-bcYxygdR3QhP+6w@mail.gmail.com>
References: <20240127161753.114685-1-apatel@ventanamicro.com>
 <20240127161753.114685-15-apatel@ventanamicro.com> <87jzn4ctks.ffs@tglx>
 <CAAhSdy2aeyJBcMVre12jGwU52oP9Z=1emB-bcYxygdR3QhP+6w@mail.gmail.com>
Date: Fri, 16 Feb 2024 21:22:13 +0100
Message-ID: <878r3kcg7e.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 16 2024 at 22:41, Anup Patel wrote:
> On Fri, Feb 16, 2024 at 9:03=E2=80=AFPM Thomas Gleixner <tglx@linutronix.=
de> wrote:
>> I don't think that removing the setup protection is correct.
>>
>> Assume you have maxcpus=3DN on the kernel command line, then the above
>> for_each_online_cpu() loop would result in cpuhp_setup =3D=3D true when =
the
>> instances for the not onlined CPUs are set up, no?
>
> A platform can have multiple PLIC instances where each PLIC
> instance targets a subset of HARTs (or CPUs).
>
> Previously (before this patch), we were probing PLIC very early so on
> a platform with multiple PLIC instances, we need to ensure that cpuhp
> setup is done only after PLIC context associated with boot CPU is
> initialized hence the plic_cpuhp_setup_done check.
>
> This patch converts PLIC driver into a platform driver so now PLIC
> instances are probed after all available CPUs are brought-up. In this
> case, the cpuhp setup must be done only after PLIC context of all
> available CPUs are initialized otherwise some of the CPUs crash
> in plic_starting_cpu() due to lack of PLIC context initialization.

You're missing the point.

Assume you have 8 CPUs and 2 PLIC instances one for CPU0-3 and one for
CPU4-7.

Add "maxcpus=3D4" on the kernel command line, then only the first 4 CPUs
are brought up.

So at probe time cpu_online_mask has bit 0,1,2,3 set.

When the first PLIC it probed the loop which checks the context for each
online CPU will not clear cpuhp_setup and the hotplug state is installed.

Now the second PLIC is probed (the one for the offline CPUs 4-7) and the
loop will again not clear cpuhp_setup and it tries to install the state
again, no?

Thanks,

        tglx