[PATCH v3 0/2] Add S-mode checks for delegation-related CSRs

[PATCH v3 0/2] Add S-mode checks for delegation-related CSRs

[Jay Chang]

Patch 1 adds a predicate to restrict access to "medeleg, mideleg, and 
medelegh" to harts that support S-mode. 

Patch 2 adds a privilege check for the "midelegh" CSR, which is defined by 
the AIA extension and only valid when Smaia is supported. This is enforced 
via an updated predicate in aia_smode32. 

Change log:
  V3:
    * Add cover letter

Jay Chang (2):
  target/riscv: Restrict mideleg/medeleg/medelegh access to S-mode harts
  target/riscv: Restrict midelegh access to S-mode harts

 target/riscv/csr.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

-- 
2.48.1

[PATCH v3 1/2] target/riscv: Restrict mideleg/medeleg/medelegh access to S-mode harts

[Jay Chang]

RISC-V Privileged Spec states:
"In harts with S-mode, the medeleg and mideleg registers must exist, and
setting a bit in medeleg or mideleg will delegate the corresponding trap
, when occurring in S-mode or U-mode, to the S-mode trap handler. In
harts without S-mode, the medeleg and mideleg registers should not
exist."

Add smode predicate to ensure these CSRs are only accessible when S-mode
is supported.

Reviewed-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Jay Chang <jay.chang@sifive.com>
---
 target/riscv/csr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6296ecd1e1..0e0ad37654 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -5862,8 +5862,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
                           NULL,                read_mstatus_i128           },
     [CSR_MISA]        = { "misa",       any,   read_misa,    write_misa,
                           NULL,                read_misa_i128              },
-    [CSR_MIDELEG]     = { "mideleg",    any,   NULL, NULL,   rmw_mideleg   },
-    [CSR_MEDELEG]     = { "medeleg",    any,   read_medeleg, write_medeleg },
+    [CSR_MIDELEG]     = { "mideleg",    smode,   NULL, NULL,   rmw_mideleg   },
+    [CSR_MEDELEG]     = { "medeleg",    smode,   read_medeleg, write_medeleg },
     [CSR_MIE]         = { "mie",        any,   NULL, NULL,   rmw_mie       },
     [CSR_MTVEC]       = { "mtvec",      any,   read_mtvec,   write_mtvec   },
     [CSR_MCOUNTEREN]  = { "mcounteren", umode, read_mcounteren,
@@ -5871,7 +5871,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 
     [CSR_MSTATUSH]    = { "mstatush",   any32, read_mstatush,
                           write_mstatush                                   },
-    [CSR_MEDELEGH]    = { "medelegh",   any32, read_zero, write_ignore,
+    [CSR_MEDELEGH]    = { "medelegh",   smode32, read_zero, write_ignore,
                           .min_priv_ver = PRIV_VERSION_1_13_0              },
     [CSR_HEDELEGH]    = { "hedelegh",   hmode32, read_hedelegh, write_hedelegh,
                           .min_priv_ver = PRIV_VERSION_1_13_0              },
-- 
2.48.1

[PATCH v3 2/2] target/riscv: Restrict midelegh access to S-mode harts

[Jay Chang]

RISC-V AIA Spec states:
"For a machine-level environment, extension Smaia encompasses all added
CSRs and all modifications to interrupt response behavior that the AIA
specifies for a hart, over all privilege levels. For a supervisor-level
environment, extension Ssaia is essentially the same as Smaia except
excluding the machine-level CSRs and behavior not directly visible to
supervisor level."

Since midelegh is an AIA machine-mode CSR, add Smaia extension check in
aia_smode32 predicate.

Reviewed-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Jay Chang <jay.chang@sifive.com>
---
 target/riscv/csr.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0e0ad37654..74ec0e1c60 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -374,8 +374,11 @@ static RISCVException aia_smode(CPURISCVState *env, int csrno)
 static RISCVException aia_smode32(CPURISCVState *env, int csrno)
 {
     int ret;
+    int csr_priv = get_field(csrno, 0x300);
 
-    if (!riscv_cpu_cfg(env)->ext_ssaia) {
+    if (csr_priv == PRV_M && !riscv_cpu_cfg(env)->ext_smaia) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    } else if (!riscv_cpu_cfg(env)->ext_ssaia) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
 
@@ -5911,7 +5914,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MVIP]     = { "mvip",     aia_any, NULL, NULL, rmw_mvip    },
 
     /* Machine-Level High-Half CSRs (AIA) */
-    [CSR_MIDELEGH] = { "midelegh", aia_any32, NULL, NULL, rmw_midelegh },
+    [CSR_MIDELEGH] = { "midelegh", aia_smode32, NULL, NULL, rmw_midelegh },
     [CSR_MIEH]     = { "mieh",     aia_any32, NULL, NULL, rmw_mieh     },
     [CSR_MVIENH]   = { "mvienh",   aia_any32, NULL, NULL, rmw_mvienh   },
     [CSR_MVIPH]    = { "mviph",    aia_any32, NULL, NULL, rmw_mviph    },
-- 
2.48.1

Re: [PATCH v3 2/2] target/riscv: Restrict midelegh access to S-mode harts

[Nutty Liu]

On 7/1/2025 11:00 AM, Jay Chang wrote:
> RISC-V AIA Spec states:
> "For a machine-level environment, extension Smaia encompasses all added
> CSRs and all modifications to interrupt response behavior that the AIA
> specifies for a hart, over all privilege levels. For a supervisor-level
> environment, extension Ssaia is essentially the same as Smaia except
> excluding the machine-level CSRs and behavior not directly visible to
> supervisor level."
>
> Since midelegh is an AIA machine-mode CSR, add Smaia extension check in
> aia_smode32 predicate.
>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Jay Chang <jay.chang@sifive.com>
> ---
>   target/riscv/csr.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0e0ad37654..74ec0e1c60 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -374,8 +374,11 @@ static RISCVException aia_smode(CPURISCVState *env, int csrno)
>   static RISCVException aia_smode32(CPURISCVState *env, int csrno)
>   {
>       int ret;
> +    int csr_priv = get_field(csrno, 0x300);
>   
> -    if (!riscv_cpu_cfg(env)->ext_ssaia) {
> +    if (csr_priv == PRV_M && !riscv_cpu_cfg(env)->ext_smaia) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    } else if (!riscv_cpu_cfg(env)->ext_ssaia) {

+    if ((csr_priv == PRV_M && !riscv_cpu_cfg(env)->ext_smaia) ||
+        (!riscv_cpu_cfg(env)->ext_ssaia)) {

Would the above code be better ?
Otherwise,
Reviewed-by: Nutty Liu<liujingqi@lanxincomputing.com>

Thanks,
Nutty

>           return RISCV_EXCP_ILLEGAL_INST;
>       }
>   
> @@ -5911,7 +5914,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>       [CSR_MVIP]     = { "mvip",     aia_any, NULL, NULL, rmw_mvip    },
>   
>       /* Machine-Level High-Half CSRs (AIA) */
> -    [CSR_MIDELEGH] = { "midelegh", aia_any32, NULL, NULL, rmw_midelegh },
> +    [CSR_MIDELEGH] = { "midelegh", aia_smode32, NULL, NULL, rmw_midelegh },
>       [CSR_MIEH]     = { "mieh",     aia_any32, NULL, NULL, rmw_mieh     },
>       [CSR_MVIENH]   = { "mvienh",   aia_any32, NULL, NULL, rmw_mvienh   },
>       [CSR_MVIPH]    = { "mviph",    aia_any32, NULL, NULL, rmw_mviph    },

Re: [PATCH v3 1/2] target/riscv: Restrict mideleg/medeleg/medelegh access to S-mode harts

[Nutty Liu]

On 7/1/2025 11:00 AM, Jay Chang wrote:
> RISC-V Privileged Spec states:
> "In harts with S-mode, the medeleg and mideleg registers must exist, and
> setting a bit in medeleg or mideleg will delegate the corresponding trap
> , when occurring in S-mode or U-mode, to the S-mode trap handler. In
> harts without S-mode, the medeleg and mideleg registers should not
> exist."
>
> Add smode predicate to ensure these CSRs are only accessible when S-mode
> is supported.
>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Jay Chang <jay.chang@sifive.com>
> ---
>   target/riscv/csr.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 6296ecd1e1..0e0ad37654 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -5862,8 +5862,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>                             NULL,                read_mstatus_i128           },
>       [CSR_MISA]        = { "misa",       any,   read_misa,    write_misa,
>                             NULL,                read_misa_i128              },
> -    [CSR_MIDELEG]     = { "mideleg",    any,   NULL, NULL,   rmw_mideleg   },
> -    [CSR_MEDELEG]     = { "medeleg",    any,   read_medeleg, write_medeleg },
> +    [CSR_MIDELEG]     = { "mideleg",    smode,   NULL, NULL,   rmw_mideleg   },
> +    [CSR_MEDELEG]     = { "medeleg",    smode,   read_medeleg, write_medeleg },
>       [CSR_MIE]         = { "mie",        any,   NULL, NULL,   rmw_mie       },
>       [CSR_MTVEC]       = { "mtvec",      any,   read_mtvec,   write_mtvec   },
>       [CSR_MCOUNTEREN]  = { "mcounteren", umode, read_mcounteren,
> @@ -5871,7 +5871,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>   
>       [CSR_MSTATUSH]    = { "mstatush",   any32, read_mstatush,
>                             write_mstatush                                   },
> -    [CSR_MEDELEGH]    = { "medelegh",   any32, read_zero, write_ignore,
> +    [CSR_MEDELEGH]    = { "medelegh",   smode32, read_zero, write_ignore,
>                             .min_priv_ver = PRIV_VERSION_1_13_0              },
>       [CSR_HEDELEGH]    = { "hedelegh",   hmode32, read_hedelegh, write_hedelegh,
>                             .min_priv_ver = PRIV_VERSION_1_13_0              },

Reviewed-by: Nutty Liu<liujingqi@lanxincomputing.com>

Thanks,
Nutty

Re: [PATCH v3 2/2] target/riscv: Restrict midelegh access to S-mode harts

[Jay Chang]

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

I intended to separate the S-mode and M-mode handling.
Do you think this change could improve performance?

Thanks,
Jay Chang

On Tue, Jul 1, 2025 at 11:46 AM Nutty Liu <liujingqi@lanxincomputing.com>
wrote:

> On 7/1/2025 11:00 AM, Jay Chang wrote:
> > RISC-V AIA Spec states:
> > "For a machine-level environment, extension Smaia encompasses all added
> > CSRs and all modifications to interrupt response behavior that the AIA
> > specifies for a hart, over all privilege levels. For a supervisor-level
> > environment, extension Ssaia is essentially the same as Smaia except
> > excluding the machine-level CSRs and behavior not directly visible to
> > supervisor level."
> >
> > Since midelegh is an AIA machine-mode CSR, add Smaia extension check in
> > aia_smode32 predicate.
> >
> > Reviewed-by: Frank Chang <frank.chang@sifive.com>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > Signed-off-by: Jay Chang <jay.chang@sifive.com>
> > ---
> >   target/riscv/csr.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 0e0ad37654..74ec0e1c60 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -374,8 +374,11 @@ static RISCVException aia_smode(CPURISCVState *env,
> int csrno)
> >   static RISCVException aia_smode32(CPURISCVState *env, int csrno)
> >   {
> >       int ret;
> > +    int csr_priv = get_field(csrno, 0x300);
> >
> > -    if (!riscv_cpu_cfg(env)->ext_ssaia) {
> > +    if (csr_priv == PRV_M && !riscv_cpu_cfg(env)->ext_smaia) {
> > +        return RISCV_EXCP_ILLEGAL_INST;
> > +    } else if (!riscv_cpu_cfg(env)->ext_ssaia) {
>
> +    if ((csr_priv == PRV_M && !riscv_cpu_cfg(env)->ext_smaia) ||
> +        (!riscv_cpu_cfg(env)->ext_ssaia)) {
>
> Would the above code be better ?
> Otherwise,
> Reviewed-by: Nutty Liu<liujingqi@lanxincomputing.com>
>
> Thanks,
> Nutty
>
> >           return RISCV_EXCP_ILLEGAL_INST;
> >       }
> >
> > @@ -5911,7 +5914,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >       [CSR_MVIP]     = { "mvip",     aia_any, NULL, NULL, rmw_mvip    },
> >
> >       /* Machine-Level High-Half CSRs (AIA) */
> > -    [CSR_MIDELEGH] = { "midelegh", aia_any32, NULL, NULL, rmw_midelegh
> },
> > +    [CSR_MIDELEGH] = { "midelegh", aia_smode32, NULL, NULL,
> rmw_midelegh },
> >       [CSR_MIEH]     = { "mieh",     aia_any32, NULL, NULL, rmw_mieh
>  },
> >       [CSR_MVIENH]   = { "mvienh",   aia_any32, NULL, NULL, rmw_mvienh
>  },
> >       [CSR_MVIPH]    = { "mviph",    aia_any32, NULL, NULL, rmw_mviph
> },
>

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

Re: [PATCH v3 0/2] Add S-mode checks for delegation-related CSRs

[Alistair Francis]

On Tue, Jul 1, 2025 at 1:02 PM Jay Chang <jay.chang@sifive.com> wrote:
>
> Patch 1 adds a predicate to restrict access to "medeleg, mideleg, and
> medelegh" to harts that support S-mode.
>
> Patch 2 adds a privilege check for the "midelegh" CSR, which is defined by
> the AIA extension and only valid when Smaia is supported. This is enforced
> via an updated predicate in aia_smode32.
>
> Change log:
>   V3:
>     * Add cover letter
>
> Jay Chang (2):
>   target/riscv: Restrict mideleg/medeleg/medelegh access to S-mode harts
>   target/riscv: Restrict midelegh access to S-mode harts

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/csr.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> --
> 2.48.1
>
>