riscv-software-src / riscv-isa-sim

Spike, a RISC-V ISA Simulator
Other
2.36k stars 826 forks source link

Wrong mstatus.[SXL|UXL] for RV64I in M-mode only config #1781

Closed mkpest closed 2 weeks ago

mkpest commented 2 weeks ago

I'm runnig some simple asm code using Spike (current version - today's build) in RV64I, M mode only config:

spike -p1  -m0x80000000:1568768,0x80180000:131072,0x801a0000:4096 -l --isa=rv64i --pmpregions=0 --priv=m --hartids=0 --log-commits --log=tmp/log.txt --triggers=0 --dm-sba=64 output.elf

At some point code reads mstatus (from log):

core   0: 0x0000000080000104 (0x300026f3) csrr    a3, mstatus
core   0: 3 0x0000000080000104 (0x300026f3) x13 0x0000000a00001800

Read value is 0x0000000a00001800 which seems wrong assuming --priv=m flag. It seems that spike assigns mstatus.SXL|UXL as misa.MXL. Correct value should be: 0x0000000000001800 . Accroding to spec:

When MXLEN=64, if S-mode is not supported, then SXL is read-only zero. [...] When MXLEN=64, if U-mode is not supported, then UXL is read-only zero. (Volume II, 3.1.6.2. Base ISA Control in mstatus Register, p. 27)

aswaterman commented 2 weeks ago

I only looked into this one long enough to find that, at the time that the mstatus CSR is created, Spike believes that U and S modes are present, despite the —-priv setting. There is clearly a bug, but I’d appreciate it if someone else can try to ferret it out.

@mkpest As with the RV32E issue you raised earlier, you are poking around corners of Spike that we theoretically should support but that we don’t put much effort behind. Most of the maintainers build RV32I and RV64I systems with multiple privilege modes. If you’re interested in helping us out in verifying and fixing these nooks and crannies, we’d certainly appreciate your help.

rtwfroody commented 2 weeks ago

I just ran into this same bug. Looks like the first processor created gets it right, and then there's an issue where some device tree parsing code gets to run, and has MSU hard-coded:

(gdb) r -d --priv=m --isa=rv32imc my.elf
Starting program: /opt/riscv/bin/spike -d --priv=m --isa=rv32imc my.elf
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".

Breakpoint 1, isa_parser_t::isa_parser_t (this=this@entry=0x7fffffffbac0, str=0x7fffffffdac5 "rv32imc", priv=0x7fffffffdabd "m")
    at ../disasm/isa_parser.cc:30
30      isa_parser_t::isa_parser_t(const char* str, const char *priv)
(gdb) p str
$9 = 0x7fffffffdac5 "rv32imc"
(gdb) p priv
$10 = 0x7fffffffdabd "m"
(gdb) bt
#0  isa_parser_t::isa_parser_t (this=this@entry=0x7fffffffbac0, str=0x7fffffffdac5 "rv32imc", priv=0x7fffffffdabd "m")
    at ../disasm/isa_parser.cc:30
#1  0x0000555555cce529 in make_dts (insns_per_rtc_tick=insns_per_rtc_tick@entry=100, cpu_hz=cpu_hz@entry=1000000000, 
    cfg=cfg@entry=0x7fffffffc4d0, mems=std::vector of length 1, capacity 1 = {...}, 
    device_nodes="    clint@2000000 {\n      compatible = \"riscv,clint0\";\n      interrupts-extended = <&CPU0_intc 3 &CPU0_intc 7 >;\n      reg = <0x0 0x2000000 0x0 0xc0000>;\n    };\n    PLIC: plic@c000000 {\n      compatib"...) at ../riscv/dts.cc:25
#2  0x00005555558657f0 in sim_t::sim_t (this=0x7fffffffc980, cfg=0x7fffffffc4d0, halted=false, mems=std::vector of length 1, capacity 1 = {...}, 
    plugin_device_factories=..., args=..., dm_config=..., log_path=0x0, dtb_enabled=true, dtb_file=0x0, socket_enabled=false, cmd_file=0x0)
    at ../riscv/sim.cc:145
#3  0x000055555582356f in main (argc=<optimized out>, argv=<optimized out>) at ../spike_main/spike.cc:525
(gdb) c
Continuing.
[Detaching after fork from child process 566857]
[Detaching after fork from child process 566858]

Breakpoint 1, isa_parser_t::isa_parser_t (this=this@entry=0x7ffff799f020, str=0x555555e4caf8 "rv32imc", priv=priv@entry=0x555555d1d517 "MSU")
    at ../disasm/isa_parser.cc:30
30      isa_parser_t::isa_parser_t(const char* str, const char *priv)
(gdb) p str
$11 = 0x555555e4caf8 "rv32imc"
(gdb) p priv
$12 = 0x555555d1d517 "MSU"
(gdb) bt
#0  isa_parser_t::isa_parser_t (this=this@entry=0x7ffff799f020, str=0x555555e4caf8 "rv32imc", priv=priv@entry=0x555555d1d517 "MSU")
    at ../disasm/isa_parser.cc:30
#1  0x000055555585b152 in processor_t::processor_t (this=this@entry=0x7ffff799f010, isa_str=<optimized out>, 
    priv_str=priv_str@entry=0x555555d1d517 "MSU", cfg=cfg@entry=0x7fffffffc4d0, sim=sim@entry=0x7fffffffcc40, id=0, halt_on_reset=false, 
    log_file=0x7ffff7bc94e0 <_IO_2_1_stderr_>, sout_=...) at ../riscv/processor.cc:37
#2  0x00005555558652c3 in sim_t::sim_t (this=0x7fffffffc980, cfg=0x7fffffffc4d0, halted=false, mems=..., plugin_device_factories=..., args=..., 
    dm_config=..., log_path=0x0, dtb_enabled=true, dtb_file=0x0, socket_enabled=false, cmd_file=0x0) at ../riscv/sim.cc:200
#3  0x000055555582356f in main (argc=<optimized out>, argv=<optimized out>) at ../spike_main/spike.cc:525
(gdb) c
Continuing.

Breakpoint 1, isa_parser_t::isa_parser_t (this=this@entry=0x7fffffffbb30, 
    str=0x555555e5ce10 "rv32gqcvh_zfh_zfa_zba_zbb_zbc_zbs_zcb_zicbom_zicboz_zicond_zk_zks_svinval_zcmop_zimop_zawrs_zicfiss_zicfilp_zvknc_zvkg_zvfbfmin_zvfbfwma_zfbfmin", priv=priv@entry=0x555555d1d517 "MSU") at ../disasm/isa_parser.cc:30
30      isa_parser_t::isa_parser_t(const char* str, const char *priv)
(gdb) p str
$13 = 0x555555e5ce10 "rv32gqcvh_zfh_zfa_zba_zbb_zbc_zbs_zcb_zicbom_zicboz_zicond_zk_zks_svinval_zcmop_zimop_zawrs_zicfiss_zicfilp_zvknc_zvkg_zvfbfmin_zvfbfwma_zfbfmin"
(gdb) p priv
$14 = 0x555555d1d517 "MSU"
(gdb) bt
#0  isa_parser_t::isa_parser_t (this=this@entry=0x7fffffffbb30, 
    str=0x555555e5ce10 "rv32gqcvh_zfh_zfa_zba_zbb_zbc_zbs_zcb_zicbom_zicboz_zicond_zk_zks_svinval_zcmop_zimop_zawrs_zicfiss_zicfilp_zvknc_zvkg_zvfbfmin_zvfbfwma_zfbfmin", priv=priv@entry=0x555555d1d517 "MSU") at ../disasm/isa_parser.cc:30
#1  0x0000555555ce8eef in disassembler_t::disassembler_t (this=this@entry=0x555555e57880, isa=isa@entry=0x7ffff799f020)
    at /usr/include/c++/14.2.1/bits/basic_string.h:227
#2  0x000055555585b85f in processor_t::processor_t (this=this@entry=0x7ffff799f010, isa_str=<optimized out>, 
    priv_str=priv_str@entry=0x555555d1d517 "MSU", cfg=cfg@entry=0x7fffffffc4d0, sim=sim@entry=0x7fffffffcc40, id=<optimized out>, 
    halt_on_reset=<optimized out>, log_file=<optimized out>, sout_=...) at ../riscv/processor.cc:67
#3  0x00005555558652c3 in sim_t::sim_t (this=0x7fffffffc980, cfg=0x7fffffffc4d0, halted=false, mems=..., plugin_device_factories=..., args=..., 
    dm_config=..., log_path=0x0, dtb_enabled=true, dtb_file=0x0, socket_enabled=false, cmd_file=0x0) at ../riscv/sim.cc:200
#4  0x000055555582356f in main (argc=<optimized out>, argv=<optimized out>) at ../spike_main/spike.cc:525
(gdb) 

The code where MSU is hard-coded was introduced by @jerryz123 in f11bd7b511d7909f0291589e3aaab720ededdc8a

jerryz123 commented 2 weeks ago

Sorry, I think #1787 should fix this

rtwfroody commented 2 weeks ago

It's working now. Thank you.