On Mon, 2023-01-23 at 17:14 -0500, Jesse Taube 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 */
> reset_vec[2] = 0x00000513; /* li a0, 0 */
> Shouldn't it be li as a0 should contain the cpu number. The regs are
> initialized with 0 so its not necessary but nice to be explicit.
So we can't just run:
li a0, x
where x is the hart ID as each CPU runs the same reset code.
So we could run:
li a0, 0
To zero out a0, but as a0 is 0 on reset I don't think we need to.
This current instruction should be decoded as a NOP, which I think is
simpler for users to understand. Otherwise we will have code on CPU1
that sets a0 to 0, which to me seems more confusing.
Alistair
>
> Thanks,
> Jesse T
> > + }
> > +
> > /* copy in the reset vector in little_endian byte order */
> > for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
> > reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > --
> > 2.39.0
