Re: [PATCH v2 2/2] target/riscv: csr: Implement mconfigptr CSR

[Rahul Pathak]

On Wed, Oct 27, 2021 at 8:13 AM Alistair Francis <alistair23@gmail.com> wrote:

On Mon, Oct 25, 2021 at 10:55 PM Rahul Pathak <rpathak@ventanamicro.com> wrote:
>
> Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> ---
>  target/riscv/cpu_bits.h |  1 +
>  target/riscv/csr.c      | 19 +++++++++++++++----
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index cffcd3a5df..e2f154b7c5 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -140,6 +140,7 @@
>  #define CSR_MARCHID         0xf12
>  #define CSR_MIMPID          0xf13
>  #define CSR_MHARTID         0xf14
> +#define CSR_MCONFIGPTR      0xf15
>
>  /* Machine Trap Setup */
>  #define CSR_MSTATUS         0x300
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 69e4d65fcd..2d7f608d49 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -209,6 +209,16 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
>
>      return RISCV_EXCP_ILLEGAL_INST;
>  }
> +
> +static RISCVException priv1p12(CPURISCVState *env, int csrno)
> +{
> +       if (env->priv_ver >= PRIV_VERSION_1_12_0) {
> +               return RISCV_EXCP_NONE;
> +       }
> +
> +       return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
>  #endif
>
>  /* User Floating-Point CSRs */
> @@ -1569,10 +1579,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>
>      /* Machine Information Registers */
> -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> +    [CSR_MVENDORID] = { "mvendorid",   any,            read_zero    },
> +    [CSR_MARCHID]   = { "marchid",     any,            read_zero    },
> +    [CSR_MIMPID]    = { "mimpid",      any,            read_zero    },
> +    [CSR_MHARTID]   = { "mhartid",     any,            read_mhartid },

Why change these?

The alignment of all structure entries is consistent in their respective 

blocks, that's why I aligned these with the mconfigptr line.

It's really not necessary and if my observation on the alignment is

really not a requirement I will undo this.

 

> +    [CSR_MCONFIGPTR] = {"mconfigptr",  priv1p12,       read_zero    },

This looks fine, but there are more changes then this in v1.12.
Looking at the preface we need mret/sret changes at least. It also
looks like some other changes will need to be implemented or at least
checked.

 

Agree, I will look into that

 

Alistair

>
>      /* Machine Trap Setup */
>      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus     },
> --
> 2.25.1
>
>