linux-kernel - Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YR/HJQDGJ1C+ku6O@linux-8mug>
Date:   Fri, 20 Aug 2021 23:15:49 +0800
From:   Chester Lin <clin@...e.com>
To:     Marc Zyngier <maz@...nel.org>
CC:     Andreas Färber <afaerber@...e.de>,
        Rob Herring <robh+dt@...nel.org>, s32@....com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-serial@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Stefan Riedmueller <s.riedmueller@...tec.de>,
        Matthias Schiffer <matthias.schiffer@...tq-group.com>,
        Li Yang <leoyang.li@....com>,
        Fabio Estevam <festevam@...il.com>,
        Matteo Lisi <matteo.lisi@...icam.com>,
        Frieder Schrempf <frieder.schrempf@...tron.de>,
        Tim Harvey <tharvey@...eworks.com>,
        Jagan Teki <jagan@...rulasolutions.com>,
        catalin-dan.udma@....com, bogdan.hamciuc@....com,
        bogdan.folea@....com, ciprianmarian.costea@....com,
        radu-nicolae.pirea@....com, ghennadi.procopciuc@....com,
        Matthias Brugger <matthias.bgg@...il.com>,
        "Ivan T . Ivanov" <iivanov@...e.de>, "Lee, Chun-Yi" <jlee@...e.com>
Subject: Re: [PATCH 4/8] arm64: dts: add NXP S32G2 support

On Fri, Aug 20, 2021 at 02:12:13PM +0100, Marc Zyngier wrote:
> On Thu, 12 Aug 2021 18:26:28 +0100,
> Andreas Färber <afaerber@...e.de> wrote:
> > 
> > Hi Chester et al.,
> > 
> > On 05.08.21 08:54, Chester Lin wrote:
> > > Add an initial dtsi file for generic SoC features of NXP S32G2.
> > > 
> > > Signed-off-by: Chester Lin <clin@...e.com>
> > > ---
> > >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 98 ++++++++++++++++++++++++
> > >  1 file changed, 98 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > > new file mode 100644
> > > index 000000000000..3321819c1a2d
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> 
> [...]
> 
> > > +		gic: interrupt-controller@...00000 {
> > > +			compatible = "arm,gic-v3";
> > > +			#interrupt-cells = <3>;
> > > +			interrupt-controller;
> > > +			reg = <0 0x50800000 0 0x10000>,
> > > +			      <0 0x50880000 0 0x200000>,
> 
> That's enough redistributor space for 16 CPUs. However, you only
> describe 4. Either the number of CPUs is wrong, the size is wrong, or
> the GIC has been configured for more cores than the SoC has.

Confirmed the SoC can only find 4 redistributors:

localhost:~ # dmesg | grep CPU
[    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034]
[    0.000000] Detected VIPT I-cache on CPU0
[    0.000000] CPU features: detected: GIC system register CPU interface
[    0.000000] CPU features: detected: ARM erratum 845719
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=480 to nr_cpu_ids=4.
[    0.000000] GICv3: CPU0: found redistributor 0 region 0:0x0000000050880000
[    0.063865] smp: Bringing up secondary CPUs ...
[    0.068852] Detected VIPT I-cache on CPU1
[    0.068894] GICv3: CPU1: found redistributor 1 region 0:0x00000000508a0000
[    0.068963] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
[    0.069809] Detected VIPT I-cache on CPU2
[    0.069851] GICv3: CPU2: found redistributor 100 region 0:0x00000000508c0000
[    0.069903] CPU2: Booted secondary processor 0x0000000100 [0x410fd034]
[    0.070698] Detected VIPT I-cache on CPU3
[    0.070722] GICv3: CPU3: found redistributor 101 region 0:0x00000000508e0000
[    0.070749] CPU3: Booted secondary processor 0x0000000101 [0x410fd034]
[    0.070847] smp: Brought up 1 node, 4 CPUs
<..snip..>

I will correct the size to 0x80000, thanks!

> 
> > > +			      <0 0x50400000 0 0x2000>,
> > > +			      <0 0x50410000 0 0x2000>,
> > > +			      <0 0x50420000 0 0x2000>;
> > 
> > Please order reg after compatible by convention, and sort
> > interrupt-controller or at least #interrupt-cells (applying to
> > consumers) last, after the below one applying to this device itself.
> > 
> > > +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> > > +						 IRQ_TYPE_LEVEL_HIGH)>;
> > > +		};
> > 
> > CC'ing Marc for additional GIC scrutiny, often the sizes are wrong.
> 
> There is more than just sizes. The interrupt specifier for the
> maintenance interrupt is also wrong.
> 
> 	M.

I will remove the wrong interrupt specifier. Thanks!

Chester.

> 
> -- 
> Without deviation from the norm, progress is not possible.
>