[PATCH v3 0/5] Add support for T-HEAD C9xx PMU extensions

[PATCH v3 0/5] Add support for T-HEAD C9xx PMU extensions

[Heiko Stuebner]

The T-HEAD C9XX cores implement functionality very similar to SSCOFPMF.
Instead of implementing a separate interface into SBI as done in v1,
we can add the C9XX tidbits with quite minimal overhead and leaverage
the existing SBI pmu interface including giving access to the firmware
counters.

The current only hickup is the detection override in sbi_hart, where
I still need to find out why the MHPMCOUNTERs are not writeable
at _that_ point of the boot, but with this series on top of openSBI 1.1
and the matching kernel perf patch I get reasonable results for example
for:

perf stat -e cycles -e instructions -e L1-icache-load-misses -e L1-icache-loads ls

 Performance counter stats for 'ls':

          17119496      cycles
           2867765      instructions              #    0.17  insn per cycle
             55929      L1-icache-load-misses     #    1.61% of all L1-icache accesses
           3467346      L1-icache-loads

       0.030156216 seconds time elapsed

       0.000000000 seconds user
       0.023496000 seconds sys

changes in v3:
- follow Atish's advice and implement an abstraction
  to not pollute the core pmu with cpu-specific variants

changes in v2:
- don't implement a separate interface but instead modify
  the sbi-pmu to allow the c9xx to use standard pmu interface


Heiko Stuebner (5):
  lib: sbi: do platform-specific extension population earlier
  lib: sbi_pmu: move pmu irq information into pmu itself
  lib: sbi_pmu: add ability to override methods on non-standard
    platforms
  platform: generic: add extensions_init handler and platform-override
  platform: generic: allwinner: add support for c9xx pmu

 include/sbi/sbi_pmu.h                        |  16 +++
 lib/sbi/sbi_hart.c                           |  21 ++-
 lib/sbi/sbi_pmu.c                            |  31 +++++
 platform/generic/allwinner/sun20i-d1.c       |  76 +++++++++++
 platform/generic/include/platform_override.h |   1 +
 platform/generic/include/thead_c9xx.h        | 127 +++++++++++++++++++
 platform/generic/platform.c                  |   9 ++
 7 files changed, 274 insertions(+), 7 deletions(-)
 create mode 100644 platform/generic/include/thead_c9xx.h

-- 
2.35.1

[PATCH v3 1/5] lib: sbi: do platform-specific extension population earlier

[Heiko Stuebner]

Some of the more specific detections in hart_detect_features()
might need to check platform-specific extensions so might need
the platform-extensions being populated earlier.

So move the call to sbi_platform_extensions_init() to an earlier
place.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 lib/sbi/sbi_hart.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 1294868..9540e5c 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -553,6 +553,11 @@ static int hart_detect_features(struct sbi_scratch *scratch)
 	hfeatures->pmp_count = 0;
 	hfeatures->mhpm_count = 0;
 
+	/* Let platform populate extensions */
+	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
+	if (rc)
+		return rc;
+
 #define __check_csr(__csr, __rdonly, __wrval, __field, __skip)	\
 	oldval = csr_read_allowed(__csr, (ulong)&trap);			\
 	if (!trap.cause) {						\
@@ -681,11 +686,6 @@ __mhpm_skip:
 					SBI_HART_EXT_SMSTATEEN, true);
 	}
 
-	/* Let platform populate extensions */
-	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr());
-	if (rc)
-		return rc;
-
 	/* Mark hart feature detection done */
 	hfeatures->detected = true;
 
-- 
2.35.1

[PATCH v3 2/5] lib: sbi_pmu: move pmu irq information into pmu itself

[Heiko Stuebner]

Don't spread checking for pmu extensions through the code
but instead introduce a sbi-pmu function that other code can
call to get the correct information about the existence of the
pmu interrupt.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 include/sbi/sbi_pmu.h |  3 +++
 lib/sbi/sbi_hart.c    |  4 ++--
 lib/sbi/sbi_pmu.c     | 10 ++++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index f5e3dbd..1c9a997 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -34,6 +34,9 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
 /** Reset PMU during hart exit */
 void sbi_pmu_exit(struct sbi_scratch *scratch);
 
+/** Return the pmu irq bit depending on extension existence */
+int sbi_pmu_irq_bit(void);
+
 /**
  * Add the hardware event to counter mapping information. This should be called
  * from the platform code to update the mapping table.
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 9540e5c..45fbcde 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -19,6 +19,7 @@
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_math.h>
 #include <sbi/sbi_platform.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_trap.h>
 
@@ -208,8 +209,7 @@ static int delegate_traps(struct sbi_scratch *scratch)
 
 	/* Send M-mode interrupts and most exceptions to S-mode */
 	interrupts = MIP_SSIP | MIP_STIP | MIP_SEIP;
-	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
-		interrupts |= MIP_LCOFIP;
+	interrupts |= sbi_pmu_irq_bit();
 
 	exceptions = (1U << CAUSE_MISALIGNED_FETCH) | (1U << CAUSE_BREAKPOINT) |
 		     (1U << CAUSE_USER_ECALL);
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 5a294a2..efbfbb6 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -367,6 +367,16 @@ skip_inhibit_update:
 	return 0;
 }
 
+int sbi_pmu_irq_bit(void)
+{
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+
+	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
+		return MIP_LCOFIP;
+
+	return 0;
+}
+
 static int pmu_ctr_start_fw(uint32_t cidx, uint32_t fw_evt_code,
 			    uint64_t ival, bool ival_update)
 {
-- 
2.35.1

[PATCH v3 3/5] lib: sbi_pmu: add ability to override methods on non-standard platforms

[Heiko Stuebner]

Some platforms may not implement the PMU in a standard way,
but instead deviate on some functionality like overflow
handling.

Implement an abstraction that those platforms can hook into.
This way they can still use the standard pmu interface between
kernel and firmware and profit from things like firmware counters,
without needing to create their own.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 include/sbi/sbi_pmu.h | 13 +++++++++++++
 lib/sbi/sbi_hart.c    |  7 +++++++
 lib/sbi/sbi_pmu.c     | 21 +++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index 1c9a997..7c8cfe7 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -28,6 +28,16 @@
 #define SBI_PMU_CTR_MAX	   (SBI_PMU_HW_CTR_MAX + SBI_PMU_FW_CTR_MAX)
 #define SBI_PMU_FIXED_CTR_MASK 0x07
 
+struct sbi_pmu_platform_ops {
+	int (*ctr_enable_irq_hw)(int ctr_idx);
+	int (*ctr_enable_ovf)(int ctr_idx);
+	int (*irq_bit)(void);
+	void (*counter_data)(unsigned int *count, unsigned int *bits);
+};
+
+/** Set platform-specific override ops */
+void sbi_pmu_set_platform_ops(struct sbi_pmu_platform_ops *ops);
+
 /** Initialize PMU */
 int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
 
@@ -37,6 +47,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch);
 /** Return the pmu irq bit depending on extension existence */
 int sbi_pmu_irq_bit(void);
 
+/** Allow non-standard platforms to override probed counter information */
+void sbi_pmu_override_counter_data(unsigned int *count, unsigned int *bits);
+
 /**
  * Add the hardware event to counter mapping information. This should be called
  * from the platform code to update the mapping table.
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 45fbcde..6506a19 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -632,6 +632,13 @@ __mhpm_skip:
 #undef __check_csr_2
 #undef __check_csr
 
+	/**
+	 * Allow non-standard implementations to override the detected
+	 * values for number of counters and bits.
+	 */
+	sbi_pmu_override_counter_data(&hfeatures->mhpm_count,
+				      &hfeatures->mhpm_bits);
+
 	/* Detect if hart supports Priv v1.10 */
 	val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
 	if (!trap.cause)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index efbfbb6..08fb242 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -77,6 +77,8 @@ static uint32_t num_hw_ctrs;
 /* Maximum number of counters available */
 static uint32_t total_ctrs;
 
+static struct sbi_pmu_platform_ops *pmu_platform_ops;
+
 /* Helper macros to retrieve event idx and code type */
 #define get_cidx_type(x) ((x & SBI_PMU_EVENT_IDX_TYPE_MASK) >> 16)
 #define get_cidx_code(x) (x & SBI_PMU_EVENT_IDX_CODE_MASK)
@@ -358,6 +360,9 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
 
 	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
 		pmu_ctr_enable_irq_hw(cidx);
+	else if (pmu_platform_ops && pmu_platform_ops->ctr_enable_irq_hw)
+		pmu_platform_ops->ctr_enable_irq_hw(cidx);
+
 	csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
 
 skip_inhibit_update:
@@ -373,6 +378,8 @@ int sbi_pmu_irq_bit(void)
 
 	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
 		return MIP_LCOFIP;
+	else if (pmu_platform_ops && pmu_platform_ops->irq_bit)
+		return pmu_platform_ops->irq_bit();
 
 	return 0;
 }
@@ -534,6 +541,8 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
 	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
 		mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) |
 				 MHPMEVENT_MINH | MHPMEVENT_OF;
+	else if (pmu_platform_ops && pmu_platform_ops->ctr_enable_ovf)
+		pmu_platform_ops->ctr_enable_ovf(ctr_idx);
 
 	/* Update the inhibit flags based on inhibit flags received from supervisor */
 	pmu_update_inhibit_flags(flags, &mhpmevent_val);
@@ -822,3 +831,15 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
 
 	return 0;
 }
+
+void sbi_pmu_override_counter_data(unsigned int *count,
+				   unsigned int *bits)
+{
+	if (pmu_platform_ops && pmu_platform_ops->counter_data)
+		pmu_platform_ops->counter_data(count, bits);
+}
+
+void sbi_pmu_set_platform_ops(struct sbi_pmu_platform_ops *ops)
+{
+	pmu_platform_ops = ops;
+}
-- 
2.35.1

[PATCH v3 4/5] platform: generic: add extensions_init handler and platform-override

[Heiko Stuebner]

Init of non-standard extensions is a platform-specific thing,
so allow generic platforms to do this via a platform-override.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 platform/generic/include/platform_override.h | 1 +
 platform/generic/platform.c                  | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/platform/generic/include/platform_override.h b/platform/generic/include/platform_override.h
index e55da25..06a26dd 100644
--- a/platform/generic/include/platform_override.h
+++ b/platform/generic/include/platform_override.h
@@ -22,6 +22,7 @@ struct platform_override {
 	void (*early_exit)(const struct fdt_match *match);
 	void (*final_exit)(const struct fdt_match *match);
 	int (*fdt_fixup)(void *fdt, const struct fdt_match *match);
+	int (*extensions_init)(const struct fdt_match *match);
 	int (*vendor_ext_check)(long extid, const struct fdt_match *match);
 	int (*vendor_ext_provider)(long extid, long funcid,
 				   const struct sbi_trap_regs *regs,
diff --git a/platform/generic/platform.c b/platform/generic/platform.c
index cc3620f..de406e3 100644
--- a/platform/generic/platform.c
+++ b/platform/generic/platform.c
@@ -203,6 +203,14 @@ static void generic_final_exit(void)
 		generic_plat->final_exit(generic_plat_match);
 }
 
+static int generic_extensions_init(void)
+{
+	if (generic_plat && generic_plat->extensions_init)
+		return generic_plat->extensions_init(generic_plat_match);
+
+	return 0;
+}
+
 static int generic_domains_init(void)
 {
 	return fdt_domains_populate(fdt_get_address());
@@ -248,6 +256,7 @@ const struct sbi_platform_operations platform_ops = {
 	.final_init		= generic_final_init,
 	.early_exit		= generic_early_exit,
 	.final_exit		= generic_final_exit,
+	.extensions_init	= generic_extensions_init,
 	.domains_init		= generic_domains_init,
 	.console_init		= fdt_serial_init,
 	.irqchip_init		= fdt_irqchip_init,
-- 
2.35.1

[PATCH v3 5/5] platform: generic: allwinner: add support for c9xx pmu

[Heiko Stuebner]

With the T-HEAD C9XX cores being designed before or during ratification
of the SSCOFPMF extension, they implement a PMU extension that behaves
very similar but not equal to it by providing overflow interrupts though
in a slightly different registers format.

The sun20i-d1 is using this core. So implement the necessary overrides
to allow its pmu to be used via the standard sbi-pmu extension.

For now it's also the only soc using this core, so keep the additional
code in the d1-space for now.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 platform/generic/allwinner/sun20i-d1.c |  76 +++++++++++++++
 platform/generic/include/thead_c9xx.h  | 127 +++++++++++++++++++++++++
 2 files changed, 203 insertions(+)
 create mode 100644 platform/generic/include/thead_c9xx.h

diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
index 5b2656c..55d58eb 100644
--- a/platform/generic/allwinner/sun20i-d1.c
+++ b/platform/generic/allwinner/sun20i-d1.c
@@ -5,11 +5,13 @@
  */
 
 #include <platform_override.h>
+#include <thead_c9xx.h>
 #include <sbi/riscv_io.h>
 #include <sbi/sbi_bitops.h>
 #include <sbi/sbi_ecall_interface.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_hsm.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 #include <sbi_utils/irqchip/fdt_irqchip_plic.h>
 
@@ -199,6 +201,79 @@ static int sun20i_d1_final_init(bool cold_boot, const struct fdt_match *match)
 	return 0;
 }
 
+static int thead_c9xx_pmu_ctr_enable_irq_hw(int ctr_idx)
+{
+	unsigned long val;
+	unsigned long mip_val;
+
+	if (ctr_idx >= SBI_PMU_HW_CTR_MAX)
+		return SBI_EFAIL;
+
+	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 & THEAD_C9XX_MIP_MOIP)) {
+		val = csr_read(THEAD_C9XX_CSR_MCOUNTEROF);
+		val &= ~(1 << ctr_idx);
+		csr_write(THEAD_C9XX_CSR_MCOUNTEROF, val);
+	}
+
+	/**
+	 * SSCOFPMF uses the OF bit for enabling/disabling the interrupt,
+	 * while the C9XX has designated enable bits.
+	 * So enable per-counter interrupt on C9xx here.
+	 */
+	val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
+	val |= (1 << ctr_idx);
+	csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
+
+	return 0;
+}
+
+static int thead_c9xx_pmu_ctr_enable_ovf(int ctr_idx)
+{
+	unsigned long val;
+
+	val = csr_read(THEAD_C9XX_CSR_MCOUNTERINTEN);
+	val &= ~(1 << ctr_idx);
+	csr_write(THEAD_C9XX_CSR_MCOUNTERINTEN, val);
+
+	return 0;
+}
+
+static int thead_c9xx_pmu_irq_bit(void)
+{
+	return THEAD_C9XX_MIP_MOIP;
+}
+
+static void thead_c9xx_pmu_counter_data(unsigned int *count,
+					unsigned int *bits)
+{
+	/* auto-detection doesn't work on t-head c9xx cores */
+	*count = 29;
+	*bits = 64;
+}
+
+struct sbi_pmu_platform_ops thead_c9xx_pmu_platform_ops = {
+	.ctr_enable_irq_hw = thead_c9xx_pmu_ctr_enable_irq_hw,
+	.ctr_enable_ovf = thead_c9xx_pmu_ctr_enable_ovf,
+	.irq_bit = thead_c9xx_pmu_irq_bit,
+	.counter_data = thead_c9xx_pmu_counter_data,
+};
+
+static int sun20i_d1_extensions_init(const struct fdt_match *match)
+{
+	sbi_pmu_set_platform_ops(&thead_c9xx_pmu_platform_ops);
+
+	return 0;
+}
+
 static const struct fdt_match sun20i_d1_match[] = {
 	{ .compatible = "allwinner,sun20i-d1" },
 	{ },
@@ -207,4 +282,5 @@ static const struct fdt_match sun20i_d1_match[] = {
 const struct platform_override sun20i_d1 = {
 	.match_table	= sun20i_d1_match,
 	.final_init	= sun20i_d1_final_init,
+	.extensions_init = sun20i_d1_extensions_init,
 };
diff --git a/platform/generic/include/thead_c9xx.h b/platform/generic/include/thead_c9xx.h
new file mode 100644
index 0000000..bab0408
--- /dev/null
+++ b/platform/generic/include/thead_c9xx.h
@@ -0,0 +1,127 @@
+#ifndef __RISCV_THEAD_C9XX_H____
+#define __RISCV_THEAD_C9XX_H____
+
+/* T-HEAD C9xx M mode CSR.  */
+#define THEAD_C9XX_CSR_MXSTATUS		0x7c0
+#define THEAD_C9XX_CSR_MHCR		0x7c1
+#define THEAD_C9XX_CSR_MCOR		0x7c2
+#define THEAD_C9XX_CSR_MCCR2		0x7c3
+#define THEAD_C9XX_CSR_MCER2		0x7c4
+#define THEAD_C9XX_CSR_MHINT		0x7c5
+#define THEAD_C9XX_CSR_MRMR		0x7c6
+#define THEAD_C9XX_CSR_MRVBR		0x7c7
+#define THEAD_C9XX_CSR_MCER		0x7c8
+#define THEAD_C9XX_CSR_MCOUNTERWEN	0x7c9
+#define THEAD_C9XX_CSR_MCOUNTERINTEN	0x7ca
+#define THEAD_C9XX_CSR_MCOUNTEROF	0x7cb
+#define THEAD_C9XX_CSR_MHINT2		0x7cc
+#define THEAD_C9XX_CSR_MHINT3		0x7cd
+#define THEAD_C9XX_CSR_MRADDR		0x7e0
+#define THEAD_C9XX_CSR_MEXSTATUS	0x7e1
+#define THEAD_C9XX_CSR_MNMICAUSE	0x7e2
+#define THEAD_C9XX_CSR_MNMIPC		0x7e3
+#define THEAD_C9XX_CSR_MHPMCR		0x7f0
+#define THEAD_C9XX_CSR_MHPMSR		0x7f1
+#define THEAD_C9XX_CSR_MHPMER		0x7f2
+#define THEAD_C9XX_CSR_MSMPR		0x7f3
+#define THEAD_C9XX_CSR_MTEECFG		0x7f4
+#define THEAD_C9XX_CSR_MZONEID		0x7f5
+#define THEAD_C9XX_CSR_ML2CPID		0x7f6
+#define THEAD_C9XX_CSR_ML2WP		0x7f7
+#define THEAD_C9XX_CSR_MDTCMCR		0x7f8
+#define THEAD_C9XX_CSR_USP		0x7d1
+#define THEAD_C9XX_CSR_MCINS		0x7d2
+#define THEAD_C9XX_CSR_MCINDEX		0x7d3
+#define THEAD_C9XX_CSR_MCDATA0		0x7d4
+#define THEAD_C9XX_CSR_MCDATA1		0x7d5
+#define THEAD_C9XX_CSR_MEICR		0x7d6
+#define THEAD_C9XX_CSR_MEICR2		0x7d7
+#define THEAD_C9XX_CSR_MBEADDR		0x7d8
+#define THEAD_C9XX_CSR_MCPUID		0xfc0
+#define THEAD_C9XX_CSR_MAPBADDR		0xfc1
+#define THEAD_C9XX_CSR_MWMSR		0xfc2
+#define THEAD_C9XX_CSR_MHALTCAUSE	0xfe0
+#define THEAD_C9XX_CSR_MDBGINFO		0xfe1
+#define THEAD_C9XX_CSR_MPCFIFO		0xfe2
+
+/* T-HEAD C9xx S mode CSR.  */
+#define THEAD_C9XX_CSR_SXSTATUS		0x5c0
+#define THEAD_C9XX_CSR_SHCR		0x5c1
+#define THEAD_C9XX_CSR_SCER2		0x5c2
+#define THEAD_C9XX_CSR_SCER		0x5c3
+#define THEAD_C9XX_CSR_SCOUNTERINTEN	0x5c4
+#define THEAD_C9XX_CSR_SCOUNTEROF	0x5c5
+#define THEAD_C9XX_CSR_SHINT		0x5c6
+#define THEAD_C9XX_CSR_SHINT2		0x5c7
+#define THEAD_C9XX_CSR_SHPMINHIBIT	0x5c8
+#define THEAD_C9XX_CSR_SHPMCR		0x5c9
+#define THEAD_C9XX_CSR_SHPMSR		0x5ca
+#define THEAD_C9XX_CSR_SHPMER		0x5cb
+#define THEAD_C9XX_CSR_SL2CPID		0x5cc
+#define THEAD_C9XX_CSR_SL2WP		0x5cd
+#define THEAD_C9XX_CSR_SBEADDR		0x5d0
+#define THEAD_C9XX_CSR_SCYCLE		0x5e0
+#define THEAD_C9XX_CSR_SHPMCOUNTER1	0x5e1
+#define THEAD_C9XX_CSR_SHPMCOUNTER2	0x5e2
+#define THEAD_C9XX_CSR_SHPMCOUNTER3	0x5e3
+#define THEAD_C9XX_CSR_SHPMCOUNTER4	0x5e4
+#define THEAD_C9XX_CSR_SHPMCOUNTER5	0x5e5
+#define THEAD_C9XX_CSR_SHPMCOUNTER6	0x5e6
+#define THEAD_C9XX_CSR_SHPMCOUNTER7	0x5e7
+#define THEAD_C9XX_CSR_SHPMCOUNTER8	0x5e8
+#define THEAD_C9XX_CSR_SHPMCOUNTER9	0x5e9
+#define THEAD_C9XX_CSR_SHPMCOUNTER10	0x5ea
+#define THEAD_C9XX_CSR_SHPMCOUNTER11	0x5eb
+#define THEAD_C9XX_CSR_SHPMCOUNTER12	0x5ec
+#define THEAD_C9XX_CSR_SHPMCOUNTER13	0x5ed
+#define THEAD_C9XX_CSR_SHPMCOUNTER14	0x5ee
+#define THEAD_C9XX_CSR_SHPMCOUNTER15	0x5ef
+#define THEAD_C9XX_CSR_SHPMCOUNTER16	0x5f0
+#define THEAD_C9XX_CSR_SHPMCOUNTER17	0x5f1
+#define THEAD_C9XX_CSR_SHPMCOUNTER18	0x5f2
+#define THEAD_C9XX_CSR_SHPMCOUNTER19	0x5f3
+#define THEAD_C9XX_CSR_SHPMCOUNTER20	0x5f4
+#define THEAD_C9XX_CSR_SHPMCOUNTER21	0x5f5
+#define THEAD_C9XX_CSR_SHPMCOUNTER22	0x5f6
+#define THEAD_C9XX_CSR_SHPMCOUNTER23	0x5f7
+#define THEAD_C9XX_CSR_SHPMCOUNTER24	0x5f8
+#define THEAD_C9XX_CSR_SHPMCOUNTER25	0x5f9
+#define THEAD_C9XX_CSR_SHPMCOUNTER26	0x5fa
+#define THEAD_C9XX_CSR_SHPMCOUNTER27	0x5fb
+#define THEAD_C9XX_CSR_SHPMCOUNTER28	0x5fc
+#define THEAD_C9XX_CSR_SHPMCOUNTER29	0x5fd
+#define THEAD_C9XX_CSR_SHPMCOUNTER30	0x5fe
+#define THEAD_C9XX_CSR_SHPMCOUNTER31	0x5ff
+
+/* T-HEAD C9xx U mode CSR.  */
+#define THEAD_C9XX_CSR_FXCR		0x800
+
+/* T-HEAD C9xx MMU extentions.  */
+#define THEAD_C9XX_CSR_SMIR		0x9c0
+#define THEAD_C9XX_CSR_SMEL		0x9c1
+#define THEAD_C9XX_CSR_SMEH		0x9c2
+#define THEAD_C9XX_CSR_SMCIR		0x9c3
+
+/* T-HEAD C9xx Security CSR(May be droped).  */
+#define THEAD_C9XX_CSR_MEBR		0xbe0
+#define THEAD_C9XX_CSR_NT_MSTATUS	0xbe1
+#define THEAD_C9XX_CSR_NT_MIE		0xbe2
+#define THEAD_C9XX_CSR_NT_MTVEC		0xbe3
+#define THEAD_C9XX_CSR_NT_MTVT		0xbe4
+#define THEAD_C9XX_CSR_NT_MEPC		0xbe5
+#define THEAD_C9XX_CSR_NT_MCAUSE	0xbe6
+#define THEAD_C9XX_CSR_NT_MIP		0xbe7
+#define THEAD_C9XX_CSR_NT_MINTSTATE	0xbe8
+#define THEAD_C9XX_CSR_NT_MXSTATUS	0xbe9
+#define THEAD_C9XX_CSR_NT_MEBR		0xbea
+#define THEAD_C9XX_CSR_NT_MSP		0xbeb
+#define THEAD_C9XX_CSR_T_USP		0xbec
+#define THEAD_C9XX_CSR_T_MDCR		0xbed
+#define THEAD_C9XX_CSR_T_MPCR		0xbee
+#define THEAD_C9XX_CSR_PMPTEECFG	0xbef
+
+/* T-HEAD C9xx MIP CSR extension */
+#define THEAD_C9XX_IRQ_PMU_OVF		17
+#define THEAD_C9XX_MIP_MOIP		(_UL(1) << THEAD_C9XX_IRQ_PMU_OVF)
+
+#endif
-- 
2.35.1

[PATCH v3 3/5] lib: sbi_pmu: add ability to override methods on non-standard platforms

[Andrew Jones]

On Thu, Sep 08, 2022 at 03:42:40PM +0200, Heiko Stuebner wrote:
> Some platforms may not implement the PMU in a standard way,
> but instead deviate on some functionality like overflow
> handling.
> 
> Implement an abstraction that those platforms can hook into.
> This way they can still use the standard pmu interface between
> kernel and firmware and profit from things like firmware counters,
> without needing to create their own.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  include/sbi/sbi_pmu.h | 13 +++++++++++++
>  lib/sbi/sbi_hart.c    |  7 +++++++
>  lib/sbi/sbi_pmu.c     | 21 +++++++++++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index 1c9a997..7c8cfe7 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -28,6 +28,16 @@
>  #define SBI_PMU_CTR_MAX	   (SBI_PMU_HW_CTR_MAX + SBI_PMU_FW_CTR_MAX)
>  #define SBI_PMU_FIXED_CTR_MASK 0x07
>  
> +struct sbi_pmu_platform_ops {
> +	int (*ctr_enable_irq_hw)(int ctr_idx);
> +	int (*ctr_enable_ovf)(int ctr_idx);
> +	int (*irq_bit)(void);
> +	void (*counter_data)(unsigned int *count, unsigned int *bits);
> +};
> +
> +/** Set platform-specific override ops */
> +void sbi_pmu_set_platform_ops(struct sbi_pmu_platform_ops *ops);
> +
>  /** Initialize PMU */
>  int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
>  
> @@ -37,6 +47,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch);
>  /** Return the pmu irq bit depending on extension existence */
>  int sbi_pmu_irq_bit(void);
>  
> +/** Allow non-standard platforms to override probed counter information */
> +void sbi_pmu_override_counter_data(unsigned int *count, unsigned int *bits);
> +
>  /**
>   * Add the hardware event to counter mapping information. This should be called
>   * from the platform code to update the mapping table.
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 45fbcde..6506a19 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -632,6 +632,13 @@ __mhpm_skip:
>  #undef __check_csr_2
>  #undef __check_csr
>  
> +	/**
> +	 * Allow non-standard implementations to override the detected
> +	 * values for number of counters and bits.
> +	 */
> +	sbi_pmu_override_counter_data(&hfeatures->mhpm_count,
> +				      &hfeatures->mhpm_bits);
> +
>  	/* Detect if hart supports Priv v1.10 */
>  	val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
>  	if (!trap.cause)
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index efbfbb6..08fb242 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -77,6 +77,8 @@ static uint32_t num_hw_ctrs;
>  /* Maximum number of counters available */
>  static uint32_t total_ctrs;
>  
> +static struct sbi_pmu_platform_ops *pmu_platform_ops;
> +
>  /* Helper macros to retrieve event idx and code type */
>  #define get_cidx_type(x) ((x & SBI_PMU_EVENT_IDX_TYPE_MASK) >> 16)
>  #define get_cidx_code(x) (x & SBI_PMU_EVENT_IDX_CODE_MASK)
> @@ -358,6 +360,9 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
>  
>  	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
>  		pmu_ctr_enable_irq_hw(cidx);
> +	else if (pmu_platform_ops && pmu_platform_ops->ctr_enable_irq_hw)
> +		pmu_platform_ops->ctr_enable_irq_hw(cidx);
> +
>  	csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
>  
>  skip_inhibit_update:
> @@ -373,6 +378,8 @@ int sbi_pmu_irq_bit(void)
>  
>  	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
>  		return MIP_LCOFIP;
> +	else if (pmu_platform_ops && pmu_platform_ops->irq_bit)
> +		return pmu_platform_ops->irq_bit();
>  
>  	return 0;
>  }
> @@ -534,6 +541,8 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
>  	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
>  		mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) |
>  				 MHPMEVENT_MINH | MHPMEVENT_OF;
> +	else if (pmu_platform_ops && pmu_platform_ops->ctr_enable_ovf)
> +		pmu_platform_ops->ctr_enable_ovf(ctr_idx);
>  

Based on these 'if sscofpmf else pmu_platform' conditionals it looks like
the pmu platform framework is actually specifically for a sscofpmf quirk.
How about introducing a general quirk / pmu quirk framework and then
adding a sscofpmf quirk. Then, where needed we'd have something like

  if (pmu_quirks[SSCOFPMF])
     sscofpmf_ops = (struct sscofpmf_quirk_ops *)pmu_quirks[SSCOFPMF];

  if sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF)
     ...
  else if (sscofpmf_ops && sscofpmf_ops->func_needed_here)
     sscofpmf_ops->func_needed_here(...);

Thanks,
drew

>  	/* Update the inhibit flags based on inhibit flags received from supervisor */
>  	pmu_update_inhibit_flags(flags, &mhpmevent_val);
> @@ -822,3 +831,15 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
>  
>  	return 0;
>  }
> +
> +void sbi_pmu_override_counter_data(unsigned int *count,
> +				   unsigned int *bits)
> +{
> +	if (pmu_platform_ops && pmu_platform_ops->counter_data)
> +		pmu_platform_ops->counter_data(count, bits);
> +}
> +
> +void sbi_pmu_set_platform_ops(struct sbi_pmu_platform_ops *ops)
> +{
> +	pmu_platform_ops = ops;
> +}
> -- 
> 2.35.1
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 4/5] platform: generic: add extensions_init handler and platform-override

[Andrew Jones]

On Thu, Sep 08, 2022 at 03:42:41PM +0200, Heiko Stuebner wrote:
> Init of non-standard extensions is a platform-specific thing,
> so allow generic platforms to do this via a platform-override.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  platform/generic/include/platform_override.h | 1 +
>  platform/generic/platform.c                  | 9 +++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/platform/generic/include/platform_override.h b/platform/generic/include/platform_override.h
> index e55da25..06a26dd 100644
> --- a/platform/generic/include/platform_override.h
> +++ b/platform/generic/include/platform_override.h
> @@ -22,6 +22,7 @@ struct platform_override {
>  	void (*early_exit)(const struct fdt_match *match);
>  	void (*final_exit)(const struct fdt_match *match);
>  	int (*fdt_fixup)(void *fdt, const struct fdt_match *match);
> +	int (*extensions_init)(const struct fdt_match *match);

If we go with the quirk concept, then maybe this should be renamed to
quirks_init() and it'd be where pmu_quirks[] would get initialized by
the platform.

Thanks,
drew

>  	int (*vendor_ext_check)(long extid, const struct fdt_match *match);
>  	int (*vendor_ext_provider)(long extid, long funcid,
>  				   const struct sbi_trap_regs *regs,
> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> index cc3620f..de406e3 100644
> --- a/platform/generic/platform.c
> +++ b/platform/generic/platform.c
> @@ -203,6 +203,14 @@ static void generic_final_exit(void)
>  		generic_plat->final_exit(generic_plat_match);
>  }
>  
> +static int generic_extensions_init(void)
> +{
> +	if (generic_plat && generic_plat->extensions_init)
> +		return generic_plat->extensions_init(generic_plat_match);
> +
> +	return 0;
> +}
> +
>  static int generic_domains_init(void)
>  {
>  	return fdt_domains_populate(fdt_get_address());
> @@ -248,6 +256,7 @@ const struct sbi_platform_operations platform_ops = {
>  	.final_init		= generic_final_init,
>  	.early_exit		= generic_early_exit,
>  	.final_exit		= generic_final_exit,
> +	.extensions_init	= generic_extensions_init,
>  	.domains_init		= generic_domains_init,
>  	.console_init		= fdt_serial_init,
>  	.irqchip_init		= fdt_irqchip_init,
> -- 
> 2.35.1
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 0/5] Add support for T-HEAD C9xx PMU extensions

[Anup Patel]

Hi Heiko,

On Thu, Sep 8, 2022 at 7:13 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> The T-HEAD C9XX cores implement functionality very similar to SSCOFPMF.
> Instead of implementing a separate interface into SBI as done in v1,
> we can add the C9XX tidbits with quite minimal overhead and leaverage
> the existing SBI pmu interface including giving access to the firmware
> counters.
>
> The current only hickup is the detection override in sbi_hart, where
> I still need to find out why the MHPMCOUNTERs are not writeable
> at _that_ point of the boot, but with this series on top of openSBI 1.1
> and the matching kernel perf patch I get reasonable results for example
> for:
>
> perf stat -e cycles -e instructions -e L1-icache-load-misses -e L1-icache-loads ls
>
>  Performance counter stats for 'ls':
>
>           17119496      cycles
>            2867765      instructions              #    0.17  insn per cycle
>              55929      L1-icache-load-misses     #    1.61% of all L1-icache accesses
>            3467346      L1-icache-loads
>
>        0.030156216 seconds time elapsed
>
>        0.000000000 seconds user
>        0.023496000 seconds sys
>
> changes in v3:
> - follow Atish's advice and implement an abstraction
>   to not pollute the core pmu with cpu-specific variants
>
> changes in v2:
> - don't implement a separate interface but instead modify
>   the sbi-pmu to allow the c9xx to use standard pmu interface
>
>
> Heiko Stuebner (5):
>   lib: sbi: do platform-specific extension population earlier
>   lib: sbi_pmu: move pmu irq information into pmu itself
>   lib: sbi_pmu: add ability to override methods on non-standard
>     platforms
>   platform: generic: add extensions_init handler and platform-override
>   platform: generic: allwinner: add support for c9xx pmu

Please rebase this upon the latest OpenSBI. We already have
"struct sbi_pmu_device" which platform can register and you
can extend it in this series.

Regards,
Anup

>
>  include/sbi/sbi_pmu.h                        |  16 +++
>  lib/sbi/sbi_hart.c                           |  21 ++-
>  lib/sbi/sbi_pmu.c                            |  31 +++++
>  platform/generic/allwinner/sun20i-d1.c       |  76 +++++++++++
>  platform/generic/include/platform_override.h |   1 +
>  platform/generic/include/thead_c9xx.h        | 127 +++++++++++++++++++
>  platform/generic/platform.c                  |   9 ++
>  7 files changed, 274 insertions(+), 7 deletions(-)
>  create mode 100644 platform/generic/include/thead_c9xx.h
>
> --
> 2.35.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH v3 0/5] Add support for T-HEAD C9xx PMU extensions

[Heiko Stübner]

Am Donnerstag, 8. September 2022, 18:07:59 CEST schrieb Anup Patel:
> Hi Heiko,
> 
> On Thu, Sep 8, 2022 at 7:13 PM Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > The T-HEAD C9XX cores implement functionality very similar to SSCOFPMF.
> > Instead of implementing a separate interface into SBI as done in v1,
> > we can add the C9XX tidbits with quite minimal overhead and leaverage
> > the existing SBI pmu interface including giving access to the firmware
> > counters.
> >
> > The current only hickup is the detection override in sbi_hart, where
> > I still need to find out why the MHPMCOUNTERs are not writeable
> > at _that_ point of the boot, but with this series on top of openSBI 1.1
> > and the matching kernel perf patch I get reasonable results for example
> > for:
> >
> > perf stat -e cycles -e instructions -e L1-icache-load-misses -e L1-icache-loads ls
> >
> >  Performance counter stats for 'ls':
> >
> >           17119496      cycles
> >            2867765      instructions              #    0.17  insn per cycle
> >              55929      L1-icache-load-misses     #    1.61% of all L1-icache accesses
> >            3467346      L1-icache-loads
> >
> >        0.030156216 seconds time elapsed
> >
> >        0.000000000 seconds user
> >        0.023496000 seconds sys
> >
> > changes in v3:
> > - follow Atish's advice and implement an abstraction
> >   to not pollute the core pmu with cpu-specific variants
> >
> > changes in v2:
> > - don't implement a separate interface but instead modify
> >   the sbi-pmu to allow the c9xx to use standard pmu interface
> >
> >
> > Heiko Stuebner (5):
> >   lib: sbi: do platform-specific extension population earlier
> >   lib: sbi_pmu: move pmu irq information into pmu itself
> >   lib: sbi_pmu: add ability to override methods on non-standard
> >     platforms
> >   platform: generic: add extensions_init handler and platform-override
> >   platform: generic: allwinner: add support for c9xx pmu
> 
> Please rebase this upon the latest OpenSBI. We already have
> "struct sbi_pmu_device" which platform can register and you
> can extend it in this series.

oh interesting, that sneaked past me it seems :-)
Will update to use this.

Thanks
Heiko

> >
> >  include/sbi/sbi_pmu.h                        |  16 +++
> >  lib/sbi/sbi_hart.c                           |  21 ++-
> >  lib/sbi/sbi_pmu.c                            |  31 +++++
> >  platform/generic/allwinner/sun20i-d1.c       |  76 +++++++++++
> >  platform/generic/include/platform_override.h |   1 +
> >  platform/generic/include/thead_c9xx.h        | 127 +++++++++++++++++++
> >  platform/generic/platform.c                  |   9 ++
> >  7 files changed, 274 insertions(+), 7 deletions(-)
> >  create mode 100644 platform/generic/include/thead_c9xx.h
> >
> > --
> > 2.35.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
> 

[PATCH v3 3/5] lib: sbi_pmu: add ability to override methods on non-standard platforms

[Heiko Stuebner]

Hi Drew,

Am Donnerstag, 8. September 2022, 18:02:11 CEST schrieb Andrew Jones:
> On Thu, Sep 08, 2022 at 03:42:40PM +0200, Heiko Stuebner wrote:
> > Some platforms may not implement the PMU in a standard way,
> > but instead deviate on some functionality like overflow
> > handling.
> > 
> > Implement an abstraction that those platforms can hook into.
> > This way they can still use the standard pmu interface between
> > kernel and firmware and profit from things like firmware counters,
> > without needing to create their own.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  include/sbi/sbi_pmu.h | 13 +++++++++++++
> >  lib/sbi/sbi_hart.c    |  7 +++++++
> >  lib/sbi/sbi_pmu.c     | 21 +++++++++++++++++++++
> >  3 files changed, 41 insertions(+)
> > 
> > diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> > index 1c9a997..7c8cfe7 100644
> > --- a/include/sbi/sbi_pmu.h
> > +++ b/include/sbi/sbi_pmu.h
> > @@ -28,6 +28,16 @@
> >  #define SBI_PMU_CTR_MAX	   (SBI_PMU_HW_CTR_MAX + SBI_PMU_FW_CTR_MAX)
> >  #define SBI_PMU_FIXED_CTR_MASK 0x07
> >  
> > +struct sbi_pmu_platform_ops {
> > +	int (*ctr_enable_irq_hw)(int ctr_idx);
> > +	int (*ctr_enable_ovf)(int ctr_idx);
> > +	int (*irq_bit)(void);
> > +	void (*counter_data)(unsigned int *count, unsigned int *bits);
> > +};
> > +
> > +/** Set platform-specific override ops */
> > +void sbi_pmu_set_platform_ops(struct sbi_pmu_platform_ops *ops);
> > +
> >  /** Initialize PMU */
> >  int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
> >  
> > @@ -37,6 +47,9 @@ void sbi_pmu_exit(struct sbi_scratch *scratch);
> >  /** Return the pmu irq bit depending on extension existence */
> >  int sbi_pmu_irq_bit(void);
> >  
> > +/** Allow non-standard platforms to override probed counter information */
> > +void sbi_pmu_override_counter_data(unsigned int *count, unsigned int *bits);
> > +
> >  /**
> >   * Add the hardware event to counter mapping information. This should be called
> >   * from the platform code to update the mapping table.
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index 45fbcde..6506a19 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -632,6 +632,13 @@ __mhpm_skip:
> >  #undef __check_csr_2
> >  #undef __check_csr
> >  
> > +	/**
> > +	 * Allow non-standard implementations to override the detected
> > +	 * values for number of counters and bits.
> > +	 */
> > +	sbi_pmu_override_counter_data(&hfeatures->mhpm_count,
> > +				      &hfeatures->mhpm_bits);
> > +
> >  	/* Detect if hart supports Priv v1.10 */
> >  	val = csr_read_allowed(CSR_MCOUNTEREN, (unsigned long)&trap);
> >  	if (!trap.cause)
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index efbfbb6..08fb242 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -77,6 +77,8 @@ static uint32_t num_hw_ctrs;
> >  /* Maximum number of counters available */
> >  static uint32_t total_ctrs;
> >  
> > +static struct sbi_pmu_platform_ops *pmu_platform_ops;
> > +
> >  /* Helper macros to retrieve event idx and code type */
> >  #define get_cidx_type(x) ((x & SBI_PMU_EVENT_IDX_TYPE_MASK) >> 16)
> >  #define get_cidx_code(x) (x & SBI_PMU_EVENT_IDX_CODE_MASK)
> > @@ -358,6 +360,9 @@ static int pmu_ctr_start_hw(uint32_t cidx, uint64_t ival, bool ival_update)
> >  
> >  	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> >  		pmu_ctr_enable_irq_hw(cidx);
> > +	else if (pmu_platform_ops && pmu_platform_ops->ctr_enable_irq_hw)
> > +		pmu_platform_ops->ctr_enable_irq_hw(cidx);
> > +
> >  	csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
> >  
> >  skip_inhibit_update:
> > @@ -373,6 +378,8 @@ int sbi_pmu_irq_bit(void)
> >  
> >  	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> >  		return MIP_LCOFIP;
> > +	else if (pmu_platform_ops && pmu_platform_ops->irq_bit)
> > +		return pmu_platform_ops->irq_bit();
> >  
> >  	return 0;
> >  }
> > @@ -534,6 +541,8 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
> >  	if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF))
> >  		mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) |
> >  				 MHPMEVENT_MINH | MHPMEVENT_OF;
> > +	else if (pmu_platform_ops && pmu_platform_ops->ctr_enable_ovf)
> > +		pmu_platform_ops->ctr_enable_ovf(ctr_idx);
> >  
> 
> Based on these 'if sscofpmf else pmu_platform' conditionals it looks like
> the pmu platform framework is actually specifically for a sscofpmf quirk.
> How about introducing a general quirk / pmu quirk framework and then
> adding a sscofpmf quirk. Then, where needed we'd have something like
> 
>   if (pmu_quirks[SSCOFPMF])
>      sscofpmf_ops = (struct sscofpmf_quirk_ops *)pmu_quirks[SSCOFPMF];
> 
>   if sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF)
>      ...
>   else if (sscofpmf_ops && sscofpmf_ops->func_needed_here)
>      sscofpmf_ops->func_needed_here(...);

as Anup wrote in his mail, the sbi-pmu meanwhile already got that
struct sbi_pmu_device as abstraction from somewhere else.

Which in turn handles separately from the sscofpmf extension
(similar to what I did here originally).

So for the next version I'll only use that with some extensions :-)


Heiko


> >  	/* Update the inhibit flags based on inhibit flags received from supervisor */
> >  	pmu_update_inhibit_flags(flags, &mhpmevent_val);
> > @@ -822,3 +831,15 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
> >  
> >  	return 0;
> >  }
> > +
> > +void sbi_pmu_override_counter_data(unsigned int *count,
> > +				   unsigned int *bits)
> > +{
> > +	if (pmu_platform_ops && pmu_platform_ops->counter_data)
> > +		pmu_platform_ops->counter_data(count, bits);
> > +}
> > +
> > +void sbi_pmu_set_platform_ops(struct sbi_pmu_platform_ops *ops)
> > +{
> > +	pmu_platform_ops = ops;
> > +}
>