Hi Shuai,

On Mon, Aug 24, 2026 at 7:09 PM Gong Shuai <[email protected]> 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 <[email protected]>
>
>
> 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?

Yes, this change is required. I will add it in v2.

Thanks,
Mayuresh.
>
> --- 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

Reply via email to