* [PATCH 1/7] lib: sbi_pmu: Remove "event_idx" member from struct sbi_pmu_fw_event
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
@ 2022-08-25 4:51 ` Anup Patel
2022-08-25 4:51 ` [PATCH 2/7] lib: sbi_pmu: Replace sbi_pmu_ctr_read() with sbi_pmu_ctr_fw_read() Anup Patel
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Anup Patel @ 2022-08-25 4:51 UTC (permalink / raw)
To: opensbi
The "event_idx" member of struct sbi_pmu_fw_event is not used
anywhere so let us remove it.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
lib/sbi/sbi_pmu.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 5a294a2..535e5cc 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -35,9 +35,6 @@ struct sbi_pmu_hw_event {
/** Representation of a firmware event */
struct sbi_pmu_fw_event {
- /* Event associated with the particular counter */
- uint32_t event_idx;
-
/* Current value of the counter */
unsigned long curr_count;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/7] lib: sbi_pmu: Replace sbi_pmu_ctr_read() with sbi_pmu_ctr_fw_read()
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
2022-08-25 4:51 ` [PATCH 1/7] lib: sbi_pmu: Remove "event_idx" member from struct sbi_pmu_fw_event Anup Patel
@ 2022-08-25 4:51 ` Anup Patel
2022-08-25 4:51 ` [PATCH 3/7] lib: sbi_pmu: Firmware counters are always 64 bits wide Anup Patel
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Anup Patel @ 2022-08-25 4:51 UTC (permalink / raw)
To: opensbi
The "read a firmware counter" SBI call should only work for firmware
counters so let us replace sbi_pmu_ctr_read() with sbi_pmu_ctr_fw_read()
which works only on firmware counters.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
include/sbi/sbi_pmu.h | 2 +-
lib/sbi/sbi_ecall_pmu.c | 3 ++-
lib/sbi/sbi_pmu.c | 45 +++++++----------------------------------
3 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index f5e3dbd..34336f7 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -53,7 +53,7 @@ int sbi_pmu_add_hw_event_counter_map(u32 eidx_start, u32 eidx_end, u32 cmap);
int sbi_pmu_add_raw_event_counter_map(uint64_t select, uint64_t select_mask, u32 cmap);
-int sbi_pmu_ctr_read(uint32_t cidx, unsigned long *cval);
+int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval);
int sbi_pmu_ctr_stop(unsigned long cidx_base, unsigned long cidx_mask,
unsigned long flag);
diff --git a/lib/sbi/sbi_ecall_pmu.c b/lib/sbi/sbi_ecall_pmu.c
index 9ee9e81..826c8a8 100644
--- a/lib/sbi/sbi_ecall_pmu.c
+++ b/lib/sbi/sbi_ecall_pmu.c
@@ -51,7 +51,8 @@ static int sbi_ecall_pmu_handler(unsigned long extid, unsigned long funcid,
break;
case SBI_EXT_PMU_COUNTER_FW_READ:
- ret = sbi_pmu_ctr_read(regs->a0, out_val);
+ ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
+ *out_val = temp;
break;
case SBI_EXT_PMU_COUNTER_START:
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 535e5cc..250808e 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -167,50 +167,19 @@ static int pmu_ctr_validate(uint32_t cidx, uint32_t *event_idx_code)
return event_idx_type;
}
-static int pmu_ctr_read_fw(uint32_t cidx, unsigned long *cval,
- uint32_t fw_evt_code)
-{
- u32 hartid = current_hartid();
- struct sbi_pmu_fw_event fevent;
-
- fevent = fw_event_map[hartid][fw_evt_code];
- *cval = fevent.curr_count;
-
- return 0;
-}
-
-/* Add a hardware counter read for completeness for future purpose */
-static int pmu_ctr_read_hw(uint32_t cidx, uint64_t *cval)
-{
- /* Check for invalid hw counter read requests */
- if (unlikely(cidx == 1))
- return SBI_EINVAL;
-#if __riscv_xlen == 32
- uint32_t temp, temph = 0;
-
- temp = csr_read_num(CSR_MCYCLE + cidx);
- temph = csr_read_num(CSR_MCYCLEH + cidx);
- *cval = ((uint64_t)temph << 32) | temp;
-#else
- *cval = csr_read_num(CSR_MCYCLE + cidx);
-#endif
-
- return 0;
-}
-
-int sbi_pmu_ctr_read(uint32_t cidx, unsigned long *cval)
+int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
{
int event_idx_type;
uint32_t event_code;
- uint64_t cval64;
+ u32 hartid = current_hartid();
+ struct sbi_pmu_fw_event *fevent;
event_idx_type = pmu_ctr_validate(cidx, &event_code);
- if (event_idx_type < 0)
+ if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
return SBI_EINVAL;
- else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
- pmu_ctr_read_fw(cidx, cval, event_code);
- else
- pmu_ctr_read_hw(cidx, &cval64);
+
+ fevent = &fw_event_map[hartid][event_code];
+ *cval = fevent->curr_count;
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/7] lib: sbi_pmu: Firmware counters are always 64 bits wide
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
2022-08-25 4:51 ` [PATCH 1/7] lib: sbi_pmu: Remove "event_idx" member from struct sbi_pmu_fw_event Anup Patel
2022-08-25 4:51 ` [PATCH 2/7] lib: sbi_pmu: Replace sbi_pmu_ctr_read() with sbi_pmu_ctr_fw_read() Anup Patel
@ 2022-08-25 4:51 ` Anup Patel
2022-08-25 4:51 ` [PATCH 4/7] lib: sbi_pmu: Simplify FW counters to reduce memory usage Anup Patel
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Anup Patel @ 2022-08-25 4:51 UTC (permalink / raw)
To: opensbi
As-per SBI specification, all firmware counters are always 64 bits
wide so let us update the SBI PMU implementation to reflect this fact.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
lib/sbi/sbi_pmu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 250808e..1c01d34 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -36,7 +36,7 @@ struct sbi_pmu_hw_event {
/** Representation of a firmware event */
struct sbi_pmu_fw_event {
/* Current value of the counter */
- unsigned long curr_count;
+ uint64_t curr_count;
/* A flag indicating pmu event monitoring is started */
bool bStarted;
@@ -719,8 +719,8 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
} else {
/* it's a firmware counter */
cinfo.type = SBI_PMU_CTR_TYPE_FW;
- /* Firmware counters are XLEN bits wide */
- cinfo.width = BITS_PER_LONG - 1;
+ /* Firmware counters are always 64 bits wide */
+ cinfo.width = 63;
}
*ctr_info = cinfo.value;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/7] lib: sbi_pmu: Simplify FW counters to reduce memory usage
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
` (2 preceding siblings ...)
2022-08-25 4:51 ` [PATCH 3/7] lib: sbi_pmu: Firmware counters are always 64 bits wide Anup Patel
@ 2022-08-25 4:51 ` Anup Patel
2022-08-25 4:51 ` [PATCH 5/7] lib: sbi_pmu: Add custom PMU device operations Anup Patel
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Anup Patel @ 2022-08-25 4:51 UTC (permalink / raw)
To: opensbi
Currently, we have 32 elements (i.e. SBI_PMU_FW_EVENT_MAX) array of
"struct sbi_pmu_fw_event" for each of 128 possible HARTs
(i.e. SBI_HARTMASK_MAX_BITS).
To reduce memory usage of OpenSBI, we update FW counter implementation
as follows:
1) Remove SBI_PMU_FW_EVENT_MAX
2) Remove "struct sbi_pmu_fw_event"
3) Create per-HART bitmap of XLEN bits to track FW counters
which are started on each HART
4) Create per-HART uint64_t array to track values of FW
counters on each HART.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
include/sbi/sbi_pmu.h | 3 --
lib/sbi/sbi_pmu.c | 78 ++++++++++++++++++++-----------------------
2 files changed, 36 insertions(+), 45 deletions(-)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index 34336f7..39cf007 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -19,9 +19,6 @@
/* Maximum number of hardware events that can mapped by OpenSBI */
#define SBI_PMU_HW_EVENT_MAX 256
-/* Maximum number of firmware events that can mapped by OpenSBI */
-#define SBI_PMU_FW_EVENT_MAX 32
-
/* Counter related macros */
#define SBI_PMU_FW_CTR_MAX 16
#define SBI_PMU_HW_CTR_MAX 32
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 1c01d34..26c9406 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -33,15 +33,6 @@ struct sbi_pmu_hw_event {
uint64_t select_mask;
};
-/** Representation of a firmware event */
-struct sbi_pmu_fw_event {
- /* Current value of the counter */
- uint64_t curr_count;
-
- /* A flag indicating pmu event monitoring is started */
- bool bStarted;
-};
-
/* Information about PMU counters as per SBI specification */
union sbi_pmu_ctr_info {
unsigned long value;
@@ -63,8 +54,14 @@ static struct sbi_pmu_hw_event hw_event_map[SBI_PMU_HW_EVENT_MAX] = {0};
/* counter to enabled event mapping */
static uint32_t active_events[SBI_HARTMASK_MAX_BITS][SBI_PMU_HW_CTR_MAX + SBI_PMU_FW_CTR_MAX];
-/* Contains all the information about firmwares events */
-static struct sbi_pmu_fw_event fw_event_map[SBI_HARTMASK_MAX_BITS][SBI_PMU_FW_EVENT_MAX] = {0};
+/* Bitmap of firmware counters started on each HART */
+#if SBI_PMU_FW_CTR_MAX >= BITS_PER_LONG
+#error "Can't handle firmware counters beyond BITS_PER_LONG"
+#endif
+static unsigned long fw_counters_started[SBI_HARTMASK_MAX_BITS];
+
+/* Values of firmwares counters on each HART */
+static uint64_t fw_counters_value[SBI_HARTMASK_MAX_BITS][SBI_PMU_FW_CTR_MAX] = {0};
/* Maximum number of hardware events available */
static uint32_t num_hw_events;
@@ -172,14 +169,12 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
int event_idx_type;
uint32_t event_code;
u32 hartid = current_hartid();
- struct sbi_pmu_fw_event *fevent;
event_idx_type = pmu_ctr_validate(cidx, &event_code);
if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
return SBI_EINVAL;
- fevent = &fw_event_map[hartid][event_code];
- *cval = fevent->curr_count;
+ *cval = fw_counters_value[hartid][cidx - num_hw_ctrs];
return 0;
}
@@ -333,16 +328,13 @@ skip_inhibit_update:
return 0;
}
-static int pmu_ctr_start_fw(uint32_t cidx, uint32_t fw_evt_code,
- uint64_t ival, bool ival_update)
+static int pmu_ctr_start_fw(uint32_t cidx, uint64_t ival, bool ival_update)
{
u32 hartid = current_hartid();
- struct sbi_pmu_fw_event *fevent;
- fevent = &fw_event_map[hartid][fw_evt_code];
if (ival_update)
- fevent->curr_count = ival;
- fevent->bStarted = TRUE;
+ fw_counters_value[hartid][cidx - num_hw_ctrs] = ival;
+ fw_counters_started[hartid] |= BIT(cidx - num_hw_ctrs);
return 0;
}
@@ -369,7 +361,7 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
/* Continue the start operation for other counters */
continue;
else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
- ret = pmu_ctr_start_fw(cidx, event_code, ival, bUpdate);
+ ret = pmu_ctr_start_fw(cidx, ival, bUpdate);
else
ret = pmu_ctr_start_hw(cidx, ival, bUpdate);
}
@@ -399,11 +391,9 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
return SBI_EALREADY_STOPPED;
}
-static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t fw_evt_code)
+static int pmu_ctr_stop_fw(uint32_t cidx)
{
- u32 hartid = current_hartid();
-
- fw_event_map[hartid][fw_evt_code].bStarted = FALSE;
+ fw_counters_started[current_hartid()] &= ~BIT(cidx - num_hw_ctrs);
return 0;
}
@@ -444,7 +434,7 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
continue;
else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
- ret = pmu_ctr_stop_fw(cidx, event_code);
+ ret = pmu_ctr_stop_fw(cidx);
else
ret = pmu_ctr_stop_hw(cidx);
@@ -624,8 +614,6 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
int ctr_idx = SBI_ENOTSUPP;
u32 hartid = current_hartid();
int event_type;
- struct sbi_pmu_fw_event *fevent;
- uint32_t fw_evt_code;
/* Do a basic sanity check of counter base & mask */
if ((cidx_base + sbi_fls(cidx_mask)) >= total_ctrs)
@@ -665,30 +653,36 @@ skip_match:
if (flags & SBI_PMU_CFG_FLAG_AUTO_START)
pmu_ctr_start_hw(ctr_idx, 0, false);
} else if (event_type == SBI_PMU_EVENT_TYPE_FW) {
- fw_evt_code = get_cidx_code(event_idx);
- fevent = &fw_event_map[hartid][fw_evt_code];
if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
- fevent->curr_count = 0;
+ fw_counters_value[hartid][ctr_idx - num_hw_ctrs] = 0;
if (flags & SBI_PMU_CFG_FLAG_AUTO_START)
- fevent->bStarted = TRUE;
+ fw_counters_started[hartid] |= BIT(ctr_idx - num_hw_ctrs);
}
return ctr_idx;
}
-inline int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id fw_id)
+int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id fw_id)
{
- u32 hartid = current_hartid();
- struct sbi_pmu_fw_event *fevent;
+ u32 cidx, hartid = current_hartid();
+ uint64_t *fcounter = NULL;
+
+ if (likely(!fw_counters_started[hartid]))
+ return 0;
if (unlikely(fw_id >= SBI_PMU_FW_MAX))
return SBI_EINVAL;
- fevent = &fw_event_map[hartid][fw_id];
+ for (cidx = num_hw_ctrs; cidx < total_ctrs; cidx++) {
+ if (get_cidx_code(active_events[hartid][cidx]) == fw_id &&
+ (fw_counters_started[hartid] & BIT(cidx - num_hw_ctrs))) {
+ fcounter = &fw_counters_value[hartid][cidx - num_hw_ctrs];
+ break;
+ }
+ }
- /* PMU counters will be only enabled during performance debugging */
- if (unlikely(fevent->bStarted))
- fevent->curr_count++;
+ if (fcounter)
+ (*fcounter)++;
return 0;
}
@@ -735,9 +729,9 @@ static void pmu_reset_event_map(u32 hartid)
/* Initialize the counter to event mapping table */
for (j = 3; j < total_ctrs; j++)
active_events[hartid][j] = SBI_PMU_EVENT_IDX_INVALID;
- for (j = 0; j < SBI_PMU_FW_EVENT_MAX; j++)
- sbi_memset(&fw_event_map[hartid][j], 0,
- sizeof(struct sbi_pmu_fw_event));
+ for (j = 0; j < SBI_PMU_FW_CTR_MAX; j++)
+ fw_counters_value[hartid][j] = 0;
+ fw_counters_started[hartid] = 0;
}
void sbi_pmu_exit(struct sbi_scratch *scratch)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/7] lib: sbi_pmu: Add custom PMU device operations
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
` (3 preceding siblings ...)
2022-08-25 4:51 ` [PATCH 4/7] lib: sbi_pmu: Simplify FW counters to reduce memory usage Anup Patel
@ 2022-08-25 4:51 ` Anup Patel
2022-08-25 8:05 ` Andrew Jones
2022-08-25 4:51 ` [PATCH 6/7] lib: sbi: Print platform PMU device at boot-time Anup Patel
` (2 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Anup Patel @ 2022-08-25 4:51 UTC (permalink / raw)
To: opensbi
We extend SBI PMU implementation to allow custom PMU device operations
which a platform can use for platform specific quirks.
The custom PMU device operations added by this patch include:
1) Operations to allow a platform implement custom firmware events.
These custom firmware events can be SBI vendor extension related
events or platform specific per-HART events are not possible to
count through HPM CSRs.
2) Operations to allow a platform implement custom way for enabling
(or disabling) an overflow interrupt (e.g. T-Head C9xx).
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
include/sbi/sbi_pmu.h | 57 ++++++++++++++++++++++
lib/sbi/sbi_pmu.c | 108 ++++++++++++++++++++++++++++++++++--------
2 files changed, 144 insertions(+), 21 deletions(-)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index 39cf007..d787575 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -25,6 +25,63 @@
#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_device {
+ /** Name of the PMU platform device */
+ char name[32];
+
+ /**
+ * Validate event code of custom firmware event
+ * Note: SBI_PMU_FW_MAX <= event_idx_code
+ */
+ int (*fw_event_validate_code)(uint32_t event_idx_code);
+
+ /**
+ * Match custom firmware counter with custom firmware event
+ * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
+ */
+ bool (*fw_counter_match_code)(uint32_t counter_index,
+ uint32_t event_idx_code);
+
+ /**
+ * Read value of custom firmware counter
+ * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
+ */
+ uint64_t (*fw_counter_read_value)(uint32_t counter_index);
+
+ /**
+ * Start custom firmware counter
+ * Note: SBI_PMU_FW_MAX <= event_idx_code
+ * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
+ */
+ int (*fw_counter_start)(uint32_t counter_index,
+ uint32_t event_idx_code,
+ uint64_t init_val, bool init_val_update);
+
+ /**
+ * Stop custom firmware counter
+ * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
+ */
+ int (*fw_counter_stop)(uint32_t counter_index);
+
+ /**
+ * Custom enable irq for hardware counter
+ * Note: 0 <= counter_index < SBI_PMU_HW_CTR_MAX
+ */
+ void (*hw_counter_enable_irq)(uint32_t counter_index);
+
+ /**
+ * Custom disable irq for hardware counter
+ * Note: 0 <= counter_index < SBI_PMU_HW_CTR_MAX
+ */
+ void (*hw_counter_disable_irq)(uint32_t counter_index);
+};
+
+/** Get the PMU platform device */
+const struct sbi_pmu_device *sbi_pmu_get_device(void);
+
+/** Set the PMU platform device */
+void sbi_pmu_set_device(const struct sbi_pmu_device *dev);
+
/** Initialize PMU */
int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot);
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 26c9406..87e1ff8 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -48,6 +48,9 @@ union sbi_pmu_ctr_info {
};
};
+/* Platform specific PMU device */
+static const struct sbi_pmu_device *pmu_dev = NULL;
+
/* Mapping between event range and possible counters */
static struct sbi_pmu_hw_event hw_event_map[SBI_PMU_HW_EVENT_MAX] = {0};
@@ -113,7 +116,11 @@ static int pmu_event_validate(unsigned long event_idx)
event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
break;
case SBI_PMU_EVENT_TYPE_FW:
- event_idx_code_max = SBI_PMU_FW_MAX;
+ if (SBI_PMU_FW_MAX <= event_idx_code &&
+ pmu_dev && pmu_dev->fw_event_validate_code)
+ return pmu_dev->fw_event_validate_code(event_idx_code);
+ else
+ event_idx_code_max = SBI_PMU_FW_MAX;
break;
case SBI_PMU_EVENT_TYPE_HW_CACHE:
cache_ops_result = event_idx_code &
@@ -174,6 +181,11 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
return SBI_EINVAL;
+ if (SBI_PMU_FW_MAX <= event_code &&
+ pmu_dev && pmu_dev->fw_counter_read_value)
+ fw_counters_value[hartid][cidx - num_hw_ctrs] =
+ pmu_dev->fw_counter_read_value(cidx - num_hw_ctrs);
+
*cval = fw_counters_value[hartid][cidx - num_hw_ctrs];
return 0;
@@ -319,6 +331,8 @@ 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);
+ if (pmu_dev && pmu_dev->hw_counter_enable_irq)
+ pmu_dev->hw_counter_enable_irq(cidx);
csr_write(CSR_MCOUNTINHIBIT, mctr_inhbt);
skip_inhibit_update:
@@ -328,10 +342,21 @@ skip_inhibit_update:
return 0;
}
-static int pmu_ctr_start_fw(uint32_t cidx, uint64_t ival, bool ival_update)
+static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
+ uint64_t ival, bool ival_update)
{
+ int ret;
u32 hartid = current_hartid();
+ if (SBI_PMU_FW_MAX <= event_code &&
+ pmu_dev && pmu_dev->fw_counter_start) {
+ ret = pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
+ event_code,
+ ival, ival_update);
+ if (ret)
+ return ret;
+ }
+
if (ival_update)
fw_counters_value[hartid][cidx - num_hw_ctrs] = ival;
fw_counters_started[hartid] |= BIT(cidx - num_hw_ctrs);
@@ -361,7 +386,7 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
/* Continue the start operation for other counters */
continue;
else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
- ret = pmu_ctr_start_fw(cidx, ival, bUpdate);
+ ret = pmu_ctr_start_fw(cidx, event_code, ival, bUpdate);
else
ret = pmu_ctr_start_hw(cidx, ival, bUpdate);
}
@@ -391,8 +416,17 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
return SBI_EALREADY_STOPPED;
}
-static int pmu_ctr_stop_fw(uint32_t cidx)
+static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
{
+ int ret;
+
+ if (SBI_PMU_FW_MAX <= event_code &&
+ pmu_dev && pmu_dev->fw_counter_stop) {
+ ret = pmu_dev->fw_counter_stop(cidx - num_hw_ctrs);
+ if (ret)
+ return ret;
+ }
+
fw_counters_started[current_hartid()] &= ~BIT(cidx - num_hw_ctrs);
return 0;
@@ -434,7 +468,7 @@ int sbi_pmu_ctr_stop(unsigned long cbase, unsigned long cmask,
continue;
else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
- ret = pmu_ctr_stop_fw(cidx);
+ ret = pmu_ctr_stop_fw(cidx, event_code);
else
ret = pmu_ctr_stop_hw(cidx);
@@ -481,6 +515,9 @@ static int pmu_update_hw_mhpmevent(struct sbi_pmu_hw_event *hw_evt, int ctr_idx,
mhpmevent_val = (mhpmevent_val & ~MHPMEVENT_SSCOF_MASK) |
MHPMEVENT_MINH | MHPMEVENT_OF;
+ if (pmu_dev && pmu_dev->hw_counter_disable_irq)
+ pmu_dev->hw_counter_disable_irq(ctr_idx);
+
/* Update the inhibit flags based on inhibit flags received from supervisor */
pmu_update_inhibit_flags(flags, &mhpmevent_val);
@@ -588,21 +625,26 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
* Thus, select the first available fw counter after sanity
* check.
*/
-static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask, u32 hartid)
+static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
+ uint32_t event_code, u32 hartid)
{
- int i = 0;
- int fw_base;
- unsigned long ctr_mask = cmask << cbase;
+ int i, cidx;
- if (cbase < num_hw_ctrs)
- fw_base = num_hw_ctrs;
- else
- fw_base = cbase;
+ for_each_set_bit(i, &cmask, BITS_PER_LONG) {
+ cidx = i + cbase;
+ if (cidx < num_hw_ctrs || total_ctrs <= cidx)
+ continue;
+ if (active_events[hartid][i] != SBI_PMU_EVENT_IDX_INVALID)
+ continue;
+ if (SBI_PMU_FW_MAX <= event_code &&
+ pmu_dev && pmu_dev->fw_counter_match_code) {
+ if (!pmu_dev->fw_counter_match_code(cidx - num_hw_ctrs,
+ event_code))
+ continue;
+ }
- for (i = fw_base; i < total_ctrs; i++)
- if ((active_events[hartid][i] == SBI_PMU_EVENT_IDX_INVALID) &&
- ((1UL << i) & ctr_mask))
- return i;
+ return i;
+ }
return SBI_ENOTSUPP;
}
@@ -611,8 +653,8 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
unsigned long flags, unsigned long event_idx,
uint64_t event_data)
{
- int ctr_idx = SBI_ENOTSUPP;
- u32 hartid = current_hartid();
+ int ret, ctr_idx = SBI_ENOTSUPP;
+ u32 event_code, hartid = current_hartid();
int event_type;
/* Do a basic sanity check of counter base & mask */
@@ -622,6 +664,7 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
event_type = pmu_event_validate(event_idx);
if (event_type < 0)
return SBI_EINVAL;
+ event_code = get_cidx_code(event_idx);
if (flags & SBI_PMU_CFG_FLAG_SKIP_MATCH) {
/* The caller wants to skip the match because it already knows the
@@ -636,7 +679,7 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
if (event_type == SBI_PMU_EVENT_TYPE_FW) {
/* Any firmware counter can be used track any firmware event */
- ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, hartid);
+ ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, event_code, hartid);
} else {
ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
event_data);
@@ -655,8 +698,18 @@ skip_match:
} else if (event_type == SBI_PMU_EVENT_TYPE_FW) {
if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
fw_counters_value[hartid][ctr_idx - num_hw_ctrs] = 0;
- if (flags & SBI_PMU_CFG_FLAG_AUTO_START)
+ if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
+ if (SBI_PMU_FW_MAX <= event_code &&
+ pmu_dev && pmu_dev->fw_counter_start) {
+ ret = pmu_dev->fw_counter_start(
+ ctr_idx - num_hw_ctrs, event_code,
+ fw_counters_value[hartid][ctr_idx - num_hw_ctrs],
+ true);
+ if (ret)
+ return ret;
+ }
fw_counters_started[hartid] |= BIT(ctr_idx - num_hw_ctrs);
+ }
}
return ctr_idx;
@@ -734,6 +787,19 @@ static void pmu_reset_event_map(u32 hartid)
fw_counters_started[hartid] = 0;
}
+const struct sbi_pmu_device *sbi_pmu_get_device(void)
+{
+ return pmu_dev;
+}
+
+void sbi_pmu_set_device(const struct sbi_pmu_device *dev)
+{
+ if (!dev || pmu_dev)
+ return;
+
+ pmu_dev = dev;
+}
+
void sbi_pmu_exit(struct sbi_scratch *scratch)
{
u32 hartid = current_hartid();
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/7] lib: sbi_pmu: Add custom PMU device operations
2022-08-25 4:51 ` [PATCH 5/7] lib: sbi_pmu: Add custom PMU device operations Anup Patel
@ 2022-08-25 8:05 ` Andrew Jones
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2022-08-25 8:05 UTC (permalink / raw)
To: opensbi
On Thu, Aug 25, 2022 at 10:21:42AM +0530, Anup Patel wrote:
> We extend SBI PMU implementation to allow custom PMU device operations
> which a platform can use for platform specific quirks.
>
> The custom PMU device operations added by this patch include:
> 1) Operations to allow a platform implement custom firmware events.
> These custom firmware events can be SBI vendor extension related
> events or platform specific per-HART events are not possible to
> count through HPM CSRs.
> 2) Operations to allow a platform implement custom way for enabling
> (or disabling) an overflow interrupt (e.g. T-Head C9xx).
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> include/sbi/sbi_pmu.h | 57 ++++++++++++++++++++++
> lib/sbi/sbi_pmu.c | 108 ++++++++++++++++++++++++++++++++++--------
> 2 files changed, 144 insertions(+), 21 deletions(-)
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 6/7] lib: sbi: Print platform PMU device at boot-time
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
` (4 preceding siblings ...)
2022-08-25 4:51 ` [PATCH 5/7] lib: sbi_pmu: Add custom PMU device operations Anup Patel
@ 2022-08-25 4:51 ` Anup Patel
[not found] ` <CAOnJCUL2kD_8dbwyBOX1XbaGizUqRAymPUhRWddavtG=aLojMA@mail.gmail.com>
2022-08-25 4:51 ` [PATCH 7/7] include: sbi: Reduce includes in sbi_pmu.h Anup Patel
2022-09-01 11:30 ` [PATCH 0/7] OpenSBI PMU improvements Anup Patel
7 siblings, 1 reply; 11+ messages in thread
From: Anup Patel @ 2022-08-25 4:51 UTC (permalink / raw)
To: opensbi
Let us print the platform PMU device name at the boot-time so that users
know whether the underlying platform has custom per-HART PMU operations.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
lib/sbi/sbi_init.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/sbi/sbi_init.c b/lib/sbi/sbi_init.c
index d57efa7..a8500e5 100644
--- a/lib/sbi/sbi_init.c
+++ b/lib/sbi/sbi_init.c
@@ -63,6 +63,7 @@ static void sbi_boot_print_banner(struct sbi_scratch *scratch)
static void sbi_boot_print_general(struct sbi_scratch *scratch)
{
char str[128];
+ const struct sbi_pmu_device *pdev;
const struct sbi_hsm_device *hdev;
const struct sbi_ipi_device *idev;
const struct sbi_timer_device *tdev;
@@ -93,6 +94,9 @@ static void sbi_boot_print_general(struct sbi_scratch *scratch)
hdev = sbi_hsm_get_device();
sbi_printf("Platform HSM Device : %s\n",
(hdev) ? hdev->name : "---");
+ pdev = sbi_pmu_get_device();
+ sbi_printf("Platform PMU Device : %s\n",
+ (pdev) ? pdev->name : "---");
srdev = sbi_system_reset_get_device(SBI_SRST_RESET_TYPE_COLD_REBOOT, 0);
sbi_printf("Platform Reboot Device : %s\n",
(srdev) ? srdev->name : "---");
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 7/7] include: sbi: Reduce includes in sbi_pmu.h
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
` (5 preceding siblings ...)
2022-08-25 4:51 ` [PATCH 6/7] lib: sbi: Print platform PMU device at boot-time Anup Patel
@ 2022-08-25 4:51 ` Anup Patel
2022-09-01 11:30 ` [PATCH 0/7] OpenSBI PMU improvements Anup Patel
7 siblings, 0 replies; 11+ messages in thread
From: Anup Patel @ 2022-08-25 4:51 UTC (permalink / raw)
To: opensbi
The sbi_pmu.h should only include minimal required headers whereas
sbi_pmu.c should include all required headers.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
include/sbi/sbi_pmu.h | 5 ++---
lib/sbi/sbi_pmu.c | 2 ++
lib/sbi/sbi_trap.c | 1 +
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index d787575..a2ce42d 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -11,9 +11,8 @@
#define __SBI_PMU_H__
#include <sbi/sbi_types.h>
-#include <sbi/sbi_hartmask.h>
-#include <sbi/sbi_scratch.h>
-#include <sbi/sbi_ecall_interface.h>
+
+struct sbi_scratch;
/* Event related macros */
/* Maximum number of hardware events that can mapped by OpenSBI */
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 87e1ff8..214d5e8 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -10,7 +10,9 @@
#include <sbi/riscv_asm.h>
#include <sbi/sbi_bitops.h>
#include <sbi/sbi_console.h>
+#include <sbi/sbi_ecall_interface.h>
#include <sbi/sbi_hart.h>
+#include <sbi/sbi_hartmask.h>
#include <sbi/sbi_platform.h>
#include <sbi/sbi_pmu.h>
#include <sbi/sbi_scratch.h>
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 0e30946..4c1339e 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -9,6 +9,7 @@
#include <sbi/riscv_asm.h>
#include <sbi/riscv_encoding.h>
+#include <sbi/sbi_bitops.h>
#include <sbi/sbi_console.h>
#include <sbi/sbi_ecall.h>
#include <sbi/sbi_error.h>
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 0/7] OpenSBI PMU improvements
2022-08-25 4:51 [PATCH 0/7] OpenSBI PMU improvements Anup Patel
` (6 preceding siblings ...)
2022-08-25 4:51 ` [PATCH 7/7] include: sbi: Reduce includes in sbi_pmu.h Anup Patel
@ 2022-09-01 11:30 ` Anup Patel
7 siblings, 0 replies; 11+ messages in thread
From: Anup Patel @ 2022-09-01 11:30 UTC (permalink / raw)
To: opensbi
On Thu, Aug 25, 2022 at 10:22 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> This series aims to improve SBI PMU implementation in following ways:
> 1) Optimize memory usage by reducing global data to track per-HART firmware
> counters (almost 80KB saved)
> 2) Allow platform to implement custom firmware counters and quirks related
> to hardware counters (This will be useful for T-Head)
> 3) Few other fixes for better SBI spec compliance
>
> These patches can also be found in sbi_pmu_imp_v1 branch at:
> https://github.com/avpatel/opensbi.git
>
> Anup Patel (7):
> lib: sbi_pmu: Remove "event_idx" member from struct sbi_pmu_fw_event
> lib: sbi_pmu: Replace sbi_pmu_ctr_read() with sbi_pmu_ctr_fw_read()
> lib: sbi_pmu: Firmware counters are always 64 bits wide
> lib: sbi_pmu: Simplify FW counters to reduce memory usage
> lib: sbi_pmu: Add custom PMU device operations
> lib: sbi: Print platform PMU device at boot-time
> include: sbi: Reduce includes in sbi_pmu.h
Applied this series to the riscv/opensbi repo.
Andrew (Drew) had already reviewed most of the patches
before it landed on OpenSBI mailing list so I have added
his Reviewed-by.
Thanks,
Anup
>
> include/sbi/sbi_pmu.h | 67 ++++++++++--
> lib/sbi/sbi_ecall_pmu.c | 3 +-
> lib/sbi/sbi_init.c | 4 +
> lib/sbi/sbi_pmu.c | 218 +++++++++++++++++++++++-----------------
> lib/sbi/sbi_trap.c | 1 +
> 5 files changed, 190 insertions(+), 103 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread