chipsalliance / dromajo

RISC-V RV64GC emulator designed for RTL co-simulation
Apache License 2.0
213 stars 63 forks source link

Add flag to clear mimpid, marchid, mvendorid #4

Closed abejgonzalez closed 4 years ago

abejgonzalez commented 4 years ago

Related Issues/PRs: riscv-boom/riscv-boom#415

Release Notes:

Adds a flag to clear mimpid, marchid, and mvendorid which are not implemented in BOOM.

et-tommythorn commented 4 years ago

Thanks for the patch, but I think we need to go a different direction. Adding more command line option clearly doesn't scale. I'd think perhaps some new configuration file override parameters might work better here, named simply mipid, marchid, and mvendorid. To be legal JSON we can allow both a decimal version and a hexidecimal one, given as a string.

What do you think?

In general, I like to move away from adding more command line options, except for one: an option to add inline configuration parameters. Thus one mechanism could subsume both.

abejgonzalez commented 4 years ago

I also agree that adding extra options will not scale well. I think doing a JSON input makes sense and shouldn't be too hard to implement. Ill go ahead and close this try to modify the config. input.

As for the inline config parameters, I think that is also fine.