chipsalliance / dromajo

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

[Bug Report] MISC Suggestion #60

Closed Phantom1003 closed 2 years ago

Phantom1003 commented 2 years ago
  1. there is an exit backdoor during handling ecall. The bare metal test case may not follow the sbi specification, I coincidentally triggered it, and I think it may be an unnecessary feature leftover from the development process. https://github.com/chipsalliance/dromajo/blob/fae7b3a13ae5e28baceacf64f064af254501aa4a/include/dromajo_template.h#L1173-L1175
  2. dromajo uses the old specification that fill 0 to tval when illegal instruction, I suggest writing the illegal value in it. https://github.com/chipsalliance/dromajo/blob/fae7b3a13ae5e28baceacf64f064af254501aa4a/include/dromajo_template.h#L1636-L1638
  3. In hardware, some CSR are not guaranteed to be 0 during reset, I suggest using random values to fill them, like mie. We can see some firmware has the reset mie behavior. https://github.com/riscv-software-src/opensbi/blob/bd355213bfbb209c047e8cc0df56936f6705477f/firmware/payloads/test_head.S#L52
et-tommythorn commented 2 years ago
  1. I agree. Ideally dromajo should be more composable so this kind of policy can be lifted to the end-user application. It doesn't belong in the library. For now I'm happy to see it removed.
  2. Both are legal according to spec 1.11 (this is one of the problems with the spec, there are too many options). I'm ok changing it to fill in the faulting instruction.
  3. Sure, by all means. However, getting consistent runs from simulation is critical, so I suggest adding the option to give the random seen in the config file.

Thanks for all the issues. I'd encourage you to submit a pull request, but otherwise we'll get to it eventually.

Phantom1003 commented 2 years ago

Thanks Tommy, some of the changes I had made are just temporary patches, and I'm worried that committing them directly will break dromajo's stability, but I'll try to commit some simple ones first.