riscv-software-src / riscv-isa-sim

Spike, a RISC-V ISA Simulator
Other
2.39k stars 839 forks source link

Building spike with "--with-priv=m" config option renders spike unusable #1802

Open mkozden opened 3 weeks ago

mkozden commented 3 weeks ago

To test a RISC-V core with only Machine mode implemented, I require spike to run with M privilege mode.

Building spike as normal with the config options --prefix=$RISCV --with-isa=rv32imf and then using the --priv=m argument seems to not work, as it still checks for the mstatus.FS value (which, to my understanding, doesn't exist if S mode is not implemented).

To work around this issue, I built spike with config options ../configure --prefix=$RISCV --with-isa=rv32imf --with-priv=m. This causes an even stranger issue, where no matter which binary I try to load into spike, it raises the error:

error: bad --isa option 'rv32gqcvh_zfh_zfa_zba_zbb_zbc_zbs_zcb_zicbom_zicboz_zicond_zk_zks_svinval_zcmop_zimop_zawrs_zicfiss_zicfilp_zvknc_zvkg_zvfbfmin_zvfbfwma_zfbfmin'. 'H' extension requires S mode

Which is clearly wrong as that's entirely different than the ISA option that i defined in the config. (It makes no difference if i force any other ISA with the --isa argument)

jerryz123 commented 3 weeks ago

It should no longer be necessary to set the ISA at compile time.

Try building spike without the --with-isa flag to configure. Then, does the same error appear if you run spike with spike --isa=rv32imf --priv=m?

mkozden commented 3 weeks ago

Building with ../configure --prefix=$RISCV --with-priv=m has resulted in the same error. Previously, when I built with only ../configure --prefix=$RISCV, this issue wasn't seen, as expected.

jerryz123 commented 3 weeks ago

What I meant is that the --with-priv and --with-isa flags to ./configure are no longer necessary. Can you try building spike without any configure flags (besides --prefix), then use the --isa and --priv flags at the command line to set the ISA and priv?

spike --isa=rv32if --priv=m <test.elf> works for me, when I have spike compiled without any modifications.

mkozden commented 3 weeks ago

I understand what you meant. As I've said, in that case it works fine. I realized that my initial impression of the working principle of the CSR register value mstatus.FS is flawed, and the --priv argument indeed works as intended.

I think this is simply a problem caused by --with-priv and --with-isa being obsolete. If you also think that way, feel free to close this issue.

jerryz123 commented 3 weeks ago

Glad to hear it. We had some other recent problems with non-default privilege mode support, so a bug here would not have been surprising.

@aswaterman @YenHaoChen @rtwfroody do you have any opinions on removing the compile-time select of DEFAULT_ISA and DEFAULT_PRIV?

aswaterman commented 3 weeks ago

I wouldn't be surprised if some users rely on them for convenience. If they were a major burden to maintain, then I'd say ditch them, but that doesn't appear to be the case. So I guess I lean towards keeping them, but it isn't a strong feeling.

YenHaoChen commented 3 weeks ago

I have never used or been aware of the compile-time select of DEFAULT_ISA and DEFAULT_PRIV before.

rtwfroody commented 3 weeks ago

Compile-time select for DEFAULT_ISA and DEFAULT_PRIV just leads to confusion, because people could run the same spike command and end up with different ISA.

This does lead me to another question I have. In my spike builds (latest master, just now, configured with nothing except --prefix):

~/proj/ri/riscof-/risco/r/S/s/s/dut $ spike -d --isa=rv32imc my.elf
(spike) reg 0 misa
0x40141104

Why is S set in misa for this ISA?

aswaterman commented 3 weeks ago

So, remove it, see if anyone complains, and revert if need be?

YenHaoChen commented 2 weeks ago

Users expect backward compatibility, and developers prefer deprecation. Spike does occasionally deprecate existing features to provide an elegant software implementation, which is essential for others to get involved in development. The deprecation of the P extension is a solid example.

I'd like to know whether Spike could aim at keeping all features in the current release, i.e., Version 1.1.0, and have a deprecation plan for the future release, e.g., Version 1.2.0.

YenHaoChen commented 2 weeks ago

@rtwfroody

It seems the --priv=msu is the default configuration if not provided. The following shows the description of Spike when typing ./spike --help.

image

rtwfroody commented 2 weeks ago

It seems the --priv=msu is the default configuration if not provided. The following shows the description of Spike when typing ./spike --help.

But I find that default configuration very confusing and not what I want. Should I open a separate issue?

Henryjqzhou commented 2 weeks ago

Hello,I also have the same problem when I run spike with --priv=m,I need try the new merge?