Help: commit 6112d58 breaks my kernel boot

Help: commit 6112d58 breaks my kernel boot

[Gabriel L. Somlo]

Hi,

I recently updated my opensbi sources, and noticed that commit
6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
setup. Without that commit, the kernel boots fine. With the commit
applied, I get:

...
 riscv: providing IPIs using SBI IPI extension
 rcu: srcu_init: Setting srcu_struct sizes based on contention.
 clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 7052723233920 ns
 sched_clock: 64 bits at 500kHz, resolution 2000ns, wraps every 4398046511000ns
 ------------[ cut here ]------------
 Missing cycle counter and fallback timer; RNG entropy collection will consequently suffer.
 WARNING: CPU: 0 PID: 0 at drivers/char/random.c:905 random_init+0xd0/0xf2
 CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.7.0-rc7-00022-ga42afb217b9c #103
 Hardware name: freechips,rocketchip-unknown (DT)
 epc : random_init+0xd0/0xf2
  ra : random_init+0xd0/0xf2
 epc : ffffffff8061b364 ra : ffffffff8061b364 sp : ffffffff80e03e40
  gp : ffffffff80ebaac0 tp : ffffffff80e0aa40 t0 : ffffffff80e137a8
  t1 : 000000000000002d t2 : 2d2d2d2d2d2d2d2d s0 : ffffffff80e03e80
  s1 : ffffffff80ebc320 a0 : 000000000000005a a1 : ffffffff80e82678
  a2 : 0000000000000010 a3 : fffffffffffffffe a4 : 0000000000000000
  a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000
  s2 : ffffffff80b0ca70 s3 : ffffffd81f7f5640 s4 : 0000000000000000
  s5 : ffffffff80ebb018 s6 : ffffffff80800018 s7 : 000000000000007f
  s8 : 0000000000002000 s9 : 00000000800436f0 s10: 0000000000000000
  s11: 0000000000000000 t3 : ffffffff80ecab67 t4 : ffffffff80ecab67
  t5 : ffffffff80ecab68 t6 : ffffffff80e03c58
 status: 0000000200000100 badaddr: ffffffff8061b364 cause: 0000000000000003
 [<ffffffff8061b364>] random_init+0xd0/0xf2
 [<ffffffff80600bb6>] start_kernel+0x45c/0x626
 ---[ end trace 0000000000000000 ]---

My DTS is enclosed in case it helps pinpoint what I might need to
update, or as a way to figure out whether commit 6112d58 is missing
some otherwise valid corner case.

Any ideas re. what I'm missing, what else about my setup I should
include to help you help me troubleshoot this, etc. -- much
appreciated!

Thanks,
--Gabriel


/dts-v1/;

/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "freechips,rocketchip-unknown-dev";
	model = "freechips,rocketchip-unknown";
	chosen {
		bootargs = "console=liteuart earlycon=liteuart,0x12009000 swiotlb=noforce rootwait root=/dev/ram0";
		linux,initrd-start = <0x82000000>;
		linux,initrd-end   = <0x820BA974>; /* end initrd.gz + 12814 (?) bytes */
	};
	L13: cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		timebase-frequency = <500000>;
		L6: cpu at 0 {
			clock-frequency = <50000000>;
			compatible = "sifive,rocket0", "riscv";
			d-cache-block-size = <64>;
			d-cache-sets = <64>;
			d-cache-size = <16384>;
			d-tlb-sets = <1>;
			d-tlb-size = <32>;
			device_type = "cpu";
			hardware-exec-breakpoint-count = <1>;
			i-cache-block-size = <64>;
			i-cache-sets = <64>;
			i-cache-size = <16384>;
			i-tlb-sets = <1>;
			i-tlb-size = <32>;
			mmu-type = "riscv,sv39";
			next-level-cache = <&L8>;
			reg = <0x0>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			tlb-split;
			L4: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
		L16: cpu at 1 {
			clock-frequency = <50000000>;
			compatible = "sifive,rocket0", "riscv";
			d-cache-block-size = <64>;
			d-cache-sets = <64>;
			d-cache-size = <16384>;
			d-tlb-sets = <1>;
			d-tlb-size = <32>;
			device_type = "cpu";
			hardware-exec-breakpoint-count = <1>;
			i-cache-block-size = <64>;
			i-cache-sets = <64>;
			i-cache-size = <16384>;
			i-tlb-sets = <1>;
			i-tlb-size = <32>;
			mmu-type = "riscv,sv39";
			next-level-cache = <&L8>;
			reg = <0x1>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			tlb-split;
			L14: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
		L26: cpu at 2 {
			clock-frequency = <50000000>;
			compatible = "sifive,rocket0", "riscv";
			d-cache-block-size = <64>;
			d-cache-sets = <64>;
			d-cache-size = <16384>;
			d-tlb-sets = <1>;
			d-tlb-size = <32>;
			device_type = "cpu";
			hardware-exec-breakpoint-count = <1>;
			i-cache-block-size = <64>;
			i-cache-sets = <64>;
			i-cache-size = <16384>;
			i-tlb-sets = <1>;
			i-tlb-size = <32>;
			mmu-type = "riscv,sv39";
			next-level-cache = <&L8>;
			reg = <0x2>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			tlb-split;
			L24: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
		L36: cpu at 3 {
			clock-frequency = <50000000>;
			compatible = "sifive,rocket0", "riscv";
			d-cache-block-size = <64>;
			d-cache-sets = <64>;
			d-cache-size = <16384>;
			d-tlb-sets = <1>;
			d-tlb-size = <32>;
			device_type = "cpu";
			hardware-exec-breakpoint-count = <1>;
			i-cache-block-size = <64>;
			i-cache-sets = <64>;
			i-cache-size = <16384>;
			i-tlb-sets = <1>;
			i-tlb-size = <32>;
			mmu-type = "riscv,sv39";
			next-level-cache = <&L8>;
			reg = <0x3>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			tlb-split;
			L34: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
	};
	L8: memory at 80000000 {
		device_type = "memory";
		reg = <0x80000000 0x20000000>; /* 512MB (ecpix5, nexys_video) */
	};
	clocks {
		sys_clk: litex_sys_clk {
			#clock-cells = <0>;
			compatible = "fixed-clock";
			clock-frequency = <50000000>;
		};
	};
	vreg_mmc: vreg_mmc {
		compatible = "regulator-fixed";
		regulator-name = "vreg_mmc";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		regulator-always-on;
	};
	L12: soc {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "freechips,rocketchip-unknown-soc", "simple-bus";
		ranges;
		L2: clint at 2000000 {
			compatible = "riscv,clint0";
			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
			reg = <0x2000000 0x10000>;
			reg-names = "control";
		};
		L3: debug-controller at 0 {
			compatible = "sifive,debug-013", "riscv,debug-013";
			//interrupts-extended = <&L4 0x3F &L14 0x3F &L24 0x3F &L34 0x3F>;
			//FIXME: had to reduce this to 0x3F before, now there
			//are no more kernel errors upon initialization, should
			//I just go with the chisel-elaborated defaults (below)?
			interrupts-extended = <&L4 0xFFFF &L14 0xFFFF &L24 0xFFFF &L34 0xFFFF>;
			reg = <0x0 0x1000>;
			reg-names = "control";
		};
		L0: error-device at 3000 {
			compatible = "sifive,error0";
			reg = <0x3000 0x1000>;
		};
		L7: external-interrupts {
			interrupt-parent = <&L1>;
			interrupts = <1 2 3 4 5 6 7 8>;
		};
		L1: interrupt-controller at c000000 {
			#interrupt-cells = <1>;
			compatible = "riscv,plic0";
			interrupt-controller;
			interrupts-extended = <&L4 11 &L4 9 &L14 11 &L14 9 &L24 11 &L24 9 &L34 11 &L34 9>;
			reg = <0xc000000 0x4000000>;
			reg-names = "control";
			riscv,max-priority = <7>;
			riscv,ndev = <8>;
		};
		L10: rom at 10000 {
			compatible = "sifive,rom0";
			reg = <0x10000 0x10000>;
			reg-names = "mem";
		};
		soc_ctrl0: soc_controller at 12000000 {
			compatible = "litex,soc-controller";
			reg = <0x12000000 0xc>;
		};
		liteuart0: serial at 12009000 {
			compatible = "litex,liteuart";
			reg = <0x12009000 0x100>;
			interrupt-parent = <&L1>;
			interrupts = <1>;
		};
		mac0: mac at 12001000 {
			compatible = "litex,liteeth";
			reg = <0x12001000 0x100>,
				<0x12001800 0x100>,
				<0x30000000 0x2000>;
			reg-names = "mac", "mdio", "buffer";
			litex,rx-slots = <2>;
			litex,tx-slots = <2>;
			litex,slot-size = <0x800>;
			interrupt-parent = <&L1>;
			interrupts = <3>;
		};
		mmc0: mmc at 12007800 {
			compatible = "litex,mmc";
			reg = <0x12007800 0x100>,
				<0x12006000 0x100>,
				<0x12005800 0x100>,
				<0x12007000 0x100>,
				<0x12006800 0x100>;
			reg-names = "phy", "core", "reader", "writer", "irq";
			clocks = <&sys_clk>;
			vmmc-supply = <&vreg_mmc>;
			interrupt-parent = <&L1>;
			interrupts = <5>;
		};
		litesata0: litesata at 12003000 {
			compatible = "litex,litesata";
			reg = <0x12003000 0x100>,
				<0x12004800 0x100>,
				<0x12005000 0x100>,
				<0x12004000 0x100>,
				<0x12003800 0x100>;
			reg-names = "ident", "phy", "reader", "writer", "irq";
			interrupt-parent = <&L1>;
			interrupts = <4>;
		};
		timer at 12008800 {
			compatible = "litex,timer";
			reg = <0x12008800 0x20>;
			clocks = <&sys_clk>;
			interrupt-parent = <&L1>;
			interrupts = <2>;
			//litex,width = <32>;
		};
	};
};

Help: commit 6112d58 breaks my kernel boot

[Conor Dooley]

On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> Hi,
> 
> I recently updated my opensbi sources, and noticed that commit
> 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> setup. Without that commit, the kernel boots fine. With the commit
> applied, I get:

> 		L2: clint at 2000000 {
> 			compatible = "riscv,clint0";
> 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> 			reg = <0x2000000 0x10000>;

> 			reg-names = "control";

If you remove this does it boot?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20231226/e20b3ed6/attachment.sig>

Help: commit 6112d58 breaks my kernel boot

[Gabriel L. Somlo]

On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
> On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> > Hi,
> > 
> > I recently updated my opensbi sources, and noticed that commit
> > 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> > breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> > setup. Without that commit, the kernel boots fine. With the commit
> > applied, I get:
> 
> > 		L2: clint at 2000000 {
> > 			compatible = "riscv,clint0";
> > 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> > 			reg = <0x2000000 0x10000>;
> 
> > 			reg-names = "control";
> 
> If you remove this does it boot?
> 

Yes, removing the `reg-names` line from the clint node allows the
kernel to boot without issues.

Thanks,
--Gabriel

Help: commit 6112d58 breaks my kernel boot

[Conor Dooley]

On Tue, Dec 26, 2023 at 12:19:20PM -0500, Gabriel L. Somlo wrote:
> On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
> > On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> > > Hi,
> > > 
> > > I recently updated my opensbi sources, and noticed that commit
> > > 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> > > breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> > > setup. Without that commit, the kernel boots fine. With the commit
> > > applied, I get:
> > 
> > > 		L2: clint at 2000000 {
> > > 			compatible = "riscv,clint0";
> > > 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> > > 			reg = <0x2000000 0x10000>;
> > 
> > > 			reg-names = "control";
> > 
> > If you remove this does it boot?
> > 
> 
> Yes, removing the `reg-names` line from the clint node allows the
> kernel to boot without issues.

Cool, sounds like a dts issue and not U-Boot's fault then. reg-names is
not a documented property for the regular clint, so it shouldn't be
here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20231226/4c42f53d/attachment.sig>

Help: commit 6112d58 breaks my kernel boot

[Gabriel L. Somlo]

On Tue, Dec 26, 2023 at 12:19:22PM -0500, Gabriel L. Somlo wrote:
> On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
> > On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> > > Hi,
> > > 
> > > I recently updated my opensbi sources, and noticed that commit
> > > 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> > > breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> > > setup. Without that commit, the kernel boots fine. With the commit
> > > applied, I get:
> > 
> > > 		L2: clint at 2000000 {
> > > 			compatible = "riscv,clint0";
> > > 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> > > 			reg = <0x2000000 0x10000>;
> > 
> > > 			reg-names = "control";
> > 
> > If you remove this does it boot?
> > 
> 
> Yes, removing the `reg-names` line from the clint node allows the
> kernel to boot without issues.

I have to wonder, though, is that the "proper" fix, or just a
temporary workaround? Is the `reg-names` entry actively "wrong"
in the context of the rest of my clint node, or is parsing in opensbi
actually buggy at the moment?

Thanks,
--G

Help: commit 6112d58 breaks my kernel boot

[Gabriel L. Somlo]

On Tue, Dec 26, 2023 at 05:27:55PM +0000, Conor Dooley wrote:
> On Tue, Dec 26, 2023 at 12:19:20PM -0500, Gabriel L. Somlo wrote:
> > On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
> > > On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> > > > Hi,
> > > > 
> > > > I recently updated my opensbi sources, and noticed that commit
> > > > 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> > > > breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> > > > setup. Without that commit, the kernel boots fine. With the commit
> > > > applied, I get:
> > > 
> > > > 		L2: clint at 2000000 {
> > > > 			compatible = "riscv,clint0";
> > > > 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> > > > 			reg = <0x2000000 0x10000>;
> > > 
> > > > 			reg-names = "control";
> > > 
> > > If you remove this does it boot?
> > > 
> > 
> > Yes, removing the `reg-names` line from the clint node allows the
> > kernel to boot without issues.

> reg-names is not a documented property for the regular clint,
> so it shouldn't be here.

OK, thanks for pointing that out. `reg-names` is generated by
Rocketchip during chisel-to-verilog elaboration, so I never questioned
whether it actually belonged there or not... :)

Thanks,
--G

Help: commit 6112d58 breaks my kernel boot

[Conor Dooley]

On Tue, Dec 26, 2023 at 12:34:59PM -0500, Gabriel L. Somlo wrote:
> On Tue, Dec 26, 2023 at 05:27:55PM +0000, Conor Dooley wrote:
> > On Tue, Dec 26, 2023 at 12:19:20PM -0500, Gabriel L. Somlo wrote:
> > > On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
> > > > On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> > > > > Hi,
> > > > > 
> > > > > I recently updated my opensbi sources, and noticed that commit
> > > > > 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> > > > > breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> > > > > setup. Without that commit, the kernel boots fine. With the commit
> > > > > applied, I get:
> > > > 
> > > > > 		L2: clint at 2000000 {
> > > > > 			compatible = "riscv,clint0";
> > > > > 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> > > > > 			reg = <0x2000000 0x10000>;
> > > > 
> > > > > 			reg-names = "control";
> > > > 
> > > > If you remove this does it boot?
> > > > 
> > > 
> > > Yes, removing the `reg-names` line from the clint node allows the
> > > kernel to boot without issues.
> 
> > reg-names is not a documented property for the regular clint,
> > so it shouldn't be here.
> 
> OK, thanks for pointing that out. `reg-names` is generated by
> Rocketchip during chisel-to-verilog elaboration, so I never questioned
> whether it actually belonged there or not... :)

Sounds like a bug in rocketchip to me :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20231226/4567a82d/attachment.sig>

Help: commit 6112d58 breaks my kernel boot

[Conor Dooley]

On Tue, Dec 26, 2023 at 12:28:52PM -0500, Gabriel L. Somlo wrote:
> On Tue, Dec 26, 2023 at 12:19:22PM -0500, Gabriel L. Somlo wrote:
> > On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
> > > On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> > > > Hi,
> > > > 
> > > > I recently updated my opensbi sources, and noticed that commit
> > > > 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> > > > breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> > > > setup. Without that commit, the kernel boots fine. With the commit
> > > > applied, I get:
> > > 
> > > > 		L2: clint at 2000000 {
> > > > 			compatible = "riscv,clint0";
> > > > 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> > > > 			reg = <0x2000000 0x10000>;
> > > 
> > > > 			reg-names = "control";
> > > 
> > > If you remove this does it boot?
> > > 
> > 
> > Yes, removing the `reg-names` line from the clint node allows the
> > kernel to boot without issues.
> 
> I have to wonder, though, is that the "proper" fix, or just a
> temporary workaround? Is the `reg-names` entry actively "wrong"
> in the context of the rest of my clint node, or is parsing in opensbi
> actually buggy at the moment?

The parsing in OpenSBI is binding compliant (as far as I can see from a
brief look at the commit you identified) but I suppose it could be
limited to avoid checking reg-names for things matching the regular clint,
if there are a lot of incorrect devicetrees out there likely to be
affected.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/opensbi/attachments/20231226/07425282/attachment.sig>

Help: commit 6112d58 breaks my kernel boot

[Gabriel Somlo]

On Tue, Dec 26, 2023 at 09:56:56PM +0000, Conor Dooley wrote:
> On Tue, Dec 26, 2023 at 12:28:52PM -0500, Gabriel L. Somlo wrote:
> > On Tue, Dec 26, 2023 at 12:19:22PM -0500, Gabriel L. Somlo wrote:
> > > On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
> > > > On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
> > > > > Hi,
> > > > > 
> > > > > I recently updated my opensbi sources, and noticed that commit
> > > > > 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
> > > > > breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
> > > > > setup. Without that commit, the kernel boots fine. With the commit
> > > > > applied, I get:
> > > > 
> > > > > 		L2: clint at 2000000 {
> > > > > 			compatible = "riscv,clint0";
> > > > > 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
> > > > > 			reg = <0x2000000 0x10000>;
> > > > 
> > > > > 			reg-names = "control";
> > > > 
> > > > If you remove this does it boot?
> > > > 
> > > 
> > > Yes, removing the `reg-names` line from the clint node allows the
> > > kernel to boot without issues.
> > 
> > I have to wonder, though, is that the "proper" fix, or just a
> > temporary workaround? Is the `reg-names` entry actively "wrong"
> > in the context of the rest of my clint node, or is parsing in opensbi
> > actually buggy at the moment?
> 
> The parsing in OpenSBI is binding compliant (as far as I can see from a
> brief look at the commit you identified) but I suppose it could be
> limited to avoid checking reg-names for things matching the regular clint,
> if there are a lot of incorrect devicetrees out there likely to be
> affected.

As luck would have it, mine is a soft-core fpga-based cpu setup, and
the Rocket-provided dts is a sample that can be (and is) edited before
deployment -- so I can easily comment out or remove the `reg-names`
line in addition to all the other "massaging" I do to the sample they
provide. In some plausible future I might even figure out how they
generate the dts and submit a patch to stop them from adding the offending
line in the first place.

It would be a problem for any actual hardware that ships with a "bad"
DTS in ROM -- but that's for someone else to complain about... ;)

Thanks again,
--Gabriel

Help: commit 6112d58 breaks my kernel boot

[Inochi Amaoto]

>On Tue, Dec 26, 2023 at 09:56:56PM +0000, Conor Dooley wrote:
>> On Tue, Dec 26, 2023 at 12:28:52PM -0500, Gabriel L. Somlo wrote:
>>> On Tue, Dec 26, 2023 at 12:19:22PM -0500, Gabriel L. Somlo wrote:
>>>> On Tue, Dec 26, 2023 at 05:11:00PM +0000, Conor Dooley wrote:
>>>>> On Tue, Dec 26, 2023 at 11:59:41AM -0500, Gabriel L. Somlo wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I recently updated my opensbi sources, and noticed that commit
>>>>>> 6112d58 ("lib: utils/fdt: Allow to use reg-names when parsing ACLINT")
>>>>>> breaks booting the (latest upstream) kernel on my LiteX + 4-core Rocket
>>>>>> setup. Without that commit, the kernel boots fine. With the commit
>>>>>> applied, I get:
>>>>>
>>>>>> 		L2: clint at 2000000 {
>>>>>> 			compatible = "riscv,clint0";
>>>>>> 			interrupts-extended = <&L4 3 &L4 7 &L14 3 &L14 7 &L24 3 &L24 7 &L34 3 &L34 7>;
>>>>>> 			reg = <0x2000000 0x10000>;
>>>>>
>>>>>> 			reg-names = "control";
>>>>>
>>>>> If you remove this does it boot?
>>>>>
>>>>
>>>> Yes, removing the `reg-names` line from the clint node allows the
>>>> kernel to boot without issues.
>>>
>>> I have to wonder, though, is that the "proper" fix, or just a
>>> temporary workaround? Is the `reg-names` entry actively "wrong"
>>> in the context of the rest of my clint node, or is parsing in opensbi
>>> actually buggy at the moment?
>>
>> The parsing in OpenSBI is binding compliant (as far as I can see from a
>> brief look at the commit you identified) but I suppose it could be
>> limited to avoid checking reg-names for things matching the regular clint,
>> if there are a lot of incorrect devicetrees out there likely to be
>> affected.
>
>As luck would have it, mine is a soft-core fpga-based cpu setup, and
>the Rocket-provided dts is a sample that can be (and is) edited before
>deployment -- so I can easily comment out or remove the `reg-names`
>line in addition to all the other "massaging" I do to the sample they
>provide. In some plausible future I might even figure out how they
>generate the dts and submit a patch to stop them from adding the offending
>line in the first place.
>
>It would be a problem for any actual hardware that ships with a "bad"
>DTS in ROM -- but that's for someone else to complain about... ?
>
>Thanks again,
>--Gabriel
>

Hi Gabriel,

I have already submit a new patch to restrict the use of regname to aclint
only. Can you test that patch? It should solve your problem.

Patch Link:
https://lists.infradead.org/pipermail/opensbi/2023-December/006191.html

Regards,
Inochi.