From: Conor Dooley <[email protected]>
Hey Emil,
Following on from my RFC [0], here's the real patches adding the
monitor core for the JH7100 w/ the ordering changed as requested.
I had a look in the SiFive E24 docs [1] & in [2] which said "16KB
I-cache with 32 Byte cache line". Didn't have anything else to go on,
so I kept the same ratio between lines/sets/size as other SiFive
monitor cores, but since they're not 32 bit I dunno if that's correct
(IOW it is a wild guess).
The dts patch depends on adding the series adding the cpu-map [3]:
Thanks,
Conor.
0: https://lore.kernel.org/linux-riscv/[email protected]/
1: https://sifive.cdn.prismic.io/sifive/dc408861-94ce-4d82-a704-caddec98609d_e24_core_complex_manual_21G3.pdf
2: https://github.com/starfive-tech/JH7100_Docs/blob/main/JH7100%20Data%20Sheet%20V01.01.04-EN%20(4-21-2021).pdf
3: https://lore.kernel.org/linux-riscv/[email protected]/
Conor Dooley (2):
dt-bindings: riscv: document the sifive e24
riscv: dts: starfive: add the missing monitor core
.../devicetree/bindings/riscv/cpus.yaml | 2 ++
arch/riscv/boot/dts/starfive/jh7100.dtsi | 21 +++++++++++++++++++
2 files changed, 23 insertions(+)
base-commit: b6f1f2fa2bddd69ff46a190b8120bd440fd50563
prerequisite-patch-id: 946df827e9dad8ca6e9751fe4ba01fd5fe9d18cc
prerequisite-patch-id: 59626290ee7b1e725bd446a4b7170ee2d6ca9bc0
prerequisite-patch-id: e57a94cb7d69855a4e2d6044dcf86fbfe35cb696
prerequisite-patch-id: 0ba94fd09377953ec4e9692358de569a7932bfa3
prerequisite-patch-id: 398e9b178ce51c924bbd9115020d84b677f773bb
--
2.37.0
From: Conor Dooley <[email protected]>
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 <[email protected]>
---
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>;
+ };
};
};
};
--
2.37.0
From: Conor Dooley <[email protected]>
The SiFive E24 is a 32 bit monitor core present on the JH7100.
Signed-off-by: Conor Dooley <[email protected]>
---
Documentation/devicetree/bindings/riscv/cpus.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index d632ac76532e..195e762094a8 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -29,6 +29,7 @@ properties:
- enum:
- sifive,rocket0
- sifive,bullet0
+ - sifive,e24
- sifive,e5
- sifive,e7
- sifive,e71
@@ -75,6 +76,7 @@ properties:
lowercase to simplify parsing.
$ref: "/schemas/types.yaml#/definitions/string"
enum:
+ - rv32imafc
- rv64imac
- rv64imafdc
--
2.37.0
On 11/07/2022 20:43, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> The SiFive E24 is a 32 bit monitor core present on the JH7100.
>
> Signed-off-by: Conor Dooley <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Mon, 11 Jul 2022 at 20:44, Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> 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 <[email protected]>
Reviewed-by: Emil Renner Berthing <[email protected]>
Thanks!
/Emil
> ---
> 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>;
> + };
> };
> };
> };
> --
> 2.37.0
>
在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> From: Conor Dooley <[email protected]>
>
> 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 <[email protected]>
> ---
> 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.
> };
> };
> };
On 13/07/2022 15:26, Icenowy Zheng wrote:
>
> 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
>> From: Conor Dooley <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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.
Maybe I am missing something, but I don't quite get what the detail
of how we access this in code has to do with the devicetree?
It is added here in a disabled state, and will not be used by Linux.
The various SiFive SoCs & SiFive corecomplex users that have a hart
not capable of running Linux also have that hart documented in the
devicetree.
To me, what we are choosing to do with this hart does not really
matter very much, since this is a description of what the hardware
actually looks like.
Thanks,
Conor.
在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> From: Conor Dooley <[email protected]>
>
> 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 <[email protected]>
> ---
> 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";
Oh, by the way "sifive,e24" is not a documented compatible in the DT
binding.
If you really want to add it here, you need to add the compatible
string to the DT binding first.
> + 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>;
> + };
> };
> };
> };
On 13/07/2022 16:15, Icenowy Zheng wrote:
> 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
>> From: Conor Dooley <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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";
>
> Oh, by the way "sifive,e24" is not a documented compatible in the DT
> binding.
>
> If you really want to add it here, you need to add the compatible
> string to the DT binding first.
Check patch 1/2.
>
>> + 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>;
>> + };
>> };
>> };
>> };
>
>
On 13/07/2022 16:12, Icenowy Zheng wrote:
> 在 2022-07-13星期三的 15:09 +0000，[email protected]写道：
>>
>>
>> On 13/07/2022 16:02, Icenowy Zheng wrote:
>>> 在 2022-07-13星期三的 14:55 +0000，[email protected]写道：
>>>> On 13/07/2022 15:26, Icenowy Zheng wrote:
>>>>>
>>>>> 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
>>>>>> From: Conor Dooley <[email protected]>
>>>>>>
>>>>>> 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 <[email protected]>
>>>>>> ---
>>>>>> 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.
>>>>
>>>> Maybe I am missing something, but I don't quite get what the
>>>> detail
>>>> of how we access this in code has to do with the devicetree?
>>>> It is added here in a disabled state, and will not be used by
>>>> Linux.
>>>> The various SiFive SoCs & SiFive corecomplex users that have a
>>>> hart
>>>> not capable of running Linux also have that hart documented in
>>>> the
>>>> devicetree.
>>>> To me, what we are choosing to do with this hart does not really
>>>> matter very much, since this is a description of what the
>>>> hardware
>>>> actually looks like.
>>>
>>> The E24 is not in the core complex at all. It's just a dedicate CPU
>>> connected to another bus (well as I saw the document says the E24
>>> bus
>>> is maximum 2G, I doubt whether it's the same bus with the U74 one).
>>>
>>> The U74 MC only allows S5 management cores to be part of it, not
>>> E24.
>>
>> So is the correct topology more like:
>> cpu-map {
>> cluster0 {
>> core0 {
>> cpu = <&U74_0>;
>> };
>> core1 {
>> cpu = <&U74_1>;
>> };
>> };
>> cluster1 {
>> core0 {
>> cpu = <&E24>;
>> };
>> };
>> };
>
> Considering E24 seems to see a total different bus connected to it, I
> don't think it even proper to add it to cpus node.
Well, it is a CPU is it not? How one is supposed to document that a
CPU is not attached to the same buses I do not know however.
>
> And I don't think it has a hart id of 2, as your node describes.
Do you have any idea what it would be then?
Rob/Krzk,
Got a question about how to represent one of the cpu cores on
this SoC at the end of the mail
On 13/07/2022 16:26, Icenowy Zheng wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 在 2022-07-13星期三的 15:21 +0000，[email protected]写道：
>> On 13/07/2022 16:12, Icenowy Zheng wrote:
>>> 在 2022-07-13星期三的 15:09 +0000，[email protected]写道：
>>>>
>>>>
>>>> On 13/07/2022 16:02, Icenowy Zheng wrote:
>>>>> 在 2022-07-13星期三的 14:55 +0000，[email protected]写道：
>>>>>> On 13/07/2022 15:26, Icenowy Zheng wrote:
>>>>>>> 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.
>>>>>>
>>>>>> Maybe I am missing something, but I don't quite get what the
>>>>>> detail
>>>>>> of how we access this in code has to do with the devicetree?
>>>>>> It is added here in a disabled state, and will not be used by
>>>>>> Linux.
>>>>>> The various SiFive SoCs & SiFive corecomplex users that have
>>>>>> a
>>>>>> hart
>>>>>> not capable of running Linux also have that hart documented
>>>>>> in
>>>>>> the
>>>>>> devicetree.
>>>>>> To me, what we are choosing to do with this hart does not
>>>>>> really
>>>>>> matter very much, since this is a description of what the
>>>>>> hardware
>>>>>> actually looks like.
>>>>>
>>>>> The E24 is not in the core complex at all. It's just a dedicate
>>>>> CPU
>>>>> connected to another bus (well as I saw the document says the
>>>>> E24
>>>>> bus
>>>>> is maximum 2G, I doubt whether it's the same bus with the U74
>>>>> one).
>>>>>
>>>>> The U74 MC only allows S5 management cores to be part of it,
>>>>> not
>>>>> E24.
>>>>
---8<---
>>>
>>> Considering E24 seems to see a total different bus connected to it,
>>> I
>>> don't think it even proper to add it to cpus node.
>>
>> Well, it is a CPU is it not? How one is supposed to document that a
>> CPU is not attached to the same buses I do not know however.
>
> I don't think this kind of CPUs should exist in /cpus, they should just
> be seen as peripherals as the main system. The speciality of FU[57]40's
> management core is that they're in the same core complex with the CPU
> cores that run Linux, just cores with a different capability that we
> could not expand Linux to them.
Maybe Rob or Krzysztof can shed some light on what would be the correct
way to depict this cpu.
>
>>
>>>
>>> And I don't think it has a hart id of 2, as your node describes.
>>
>> Do you have any idea what it would be then?
>
> As I asked one of my friend who has JTAG access to JH7110, the hart id
> is 0, the same with the first hart in U74-MC.
Hmm, my understanding is that the regs property needs to be unique, so
it'd have to stay as 2.
Thanks,
Conor.
On 13/07/2022 16:02, Icenowy Zheng wrote:
> 在 2022-07-13星期三的 14:55 +0000，[email protected]写道：
>> On 13/07/2022 15:26, Icenowy Zheng wrote:
>>>
>>> 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
>>>> From: Conor Dooley <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> ---
>>>> 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.
>>
>> Maybe I am missing something, but I don't quite get what the detail
>> of how we access this in code has to do with the devicetree?
>> It is added here in a disabled state, and will not be used by Linux.
>> The various SiFive SoCs & SiFive corecomplex users that have a hart
>> not capable of running Linux also have that hart documented in the
>> devicetree.
>> To me, what we are choosing to do with this hart does not really
>> matter very much, since this is a description of what the hardware
>> actually looks like.
>
> The E24 is not in the core complex at all. It's just a dedicate CPU
> connected to another bus (well as I saw the document says the E24 bus
> is maximum 2G, I doubt whether it's the same bus with the U74 one).
>
> The U74 MC only allows S5 management cores to be part of it, not E24.
So is the correct topology more like:
cpu-map {
cluster0 {
core0 {
cpu = <&U74_0>;
};
core1 {
cpu = <&U74_1>;
};
};
cluster1 {
core0 {
cpu = <&E24>;
};
};
};
在 2022-07-13星期三的 15:21 +0000，[email protected]写道：
> On 13/07/2022 16:12, Icenowy Zheng wrote:
> > 在 2022-07-13星期三的 15:09 +0000，[email protected]写道：
> > >
> > >
> > > On 13/07/2022 16:02, Icenowy Zheng wrote:
> > > > 在 2022-07-13星期三的 14:55 +0000，[email protected]写道：
> > > > > On 13/07/2022 15:26, Icenowy Zheng wrote:
> > > > > >
> > > > > > 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> > > > > > > From: Conor Dooley <[email protected]>
> > > > > > >
> > > > > > > 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 <[email protected]>
> > > > > > > ---
> > > > > > > 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.
> > > > >
> > > > > Maybe I am missing something, but I don't quite get what the
> > > > > detail
> > > > > of how we access this in code has to do with the devicetree?
> > > > > It is added here in a disabled state, and will not be used by
> > > > > Linux.
> > > > > The various SiFive SoCs & SiFive corecomplex users that have
> > > > > a
> > > > > hart
> > > > > not capable of running Linux also have that hart documented
> > > > > in
> > > > > the
> > > > > devicetree.
> > > > > To me, what we are choosing to do with this hart does not
> > > > > really
> > > > > matter very much, since this is a description of what the
> > > > > hardware
> > > > > actually looks like.
> > > >
> > > > The E24 is not in the core complex at all. It's just a dedicate
> > > > CPU
> > > > connected to another bus (well as I saw the document says the
> > > > E24
> > > > bus
> > > > is maximum 2G, I doubt whether it's the same bus with the U74
> > > > one).
> > > >
> > > > The U74 MC only allows S5 management cores to be part of it,
> > > > not
> > > > E24.
> > >
> > > So is the correct topology more like:
> > > cpu-map {
> > > cluster0 {
> > > core0 {
> > > cpu = <&U74_0>;
> > > };
> > > core1 {
> > > cpu = <&U74_1>;
> > > };
> > > };
> > > cluster1 {
> > > core0 {
> > > cpu = <&E24>;
> > > };
> > > };
> > > };
> >
> > Considering E24 seems to see a total different bus connected to it,
> > I
> > don't think it even proper to add it to cpus node.
>
> Well, it is a CPU is it not? How one is supposed to document that a
> CPU is not attached to the same buses I do not know however.
I don't think this kind of CPUs should exist in /cpus, they should just
be seen as peripherals as the main system. The speciality of FU[57]40's
management core is that they're in the same core complex with the CPU
cores that run Linux, just cores with a different capability that we
could not expand Linux to them.
>
> >
> > And I don't think it has a hart id of 2, as your node describes.
>
> Do you have any idea what it would be then?
As I asked one of my friend who has JTAG access to JH7110, the hart id
is 0, the same with the first hart in U74-MC.
在 2022-07-13星期三的 14:55 +0000，[email protected]写道：
> On 13/07/2022 15:26, Icenowy Zheng wrote:
> >
> > 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> > > From: Conor Dooley <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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.
>
> Maybe I am missing something, but I don't quite get what the detail
> of how we access this in code has to do with the devicetree?
> It is added here in a disabled state, and will not be used by Linux.
> The various SiFive SoCs & SiFive corecomplex users that have a hart
> not capable of running Linux also have that hart documented in the
> devicetree.
> To me, what we are choosing to do with this hart does not really
> matter very much, since this is a description of what the hardware
> actually looks like.
The E24 is not in the core complex at all. It's just a dedicate CPU
connected to another bus (well as I saw the document says the E24 bus
is maximum 2G, I doubt whether it's the same bus with the U74 one).
The U74 MC only allows S5 management cores to be part of it, not E24.
BTW I don't think a core complex can have multiple TAPs for multiple
harts, but E24 has its own TAP.
>
> Thanks,
> Conor.
>
在 2022-07-13星期三的 15:09 +0000，[email protected]写道：
>
>
> On 13/07/2022 16:02, Icenowy Zheng wrote:
> > 在 2022-07-13星期三的 14:55 +0000，[email protected]写道：
> > > On 13/07/2022 15:26, Icenowy Zheng wrote:
> > > >
> > > > 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> > > > > From: Conor Dooley <[email protected]>
> > > > >
> > > > > 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 <[email protected]>
> > > > > ---
> > > > > 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.
> > >
> > > Maybe I am missing something, but I don't quite get what the
> > > detail
> > > of how we access this in code has to do with the devicetree?
> > > It is added here in a disabled state, and will not be used by
> > > Linux.
> > > The various SiFive SoCs & SiFive corecomplex users that have a
> > > hart
> > > not capable of running Linux also have that hart documented in
> > > the
> > > devicetree.
> > > To me, what we are choosing to do with this hart does not really
> > > matter very much, since this is a description of what the
> > > hardware
> > > actually looks like.
> >
> > The E24 is not in the core complex at all. It's just a dedicate CPU
> > connected to another bus (well as I saw the document says the E24
> > bus
> > is maximum 2G, I doubt whether it's the same bus with the U74 one).
> >
> > The U74 MC only allows S5 management cores to be part of it, not
> > E24.
>
> So is the correct topology more like:
> cpu-map {
> cluster0 {
> core0 {
> cpu = <&U74_0>;
> };
> core1 {
> cpu = <&U74_1>;
> };
> };
> cluster1 {
> core0 {
> cpu = <&E24>;
> };
> };
> };
Considering E24 seems to see a total different bus connected to it, I
don't think it even proper to add it to cpus node.
And I don't think it has a hart id of 2, as your node describes.
>
在 2022-07-13星期三的 15:16 +0000，[email protected]写道：
> On 13/07/2022 16:15, Icenowy Zheng wrote:
> > 在 2022-07-11星期一的 19:43 +0100，Conor Dooley写道：
> > > From: Conor Dooley <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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";
> >
> > Oh, by the way "sifive,e24" is not a documented compatible in the
> > DT
> > binding.
> >
> > If you really want to add it here, you need to add the compatible
> > string to the DT binding first.
>
> Check patch 1/2.
Oh sorry I forgot this.
Nevermind.
>
> >
> > > + 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>;
> > > + };
> > > };
> > > };
> > > };
> >
> >