[PATCH 3/5] target/riscv: Don't OR mip.SEIP when mvien is one

alistair23@gmail.com posted 5 patches 4 days, 14 hours ago
Maintainers: Alistair Francis <Alistair.Francis@wdc.com>, Laurent Vivier <laurent@vivier.eu>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Palmer Dabbelt <palmer@dabbelt.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>
[PATCH 3/5] target/riscv: Don't OR mip.SEIP when mvien is one
Posted by alistair23@gmail.com 4 days, 14 hours ago
From: Alistair Francis <alistair.francis@wdc.com>

The RISC-V spec states that

"""
But when bit 9 of mvien is one, bit SEIP in mip is read-only and does
not include the value of bit 9 of mvip. Rather, the value of mip.SEIP
is simply the supervisor external interrupt signal from the hart’s
external interrupt controller (APLIC or IMSIC).
"""

From my understanding this means we should remove MIP_SEIP from the
alias mask.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2828
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/csr.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a75281539b..2a2f9497db 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3796,6 +3796,14 @@ static RISCVException rmw_mvip64(CPURISCVState *env, int csrno,
         /* Remove bits that are zero in both mideleg and mvien. */
         alias_mask &= (env->mideleg | env->mvien);
         nalias_mask &= (env->mideleg | env->mvien);
+    } else {
+        if (env->mvien & MIP_SEIP) {
+            /*
+             * Bit SEIP in mip is read-only and does not
+             * include the value of bit 9 of mvip
+             */
+            alias_mask &= ~MIP_SEIP;
+        }
     }
 
     /*
-- 
2.53.0


Re: [PATCH 3/5] target/riscv: Don't OR mip.SEIP when mvien is one
Posted by Chao Liu 3 days, 15 hours ago
On Tue, Apr 07, 2026 at 02:36:12PM +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> The RISC-V spec states that
> 
> """
> But when bit 9 of mvien is one, bit SEIP in mip is read-only and does
> not include the value of bit 9 of mvip. Rather, the value of mip.SEIP
> is simply the supervisor external interrupt signal from the hart’s
> external interrupt controller (APLIC or IMSIC).
> """
> 
> From my understanding this means we should remove MIP_SEIP from the
> alias mask.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2828
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/csr.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a75281539b..2a2f9497db 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3796,6 +3796,14 @@ static RISCVException rmw_mvip64(CPURISCVState *env, int csrno,
>          /* Remove bits that are zero in both mideleg and mvien. */
>          alias_mask &= (env->mideleg | env->mvien);
>          nalias_mask &= (env->mideleg | env->mvien);
> +    } else {
> +        if (env->mvien & MIP_SEIP) {
> +            /*
> +             * Bit SEIP in mip is read-only and does not
> +             * include the value of bit 9 of mvip
> +             */
> +            alias_mask &= ~MIP_SEIP;
This needs some discussion. I did a simple derivation of the
alias_mask initial value for bit 9 across all four combinations:

  alias_mask = ((S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
      (env->mideleg | ~env->mvien)) | MIP_STIP;

  mideleg[9] mvien[9] | ~mvien[9] mideleg|~mvien | alias[9] nalias[9]
  ---------- -------- + --------- -------------- + -------- ---------
       0        0     |     1           1        |    1        0
       0        1     |     0           0        |    0        1
       1        0     |     1           1        |    1        0
       1        1     |     0           1        |    1        0

Checking only mvien[9]=1 actually includes two cases:
 - combo 2: mideleg=0, mvien=1
 - combo 4: mideleg=1, mvien=1

Combo 2 already has alias_mask[9]=0, so the else branch is a
no-op there. The only effective case is combo 4, where
alias_mask[9] goes from 1 to 0. But nalias_mask[9] is also 0
for combo 4, so SEIP disappears from MVIP entirely.

Per the truth table in the function header:

  mideleg[i]=1, mvien[i]=X -> mvip[i] aliases mip[i]

Is there a special intent to break this alias for SEIP? Could
you clarify?

Per the AIA spec, I think the fix should be in rmw_mip64()
instead — make mip.SEIP read-only when mvien[9]=1, similar
to how sstc makes STIP read-only:

  static RISCVException rmw_mip64(CPURISCVState *env, int csrno,
                                  uint64_t *ret_val,
                                  uint64_t new_val, uint64_t wr_mask)
  {
      uint64_t old_mip, mask = wr_mask & delegable_ints;
      uint32_t gin;

  +   /*
  +    * When mvien[9]=1, mip.SEIP is read-only and reflects only
  +    * the external interrupt signal from the interrupt controller.
  +    */
  +   if (env->mvien & MIP_SEIP) {
  +       mask &= ~MIP_SEIP;
  +   }
  +
      if (mask & MIP_SEIP) {
          env->software_seip = new_val & MIP_SEIP;
          new_val |= env->external_seip * MIP_SEIP;
      }
      ...

Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

Thanks,
Chao
> +        }
>      }
>  
>      /*
> -- 
> 2.53.0
> 

Re: [PATCH 3/5] target/riscv: Don't OR mip.SEIP when mvien is one
Posted by Daniel Henrique Barboza 4 days, 8 hours ago

On 4/7/2026 1:36 AM, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> The RISC-V spec states that
> 
> """
> But when bit 9 of mvien is one, bit SEIP in mip is read-only and does
> not include the value of bit 9 of mvip. Rather, the value of mip.SEIP
> is simply the supervisor external interrupt signal from the hart’s
> external interrupt controller (APLIC or IMSIC).
> """
> 
>  From my understanding this means we should remove MIP_SEIP from the
> alias mask.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2828
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---


Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/csr.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a75281539b..2a2f9497db 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3796,6 +3796,14 @@ static RISCVException rmw_mvip64(CPURISCVState *env, int csrno,
>           /* Remove bits that are zero in both mideleg and mvien. */
>           alias_mask &= (env->mideleg | env->mvien);
>           nalias_mask &= (env->mideleg | env->mvien);
> +    } else {
> +        if (env->mvien & MIP_SEIP) {
> +            /*
> +             * Bit SEIP in mip is read-only and does not
> +             * include the value of bit 9 of mvip
> +             */
> +            alias_mask &= ~MIP_SEIP;
> +        }
>       }
>   
>       /*