Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement modularization of aclint #3330

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

PorterLu
Copy link

@PorterLu PorterLu commented Apr 24, 2023

Related issue: Implement modularization of aclint, which divide clint into mtimer and mswi.

Type of change: feature request

Impact: no functional change

Development Phase: implementation

Release Notes

This PR implements the modular features in ACLINT, dividing CLINT into MTIMER and MSWI. Users can use CLINTKey to generate an ACLINT. When isACLINT is false, ACLINT will be compatible with CLINT, otherwise it will not be compatible.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 24, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/ACLINT.scala Outdated Show resolved Hide resolved
@jerryz123
Copy link
Contributor

jerryz123 commented Apr 24, 2023

Can you summarize the changes and the justification in this PR thread?

Is it possible to add ACLINT as a separate peripheral device, and allow user to select between CLINT and ACLINT in the Config?

@ZenithalHourlyRate
Copy link
Contributor

Can you summarize the changes and the justification in this PR thread?

ACLINT is an "advanced" version of CLINT and can be made compatible with CLINT, and it has spec (though in draft state, and not updated for a year).

I've had some prior communication with the PR author. I intend to replace CLINT with ACLINT in all the rocket-chip codebase, and only leave CLINT.scala there for other projects using it.

For end users, that might be a breaking change as the device tree will change. We may mitigate this issue by adding a compatible string riscv,clint0 for aclint when the base addresses are placed the same as CLINT.

Is it possible to add ACLINT as a separate peripheral device, and allow user to select between CLINT and ACLINT in the Config?

Are there cases where people really need that specific CLINT RTL instead of a compatible ACLINT?

Another benefit of this is that SSWI can be implemented and will be implemented in another PR.

@jerryz123
Copy link
Contributor

Can you summarize the changes and the justification in this PR thread?

ACLINT is an "advanced" version of CLINT and can be made compatible with CLINT, and it has spec (though in draft state, and not updated for a year).

Is the spec frozen? How is software support for ACLINT?

I've had some prior communication with the PR author. I intend to replace CLINT with ACLINT in all the rocket-chip codebase, and only leave CLINT.scala there for other projects using it.

What is the justification for replacing CLINT with ACLINT in default designs?

For end users, that might be a breaking change as the device tree will change. We may mitigate this issue by adding a compatible string riscv,clint0 for aclint when the base addresses are placed the same as CLINT.

I prefer that we allow both a) using existing CLINT, with no changes to DTS, or b) using ACLINT, with non-backwards-compatible DTS.

Are there cases where people really need that specific CLINT RTL instead of a compatible ACLINT?

It depends on what software support for ACLINT looks like, and if users want to run legacy software that does not support ACLINT.

Is there any problem with both CanHavePeripheryCLINT and CanHavePeripheryACLINT, and disallow p(CLINTKey).isDefined && p(ACLINTKey).isDefined

@ZenithalHourlyRate
Copy link
Contributor

Is the spec frozen? How is software support for ACLINT?

It is not frozen, and unfortunately I do not know why. As for software support, OpenSBI has merged CLINT support to ACLINT

What is the justification for replacing CLINT with ACLINT in default designs?

ACLINT can be made fully compatible with CLINT, and that can be a default config for ACLINT.

It depends on what software support for ACLINT looks like, and if users want to run legacy software that does not support ACLINT.

legacy code can use ACLINT just like CLINT for the above default config.

Is there any problem with both CanHavePeripheryCLINT and CanHavePeripheryACLINT, and disallow p(CLINTKey).isDefined && p(ACLINTKey).isDefined

Technically no.

@jerryz123
Copy link
Contributor

Ok. If backwards compatibility can be done, then we can just preserve the old CanHavePeripheryCLINT and CLINTKey, and add flags to CLINTParams to enable non backwards compatible features.

Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we stick to 2-space indentation in all files?

@PorterLu
Copy link
Author

Yes, I will make modifications according to the discussion results.

@PorterLu
Copy link
Author

PorterLu commented Apr 25, 2023

I have encountered a problem. In order to achieve compatibility with CLINT for ACLINT, I placed the MTIMER device address in ACLINT at 0x02004000. However, in compatibility mode, the size here is 0x7ff8 (mtimecmp size)+8 (mtime size), and AddressSet (0x02004000, 0x8000-1) cannot be successfully compiled.

// AddressSets specify the address space managed by the manager
// Base is the base address, and mask are the bits consumed by the manager
// e.g: base=0x200, mask=0xff describes a device managing 0x200-0x2ff
// e.g: base=0x1000, mask=0xf0f decribes a device managing 0x1000-0x100f, 0x1100-0x110f, ...
case class AddressSet(base: BigInt, mask: BigInt) extends Ordered[AddressSet]
{
// Forbid misaligned base address (and empty sets)
require ((base & mask) == 0, s"Mis-aligned AddressSets are forbidden, got: ${this.toString}")

Here is my error message:

 Caused by: java.lang.IllegalArgumentException: requirement failed: Mis-aligned AddressSets are forbidden, got: AddressSet(0x2004000, 0x7fff)

It may be difficult to achieve backward compatibility here.

@jerryz123
Copy link
Contributor

What does the DTS look like when using CLINT compatibility mode?

@PorterLu
Copy link
Author

PorterLu commented Apr 26, 2023

Generated Address Map
	       0 -     1000 ARWX  debug-controller@0
	    3000 -     4000 ARWX  error-device@3000
	   10000 -    20000  R X  rom@10000
	 1ffc000 -  2000000 ARW   mswi@1ffc000
	 2000000 -  2008000 ARW   mtimer@2000000
	 c000000 - 10000000 ARW   interrupt-controller@c000000
	60000000 - 80000000  RWX  mmio-port-axi4@60000000
	80000000 - 90000000  RWXC memory@80000000

Like this.

@jerryz123
Copy link
Contributor

That's not the DTS

@jerryz123
Copy link
Contributor

DTS is the json-esque thing.

@PorterLu
Copy link
Author

PorterLu commented Apr 26, 2023

sorry.

/dts-v1/;

/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "freechips,rocketchip-unknown-dev";
	model = "freechips,rocketchip-unknown";
	L16: cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		timebase-frequency = <1000000>;
		L4: cpu@0 {
			clock-frequency = <0>;
			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 = <&L11>;
			reg = <0x0>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			timebase-frequency = <1000000>;
			tlb-split;
			L2: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
	};
	L11: memory@80000000 {
		device_type = "memory";
		reg = <0x80000000 0x10000000>;
	};
	L15: soc {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "freechips,rocketchip-unknown-soc", "simple-bus";
		ranges;
		L8: debug-controller@0 {
			compatible = "sifive,debug-013", "riscv,debug-013";
			debug-attach = "dmi";
			interrupts-extended = <&L2 65535>;
			reg = <0x0 0x1000>;
			reg-names = "control";
		};
		L1: error-device@3000 {
			compatible = "sifive,error0";
			reg = <0x3000 0x1000>;
		};
		L10: external-interrupts {
			interrupt-parent = <&L5>;
			interrupts = <1 2>;
		};
		L5: interrupt-controller@c000000 {
			#interrupt-cells = <1>;
			compatible = "riscv,plic0";
			interrupt-controller;
			interrupts-extended = <&L2 11 &L2 9>;
			reg = <0xc000000 0x4000000>;
			reg-names = "control";
			riscv,max-priority = <3>;
			riscv,ndev = <2>;
		};
		L12: mmio-port-axi4@60000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "simple-bus";
			ranges = <0x60000000 0x60000000 0x20000000>;
		};
		L7: mswi@1ffc000 {
			compatible = "riscv,aclint-mswi";
			interrupts-extended = <&L2 3>;
			reg = <0x1ffc000 0x4000>;
			reg-names = "control";
		};
		L6: mtimer@2000000 {
			compatible = "riscv,aclint-mtimer";
			interrupts-extended = <&L2 7>;
			reg = <0x2000000 0x8000>;
			reg-names = "control";
		};
		L13: rom@10000 {
			compatible = "sifive,rom0";
			reg = <0x10000 0x10000>;
			reg-names = "mem";
		};
		L0: subsystem_pbus_clock {
			#clock-cells = <0>;
			clock-frequency = <100000000>;
			clock-output-names = "subsystem_pbus_clock";
			compatible = "fixed-clock";
		};
	};
};

@jerryz123
Copy link
Contributor

In normal Clint mode, can we also make it generate the normal CLINT DTS?

I grow more convinced that we should just have separate CanHavePeripheryCLINT and CanHavePeripheryACLINT

@PorterLu
Copy link
Author

In normal Clint mode, dts must grow like this because of the alignment requirements of the AddressSet.

I grow more convinced that we should just have separate CanHavePeripheryCLINT and CanHavePeripheryACLINT

If that's the case, I need to write different code in HasRTC and HasTile based on ACLINT or CLINT, and I need to further investigate this part.

@PorterLu
Copy link
Author

PorterLu commented Apr 28, 2023

Related issue: Implement modularization of aclint, which divide clint into mtimer and mswi.

Type of change: feature request

Impact: no functional change

Development Phase: implementation

Release Notes:

  1. This PR implements the modular features in ACLINT, dividing CLINT into MTIMER and MSWI. Users can use CLINTKey to generate an ACLINT. When isACLINT is true, ACLINT will be generated, otherwise clint will be generated.

  2. If user decide to use clint, the clint base address will be the base address of MSWI device, which is just like normal clint.

  3. The reason why I still only use CanHavePeripheryCLINT is that using separate CanHavePeripheryCLINT and CanHavePeripheryACLINT require a lot of code modifications.

Here is the clint dts:

/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "freechips,rocketchip-unknown-dev";
	model = "freechips,rocketchip-unknown";
	L15: cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		timebase-frequency = <1000000>;
		L4: cpu@0 {
			clock-frequency = <0>;
			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 = <&L10>;
			reg = <0x0>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			timebase-frequency = <1000000>;
			tlb-split;
			L2: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
	};
	L10: memory@80000000 {
		device_type = "memory";
		reg = <0x80000000 0x10000000>;
	};
	L14: soc {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "freechips,rocketchip-unknown-soc", "simple-bus";
		ranges;
		L6: clint@2000000 {
			compatible = "riscv,clint0";
			interrupts-extended = <&L2 3 &L2 7>;
			reg = <0x2000000 0x10000>;
			reg-names = "control";
		};
		L7: debug-controller@0 {
			compatible = "sifive,debug-013", "riscv,debug-013";
			debug-attach = "dmi";
			interrupts-extended = <&L2 65535>;
			reg = <0x0 0x1000>;
			reg-names = "control";
		};
		L1: error-device@3000 {
			compatible = "sifive,error0";
			reg = <0x3000 0x1000>;
		};
		L9: external-interrupts {
			interrupt-parent = <&L5>;
			interrupts = <1 2>;
		};
		L5: interrupt-controller@c000000 {
			#interrupt-cells = <1>;
			compatible = "riscv,plic0";
			interrupt-controller;
			interrupts-extended = <&L2 11 &L2 9>;
			reg = <0xc000000 0x4000000>;
			reg-names = "control";
			riscv,max-priority = <3>;
			riscv,ndev = <2>;
		};
		L11: mmio-port-axi4@60000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "simple-bus";
			ranges = <0x60000000 0x60000000 0x20000000>;
		};
		L12: rom@10000 {
			compatible = "sifive,rom0";
			reg = <0x10000 0x10000>;
			reg-names = "mem";
		};
		L0: subsystem_pbus_clock {
			#clock-cells = <0>;
			clock-frequency = <100000000>;
			clock-output-names = "subsystem_pbus_clock";
			compatible = "fixed-clock";
		};
	};
};

@PorterLu
Copy link
Author

PorterLu commented Apr 28, 2023

Here is the aclint dts:

/dts-v1/;

/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "freechips,rocketchip-unknown-dev";
	model = "freechips,rocketchip-unknown";
	L16: cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		timebase-frequency = <1000000>;
		L4: cpu@0 {
			clock-frequency = <0>;
			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 = <&L11>;
			reg = <0x0>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			timebase-frequency = <1000000>;
			tlb-split;
			L2: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
	};
	L11: memory@80000000 {
		device_type = "memory";
		reg = <0x80000000 0x10000000>;
	};
	L15: soc {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "freechips,rocketchip-unknown-soc", "simple-bus";
		ranges;
		L8: debug-controller@0 {
			compatible = "sifive,debug-013", "riscv,debug-013";
			debug-attach = "dmi";
			interrupts-extended = <&L2 65535>;
			reg = <0x0 0x1000>;
			reg-names = "control";
		};
		L1: error-device@3000 {
			compatible = "sifive,error0";
			reg = <0x3000 0x1000>;
		};
		L10: external-interrupts {
			interrupt-parent = <&L5>;
			interrupts = <1 2>;
		};
		L5: interrupt-controller@c000000 {
			#interrupt-cells = <1>;
			compatible = "riscv,plic0";
			interrupt-controller;
			interrupts-extended = <&L2 11 &L2 9>;
			reg = <0xc000000 0x4000000>;
			reg-names = "control";
			riscv,max-priority = <3>;
			riscv,ndev = <2>;
		};
		L12: mmio-port-axi4@60000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "simple-bus";
			ranges = <0x60000000 0x60000000 0x20000000>;
		};
		L6: mswi@2000000 {
			compatible = "riscv,aclint-mswi";
			interrupts-extended = <&L2 3>;
			reg = <0x2000000 0x4000>;
			reg-names = "control";
		};
		L7: mtimer@2008000 {
			compatible = "riscv,aclint-mtimer";
			interrupts-extended = <&L2 7>;
			reg = <0x2008000 0x8100>;
			reg-names = "control";
		};
		L13: rom@10000 {
			compatible = "sifive,rom0";
			reg = <0x10000 0x10000>;
			reg-names = "mem";
		};
		L0: subsystem_pbus_clock {
			#clock-cells = <0>;
			clock-frequency = <100000000>;
			clock-output-names = "subsystem_pbus_clock";
			compatible = "fixed-clock";
		};
	};
};

@jerryz123
Copy link
Contributor

Thanks for making this change, the DTS for both cases look fine for me. I'll need to review more thoroughly when I have time, but I think fundamentally this change is OK

src/main/scala/devices/tilelink/CLINT.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/CLINT.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/CLINT.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/CLINT.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/subsystem/HasTiles.scala Outdated Show resolved Hide resolved
src/main/scala/subsystem/RTC.scala Outdated Show resolved Hide resolved
@PorterLu
Copy link
Author

PorterLu commented May 5, 2023

Here is the no-clint dts:

/dts-v1/;

/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "freechips,rocketchip-unknown-dev";
	model = "freechips,rocketchip-unknown";
	L14: cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		timebase-frequency = <1000000>;
		L4: cpu@0 {
			clock-frequency = <0>;
			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 = <&L9>;
			reg = <0x0>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			timebase-frequency = <1000000>;
			tlb-split;
			L2: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
	};
	L9: memory@80000000 {
		device_type = "memory";
		reg = <0x80000000 0x10000000>;
	};
	L13: soc {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "freechips,rocketchip-unknown-soc", "simple-bus";
		ranges;
		L6: debug-controller@0 {
			compatible = "sifive,debug-013", "riscv,debug-013";
			debug-attach = "dmi";
			interrupts-extended = <&L2 65535>;
			reg = <0x0 0x1000>;
			reg-names = "control";
		};
		L1: error-device@3000 {
			compatible = "sifive,error0";
			reg = <0x3000 0x1000>;
		};
		L8: external-interrupts {
			interrupt-parent = <&L5>;
			interrupts = <1 2>;
		};
		L5: interrupt-controller@c000000 {
			#interrupt-cells = <1>;
			compatible = "riscv,plic0";
			interrupt-controller;
			interrupts-extended = <&L2 11 &L2 9>;
			reg = <0xc000000 0x4000000>;
			reg-names = "control";
			riscv,max-priority = <3>;
			riscv,ndev = <2>;
		};
		L10: mmio-port-axi4@60000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "simple-bus";
			ranges = <0x60000000 0x60000000 0x20000000>;
		};
		L11: rom@10000 {
			compatible = "sifive,rom0";
			reg = <0x10000 0x10000>;
			reg-names = "mem";
		};
		L0: subsystem_pbus_clock {
			#clock-cells = <0>;
			clock-frequency = <100000000>;
			clock-output-names = "subsystem_pbus_clock";
			compatible = "fixed-clock";
		};
	};
};

@PorterLu PorterLu requested a review from jerryz123 May 5, 2023 06:18
@jerryz123
Copy link
Contributor

Please squash the github-reply-to-suggestion commits into one commit.
FYI, there is also an option to batch apply those changes

@PorterLu PorterLu force-pushed the aclint_dev branch 2 times, most recently from dcaf81a to 1558a88 Compare May 5, 2023 17:34
@PorterLu
Copy link
Author

PorterLu commented May 5, 2023

  1. I set beatBytes according bus type.
  2. I do not allow mtimerParams and mswiParams to be empty at same time.
mtimer: not None, mswi: not None
mtimer: None, mswi: not None
mtimer: not None, mswi: None

All above cases generate properly.
Here is the dts with mtimerParams:None:

/dts-v1/;

/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "freechips,rocketchip-unknown-dev";
	model = "freechips,rocketchip-unknown";
	L15: cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		timebase-frequency = <1000000>;
		L4: cpu@0 {
			clock-frequency = <0>;
			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 = <&L10>;
			reg = <0x0>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			timebase-frequency = <1000000>;
			tlb-split;
			L2: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
	};
	L10: memory@80000000 {
		device_type = "memory";
		reg = <0x80000000 0x10000000>;
	};
	L14: soc {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "freechips,rocketchip-unknown-soc", "simple-bus";
		ranges;
		L7: debug-controller@0 {
			compatible = "sifive,debug-013", "riscv,debug-013";
			debug-attach = "dmi";
			interrupts-extended = <&L2 65535>;
			reg = <0x0 0x1000>;
			reg-names = "control";
		};
		L1: error-device@3000 {
			compatible = "sifive,error0";
			reg = <0x3000 0x1000>;
		};
		L9: external-interrupts {
			interrupt-parent = <&L5>;
			interrupts = <1 2>;
		};
		L5: interrupt-controller@c000000 {
			#interrupt-cells = <1>;
			compatible = "riscv,plic0";
			interrupt-controller;
			interrupts-extended = <&L2 11 &L2 9>;
			reg = <0xc000000 0x4000000>;
			reg-names = "control";
			riscv,max-priority = <3>;
			riscv,ndev = <2>;
		};
		L11: mmio-port-axi4@60000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "simple-bus";
			ranges = <0x60000000 0x60000000 0x20000000>;
		};
		L6: mswi@2000000 {
			compatible = "riscv,aclint-mswi";
			interrupts-extended = <&L2 3>;
			reg = <0x2000000 0x4000>;
			reg-names = "control";
		};
		L12: rom@10000 {
			compatible = "sifive,rom0";
			reg = <0x10000 0x10000>;
			reg-names = "mem";
		};
		L0: subsystem_pbus_clock {
			#clock-cells = <0>;
			clock-frequency = <100000000>;
			clock-output-names = "subsystem_pbus_clock";
			compatible = "fixed-clock";
		};
	};
};

Here is the dts with mswiParams: None:

/dts-v1/;

/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "freechips,rocketchip-unknown-dev";
	model = "freechips,rocketchip-unknown";
	L15: cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		timebase-frequency = <1000000>;
		L4: cpu@0 {
			clock-frequency = <0>;
			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 = <&L10>;
			reg = <0x0>;
			riscv,isa = "rv64imafdcZicsr_Zifencei_Zihpm_Xrocket";
			riscv,pmpgranularity = <4>;
			riscv,pmpregions = <8>;
			status = "okay";
			timebase-frequency = <1000000>;
			tlb-split;
			L2: interrupt-controller {
				#interrupt-cells = <1>;
				compatible = "riscv,cpu-intc";
				interrupt-controller;
			};
		};
	};
	L10: memory@80000000 {
		device_type = "memory";
		reg = <0x80000000 0x10000000>;
	};
	L14: soc {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "freechips,rocketchip-unknown-soc", "simple-bus";
		ranges;
		L7: debug-controller@0 {
			compatible = "sifive,debug-013", "riscv,debug-013";
			debug-attach = "dmi";
			interrupts-extended = <&L2 65535>;
			reg = <0x0 0x1000>;
			reg-names = "control";
		};
		L1: error-device@3000 {
			compatible = "sifive,error0";
			reg = <0x3000 0x1000>;
		};
		L9: external-interrupts {
			interrupt-parent = <&L5>;
			interrupts = <1 2>;
		};
		L5: interrupt-controller@c000000 {
			#interrupt-cells = <1>;
			compatible = "riscv,plic0";
			interrupt-controller;
			interrupts-extended = <&L2 11 &L2 9>;
			reg = <0xc000000 0x4000000>;
			reg-names = "control";
			riscv,max-priority = <3>;
			riscv,ndev = <2>;
		};
		L11: mmio-port-axi4@60000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "simple-bus";
			ranges = <0x60000000 0x60000000 0x20000000>;
		};
		L6: mtimer@2008000 {
			compatible = "riscv,aclint-mtimer";
			interrupts-extended = <&L2 7>;
			reg = <0x2008000 0x8100>;
			reg-names = "control";
		};
		L12: rom@10000 {
			compatible = "sifive,rom0";
			reg = <0x10000 0x10000>;
			reg-names = "mem";
		};
		L0: subsystem_pbus_clock {
			#clock-cells = <0>;
			clock-frequency = <100000000>;
			clock-output-names = "subsystem_pbus_clock";
			compatible = "fixed-clock";
		};
	};
};

src/main/scala/devices/tilelink/CLINT.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/CLINT.scala Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/CLINT.scala Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
Copy link
Contributor

@ZenithalHourlyRate ZenithalHourlyRate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fairly happy with this implementation now. Comments left should still be addressed. I need some time to test it personally to see if it works, and I also need some time to investigate into the diplomacy issue proposed in private communication.

src/main/scala/devices/tilelink/MSWI.scala Outdated Show resolved Hide resolved
src/main/scala/devices/tilelink/Mtimer.scala Outdated Show resolved Hide resolved
ZenithalHourlyRate

This comment was marked as duplicate.

…d mswi.

Co-authored-by: Hongren (Zenithal) Zheng <[email protected]>
Co-authored-by: jerryz123 <[email protected]>
lbmeng added a commit to lbmeng/u-boot that referenced this pull request Jun 12, 2023
As the RISC-V ACLINT specification is defined to be backward compatible
with the SiFive CLINT specification, we rename SiFive CLINT to RISC-V
ALINT in the source tree to be future-proof.

Signed-off-by: Bin Meng <[email protected]>

Series-to: [email protected]
Cover-letter:
riscv: Add ACLINT mtimer and mswi devices support

This RISC-V ACLINT specification [1] defines a set of memory mapped
devices which provide inter-processor interrupts (IPI) and timer
functionalities for each HART on a multi-HART RISC-V platform.

This seriesl updates U-Boot existing SiFive CLINT driver to handle
the ACLINT changes, and is now able to support both CLINT and ACLINT.

With this series, U-Boot is able to boot on:

- QEMU 'virt' machine with 'aclint=on'
- Rocket Chip with ACLINT changes [2]

[1] https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
[2] chipsalliance/rocket-chip#3330
END
@ZenithalHourlyRate
Copy link
Contributor

@PorterLu, @cyyself, and we together have managed to boot Linux with OpenSBI and a patched u-boot (thanks to @lbmeng) on a dual-core Rocket with ACLINT of this branch enabled on a 7k325t FPGA. I think this suffices for testing.

As for the dts issue, due to diplomacy, what rocket will generate for aclint mtimer is

reg = <0x2010000 0x1000 0x2008000 0x8000>;

instead of the example in the RFC spec (https://lore.kernel.org/lkml/[email protected]/)

reg = <0x2010000 0x8>, <0x2008000 0x7ff8>;

though in practice OpenSBI and the patched u-boot worked happily.

This is ready for more review. And for my part, after some minor fixes (e.g. code style, a WithACLINT option, and a config for emulator in CI), I think this can be merged.

@ZenithalHourlyRate
Copy link
Contributor

As for the dts issue, due to diplomacy, what rocket will generate for aclint mtimer is

reg = <0x2010000 0x1000 0x2008000 0x8000>;

instead of the example in the RFC spec (https://lore.kernel.org/lkml/[email protected]/)

reg = <0x2010000 0x8>, <0x2008000 0x7ff8>;

though in practice OpenSBI and the patched u-boot worked happily.

dtc is not too happy

Warning (simple_bus_reg): /soc/mtimer@2008000: simple-bus unit address format error, expected "2010000"

for

        L7: mtimer@2008000 {
            compatible = "riscv,aclint-mtimer";
            interrupts-extended = <&L2 7>;
            reg = <0x2010000 0x1000 0x2008000 0x8000>;
            reg-names = "control";
        };

changing the node name to mtimer@2010000 would eliminate the warning. But the example in the RFC dt-binding does use mtimer@2004000 instead of mtimer@200bff8.

lbmeng added a commit to lbmeng/u-boot that referenced this pull request Jun 20, 2023
As the RISC-V ACLINT specification is defined to be backward compatible
with the SiFive CLINT specification, we rename SiFive CLINT to RISC-V
ALINT in the source tree to be future-proof.

Signed-off-by: Bin Meng <[email protected]>
Reviewed-by: Rick Chen <[email protected]>

Series-version: 2
Cover-letter:
riscv: Add ACLINT mtimer and mswi devices support
This RISC-V ACLINT specification [1] defines a set of memory mapped
devices which provide inter-processor interrupts (IPI) and timer
functionalities for each HART on a multi-HART RISC-V platform.

This seriesl updates U-Boot existing SiFive CLINT driver to handle
the ACLINT changes, and is now able to support both CLINT and ACLINT.

With this series, U-Boot is able to boot on:

- QEMU 'virt' machine with 'aclint=on'
- Rocket Chip with ACLINT changes [2]

[1] https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
[2] chipsalliance/rocket-chip#3330
END
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants