1. Patchew
  2. QEMU
  3. View series

[PATCH] target/riscv: Fix LCOFI masking in sie/sip/mvip

Mayuresh Chitale posted 1 patch 1 week, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260813134002.862578-1-mayuresh.chitale@oss.qualcomm.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.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@processmission.com>
target/riscv/tcg/csr.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
[PATCH] target/riscv: Fix LCOFI masking in sie/sip/mvip
Posted by Mayuresh Chitale 1 week, 6 days ago 
Commit 27f9566dcd98 ("target/riscv: Update the local interrupt mask")
dropped bit 13 (MIP_LCOFIP) from the supervisor-visible interrupt masks.
As a result, with Sscofpmf enabled and LCOFIP delegated to S-mode, the
overflow interrupt would be raised in mip but never delivered and hence
perf sampling stopped receiving overflow interrupts.

Re-add MIP_LCOFIP to all_ints, sip_writable_mask, mvip_writable_mask
to allow it to be writable again.

Fixes: 27f9566dcd98 ("target/riscv: Update the local interrupt mask")
Signed-off-by: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>
---
 target/riscv/tcg/csr.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c
index c05c8ee076..ca68c1f5f1 100644
--- a/target/riscv/tcg/csr.c
+++ b/target/riscv/tcg/csr.c
@@ -1890,7 +1890,8 @@ static const uint64_t delegable_ints =
 static const uint64_t vs_delegable_ints =
     (VS_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & ~MIP_LCOFIP;
 static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
-                                     HS_MODE_INTERRUPTS | LOCAL_INTERRUPTS;
+                                     HS_MODE_INTERRUPTS | MIP_LCOFIP |
+                                     LOCAL_INTERRUPTS;
 #define DELEGABLE_EXCPS ((1ULL << (RISCV_EXCP_INST_ADDR_MIS)) | \
                          (1ULL << (RISCV_EXCP_INST_ACCESS_FAULT)) | \
                          (1ULL << (RISCV_EXCP_ILLEGAL_INST)) | \
@@ -1936,11 +1937,12 @@ static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
 
 /* Bit STIP can be an alias of mip.STIP that's why it's writable in mvip. */
 static const uint64_t mvip_writable_mask = MIP_SSIP | MIP_STIP | MIP_SEIP |
-                                    LOCAL_INTERRUPTS;
+                                    MIP_LCOFIP | LOCAL_INTERRUPTS;
 static const uint64_t mvien_writable_mask = MIP_SSIP | MIP_SEIP |
                                     LOCAL_INTERRUPTS;
 
-static const uint64_t sip_writable_mask = SIP_SSIP | LOCAL_INTERRUPTS;
+static const uint64_t sip_writable_mask = SIP_SSIP | SIP_LCOFIP |
+                                          LOCAL_INTERRUPTS;
 static const uint64_t hip_writable_mask = MIP_VSSIP;
 static const uint64_t hvip_writable_mask = MIP_VSSIP | MIP_VSTIP |
                                     MIP_VSEIP | LOCAL_INTERRUPTS;
@@ -3953,9 +3955,9 @@ static RISCVException rmw_mvip64(CPURISCVState *env, int csrno,
      *  alias_mask denotes the bits that come from mip nalias_mask denotes bits
      *  that come from hvip.
      */
-    uint64_t alias_mask = ((S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
+    uint64_t alias_mask = ((S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
         (env->mideleg | ~env->mvien)) | MIP_STIP;
-    uint64_t nalias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
+    uint64_t nalias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
         (~env->mideleg & env->mvien);
     uint64_t wr_mask_mvip;
     uint64_t wr_mask_mip;
@@ -4188,9 +4190,10 @@ static RISCVException rmw_sie64(CPURISCVState *env, int csrno,
                                 uint64_t *ret_val,
                                 uint64_t new_val, uint64_t wr_mask)
 {
-    uint64_t nalias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
+    uint64_t nalias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
         (~env->mideleg & env->mvien);
-    uint64_t alias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & env->mideleg;
+    uint64_t alias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
+        env->mideleg;
     uint64_t sie_mask = wr_mask & nalias_mask;
     RISCVException ret;
 
-- 
2.43.0
Re: [PATCH] target/riscv: Fix LCOFI masking in sie/sip/mvip
Posted by Gong Shuai 2 days, 8 hours ago 
> Commit 27f9566dcd98 ("target/riscv: Update the local interrupt mask")
> dropped bit 13 (MIP_LCOFIP) from the supervisor-visible interrupt masks.
> As a result, with Sscofpmf enabled and LCOFIP delegated to S-mode, the
> overflow interrupt would be raised in mip but never delivered and hence
> perf sampling stopped receiving overflow interrupts.
>
> Re-add MIP_LCOFIP to all_ints, sip_writable_mask, mvip_writable_mask
> to allow it to be writable again.
>
> Fixes: 27f9566dcd98 ("target/riscv: Update the local interrupt mask")
> Signed-off-by: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>


Hi Mayuresh,

I applied your patch on top of QEMU v11.1.0 and it fixes the perf sampling
regression.

While the read path is still inconsistent with the write path:

In rmw_sip64(), the read-back mask still truncates the result with
(S_MODE_INTERRUPTS | LOCAL_INTERRUPTS), which does not include bit 13.
As a result, sip.LCOFIP always reads 0 even when mideleg.LCOFIP is set
and mip.LCOFIP is pending. This contradicts the rule that sip[i] aliases
mip[i] for bits delegated to S-mode, and it breaks consumers that check
sip.LCOFIP.

Would the following one-line change be suitable as a follow-up?

--- a/target/riscv/tcg/csr.c
+++ b/target/riscv/tcg/csr.c
@@ -4378,7 +4378,8 @@ static RISCVException rmw_sip64(CPURISCVState *env, int csrno,
  if (ret_val) {
 *ret_val &= (env->mideleg | env->mvien) &
-            (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS);
+            (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS | MIP_LCOFIP);
  }

  return ret;
}

Thanks for the fix and best regards,
Shuai


> ---
>  target/riscv/tcg/csr.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c
> index c05c8ee076..ca68c1f5f1 100644
> --- a/target/riscv/tcg/csr.c
> +++ b/target/riscv/tcg/csr.c
> @@ -1890,7 +1890,8 @@ static const uint64_t delegable_ints =
>  static const uint64_t vs_delegable_ints =
>      (VS_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & ~MIP_LCOFIP;
>  static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
> -                                     HS_MODE_INTERRUPTS | LOCAL_INTERRUPTS;
> +                                     HS_MODE_INTERRUPTS | MIP_LCOFIP |
> +                                     LOCAL_INTERRUPTS;
>  #define DELEGABLE_EXCPS ((1ULL << (RISCV_EXCP_INST_ADDR_MIS)) | \
>                           (1ULL << (RISCV_EXCP_INST_ACCESS_FAULT)) | \
>                           (1ULL << (RISCV_EXCP_ILLEGAL_INST)) | \
> @@ -1936,11 +1937,12 @@ static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
>  
>  /* Bit STIP can be an alias of mip.STIP that's why it's writable in mvip. */
>  static const uint64_t mvip_writable_mask = MIP_SSIP | MIP_STIP | MIP_SEIP |
> -                                    LOCAL_INTERRUPTS;
> +                                    MIP_LCOFIP | LOCAL_INTERRUPTS;
>  static const uint64_t mvien_writable_mask = MIP_SSIP | MIP_SEIP |
>                                      LOCAL_INTERRUPTS;
>  
> -static const uint64_t sip_writable_mask = SIP_SSIP | LOCAL_INTERRUPTS;
> +static const uint64_t sip_writable_mask = SIP_SSIP | SIP_LCOFIP |
> +                                          LOCAL_INTERRUPTS;
>  static const uint64_t hip_writable_mask = MIP_VSSIP;
>  static const uint64_t hvip_writable_mask = MIP_VSSIP | MIP_VSTIP |
>                                      MIP_VSEIP | LOCAL_INTERRUPTS;
> @@ -3953,9 +3955,9 @@ static RISCVException rmw_mvip64(CPURISCVState *env, int csrno,
>       *  alias_mask denotes the bits that come from mip nalias_mask denotes bits
>       *  that come from hvip.
>       */
> -    uint64_t alias_mask = ((S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
> +    uint64_t alias_mask = ((S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
>          (env->mideleg | ~env->mvien)) | MIP_STIP;
> -    uint64_t nalias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
> +    uint64_t nalias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
>          (~env->mideleg & env->mvien);
>      uint64_t wr_mask_mvip;
>      uint64_t wr_mask_mip;
> @@ -4188,9 +4190,10 @@ static RISCVException rmw_sie64(CPURISCVState *env, int csrno,
>                                  uint64_t *ret_val,
>                                  uint64_t new_val, uint64_t wr_mask)
>  {
> -    uint64_t nalias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
> +    uint64_t nalias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
>          (~env->mideleg & env->mvien);
> -    uint64_t alias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & env->mideleg;
> +    uint64_t alias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
> +        env->mideleg;
>      uint64_t sie_mask = wr_mask & nalias_mask;
>      RISCVException ret;
>  
> --
> 2.43.0
Re: [PATCH] target/riscv: Fix LCOFI masking in sie/sip/mvip
Posted by Daniel Henrique Barboza 2 days, 9 hours ago 

On 8/13/2026 10:40 AM, Mayuresh Chitale wrote:
> Commit 27f9566dcd98 ("target/riscv: Update the local interrupt mask")
> dropped bit 13 (MIP_LCOFIP) from the supervisor-visible interrupt masks.
> As a result, with Sscofpmf enabled and LCOFIP delegated to S-mode, the
> overflow interrupt would be raised in mip but never delivered and hence
> perf sampling stopped receiving overflow interrupts.
> 
> Re-add MIP_LCOFIP to all_ints, sip_writable_mask, mvip_writable_mask
> to allow it to be writable again.
> 
> Fixes: 27f9566dcd98 ("target/riscv: Update the local interrupt mask")
> Signed-off-by: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>
> ---

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

>   target/riscv/tcg/csr.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c
> index c05c8ee076..ca68c1f5f1 100644
> --- a/target/riscv/tcg/csr.c
> +++ b/target/riscv/tcg/csr.c
> @@ -1890,7 +1890,8 @@ static const uint64_t delegable_ints =
>   static const uint64_t vs_delegable_ints =
>       (VS_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & ~MIP_LCOFIP;
>   static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
> -                                     HS_MODE_INTERRUPTS | LOCAL_INTERRUPTS;
> +                                     HS_MODE_INTERRUPTS | MIP_LCOFIP |
> +                                     LOCAL_INTERRUPTS;
>   #define DELEGABLE_EXCPS ((1ULL << (RISCV_EXCP_INST_ADDR_MIS)) | \
>                            (1ULL << (RISCV_EXCP_INST_ACCESS_FAULT)) | \
>                            (1ULL << (RISCV_EXCP_ILLEGAL_INST)) | \
> @@ -1936,11 +1937,12 @@ static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
>   
>   /* Bit STIP can be an alias of mip.STIP that's why it's writable in mvip. */
>   static const uint64_t mvip_writable_mask = MIP_SSIP | MIP_STIP | MIP_SEIP |
> -                                    LOCAL_INTERRUPTS;
> +                                    MIP_LCOFIP | LOCAL_INTERRUPTS;
>   static const uint64_t mvien_writable_mask = MIP_SSIP | MIP_SEIP |
>                                       LOCAL_INTERRUPTS;
>   
> -static const uint64_t sip_writable_mask = SIP_SSIP | LOCAL_INTERRUPTS;
> +static const uint64_t sip_writable_mask = SIP_SSIP | SIP_LCOFIP |
> +                                          LOCAL_INTERRUPTS;
>   static const uint64_t hip_writable_mask = MIP_VSSIP;
>   static const uint64_t hvip_writable_mask = MIP_VSSIP | MIP_VSTIP |
>                                       MIP_VSEIP | LOCAL_INTERRUPTS;
> @@ -3953,9 +3955,9 @@ static RISCVException rmw_mvip64(CPURISCVState *env, int csrno,
>        *  alias_mask denotes the bits that come from mip nalias_mask denotes bits
>        *  that come from hvip.
>        */
> -    uint64_t alias_mask = ((S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
> +    uint64_t alias_mask = ((S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
>           (env->mideleg | ~env->mvien)) | MIP_STIP;
> -    uint64_t nalias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
> +    uint64_t nalias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
>           (~env->mideleg & env->mvien);
>       uint64_t wr_mask_mvip;
>       uint64_t wr_mask_mip;
> @@ -4188,9 +4190,10 @@ static RISCVException rmw_sie64(CPURISCVState *env, int csrno,
>                                   uint64_t *ret_val,
>                                   uint64_t new_val, uint64_t wr_mask)
>   {
> -    uint64_t nalias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) &
> +    uint64_t nalias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
>           (~env->mideleg & env->mvien);
> -    uint64_t alias_mask = (S_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & env->mideleg;
> +    uint64_t alias_mask = (S_MODE_INTERRUPTS | MIP_LCOFIP | LOCAL_INTERRUPTS) &
> +        env->mideleg;
>       uint64_t sie_mask = wr_mask & nalias_mask;
>       RISCVException ret;
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.