2024-03-04 21:05:05

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 0/7] riscv: add initial support for Canaan Kendryte K230

K230 is an ideal chip for RISC-V Vector 1.0 evaluation now. Add initial
support for it to allow more people to participate in building drivers
to mainline for it.

This kernel has been tested upon factory SDK [1] with
k230_evb_only_linux_defconfig and patched mainline opensbi [2] to skip
locked pmp and successfully booted to busybox on initrd with this log [3].

[1] https://github.com/kendryte/k230_sdk
[2] https://github.com/cyyself/opensbi/tree/k230
[3] https://gist.github.com/cyyself/b9445f38cc3ba1094924bd41c9086176

Changes since v2:
- Add MIT License to dts file
- Sort dt-binding stings in alphanumerical order
- Sort filename in dts Makefile in alphanumerical order
- Rename canmv-k230.dts to k230-canmv.dts

v2: https://lore.kernel.org/linux-riscv/[email protected]/

Changes since v1:
- Patch dt-bindings in clint and plic
- Use enum in K230 compatible dt bindings
- Fix dts to pass `make dtbs_check`
- Add more details in commit message

v1: https://lore.kernel.org/linux-riscv/[email protected]/

Yangyu Chen (7):
dt-bindings: riscv: Add T-HEAD C908 compatible
dt-bindings: add Canaan K230 boards compatible strings
dt-bindings: timer: Add Canaan K230 CLINT
dt-bindings: interrupt-controller: Add Canaan K230 PLIC
riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230
riscv: dts: add initial canmv-k230 and k230-evb dts
riscv: config: enable SOC_CANAAN in defconfig

.../sifive,plic-1.0.0.yaml | 1 +
.../devicetree/bindings/riscv/canaan.yaml | 8 +-
.../devicetree/bindings/riscv/cpus.yaml | 1 +
.../bindings/timer/sifive,clint.yaml | 1 +
arch/riscv/Kconfig.socs | 5 +-
arch/riscv/boot/dts/canaan/Makefile | 2 +
arch/riscv/boot/dts/canaan/k230-canmv.dts | 24 +++
arch/riscv/boot/dts/canaan/k230-evb.dts | 24 +++
arch/riscv/boot/dts/canaan/k230.dtsi | 140 ++++++++++++++++++
arch/riscv/configs/defconfig | 1 +
10 files changed, 203 insertions(+), 4 deletions(-)
create mode 100644 arch/riscv/boot/dts/canaan/k230-canmv.dts
create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi

base-commit: 45e0b0fd6dc574101825ac2738b890da024e4cda
prerequisite-patch-id: 2374c56c0032e616e45854d2bc2bb1073996313d

Dependencies: https://lore.kernel.org/linux-riscv/[email protected]/
--
2.43.0



2024-03-04 22:14:05

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 4/7] dt-bindings: interrupt-controller: Add Canaan K230 PLIC

Add compatible string for Canaan K230 PLIC.

Signed-off-by: Yangyu Chen <[email protected]>
---
.../bindings/interrupt-controller/sifive,plic-1.0.0.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
index 709b2211276b..122f9b7b3f52 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
@@ -65,6 +65,7 @@ properties:
- items:
- enum:
- allwinner,sun20i-d1-plic
+ - canaan,k230-plic
- sophgo,cv1800b-plic
- sophgo,cv1812h-plic
- sophgo,sg2042-plic
--
2.43.0


2024-03-04 21:07:53

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 7/7] riscv: config: enable SOC_CANAAN in defconfig

Since K230 has been supported, allow SOC_CANAAN to be selected to build dt
and drivers for it in defconfig.

Signed-off-by: Yangyu Chen <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
arch/riscv/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 89a009a580fe..20b557ec28df 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -33,6 +33,7 @@ CONFIG_SOC_STARFIVE=y
CONFIG_ARCH_SUNXI=y
CONFIG_ARCH_THEAD=y
CONFIG_SOC_VIRT=y
+CONFIG_SOC_CANAAN=y
CONFIG_SMP=y
CONFIG_HOTPLUG_CPU=y
CONFIG_PM=y
--
2.43.0


2024-03-04 22:07:49

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 2/7] dt-bindings: add Canaan K230 boards compatible strings

Since K230 was released, K210 is no longer the only SoC in the Kendryte
series, so remove the K210 string from the description. Also, add two
boards based on k230 to compatible strings to allow them to be used in the
dt.

Signed-off-by: Yangyu Chen <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
Documentation/devicetree/bindings/riscv/canaan.yaml | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/riscv/canaan.yaml b/Documentation/devicetree/bindings/riscv/canaan.yaml
index 41fd11f70a49..12a78bc95992 100644
--- a/Documentation/devicetree/bindings/riscv/canaan.yaml
+++ b/Documentation/devicetree/bindings/riscv/canaan.yaml
@@ -10,7 +10,7 @@ maintainers:
- Damien Le Moal <[email protected]>

description:
- Canaan Kendryte K210 SoC-based boards
+ Canaan Kendryte SoC-based boards

properties:
$nodename:
@@ -42,6 +42,12 @@ properties:
- items:
- const: canaan,kendryte-k210

+ - items:
+ - enum:
+ - canaan,k230-usip-lp3-evb
+ - canaan,canmv-k230
+ - const: canaan,kendryte-k230
+
additionalProperties: true

...
--
2.43.0


2024-03-04 22:07:03

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 3/7] dt-bindings: timer: Add Canaan K230 CLINT

Add compatible string for Canaan K230 CLINT.

Signed-off-by: Yangyu Chen <[email protected]>
---
Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
index fced6f2d8ecb..06c67f20ad3c 100644
--- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml
+++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
@@ -38,6 +38,7 @@ properties:
- items:
- enum:
- allwinner,sun20i-d1-clint
+ - canaan,k230-clint
- sophgo,cv1800b-clint
- sophgo,cv1812h-clint
- thead,th1520-clint
--
2.43.0



2024-03-04 21:08:24

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230

Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
Remove it depends on !MMU will allow building dts for K230 and remove the
K210 string from the help message.

Signed-off-by: Yangyu Chen <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
arch/riscv/Kconfig.socs | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 623de5f8a208..b4e9b7f75510 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -75,13 +75,12 @@ config ARCH_CANAAN
def_bool SOC_CANAAN

config SOC_CANAAN
- bool "Canaan Kendryte K210 SoC"
- depends on !MMU
+ bool "Canaan Kendryte SoC"
select CLINT_TIMER if RISCV_M_MODE
select ARCH_HAS_RESET_CONTROLLER
select PINCTRL
select COMMON_CLK
help
- This enables support for Canaan Kendryte K210 SoC platform hardware.
+ This enables support for Canaan Kendryte SoC platform hardware.

endmenu # "SoC selection"
--
2.43.0


2024-03-04 21:08:04

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts

Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
K230 SoC [1].

Some key consideration:
- Only place BigCore which is 1.6GHz RV64GCBV

The existence of cache coherence between the two cores remains unknown
since they have dedicated L2 caches. And the factory SDK uses it for
other OS by default. I don't know whether the two CPUs on K230 SoC
can be used in one system. So only place BigCore here.

Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
CPU1, the csr.mhartid of this core is 0.

- Support for "zba" "zbb" "zbc" "zbs" are tested by hand

The user manual of C908 from T-Head does not document it specifically.
It just said it supports B extension V1.0-rc1. [2]

I have tested it by using this [3] which attempts to execute "add.uw",
"andn", "clmulr", "bclr" and they doesn't traps on K230. But on JH7110,
"clmulr" and "bclr" will trap.

- Support for "zicbom" is tested by hand

Have tested with some out-of-tree drivers from [4] that need DMA and they
do not come to the dts currently.

- Cache parameters are inferred from T-Head docs [2] and Canaan docs [1]

L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
L2: 256KB, PIPT 16-way set-associative, 64B Cacheline

The numbers of cache sets are calculated from these parameters.

- MMU only supports Sv39

Since T-Head docs [2] say C908 should support Sv48. However, it will fail
during the kernel probe when running Linux on K230. I also tested it by
hand on M-Mode software, writing Sv48 to satp.mode will not trap but will
leave the csr unchanged. While writing Sv39 it will take effect. It shows
that this CPU does not support Sv48.

- Svpbmt and T-Head MAEE both supported

T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
attributes and is controlled by csr.mxstatus. If the kernel wants to use
svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
lines modification that haven't been written back to the memory.

[1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
[2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf
[3] https://github.com/cyyself/rvb_test
[4] https://github.com/cyyself/linux/tree/k230-mainline

Signed-off-by: Yangyu Chen <[email protected]>
---
arch/riscv/boot/dts/canaan/Makefile | 2 +
arch/riscv/boot/dts/canaan/k230-canmv.dts | 24 ++++
arch/riscv/boot/dts/canaan/k230-evb.dts | 24 ++++
arch/riscv/boot/dts/canaan/k230.dtsi | 140 ++++++++++++++++++++++
4 files changed, 190 insertions(+)
create mode 100644 arch/riscv/boot/dts/canaan/k230-canmv.dts
create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi

diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
index 987d1f0c41f0..7d54ea5c6f3d 100644
--- a/arch/riscv/boot/dts/canaan/Makefile
+++ b/arch/riscv/boot/dts/canaan/Makefile
@@ -1,6 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
dtb-$(CONFIG_ARCH_CANAAN) += canaan_kd233.dtb
dtb-$(CONFIG_ARCH_CANAAN) += k210_generic.dtb
+dtb-$(CONFIG_ARCH_CANAAN) += k230-canmv.dtb
+dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
diff --git a/arch/riscv/boot/dts/canaan/k230-canmv.dts b/arch/riscv/boot/dts/canaan/k230-canmv.dts
new file mode 100644
index 000000000000..3ab5c8de11a8
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/k230-canmv.dts
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2024 Yangyu Chen <[email protected]>
+ */
+
+#include "k230.dtsi"
+
+/ {
+ model = "Canaan CanMV-K230";
+ compatible = "canaan,canmv-k230", "canaan,kendryte-k230";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ ddr: memory@0 {
+ device_type = "memory";
+ reg = <0x0 0x0 0x0 0x1fdff000>;
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
diff --git a/arch/riscv/boot/dts/canaan/k230-evb.dts b/arch/riscv/boot/dts/canaan/k230-evb.dts
new file mode 100644
index 000000000000..42720113c566
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/k230-evb.dts
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2024 Yangyu Chen <[email protected]>
+ */
+
+#include "k230.dtsi"
+
+/ {
+ model = "Kendryte K230 EVB";
+ compatible = "canaan,k230-usip-lp3-evb", "canaan,kendryte-k230";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ ddr: memory@0 {
+ device_type = "memory";
+ reg = <0x0 0x0 0x0 0x1fdff000>;
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
new file mode 100644
index 000000000000..0bcff67b78a8
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/k230.dtsi
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2024 Yangyu Chen <[email protected]>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/dts-v1/;
+/ {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ compatible = "canaan,kendryte-k230";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ timebase-frequency = <27000000>;
+
+ cpu@0 {
+ compatible = "thead,c908", "riscv";
+ device_type = "cpu";
+ reg = <0>;
+ riscv,isa = "rv64imafdcv_zba_zbb_zbc_zbs_zicbom_svpbmt";
+ riscv,isa-base = "rv64i";
+ riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "v", "zba", "zbb",
+ "zbc", "zbs", "zicbom", "zicntr", "zicsr",
+ "zifencei", "zihpm", "svpbmt";
+ riscv,cbom-block-size = <64>;
+ d-cache-block-size = <64>;
+ d-cache-sets = <128>;
+ d-cache-size = <32768>;
+ i-cache-block-size = <64>;
+ i-cache-sets = <128>;
+ i-cache-size = <32768>;
+ next-level-cache = <&l2_cache>;
+ mmu-type = "riscv,sv39";
+
+ cpu0_intc: interrupt-controller {
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+ };
+
+ l2_cache: l2-cache {
+ compatible = "cache";
+ cache-block-size = <64>;
+ cache-level = <2>;
+ cache-size = <262144>;
+ cache-sets = <256>;
+ cache-unified;
+ };
+ };
+
+ apb_clk: apb-clk-clock {
+ compatible = "fixed-clock";
+ clock-frequency = <50000000>;
+ clock-output-names = "apb_clk";
+ #clock-cells = <0>;
+ };
+
+ soc {
+ compatible = "simple-bus";
+ interrupt-parent = <&plic>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ dma-noncoherent;
+ ranges;
+
+ plic: interrupt-controller@f00000000 {
+ compatible = "canaan,k230-plic" ,"thead,c900-plic";
+ reg = <0xf 0x00000000 0x0 0x04000000>;
+ interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
+ interrupt-controller;
+ #address-cells = <0>;
+ #interrupt-cells = <2>;
+ riscv,ndev = <208>;
+ };
+
+ clint: timer@f04000000 {
+ compatible = "canaan,k230-clint", "thead,c900-clint";
+ reg = <0xf 0x04000000 0x0 0x04000000>;
+ interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
+ };
+
+ uart0: serial@91400000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x0 0x91400000 0x0 0x1000>;
+ clocks = <&apb_clk>;
+ interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ status = "disabled";
+ };
+
+ uart1: serial@91401000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x0 0x91401000 0x0 0x1000>;
+ clocks = <&apb_clk>;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ status = "disabled";
+ };
+
+ uart2: serial@91402000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x0 0x91402000 0x0 0x1000>;
+ clocks = <&apb_clk>;
+ interrupts = <18 IRQ_TYPE_LEVEL_HIGH>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ status = "disabled";
+ };
+
+ uart3: serial@91403000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x0 0x91403000 0x0 0x1000>;
+ clocks = <&apb_clk>;
+ interrupts = <19 IRQ_TYPE_LEVEL_HIGH>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ status = "disabled";
+ };
+
+ uart4: serial@91404000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x0 0x91404000 0x0 0x1000>;
+ clocks = <&apb_clk>;
+ interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ status = "disabled";
+ };
+ };
+};
--
2.43.0


2024-03-04 21:06:58

by Yangyu Chen

[permalink] [raw]
Subject: [PATCH v3 1/7] dt-bindings: riscv: Add T-HEAD C908 compatible

The thead,c908 is a RISC-V CPU core from T-HEAD Semiconductor which used
in Canaan Kendryte K230 SoC.

Signed-off-by: Yangyu Chen <[email protected]>
Acked-by: Conor Dooley <[email protected]>
---
Documentation/devicetree/bindings/riscv/cpus.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index 9d8670c00e3b..e853a7fcee8a 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -47,6 +47,7 @@ properties:
- sifive,u74
- sifive,u74-mc
- thead,c906
+ - thead,c908
- thead,c910
- thead,c920
- const: riscv
--
2.43.0


2024-03-05 14:48:00

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 3/7] dt-bindings: timer: Add Canaan K230 CLINT


On Tue, 05 Mar 2024 05:05:57 +0800, Yangyu Chen wrote:
> Add compatible string for Canaan K230 CLINT.
>
> Signed-off-by: Yangyu Chen <[email protected]>
> ---
> Documentation/devicetree/bindings/timer/sifive,clint.yaml | 1 +
> 1 file changed, 1 insertion(+)
>

Acked-by: Rob Herring <[email protected]>


2024-03-05 14:48:33

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 4/7] dt-bindings: interrupt-controller: Add Canaan K230 PLIC


On Tue, 05 Mar 2024 05:05:58 +0800, Yangyu Chen wrote:
> Add compatible string for Canaan K230 PLIC.
>
> Signed-off-by: Yangyu Chen <[email protected]>
> ---
> .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml | 1 +
> 1 file changed, 1 insertion(+)
>

Acked-by: Rob Herring <[email protected]>


2024-03-05 15:54:20

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts

On Mon, 04 Mar 2024 13:06:00 PST (-0800), [email protected] wrote:
> Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
> K230 SoC [1].
>
> Some key consideration:
> - Only place BigCore which is 1.6GHz RV64GCBV
>
> The existence of cache coherence between the two cores remains unknown
> since they have dedicated L2 caches. And the factory SDK uses it for
> other OS by default. I don't know whether the two CPUs on K230 SoC
> can be used in one system. So only place BigCore here.
>
> Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
> CPU1, the csr.mhartid of this core is 0.
>
> - Support for "zba" "zbb" "zbc" "zbs" are tested by hand
>
> The user manual of C908 from T-Head does not document it specifically.
> It just said it supports B extension V1.0-rc1. [2]

I'm seeing

3.1.6 Vector instruction set (RVV)
For more information, see RISC-V “V”Vector Extension, Version
1.0-rc1-20210608
URL:https://github.com/riscv/riscv-v-spec/releases/download/v1.0-rc1/riscv-v-spec-1.0-rc1.pdf

3.1.7 Bit operation instruction set (RV64B)
For more information, see :RISC-V Bit-Manipulation ISA-extensions,
Version 1.0.0, 2021-06-12: public review
URL:https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0.pdf

so I think maybe that's vector 1.0-rc1 and the final bitmanip?

> I have tested it by using this [3] which attempts to execute "add.uw",
> "andn", "clmulr", "bclr" and they doesn't traps on K230. But on JH7110,
> "clmulr" and "bclr" will trap.
>
> - Support for "zicbom" is tested by hand
>
> Have tested with some out-of-tree drivers from [4] that need DMA and they
> do not come to the dts currently.
>
> - Cache parameters are inferred from T-Head docs [2] and Canaan docs [1]
>
> L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L2: 256KB, PIPT 16-way set-associative, 64B Cacheline
>
> The numbers of cache sets are calculated from these parameters.
>
> - MMU only supports Sv39
>
> Since T-Head docs [2] say C908 should support Sv48. However, it will fail
> during the kernel probe when running Linux on K230. I also tested it by
> hand on M-Mode software, writing Sv48 to satp.mode will not trap but will
> leave the csr unchanged. While writing Sv39 it will take effect. It shows
> that this CPU does not support Sv48.

I think that only works due to a bug in our sv48 probing routines --
unless I'm missing something, we mixed up the sv57 and sv48 probing and
if sv57 fails to probe then we disable sv48 as well. So I think we
probably need some cleanup in there.

> - Svpbmt and T-Head MAEE both supported
>
> T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
> attributes and is controlled by csr.mxstatus. If the kernel wants to use
> svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
> before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
> on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
> lines modification that haven't been written back to the memory.

So I guess we need the bootloader to just be accurate here? ie:
whatever extension it tells S-mode kernels is enabled is how S-mode
behaves, and then this should just work itself out.

> [1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
> [2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf
> [3] https://github.com/cyyself/rvb_test
> [4] https://github.com/cyyself/linux/tree/k230-mainline
>
> Signed-off-by: Yangyu Chen <[email protected]>
> ---
> arch/riscv/boot/dts/canaan/Makefile | 2 +
> arch/riscv/boot/dts/canaan/k230-canmv.dts | 24 ++++
> arch/riscv/boot/dts/canaan/k230-evb.dts | 24 ++++
> arch/riscv/boot/dts/canaan/k230.dtsi | 140 ++++++++++++++++++++++
> 4 files changed, 190 insertions(+)
> create mode 100644 arch/riscv/boot/dts/canaan/k230-canmv.dts
> create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
> create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi
>
> diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
> index 987d1f0c41f0..7d54ea5c6f3d 100644
> --- a/arch/riscv/boot/dts/canaan/Makefile
> +++ b/arch/riscv/boot/dts/canaan/Makefile
> @@ -1,6 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
> dtb-$(CONFIG_ARCH_CANAAN) += canaan_kd233.dtb
> dtb-$(CONFIG_ARCH_CANAAN) += k210_generic.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += k230-canmv.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
> diff --git a/arch/riscv/boot/dts/canaan/k230-canmv.dts b/arch/riscv/boot/dts/canaan/k230-canmv.dts
> new file mode 100644
> index 000000000000..3ab5c8de11a8
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k230-canmv.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
> + */
> +
> +#include "k230.dtsi"
> +
> +/ {
> + model = "Canaan CanMV-K230";
> + compatible = "canaan,canmv-k230", "canaan,kendryte-k230";
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + ddr: memory@0 {
> + device_type = "memory";
> + reg = <0x0 0x0 0x0 0x1fdff000>;
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/canaan/k230-evb.dts b/arch/riscv/boot/dts/canaan/k230-evb.dts
> new file mode 100644
> index 000000000000..42720113c566
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k230-evb.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
> + */
> +
> +#include "k230.dtsi"
> +
> +/ {
> + model = "Kendryte K230 EVB";
> + compatible = "canaan,k230-usip-lp3-evb", "canaan,kendryte-k230";
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + ddr: memory@0 {
> + device_type = "memory";
> + reg = <0x0 0x0 0x0 0x1fdff000>;
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
> new file mode 100644
> index 000000000000..0bcff67b78a8
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k230.dtsi
> @@ -0,0 +1,140 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/dts-v1/;
> +/ {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + compatible = "canaan,kendryte-k230";
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + timebase-frequency = <27000000>;
> +
> + cpu@0 {
> + compatible = "thead,c908", "riscv";
> + device_type = "cpu";
> + reg = <0>;
> + riscv,isa = "rv64imafdcv_zba_zbb_zbc_zbs_zicbom_svpbmt";
> + riscv,isa-base = "rv64i";
> + riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "v", "zba", "zbb",
> + "zbc", "zbs", "zicbom", "zicntr", "zicsr",
> + "zifencei", "zihpm", "svpbmt";
> + riscv,cbom-block-size = <64>;
> + d-cache-block-size = <64>;
> + d-cache-sets = <128>;
> + d-cache-size = <32768>;
> + i-cache-block-size = <64>;
> + i-cache-sets = <128>;
> + i-cache-size = <32768>;
> + next-level-cache = <&l2_cache>;
> + mmu-type = "riscv,sv39";
> +
> + cpu0_intc: interrupt-controller {
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> + };
> +
> + l2_cache: l2-cache {
> + compatible = "cache";
> + cache-block-size = <64>;
> + cache-level = <2>;
> + cache-size = <262144>;
> + cache-sets = <256>;
> + cache-unified;
> + };
> + };
> +
> + apb_clk: apb-clk-clock {
> + compatible = "fixed-clock";
> + clock-frequency = <50000000>;
> + clock-output-names = "apb_clk";
> + #clock-cells = <0>;
> + };
> +
> + soc {
> + compatible = "simple-bus";
> + interrupt-parent = <&plic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + dma-noncoherent;
> + ranges;
> +
> + plic: interrupt-controller@f00000000 {
> + compatible = "canaan,k230-plic" ,"thead,c900-plic";
> + reg = <0xf 0x00000000 0x0 0x04000000>;
> + interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
> + interrupt-controller;
> + #address-cells = <0>;
> + #interrupt-cells = <2>;
> + riscv,ndev = <208>;
> + };
> +
> + clint: timer@f04000000 {
> + compatible = "canaan,k230-clint", "thead,c900-clint";
> + reg = <0xf 0x04000000 0x0 0x04000000>;
> + interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> + };
> +
> + uart0: serial@91400000 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0x0 0x91400000 0x0 0x1000>;
> + clocks = <&apb_clk>;
> + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + status = "disabled";
> + };
> +
> + uart1: serial@91401000 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0x0 0x91401000 0x0 0x1000>;
> + clocks = <&apb_clk>;
> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + status = "disabled";
> + };
> +
> + uart2: serial@91402000 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0x0 0x91402000 0x0 0x1000>;
> + clocks = <&apb_clk>;
> + interrupts = <18 IRQ_TYPE_LEVEL_HIGH>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + status = "disabled";
> + };
> +
> + uart3: serial@91403000 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0x0 0x91403000 0x0 0x1000>;
> + clocks = <&apb_clk>;
> + interrupts = <19 IRQ_TYPE_LEVEL_HIGH>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + status = "disabled";
> + };
> +
> + uart4: serial@91404000 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0x0 0x91404000 0x0 0x1000>;
> + clocks = <&apb_clk>;
> + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + status = "disabled";
> + };
> + };
> +};

2024-03-05 17:02:02

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts

Hey,

Just chiming so that things don't get misinterpreted.

On Wed, Mar 06, 2024 at 12:37:16AM +0800, Yangyu Chen wrote:
> > On Mar 6, 2024, at 00:22, Yangyu Chen <[email protected]> wrote:
> >> On Mar 5, 2024, at 23:54, Palmer Dabbelt <[email protected]> wrote:
> >> On Mon, 04 Mar 2024 13:06:00 PST (-0800), [email protected] wrote:
> >>> - Svpbmt and T-Head MAEE both supported
> >>>
> >>> T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
> >>> attributes and is controlled by csr.mxstatus. If the kernel wants to use
> >>> svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
> >>> before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
> >>> on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
> >>> lines modification that haven't been written back to the memory.
> >>
> >> So I guess we need the bootloader to just be accurate here? ie: whatever
> >> extension it tells S-mode kernels is enabled is how S-mode behaves, and
> >> then this should just work itself out.

Correct, the bootloader/firmware "just" needs to write this bit to match
what it passes to onwards in the devicetree.

> > Yes. Currently, I have patched OpenSBI to disable MAEE. Conor Dooley said
> > from a public irc group that he wants to control the enable of T-Head

(#riscv on libera, the usual location)

I also suggested that that, given we can use the standard extensions,
we should use them instead of the custom extensions/errata.

> > variation of zicbom and svpbmt from dts, in addition to mimplid or
> > something now.

Correct. I'm find with the impid == archid == 0 condition, given that's
what we need to keep to avoid regressions, but if any future T-Head CPUs
want to enable MAEE (ERRATA_THEAD_PBMT) or the custom CMOs
(ERRATA_THEAD_CMO) these should be enabled from DT. Particularly when
these CPUs can be configured to either use the T-Head versions or the
standard extensions.

> > I think that will be a better way for the bootloader to tell
> > the kernel whether the T-Head MAEE should be enabled.

You've got three options I guess. You could patch the DT in the bootloader,
or use a fixed DT that matches the bootloader, or you could use the DT
passed to the bootloader and parse the extensions to decide whether or not
to enable MAEE or Svpbmt. Seems you're going for option 2.

> > Link: https://github.com/cyyself/opensbi/commit/b113c1c01d700314a4a696297ec09031a9399354
> >
> > Furthermore, I wonder whether a CPU node like this would be acceptable.
> > I don't have any other details of how another CPU from K230 SoC works on
> > Linux.

A CPU node like what? It is not clear to me.


Attachments:
(No filename) (2.67 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-05 17:21:16

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230

On Tue, Mar 05, 2024 at 03:47:15PM +0800, Yangyu Chen wrote:
> On 2024/3/5 07:46, Damien Le Moal wrote:
> > On 3/5/24 06:05, Yangyu Chen wrote:
> > > Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
> > > Remove it depends on !MMU will allow building dts for K230 and remove the
> > > K210 string from the help message.
> > >
> > > Signed-off-by: Yangyu Chen <[email protected]>
> > > Reviewed-by: Conor Dooley <[email protected]>
> > > ---
> > > arch/riscv/Kconfig.socs | 5 ++---
> > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > > index 623de5f8a208..b4e9b7f75510 100644
> > > --- a/arch/riscv/Kconfig.socs
> > > +++ b/arch/riscv/Kconfig.socs
> > > @@ -75,13 +75,12 @@ config ARCH_CANAAN
> > > def_bool SOC_CANAAN
> > > config SOC_CANAAN
> > > - bool "Canaan Kendryte K210 SoC"
> > > - depends on !MMU
> >
> > This seems wrong to me. The k210 support does require no-mmu. So why remove
> > this ?
>
> It just allows SOC_CANAAN to be selected when MMU=y. With this patch,
> nommu_k210_defconfig still works.

I think the concern here is that this would allow people to build a
kernel for the k120 with the MMU enabled, not that the existing nommu
build will be affected.

Maybe you could squash in something like the following?

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index b4e9b7f75510..75d55059163f 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -72,15 +72,19 @@ config SOC_VIRT
This enables support for QEMU Virt Machine.

config ARCH_CANAAN
- def_bool SOC_CANAAN
+ bool "Canaan Kendryte SoCs"
+ help
+ This enables support for Canaan Kendryte SoC platform hardware.

config SOC_CANAAN
- bool "Canaan Kendryte SoC"
+ bool "Canaan Kendryte K210 SoC"
+ depends on !MMU
+ depends on ARCH_CANAAN
select CLINT_TIMER if RISCV_M_MODE
select ARCH_HAS_RESET_CONTROLLER
select PINCTRL
select COMMON_CLK
help
- This enables support for Canaan Kendryte SoC platform hardware.
+ This enables support for Canaan Kendryte K210 SoC platform hardware.

endmenu # "SoC selection"

(Which reminds me, I really need to go and finish sorting out the ARCH_
stuff)


Attachments:
(No filename) (2.26 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-05 17:25:00

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts

On Wed, Mar 06, 2024 at 01:17:29AM +0800, Yangyu Chen wrote:
> > On Mar 6, 2024, at 01:01, Conor Dooley <[email protected]> wrote:
> > On Wed, Mar 06, 2024 at 12:37:16AM +0800, Yangyu Chen wrote:

> >>> Link: https://github.com/cyyself/opensbi/commit/b113c1c01d700314a4a696297ec09031a9399354
> >>>
> >>> Furthermore, I wonder whether a CPU node like this would be acceptable.
> >>> I don't have any other details of how another CPU from K230 SoC works on
> >>> Linux.
> >
> > A CPU node like what? It is not clear to me.
>
> It in the k230.dtsi file. Only has big core there.

The node that is currently there looks fine to me.


Attachments:
(No filename) (645.00 B)
signature.asc (235.00 B)
Download all attachments

2024-03-06 00:01:15

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230

On 3/6/24 02:20, Conor Dooley wrote:
> On Tue, Mar 05, 2024 at 03:47:15PM +0800, Yangyu Chen wrote:
>> On 2024/3/5 07:46, Damien Le Moal wrote:
>>> On 3/5/24 06:05, Yangyu Chen wrote:
>>>> Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
>>>> Remove it depends on !MMU will allow building dts for K230 and remove the
>>>> K210 string from the help message.
>>>>
>>>> Signed-off-by: Yangyu Chen <[email protected]>
>>>> Reviewed-by: Conor Dooley <[email protected]>
>>>> ---
>>>> arch/riscv/Kconfig.socs | 5 ++---
>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>> index 623de5f8a208..b4e9b7f75510 100644
>>>> --- a/arch/riscv/Kconfig.socs
>>>> +++ b/arch/riscv/Kconfig.socs
>>>> @@ -75,13 +75,12 @@ config ARCH_CANAAN
>>>> def_bool SOC_CANAAN
>>>> config SOC_CANAAN
>>>> - bool "Canaan Kendryte K210 SoC"
>>>> - depends on !MMU
>>>
>>> This seems wrong to me. The k210 support does require no-mmu. So why remove
>>> this ?
>>
>> It just allows SOC_CANAAN to be selected when MMU=y. With this patch,
>> nommu_k210_defconfig still works.
>
> I think the concern here is that this would allow people to build a
> kernel for the k120 with the MMU enabled, not that the existing nommu
> build will be affected.

Yes, this is my concern. Apologies for the lack of clarity.

>
> Maybe you could squash in something like the following?
>
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index b4e9b7f75510..75d55059163f 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -72,15 +72,19 @@ config SOC_VIRT
> This enables support for QEMU Virt Machine.
>
> config ARCH_CANAAN
> - def_bool SOC_CANAAN
> + bool "Canaan Kendryte SoCs"
> + help
> + This enables support for Canaan Kendryte SoC platform hardware.
>
> config SOC_CANAAN
> - bool "Canaan Kendryte SoC"
> + bool "Canaan Kendryte K210 SoC"
> + depends on !MMU
> + depends on ARCH_CANAAN
> select CLINT_TIMER if RISCV_M_MODE
> select ARCH_HAS_RESET_CONTROLLER
> select PINCTRL
> select COMMON_CLK
> help
> - This enables support for Canaan Kendryte SoC platform hardware.
> + This enables support for Canaan Kendryte K210 SoC platform hardware.
>
> endmenu # "SoC selection"
>
> (Which reminds me, I really need to go and finish sorting out the ARCH_
> stuff)

--
Damien Le Moal
Western Digital Research


2024-03-13 18:01:11

by Yangyu Chen

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230



On 2024/3/8 05:03, Yangyu Chen wrote:
>
>
>> On Mar 6, 2024, at 07:58, Damien Le Moal <[email protected]> wrote:
>>
>> On 3/6/24 02:20, Conor Dooley wrote:
>>> On Tue, Mar 05, 2024 at 03:47:15PM +0800, Yangyu Chen wrote:
>>>> On 2024/3/5 07:46, Damien Le Moal wrote:
>>>>> On 3/5/24 06:05, Yangyu Chen wrote:
>>>>>> Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
>>>>>> Remove it depends on !MMU will allow building dts for K230 and remove the
>>>>>> K210 string from the help message.
>>>>>>
>>>>>> Signed-off-by: Yangyu Chen <[email protected]>
>>>>>> Reviewed-by: Conor Dooley <[email protected]>
>>>>>> ---
>>>>>> arch/riscv/Kconfig.socs | 5 ++---
>>>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>>>> index 623de5f8a208..b4e9b7f75510 100644
>>>>>> --- a/arch/riscv/Kconfig.socs
>>>>>> +++ b/arch/riscv/Kconfig.socs
>>>>>> @@ -75,13 +75,12 @@ config ARCH_CANAAN
>>>>>> def_bool SOC_CANAAN
>>>>>> config SOC_CANAAN
>>>>>> - bool "Canaan Kendryte K210 SoC"
>>>>>> - depends on !MMU
>>>>>
>>>>> This seems wrong to me. The k210 support does require no-mmu. So why remove
>>>>> this ?
>>>>
>>>> It just allows SOC_CANAAN to be selected when MMU=y. With this patch,
>>>> nommu_k210_defconfig still works.
>>>
>>> I think the concern here is that this would allow people to build a
>>> kernel for the k120 with the MMU enabled, not that the existing nommu
>>> build will be affected.
>>
>> Yes, this is my concern. Apologies for the lack of clarity.
>>
>
> Hi,
>
> Thanks for the review comments. After thinking about it for a while,
> I think we don't need to change it as we have changed the help
> message which deleted the "K210". And the dts on k210.dtsi shows
> mmu-type is riscv.none, I think if someone noticed this would know
> why it fails to boot on the S-Mode MMU Kernel on K210. The only
> special thing for ARCH_CANAAN is that a loader.bin will be built
> when M-Mode is on arch/riscv/Makefile. However, Canaan has no other
> M-Mode chips except for K210. So I think we don't need to change
> it.
>
> Another reason is that SOC_CANAAN for K210 is somehow hard to change.
> If we continue using SOC_CANAAN for K210 but not for other Canaan
> SoCs such as K230, it will cause some confusion to users. If we
> rename SOC_CANAAN to SOC_CANAAN_K210, it will change many drivers
> in many subsystems like my patch v5 [1]. So I don't think we need
> to fix it.
>
>
> If we don't change it, A concern for this is that some drivers for
> K210 will be built when SOC_CANAAN=y and if we add this to defconfig,
> all riscv builds will also build some K210 drivers even on MMU. But
> I think this will not be a problem just need some memory/storage
> for a slightly bigger kernel. Also, we will enable some new configs
> in defconfig when a new soc gets supported, it's normal for K210
> SoC drivers.
>
> Thus, I think we don't need to change it. If you have some other
> opinions, please let me know.
>
> [1] https://lore.kernel.org/linux-riscv/[email protected]/
>
> Thanks,
> Yangyu Chen
>

Hi Damien,

I'm waiting for your decision before sending the next patch revision.
Please reply to this when you are free.

Thanks,
Yangyu Chen

>>>
>>> Maybe you could squash in something like the following?
>>>
>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>> index b4e9b7f75510..75d55059163f 100644
>>> --- a/arch/riscv/Kconfig.socs
>>> +++ b/arch/riscv/Kconfig.socs
>>> @@ -72,15 +72,19 @@ config SOC_VIRT
>>> This enables support for QEMU Virt Machine.
>>>
>>> config ARCH_CANAAN
>>> - def_bool SOC_CANAAN
>>> + bool "Canaan Kendryte SoCs"
>>> + help
>>> + This enables support for Canaan Kendryte SoC platform hardware.
>>>
>>> config SOC_CANAAN
>>> - bool "Canaan Kendryte SoC"
>>> + bool "Canaan Kendryte K210 SoC"
>>> + depends on !MMU
>>> + depends on ARCH_CANAAN
>>> select CLINT_TIMER if RISCV_M_MODE
>>> select ARCH_HAS_RESET_CONTROLLER
>>> select PINCTRL
>>> select COMMON_CLK
>>> help
>>> - This enables support for Canaan Kendryte SoC platform hardware.
>>> + This enables support for Canaan Kendryte K210 SoC platform hardware.
>>>
>>> endmenu # "SoC selection"
>>>
>>> (Which reminds me, I really need to go and finish sorting out the ARCH_
>>> stuff)
>>
>> --
>> Damien Le Moal
>> Western Digital Research
>
>


2024-03-07 21:04:07

by Yangyu Chen

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230



> On Mar 6, 2024, at 07:58, Damien Le Moal <[email protected]> wrote:
>
> On 3/6/24 02:20, Conor Dooley wrote:
>> On Tue, Mar 05, 2024 at 03:47:15PM +0800, Yangyu Chen wrote:
>>> On 2024/3/5 07:46, Damien Le Moal wrote:
>>>> On 3/5/24 06:05, Yangyu Chen wrote:
>>>>> Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
>>>>> Remove it depends on !MMU will allow building dts for K230 and remove the
>>>>> K210 string from the help message.
>>>>>
>>>>> Signed-off-by: Yangyu Chen <[email protected]>
>>>>> Reviewed-by: Conor Dooley <[email protected]>
>>>>> ---
>>>>> arch/riscv/Kconfig.socs | 5 ++---
>>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>>> index 623de5f8a208..b4e9b7f75510 100644
>>>>> --- a/arch/riscv/Kconfig.socs
>>>>> +++ b/arch/riscv/Kconfig.socs
>>>>> @@ -75,13 +75,12 @@ config ARCH_CANAAN
>>>>> def_bool SOC_CANAAN
>>>>> config SOC_CANAAN
>>>>> - bool "Canaan Kendryte K210 SoC"
>>>>> - depends on !MMU
>>>>
>>>> This seems wrong to me. The k210 support does require no-mmu. So why remove
>>>> this ?
>>>
>>> It just allows SOC_CANAAN to be selected when MMU=y. With this patch,
>>> nommu_k210_defconfig still works.
>>
>> I think the concern here is that this would allow people to build a
>> kernel for the k120 with the MMU enabled, not that the existing nommu
>> build will be affected.
>
> Yes, this is my concern. Apologies for the lack of clarity.
>

Hi,

Thanks for the review comments. After thinking about it for a while,
I think we don't need to change it as we have changed the help
message which deleted the "K210". And the dts on k210.dtsi shows
mmu-type is riscv.none, I think if someone noticed this would know
why it fails to boot on the S-Mode MMU Kernel on K210. The only
special thing for ARCH_CANAAN is that a loader.bin will be built
when M-Mode is on arch/riscv/Makefile. However, Canaan has no other
M-Mode chips except for K210. So I think we don't need to change
it.

Another reason is that SOC_CANAAN for K210 is somehow hard to change.
If we continue using SOC_CANAAN for K210 but not for other Canaan
SoCs such as K230, it will cause some confusion to users. If we
rename SOC_CANAAN to SOC_CANAAN_K210, it will change many drivers
in many subsystems like my patch v5 [1]. So I don't think we need
to fix it.


If we don't change it, A concern for this is that some drivers for
K210 will be built when SOC_CANAAN=y and if we add this to defconfig,
all riscv builds will also build some K210 drivers even on MMU. But
I think this will not be a problem just need some memory/storage
for a slightly bigger kernel. Also, we will enable some new configs
in defconfig when a new soc gets supported, it's normal for K210
SoC drivers.

Thus, I think we don't need to change it. If you have some other
opinions, please let me know.

[1] https://lore.kernel.org/linux-riscv/[email protected]/

Thanks,
Yangyu Chen

>>
>> Maybe you could squash in something like the following?
>>
>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>> index b4e9b7f75510..75d55059163f 100644
>> --- a/arch/riscv/Kconfig.socs
>> +++ b/arch/riscv/Kconfig.socs
>> @@ -72,15 +72,19 @@ config SOC_VIRT
>> This enables support for QEMU Virt Machine.
>>
>> config ARCH_CANAAN
>> - def_bool SOC_CANAAN
>> + bool "Canaan Kendryte SoCs"
>> + help
>> + This enables support for Canaan Kendryte SoC platform hardware.
>>
>> config SOC_CANAAN
>> - bool "Canaan Kendryte SoC"
>> + bool "Canaan Kendryte K210 SoC"
>> + depends on !MMU
>> + depends on ARCH_CANAAN
>> select CLINT_TIMER if RISCV_M_MODE
>> select ARCH_HAS_RESET_CONTROLLER
>> select PINCTRL
>> select COMMON_CLK
>> help
>> - This enables support for Canaan Kendryte SoC platform hardware.
>> + This enables support for Canaan Kendryte K210 SoC platform hardware.
>>
>> endmenu # "SoC selection"
>>
>> (Which reminds me, I really need to go and finish sorting out the ARCH_
>> stuff)
>
> --
> Damien Le Moal
> Western Digital Research



2024-03-05 16:27:08

by Yangyu Chen

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts


> On Mar 5, 2024, at 23:54, Palmer Dabbelt <[email protected]> wrote:
>
> On Mon, 04 Mar 2024 13:06:00 PST (-0800), [email protected] wrote:
>> Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
>> K230 SoC [1].
>>
>> Some key consideration:
>> - Only place BigCore which is 1.6GHz RV64GCBV
>>
>> The existence of cache coherence between the two cores remains unknown
>> since they have dedicated L2 caches. And the factory SDK uses it for
>> other OS by default. I don't know whether the two CPUs on K230 SoC
>> can be used in one system. So only place BigCore here.
>>
>> Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
>> CPU1, the csr.mhartid of this core is 0.
>>
>> - Support for "zba" "zbb" "zbc" "zbs" are tested by hand
>>
>> The user manual of C908 from T-Head does not document it specifically.
>> It just said it supports B extension V1.0-rc1. [2]
>
> I'm seeing
>
> 3.1.6 Vector instruction set (RVV)
> For more information, see RISC-V “V”Vector Extension, Version 1.0-rc1-20210608
> URL:https://github.com/riscv/riscv-v-spec/releases/download/v1.0-rc1/riscv-v-spec-1.0-rc1.pdf
>
> 3.1.7 Bit operation instruction set (RV64B)
> For more information, see :RISC-V Bit-Manipulation ISA-extensions, Version 1.0.0, 2021-06-12: public review
> URL:https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0.pdf
>
> so I think maybe that's vector 1.0-rc1 and the final bitmanip?
>

Yeah. I just want to say what subset of the B extension is supported by
C908.

>> I have tested it by using this [3] which attempts to execute "add.uw",
>> "andn", "clmulr", "bclr" and they doesn't traps on K230. But on JH7110,
>> "clmulr" and "bclr" will trap.
>>
>> - Support for "zicbom" is tested by hand
>>
>> Have tested with some out-of-tree drivers from [4] that need DMA and they
>> do not come to the dts currently.
>>
>> - Cache parameters are inferred from T-Head docs [2] and Canaan docs [1]
>>
>> L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
>> L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
>> L2: 256KB, PIPT 16-way set-associative, 64B Cacheline
>>
>> The numbers of cache sets are calculated from these parameters.
>>
>> - MMU only supports Sv39
>>
>> Since T-Head docs [2] say C908 should support Sv48. However, it will fail
>> during the kernel probe when running Linux on K230. I also tested it by
>> hand on M-Mode software, writing Sv48 to satp.mode will not trap but will
>> leave the csr unchanged. While writing Sv39 it will take effect. It shows
>> that this CPU does not support Sv48.
>
> I think that only works due to a bug in our sv48 probing routines -- unless I'm missing something, we mixed up the sv57 and sv48 probing and if sv57 fails to probe then we disable sv48 as well. So I think we probably need some cleanup in there.
>

However, the sv48 probing routines work on some other chips with sv48. At
least I have tested mainline kernel v6.8-rc2. And the attempt to write sv48
to satp by hand tells me it does not support sv48. I think it might be
configurable from CPU RTL, and the K230 taped out with no sv48 support.

>> - Svpbmt and T-Head MAEE both supported
>>
>> T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
>> attributes and is controlled by csr.mxstatus. If the kernel wants to use
>> svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
>> before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
>> on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
>> lines modification that haven't been written back to the memory.
>
> So I guess we need the bootloader to just be accurate here? ie: whatever extension it tells S-mode kernels is enabled is how S-mode behaves, and then this should just work itself out.
>

Yes. Currently, I have patched OpenSBI to disable MAEE. Conor Dooley said
from a public irc group that he wants to control the enable of T-Head
variation of zicbom and svpbmt from dts, in addition to mimplid or
something now. I think that will be a better way for the bootloader to tell
the kernel whether the T-Head MAEE should be enabled.

Link: https://github.com/cyyself/opensbi/commit/b113c1c01d700314a4a696297ec09031a9399354

Furthermore, I wonder whether a CPU node like this would be acceptable.
I don't have any other details of how another CPU from K230 SoC works on
Linux.

>> [1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
>> [2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf
>> [3] https://github.com/cyyself/rvb_test
>> [4] https://github.com/cyyself/linux/tree/k230-mainline
>>
>> Signed-off-by: Yangyu Chen <[email protected]>
>> ---
>> arch/riscv/boot/dts/canaan/Makefile | 2 +
>> arch/riscv/boot/dts/canaan/k230-canmv.dts | 24 ++++
>> arch/riscv/boot/dts/canaan/k230-evb.dts | 24 ++++
>> arch/riscv/boot/dts/canaan/k230.dtsi | 140 ++++++++++++++++++++++
>> 4 files changed, 190 insertions(+)
>> create mode 100644 arch/riscv/boot/dts/canaan/k230-canmv.dts
>> create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
>> create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi
>>
>> diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
>> index 987d1f0c41f0..7d54ea5c6f3d 100644
>> --- a/arch/riscv/boot/dts/canaan/Makefile
>> +++ b/arch/riscv/boot/dts/canaan/Makefile
>> @@ -1,6 +1,8 @@
>> # SPDX-License-Identifier: GPL-2.0
>> dtb-$(CONFIG_ARCH_CANAAN) += canaan_kd233.dtb
>> dtb-$(CONFIG_ARCH_CANAAN) += k210_generic.dtb
>> +dtb-$(CONFIG_ARCH_CANAAN) += k230-canmv.dtb
>> +dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
>> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
>> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
>> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
>> diff --git a/arch/riscv/boot/dts/canaan/k230-canmv.dts b/arch/riscv/boot/dts/canaan/k230-canmv.dts
>> new file mode 100644
>> index 000000000000..3ab5c8de11a8
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/canaan/k230-canmv.dts
>> @@ -0,0 +1,24 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +/*
>> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
>> + */
>> +
>> +#include "k230.dtsi"
>> +
>> +/ {
>> + model = "Canaan CanMV-K230";
>> + compatible = "canaan,canmv-k230", "canaan,kendryte-k230";
>> +
>> + chosen {
>> + stdout-path = "serial0:115200n8";
>> + };
>> +
>> + ddr: memory@0 {
>> + device_type = "memory";
>> + reg = <0x0 0x0 0x0 0x1fdff000>;
>> + };
>> +};
>> +
>> +&uart0 {
>> + status = "okay";
>> +};
>> diff --git a/arch/riscv/boot/dts/canaan/k230-evb.dts b/arch/riscv/boot/dts/canaan/k230-evb.dts
>> new file mode 100644
>> index 000000000000..42720113c566
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/canaan/k230-evb.dts
>> @@ -0,0 +1,24 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +/*
>> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
>> + */
>> +
>> +#include "k230.dtsi"
>> +
>> +/ {
>> + model = "Kendryte K230 EVB";
>> + compatible = "canaan,k230-usip-lp3-evb", "canaan,kendryte-k230";
>> +
>> + chosen {
>> + stdout-path = "serial0:115200n8";
>> + };
>> +
>> + ddr: memory@0 {
>> + device_type = "memory";
>> + reg = <0x0 0x0 0x0 0x1fdff000>;
>> + };
>> +};
>> +
>> +&uart0 {
>> + status = "okay";
>> +};
>> diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
>> new file mode 100644
>> index 000000000000..0bcff67b78a8
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/canaan/k230.dtsi
>> @@ -0,0 +1,140 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +/*
>> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +/dts-v1/;
>> +/ {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + compatible = "canaan,kendryte-k230";
>> +
>> + aliases {
>> + serial0 = &uart0;
>> + };
>> +
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + timebase-frequency = <27000000>;
>> +
>> + cpu@0 {
>> + compatible = "thead,c908", "riscv";
>> + device_type = "cpu";
>> + reg = <0>;
>> + riscv,isa = "rv64imafdcv_zba_zbb_zbc_zbs_zicbom_svpbmt";
>> + riscv,isa-base = "rv64i";
>> + riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "v", "zba", "zbb",
>> + "zbc", "zbs", "zicbom", "zicntr", "zicsr",
>> + "zifencei", "zihpm", "svpbmt";
>> + riscv,cbom-block-size = <64>;
>> + d-cache-block-size = <64>;
>> + d-cache-sets = <128>;
>> + d-cache-size = <32768>;
>> + i-cache-block-size = <64>;
>> + i-cache-sets = <128>;
>> + i-cache-size = <32768>;
>> + next-level-cache = <&l2_cache>;
>> + mmu-type = "riscv,sv39";
>> +
>> + cpu0_intc: interrupt-controller {
>> + compatible = "riscv,cpu-intc";
>> + interrupt-controller;
>> + #interrupt-cells = <1>;
>> + };
>> + };
>> +
>> + l2_cache: l2-cache {
>> + compatible = "cache";
>> + cache-block-size = <64>;
>> + cache-level = <2>;
>> + cache-size = <262144>;
>> + cache-sets = <256>;
>> + cache-unified;
>> + };
>> + };
>> +
>> + apb_clk: apb-clk-clock {
>> + compatible = "fixed-clock";
>> + clock-frequency = <50000000>;
>> + clock-output-names = "apb_clk";
>> + #clock-cells = <0>;
>> + };
>> +
>> + soc {
>> + compatible = "simple-bus";
>> + interrupt-parent = <&plic>;
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + dma-noncoherent;
>> + ranges;
>> +
>> + plic: interrupt-controller@f00000000 {
>> + compatible = "canaan,k230-plic" ,"thead,c900-plic";
>> + reg = <0xf 0x00000000 0x0 0x04000000>;
>> + interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
>> + interrupt-controller;
>> + #address-cells = <0>;
>> + #interrupt-cells = <2>;
>> + riscv,ndev = <208>;
>> + };
>> +
>> + clint: timer@f04000000 {
>> + compatible = "canaan,k230-clint", "thead,c900-clint";
>> + reg = <0xf 0x04000000 0x0 0x04000000>;
>> + interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
>> + };
>> +
>> + uart0: serial@91400000 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x0 0x91400000 0x0 0x1000>;
>> + clocks = <&apb_clk>;
>> + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + status = "disabled";
>> + };
>> +
>> + uart1: serial@91401000 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x0 0x91401000 0x0 0x1000>;
>> + clocks = <&apb_clk>;
>> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + status = "disabled";
>> + };
>> +
>> + uart2: serial@91402000 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x0 0x91402000 0x0 0x1000>;
>> + clocks = <&apb_clk>;
>> + interrupts = <18 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + status = "disabled";
>> + };
>> +
>> + uart3: serial@91403000 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x0 0x91403000 0x0 0x1000>;
>> + clocks = <&apb_clk>;
>> + interrupts = <19 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + status = "disabled";
>> + };
>> +
>> + uart4: serial@91404000 {
>> + compatible = "snps,dw-apb-uart";
>> + reg = <0x0 0x91404000 0x0 0x1000>;
>> + clocks = <&apb_clk>;
>> + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + status = "disabled";
>> + };
>> + };
>> +};



2024-03-05 16:37:53

by Yangyu Chen

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts


> On Mar 6, 2024, at 00:22, Yangyu Chen <[email protected]> wrote:
>
>>
>> On Mar 5, 2024, at 23:54, Palmer Dabbelt <[email protected]> wrote:
>>
>> On Mon, 04 Mar 2024 13:06:00 PST (-0800), [email protected] wrote:
>>> Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
>>> K230 SoC [1].
>>>
>>> Some key consideration:
>>> - Only place BigCore which is 1.6GHz RV64GCBV
>>>
>>> The existence of cache coherence between the two cores remains unknown
>>> since they have dedicated L2 caches. And the factory SDK uses it for
>>> other OS by default. I don't know whether the two CPUs on K230 SoC
>>> can be used in one system. So only place BigCore here.
>>>
>>> Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
>>> CPU1, the csr.mhartid of this core is 0.
>>>
>>> - Support for "zba" "zbb" "zbc" "zbs" are tested by hand
>>>
>>> The user manual of C908 from T-Head does not document it specifically.
>>> It just said it supports B extension V1.0-rc1. [2]
>>
>> I'm seeing
>>
>> 3.1.6 Vector instruction set (RVV)
>> For more information, see RISC-V “V”Vector Extension, Version 1.0-rc1-20210608
>> URL:https://github.com/riscv/riscv-v-spec/releases/download/v1.0-rc1/riscv-v-spec-1.0-rc1.pdf
>>
>> 3.1.7 Bit operation instruction set (RV64B)
>> For more information, see :RISC-V Bit-Manipulation ISA-extensions, Version 1.0.0, 2021-06-12: public review
>> URL:https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0.pdf
>>
>> so I think maybe that's vector 1.0-rc1 and the final bitmanip?
>>
>
> Yeah. I just want to say what subset of the B extension is supported by
> C908.
>

Oh. I misunderstand it. The version of bitmanip is 1.0 rather than 1.0-rc1
I said before. The version of the vector is 1.0-rc1. I will change that in
the next revision.

>>> I have tested it by using this [3] which attempts to execute "add.uw",
>>> "andn", "clmulr", "bclr" and they doesn't traps on K230. But on JH7110,
>>> "clmulr" and "bclr" will trap.
>>>
>>> - Support for "zicbom" is tested by hand
>>>
>>> Have tested with some out-of-tree drivers from [4] that need DMA and they
>>> do not come to the dts currently.
>>>
>>> - Cache parameters are inferred from T-Head docs [2] and Canaan docs [1]
>>>
>>> L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
>>> L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
>>> L2: 256KB, PIPT 16-way set-associative, 64B Cacheline
>>>
>>> The numbers of cache sets are calculated from these parameters.
>>>
>>> - MMU only supports Sv39
>>>
>>> Since T-Head docs [2] say C908 should support Sv48. However, it will fail
>>> during the kernel probe when running Linux on K230. I also tested it by
>>> hand on M-Mode software, writing Sv48 to satp.mode will not trap but will
>>> leave the csr unchanged. While writing Sv39 it will take effect. It shows
>>> that this CPU does not support Sv48.
>>
>> I think that only works due to a bug in our sv48 probing routines -- unless I'm missing something, we mixed up the sv57 and sv48 probing and if sv57 fails to probe then we disable sv48 as well. So I think we probably need some cleanup in there.
>>
>
> However, the sv48 probing routines work on some other chips with sv48. At
> least I have tested mainline kernel v6.8-rc2. And the attempt to write sv48
> to satp by hand tells me it does not support sv48. I think it might be
> configurable from CPU RTL, and the K230 taped out with no sv48 support.
>
>>> - Svpbmt and T-Head MAEE both supported
>>>
>>> T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
>>> attributes and is controlled by csr.mxstatus. If the kernel wants to use
>>> svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
>>> before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
>>> on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
>>> lines modification that haven't been written back to the memory.
>>
>> So I guess we need the bootloader to just be accurate here? ie: whatever extension it tells S-mode kernels is enabled is how S-mode behaves, and then this should just work itself out.
>>
>
> Yes. Currently, I have patched OpenSBI to disable MAEE. Conor Dooley said
> from a public irc group that he wants to control the enable of T-Head
> variation of zicbom and svpbmt from dts, in addition to mimplid or
> something now. I think that will be a better way for the bootloader to tell
> the kernel whether the T-Head MAEE should be enabled.
>
> Link: https://github.com/cyyself/opensbi/commit/b113c1c01d700314a4a696297ec09031a9399354
>
> Furthermore, I wonder whether a CPU node like this would be acceptable.
> I don't have any other details of how another CPU from K230 SoC works on
> Linux.
>
>>> [1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
>>> [2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf
>>> [3] https://github.com/cyyself/rvb_test
>>> [4] https://github.com/cyyself/linux/tree/k230-mainline
>>>
>>> Signed-off-by: Yangyu Chen <[email protected]>
>>> ---
>>> arch/riscv/boot/dts/canaan/Makefile | 2 +
>>> arch/riscv/boot/dts/canaan/k230-canmv.dts | 24 ++++
>>> arch/riscv/boot/dts/canaan/k230-evb.dts | 24 ++++
>>> arch/riscv/boot/dts/canaan/k230.dtsi | 140 ++++++++++++++++++++++
>>> 4 files changed, 190 insertions(+)
>>> create mode 100644 arch/riscv/boot/dts/canaan/k230-canmv.dts
>>> create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
>>> create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi
>>>
>>> diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
>>> index 987d1f0c41f0..7d54ea5c6f3d 100644
>>> --- a/arch/riscv/boot/dts/canaan/Makefile
>>> +++ b/arch/riscv/boot/dts/canaan/Makefile
>>> @@ -1,6 +1,8 @@
>>> # SPDX-License-Identifier: GPL-2.0
>>> dtb-$(CONFIG_ARCH_CANAAN) += canaan_kd233.dtb
>>> dtb-$(CONFIG_ARCH_CANAAN) += k210_generic.dtb
>>> +dtb-$(CONFIG_ARCH_CANAAN) += k230-canmv.dtb
>>> +dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
>>> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
>>> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
>>> dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
>>> diff --git a/arch/riscv/boot/dts/canaan/k230-canmv.dts b/arch/riscv/boot/dts/canaan/k230-canmv.dts
>>> new file mode 100644
>>> index 000000000000..3ab5c8de11a8
>>> --- /dev/null
>>> +++ b/arch/riscv/boot/dts/canaan/k230-canmv.dts
>>> @@ -0,0 +1,24 @@
>>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>>> +/*
>>> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
>>> + */
>>> +
>>> +#include "k230.dtsi"
>>> +
>>> +/ {
>>> + model = "Canaan CanMV-K230";
>>> + compatible = "canaan,canmv-k230", "canaan,kendryte-k230";
>>> +
>>> + chosen {
>>> + stdout-path = "serial0:115200n8";
>>> + };
>>> +
>>> + ddr: memory@0 {
>>> + device_type = "memory";
>>> + reg = <0x0 0x0 0x0 0x1fdff000>;
>>> + };
>>> +};
>>> +
>>> +&uart0 {
>>> + status = "okay";
>>> +};
>>> diff --git a/arch/riscv/boot/dts/canaan/k230-evb.dts b/arch/riscv/boot/dts/canaan/k230-evb.dts
>>> new file mode 100644
>>> index 000000000000..42720113c566
>>> --- /dev/null
>>> +++ b/arch/riscv/boot/dts/canaan/k230-evb.dts
>>> @@ -0,0 +1,24 @@
>>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>>> +/*
>>> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
>>> + */
>>> +
>>> +#include "k230.dtsi"
>>> +
>>> +/ {
>>> + model = "Kendryte K230 EVB";
>>> + compatible = "canaan,k230-usip-lp3-evb", "canaan,kendryte-k230";
>>> +
>>> + chosen {
>>> + stdout-path = "serial0:115200n8";
>>> + };
>>> +
>>> + ddr: memory@0 {
>>> + device_type = "memory";
>>> + reg = <0x0 0x0 0x0 0x1fdff000>;
>>> + };
>>> +};
>>> +
>>> +&uart0 {
>>> + status = "okay";
>>> +};
>>> diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
>>> new file mode 100644
>>> index 000000000000..0bcff67b78a8
>>> --- /dev/null
>>> +++ b/arch/riscv/boot/dts/canaan/k230.dtsi
>>> @@ -0,0 +1,140 @@
>>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>>> +/*
>>> + * Copyright (C) 2024 Yangyu Chen <[email protected]>
>>> + */
>>> +
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +/dts-v1/;
>>> +/ {
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + compatible = "canaan,kendryte-k230";
>>> +
>>> + aliases {
>>> + serial0 = &uart0;
>>> + };
>>> +
>>> + cpus {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + timebase-frequency = <27000000>;
>>> +
>>> + cpu@0 {
>>> + compatible = "thead,c908", "riscv";
>>> + device_type = "cpu";
>>> + reg = <0>;
>>> + riscv,isa = "rv64imafdcv_zba_zbb_zbc_zbs_zicbom_svpbmt";
>>> + riscv,isa-base = "rv64i";
>>> + riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "v", "zba", "zbb",
>>> + "zbc", "zbs", "zicbom", "zicntr", "zicsr",
>>> + "zifencei", "zihpm", "svpbmt";
>>> + riscv,cbom-block-size = <64>;
>>> + d-cache-block-size = <64>;
>>> + d-cache-sets = <128>;
>>> + d-cache-size = <32768>;
>>> + i-cache-block-size = <64>;
>>> + i-cache-sets = <128>;
>>> + i-cache-size = <32768>;
>>> + next-level-cache = <&l2_cache>;
>>> + mmu-type = "riscv,sv39";
>>> +
>>> + cpu0_intc: interrupt-controller {
>>> + compatible = "riscv,cpu-intc";
>>> + interrupt-controller;
>>> + #interrupt-cells = <1>;
>>> + };
>>> + };
>>> +
>>> + l2_cache: l2-cache {
>>> + compatible = "cache";
>>> + cache-block-size = <64>;
>>> + cache-level = <2>;
>>> + cache-size = <262144>;
>>> + cache-sets = <256>;
>>> + cache-unified;
>>> + };
>>> + };
>>> +
>>> + apb_clk: apb-clk-clock {
>>> + compatible = "fixed-clock";
>>> + clock-frequency = <50000000>;
>>> + clock-output-names = "apb_clk";
>>> + #clock-cells = <0>;
>>> + };
>>> +
>>> + soc {
>>> + compatible = "simple-bus";
>>> + interrupt-parent = <&plic>;
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + dma-noncoherent;
>>> + ranges;
>>> +
>>> + plic: interrupt-controller@f00000000 {
>>> + compatible = "canaan,k230-plic" ,"thead,c900-plic";
>>> + reg = <0xf 0x00000000 0x0 0x04000000>;
>>> + interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
>>> + interrupt-controller;
>>> + #address-cells = <0>;
>>> + #interrupt-cells = <2>;
>>> + riscv,ndev = <208>;
>>> + };
>>> +
>>> + clint: timer@f04000000 {
>>> + compatible = "canaan,k230-clint", "thead,c900-clint";
>>> + reg = <0xf 0x04000000 0x0 0x04000000>;
>>> + interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
>>> + };
>>> +
>>> + uart0: serial@91400000 {
>>> + compatible = "snps,dw-apb-uart";
>>> + reg = <0x0 0x91400000 0x0 0x1000>;
>>> + clocks = <&apb_clk>;
>>> + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
>>> + reg-io-width = <4>;
>>> + reg-shift = <2>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + uart1: serial@91401000 {
>>> + compatible = "snps,dw-apb-uart";
>>> + reg = <0x0 0x91401000 0x0 0x1000>;
>>> + clocks = <&apb_clk>;
>>> + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
>>> + reg-io-width = <4>;
>>> + reg-shift = <2>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + uart2: serial@91402000 {
>>> + compatible = "snps,dw-apb-uart";
>>> + reg = <0x0 0x91402000 0x0 0x1000>;
>>> + clocks = <&apb_clk>;
>>> + interrupts = <18 IRQ_TYPE_LEVEL_HIGH>;
>>> + reg-io-width = <4>;
>>> + reg-shift = <2>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + uart3: serial@91403000 {
>>> + compatible = "snps,dw-apb-uart";
>>> + reg = <0x0 0x91403000 0x0 0x1000>;
>>> + clocks = <&apb_clk>;
>>> + interrupts = <19 IRQ_TYPE_LEVEL_HIGH>;
>>> + reg-io-width = <4>;
>>> + reg-shift = <2>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + uart4: serial@91404000 {
>>> + compatible = "snps,dw-apb-uart";
>>> + reg = <0x0 0x91404000 0x0 0x1000>;
>>> + clocks = <&apb_clk>;
>>> + interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
>>> + reg-io-width = <4>;
>>> + reg-shift = <2>;
>>> + status = "disabled";
>>> + };
>>> + };
>>> +};



2024-03-05 17:19:04

by Yangyu Chen

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts



> On Mar 6, 2024, at 01:01, Conor Dooley <[email protected]> wrote:
>
> Hey,
>
> Just chiming so that things don't get misinterpreted.
>
> On Wed, Mar 06, 2024 at 12:37:16AM +0800, Yangyu Chen wrote:
>>> On Mar 6, 2024, at 00:22, Yangyu Chen <[email protected]> wrote:
>>>> On Mar 5, 2024, at 23:54, Palmer Dabbelt <[email protected]> wrote:
>>>> On Mon, 04 Mar 2024 13:06:00 PST (-0800), [email protected] wrote:
>>>>> - Svpbmt and T-Head MAEE both supported
>>>>>
>>>>> T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
>>>>> attributes and is controlled by csr.mxstatus. If the kernel wants to use
>>>>> svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
>>>>> before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
>>>>> on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
>>>>> lines modification that haven't been written back to the memory.
>>>>
>>>> So I guess we need the bootloader to just be accurate here? ie: whatever
>>>> extension it tells S-mode kernels is enabled is how S-mode behaves, and
>>>> then this should just work itself out.
>
> Correct, the bootloader/firmware "just" needs to write this bit to match
> what it passes to onwards in the devicetree.
>
>>> Yes. Currently, I have patched OpenSBI to disable MAEE. Conor Dooley said
>>> from a public irc group that he wants to control the enable of T-Head
>
> (#riscv on libera, the usual location)
>
> I also suggested that that, given we can use the standard extensions,
> we should use them instead of the custom extensions/errata.
>
>>> variation of zicbom and svpbmt from dts, in addition to mimplid or
>>> something now.
>
> Correct. I'm find with the impid == archid == 0 condition, given that's
> what we need to keep to avoid regressions, but if any future T-Head CPUs
> want to enable MAEE (ERRATA_THEAD_PBMT) or the custom CMOs
> (ERRATA_THEAD_CMO) these should be enabled from DT. Particularly when
> these CPUs can be configured to either use the T-Head versions or the
> standard extensions.
>
>>> I think that will be a better way for the bootloader to tell
>>> the kernel whether the T-Head MAEE should be enabled.
>
> You've got three options I guess. You could patch the DT in the bootloader,
> or use a fixed DT that matches the bootloader, or you could use the DT
> passed to the bootloader and parse the extensions to decide whether or not
> to enable MAEE or Svpbmt. Seems you're going for option 2.
>

The patched opensbi is only for work around now as the factory-provided
M-Mode uboot will enable MAEE. I would like option 1 to let the bootloader
choose what to use and patch the DT to tell the linux whether should use
T-Head MAEE as T-Head is likely to use this feature on TEE
(Trusted Execution Environment), so it might never be removed.

>>> Link: https://github.com/cyyself/opensbi/commit/b113c1c01d700314a4a696297ec09031a9399354
>>>
>>> Furthermore, I wonder whether a CPU node like this would be acceptable.
>>> I don't have any other details of how another CPU from K230 SoC works on
>>> Linux.
>
> A CPU node like what? It is not clear to me.

It in the k230.dtsi file. Only has big core there.


2024-03-05 17:52:02

by Yangyu Chen

[permalink] [raw]
Subject: Resend: Re: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230

The last email was not received in the list due to I didn’t change the mode
to plaintext. Sorry for that. This is a resend email.

> On Mar 6, 2024, at 01:20, Conor Dooley <[email protected]> wrote:
>
> On Tue, Mar 05, 2024 at 03:47:15PM +0800, Yangyu Chen wrote:
>> On 2024/3/5 07:46, Damien Le Moal wrote:
>>> On 3/5/24 06:05, Yangyu Chen wrote:
>>>> Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
>>>> Remove it depends on !MMU will allow building dts for K230 and remove the
>>>> K210 string from the help message.
>>>>
>>>> Signed-off-by: Yangyu Chen <[email protected]>
>>>> Reviewed-by: Conor Dooley <[email protected]>
>>>> ---
>>>> arch/riscv/Kconfig.socs | 5 ++---
>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>> index 623de5f8a208..b4e9b7f75510 100644
>>>> --- a/arch/riscv/Kconfig.socs
>>>> +++ b/arch/riscv/Kconfig.socs
>>>> @@ -75,13 +75,12 @@ config ARCH_CANAAN
>>>> def_bool SOC_CANAAN
>>>> config SOC_CANAAN
>>>> - bool "Canaan Kendryte K210 SoC"
>>>> - depends on !MMU
>>>
>>> This seems wrong to me. The k210 support does require no-mmu. So why remove
>>> this ?
>>
>> It just allows SOC_CANAAN to be selected when MMU=y. With this patch,
>> nommu_k210_defconfig still works.
>
> I think the concern here is that this would allow people to build a
> kernel for the k120 with the MMU enabled, not that the existing nommu
> build will be affected.
>

Aha. I don't think is there anyone will try a build with MMU on
K210. It only has 6MB of memory available to the CPU. For rv64 if
the S-Mode software starts at 2MB, it will only have 4MB for S-Mode
Linux. It's hard to trim the kernel and fit in 4MB and even preserve
some memory for userspace.

I also tried to make nommu_k210_defconfig on gcc 13.2.0, it produced a
1.6M kernel in arch/riscv/boot/Image. But if I use nconfig to set
CONFIG_MMU=y and rebuild, the kernel file will increase to 11M.

> Maybe you could squash in something like the following?
>
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index b4e9b7f75510..75d55059163f 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -72,15 +72,19 @@ config SOC_VIRT
> This enables support for QEMU Virt Machine.
>
> config ARCH_CANAAN
> - def_bool SOC_CANAAN
> + bool "Canaan Kendryte SoCs"
> + help
> + This enables support for Canaan Kendryte SoC platform hardware.
>
> config SOC_CANAAN
> - bool "Canaan Kendryte SoC"
> + bool "Canaan Kendryte K210 SoC"
> + depends on !MMU
> + depends on ARCH_CANAAN
> select CLINT_TIMER if RISCV_M_MODE
> select ARCH_HAS_RESET_CONTROLLER
> select PINCTRL
> select COMMON_CLK
> help
> - This enables support for Canaan Kendryte SoC platform hardware.
> + This enables support for Canaan Kendryte K210 SoC platform hardware.
>
> endmenu # "SoC selection"
>
> (Which reminds me, I really need to go and finish sorting out the ARCH_
> stuff)

Seems like an idea if we want this consideration. But I don't think we need
this as the opinions shown above.

If it should be, I think SOC_CANAAN should be renamed to SOC_K210 or
SOC_CANAAN_K210.


2024-03-15 08:19:14

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH v3 5/7] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230

On 3/14/24 01:56, Yangyu Chen wrote:
>> Hi,
>>
>> Thanks for the review comments. After thinking about it for a while,
>> I think we don't need to change it as we have changed the help
>> message which deleted the "K210". And the dts on k210.dtsi shows
>> mmu-type is riscv.none, I think if someone noticed this would know
>> why it fails to boot on the S-Mode MMU Kernel on K210. The only
>> special thing for ARCH_CANAAN is that a loader.bin will be built
>> when M-Mode is on arch/riscv/Makefile. However, Canaan has no other
>> M-Mode chips except for K210. So I think we don't need to change
>> it.

You completely lost me here. I do not understand what you are trying to say.

>> Another reason is that SOC_CANAAN for K210 is somehow hard to change.
>> If we continue using SOC_CANAAN for K210 but not for other Canaan
>> SoCs such as K230, it will cause some confusion to users. If we
>> rename SOC_CANAAN to SOC_CANAAN_K210, it will change many drivers
>> in many subsystems like my patch v5 [1]. So I don't think we need
>> to fix it.
>>
>>
>> If we don't change it, A concern for this is that some drivers for
>> K210 will be built when SOC_CANAAN=y and if we add this to defconfig,
>> all riscv builds will also build some K210 drivers even on MMU. But
>> I think this will not be a problem just need some memory/storage
>> for a slightly bigger kernel. Also, we will enable some new configs
>> in defconfig when a new soc gets supported, it's normal for K210
>> SoC drivers.
>>
>> Thus, I think we don't need to change it. If you have some other
>> opinions, please let me know.

1) Rename SOC_CANAAN to SOC_CANAAN_K210 and use that for any conditional code or
driver selection that is specific to the K210, which is what's done now.
2) Create a "new" SOC_CANAAN config and make SOC_CANAAN_K210 depend on it and on
!MMU

You could also add SOC_CANAAN_K230 if needed and make it depend on SOC_CANAAN &&
MMU.

With that, dirvers common to both the K210 and K230 can easilly be selected and
selecting SOC_CANAAN will end up either building for the K230 or the K210,
depending on MMU being set or not.

That's my 2 cents. Feel free to ignore. I am not involved that much with riscv
these days and I am far too busy with other kernel areas to be of any help. But
I really think that allowing building the K210 when MMU is enabled and "hoping
that the user understand his mistake" is not a great approach.

--
Damien Le Moal
Western Digital Research