Re: [PATCH] hw/riscv: boot: Don't use CSRs if they are disabled

[Jesse Taube]

> From: Alistair Francis <alistair.francis@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.francis@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.

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