Skip to content

soc: add OpenHW Group CVA6 SoC #77732

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

Merged

Conversation

WorldofJARcraft
Copy link
Contributor

Adds support for the CVA6 family of RISC-V CPUs.
CVA6 is commonly found as a soft core CPU on FPGA designs. Different configurations and instruction set extensions can be configured, and different SoCs targeting various FPGA boards are available.
This commit adds support for the 64-bit configuration of CVA6, as well as two slightly different SoCs (the main difference being the Ethernet subsystem).
The configuration can also optionally target the hardware simulation environment (test harness) of the CVA6 project and can indicate test/failure to the environment.

@WorldofJARcraft
Copy link
Contributor Author

Full support for this SoC depends on drivers for the Ethernet subsystem: #73986, #73982, #73926

@@ -0,0 +1,5 @@
# Copyright 2024 CISPA Helmholtz Center for Information Security gGmbH
# SPDX-License-Identifier: Apache-2.0
config BOARD_CV32A6_ARTY_A7_100
Copy link
Collaborator

Choose a reason for hiding this comment

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

split this into commits, not going to review 77 files spanning multiple boards, multiple socs and even samples in a single commits

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the boards and samples from this PR and created new PRs:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Have this PR have a soc (in one commit), drivers (each in different commits) and one board (in another commit), then once that is merged you can add additional boards in a new PR each with their own commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Ethernet subsystem is not specific to this hardware (e.g., it is also used on ARM boards), so I will leave it as separate PRs.
This PR now has one commit with the SoC and one with a board in 32 and 64 bit variants.

@kartben
Copy link
Collaborator

kartben commented Nov 6, 2024

@WorldofJARcraft please don't forget that new boards must come with associated documentation. See https://docs.zephyrproject.org/latest/hardware/porting/board_porting.html#contributing-your-board

@WorldofJARcraft
Copy link
Contributor Author

@WorldofJARcraft please don't forget that new boards must come with associated documentation. See https://docs.zephyrproject.org/latest/hardware/porting/board_porting.html#contributing-your-board

Thank you for the reminder, I will finalize my changes and add the documentation before un-drafting the PR.

@WorldofJARcraft WorldofJARcraft force-pushed the openhwgroup-cv64a6-soc branch 3 times, most recently from 125e01c to 5cd33ae Compare November 7, 2024 08:11
@DREXX-lab DREXX-lab mentioned this pull request Nov 7, 2024
Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

apply comments throughout, they have only been added to the first instance

@@ -0,0 +1,30 @@
// SPDX-License-Identifier: Apache-2.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

/* comment style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

status = "okay";
compatible = "riscv";
riscv,isa = "rv32ima";

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

};
};
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ranges;

memory0:memory@80000000 {
device_type = "memory";
Copy link
Collaborator

Choose a reason for hiding this comment

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

tab indents

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +18 to +22
default "cv32a6" if SOC_SERIES_CV32A6

config SOC
default "cv32a6" if SOC_CV32A6
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you really need a soc series and a soc if they are the same value? If not, remove the soc series from Kconfig and soc.yml and just have the soc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are several slightly different versions of the CVA6 32-bit SOC: with and without MMU, with and without FPU, and a RV32E version is currently being developed. I have added the least common denominator here, future PRs can add the other variants as soon as they are finalized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the RV32I SoCs work with the config I provided, but the RV32E really needs to be separate (it has fewer registers).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are they going to be under the CV32A6 soc series or something else? Because if it's going to be just a 1:1 mapping of SoC to SoC series, the series can go, it's not required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least the embedded version will be - it has a near-identical configuration, but with the RV32E instead of the RV32I instruction set. There is also an ASIC version of the 32-bit SoC (the CV32A65X by Thales), which has some additional instruction set extensions enabled.

# SPDX-License-Identifier: Apache-2.0
# RISCV64 OpenHW Group cva6 configuration options

config SOC_CV64A6_IMAFDC
Copy link
Collaborator

Choose a reason for hiding this comment

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

have a config SOC_CV64A6 symbol which selects common this, then you can select the unique ones in the separate Kconfigs. And select the common Kconfig from Kconfig.soc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

default "cv64a6" if SOC_CV64A6_IMAFDC
default "cv64a6" if SOC_CV64A6_IMAC
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default "cv64a6" if SOC_CV64A6_IMAFDC
default "cv64a6" if SOC_CV64A6_IMAC
default "cv64a6" if SOC_CV64A6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


#include <stdint.h>

void z_cva6_finish_test(const int32_t status);
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the "board" configurations from the other PRs describes a verification harness in which the CVA6 CPU is run in verilator (a HDL simulator). This is used to run test programs in the simulator and check whether the CPU behaves as expected. To this end, the simulator provides an interface that allows completing the test case successfully or with an error code. This is exposed via this SoC-specific function.
The samples I added in #81017 show how this can be used in software.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a comment that explains what the function does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove this functionality from this PR and re-introduce it with the board that actually uses this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

From a brief look at this, it does seem to me that this more likely belongs with the board.

Only exception I can see is if any board using the cva6 SoC in one way or another can run with the verilator, which again means that all boards needs this function.
The next question though is, is a cva6.h file best place for simulator related functions ?

Would be nice if @WorldofJARcraft can give a more thoughouh explanation of the feature and any considerations of where this function could have been declared instead, and why this was not chosen.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not seeing any response from @WorldofJARcraft here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed this feature from the current PR.
I will re-introduce it in a separate commit when I add the board (actually a simulation environment, no physical board) that actually uses this.
In case the board is rejected, the feature is not really useful.
Please note that I also removed the poweroff functionality, as this is also only supported in the simulation environment currently.


#include "cva6.dtsi"

// minimal configuration of CVA6 32-bit CPUs: no instruction set extensions,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Capitalise first letter of comments i.e. English

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@WorldofJARcraft
Copy link
Contributor Author

I have incorporated the requested changes and re-written the commits.

kartben
kartben previously approved these changes Mar 27, 2025
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

+1 for docs (I pushed a few fixes that were easier to directly tackle rather than commenting, hope you don't mind https://github.com/zephyrproject-rtos/zephyr/compare/4a509080a693f8cd385fbe2d06427239a03fb13d..ea5e092f390bebe62b3a5e97082bebf35174d9c2)

@WorldofJARcraft
Copy link
Contributor Author

No problem, thanks for your help!

@kartben
Copy link
Collaborator

kartben commented Mar 27, 2025

No problem, thanks for your help!

@WorldofJARcraft not sure anyone cares but if you do (which is fine of course!), maybe re-sign the commit?

image

@WorldofJARcraft WorldofJARcraft force-pushed the openhwgroup-cv64a6-soc branch from ea5e092 to 6648a04 Compare March 27, 2025 17:55
@WorldofJARcraft
Copy link
Contributor Author

I added my signature to the commit and made no changes otherwise.

@WorldofJARcraft WorldofJARcraft force-pushed the openhwgroup-cv64a6-soc branch 3 times, most recently from ec1fa35 to 5b9d4bd Compare March 28, 2025 09:35
fkokosinski
fkokosinski previously approved these changes Mar 28, 2025
kartben
kartben previously approved these changes Apr 8, 2025
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

The comment I left is really only if this goes through another round of updates. Great work!

References
**********

.. _CVA6 documentation:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.. _CVA6 documentation:
.. target-notes::
.. _CVA6 documentation:

Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

couple more documentation things probably worth fixing imo

# Copyright (c) 2025 CISPA Helmholtz Center for Information Security gGmbH
# SPDX-License-Identifier: Apache-2.0

description: Specific device tree attributes for OpenHWGroup cva6 CPUs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
description: Specific device tree attributes for OpenHWGroup cva6 CPUs
description: OpenHW Group CVA6 CPU

@kartben
Copy link
Collaborator

kartben commented Apr 9, 2025

@nordicjm please revisit

@nordicjm
Copy link
Collaborator

Awaiting response from @WorldofJARcraft on #77732 (comment)

@WorldofJARcraft WorldofJARcraft dismissed stale reviews from kartben and fkokosinski via e2ae023 April 10, 2025 07:00
@WorldofJARcraft WorldofJARcraft force-pushed the openhwgroup-cv64a6-soc branch 2 times, most recently from e2ae023 to 5f67135 Compare April 10, 2025 07:03
@WorldofJARcraft
Copy link
Contributor Author

WorldofJARcraft commented Apr 10, 2025

Awaiting response from @WorldofJARcraft on #77732 (comment)

Sorry, I forgot about this thread @nordicjm.

Adds support for the CVA6 family of RISC-V CPUs.
CVA6 is commonly found as a soft core CPU on FPGA designs.
Different configurations and instruction set extensions can be
configured, and different SoCs targeting various FPGA boards are
available.
This commit adds support for the 32-bit and 64-bit configurations
of CVA6, as well as three slightly different SoCs (a minimal 32-bit
configuration, a 64-bit configuration without FPU, a 64-bit
configuration with FPU).

Signed-off-by: Eric Ackermann <[email protected]>
Adds support for the CVA6 CPU on a Genesys 2 FPGA board
(https://github.com/openhwgroup/cva6).
The SoC currently contains the CVA6 CPU  with the SV39 MMU, interrupt
controllers (CLINT and PLIC), UART, a SPI for booting from SD, a boot
ROM, and I2C controller for on-board audio, a GPIO and the lowRISC
ethernet subsystem.
Two slightly different versions of the board are added, with a 64-bit
and a 64-bit configuration of CVA6, respectively.

Signed-off-by: Eric Ackermann <[email protected]>
@WorldofJARcraft WorldofJARcraft force-pushed the openhwgroup-cv64a6-soc branch from 5f67135 to 929d488 Compare April 10, 2025 07:05
@kartben kartben requested a review from fkokosinski April 11, 2025 07:24
@kartben kartben merged commit c2f024e into zephyrproject-rtos:main Apr 11, 2025
24 checks passed
Comment on lines +54 to +57
.. zephyr-app-commands::
:zephyr-app: samples/hello_world
:board: cv64a6_genesys_2
:goals: build flash
Copy link
Collaborator

Choose a reason for hiding this comment

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

@WorldofJARcraft have you actually tested this? It doesn't seem like it's possible to build a hello world at the moment?
Also, you will want to add twister files so that this board is tested in CI, this will allow to catch such issues in the future. Somehow this was missed during the review of this PR.

I tried to kick things off here but you're probably better positioned to take it it from here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kartben I have tested the configuration, but against an outdated main branch, sorry.
Apparently, at some point during when I initially composed the PR and now, the device tree requirements for the RISC-V machine timer changed.
I have force-pushed a corrected version to the branch I have been using for this PR. I am also making the necessary changes for integrating the board into twister.
Should I make a new PR with the necessary changes or can I somehow overwrite the commits that have already been merged?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@WorldofJARcraft Thanks for the quick feedback, really appreciated! It needs to be a new branch/PR based off of current main. FWIW what you now have in your branch looks in a much better shape but I encourage you to actually run Twister to check where things are at (for example I don't think you want "simulation: qemu" in there)

west twister -p cv32a6_genesys_2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kartben I have made a new PR for this: #88511
I have successfully tested the new PR in hardware against cv64a6.
I will set up and test on cv32a6 next week.
Until then, I will leave the PR in draft mode.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice, thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree area: RISCV RISCV Architecture (32-bit & 64-bit)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants