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. That's why I believe we shouldn't allow users to disable Zicsr for qemu-system-riscv. > 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
