diff mbox series

[RFC,5/9] lib:sbi: Support sscof extension in OpenSBI

Message ID 20210909204031.1239254-6-atish.patra@wdc.com
State Superseded
Headers show
Series Sscof extension support | expand

Commit Message

Atish Patra Sept. 9, 2021, 8:40 p.m. UTC
This patch adds sscof extension in pmu module which includes
following things.

1. Enable overflow irq when starting a counter.
2. Setting the correct event filters passed from supervisor.
3. Delegating the overflow interrupt to the supervisor.
4. Add RV32 support for sscof.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 include/sbi/riscv_encoding.h | 19 ++++++++
 lib/sbi/sbi_pmu.c            | 85 ++++++++++++++++++++++++++++++------
 2 files changed, 90 insertions(+), 14 deletions(-)

Comments

Anup Patel Sept. 23, 2021, 5:42 a.m. UTC | #1
On Fri, Sep 10, 2021 at 2:10 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> This patch adds sscof extension in pmu module which includes
> following things.
>
> 1. Enable overflow irq when starting a counter.
> 2. Setting the correct event filters passed from supervisor.
> 3. Delegating the overflow interrupt to the supervisor.
> 4. Add RV32 support for sscof.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
>  include/sbi/riscv_encoding.h | 19 ++++++++
>  lib/sbi/sbi_pmu.c            | 85 ++++++++++++++++++++++++++++++------
>  2 files changed, 90 insertions(+), 14 deletions(-)
>
> diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
> index c2cf0f4d69f4..f71412eae563 100644
> --- a/include/sbi/riscv_encoding.h
> +++ b/include/sbi/riscv_encoding.h
> @@ -173,6 +173,25 @@
>  #define HGATP_MODE_SHIFT               HGATP32_MODE_SHIFT
>  #endif
>
> +#if __riscv_xlen == 64
> +#define MHPMEVENT_OF                   (_UL(1) << 63)
> +#define MHPMEVENT_MINH                 (_UL(1) << 62)
> +#define MHPMEVENT_SINH                 (_UL(1) << 61)
> +#define MHPMEVENT_UINH                 (_UL(1) << 60)
> +#define MHPMEVENT_VSINH                        (_UL(1) << 59)
> +#define MHPMEVENT_VUINH                        (_UL(1) << 58)
> +#else
> +#define MHPMEVENT_OF                   (_ULL(1) << 63)
> +#define MHPMEVENT_MINH                 (_ULL(1) << 62)
> +#define MHPMEVENT_SINH                 (_ULL(1) << 61)
> +#define MHPMEVENT_UINH                 (_ULL(1) << 60)
> +#define MHPMEVENT_VSINH                        (_ULL(1) << 59)
> +#define MHPMEVENT_VUINH                        (_ULL(1) << 58)

These defines are misleading because filter bits are in MHPMEVENTHx CSRs.

Try to keep the riscv_encoding.h closer to the priv spec.

> +
> +#define MHPMEVENTH_OF                  (_UL(1) << 31)
> +#endif
> +
> +#define MHPMEVENT_SSCOF_MASK           _ULL(0xFFFF000000000000)
>  /* ===== User-level CSRs ===== */
>
>  /* User Trap Setup (N-extension) */
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index c276a46e2876..bbbd033b4cfe 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -226,11 +226,47 @@ int sbi_pmu_add_raw_event_counter_map(uint64_t select, u32 cmap)
>                                     SBI_PMU_EVENT_RAW_IDX, cmap, select);
>  }
>
> +static int pmu_ctr_enable_irq_hw(int ctr_idx)
> +{
> +       unsigned long mhpmevent_csr;
> +       unsigned long mhpmevent_curr;
> +       unsigned long mip_val;
> +       unsigned long of_mask;
> +
> +       if (ctr_idx < 3 || ctr_idx >= SBI_PMU_HW_CTR_MAX)
> +               return SBI_EFAIL;
> +
> +#if __riscv_xlen == 32
> +       mhpmevent_csr = CSR_MHPMEVENT3H  + ctr_idx - 3;
> +       of_mask = ~MHPMEVENTH_OF;
> +#else
> +       mhpmevent_csr = CSR_MCOUNTINHIBIT + ctr_idx;
> +       of_mask = ~MHPMEVENT_OF;
> +#endif
> +
> +       mhpmevent_curr = csr_read_num(mhpmevent_csr);
> +       mip_val = csr_read(CSR_MIP);
> +       /**
> +        * Clear out the OF bit so that next interrupt can be enabled.
> +        * This should be done only when the corresponding overflow interrupt
> +        * bit is cleared. That indicates that software has already handled the
> +        * previous interrupts or the hardware yet to set an overflow interrupt.
> +        * Otherwise, there will be race conditions where we may clear the bit
> +        * the software is yet to handle the interrupt.
> +        */
> +       if (!(mip_val & MIP_LCOFIP)) {
> +               mhpmevent_curr &= of_mask;
> +               csr_write_num(mhpmevent_csr, mhpmevent_curr);
> +       }

This needs to change as well.

We should program mhpmenventh CSRs under "#ifdef" for RV32.

> +
> +       return 0;
> +}
> +
>  static void pmu_ctr_write_hw(uint32_t cidx, uint64_t ival)
>  {
>  #if __riscv_xlen == 32
>         csr_write_num(CSR_MCYCLE + cidx, 0);
> -       csr_write_num(CSR_MCYCLE + cidx, ival & 0xFFFF);
> +       csr_write_num(CSR_MCYCLE + cidx, ival & 0xFFFFFFFF);
>         csr_write_num(CSR_MCYCLEH + cidx, ival >> BITS_PER_LONG);
>  #else
>         csr_write_num(CSR_MCYCLE + cidx, ival);
> @@ -252,12 +288,14 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
>         __set_bit(cidx, &mctr_en);
>         __clear_bit(cidx, &mctr_inhbt);
>
> -       if (ival_update)
> -               pmu_ctr_write_hw(cidx, ival);
> -
> +       if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOF))
> +               pmu_ctr_enable_irq_hw(cidx);
>         csr_write(CSR_MCOUNTEREN, mctr_en);
>         csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
>
> +       if (ival_update)
> +               pmu_ctr_write_hw(cidx, ival);
> +
>         return 0;
>  }
>
> @@ -326,7 +364,6 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
>  static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t fw_evt_code)
>  {
>         u32 hartid = current_hartid();
> -
>         fw_event_map[hartid][fw_evt_code].bStarted = FALSE;
>
>         return 0;
> @@ -362,8 +399,21 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
>         return ret;
>  }
>
> +static void pmu_update_inhibit_flags(unsigned long flags, uint64_t *mhpmevent_val)
> +{
> +       if (flags & SBI_PMU_CFG_FLAG_SET_VUINH)
> +               *mhpmevent_val |= MHPMEVENT_VUINH;
> +       if (flags & SBI_PMU_CFG_FLAG_SET_VSINH)
> +               *mhpmevent_val |= MHPMEVENT_VSINH;
> +       if (flags & SBI_PMU_CFG_FLAG_SET_UINH)
> +               *mhpmevent_val |= MHPMEVENT_UINH;
> +       if (flags & SBI_PMU_CFG_FLAG_SET_SINH)
> +               *mhpmevent_val |= MHPMEVENT_SINH;
> +}
> +
>  static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
> -                                   unsigned long eindex, uint64_t data)
> +                                  unsigned long flags, unsigned long eindex,
> +                                  uint64_t data)
>  {
>         struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>         const struct sbi_platform *plat = sbi_platform_ptr(scratch);
> @@ -375,17 +425,23 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
>         if (!mhpmevent_val || ctr_idx < 3 || ctr_idx >= SBI_PMU_HW_CTR_MAX)
>                 return SBI_EFAIL;
>
> -       /* TODO: The upper 16 bits of mhpmevent is reserved by sscofpmf extension.
> -        * Update those bits based on the flags received from supervisor.
> -        * The OVF bit also should be cleared here in case it was not cleared
> -        * during event stop.
> -        */
> +       /* Always clear the OVF bit and inhibit countin of events in M-mode */
> +       mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) | MHPMEVENT_MINH;

I think this line has more than 80 characters.

> +
> +       /* Update the inhibit flags based on inhibit flags received from supervisor */
> +       pmu_update_inhibit_flags(flags, &mhpmevent_val);
> +
> +#if __riscv_xlen == 32
> +       csr_write_num(CSR_MCOUNTINHIBIT + ctr_idx, mhpmevent_val & 0xFFFFFFFF);
> +       csr_write_num(CSR_MHPMEVENT3H + ctr_idx - 3, mhpmevent_val >> BITS_PER_LONG);
> +#else
>         csr_write_num(CSR_MCOUNTINHIBIT + ctr_idx, mhpmevent_val);
> +#endif
>
>         return 0;
>  }
>
> -static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask,
> +static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned long flags,
>                            unsigned long event_idx, uint64_t data)
>  {
>         unsigned long ctr_mask;
> @@ -427,7 +483,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask,
>         if (ctr_idx == SBI_ENOTSUPP)
>                 return SBI_EFAIL;
>
> -       ret = pmu_update_hw_mhpmevent(temp, ctr_idx, event_idx, data);
> +       ret = pmu_update_hw_mhpmevent(temp, ctr_idx, flags, event_idx, data);
>
>         if (!ret)
>                 ret = ctr_idx;
> @@ -490,7 +546,8 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
>                 /* Any firmware counter can be used track any firmware event */
>                 ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, hartid);
>         } else {
> -               ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, event_idx, event_data);
> +               ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
> +                                         event_data);
>         }
>
>         if (ctr_idx < 0)
> --
> 2.31.1
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Regards,
Anup
Anup Patel Sept. 24, 2021, 3:45 p.m. UTC | #2
On Thu, Sep 23, 2021 at 11:12 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Fri, Sep 10, 2021 at 2:10 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > This patch adds sscof extension in pmu module which includes
> > following things.
> >
> > 1. Enable overflow irq when starting a counter.
> > 2. Setting the correct event filters passed from supervisor.
> > 3. Delegating the overflow interrupt to the supervisor.
> > 4. Add RV32 support for sscof.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> >  include/sbi/riscv_encoding.h | 19 ++++++++
> >  lib/sbi/sbi_pmu.c            | 85 ++++++++++++++++++++++++++++++------
> >  2 files changed, 90 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
> > index c2cf0f4d69f4..f71412eae563 100644
> > --- a/include/sbi/riscv_encoding.h
> > +++ b/include/sbi/riscv_encoding.h
> > @@ -173,6 +173,25 @@
> >  #define HGATP_MODE_SHIFT               HGATP32_MODE_SHIFT
> >  #endif
> >
> > +#if __riscv_xlen == 64
> > +#define MHPMEVENT_OF                   (_UL(1) << 63)
> > +#define MHPMEVENT_MINH                 (_UL(1) << 62)
> > +#define MHPMEVENT_SINH                 (_UL(1) << 61)
> > +#define MHPMEVENT_UINH                 (_UL(1) << 60)
> > +#define MHPMEVENT_VSINH                        (_UL(1) << 59)
> > +#define MHPMEVENT_VUINH                        (_UL(1) << 58)
> > +#else
> > +#define MHPMEVENT_OF                   (_ULL(1) << 63)
> > +#define MHPMEVENT_MINH                 (_ULL(1) << 62)
> > +#define MHPMEVENT_SINH                 (_ULL(1) << 61)
> > +#define MHPMEVENT_UINH                 (_ULL(1) << 60)
> > +#define MHPMEVENT_VSINH                        (_ULL(1) << 59)
> > +#define MHPMEVENT_VUINH                        (_ULL(1) << 58)
>
> These defines are misleading because filter bits are in MHPMEVENTHx CSRs.
>
> Try to keep the riscv_encoding.h closer to the priv spec.
>
> > +
> > +#define MHPMEVENTH_OF                  (_UL(1) << 31)
> > +#endif
> > +
> > +#define MHPMEVENT_SSCOF_MASK           _ULL(0xFFFF000000000000)
> >  /* ===== User-level CSRs ===== */
> >
> >  /* User Trap Setup (N-extension) */
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index c276a46e2876..bbbd033b4cfe 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -226,11 +226,47 @@ int sbi_pmu_add_raw_event_counter_map(uint64_t select, u32 cmap)
> >                                     SBI_PMU_EVENT_RAW_IDX, cmap, select);
> >  }
> >
> > +static int pmu_ctr_enable_irq_hw(int ctr_idx)
> > +{
> > +       unsigned long mhpmevent_csr;
> > +       unsigned long mhpmevent_curr;
> > +       unsigned long mip_val;
> > +       unsigned long of_mask;
> > +
> > +       if (ctr_idx < 3 || ctr_idx >= SBI_PMU_HW_CTR_MAX)
> > +               return SBI_EFAIL;
> > +
> > +#if __riscv_xlen == 32
> > +       mhpmevent_csr = CSR_MHPMEVENT3H  + ctr_idx - 3;
> > +       of_mask = ~MHPMEVENTH_OF;
> > +#else
> > +       mhpmevent_csr = CSR_MCOUNTINHIBIT + ctr_idx;
> > +       of_mask = ~MHPMEVENT_OF;
> > +#endif
> > +
> > +       mhpmevent_curr = csr_read_num(mhpmevent_csr);
> > +       mip_val = csr_read(CSR_MIP);
> > +       /**
> > +        * Clear out the OF bit so that next interrupt can be enabled.
> > +        * This should be done only when the corresponding overflow interrupt
> > +        * bit is cleared. That indicates that software has already handled the
> > +        * previous interrupts or the hardware yet to set an overflow interrupt.
> > +        * Otherwise, there will be race conditions where we may clear the bit
> > +        * the software is yet to handle the interrupt.
> > +        */
> > +       if (!(mip_val & MIP_LCOFIP)) {
> > +               mhpmevent_curr &= of_mask;
> > +               csr_write_num(mhpmevent_csr, mhpmevent_curr);
> > +       }
>
> This needs to change as well.
>
> We should program mhpmenventh CSRs under "#ifdef" for RV32.

Ignore this comment. I misread csr_write_num() as csr_write().

Regards,
Anup

>
> > +
> > +       return 0;
> > +}
> > +
> >  static void pmu_ctr_write_hw(uint32_t cidx, uint64_t ival)
> >  {
> >  #if __riscv_xlen == 32
> >         csr_write_num(CSR_MCYCLE + cidx, 0);
> > -       csr_write_num(CSR_MCYCLE + cidx, ival & 0xFFFF);
> > +       csr_write_num(CSR_MCYCLE + cidx, ival & 0xFFFFFFFF);
> >         csr_write_num(CSR_MCYCLEH + cidx, ival >> BITS_PER_LONG);
> >  #else
> >         csr_write_num(CSR_MCYCLE + cidx, ival);
> > @@ -252,12 +288,14 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
> >         __set_bit(cidx, &mctr_en);
> >         __clear_bit(cidx, &mctr_inhbt);
> >
> > -       if (ival_update)
> > -               pmu_ctr_write_hw(cidx, ival);
> > -
> > +       if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOF))
> > +               pmu_ctr_enable_irq_hw(cidx);
> >         csr_write(CSR_MCOUNTEREN, mctr_en);
> >         csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
> >
> > +       if (ival_update)
> > +               pmu_ctr_write_hw(cidx, ival);
> > +
> >         return 0;
> >  }
> >
> > @@ -326,7 +364,6 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
> >  static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t fw_evt_code)
> >  {
> >         u32 hartid = current_hartid();
> > -
> >         fw_event_map[hartid][fw_evt_code].bStarted = FALSE;
> >
> >         return 0;
> > @@ -362,8 +399,21 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
> >         return ret;
> >  }
> >
> > +static void pmu_update_inhibit_flags(unsigned long flags, uint64_t *mhpmevent_val)
> > +{
> > +       if (flags & SBI_PMU_CFG_FLAG_SET_VUINH)
> > +               *mhpmevent_val |= MHPMEVENT_VUINH;
> > +       if (flags & SBI_PMU_CFG_FLAG_SET_VSINH)
> > +               *mhpmevent_val |= MHPMEVENT_VSINH;
> > +       if (flags & SBI_PMU_CFG_FLAG_SET_UINH)
> > +               *mhpmevent_val |= MHPMEVENT_UINH;
> > +       if (flags & SBI_PMU_CFG_FLAG_SET_SINH)
> > +               *mhpmevent_val |= MHPMEVENT_SINH;
> > +}
> > +
> >  static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
> > -                                   unsigned long eindex, uint64_t data)
> > +                                  unsigned long flags, unsigned long eindex,
> > +                                  uint64_t data)
> >  {
> >         struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >         const struct sbi_platform *plat = sbi_platform_ptr(scratch);
> > @@ -375,17 +425,23 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
> >         if (!mhpmevent_val || ctr_idx < 3 || ctr_idx >= SBI_PMU_HW_CTR_MAX)
> >                 return SBI_EFAIL;
> >
> > -       /* TODO: The upper 16 bits of mhpmevent is reserved by sscofpmf extension.
> > -        * Update those bits based on the flags received from supervisor.
> > -        * The OVF bit also should be cleared here in case it was not cleared
> > -        * during event stop.
> > -        */
> > +       /* Always clear the OVF bit and inhibit countin of events in M-mode */
> > +       mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) | MHPMEVENT_MINH;
>
> I think this line has more than 80 characters.
>
> > +
> > +       /* Update the inhibit flags based on inhibit flags received from supervisor */
> > +       pmu_update_inhibit_flags(flags, &mhpmevent_val);
> > +
> > +#if __riscv_xlen == 32
> > +       csr_write_num(CSR_MCOUNTINHIBIT + ctr_idx, mhpmevent_val & 0xFFFFFFFF);
> > +       csr_write_num(CSR_MHPMEVENT3H + ctr_idx - 3, mhpmevent_val >> BITS_PER_LONG);
> > +#else
> >         csr_write_num(CSR_MCOUNTINHIBIT + ctr_idx, mhpmevent_val);
> > +#endif
> >
> >         return 0;
> >  }
> >
> > -static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask,
> > +static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned long flags,
> >                            unsigned long event_idx, uint64_t data)
> >  {
> >         unsigned long ctr_mask;
> > @@ -427,7 +483,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask,
> >         if (ctr_idx == SBI_ENOTSUPP)
> >                 return SBI_EFAIL;
> >
> > -       ret = pmu_update_hw_mhpmevent(temp, ctr_idx, event_idx, data);
> > +       ret = pmu_update_hw_mhpmevent(temp, ctr_idx, flags, event_idx, data);
> >
> >         if (!ret)
> >                 ret = ctr_idx;
> > @@ -490,7 +546,8 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
> >                 /* Any firmware counter can be used track any firmware event */
> >                 ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, hartid);
> >         } else {
> > -               ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, event_idx, event_data);
> > +               ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
> > +                                         event_data);
> >         }
> >
> >         if (ctr_idx < 0)
> > --
> > 2.31.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
>
> Regards,
> Anup
diff mbox series

Patch

diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index c2cf0f4d69f4..f71412eae563 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -173,6 +173,25 @@ 
 #define HGATP_MODE_SHIFT		HGATP32_MODE_SHIFT
 #endif
 
+#if __riscv_xlen == 64
+#define MHPMEVENT_OF			(_UL(1) << 63)
+#define MHPMEVENT_MINH			(_UL(1) << 62)
+#define MHPMEVENT_SINH			(_UL(1) << 61)
+#define MHPMEVENT_UINH			(_UL(1) << 60)
+#define MHPMEVENT_VSINH			(_UL(1) << 59)
+#define MHPMEVENT_VUINH			(_UL(1) << 58)
+#else
+#define MHPMEVENT_OF			(_ULL(1) << 63)
+#define MHPMEVENT_MINH			(_ULL(1) << 62)
+#define MHPMEVENT_SINH			(_ULL(1) << 61)
+#define MHPMEVENT_UINH			(_ULL(1) << 60)
+#define MHPMEVENT_VSINH			(_ULL(1) << 59)
+#define MHPMEVENT_VUINH			(_ULL(1) << 58)
+
+#define MHPMEVENTH_OF			(_UL(1) << 31)
+#endif
+
+#define MHPMEVENT_SSCOF_MASK		_ULL(0xFFFF000000000000)
 /* ===== User-level CSRs ===== */
 
 /* User Trap Setup (N-extension) */
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index c276a46e2876..bbbd033b4cfe 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -226,11 +226,47 @@  int sbi_pmu_add_raw_event_counter_map(uint64_t select, u32 cmap)
 				    SBI_PMU_EVENT_RAW_IDX, cmap, select);
 }
 
+static int pmu_ctr_enable_irq_hw(int ctr_idx)
+{
+	unsigned long mhpmevent_csr;
+	unsigned long mhpmevent_curr;
+	unsigned long mip_val;
+	unsigned long of_mask;
+
+	if (ctr_idx < 3 || ctr_idx >= SBI_PMU_HW_CTR_MAX)
+		return SBI_EFAIL;
+
+#if __riscv_xlen == 32
+	mhpmevent_csr = CSR_MHPMEVENT3H  + ctr_idx - 3;
+	of_mask = ~MHPMEVENTH_OF;
+#else
+	mhpmevent_csr = CSR_MCOUNTINHIBIT + ctr_idx;
+	of_mask = ~MHPMEVENT_OF;
+#endif
+
+	mhpmevent_curr = csr_read_num(mhpmevent_csr);
+	mip_val = csr_read(CSR_MIP);
+	/**
+	 * Clear out the OF bit so that next interrupt can be enabled.
+	 * This should be done only when the corresponding overflow interrupt
+	 * bit is cleared. That indicates that software has already handled the
+	 * previous interrupts or the hardware yet to set an overflow interrupt.
+	 * Otherwise, there will be race conditions where we may clear the bit
+	 * the software is yet to handle the interrupt.
+	 */
+	if (!(mip_val & MIP_LCOFIP)) {
+		mhpmevent_curr &= of_mask;
+		csr_write_num(mhpmevent_csr, mhpmevent_curr);
+	}
+
+	return 0;
+}
+
 static void pmu_ctr_write_hw(uint32_t cidx, uint64_t ival)
 {
 #if __riscv_xlen == 32
 	csr_write_num(CSR_MCYCLE + cidx, 0);
-	csr_write_num(CSR_MCYCLE + cidx, ival & 0xFFFF);
+	csr_write_num(CSR_MCYCLE + cidx, ival & 0xFFFFFFFF);
 	csr_write_num(CSR_MCYCLEH + cidx, ival >> BITS_PER_LONG);
 #else
 	csr_write_num(CSR_MCYCLE + cidx, ival);
@@ -252,12 +288,14 @@  static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
 	__set_bit(cidx, &mctr_en);
 	__clear_bit(cidx, &mctr_inhbt);
 
-	if (ival_update)
-		pmu_ctr_write_hw(cidx, ival);
-
+	if (sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOF))
+		pmu_ctr_enable_irq_hw(cidx);
 	csr_write(CSR_MCOUNTEREN, mctr_en);
 	csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
 
+	if (ival_update)
+		pmu_ctr_write_hw(cidx, ival);
+
 	return 0;
 }
 
@@ -326,7 +364,6 @@  static int pmu_ctr_stop_hw(uint32_t cidx)
 static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t fw_evt_code)
 {
 	u32 hartid = current_hartid();
-
 	fw_event_map[hartid][fw_evt_code].bStarted = FALSE;
 
 	return 0;
@@ -362,8 +399,21 @@  int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
 	return ret;
 }
 
+static void pmu_update_inhibit_flags(unsigned long flags, uint64_t *mhpmevent_val)
+{
+	if (flags & SBI_PMU_CFG_FLAG_SET_VUINH)
+		*mhpmevent_val |= MHPMEVENT_VUINH;
+	if (flags & SBI_PMU_CFG_FLAG_SET_VSINH)
+		*mhpmevent_val |= MHPMEVENT_VSINH;
+	if (flags & SBI_PMU_CFG_FLAG_SET_UINH)
+		*mhpmevent_val |= MHPMEVENT_UINH;
+	if (flags & SBI_PMU_CFG_FLAG_SET_SINH)
+		*mhpmevent_val |= MHPMEVENT_SINH;
+}
+
 static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
-				    unsigned long eindex, uint64_t data)
+				   unsigned long flags, unsigned long eindex,
+				   uint64_t data)
 {
 	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 	const struct sbi_platform *plat = sbi_platform_ptr(scratch);
@@ -375,17 +425,23 @@  static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
 	if (!mhpmevent_val || ctr_idx < 3 || ctr_idx >= SBI_PMU_HW_CTR_MAX)
 		return SBI_EFAIL;
 
-	/* TODO: The upper 16 bits of mhpmevent is reserved by sscofpmf extension.
-	 * Update those bits based on the flags received from supervisor.
-	 * The OVF bit also should be cleared here in case it was not cleared
-	 * during event stop.
-	 */
+	/* Always clear the OVF bit and inhibit countin of events in M-mode */
+	mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) | MHPMEVENT_MINH;
+
+	/* Update the inhibit flags based on inhibit flags received from supervisor */
+	pmu_update_inhibit_flags(flags, &mhpmevent_val);
+
+#if __riscv_xlen == 32
+	csr_write_num(CSR_MCOUNTINHIBIT + ctr_idx, mhpmevent_val & 0xFFFFFFFF);
+	csr_write_num(CSR_MHPMEVENT3H + ctr_idx - 3, mhpmevent_val >> BITS_PER_LONG);
+#else
 	csr_write_num(CSR_MCOUNTINHIBIT + ctr_idx, mhpmevent_val);
+#endif
 
 	return 0;
 }
 
-static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask,
+static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned long flags,
 			   unsigned long event_idx, uint64_t data)
 {
 	unsigned long ctr_mask;
@@ -427,7 +483,7 @@  static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask,
 	if (ctr_idx == SBI_ENOTSUPP)
 		return SBI_EFAIL;
 
-	ret = pmu_update_hw_mhpmevent(temp, ctr_idx, event_idx, data);
+	ret = pmu_update_hw_mhpmevent(temp, ctr_idx, flags, event_idx, data);
 
 	if (!ret)
 		ret = ctr_idx;
@@ -490,7 +546,8 @@  int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
 		/* Any firmware counter can be used track any firmware event */
 		ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, hartid);
 	} else {
-		ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, event_idx, event_data);
+		ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
+					  event_data);
 	}
 
 	if (ctr_idx < 0)