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

platforms.yml: fix RISC-V march #385

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

platforms.yml: fix RISC-V march #385

wants to merge 1 commit into from

Conversation

Ivan-Velickovic
Copy link
Contributor

I think this is only used for the name of the CI job and nothing else. Now that the kernel defaults to FPU support on for RISC-V, we should probably change all of these to reflect that.

I think this is only used for the name of the CI job and nothing
else. Now that the kernel defaults to FPU support on for RISC-V,
we should probably change all of these to reflect that.

Signed-off-by: Ivan-Velickovic <[email protected]>
@Ivan-Velickovic Ivan-Velickovic requested a review from lsf37 as a code owner March 10, 2025 01:58
@lsf37
Copy link
Member

lsf37 commented Mar 10, 2025

It turns out there are lots of mentions of this still in various workflows (21 if I counted correctly). We need to update those simultaneously. Most of these are from invocations of the sel4test and sel4bench tests, which I could point to the factored out workflow in this repo now. I've trialled this in the seL4 repo and it has been working fairly well there.

The other ones are deployment workflows in seL4, sel4test, sel4bench, various mentions in the ci-action repo, and the simulation workflows for PRs. The simulation workflow also has a factored out version here, but the repos still need to be updated.

So overall, if everything is updated to point to the factored out workflows, it should be feasible to find all mentions.

Once that is done, we could remove march completely and either introduce some other ID that jobs are split by our do the matrix split differently.

@lsf37
Copy link
Member

lsf37 commented Mar 10, 2025

There are also still 3 mentions of rv64imac in various dts files in the seL4 repo:

tools/dts/star64.dts
201:			riscv,isa = "rv64imac";

tools/dts/hifive.dts
35:      riscv,isa = "rv64imac";

tools/dts/mpfs_icicle.dts
23:			riscv,isa = "rv64imac";

@lsf37
Copy link
Member

lsf37 commented Mar 10, 2025

Not really relevant for the setting here, but not clear to me if they need to be updated or not now that we have switched to a different default. We are still on rv64imac for verification, though.

@Ivan-Velickovic
Copy link
Contributor Author

There are also still 3 mentions of rv64imac in various dts files in the seL4 repo:

tools/dts/star64.dts
201:			riscv,isa = "rv64imac";

tools/dts/hifive.dts
35:      riscv,isa = "rv64imac";

tools/dts/mpfs_icicle.dts
23:			riscv,isa = "rv64imac";

Those are all referring to the ISA of an extra 'supervisor core' on those platforms from memory. seL4 does not run on those cores.

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.

2 participants