Re: [PATCH v2 02/13] target/riscv: Decouple AIA processing from xiselect and xireg

[Alistair Francis]

On Wed, Jul 24, 2024 at 9:32 AM Atish Patra <atishp@rivosinc.com> wrote:
>
> 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/csr.c | 165 
> ++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 139 insertions(+), 26 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index ea3560342c4f..58be8bc3cc8c 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -29,6 +29,7 @@
>  #include "sysemu/cpu-timers.h"
>  #include "qemu/guest-random.h"
>  #include "qapi/error.h"
> +#include <stdbool.h>
>
>  /* CSR function table public API */
>  void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
> @@ -286,6 +287,15 @@ static RISCVException aia_any32(CPURISCVState *env, int 
> csrno)
>      return any32(env, csrno);
>  }
>
> +static RISCVException csrind_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)) {
> @@ -322,6 +332,30 @@ static RISCVException aia_smode32(CPURISCVState *env, 
> int csrno)
>      return smode32(env, csrno);
>  }
>
> +static bool csrind_extensions_present(CPURISCVState *env)
> +{
> +    return riscv_cpu_cfg(env)->ext_smcsrind || 
> riscv_cpu_cfg(env)->ext_sscsrind;
> +}
> +
> +static bool aia_extensions_present(CPURISCVState *env)
> +{
> +    return riscv_cpu_cfg(env)->ext_smaia || riscv_cpu_cfg(env)->ext_ssaia;
> +}
> +
> +static bool csrind_or_aia_extensions_present(CPURISCVState *env)
> +{
> +    return csrind_extensions_present(env) || aia_extensions_present(env);
> +}
> +
> +static RISCVException csrind_or_aia_smode(CPURISCVState *env, int csrno)
> +{
> +    if (!csrind_or_aia_extensions_present(env)) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    return smode(env, csrno);
> +}
> +
>  static RISCVException hmode(CPURISCVState *env, int csrno)
>  {
>      if (riscv_has_ext(env, RVH)) {
> @@ -341,6 +375,15 @@ static RISCVException hmode32(CPURISCVState *env, int 
> csrno)
>
>  }
>
> +static RISCVException csrind_or_aia_hmode(CPURISCVState *env, int csrno)
> +{
> +    if (!csrind_or_aia_extensions_present(env)) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    return hmode(env, csrno);
> +}
> +
>  static RISCVException umode(CPURISCVState *env, int csrno)
>  {
>      if (riscv_has_ext(env, RVU)) {
> @@ -1928,6 +1971,22 @@ static int aia_xlate_vs_csrno(CPURISCVState *env, int 
> csrno)
>      };
>  }
>
> +static int csrind_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 RISCVException rmw_xiselect(CPURISCVState *env, int csrno,
>                                     target_ulong *val, target_ulong new_val,
>                                     target_ulong wr_mask)
> @@ -1935,7 +1994,7 @@ static RISCVException rmw_xiselect(CPURISCVState *env, 
> int csrno,
>      target_ulong *iselect;
>
>      /* Translate CSR number for VS-mode */
> -    csrno = aia_xlate_vs_csrno(env, csrno);
> +    csrno = csrind_xlate_vs_csrno(env, csrno);
>
>      /* Find the iselect CSR based on CSR number */
>      switch (csrno) {
> @@ -1964,6 +2023,12 @@ static RISCVException rmw_xiselect(CPURISCVState *env, 
> int csrno,
>      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,
> @@ -2009,45 +2074,44 @@ static int rmw_iprio(target_ulong xlen,
>      return 0;
>  }
>
> -static RISCVException rmw_xireg(CPURISCVState *env, int csrno,
> -                                target_ulong *val, target_ulong new_val,
> -                                target_ulong wr_mask)
> +static RISCVException 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 */
> @@ -2078,10 +2142,54 @@ static RISCVException rmw_xireg(CPURISCVState *env, 
> int csrno,
>      }
>
>  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 RISCVException 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 = csrind_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;
>  }
>
> @@ -4981,8 +5089,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", csrind_or_aia_any,   NULL, NULL,
> +                       rmw_xiselect                                    },
> +    [CSR_MIREG]    = { "mireg",    csrind_or_aia_any,   NULL, NULL,
> +                       rmw_xireg                                       },
>
>      /* Machine-Level Interrupts (AIA) */
>      [CSR_MTOPEI]   = { "mtopei",   aia_any, NULL, NULL, rmw_xtopei },
> @@ -5100,8 +5210,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",   csrind_or_aia_smode, NULL, NULL,
> +                         rmw_xiselect                                       
> },
> +    [CSR_SIREG]      = { "sireg",      csrind_or_aia_smode, NULL, NULL,
> +                         rmw_xireg                                          
> },
>
>      /* Supervisor-Level Interrupts (AIA) */
>      [CSR_STOPEI]     = { "stopei",     aia_smode, NULL, NULL, rmw_xtopei },
> @@ -5180,9 +5292,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",   csrind_or_aia_hmode, NULL, NULL,
> +                          rmw_xiselect                                      
> },
> +    [CSR_VSIREG]      = { "vsireg",      csrind_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
>
>