Changes to processor cores maintained by lowRISC and OpenHW Group

[vogelpi]

With Ibex and CV32E40P, the two main processor cores supported inside PULP are no longer maintained by the PULP team itself. This has several advantages for the PULP team but it can complicate things around PULP-specific changes in those cores and it should probably be documented how the PULP team wants to deal with such changes.

For Ibex, we try to merge as many changes as possible upstream but for very PULP-specific stuff, we have a branch called pulpissimo upstream that contains such PULP-specific changes. From time to time, we rebase this branch on the current master and create a tag that survives future rebases. Between rebases, contributors can still do PRs into the pulpissimo branch and we try to carry those changes forward. This is not perfect but so far it meant quite low overhead.

For CV32E40P, the situation seems to be that PULP is using a slightly older version from Nov 2019 pulpissimo-v3.4.0. I can imagine that it would be quite some work to update the core to the latest version. Maybe someone is already working on that. However, it's not clear how even simple changes such as modifications to the Bender.yml file can be made and used within PULP.

Of course, the creation of a fork is always possible but IMO this should be avoided as it again introduced the risk of having different versions that drift apart over time.

I would be interested to get your views @davideschiavone @daviderossi1982 @bluewww @meggiman @accuminium @zarubaf
I don't have a strong preference but I feel the team should take a decision and document this to clarify the situation for everybody involved.

[vogelpi]

FYI @micprog

[andreaskurth]

Also adding @OttG as maintainer of pulp_cluster.

[davideschiavone]

Hello @vogelpi , thanks for driving this into GitHub.

We need to be involved directly in this conversation with the PULPissimo maintainers from ETHZ @bluewww and @meggiman .

Talking about CV32E40P, the current verified version from OpenHW (with no PULP extensions yet verified) will be used in CORE-V-MCU, which is a fork of the current PULPissimo. @timsaxe is working on it, and recently he raised the question of finding a joint effort to make the two MCUs as similar as possible. Thus, the work that is going to be done in OpenHW Group to support CV32E40P should be, in theory, easily portable through PRs to PULPissimo.

Still, a couple of questions are open - maybe we can organize a meeting to solve these questions, but at the moment, I write them down here:

1. CV32E40P has 19 interrupts instead of 32 as RI5CY.
   The design is done in a way that having again 32 interrupts is easy, but it requires forks as the parameter to control that is not exposed at user level to force compliance with RISC-V (this problem is also present in Ibex AFAIK)
   This can be solved either by forking from PULP (not recommended), or maybe by making a new file in OHW (like a possible configuration file with localparameters) that PULP can then replace. That's the best solution in my opinion and previously discussed.

2. CV32E40P is compliant with RISC-V
   The old RI5CY was not for what concerns performance counters for example. This breaks the pulp-sdk/runtime. Thus, someone from the PULP team should make the effort to align the SDK. This would be anyway needed for Ibex as Ibex is compliant with RISC-V.

3. OpenHW Group driven by its members as ETH Zurich and Embecosm are moving towards freeRTOS. This is the OS that will be maintained in core-v-mcu. If there is any intention to do the same in PULPissimo as standard, then possible (but not necessarily) changes needed in the MCU to support freeRTOS can then be ported to PULPissimo, reducing the diversifications between the 2 MCUs.

[vogelpi]

Hi @davideschiavone ,
thanks for your input. You brought up a couple of relevant points. Even though some of them have an impact on the decision we take for the original issue, I believe they should be discussed separately for the sake of overview.

Regarding the original issue on

Where should PULP-specific changes to the processor cores live?

it seems that we actually have a cv32e40p fork since mid Dec here: https://github.com/pulp-platform/cv32e40p

It would be great to get @meggiman and @bluewww 's views on the matter.

[davideschiavone]

Updates to add to my points.

@timsaxe and @hpollittsmith , that are working on the OpenHW Group PULPissimo version, want to update the PULPissimo repository with an extra parameter to support the last version of the CV32E40P core.

This version will be working only with freeRTOS, unless someone updates the pulp-runtime (but not OpenHW Group).

So the pulp-specific changes are not really needed for this specific case.

I see the PULP-specific changes living in a fork as it is today, but that needs a person that periodically rebase the core to the mainline. It should not be difficult. But it must be done.

[bluewww]

The fork https://github.com/pulp-platform/cv32e40p is meant for super light downstream patches (as in changing a handful lines of code at most). It also serves as staging ground for things that are not pulp specific and can be upstreamed.

@vogelpi I think we can keep small pulp specific patches in https://github.com/pulp-platform/cv32e40p

[meggiman]

I share Robert's opinion. The pulp-platform/cv32e40p repo should not act as a slowly diverging fork of the core but a slightly patched version of a reasonably recent version of the upstream repo that is rebased on regular intervals.

[vogelpi]

Thanks @bluewww and @meggiman , this sounds very reasonable. To make sure the fork indeed remains a staging ground and only contains light downstream patches I think someone should someone in the team should become the repository owner. Just like we have owners for the other PULP repos. Do you have someone in mind already?

[bluewww]

Yes me and manuel are currently the owners

[vogelpi]

Perfect, I just cross-checked the IP owner's list and the fork + owners are listed there. This is great. I will also add/propose an entry for Ibex.

Sorry, I probably should have checked the list earlier.