Re: [PATCH 12/15] hw/riscv: sifive: Change SiFive E/U CPU reset vector to 0x1004

[Alistair Francis <alistair23@gmail.com>]

On Mon, Jun 8, 2020 at 7:27 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Per the SiFive manual, all E/U series CPU cores' reset vector is
> at 0x1004. Update our codes to match the hardware.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  hw/riscv/sifive_e.c | 8 +++++---
>  hw/riscv/sifive_u.c | 6 +++---
>  target/riscv/cpu.c  | 4 ++--
>  3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 8fab152..f05cabd 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -98,9 +98,11 @@ static void sifive_e_machine_init(MachineState *machine)
>          memmap[SIFIVE_E_DTIM].base, main_mem);
>
>      /* Mask ROM reset vector */
> -    uint32_t reset_vec[2] = {
> -        0x204002b7,        /* 0x1000: lui     t0,0x20400 */
> -        0x00028067,        /* 0x1004: jr      t0 */
> +    uint32_t reset_vec[4] = {
> +        0x00000000,
> +        0x204002b7,        /* 0x1004: lui     t0,0x20400 */
> +        0x00028067,        /* 0x1008: jr      t0 */
> +        0x00000000,
>      };
>
>      /* copy in the reset vector in little_endian byte order */
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index aaa5adb..0a86ffc 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -433,18 +433,18 @@ static void sifive_u_machine_init(MachineState *machine)
>
>      /* reset vector */
>      uint32_t reset_vec[8] = {
> +        0x00000000,
>          0x00000297,                    /* 1:  auipc  t0, %pcrel_hi(dtb) */
> -        0x02028593,                    /*     addi   a1, t0, %pcrel_lo(1b) */
> +        0x01c28593,                    /*     addi   a1, t0, %pcrel_lo(1b) */
>          0xf1402573,                    /*     csrr   a0, mhartid  */
>  #if defined(TARGET_RISCV32)
>          0x0182a283,                    /*     lw     t0, 24(t0) */
>  #elif defined(TARGET_RISCV64)
> -        0x0182b283,                    /*     ld     t0, 24(t0) */
> +        0x0182e283,                    /*     lwu    t0, 24(t0) */
>  #endif
>          0x00028067,                    /*     jr     t0 */
>          0x00000000,
>          start_addr,                    /* start: .dword */
> -        0x00000000,
>                                         /* dtb: */
>      };
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 5060534..1944ad6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -139,7 +139,7 @@ static void rvnn_gcsu_priv1_10_0_cpu_init(Object *obj)
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
>      set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>      set_priv_version(env, PRIV_VERSION_1_10_0);
> -    set_resetvec(env, DEFAULT_RSTVEC);
> +    set_resetvec(env, 0x1004);
>  }
>
>  static void rvnn_imacu_nommu_cpu_init(Object *obj)
> @@ -147,7 +147,7 @@ static void rvnn_imacu_nommu_cpu_init(Object *obj)
>      CPURISCVState *env = &RISCV_CPU(obj)->env;
>      set_misa(env, RVXLEN | RVI | RVM | RVA | RVC | RVU);
>      set_priv_version(env, PRIV_VERSION_1_10_0);
> -    set_resetvec(env, DEFAULT_RSTVEC);
> +    set_resetvec(env, 0x1004);

I know the Ibex CPU does the same thing, but I'm a little worried about this.

At this rate we will have multiple generic (rvx_im*_cpu_init()) CPUs
that don't have generic reset vectors. It might be best to rename the
functions to match the actual CPUs in this case.

Alistair

>      qdev_prop_set_bit(DEVICE(obj), "mmu", false);
>  }
>
> --
> 2.7.4
>
>