[PATCH v2 0/2] mconfigptr support

[PATCH v2 0/2] mconfigptr support

[Rahul Pathak]

Patches add the mconfigptr csr support. 
mconfigptr is newly incorporated in risc-v privileged architecture
specification 1.12 version. 
priv spec 1.12.0 version check is also added.


qemu-system-riscv64 -nographic -machine virt -cpu rv64,priv_spec=v1.12.0

Changelog:

v1->v2
------
1. Added privileged architecture spec version 1.12 ("v1.12.0") check
2. Added predicate function for mconfigptr which verifies
for priv spec version v1.12.0 or higher.

Thanks
Rahul

Rahul Pathak (2):
  target/riscv: Add priv spec 1.12.0 version check
  target/riscv: csr: Implement mconfigptr CSR

 target/riscv/cpu.c      |  4 +++-
 target/riscv/cpu.h      |  1 +
 target/riscv/cpu_bits.h |  1 +
 target/riscv/csr.c      | 19 +++++++++++++++----
 4 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] target/riscv: Add priv spec 1.12.0 version check

[Rahul Pathak]

Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
---
 target/riscv/cpu.c | 4 +++-
 target/riscv/cpu.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 788fa0b11c..83c3814a5a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -405,7 +405,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     }
 
     if (cpu->cfg.priv_spec) {
-        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
+        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
+            priv_version = PRIV_VERSION_1_12_0;
+        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
             priv_version = PRIV_VERSION_1_11_0;
         } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
             priv_version = PRIV_VERSION_1_10_0;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a33dc30be8..67c52e6f9e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -79,6 +79,7 @@ enum {
 
 #define PRIV_VERSION_1_10_0 0x00011000
 #define PRIV_VERSION_1_11_0 0x00011100
+#define PRIV_VERSION_1_12_0 0x00011200
 
 #define VEXT_VERSION_0_07_1 0x00000701
 
-- 
2.25.1

Re: [PATCH v2 1/2] target/riscv: Add priv spec 1.12.0 version check

[Alistair Francis]

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.c | 4 +++-
>  target/riscv/cpu.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 788fa0b11c..83c3814a5a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -405,7 +405,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>      }
>
>      if (cpu->cfg.priv_spec) {
> -        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
> +        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
> +            priv_version = PRIV_VERSION_1_12_0;
> +        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {

This change, actually allowing the user to enable the spec, should be
in a separate patch at the end of the series.

The idea is to add the feature, then expose it.

Alistair


>              priv_version = PRIV_VERSION_1_11_0;
>          } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
>              priv_version = PRIV_VERSION_1_10_0;
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a33dc30be8..67c52e6f9e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -79,6 +79,7 @@ enum {
>
>  #define PRIV_VERSION_1_10_0 0x00011000
>  #define PRIV_VERSION_1_11_0 0x00011100
> +#define PRIV_VERSION_1_12_0 0x00011200
>
>  #define VEXT_VERSION_0_07_1 0x00000701
>
> --
> 2.25.1
>
>

Re: [PATCH v2 1/2] target/riscv: Add priv spec 1.12.0 version check

[Rahul Pathak]

[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]

On Wed, Oct 27, 2021 at 8:08 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.c | 4 +++-
> >  target/riscv/cpu.h | 1 +
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 788fa0b11c..83c3814a5a 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -405,7 +405,9 @@ static void riscv_cpu_realize(DeviceState *dev,
> Error **errp)
> >      }
> >
> >      if (cpu->cfg.priv_spec) {
> > -        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
> > +        if (!g_strcmp0(cpu->cfg.priv_spec, "v1.12.0")) {
> > +            priv_version = PRIV_VERSION_1_12_0;
> > +        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
>
> This change, actually allowing the user to enable the spec, should be
> in a separate patch at the end of the series.
>
> The idea is to add the feature, then expose it.
>

Sure, will change in the next version

>
> Alistair
>
>
> >              priv_version = PRIV_VERSION_1_11_0;
> >          } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
> >              priv_version = PRIV_VERSION_1_10_0;
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index a33dc30be8..67c52e6f9e 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -79,6 +79,7 @@ enum {
> >
> >  #define PRIV_VERSION_1_10_0 0x00011000
> >  #define PRIV_VERSION_1_11_0 0x00011100
> > +#define PRIV_VERSION_1_12_0 0x00011200
> >
> >  #define VEXT_VERSION_0_07_1 0x00000701
> >
> > --
> > 2.25.1
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 2866 bytes --]

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

[Rahul Pathak]

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 },
+    [CSR_MCONFIGPTR] = {"mconfigptr", 	priv1p12,  	read_zero    },
 
     /* Machine Trap Setup */
     [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus     },
-- 
2.25.1

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

[Alistair Francis]

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?



> +    [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.

Alistair

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

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

[Rahul Pathak]

[-- Attachment #1: Type: text/plain, Size: 2947 bytes --]

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
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 4932 bytes --]

Re: [PATCH v2 0/2] mconfigptr support

[Alistair Francis]

On Mon, Oct 25, 2021 at 10:51 PM Rahul Pathak <rpathak@ventanamicro.com> wrote:
>
> Patches add the mconfigptr csr support.
> mconfigptr is newly incorporated in risc-v privileged architecture
> specification 1.12 version.
> priv spec 1.12.0 version check is also added.
>
>
> qemu-system-riscv64 -nographic -machine virt -cpu rv64,priv_spec=v1.12.0

Thanks for the patches!

I gave some comments in line with the code changes. Overall this looks
good, we just need to add the other v1.12.0 features.

Alistair

>
> Changelog:
>
> v1->v2
> ------
> 1. Added privileged architecture spec version 1.12 ("v1.12.0") check
> 2. Added predicate function for mconfigptr which verifies
> for priv spec version v1.12.0 or higher.
>
> Thanks
> Rahul
>
> Rahul Pathak (2):
>   target/riscv: Add priv spec 1.12.0 version check
>   target/riscv: csr: Implement mconfigptr CSR
>
>  target/riscv/cpu.c      |  4 +++-
>  target/riscv/cpu.h      |  1 +
>  target/riscv/cpu_bits.h |  1 +
>  target/riscv/csr.c      | 19 +++++++++++++++----
>  4 files changed, 20 insertions(+), 5 deletions(-)
>
> --
> 2.25.1
>
>

Re: [PATCH v2 0/2] mconfigptr support

[Rahul Pathak]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

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

> On Mon, Oct 25, 2021 at 10:51 PM Rahul Pathak <rpathak@ventanamicro.com>
> wrote:
> >
> > Patches add the mconfigptr csr support.
> > mconfigptr is newly incorporated in risc-v privileged architecture
> > specification 1.12 version.
> > priv spec 1.12.0 version check is also added.
> >
> >
> > qemu-system-riscv64 -nographic -machine virt -cpu rv64,priv_spec=v1.12.0
>
> Thanks for the patches!
>
> I gave some comments in line with the code changes. Overall this looks
> good, we just need to add the other v1.12.0 features.
>
> Alistair


Thanks Alistair, I will work on the comments and send the
next version.

>
>
>
> > Changelog:
> >
> > v1->v2
> > ------
> > 1. Added privileged architecture spec version 1.12 ("v1.12.0") check
> > 2. Added predicate function for mconfigptr which verifies
> > for priv spec version v1.12.0 or higher.
> >
> > Thanks
> > Rahul
> >
> > Rahul Pathak (2):
> >   target/riscv: Add priv spec 1.12.0 version check
> >   target/riscv: csr: Implement mconfigptr CSR
> >
> >  target/riscv/cpu.c      |  4 +++-
> >  target/riscv/cpu.h      |  1 +
> >  target/riscv/cpu_bits.h |  1 +
> >  target/riscv/csr.c      | 19 +++++++++++++++----
> >  4 files changed, 20 insertions(+), 5 deletions(-)
> >
> > --
> > 2.25.1
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 2688 bytes --]