On Tue, Jan 31, 2023 at 10:31 PM Bin Meng <bmeng...@gmail.com> wrote: > > On Mon, Jan 30, 2023 at 7:19 AM Alistair Francis <alistai...@gmail.com> wrote: > > > > On Thu, Jan 26, 2023 at 10:03 PM Bin Meng <bmeng...@gmail.com> wrote: > > > > > > On Tue, Jan 24, 2023 at 9:42 AM Alistair Francis <alistai...@gmail.com> > > > wrote: > > > > > > > > On Tue, Jan 24, 2023 at 11:24 AM Bin Meng <bmeng...@gmail.com> wrote: > > > > > > > > > > On Mon, Jan 23, 2023 at 11:58 AM Alistair Francis > > > > > <alistair.fran...@opensource.wdc.com> wrote: > > > > > > > > > > > > From: Alistair Francis <alistair.fran...@wdc.com> > > > > > > > > > > > > If the CSRs and CSR instructions are disabled because the Zicsr > > > > > > extension isn't enabled then we want to make sure we don't run any > > > > > > CSR > > > > > > instructions in the boot ROM. > > > > > > > > > > > > This patches removes the CSR instructions from the reset-vec if the > > > > > > extension isn't enabled. We replace the instruction with a NOP > > > > > > instead. > > > > > > > > > > > > Note that we don't do this for the SiFive U machine, as we are > > > > > > modelling > > > > > > the hardware in that case. > > > > > > > > > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1447 > > > > > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > > > > > > --- > > > > > > hw/riscv/boot.c | 9 +++++++++ > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > > > > > > index 2594276223..cb27798a25 100644 > > > > > > --- a/hw/riscv/boot.c > > > > > > +++ b/hw/riscv/boot.c > > > > > > @@ -356,6 +356,15 @@ void riscv_setup_rom_reset_vec(MachineState > > > > > > *machine, RISCVHartArrayState *harts > > > > > > reset_vec[4] = 0x0182b283; /* ld t0, 24(t0) */ > > > > > > } > > > > > > > > > > > > + if (!harts->harts[0].cfg.ext_icsr) { > > > > > > + /* > > > > > > + * The Zicsr extension has been disabled, so let's ensure > > > > > > we don't > > > > > > + * run the CSR instruction. Let's fill the address with a > > > > > > non > > > > > > + * compressed nop. > > > > > > + */ > > > > > > + reset_vec[2] = 0x00000013; /* addi x0, x0, 0 */ > > > > > > + } > > > > > > > > > > This is fine for a UP system. I am not sure how SMP can be supported > > > > > without Zicsr as we need to assign hartid in a0. > > > > > > > > Yeah. My thinking was that no one would be using a multicore system > > > > without Zicsr as it's such a core extension. If they are running > > > > without Zicsr they have probably hard coded a lot of things anyway and > > > > don't expect this to work. > > > > > > > > In general I think it's pretty rare to even run a RISC-V core without > > > > Zicsr at all. > > > > > > > > > > As QEMU implements Zicsr anyway, and there is no way to support SMP > > > without Zicsr, should we disallow user to disable Zicsr in QEMU? > > > > I feel like we don't need to do that. Here's my thinking: > > > > Zicsr is a RISC-V extension, the RISC-V spec splits it out so that it > > can be disabled. In theory someone could build a multi-hart CPU > > without Zicsr in hardware, so QEMU should be able to model it. > > Correct. But if Zicsr is not present, then the standard privileged > architecture which qemu-system-riscv currently supports, is inherently > not present, either. > > If a user chooses to disable Zicsr, current QEMU system emulation is > useless then.
I wouldn't say useless. If a user does disable Zicsr then effectively they have disabled the priv spec. > > That's why I believe we shouldn't allow users to disable Zicsr for > qemu-system-riscv. We currently support disabling it and it appears that people are using it, so I don't think it makes sense to remove. I agree for a standard use case Zicsr will always be enabled, but I can picture users running experiments/tests/benchmarks and wanting to disable the extension. Alistair > > > As well as that Zicsr is enabled by default, so a user has to know > > enough to disable it manually. At which point they probably know what > > they are doing, especially as no standard software will run without > > Zicsr. If that's what someone wants to do then we should allow them > > to, even if it's a bit strange. > > > > For qemu-riscv, disabling Zicsr is feasible as long as the codes does > not touch any CSR, e.g.: timer, counters, fcsr, etc. > > Regards, > Bin
