linux-kernel - Re: [PATCH v1 2/2] riscv: dts: starfive: add the missing monitor core

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: <e8543838cd221ab6699da16c985eed7514daa786.camel@icenowy.me>
Date:   Wed, 13 Jul 2022 22:26:04 +0800
From:   Icenowy Zheng <uwu@...nowy.me>
To:     Conor Dooley <mail@...chuod.ie>,
        Emil Renner Berthing <kernel@...il.dk>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Anup Patel <anup@...infault.org>,
        Conor Dooley <conor.dooley@...rochip.com>
Cc:     devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] riscv: dts: starfive: add the missing monitor
 core

在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> From: Conor Dooley <conor.dooley@...rochip.com>
> 
> The JH7100 has a 32 bit monitor core that is missing from the device
> tree. Add it (and its cpu-map entry) to more accurately reflect the
> actual topology of the SoC.
> 
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> ---
>  arch/riscv/boot/dts/starfive/jh7100.dtsi | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index c617a61e26e2..92fce5b66d3d 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -67,6 +67,23 @@ cpu1_intc: interrupt-controller {
>                         };
>                 };
>  
> +               E24: cpu@2 {
> +                       compatible = "sifive,e24", "riscv";
> +                       reg = <2>;
> +                       device_type = "cpu";
> +                       i-cache-block-size = <32>;
> +                       i-cache-sets = <256>;
> +                       i-cache-size = <16384>;
> +                       riscv,isa = "rv32imafc";
> +                       status = "disabled";
> +
> +                       cpu2_intc: interrupt-controller {
> +                               compatible = "riscv,cpu-intc";
> +                               interrupt-controller;
> +                               #interrupt-cells = <1>;
> +                       };
> +               };
> +
>                 cpu-map {
>                         cluster0 {
>                                 core0 {
> @@ -76,6 +93,10 @@ core0 {
>                                 core1 {
>                                         cpu = <&U74_1>;
>                                 };
> +
> +                               core2 {
> +                                       cpu = <&E24>;
> +                               };

Sorry but I think this change makes the topology more inaccurate.

The E24 core is very independent, just another CPU core connected the
same bus -- even no coherency (E24 takes AHB, which is not coherency-
sensible). Even the TAP of it is independent with the U74 TAP.

And by default it does not boot any proper code (if a debugger is
attached, it will discover that the E24 is in consistently fault at 0x0
(mtvec is 0x0 and when fault it jumps to 0x0 and fault again), until
its clock is just shutdown by Linux cleaning up unused clocks.)

Personally I think it should be implemented as a remoteproc instead.

>                         };
>                 };
>         };