1. Patchew
  2. QEMU
  3. [PATCH RFC 0/8] Add Counter delegation ISA extension support
  4. View patch

[PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg

Atish Patra posted 8 patches 8 months, 2 weeks ago
There is a newer version of this series
[PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg
Posted by Atish Patra 8 months, 2 weeks ago 
From: Kaiwen Xue <kaiwenx@rivosinc.com>

Since xiselect and xireg also will be of use in sxcsrind, AIA should
have its own separated interface when those CSRs are accessed.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
---
 target/riscv/csr.c | 147 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 122 insertions(+), 25 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8829dee7bc75..1af0c8890a2b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -287,6 +287,15 @@ static int aia_any32(CPURISCVState *env, int csrno)
     return any32(env, csrno);
 }
 
+static int sxcsrind_or_aia_any(CPURISCVState *env, int csrno)
+{
+    if (!riscv_cpu_cfg(env)->ext_smaia && !riscv_cpu_cfg(env)->ext_smcsrind) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return any(env, csrno);
+}
+
 static RISCVException smode(CPURISCVState *env, int csrno)
 {
     if (riscv_has_ext(env, RVS)) {
@@ -323,6 +332,15 @@ static int aia_smode32(CPURISCVState *env, int csrno)
     return smode32(env, csrno);
 }
 
+static int sxcsrind_or_aia_smode(CPURISCVState *env, int csrno)
+{
+    if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return smode(env, csrno);
+}
+
 static RISCVException hmode(CPURISCVState *env, int csrno)
 {
     if (riscv_has_ext(env, RVH)) {
@@ -342,6 +360,15 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
 
 }
 
+static int sxcsrind_or_aia_hmode(CPURISCVState *env, int csrno)
+{
+    if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    return hmode(env, csrno);
+}
+
 static RISCVException umode(CPURISCVState *env, int csrno)
 {
     if (riscv_has_ext(env, RVU)) {
@@ -1804,13 +1831,29 @@ static int aia_xlate_vs_csrno(CPURISCVState *env, int csrno)
     };
 }
 
+static int sxcsrind_xlate_vs_csrno(CPURISCVState *env, int csrno)
+{
+    if (!env->virt_enabled) {
+        return csrno;
+    }
+
+    switch (csrno) {
+    case CSR_SISELECT:
+        return CSR_VSISELECT;
+    case CSR_SIREG:
+        return CSR_VSIREG;
+    default:
+        return csrno;
+    };
+}
+
 static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
                         target_ulong new_val, target_ulong wr_mask)
 {
     target_ulong *iselect;
 
     /* Translate CSR number for VS-mode */
-    csrno = aia_xlate_vs_csrno(env, csrno);
+    csrno = sxcsrind_xlate_vs_csrno(env, csrno);
 
     /* Find the iselect CSR based on CSR number */
     switch (csrno) {
@@ -1839,6 +1882,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
     return RISCV_EXCP_NONE;
 }
 
+static bool xiselect_aia_range(target_ulong isel)
+{
+    return (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) ||
+           (ISELECT_IMSIC_FIRST <= isel && isel <= ISELECT_IMSIC_LAST);
+}
+
 static int rmw_iprio(target_ulong xlen,
                      target_ulong iselect, uint8_t *iprio,
                      target_ulong *val, target_ulong new_val,
@@ -1884,44 +1933,44 @@ static int rmw_iprio(target_ulong xlen,
     return 0;
 }
 
-static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
-                     target_ulong new_val, target_ulong wr_mask)
+static int rmw_xireg_aia(CPURISCVState *env, int csrno,
+                         target_ulong isel, target_ulong *val,
+                         target_ulong new_val, target_ulong wr_mask)
 {
-    bool virt, isel_reserved;
-    uint8_t *iprio;
+    bool virt = false, isel_reserved = false;
     int ret = -EINVAL;
-    target_ulong priv, isel, vgein;
-
-    /* Translate CSR number for VS-mode */
-    csrno = aia_xlate_vs_csrno(env, csrno);
+    uint8_t *iprio;
+    target_ulong priv, vgein;
 
-    /* Decode register details from CSR number */
-    virt = false;
-    isel_reserved = false;
+    /* VS-mode CSR number passed in has already been translated */
     switch (csrno) {
     case CSR_MIREG:
+        if (!riscv_cpu_cfg(env)->ext_smaia) {
+            goto done;
+        }
         iprio = env->miprio;
-        isel = env->miselect;
         priv = PRV_M;
         break;
     case CSR_SIREG:
-        if (env->priv == PRV_S && env->mvien & MIP_SEIP &&
+        if (!riscv_cpu_cfg(env)->ext_ssaia ||
+            (env->priv == PRV_S && env->mvien & MIP_SEIP &&
             env->siselect >= ISELECT_IMSIC_EIDELIVERY &&
-            env->siselect <= ISELECT_IMSIC_EIE63) {
+            env->siselect <= ISELECT_IMSIC_EIE63)) {
             goto done;
         }
         iprio = env->siprio;
-        isel = env->siselect;
         priv = PRV_S;
         break;
     case CSR_VSIREG:
+        if (!riscv_cpu_cfg(env)->ext_ssaia) {
+            goto done;
+        }
         iprio = env->hviprio;
-        isel = env->vsiselect;
         priv = PRV_S;
         virt = true;
         break;
     default:
-         goto done;
+        goto done;
     };
 
     /* Find the selected guest interrupt file */
@@ -1952,10 +2001,53 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
     }
 
 done:
+    /*
+     * If AIA is not enabled, illegal instruction exception is always
+     * returned regardless of whether we are in VS-mode or not
+     */
     if (ret) {
         return (env->virt_enabled && virt && !isel_reserved) ?
                RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
     }
+
+    return RISCV_EXCP_NONE;
+}
+
+static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
+                     target_ulong new_val, target_ulong wr_mask)
+{
+    bool virt = false;
+    int ret = -EINVAL;
+    target_ulong isel;
+
+    /* Translate CSR number for VS-mode */
+    csrno = sxcsrind_xlate_vs_csrno(env, csrno);
+
+    /* Decode register details from CSR number */
+    switch (csrno) {
+    case CSR_MIREG:
+        isel = env->miselect;
+        break;
+    case CSR_SIREG:
+        isel = env->siselect;
+        break;
+    case CSR_VSIREG:
+        isel = env->vsiselect;
+        virt = true;
+        break;
+    default:
+         goto done;
+    };
+
+    if (xiselect_aia_range(isel)) {
+        return rmw_xireg_aia(env, csrno, isel, val, new_val, wr_mask);
+    }
+
+done:
+    if (ret) {
+        return (env->virt_enabled && virt) ?
+               RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
+    }
     return RISCV_EXCP_NONE;
 }
 
@@ -4682,8 +4774,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
 
     /* Machine-Level Window to Indirectly Accessed Registers (AIA) */
-    [CSR_MISELECT] = { "miselect", aia_any,   NULL, NULL,    rmw_xiselect },
-    [CSR_MIREG]    = { "mireg",    aia_any,   NULL, NULL,    rmw_xireg },
+    [CSR_MISELECT] = { "miselect", sxcsrind_or_aia_any,   NULL, NULL,
+                       rmw_xiselect                                    },
+    [CSR_MIREG]    = { "mireg",    sxcsrind_or_aia_any,   NULL, NULL,
+                       rmw_xireg                                       },
 
     /* Machine-Level Interrupts (AIA) */
     [CSR_MTOPEI]   = { "mtopei",   aia_any, NULL, NULL, rmw_xtopei },
@@ -4801,8 +4895,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_SATP]     = { "satp",     satp, read_satp,     write_satp     },
 
     /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
-    [CSR_SISELECT]   = { "siselect",   aia_smode, NULL, NULL, rmw_xiselect },
-    [CSR_SIREG]      = { "sireg",      aia_smode, NULL, NULL, rmw_xireg },
+    [CSR_SISELECT]   = { "siselect",   sxcsrind_or_aia_smode, NULL, NULL,
+                         rmw_xiselect                                       },
+    [CSR_SIREG]      = { "sireg",      sxcsrind_or_aia_smode, NULL, NULL,
+                         rmw_xireg                                          },
 
     /* Supervisor-Level Interrupts (AIA) */
     [CSR_STOPEI]     = { "stopei",     aia_smode, NULL, NULL, rmw_xtopei },
@@ -4881,9 +4977,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     /*
      * VS-Level Window to Indirectly Accessed Registers (H-extension with AIA)
      */
-    [CSR_VSISELECT]   = { "vsiselect",   aia_hmode, NULL, NULL,
-                          rmw_xiselect                                     },
-    [CSR_VSIREG]      = { "vsireg",      aia_hmode, NULL, NULL, rmw_xireg  },
+    [CSR_VSISELECT]   = { "vsiselect",   sxcsrind_or_aia_hmode, NULL, NULL,
+                          rmw_xiselect                                      },
+    [CSR_VSIREG]      = { "vsireg",      sxcsrind_or_aia_hmode, NULL, NULL,
+                          rmw_xireg                                         },
 
     /* VS-Level Interrupts (H-extension with AIA) */
     [CSR_VSTOPEI]     = { "vstopei",     aia_hmode, NULL, NULL, rmw_xtopei },
-- 
2.34.1
Re: [PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg
Posted by Jason Chien 4 months, 4 weeks ago 
Atish Patra 於 2024/2/17 上午 08:01 寫道:
> From: Kaiwen Xue <kaiwenx@rivosinc.com>
>
> Since xiselect and xireg also will be of use in sxcsrind, AIA should
> have its own separated interface when those CSRs are accessed.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Kaiwen Xue <kaiwenx@rivosinc.com>
> ---
>   target/riscv/csr.c | 147 +++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 122 insertions(+), 25 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 8829dee7bc75..1af0c8890a2b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -287,6 +287,15 @@ static int aia_any32(CPURISCVState *env, int csrno)
>       return any32(env, csrno);
>   }
>   
> +static int sxcsrind_or_aia_any(CPURISCVState *env, int csrno)
Could you rename the function as csrind_or_aia_any() to keep the naming 
consistency with aia?
> +{
> +    if (!riscv_cpu_cfg(env)->ext_smaia && !riscv_cpu_cfg(env)->ext_smcsrind) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    return any(env, csrno);
> +}
> +
>   static RISCVException smode(CPURISCVState *env, int csrno)
>   {
>       if (riscv_has_ext(env, RVS)) {
> @@ -323,6 +332,15 @@ static int aia_smode32(CPURISCVState *env, int csrno)
>       return smode32(env, csrno);
>   }
>   
> +static int sxcsrind_or_aia_smode(CPURISCVState *env, int csrno)
Could you rename the function as csrind_or_aia_smode() to keep the 
naming consistency with aia?
> +{
> +    if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
S-mode CSRs are defined in Smaia and Smcsrind as well. We should ensure 
at least one of Smaia, Ssaia, Smcsrind, Sscsrind is enabled.
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    return smode(env, csrno);
> +}
> +
>   static RISCVException hmode(CPURISCVState *env, int csrno)
>   {
>       if (riscv_has_ext(env, RVH)) {
> @@ -342,6 +360,15 @@ static RISCVException hmode32(CPURISCVState *env, int csrno)
>   
>   }
>   
> +static int sxcsrind_or_aia_hmode(CPURISCVState *env, int csrno)
Could you rename the function as csrind_or_aia_hmode() to keep the 
naming consistency with aia?
> +{
> +    if (!riscv_cpu_cfg(env)->ext_ssaia && !riscv_cpu_cfg(env)->ext_sscsrind) {
Hypervisor and VS CSRs are defined in Smaia and Smcsrind as well. We 
should ensure at least one of Smaia, Ssaia, Smcsrind, Sscsrind is enabled.
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    return hmode(env, csrno);
> +}
> +
>   static RISCVException umode(CPURISCVState *env, int csrno)
>   {
>       if (riscv_has_ext(env, RVU)) {
> @@ -1804,13 +1831,29 @@ static int aia_xlate_vs_csrno(CPURISCVState *env, int csrno)
>       };
>   }
>   
> +static int sxcsrind_xlate_vs_csrno(CPURISCVState *env, int csrno)
Could you rename the function as csrind_xlate_vs_csrno() to keep the 
naming consistency with aia_xlate_vs_csrno()?
> +{
> +    if (!env->virt_enabled) {
> +        return csrno;
> +    }
> +
> +    switch (csrno) {
> +    case CSR_SISELECT:
> +        return CSR_VSISELECT;
> +    case CSR_SIREG:
> +        return CSR_VSIREG;
> +    default:
> +        return csrno;
> +    };
> +}
> +
>   static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
>                           target_ulong new_val, target_ulong wr_mask)
>   {
>       target_ulong *iselect;
>   
>       /* Translate CSR number for VS-mode */
> -    csrno = aia_xlate_vs_csrno(env, csrno);
> +    csrno = sxcsrind_xlate_vs_csrno(env, csrno);
>   
>       /* Find the iselect CSR based on CSR number */
>       switch (csrno) {
> @@ -1839,6 +1882,12 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, target_ulong *val,
>       return RISCV_EXCP_NONE;
>   }
>   
> +static bool xiselect_aia_range(target_ulong isel)
> +{
> +    return (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) ||
> +           (ISELECT_IMSIC_FIRST <= isel && isel <= ISELECT_IMSIC_LAST);
> +}
> +
>   static int rmw_iprio(target_ulong xlen,
>                        target_ulong iselect, uint8_t *iprio,
>                        target_ulong *val, target_ulong new_val,
> @@ -1884,44 +1933,44 @@ static int rmw_iprio(target_ulong xlen,
>       return 0;
>   }
>   
> -static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> -                     target_ulong new_val, target_ulong wr_mask)
> +static int rmw_xireg_aia(CPURISCVState *env, int csrno,
> +                         target_ulong isel, target_ulong *val,
> +                         target_ulong new_val, target_ulong wr_mask)
>   {
> -    bool virt, isel_reserved;
> -    uint8_t *iprio;
> +    bool virt = false, isel_reserved = false;
>       int ret = -EINVAL;
> -    target_ulong priv, isel, vgein;
> -
> -    /* Translate CSR number for VS-mode */
> -    csrno = aia_xlate_vs_csrno(env, csrno);
> +    uint8_t *iprio;
> +    target_ulong priv, vgein;
>   
> -    /* Decode register details from CSR number */
> -    virt = false;
> -    isel_reserved = false;
> +    /* VS-mode CSR number passed in has already been translated */
>       switch (csrno) {
>       case CSR_MIREG:
> +        if (!riscv_cpu_cfg(env)->ext_smaia) {
> +            goto done;
> +        }
>           iprio = env->miprio;
> -        isel = env->miselect;
>           priv = PRV_M;
>           break;
>       case CSR_SIREG:
> -        if (env->priv == PRV_S && env->mvien & MIP_SEIP &&
> +        if (!riscv_cpu_cfg(env)->ext_ssaia ||
> +            (env->priv == PRV_S && env->mvien & MIP_SEIP &&
>               env->siselect >= ISELECT_IMSIC_EIDELIVERY &&
> -            env->siselect <= ISELECT_IMSIC_EIE63) {
> +            env->siselect <= ISELECT_IMSIC_EIE63)) {
>               goto done;
>           }
>           iprio = env->siprio;
> -        isel = env->siselect;
>           priv = PRV_S;
>           break;
>       case CSR_VSIREG:
> +        if (!riscv_cpu_cfg(env)->ext_ssaia) {
> +            goto done;
> +        }
>           iprio = env->hviprio;
> -        isel = env->vsiselect;
>           priv = PRV_S;
>           virt = true;
>           break;
>       default:
> -         goto done;
> +        goto done;
>       };
>   
>       /* Find the selected guest interrupt file */
> @@ -1952,10 +2001,53 @@ static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
>       }
>   
>   done:
> +    /*
> +     * If AIA is not enabled, illegal instruction exception is always
> +     * returned regardless of whether we are in VS-mode or not
> +     */
>       if (ret) {
>           return (env->virt_enabled && virt && !isel_reserved) ?
>                  RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
>       }
> +
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static int rmw_xireg(CPURISCVState *env, int csrno, target_ulong *val,
> +                     target_ulong new_val, target_ulong wr_mask)
> +{
> +    bool virt = false;
> +    int ret = -EINVAL;
> +    target_ulong isel;
> +
> +    /* Translate CSR number for VS-mode */
> +    csrno = sxcsrind_xlate_vs_csrno(env, csrno);
> +
> +    /* Decode register details from CSR number */
> +    switch (csrno) {
> +    case CSR_MIREG:
> +        isel = env->miselect;
> +        break;
> +    case CSR_SIREG:
> +        isel = env->siselect;
> +        break;
> +    case CSR_VSIREG:
> +        isel = env->vsiselect;
> +        virt = true;
> +        break;
> +    default:
> +         goto done;
> +    };
> +
> +    if (xiselect_aia_range(isel)) {
> +        return rmw_xireg_aia(env, csrno, isel, val, new_val, wr_mask);
> +    }
> +
> +done:
> +    if (ret) {
> +        return (env->virt_enabled && virt) ?
> +               RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST;
A virtual instruction exception is raised for attempts from VS-mode or 
VU-mode to directly access vsiselect or vsireg* in riscv_csrrw_check(), 
we don't need to check again here.
> +    }
>       return RISCV_EXCP_NONE;
>   }
>   
> @@ -4682,8 +4774,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
>   
>       /* Machine-Level Window to Indirectly Accessed Registers (AIA) */
> -    [CSR_MISELECT] = { "miselect", aia_any,   NULL, NULL,    rmw_xiselect },
> -    [CSR_MIREG]    = { "mireg",    aia_any,   NULL, NULL,    rmw_xireg },
> +    [CSR_MISELECT] = { "miselect", sxcsrind_or_aia_any,   NULL, NULL,
> +                       rmw_xiselect                                    },
> +    [CSR_MIREG]    = { "mireg",    sxcsrind_or_aia_any,   NULL, NULL,
> +                       rmw_xireg                                       },
>   
>       /* Machine-Level Interrupts (AIA) */
>       [CSR_MTOPEI]   = { "mtopei",   aia_any, NULL, NULL, rmw_xtopei },
> @@ -4801,8 +4895,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>       [CSR_SATP]     = { "satp",     satp, read_satp,     write_satp     },
>   
>       /* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> -    [CSR_SISELECT]   = { "siselect",   aia_smode, NULL, NULL, rmw_xiselect },
> -    [CSR_SIREG]      = { "sireg",      aia_smode, NULL, NULL, rmw_xireg },
> +    [CSR_SISELECT]   = { "siselect",   sxcsrind_or_aia_smode, NULL, NULL,
> +                         rmw_xiselect                                       },
> +    [CSR_SIREG]      = { "sireg",      sxcsrind_or_aia_smode, NULL, NULL,
> +                         rmw_xireg                                          },
>   
>       /* Supervisor-Level Interrupts (AIA) */
>       [CSR_STOPEI]     = { "stopei",     aia_smode, NULL, NULL, rmw_xtopei },
> @@ -4881,9 +4977,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>       /*
>        * VS-Level Window to Indirectly Accessed Registers (H-extension with AIA)
>        */
> -    [CSR_VSISELECT]   = { "vsiselect",   aia_hmode, NULL, NULL,
> -                          rmw_xiselect                                     },
> -    [CSR_VSIREG]      = { "vsireg",      aia_hmode, NULL, NULL, rmw_xireg  },
> +    [CSR_VSISELECT]   = { "vsiselect",   sxcsrind_or_aia_hmode, NULL, NULL,
> +                          rmw_xiselect                                      },
> +    [CSR_VSIREG]      = { "vsireg",      sxcsrind_or_aia_hmode, NULL, NULL,
> +                          rmw_xireg                                         },
>   
>       /* VS-Level Interrupts (H-extension with AIA) */
>       [CSR_VSTOPEI]     = { "vstopei",     aia_hmode, NULL, NULL, rmw_xtopei },

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.