[PATCH v3 10/11] lib: sbi: Enable PMU extension for platforms without mcountinhibit
Anup Patel
anup at brainfault.org
Wed Nov 3 04:10:02 PDT 2021
On Tue, Nov 2, 2021 at 8:40 PM Atish Patra <atish.patra at wdc.com> wrote:
>
> Some platforms such as hifive unmatched doesn't implement mcountinhibit
> csr. However, it has hardware events that can monitored using 2 hpmcounter
s/that can monitored/that can be monitored
> it has (i.e. mhpmcounter3 & mhpmcounter4).
>
> Currently, PMU extension disabled if mcountinhibit is present. That's not
s/extension disabled if mcountinhibit is present/extension is disabled
if mcountinhibit is absent/
> really necessary as long as the supervisor OS keeps track of the delta
> value of the counters. Without mcountinhibit, the delta value won't be
> entirely accurate because the counters are freely running. However, that
> should be fine to produce an approximate counter value which can help
> performance analysis. Perf sampling won't work though as sscof extension
> is not present in hifive unmatched.
>
> Signed-off-by: Atish Patra <atish.patra at wdc.com>
Otherwise it looks good to me.
Reviewed-by: Anup Patel <anup.patel at wdc.com>
Regards,
Anup
> ---
> lib/sbi/sbi_ecall_pmu.c | 10 ++-------
> lib/sbi/sbi_pmu.c | 50 ++++++++++++++++++++++++++++-------------
> lib/utils/fdt/fdt_pmu.c | 4 ++++
> 3 files changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/lib/sbi/sbi_ecall_pmu.c b/lib/sbi/sbi_ecall_pmu.c
> index 39d3857680c1..9ee9e81802e7 100644
> --- a/lib/sbi/sbi_ecall_pmu.c
> +++ b/lib/sbi/sbi_ecall_pmu.c
> @@ -74,14 +74,8 @@ static int sbi_ecall_pmu_handler(unsigned long extid, unsigned long funcid,
>
> static int sbi_ecall_pmu_probe(unsigned long extid, unsigned long *out_val)
> {
> - struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> -
> - /* SBI PMU extension is useless without mcount inhibit features */
> - if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> - *out_val = 1;
> - else
> - *out_val = 0;
> -
> + /* PMU extension is always enabled */
> + *out_val = 1;
> return 0;
> }
>
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index d728529488dc..6948a712f739 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -279,13 +279,21 @@ static void pmu_ctr_write_hw(uint32_t cidx, uint64_t ival)
>
> static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
> {
> - unsigned long mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> + unsigned long mctr_inhbt;
>
> /* Make sure the counter index lies within the range and is not TM bit */
> if (cidx > num_hw_ctrs || cidx == 1)
> return SBI_EINVAL;
>
> + if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + goto skip_inhbit_update;
> +
> + /*
> + * Some of the hardware may not support mcountinhibit but perf stat
> + * still can work if supervisor mode programs the initial value.
> + */
> + mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> if (!__test_bit(cidx, &mctr_inhbt))
> return SBI_EALREADY_STARTED;
>
> @@ -295,6 +303,7 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
> pmu_ctr_enable_irq_hw(cidx);
> csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
>
> +skip_inhbit_update:
> if (ival_update)
> pmu_ctr_write_hw(cidx, ival);
>
> @@ -390,6 +399,7 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
> int event_idx_type;
> uint32_t event_code;
> unsigned long ctr_mask = cmask << cbase;
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>
> if (__fls(ctr_mask) >= total_ctrs)
> return SBI_EINVAL;
> @@ -402,8 +412,10 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
>
> else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
> ret = pmu_ctr_stop_fw(cbase, event_code);
> - else
> + else if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> ret = pmu_ctr_stop_hw(cbase);
> + else
> + ret = 0;
>
> if (flag & SBI_PMU_STOP_FLAG_RESET) {
> active_events[hartid][cbase] = SBI_PMU_EVENT_IDX_INVALID;
> @@ -473,7 +485,8 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> unsigned long ctr_mask;
> int i, ret = 0, fixed_ctr, ctr_idx = SBI_ENOTSUPP;
> struct sbi_pmu_hw_event *temp;
> - unsigned long mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> + unsigned long mctr_inhbt;
> + u32 hartid = current_hartid();
> struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> bool sscof_present = sbi_hart_has_feature(scratch, SBI_HART_HAS_SSCOF);
>
> @@ -488,6 +501,8 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> if (fixed_ctr >= 0 && !sscof_present)
> return fixed_ctr;
>
> + if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + mctr_inhbt = csr_read(CSR_MCOUNTINHIBIT);
> for (i = 0; i < num_hw_events; i++) {
> temp = &hw_event_map[i];
> if ((temp->start_idx > event_idx && event_idx < temp->end_idx) ||
> @@ -499,12 +514,21 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> continue;
>
> /* Fixed counters should not be part of the search */
> - ctr_mask = temp->counters & (cmask << cbase) & (~SBI_PMU_FIXED_CTR_MASK);
> + ctr_mask = temp->counters & (cmask << cbase) &
> + (~SBI_PMU_FIXED_CTR_MASK);
> for_each_set_bit_from(cbase, &ctr_mask, SBI_PMU_HW_CTR_MAX) {
> - if (__test_bit(cbase, &mctr_inhbt)) {
> - ctr_idx = cbase;
> - break;
> - }
> + /**
> + * Some of the platform may not support mcountinhibit.
> + * Checking the active_events is enough for them
> + */
> + if (active_events[hartid][cbase] != SBI_PMU_EVENT_IDX_INVALID)
> + continue;
> + /* If mcountinhibit is supported, the bit must be enabled */
> + if ((sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT)) &&
> + !__test_bit(cbase, &mctr_inhbt))
> + continue;
> + /* We found a valid counter that is not started yet.*/
> + ctr_idx = cbase;
> }
> }
>
> @@ -676,11 +700,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch)
> {
> u32 hartid = current_hartid();
>
> - /* SBI PMU is not supported if mcountinhibit is not available */
> - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> - return;
> + if (sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
>
> - csr_write(CSR_MCOUNTINHIBIT, 0xFFFFFFF8);
> csr_write(CSR_MCOUNTEREN, -1);
> pmu_reset_event_map(hartid);
> }
> @@ -690,10 +712,6 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
> const struct sbi_platform *plat;
> u32 hartid = current_hartid();
>
> - /* SBI PMU is not supported if mcountinhibit is not available */
> - if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> - return 0;
> -
> if (cold_boot) {
> plat = sbi_platform_ptr(scratch);
> /* Initialize hw pmu events */
> diff --git a/lib/utils/fdt/fdt_pmu.c b/lib/utils/fdt/fdt_pmu.c
> index 9ebf1c225017..4d86e775bcdb 100644
> --- a/lib/utils/fdt/fdt_pmu.c
> +++ b/lib/utils/fdt/fdt_pmu.c
> @@ -9,6 +9,7 @@
> */
>
> #include <libfdt.h>
> +#include <sbi/sbi_hart.h>
> #include <sbi/sbi_error.h>
> #include <sbi/sbi_pmu.h>
> #include <sbi_utils/fdt/fdt_helper.h>
> @@ -40,6 +41,7 @@ uint64_t fdt_pmu_get_select_value(uint32_t event_idx)
> int fdt_pmu_fixup(void *fdt)
> {
> int pmu_offset;
> + struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>
> if (!fdt)
> return SBI_EINVAL;
> @@ -51,6 +53,8 @@ int fdt_pmu_fixup(void *fdt)
> fdt_delprop(fdt, pmu_offset, "pmu,event-to-mhpmcounters");
> fdt_delprop(fdt, pmu_offset, "pmu,event-to-mhpmevent");
> fdt_delprop(fdt, pmu_offset, "pmu,raw-event-to-mhpmcounters");
> + if (!sbi_hart_has_feature(scratch, SBI_HART_HAS_MCOUNTINHIBIT))
> + fdt_delprop(fdt, pmu_offset, "interrupts-extended");
>
> return 0;
> }
> --
> 2.31.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
More information about the opensbi
mailing list