RISC-V support over CLINT #815

romancardenas · 2023-10-06T12:46:23Z

The Software-Level Interrupt Controller (SLIC) is a software interrupt controller that aims to mimic the PLIC but using a single interrupt source. It currently works for the CLINT peripheral, but I plan to add additional mechanisms so a broader amount of RISC-V targets can benefit from it.

This PR is a work-in-progress effort for adapting the SLIC to RTIC. I already modified some aspects:

New interrupt_mod codegen function.
New extra_modules codegen function.

But I'd like to do two additional major changes before this PR is accepted:

Remove core peripherals from rtic::export (I currently use a very dirty patch to avoid issues with my implementation)
Add a new backend field to the AppArgs struct that, depending on the selected backend, can potentially parse additional configuration parameters.

Additionally, as the SLIC is able to generate any number of synthetic interrupt sources, I would like to modify the current implementation so users won't need to define their dispatchers. Instead, the dispatchers will be automatically generated as needed by the application.

All the feedback is more than welcome, let me know what do you think about my proposals :)

romancardenas · 2023-10-06T14:41:36Z

I added backend-specific configuration arguments to the app macro. This solves #816

onsdagens

awesome, one small comment from a quick look

rtic-macros/src/codegen/pre_init.rs

romancardenas · 2023-10-09T08:01:21Z

I followed the peripherals macro argument approach and added a core macro argument. This argument defaults to true in order to be backwards compatible. However, when set to false, RTIC will remove the core peripherals from its internals.

In case of my RISC-V SLIC, this field is mandatory to be false (again, I still need a pre-processing step in which I can manipulate the app args before generating all the code).

With this approach, removing core peripherals from RTIC is very similar to #720.

Related issue: #746

romancardenas · 2023-10-09T09:44:41Z

I added the pre_init_preprocessing binding to allow backends to modify the App structure according to their needs before any other action is triggered.

You can see how the SLIC backend uses this binding here. In this case, I force the core argument to be false. I also check that the dispatchers are empty, because I later generate as much dispatchers as needed by the SLIC.

With all these changes I am happy with the final result. So it's time to discuss my proposal and fine-tune/reorder the code before merging it :) Let me know what you think.

romancardenas · 2024-01-02T13:52:17Z

Last week I've been working on integrating use cases of RTIC on a HiFive1 RISC-V microcontroller in the CI. I adapted the CI of the repo to test examples for a RISC-V target using my new backend. The main changes are explained below:

The rtic crate does not contain examples anymore.
Now, xtask accepts a new parameter platform to select which platform we want to test. By default, it is set to lm3s6965.
Each platform has custom rust flags and a default backend. Additionally, the features method allows us to define which backends are valid for every platform and, if needed, which features must be enabled when building their examples.
All the examples for a given platform are in the examples/<platform> crate. We can continue expanding the number of platforms.
For QEMU runs, the ci/expected/<platform> directory will contain a file with the expected outcome of a given example. xtask will then check that the execution matches the expected outcome.

About usage examples builds, I'm not sure if these are longer needed, as we can now treat them as regular examples. Of course, we now should adapt all the examples to be emulated in QEMU when possible. In this way, the CI will be enriched, as it does not limit to a single platform.

perlindgren · 2024-01-10T19:42:32Z

Small comment, RTIC typically should require no unsafe user code. For the Cortex and ESP32 targets setting up the interrups are done by the framework. This also ensures that the init is guaranteed to run to end in a "global critical section".

romancardenas · 2024-01-10T20:15:11Z

@perlindgren I'll take a loop to ESP's implementation to see where is the proper place to enable the interrupts right after the init code finishes

onsdagens · 2024-01-10T21:11:57Z

@perlindgren I'll take a loop to ESP's implementation to see where is the proper place to enable the interrupts right after the init code finishes

the global interrupt enable is already there here, so it's probably just a matter of removing it from the example. We were also discussing offline the premature interrupt enable being a possible cause of some concurrency issues leading to the CI weirdness.

the slic.set_interrupts() can probably be done anywhere in the pre_init, my reasoning here being it shouldn't have any side effects with the interrupts disabled

romancardenas · 2024-01-11T08:16:38Z

I fixed the unsafe code issue.

The weird thing is that the shared variable is correctly read and written from all the tasks but when it comes to idle, it reads the initial value as if the other tasks didn't run before

romancardenas · 2024-01-11T08:18:58Z

Hey wait! The issue is that idle locks the shared counter before the medium task preempts it! That makes more sense. I'll take a look to find the bug.

(still weird that CI fails depending on which host system is running QEMU)

romancardenas · 2024-01-11T10:00:02Z

QEMU version via apt is 6.2, while I'm using 8.2.0 on my machine. I'll try to build the latest version of QEMU from source and see if that was it.

Co-authored-by: Henrik Tjäder <henrik@tjaders.com>

romancardenas · 2024-02-28T09:33:50Z

closes #746, closes #816

AfoHT

This is looking great!

Love the cleanup of the examples structure, the platform makes it tidy and extensive!

Some comments and nitpicks here and there

Do you have a plan for riscv-slic crate? For long term I guess there will be some release we'll depend on, rather than the current git-rev dep :)

rtic/src/export/riscv_common.rs

xtask/src/argument_parsing.rs

AfoHT · 2024-03-06T18:12:13Z

xtask/src/main.rs

        }
        Commands::Book(args) => {
            info!("Running mdbook");
            cargo_book(globals, &args.arguments)
        }
+        // TODO these two commands are not needed anymore


Great! Then I think you should remove them together with the associated "usage" functions etc.! 👍

Done (maybe I forgot something, a second check would be very useful :D)

rtic-monotonics/CHANGELOG.md

AfoHT · 2024-03-06T18:46:29Z

rtic-macros/src/syntax/backend/cortex.rs

+            input.span(),
+            "cortex backend does not accept any arguments",


Discussion point: the features exposed are cortex-m-*, and some modules are cortex, while others are riscv_xyz.

I wonder if it makes sense to keep to cortex-m/cortex_m here too for consistency? That includes module name etc.

Good point. I guess that we should use cortex-m / cortex_m, as it follows the name of the architecture crate (for RISC-V, the crate is riscv).

rtic-macros/src/codegen/bindings/riscv_slic.rs

rtic-macros/CHANGELOG.md

examples/hifive1/examples/prio_inheritance.rs

AfoHT · 2024-03-06T19:13:44Z

examples/hifive1/Cargo.toml

+heapless = "0.8.0"
+hifive1 = { git = "https://github.com/romancardenas/hifive1.git", features = ["board-redv"] }
+e310x = { git = "https://github.com/greenlsi/e310x.git", features = ["rt"]}
+# riscv-rt = {path = "../../../riscv/riscv-rt", features = ["single-hart"]}


TODO before publish

The issue with the official e310x and hifive1 crates is that they do not follow the new riscv-peripheral crate fashion. It is something I have to start moving, but I'm now kind of busy with riscv-pac and adding support to vectored mode in riscv-rt

Once RTIC provides support to any RISC-V uC with a CLINT peripheral provided the PAC uses riscv-peripheral, maintainers will have a strong motivation to adopt the new crate 😁

I also plan to look at svd2rust and contribute there to ease the port task.

AfoHT · 2024-03-06T19:17:49Z

rtic/Cargo.toml

 esp32c3 = { version = "0.20.0", optional = true}
 riscv = {version = "0.11.0", optional = true}
 cortex-m = { version = "0.7.0", optional = true }
 bare-metal = "1.0.0"
-#portable-atomic = { version = "0.3.19" }
+# portable-atomic = { version = "0.3.19" }


Not used at all, do we need it? 🤔 @korken89

I think it is a "reminder" that we should move from atomic-polyfill to portable-atomic :D

romancardenas · 2024-03-07T09:40:15Z

Do you have a plan for riscv-slic crate? For long term I guess there will be some release we'll depend on, rather than the current git-rev dep :)

I will ship a 0.1.0 release as soon as I get your OK to merge the PR :) I'm still waiting in case we detect any deficiency during the review process

romancardenas · 2024-03-07T12:27:11Z

@AfoHT I addressed all your comments :)

AfoHT

Nice! 🚀

I think we can have this on master to really start playing with it.

@perlindgren you also had some devices to test on?

This was referenced Oct 6, 2023

(partially) removing core peripherals from RTIC context #746

Closed

rtic-macros: add backend-specific AppArgs #816

Closed

onsdagens reviewed Oct 8, 2023

View reviewed changes

rtic-macros/src/codegen/pre_init.rs Outdated Show resolved Hide resolved

romancardenas mentioned this pull request Dec 4, 2023

riscv-pac crate rust-embedded/riscv#162

Merged

romancardenas force-pushed the master branch 2 times, most recently from d1eb6be to fb20aee Compare December 31, 2023 16:48

romancardenas force-pushed the master branch 3 times, most recently from 4ee01cc to a47383b Compare January 10, 2024 20:06

romancardenas force-pushed the master branch from 17aae48 to 668c816 Compare January 11, 2024 09:58

romancardenas force-pushed the master branch 10 times, most recently from 8263e3e to 7139493 Compare January 11, 2024 10:47

romancardenas and others added 14 commits February 27, 2024 19:06

remove unsafe code in user app

53dc2f1

update dependencies

e3cfa2c

allow builds on riscv32imc

e88d40f

let's fix QEMU

3f9507c

Update .github/workflows/build.yml

68fa940

Co-authored-by: Henrik Tjäder <henrik@tjaders.com>

New build.rs

045563e

removing test features

84818d7

adapt ui test to new version of clippy

6eacc13

add more examples to RISC-V backend

b2428b8

proper configuration of heapless for riscv32imc

5aff1e4

opt-out examples for riscv32imc

bb17312

point to new version of riscv-slic

da078d8

adapt new macro bindings

dbcfe03

adapt examples and CI to stable

1debe31

romancardenas force-pushed the master branch from 334c0e9 to 1debe31 Compare February 27, 2024 18:42

fix cortex-m CI

27da4a2

AfoHT linked an issue Mar 6, 2024 that may be closed by this pull request

rtic-macros: add backend-specific AppArgs #816

Closed

AfoHT reviewed Mar 6, 2024

View reviewed changes

AfoHT mentioned this pull request Mar 6, 2024

Support for AVR boards such as the atmega32u4? #700

Closed

AfoHT linked an issue Mar 6, 2024 that may be closed by this pull request

(partially) removing core peripherals from RTIC context #746

Closed

romancardenas force-pushed the master branch from b2fa209 to 60fe87d Compare March 7, 2024 12:09

Review

97b434c

romancardenas force-pushed the master branch from 60fe87d to 97b434c Compare March 7, 2024 12:17

AfoHT approved these changes Mar 12, 2024

View reviewed changes

korken89 added this pull request to the merge queue Mar 20, 2024

Merged via the queue into rtic-rs:master with commit 4060c3d Mar 20, 2024
53 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RISC-V support over CLINT #815

RISC-V support over CLINT #815

romancardenas commented Oct 6, 2023

romancardenas commented Oct 6, 2023

onsdagens left a comment

romancardenas commented Oct 9, 2023

romancardenas commented Oct 9, 2023

romancardenas commented Jan 2, 2024

perlindgren commented Jan 10, 2024

romancardenas commented Jan 10, 2024

onsdagens commented Jan 10, 2024 •

edited

Loading

romancardenas commented Jan 11, 2024

romancardenas commented Jan 11, 2024 •

edited

Loading

romancardenas commented Jan 11, 2024

romancardenas commented Feb 28, 2024

AfoHT left a comment

AfoHT Mar 6, 2024

romancardenas Mar 7, 2024

AfoHT Mar 6, 2024

romancardenas Mar 7, 2024

AfoHT Mar 6, 2024

romancardenas Mar 7, 2024

romancardenas Mar 7, 2024

AfoHT Mar 6, 2024

romancardenas Mar 7, 2024

romancardenas commented Mar 7, 2024

romancardenas commented Mar 7, 2024

AfoHT left a comment

		input.span(),
		"cortex backend does not accept any arguments",

RISC-V support over CLINT #815

RISC-V support over CLINT #815

Conversation

romancardenas commented Oct 6, 2023

romancardenas commented Oct 6, 2023

onsdagens left a comment

Choose a reason for hiding this comment

romancardenas commented Oct 9, 2023

romancardenas commented Oct 9, 2023

romancardenas commented Jan 2, 2024

perlindgren commented Jan 10, 2024

romancardenas commented Jan 10, 2024

onsdagens commented Jan 10, 2024 • edited Loading

romancardenas commented Jan 11, 2024

romancardenas commented Jan 11, 2024 • edited Loading

romancardenas commented Jan 11, 2024

romancardenas commented Feb 28, 2024

AfoHT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romancardenas commented Mar 7, 2024

romancardenas commented Mar 7, 2024

AfoHT left a comment

Choose a reason for hiding this comment

onsdagens commented Jan 10, 2024 •

edited

Loading

romancardenas commented Jan 11, 2024 •

edited

Loading