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

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

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