diff mbox series

[04/11,v2] target/riscv: Update CSR xie in CLIC mode

Message ID 20240819160742.27586-8-Ian.Brockbank@cirrus.com (mailing list archive)
State New, archived
Headers show
Series RISC-V: support CLIC v0.9 specification | expand

Commit Message

Ian Brockbank Aug. 19, 2024, 4:02 p.m. UTC
From: Ian Brockbank <ian.brockbank@cirrus.com>

The xie CSR appears hardwired to zero in CLIC mode, replaced by separate
memory-mapped interrupt enables (clicintie[i]). Writes to xie will be
ignored and will not trap (i.e., no access faults).

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
Signed-off-by: Ian Brockbank <ian.brockbank@cirrus.com>
---
 target/riscv/csr.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

--
2.46.0.windows.1
This message and any attachments may contain privileged and confidential information that is intended solely for the person(s) to whom it is addressed. If you are not an intended recipient you must not: read; copy; distribute; discuss; take any action in or make any reliance upon the contents of this message; nor open or read any attachment. If you have received this message in error, please notify us as soon as possible on the following telephone number and destroy this message including any attachments. Thank you. Cirrus Logic International (UK) Ltd and Cirrus Logic International Semiconductor Ltd are companies registered in Scotland, with registered numbers SC089839 and SC495735 respectively. Our registered office is at 7B Nightingale Way, Quartermile, Edinburgh, EH3 9EG, UK. Tel: +44 (0)131 272 7000. www.cirrus.com

Comments

Alistair Francis Sept. 6, 2024, 2:58 a.m. UTC | #1
On Tue, Aug 20, 2024 at 2:15 AM Ian Brockbank <Ian.Brockbank@cirrus.com> wrote:
>
> From: Ian Brockbank <ian.brockbank@cirrus.com>
>
> The xie CSR appears hardwired to zero in CLIC mode, replaced by separate
> memory-mapped interrupt enables (clicintie[i]). Writes to xie will be
> ignored and will not trap (i.e., no access faults).
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> Signed-off-by: Ian Brockbank <ian.brockbank@cirrus.com>
> ---
>  target/riscv/csr.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 9c824c0d8f..a5978e0929 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -30,6 +30,10 @@
>  #include "qemu/guest-random.h"
>  #include "qapi/error.h"
>
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/intc/riscv_clic.h"
> +#endif

This doesn't seem like the way to go

> +
>  /* CSR function table public API */
>  void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
>  {
> @@ -1805,16 +1809,19 @@ static RISCVException rmw_mie64(CPURISCVState *env, int csrno,
>                                  uint64_t *ret_val,
>                                  uint64_t new_val, uint64_t wr_mask)
>  {
> -    uint64_t mask = wr_mask & all_ints;
> +    /* Access to xie will be ignored in CLIC mode and will not trap. */
> +    if (!riscv_clic_is_clic_mode(env)) {

We can just implement this check, it's only two lines

> +        uint64_t mask = wr_mask & all_ints;
>
> -    if (ret_val) {
> -        *ret_val = env->mie;
> -    }
> +        if (ret_val) {
> +            *ret_val = env->mie;
> +        }
>
> -    env->mie = (env->mie & ~mask) | (new_val & mask);
> +        env->mie = (env->mie & ~mask) | (new_val & mask);
>
> -    if (!riscv_has_ext(env, RVH)) {
> -        env->mie &= ~((uint64_t)HS_MODE_INTERRUPTS);
> +        if (!riscv_has_ext(env, RVH)) {
> +            env->mie &= ~((uint64_t)HS_MODE_INTERRUPTS);
> +        }
>      }
>
>      return RISCV_EXCP_NONE;
> @@ -2906,13 +2913,13 @@ static int read_mintstatus(CPURISCVState *env, int csrno, target_ulong *val)
>  static int read_mintthresh(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>      *val = env->mintthresh;
> -    return 0;
> +    return RISCV_EXCP_NONE;

This change should be made when these functions are added

Alistair

>  }
>
>  static int write_mintthresh(CPURISCVState *env, int csrno, target_ulong val)
>  {
>      env->mintthresh = val;
> -    return 0;
> +    return RISCV_EXCP_NONE;
>  }
>
>  /* Supervisor Trap Setup */
> @@ -3059,7 +3066,10 @@ static RISCVException rmw_sie64(CPURISCVState *env, int csrno,
>              *ret_val |= env->sie & nalias_mask;
>          }
>
> -        env->sie = (env->sie & ~sie_mask) | (new_val & sie_mask);
> +        /* Writes to xie will be ignored in CLIC mode and will not trap. */
> +        if (!riscv_clic_is_clic_mode(env)) {
> +            env->sie = (env->sie & ~sie_mask) | (new_val & sie_mask);
> +        }
>      }
>
>      return ret;
> @@ -3337,13 +3347,13 @@ static int read_sintstatus(CPURISCVState *env, int csrno, target_ulong *val)
>  static int read_sintthresh(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>      *val = env->sintthresh;
> -    return 0;
> +    return RISCV_EXCP_NONE;
>  }
>
>  static int write_sintthresh(CPURISCVState *env, int csrno, target_ulong val)
>  {
>      env->sintthresh = val;
> -    return 0;
> +    return RISCV_EXCP_NONE;
>  }
>
>  /* Supervisor Protection and Translation */
> --
> 2.46.0.windows.1
> This message and any attachments may contain privileged and confidential information that is intended solely for the person(s) to whom it is addressed. If you are not an intended recipient you must not: read; copy; distribute; discuss; take any action in or make any reliance upon the contents of this message; nor open or read any attachment. If you have received this message in error, please notify us as soon as possible on the following telephone number and destroy this message including any attachments. Thank you. Cirrus Logic International (UK) Ltd and Cirrus Logic International Semiconductor Ltd are companies registered in Scotland, with registered numbers SC089839 and SC495735 respectively. Our registered office is at 7B Nightingale Way, Quartermile, Edinburgh, EH3 9EG, UK. Tel: +44 (0)131 272 7000. www.cirrus.com
>
Alistair Francis Sept. 6, 2024, 3:20 a.m. UTC | #2
On Fri, Sep 6, 2024 at 12:58 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Aug 20, 2024 at 2:15 AM Ian Brockbank <Ian.Brockbank@cirrus.com> wrote:
> >
> > From: Ian Brockbank <ian.brockbank@cirrus.com>
> >
> > The xie CSR appears hardwired to zero in CLIC mode, replaced by separate
> > memory-mapped interrupt enables (clicintie[i]). Writes to xie will be
> > ignored and will not trap (i.e., no access faults).
> >
> > Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> > Signed-off-by: Ian Brockbank <ian.brockbank@cirrus.com>
> > ---
> >  target/riscv/csr.c | 34 ++++++++++++++++++++++------------
> >  1 file changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 9c824c0d8f..a5978e0929 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -30,6 +30,10 @@
> >  #include "qemu/guest-random.h"
> >  #include "qapi/error.h"
> >
> > +#if !defined(CONFIG_USER_ONLY)
> > +#include "hw/intc/riscv_clic.h"
> > +#endif
>
> This doesn't seem like the way to go

Urgh! Ok, it's trickier than that.

I think ideally we don't want to pull in a bunch of CLIC stuff, just
the bare minimum.

It's probably better to implement the CLIC functions for CSR access in
the target/riscv directory instead of in hw/intc

Also, to make it easier to review it would be great to add the
functions when they are used. Then patch 3 will be smaller and the
other patches easier to see what is being added

Alistair
diff mbox series

Patch

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 9c824c0d8f..a5978e0929 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -30,6 +30,10 @@ 
 #include "qemu/guest-random.h"
 #include "qapi/error.h"

+#if !defined(CONFIG_USER_ONLY)
+#include "hw/intc/riscv_clic.h"
+#endif
+
 /* CSR function table public API */
 void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
 {
@@ -1805,16 +1809,19 @@  static RISCVException rmw_mie64(CPURISCVState *env, int csrno,
                                 uint64_t *ret_val,
                                 uint64_t new_val, uint64_t wr_mask)
 {
-    uint64_t mask = wr_mask & all_ints;
+    /* Access to xie will be ignored in CLIC mode and will not trap. */
+    if (!riscv_clic_is_clic_mode(env)) {
+        uint64_t mask = wr_mask & all_ints;

-    if (ret_val) {
-        *ret_val = env->mie;
-    }
+        if (ret_val) {
+            *ret_val = env->mie;
+        }

-    env->mie = (env->mie & ~mask) | (new_val & mask);
+        env->mie = (env->mie & ~mask) | (new_val & mask);

-    if (!riscv_has_ext(env, RVH)) {
-        env->mie &= ~((uint64_t)HS_MODE_INTERRUPTS);
+        if (!riscv_has_ext(env, RVH)) {
+            env->mie &= ~((uint64_t)HS_MODE_INTERRUPTS);
+        }
     }

     return RISCV_EXCP_NONE;
@@ -2906,13 +2913,13 @@  static int read_mintstatus(CPURISCVState *env, int csrno, target_ulong *val)
 static int read_mintthresh(CPURISCVState *env, int csrno, target_ulong *val)
 {
     *val = env->mintthresh;
-    return 0;
+    return RISCV_EXCP_NONE;
 }

 static int write_mintthresh(CPURISCVState *env, int csrno, target_ulong val)
 {
     env->mintthresh = val;
-    return 0;
+    return RISCV_EXCP_NONE;
 }

 /* Supervisor Trap Setup */
@@ -3059,7 +3066,10 @@  static RISCVException rmw_sie64(CPURISCVState *env, int csrno,
             *ret_val |= env->sie & nalias_mask;
         }

-        env->sie = (env->sie & ~sie_mask) | (new_val & sie_mask);
+        /* Writes to xie will be ignored in CLIC mode and will not trap. */
+        if (!riscv_clic_is_clic_mode(env)) {
+            env->sie = (env->sie & ~sie_mask) | (new_val & sie_mask);
+        }
     }

     return ret;
@@ -3337,13 +3347,13 @@  static int read_sintstatus(CPURISCVState *env, int csrno, target_ulong *val)
 static int read_sintthresh(CPURISCVState *env, int csrno, target_ulong *val)
 {
     *val = env->sintthresh;
-    return 0;
+    return RISCV_EXCP_NONE;
 }

 static int write_sintthresh(CPURISCVState *env, int csrno, target_ulong val)
 {
     env->sintthresh = val;
-    return 0;
+    return RISCV_EXCP_NONE;
 }

 /* Supervisor Protection and Translation */