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;
Message-ID: <468a2c8578d099eef0e0106fe273f73f5d70ef94.camel@svanheule.net>
Subject: Re: [PATCH] MIPS: smp-mt: enable all hardware interrupts on second
 VPE
From:   Sander Vanheule <sander@svanheule.net>
To:     Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc:     Marc Zyngier <maz@kernel.org>,
        Aleksander Jan Bajkowski <olek2@wp.pl>,
        Hauke Mehrtens <hauke@hauke-m.de>, git@birger-koblitz.de,
        linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Thu, 07 Jul 2022 17:12:42 +0200
In-Reply-To: <20220707143930.GA14693@alpha.franken.de>
References: <20220702190705.5319-1-olek2@wp.pl>
         <3c9a032edd0fb9b9608ad3ca08d6e3cc38f21464.camel@svanheule.net>
         <87fsjen2kl.wl-maz@kernel.org> <20220706081901.GA10797@alpha.franken.de>
         <CAFBinCAsj=RNvitj2tXJU6pTLSbanRXdKM9H4vyF=N9N=PP06g@mail.gmail.com>
         <20220707100630.GC9894@alpha.franken.de>
         <CAFBinCBn3+MbKFE84Y0KjW4qG_88+HuBTzRhPQSDqzqGhyhhZw@mail.gmail.com>
         <20220707143930.GA14693@alpha.franken.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.2 (3.44.2-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Thu, 2022-07-07 at 16:39 +0200, Thomas Bogendoerfer wrote:
> On Thu, Jul 07, 2022 at 02:57:15PM +0200, Martin Blumenstingl wrote:
> > On Thu, Jul 7, 2022 at 12:11 PM Thomas Bogendoerfer
> > <tsbogend@alpha.franken.de> wrote:
> > [...]
> > > > - why can MIPS CPU interrupt 6 and 7 be enabled unconditionally whi=
le
> > > > 2-5 cannot be enabled unconditionally?
> > >=20
> > > 7 is timer interrupt and is usually wired for 34K cpus and 6 is
> > > performance counter hopefully handled as well. And I agree that
> > > this still isn't the best approach here
> > Thanks for this explanation!
> >=20
> > > > - seeing that there's also a mips_gic_present() check in the opposi=
te
> > > > case of what Aleksander's patch modifies: does this indicate that
> > > > unmasking CPU interrupt lines for VPE 1 is not handled by the MIPS =
CPU
> > > > interrupt controller driver at all at this point (and if so: do you
> > > > have any suggestions how to properly fix this)?
> > >=20
> > > I haven't checked how GIC is integrated. Iirc it does something simil=
air
> > > to Lantiq's irq controller and hides all CPU internal interrupts behi=
nd
> > > it.
> > >=20
> > > So I see two solutions for your problem.
> > >=20
> > > 1. Add "mti,cpu-interrupt-controller" to the DT and wire it up
> > I think this is the preferred way. I tried this before (if you are
> > curious, see [0] and [1]) and it didn't work.
> > Are you aware of any MIPS SoC with upstream drivers which do have
> > working IRQs on VPE 1?
>=20
> I don't know of such SoC. Looking at the comment in vsmp_init_secondary()
>=20
> /* This is Malta specific: IPI,performance and timer interrupts */
>=20
> there is probably some Malta board using it.
>=20
> > Or can you point me to the code in
> > drivers/irqchip/irq-mips-cpu.c that's responsible for enabling the
> > interrupts on VPE 1 (is it simply unmask_mips_irq)?
>=20
> IMHO there is the problem, irq-mips-cpu.c can only do CPU irq operations
> on the same CPU. I've checked MIPS MT specs and it's possible do
> modify CP0 registers between VPEs. Using that needs changes in
> irq-mips-cpu.c. But mabye that's not woth the effort as probably
> all SMP cabable platforms have some multi processort capable
> interrupt controller implemented.

Maybe I'm not getting this right, but as I understand vsmp_init_secondary()=
 is
run on VPE1, changing the local c0_status on that VPE. When unmask_mips_irq=
() is
called from code running on VPE1, I would expect it does the same thing: en=
able
the requested irq on VPE1 by modifying the local c0_status register.

Since these IRQs can be configured VPE, aren't these then also per-cpu
interrupts? I have been wondering if a cascaded IRQ controller needs to tak=
e
special care with such per-cpu interrupts, but haven't been able to figure =
that
out until now.

Sorry if the above doesn't make much sense, but like Aleksander I've been
struggling with this and would like to understand what the proper solution =
is.

Best,
Sander

> I thought about another way solve the issue. By introducing a
> new function in smp-mt.c which sets the value of the interrupt
> mask for the secondary CPU, which is then used in vsmp_init_secondary().
> Not sure if this is worth the effort compared to a .boot_secondary
> override.
>=20
> Thomas.
>=20