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

Hartid of 992 causes all sorts of troubles #107

Open
bluewww opened this issue Dec 3, 2019 · 11 comments
Open

Hartid of 992 causes all sorts of troubles #107

bluewww opened this issue Dec 3, 2019 · 11 comments

Comments

@bluewww
Copy link
Collaborator

@bluewww bluewww commented Dec 3, 2019

  • OpenOCD (unpatched) can't deal with hartid numbers larger than 32
  • The debug module spawns a 1024 bit registers where most of its bit get removed during synthesis (super noisy)
@MarekPikula

This comment has been minimized.

Copy link
Contributor

@MarekPikula MarekPikula commented Dec 3, 2019

And important to note is that not all registers are removed (at least by Quartus).

I've tested PULPissimo with hartid=0 with stock OpenOCD and it worked without hiccup.

@bluewww

This comment has been minimized.

Copy link
Collaborator Author

@bluewww bluewww commented Dec 3, 2019

ok cool. I'm preparing some patches to maybe set hartid to 0 for PULPissimo by default. I don't think I have time to patch openocd right now to support more than 32 hartids (properly) since it needs a rewrite of significant parts of it.

@stefanct

This comment has been minimized.

Copy link

@stefanct stefanct commented Dec 4, 2019

I am trying to re-create @MarekPikula's setup but for some reason OpenOCD stil thinks the hart ID is 992 i.e. the cluster ID is 31. Currently, I don't understand why.

I have changed FC_Core_CLUSTER_ID to 6'd0 and set NrHarts to 1 and re-built the bitstream (in a retry I also executed make clean_zedboard before).
The Vivado logs reflect my changes:

INFO: [Synth 8-6157] synthesizing module 'pulp_soc' [.../ips/pulp_soc/rtl/pulp_soc/pulp_soc.sv:15]
[…]
    Parameter FC_Core_CLUSTER_ID bound to: 6'b000000 
    Parameter FC_Core_CORE_ID bound to: 4'b0000 
    Parameter FC_Core_MHARTID bound to: 11'b00000000000 
    Parameter NrHarts bound to: 1 - type: integer 
    Parameter SELECTABLE_HARTS bound to: 1'b1 

Nevertheless, openocd (UCB's e03dd199e) trips over an assertion:

openocd: src/target/riscv/riscv.c:2647: riscv_set_current_hartid: Assertion `riscv_hart_enabled(target, hartid)' failed.

And when I looked at the target->coreid field it is 992. I presume this is read out via JTAG but I did not follow the code through (it all happens in openocd's TCL interpreter while reading the config file...). Any hints welcome ;)

@bluewww

This comment has been minimized.

Copy link
Collaborator Author

@bluewww bluewww commented Dec 4, 2019

Does your OpenOCD script have this line:
target create $_TARGETNAME riscv -chain-position $_TARGETNAME -coreid 0x3e0

@bluewww

This comment has been minimized.

Copy link
Collaborator Author

@bluewww bluewww commented Dec 4, 2019

I have a patched version of OpenOCD https://github.com/pulp-platform/riscv-openocd which can handle hartids up to 1024. I will try to upstream the patch but I don't know if it is going to be accepted.

@bluewww

This comment has been minimized.

Copy link
Collaborator Author

@bluewww bluewww commented Dec 4, 2019

Lastly regarding changing the default hartid I don't think I can do this since it will break too many things and is actually a problem of OpenOCD. I can patch PULPissimo though to make the hartid configurable via a parameter and mention this in the README.

@MarekPikula

This comment has been minimized.

Copy link
Contributor

@MarekPikula MarekPikula commented Dec 4, 2019

Yeah, you should change the -coreid to 0 in OpenOCD's config file.

@bluewww what exactly it breaks? I didn't encounter any problems with OpenOCD when working on hartid = 0.

@haugoug

This comment has been minimized.

Copy link
Member

@haugoug haugoug commented Dec 4, 2019

This is just for historical reasons. Pulpissimo is part of a the Pulp project. Until 2 years ago, Pulp was just having one cluster, and it got assigned hartid 0. Then when we added the main controller attached to the cluster and it got assigned this strange hartid because we were supporting up to 31 clusters and the main controller was assigned cluster id 31. Now the problem is that some parts of the SW (including tests) assume that clusters are numbered starting from 0, so we cannot change it without cleaning all this code.

@bluewww

This comment has been minimized.

Copy link
Collaborator Author

@bluewww bluewww commented Dec 6, 2019

A fix is upstreamed to handle up to 1024 harts: riscv/riscv-openocd#429

@stefanct

This comment has been minimized.

Copy link

@stefanct stefanct commented Dec 17, 2019

JFYI, yes, the -coreid parameter was the problem in my case. I have probably overlooked it because I did not expect it to be in the OpenOCD config because it is indeed useless for single-core designs. It works just fine without the parameter.

@stefanct

This comment has been minimized.

Copy link

@stefanct stefanct commented Jan 13, 2020

One other (and hopefully the last) thing that needs to be changed to get it to work when using the pulp-sdk is setting ARCHI_FC_CID correctly in runtime/archi/include/archi/chips/pulpissimo/properties.h. With that and the other changes mentioned here I was able to get to main() with the upstream openocd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.