* [PATCH v3 0/8] SBI PMU firmware counters and events improvement
@ 2023-03-09 5:51 Mayuresh Chitale
2023-03-09 5:51 ` [PATCH v3 1/8] lib: sbi_pmu: add callback for counter width Mayuresh Chitale
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
This patchset implements the improvements described in the following
proposal from Anup Patel:
https://lists.riscv.org/g/tech-prs/message/102
Changes in v3:
- Break patch 4 from v2 into multiple smaller patches
- Add reviewed-by tags
Changes in v2:
- Update Risc V platform specific firmware event handling as per
the SBI spec update: https://lists.riscv.org/g/tech-prs/message/211
Mayuresh Chitale (8):
lib: sbi_pmu: add callback for counter width
lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi
lib: sbi_pmu: Reserve space for implementation specific firmware
events
lib: sbi_pmu: Rename fw_counter_value
lib: sbi_pmu: Update sbi_pmu dev ops
lib: sbi_pmu: Use dedicated event code for platform firmware events
lib: sbi_pmu: Introduce fw_counter_write_value API
lib: sbi_pmu: Add hartid parameter PMU device ops
include/sbi/sbi_ecall_interface.h | 12 +++
include/sbi/sbi_pmu.h | 31 ++++---
lib/sbi/sbi_ecall_pmu.c | 8 ++
lib/sbi/sbi_pmu.c | 141 +++++++++++++++++++++---------
4 files changed, 141 insertions(+), 51 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/8] lib: sbi_pmu: add callback for counter width
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:18 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 2/8] lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi Mayuresh Chitale
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
This patch adds a callback to fetch the number of bits implemented for a
custom firmware counter. If the callback fails or is not implemented then
width defaults to 63.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
include/sbi/sbi_pmu.h | 5 +++++
lib/sbi/sbi_pmu.c | 6 ++++++
2 files changed, 11 insertions(+)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index c365243..b3b75c1 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -41,6 +41,11 @@ struct sbi_pmu_device {
bool (*fw_counter_match_code)(uint32_t counter_index,
uint32_t event_idx_code);
+ /**
+ * Fetch the max width of this counter in number of bits.
+ */
+ int (*fw_counter_width)(void);
+
/**
* Read value of custom firmware counter
* Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 154dbda..a99c045 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -761,6 +761,7 @@ unsigned long sbi_pmu_num_ctr(void)
int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
{
+ int width;
union sbi_pmu_ctr_info cinfo = {0};
struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
@@ -782,6 +783,11 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
cinfo.type = SBI_PMU_CTR_TYPE_FW;
/* Firmware counters are always 64 bits wide */
cinfo.width = 63;
+ if (pmu_dev && pmu_dev->fw_counter_width) {
+ width = pmu_dev->fw_counter_width();
+ if (width)
+ cinfo.width = width - 1;
+ }
}
*ctr_info = cinfo.value;
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/8] lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
2023-03-09 5:51 ` [PATCH v3 1/8] lib: sbi_pmu: add callback for counter width Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:18 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events Mayuresh Chitale
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
To support 64 bit firmware counters on RV32 systems, we implement
sbi_pmu_counter_fw_read_hi() which returns the upper 32 bits of
the firmware counter value. On RV64 (or higher) systems, this
function will always return zero.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
include/sbi/sbi_ecall_interface.h | 1 +
lib/sbi/sbi_ecall_pmu.c | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
index 4c378c3..d448212 100644
--- a/include/sbi/sbi_ecall_interface.h
+++ b/include/sbi/sbi_ecall_interface.h
@@ -101,6 +101,7 @@
#define SBI_EXT_PMU_COUNTER_START 0x3
#define SBI_EXT_PMU_COUNTER_STOP 0x4
#define SBI_EXT_PMU_COUNTER_FW_READ 0x5
+#define SBI_EXT_PMU_COUNTER_FW_READ_HI 0x6
/** General pmu event codes specified in SBI PMU extension */
enum sbi_pmu_hw_generic_events_t {
diff --git a/lib/sbi/sbi_ecall_pmu.c b/lib/sbi/sbi_ecall_pmu.c
index 826c8a8..2d1e669 100644
--- a/lib/sbi/sbi_ecall_pmu.c
+++ b/lib/sbi/sbi_ecall_pmu.c
@@ -54,6 +54,14 @@ static int sbi_ecall_pmu_handler(unsigned long extid, unsigned long funcid,
ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
*out_val = temp;
break;
+ case SBI_EXT_PMU_COUNTER_FW_READ_HI:
+#if __riscv_xlen == 32
+ ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
+ *out_val = temp >> 32
+#else
+ *out_val = 0;
+#endif
+ break;
case SBI_EXT_PMU_COUNTER_START:
#if __riscv_xlen == 32
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
2023-03-09 5:51 ` [PATCH v3 1/8] lib: sbi_pmu: add callback for counter width Mayuresh Chitale
2023-03-09 5:51 ` [PATCH v3 2/8] lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:19 ` Anup Patel
2023-03-09 9:16 ` Andrew Jones
2023-03-09 5:51 ` [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value Mayuresh Chitale
` (4 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
We reserve space for SBI implementation specific custom firmware
events which can be used by M-mode firmwares and HS-mode hypervisors
for their own use. This reserved space is intentionally large to
ensure that SBI implementation has enough space to accommodate
platform specific firmware events as well.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
include/sbi/sbi_ecall_interface.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
index d448212..8d1cb97 100644
--- a/include/sbi/sbi_ecall_interface.h
+++ b/include/sbi/sbi_ecall_interface.h
@@ -185,6 +185,17 @@ enum sbi_pmu_fw_event_code_id {
SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
SBI_PMU_FW_MAX,
+ /*
+ * Event codes 22 to 255 are reserved for future use.
+ * Event codes 256 to 65534 are reserved for SBI implementation
+ * specific custom firmware events.
+ */
+ SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
+ /*
+ * Event code 0xFFFF is used for platform specific firmware
+ * events where the event data contains any event specific information.
+ */
+ SBI_PMU_FW_PLATFORM = 0xFFFF,
};
/** SBI PMU event idx type */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
` (2 preceding siblings ...)
2023-03-09 5:51 ` [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:19 ` Anup Patel
2023-03-09 9:19 ` Andrew Jones
2023-03-09 5:51 ` [PATCH v3 5/8] lib: sbi_pmu: Update sbi_pmu dev ops Mayuresh Chitale
` (3 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
Rename and reuse fw_counter_value arrary to save both the counter values
for the SBI firmware events and event data for the SBI platform specific
firmware events.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
lib/sbi/sbi_pmu.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index a99c045..1a3c44d 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -65,8 +65,12 @@ static uint32_t active_events[SBI_HARTMASK_MAX_BITS][SBI_PMU_HW_CTR_MAX + SBI_PM
#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};
+/*
+ * Counter values for SBI firmware events and event codes for platform
+ * firmware events. Both are mutually exclusive and hence can optimally share
+ * the same memory.
+ */
+static uint64_t fw_counters_data[SBI_HARTMASK_MAX_BITS][SBI_PMU_FW_CTR_MAX] = {0};
/* Maximum number of hardware events available */
static uint32_t num_hw_events;
@@ -185,10 +189,10 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
if (SBI_PMU_FW_MAX <= event_code &&
pmu_dev && pmu_dev->fw_counter_read_value)
- fw_counters_value[hartid][cidx - num_hw_ctrs] =
+ fw_counters_data[hartid][cidx - num_hw_ctrs] =
pmu_dev->fw_counter_read_value(cidx - num_hw_ctrs);
- *cval = fw_counters_value[hartid][cidx - num_hw_ctrs];
+ *cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
return 0;
}
@@ -372,7 +376,7 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
}
if (ival_update)
- fw_counters_value[hartid][cidx - num_hw_ctrs] = ival;
+ fw_counters_data[hartid][cidx - num_hw_ctrs] = ival;
fw_counters_started[hartid] |= BIT(cidx - num_hw_ctrs);
return 0;
@@ -711,13 +715,13 @@ skip_match:
pmu_ctr_start_hw(ctr_idx, 0, false);
} 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;
+ fw_counters_data[hartid][ctr_idx - num_hw_ctrs] = 0;
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],
+ fw_counters_data[hartid][ctr_idx - num_hw_ctrs],
true);
if (ret)
return ret;
@@ -743,7 +747,7 @@ int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id 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];
+ fcounter = &fw_counters_data[hartid][cidx - num_hw_ctrs];
break;
}
}
@@ -803,7 +807,7 @@ static void pmu_reset_event_map(u32 hartid)
for (j = 3; j < total_ctrs; j++)
active_events[hartid][j] = SBI_PMU_EVENT_IDX_INVALID;
for (j = 0; j < SBI_PMU_FW_CTR_MAX; j++)
- fw_counters_value[hartid][j] = 0;
+ fw_counters_data[hartid][j] = 0;
fw_counters_started[hartid] = 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 5/8] lib: sbi_pmu: Update sbi_pmu dev ops
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
` (3 preceding siblings ...)
2023-03-09 5:51 ` [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:21 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 6/8] lib: sbi_pmu: Use dedicated event code for platform firmware events Mayuresh Chitale
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
Update fw_event_validate_code, fw_counter_match_code and fw_counter_start
ops which used a 32 bit event code to use the 64 bit event data instead.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
include/sbi/sbi_pmu.h | 9 ++++-----
lib/sbi/sbi_pmu.c | 30 +++++++++++++++++-------------
2 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index b3b75c1..3232e14 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -30,16 +30,15 @@ struct sbi_pmu_device {
/**
* 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);
+ int (*fw_event_validate_encoding)(uint64_t event_data);
/**
* 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);
+ bool (*fw_counter_match_encoding)(uint32_t counter_index,
+ uint64_t event_data);
/**
* Fetch the max width of this counter in number of bits.
@@ -58,7 +57,7 @@ struct sbi_pmu_device {
* Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
*/
int (*fw_counter_start)(uint32_t counter_index,
- uint32_t event_idx_code,
+ uint64_t event_data,
uint64_t init_val, bool init_val_update);
/**
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 1a3c44d..1169ef2 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -110,7 +110,7 @@ static bool pmu_event_select_overlap(struct sbi_pmu_hw_event *evt,
return false;
}
-static int pmu_event_validate(unsigned long event_idx)
+static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
{
uint32_t event_idx_type = get_cidx_type(event_idx);
uint32_t event_idx_code = get_cidx_code(event_idx);
@@ -123,8 +123,8 @@ static int pmu_event_validate(unsigned long event_idx)
break;
case SBI_PMU_EVENT_TYPE_FW:
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);
+ pmu_dev && pmu_dev->fw_event_validate_encoding)
+ return pmu_dev->fw_event_validate_encoding(edata);
else
event_idx_code_max = SBI_PMU_FW_MAX;
break;
@@ -361,7 +361,8 @@ int sbi_pmu_irq_bit(void)
}
static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
- uint64_t ival, bool ival_update)
+ uint64_t event_data, uint64_t ival,
+ bool ival_update)
{
int ret;
u32 hartid = current_hartid();
@@ -369,7 +370,7 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
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,
+ event_data,
ival, ival_update);
if (ret)
return ret;
@@ -390,6 +391,7 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
int ret = SBI_EINVAL;
bool bUpdate = false;
int i, cidx;
+ uint64_t edata = 0;
if ((cbase + sbi_fls(cmask)) >= total_ctrs)
return ret;
@@ -404,7 +406,8 @@ 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, event_code, edata, ival,
+ bUpdate);
else
ret = pmu_ctr_start_hw(cidx, ival, bUpdate);
}
@@ -644,7 +647,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
* check.
*/
static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
- uint32_t event_code, u32 hartid)
+ uint32_t event_code, u32 hartid, uint64_t edata)
{
int i, cidx;
@@ -655,9 +658,9 @@ static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
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))
+ pmu_dev && pmu_dev->fw_counter_match_encoding) {
+ if (!pmu_dev->fw_counter_match_encoding(cidx - num_hw_ctrs,
+ edata))
continue;
}
@@ -679,7 +682,7 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
if ((cidx_base + sbi_fls(cidx_mask)) >= total_ctrs)
return SBI_EINVAL;
- event_type = pmu_event_validate(event_idx);
+ event_type = pmu_event_validate(event_idx, event_data);
if (event_type < 0)
return SBI_EINVAL;
event_code = get_cidx_code(event_idx);
@@ -697,7 +700,8 @@ 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, event_code, hartid);
+ ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, event_code,
+ hartid, event_data);
} else {
ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
event_data);
@@ -720,7 +724,7 @@ skip_match:
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,
+ ctr_idx - num_hw_ctrs, event_data,
fw_counters_data[hartid][ctr_idx - num_hw_ctrs],
true);
if (ret)
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 6/8] lib: sbi_pmu: Use dedicated event code for platform firmware events
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
` (4 preceding siblings ...)
2023-03-09 5:51 ` [PATCH v3 5/8] lib: sbi_pmu: Update sbi_pmu dev ops Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:28 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 7/8] lib: sbi_pmu: Introduce fw_counter_write_value API Mayuresh Chitale
2023-03-09 5:51 ` [PATCH v3 8/8] lib: sbi_pmu: Add hartid parameter PMU device ops Mayuresh Chitale
7 siblings, 1 reply; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
For all platform specific firmware event operations use the dedicated
event code (0xFFFF) when matching against the input firmware event.
Furthermore save the real platform specific firmware event code received as
the event data for future use.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
lib/sbi/sbi_pmu.c | 65 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 52 insertions(+), 13 deletions(-)
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 1169ef2..d083198 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -122,7 +122,12 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
break;
case SBI_PMU_EVENT_TYPE_FW:
- if (SBI_PMU_FW_MAX <= event_idx_code &&
+ if ((event_idx_code >= SBI_PMU_FW_MAX &&
+ event_idx_code <= SBI_PMU_FW_RESERVED_MAX) ||
+ event_idx_code > SBI_PMU_FW_PLATFORM)
+ return SBI_EINVAL;
+
+ if (SBI_PMU_FW_PLATFORM == event_idx_code &&
pmu_dev && pmu_dev->fw_event_validate_encoding)
return pmu_dev->fw_event_validate_encoding(edata);
else
@@ -187,12 +192,19 @@ 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_data[hartid][cidx - num_hw_ctrs] =
- pmu_dev->fw_counter_read_value(cidx - num_hw_ctrs);
+ if ((event_code >= SBI_PMU_FW_MAX &&
+ event_code <= SBI_PMU_FW_RESERVED_MAX) ||
+ event_code > SBI_PMU_FW_PLATFORM)
+ return SBI_EINVAL;
- *cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
+ if (SBI_PMU_FW_PLATFORM == event_code) {
+ if (pmu_dev && pmu_dev->fw_counter_read_value)
+ *cval = pmu_dev->fw_counter_read_value(cidx -
+ num_hw_ctrs);
+ else
+ *cval = 0;
+ } else
+ *cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
return 0;
}
@@ -367,8 +379,17 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
int ret;
u32 hartid = current_hartid();
- if (SBI_PMU_FW_MAX <= event_code &&
- pmu_dev && pmu_dev->fw_counter_start) {
+ if ((event_code >= SBI_PMU_FW_MAX &&
+ event_code <= SBI_PMU_FW_RESERVED_MAX) ||
+ event_code > SBI_PMU_FW_PLATFORM)
+ return SBI_EINVAL;
+
+ if (SBI_PMU_FW_PLATFORM == event_code) {
+ if (!pmu_dev ||
+ !pmu_dev->fw_counter_start) {
+ return SBI_EINVAL;
+ }
+
ret = pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
event_data,
ival, ival_update);
@@ -386,12 +407,13 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
unsigned long flags, uint64_t ival)
{
+ u32 hartid = current_hartid();
int event_idx_type;
uint32_t event_code;
int ret = SBI_EINVAL;
bool bUpdate = false;
int i, cidx;
- uint64_t edata = 0;
+ uint64_t edata;
if ((cbase + sbi_fls(cmask)) >= total_ctrs)
return ret;
@@ -405,9 +427,13 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
if (event_idx_type < 0)
/* Continue the start operation for other counters */
continue;
- else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
+ else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW) {
+ edata = (event_code == SBI_PMU_FW_PLATFORM) ?
+ fw_counters_data[hartid][cidx - num_hw_ctrs]
+ : 0x0;
ret = pmu_ctr_start_fw(cidx, event_code, edata, ival,
bUpdate);
+ }
else
ret = pmu_ctr_start_hw(cidx, ival, bUpdate);
}
@@ -441,7 +467,12 @@ static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
{
int ret;
- if (SBI_PMU_FW_MAX <= event_code &&
+ if ((event_code >= SBI_PMU_FW_MAX &&
+ event_code <= SBI_PMU_FW_RESERVED_MAX) ||
+ event_code > SBI_PMU_FW_PLATFORM)
+ return SBI_EINVAL;
+
+ if (SBI_PMU_FW_PLATFORM == event_code &&
pmu_dev && pmu_dev->fw_counter_stop) {
ret = pmu_dev->fw_counter_stop(cidx - num_hw_ctrs);
if (ret)
@@ -651,13 +682,18 @@ static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
{
int i, cidx;
+ if ((event_code >= SBI_PMU_FW_MAX &&
+ event_code <= SBI_PMU_FW_RESERVED_MAX) ||
+ event_code > SBI_PMU_FW_PLATFORM)
+ return SBI_EINVAL;
+
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 &&
+ if (SBI_PMU_FW_PLATFORM == event_code &&
pmu_dev && pmu_dev->fw_counter_match_encoding) {
if (!pmu_dev->fw_counter_match_encoding(cidx - num_hw_ctrs,
edata))
@@ -702,6 +738,9 @@ 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, event_code,
hartid, event_data);
+ if (event_code == SBI_PMU_FW_PLATFORM)
+ fw_counters_data[hartid][ctr_idx - num_hw_ctrs] =
+ event_data;
} else {
ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
event_data);
@@ -721,7 +760,7 @@ skip_match:
if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
fw_counters_data[hartid][ctr_idx - num_hw_ctrs] = 0;
if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
- if (SBI_PMU_FW_MAX <= event_code &&
+ if (SBI_PMU_FW_PLATFORM == event_code &&
pmu_dev && pmu_dev->fw_counter_start) {
ret = pmu_dev->fw_counter_start(
ctr_idx - num_hw_ctrs, event_data,
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 7/8] lib: sbi_pmu: Introduce fw_counter_write_value API
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
` (5 preceding siblings ...)
2023-03-09 5:51 ` [PATCH v3 6/8] lib: sbi_pmu: Use dedicated event code for platform firmware events Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:31 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 8/8] lib: sbi_pmu: Add hartid parameter PMU device ops Mayuresh Chitale
7 siblings, 1 reply; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
Add fw_counter_write_value API for platform specific firmware events
which separates setting the counter's initial value from starting the
counter. This is required so that the fw_event_data array can be reused
to save the event data received.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
include/sbi/sbi_pmu.h | 11 ++++++++---
lib/sbi/sbi_pmu.c | 25 +++++++++++++------------
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index 3232e14..53f2700 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -51,14 +51,19 @@ struct sbi_pmu_device {
*/
uint64_t (*fw_counter_read_value)(uint32_t counter_index);
+ /**
+ * Write value to custom firmware counter
+ * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
+ */
+ void (*fw_counter_write_value)(uint32_t counter_index,
+ uint64_t value);
+
/**
* 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,
- uint64_t event_data,
- uint64_t init_val, bool init_val_update);
+ uint64_t event_data);
/**
* Stop custom firmware counter
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index d083198..00a2c3e 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -376,7 +376,6 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
uint64_t event_data, uint64_t ival,
bool ival_update)
{
- int ret;
u32 hartid = current_hartid();
if ((event_code >= SBI_PMU_FW_MAX &&
@@ -386,19 +385,22 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
if (SBI_PMU_FW_PLATFORM == event_code) {
if (!pmu_dev ||
+ !pmu_dev->fw_counter_write_value ||
!pmu_dev->fw_counter_start) {
return SBI_EINVAL;
}
- ret = pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
- event_data,
- ival, ival_update);
- if (ret)
- return ret;
+ if (ival_update)
+ pmu_dev->fw_counter_write_value(cidx - num_hw_ctrs,
+ ival);
+
+ return pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
+ event_data);
+ } else {
+ if (ival_update)
+ fw_counters_data[hartid][cidx - num_hw_ctrs] = ival;
}
- if (ival_update)
- fw_counters_data[hartid][cidx - num_hw_ctrs] = ival;
fw_counters_started[hartid] |= BIT(cidx - num_hw_ctrs);
return 0;
@@ -762,10 +764,9 @@ skip_match:
if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
if (SBI_PMU_FW_PLATFORM == event_code &&
pmu_dev && pmu_dev->fw_counter_start) {
- ret = pmu_dev->fw_counter_start(
- ctr_idx - num_hw_ctrs, event_data,
- fw_counters_data[hartid][ctr_idx - num_hw_ctrs],
- true);
+ ret = pmu_dev->fw_counter_start(ctr_idx -
+ num_hw_ctrs,
+ event_data);
if (ret)
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 8/8] lib: sbi_pmu: Add hartid parameter PMU device ops
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
` (6 preceding siblings ...)
2023-03-09 5:51 ` [PATCH v3 7/8] lib: sbi_pmu: Introduce fw_counter_write_value API Mayuresh Chitale
@ 2023-03-09 5:51 ` Mayuresh Chitale
2023-03-09 6:32 ` Anup Patel
7 siblings, 1 reply; 21+ messages in thread
From: Mayuresh Chitale @ 2023-03-09 5:51 UTC (permalink / raw)
To: opensbi
Platform specific firmware event handler may leverage the hartid to program
per hart specific registers for a given counter.
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
include/sbi/sbi_pmu.h | 14 ++++++++------
lib/sbi/sbi_pmu.c | 25 +++++++++++++++----------
2 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
index 53f2700..16f6877 100644
--- a/include/sbi/sbi_pmu.h
+++ b/include/sbi/sbi_pmu.h
@@ -31,13 +31,14 @@ struct sbi_pmu_device {
/**
* Validate event code of custom firmware event
*/
- int (*fw_event_validate_encoding)(uint64_t event_data);
+ int (*fw_event_validate_encoding)(uint32_t hartid, uint64_t event_data);
/**
* Match custom firmware counter with custom firmware event
* Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
*/
- bool (*fw_counter_match_encoding)(uint32_t counter_index,
+ bool (*fw_counter_match_encoding)(uint32_t hartid,
+ uint32_t counter_index,
uint64_t event_data);
/**
@@ -49,27 +50,28 @@ struct sbi_pmu_device {
* 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);
+ uint64_t (*fw_counter_read_value)(uint32_t hartid,
+ uint32_t counter_index);
/**
* Write value to custom firmware counter
* Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
*/
- void (*fw_counter_write_value)(uint32_t counter_index,
+ void (*fw_counter_write_value)(uint32_t hartid, uint32_t counter_index,
uint64_t value);
/**
* Start custom firmware counter
* Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
*/
- int (*fw_counter_start)(uint32_t counter_index,
+ int (*fw_counter_start)(uint32_t hartid, uint32_t counter_index,
uint64_t event_data);
/**
* Stop custom firmware counter
* Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
*/
- int (*fw_counter_stop)(uint32_t counter_index);
+ int (*fw_counter_stop)(uint32_t hartid, uint32_t counter_index);
/**
* Custom enable irq for hardware counter
diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index 00a2c3e..74d6912 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -116,6 +116,7 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
uint32_t event_idx_code = get_cidx_code(event_idx);
uint32_t event_idx_code_max = -1;
uint32_t cache_ops_result, cache_ops_id, cache_id;
+ u32 hartid = current_hartid();
switch(event_idx_type) {
case SBI_PMU_EVENT_TYPE_HW:
@@ -129,7 +130,8 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
if (SBI_PMU_FW_PLATFORM == event_idx_code &&
pmu_dev && pmu_dev->fw_event_validate_encoding)
- return pmu_dev->fw_event_validate_encoding(edata);
+ return pmu_dev->fw_event_validate_encoding(hartid,
+ edata);
else
event_idx_code_max = SBI_PMU_FW_MAX;
break;
@@ -199,7 +201,8 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
if (SBI_PMU_FW_PLATFORM == event_code) {
if (pmu_dev && pmu_dev->fw_counter_read_value)
- *cval = pmu_dev->fw_counter_read_value(cidx -
+ *cval = pmu_dev->fw_counter_read_value(hartid,
+ cidx -
num_hw_ctrs);
else
*cval = 0;
@@ -391,10 +394,11 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
}
if (ival_update)
- pmu_dev->fw_counter_write_value(cidx - num_hw_ctrs,
+ pmu_dev->fw_counter_write_value(hartid,
+ cidx - num_hw_ctrs,
ival);
- return pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
+ return pmu_dev->fw_counter_start(hartid, cidx - num_hw_ctrs,
event_data);
} else {
if (ival_update)
@@ -467,6 +471,7 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
{
+ u32 hartid = current_hartid();
int ret;
if ((event_code >= SBI_PMU_FW_MAX &&
@@ -476,7 +481,7 @@ static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
if (SBI_PMU_FW_PLATFORM == event_code &&
pmu_dev && pmu_dev->fw_counter_stop) {
- ret = pmu_dev->fw_counter_stop(cidx - num_hw_ctrs);
+ ret = pmu_dev->fw_counter_stop(hartid, cidx - num_hw_ctrs);
if (ret)
return ret;
}
@@ -697,8 +702,9 @@ static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
continue;
if (SBI_PMU_FW_PLATFORM == event_code &&
pmu_dev && pmu_dev->fw_counter_match_encoding) {
- if (!pmu_dev->fw_counter_match_encoding(cidx - num_hw_ctrs,
- edata))
+ if (!pmu_dev->fw_counter_match_encoding(hartid,
+ cidx - num_hw_ctrs,
+ edata))
continue;
}
@@ -764,9 +770,8 @@ skip_match:
if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
if (SBI_PMU_FW_PLATFORM == event_code &&
pmu_dev && pmu_dev->fw_counter_start) {
- ret = pmu_dev->fw_counter_start(ctr_idx -
- num_hw_ctrs,
- event_data);
+ ret = pmu_dev->fw_counter_start(hartid,
+ ctr_idx - num_hw_ctrs, event_data);
if (ret)
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 1/8] lib: sbi_pmu: add callback for counter width
2023-03-09 5:51 ` [PATCH v3 1/8] lib: sbi_pmu: add callback for counter width Mayuresh Chitale
@ 2023-03-09 6:18 ` Anup Patel
0 siblings, 0 replies; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:18 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> This patch adds a callback to fetch the number of bits implemented for a
> custom firmware counter. If the callback fails or is not implemented then
> width defaults to 63.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> include/sbi/sbi_pmu.h | 5 +++++
> lib/sbi/sbi_pmu.c | 6 ++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index c365243..b3b75c1 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -41,6 +41,11 @@ struct sbi_pmu_device {
> bool (*fw_counter_match_code)(uint32_t counter_index,
> uint32_t event_idx_code);
>
> + /**
> + * Fetch the max width of this counter in number of bits.
> + */
> + int (*fw_counter_width)(void);
> +
> /**
> * Read value of custom firmware counter
> * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 154dbda..a99c045 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -761,6 +761,7 @@ unsigned long sbi_pmu_num_ctr(void)
>
> int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
> {
> + int width;
> union sbi_pmu_ctr_info cinfo = {0};
> struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>
> @@ -782,6 +783,11 @@ int sbi_pmu_ctr_get_info(uint32_t cidx, unsigned long *ctr_info)
> cinfo.type = SBI_PMU_CTR_TYPE_FW;
> /* Firmware counters are always 64 bits wide */
> cinfo.width = 63;
> + if (pmu_dev && pmu_dev->fw_counter_width) {
> + width = pmu_dev->fw_counter_width();
> + if (width)
> + cinfo.width = width - 1;
> + }
> }
>
> *ctr_info = cinfo.value;
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 2/8] lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi
2023-03-09 5:51 ` [PATCH v3 2/8] lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi Mayuresh Chitale
@ 2023-03-09 6:18 ` Anup Patel
0 siblings, 0 replies; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:18 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> To support 64 bit firmware counters on RV32 systems, we implement
> sbi_pmu_counter_fw_read_hi() which returns the upper 32 bits of
> the firmware counter value. On RV64 (or higher) systems, this
> function will always return zero.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> include/sbi/sbi_ecall_interface.h | 1 +
> lib/sbi/sbi_ecall_pmu.c | 8 ++++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
> index 4c378c3..d448212 100644
> --- a/include/sbi/sbi_ecall_interface.h
> +++ b/include/sbi/sbi_ecall_interface.h
> @@ -101,6 +101,7 @@
> #define SBI_EXT_PMU_COUNTER_START 0x3
> #define SBI_EXT_PMU_COUNTER_STOP 0x4
> #define SBI_EXT_PMU_COUNTER_FW_READ 0x5
> +#define SBI_EXT_PMU_COUNTER_FW_READ_HI 0x6
>
> /** General pmu event codes specified in SBI PMU extension */
> enum sbi_pmu_hw_generic_events_t {
> diff --git a/lib/sbi/sbi_ecall_pmu.c b/lib/sbi/sbi_ecall_pmu.c
> index 826c8a8..2d1e669 100644
> --- a/lib/sbi/sbi_ecall_pmu.c
> +++ b/lib/sbi/sbi_ecall_pmu.c
> @@ -54,6 +54,14 @@ static int sbi_ecall_pmu_handler(unsigned long extid, unsigned long funcid,
> ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
> *out_val = temp;
> break;
> + case SBI_EXT_PMU_COUNTER_FW_READ_HI:
> +#if __riscv_xlen == 32
> + ret = sbi_pmu_ctr_fw_read(regs->a0, &temp);
> + *out_val = temp >> 32
> +#else
> + *out_val = 0;
> +#endif
> + break;
> case SBI_EXT_PMU_COUNTER_START:
>
> #if __riscv_xlen == 32
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events
2023-03-09 5:51 ` [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events Mayuresh Chitale
@ 2023-03-09 6:19 ` Anup Patel
2023-03-09 9:16 ` Andrew Jones
1 sibling, 0 replies; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:19 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> We reserve space for SBI implementation specific custom firmware
> events which can be used by M-mode firmwares and HS-mode hypervisors
> for their own use. This reserved space is intentionally large to
> ensure that SBI implementation has enough space to accommodate
> platform specific firmware events as well.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> include/sbi/sbi_ecall_interface.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
> index d448212..8d1cb97 100644
> --- a/include/sbi/sbi_ecall_interface.h
> +++ b/include/sbi/sbi_ecall_interface.h
> @@ -185,6 +185,17 @@ enum sbi_pmu_fw_event_code_id {
> SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
> SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
> SBI_PMU_FW_MAX,
> + /*
> + * Event codes 22 to 255 are reserved for future use.
> + * Event codes 256 to 65534 are reserved for SBI implementation
> + * specific custom firmware events.
> + */
> + SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
> + /*
> + * Event code 0xFFFF is used for platform specific firmware
> + * events where the event data contains any event specific information.
> + */
> + SBI_PMU_FW_PLATFORM = 0xFFFF,
> };
>
> /** SBI PMU event idx type */
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value
2023-03-09 5:51 ` [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value Mayuresh Chitale
@ 2023-03-09 6:19 ` Anup Patel
2023-03-09 9:19 ` Andrew Jones
1 sibling, 0 replies; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:19 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> Rename and reuse fw_counter_value arrary to save both the counter values
> for the SBI firmware events and event data for the SBI platform specific
> firmware events.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> lib/sbi/sbi_pmu.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index a99c045..1a3c44d 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -65,8 +65,12 @@ static uint32_t active_events[SBI_HARTMASK_MAX_BITS][SBI_PMU_HW_CTR_MAX + SBI_PM
> #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};
> +/*
> + * Counter values for SBI firmware events and event codes for platform
> + * firmware events. Both are mutually exclusive and hence can optimally share
> + * the same memory.
> + */
> +static uint64_t fw_counters_data[SBI_HARTMASK_MAX_BITS][SBI_PMU_FW_CTR_MAX] = {0};
>
> /* Maximum number of hardware events available */
> static uint32_t num_hw_events;
> @@ -185,10 +189,10 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
>
> if (SBI_PMU_FW_MAX <= event_code &&
> pmu_dev && pmu_dev->fw_counter_read_value)
> - fw_counters_value[hartid][cidx - num_hw_ctrs] =
> + fw_counters_data[hartid][cidx - num_hw_ctrs] =
> pmu_dev->fw_counter_read_value(cidx - num_hw_ctrs);
>
> - *cval = fw_counters_value[hartid][cidx - num_hw_ctrs];
> + *cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
>
> return 0;
> }
> @@ -372,7 +376,7 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
> }
>
> if (ival_update)
> - fw_counters_value[hartid][cidx - num_hw_ctrs] = ival;
> + fw_counters_data[hartid][cidx - num_hw_ctrs] = ival;
> fw_counters_started[hartid] |= BIT(cidx - num_hw_ctrs);
>
> return 0;
> @@ -711,13 +715,13 @@ skip_match:
> pmu_ctr_start_hw(ctr_idx, 0, false);
> } 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;
> + fw_counters_data[hartid][ctr_idx - num_hw_ctrs] = 0;
> 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],
> + fw_counters_data[hartid][ctr_idx - num_hw_ctrs],
> true);
> if (ret)
> return ret;
> @@ -743,7 +747,7 @@ int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id 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];
> + fcounter = &fw_counters_data[hartid][cidx - num_hw_ctrs];
> break;
> }
> }
> @@ -803,7 +807,7 @@ static void pmu_reset_event_map(u32 hartid)
> for (j = 3; j < total_ctrs; j++)
> active_events[hartid][j] = SBI_PMU_EVENT_IDX_INVALID;
> for (j = 0; j < SBI_PMU_FW_CTR_MAX; j++)
> - fw_counters_value[hartid][j] = 0;
> + fw_counters_data[hartid][j] = 0;
> fw_counters_started[hartid] = 0;
> }
>
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 5/8] lib: sbi_pmu: Update sbi_pmu dev ops
2023-03-09 5:51 ` [PATCH v3 5/8] lib: sbi_pmu: Update sbi_pmu dev ops Mayuresh Chitale
@ 2023-03-09 6:21 ` Anup Patel
0 siblings, 0 replies; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:21 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> Update fw_event_validate_code, fw_counter_match_code and fw_counter_start
> ops which used a 32 bit event code to use the 64 bit event data instead.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> include/sbi/sbi_pmu.h | 9 ++++-----
> lib/sbi/sbi_pmu.c | 30 +++++++++++++++++-------------
> 2 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index b3b75c1..3232e14 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -30,16 +30,15 @@ struct sbi_pmu_device {
>
> /**
> * 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);
> + int (*fw_event_validate_encoding)(uint64_t event_data);
>
> /**
> * 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);
> + bool (*fw_counter_match_encoding)(uint32_t counter_index,
> + uint64_t event_data);
>
> /**
> * Fetch the max width of this counter in number of bits.
> @@ -58,7 +57,7 @@ struct sbi_pmu_device {
> * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
> */
> int (*fw_counter_start)(uint32_t counter_index,
> - uint32_t event_idx_code,
> + uint64_t event_data,
> uint64_t init_val, bool init_val_update);
>
> /**
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 1a3c44d..1169ef2 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -110,7 +110,7 @@ static bool pmu_event_select_overlap(struct sbi_pmu_hw_event *evt,
> return false;
> }
>
> -static int pmu_event_validate(unsigned long event_idx)
> +static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
> {
> uint32_t event_idx_type = get_cidx_type(event_idx);
> uint32_t event_idx_code = get_cidx_code(event_idx);
> @@ -123,8 +123,8 @@ static int pmu_event_validate(unsigned long event_idx)
> break;
> case SBI_PMU_EVENT_TYPE_FW:
> 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);
> + pmu_dev && pmu_dev->fw_event_validate_encoding)
> + return pmu_dev->fw_event_validate_encoding(edata);
> else
> event_idx_code_max = SBI_PMU_FW_MAX;
> break;
> @@ -361,7 +361,8 @@ int sbi_pmu_irq_bit(void)
> }
>
> static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
> - uint64_t ival, bool ival_update)
> + uint64_t event_data, uint64_t ival,
> + bool ival_update)
> {
> int ret;
> u32 hartid = current_hartid();
> @@ -369,7 +370,7 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
> 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,
> + event_data,
> ival, ival_update);
> if (ret)
> return ret;
> @@ -390,6 +391,7 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
> int ret = SBI_EINVAL;
> bool bUpdate = false;
> int i, cidx;
> + uint64_t edata = 0;
>
> if ((cbase + sbi_fls(cmask)) >= total_ctrs)
> return ret;
> @@ -404,7 +406,8 @@ 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, event_code, edata, ival,
> + bUpdate);
> else
> ret = pmu_ctr_start_hw(cidx, ival, bUpdate);
> }
> @@ -644,7 +647,7 @@ static int pmu_ctr_find_hw(unsigned long cbase, unsigned long cmask, unsigned lo
> * check.
> */
> static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
> - uint32_t event_code, u32 hartid)
> + uint32_t event_code, u32 hartid, uint64_t edata)
> {
> int i, cidx;
>
> @@ -655,9 +658,9 @@ static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
> 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))
> + pmu_dev && pmu_dev->fw_counter_match_encoding) {
> + if (!pmu_dev->fw_counter_match_encoding(cidx - num_hw_ctrs,
> + edata))
> continue;
> }
>
> @@ -679,7 +682,7 @@ int sbi_pmu_ctr_cfg_match(unsigned long cidx_base, unsigned long cidx_mask,
> if ((cidx_base + sbi_fls(cidx_mask)) >= total_ctrs)
> return SBI_EINVAL;
>
> - event_type = pmu_event_validate(event_idx);
> + event_type = pmu_event_validate(event_idx, event_data);
> if (event_type < 0)
> return SBI_EINVAL;
> event_code = get_cidx_code(event_idx);
> @@ -697,7 +700,8 @@ 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, event_code, hartid);
> + ctr_idx = pmu_ctr_find_fw(cidx_base, cidx_mask, event_code,
> + hartid, event_data);
> } else {
> ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
> event_data);
> @@ -720,7 +724,7 @@ skip_match:
> 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,
> + ctr_idx - num_hw_ctrs, event_data,
> fw_counters_data[hartid][ctr_idx - num_hw_ctrs],
> true);
> if (ret)
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 6/8] lib: sbi_pmu: Use dedicated event code for platform firmware events
2023-03-09 5:51 ` [PATCH v3 6/8] lib: sbi_pmu: Use dedicated event code for platform firmware events Mayuresh Chitale
@ 2023-03-09 6:28 ` Anup Patel
2023-03-09 12:33 ` mchitale
0 siblings, 1 reply; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:28 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> For all platform specific firmware event operations use the dedicated
> event code (0xFFFF) when matching against the input firmware event.
> Furthermore save the real platform specific firmware event code received as
> the event data for future use.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
> lib/sbi/sbi_pmu.c | 65 +++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 52 insertions(+), 13 deletions(-)
>
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 1169ef2..d083198 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -122,7 +122,12 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
> event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
> break;
> case SBI_PMU_EVENT_TYPE_FW:
> - if (SBI_PMU_FW_MAX <= event_idx_code &&
> + if ((event_idx_code >= SBI_PMU_FW_MAX &&
> + event_idx_code <= SBI_PMU_FW_RESERVED_MAX) ||
> + event_idx_code > SBI_PMU_FW_PLATFORM)
> + return SBI_EINVAL;
> +
> + if (SBI_PMU_FW_PLATFORM == event_idx_code &&
> pmu_dev && pmu_dev->fw_event_validate_encoding)
> return pmu_dev->fw_event_validate_encoding(edata);
> else
> @@ -187,12 +192,19 @@ 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_data[hartid][cidx - num_hw_ctrs] =
> - pmu_dev->fw_counter_read_value(cidx - num_hw_ctrs);
> + if ((event_code >= SBI_PMU_FW_MAX &&
> + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> + event_code > SBI_PMU_FW_PLATFORM)
> + return SBI_EINVAL;
>
> - *cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
> + if (SBI_PMU_FW_PLATFORM == event_code) {
> + if (pmu_dev && pmu_dev->fw_counter_read_value)
> + *cval = pmu_dev->fw_counter_read_value(cidx -
> + num_hw_ctrs);
> + else
> + *cval = 0;
> + } else
> + *cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
>
> return 0;
> }
> @@ -367,8 +379,17 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
> int ret;
> u32 hartid = current_hartid();
>
> - if (SBI_PMU_FW_MAX <= event_code &&
> - pmu_dev && pmu_dev->fw_counter_start) {
> + if ((event_code >= SBI_PMU_FW_MAX &&
> + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> + event_code > SBI_PMU_FW_PLATFORM)
> + return SBI_EINVAL;
> +
> + if (SBI_PMU_FW_PLATFORM == event_code) {
> + if (!pmu_dev ||
> + !pmu_dev->fw_counter_start) {
> + return SBI_EINVAL;
> + }
> +
> ret = pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
> event_data,
> ival, ival_update);
> @@ -386,12 +407,13 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
> int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
> unsigned long flags, uint64_t ival)
> {
> + u32 hartid = current_hartid();
This is an unrelated change. Probably belongs to a different patch.
> int event_idx_type;
> uint32_t event_code;
> int ret = SBI_EINVAL;
> bool bUpdate = false;
> int i, cidx;
> - uint64_t edata = 0;
> + uint64_t edata;
>
> if ((cbase + sbi_fls(cmask)) >= total_ctrs)
> return ret;
> @@ -405,9 +427,13 @@ int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
> if (event_idx_type < 0)
> /* Continue the start operation for other counters */
> continue;
> - else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
> + else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW) {
> + edata = (event_code == SBI_PMU_FW_PLATFORM) ?
> + fw_counters_data[hartid][cidx - num_hw_ctrs]
> + : 0x0;
> ret = pmu_ctr_start_fw(cidx, event_code, edata, ival,
> bUpdate);
> + }
> else
> ret = pmu_ctr_start_hw(cidx, ival, bUpdate);
> }
> @@ -441,7 +467,12 @@ static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
> {
> int ret;
>
> - if (SBI_PMU_FW_MAX <= event_code &&
> + if ((event_code >= SBI_PMU_FW_MAX &&
> + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> + event_code > SBI_PMU_FW_PLATFORM)
> + return SBI_EINVAL;
> +
> + if (SBI_PMU_FW_PLATFORM == event_code &&
> pmu_dev && pmu_dev->fw_counter_stop) {
> ret = pmu_dev->fw_counter_stop(cidx - num_hw_ctrs);
> if (ret)
> @@ -651,13 +682,18 @@ static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
> {
> int i, cidx;
>
> + if ((event_code >= SBI_PMU_FW_MAX &&
> + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> + event_code > SBI_PMU_FW_PLATFORM)
> + return SBI_EINVAL;
> +
> 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 &&
> + if (SBI_PMU_FW_PLATFORM == event_code &&
> pmu_dev && pmu_dev->fw_counter_match_encoding) {
> if (!pmu_dev->fw_counter_match_encoding(cidx - num_hw_ctrs,
> edata))
> @@ -702,6 +738,9 @@ 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, event_code,
> hartid, event_data);
> + if (event_code == SBI_PMU_FW_PLATFORM)
> + fw_counters_data[hartid][ctr_idx - num_hw_ctrs] =
> + event_data;
> } else {
> ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask, flags, event_idx,
> event_data);
> @@ -721,7 +760,7 @@ skip_match:
> if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
> fw_counters_data[hartid][ctr_idx - num_hw_ctrs] = 0;
> if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
> - if (SBI_PMU_FW_MAX <= event_code &&
> + if (SBI_PMU_FW_PLATFORM == event_code &&
> pmu_dev && pmu_dev->fw_counter_start) {
> ret = pmu_dev->fw_counter_start(
> ctr_idx - num_hw_ctrs, event_data,
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
Otherwise, it looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 7/8] lib: sbi_pmu: Introduce fw_counter_write_value API
2023-03-09 5:51 ` [PATCH v3 7/8] lib: sbi_pmu: Introduce fw_counter_write_value API Mayuresh Chitale
@ 2023-03-09 6:31 ` Anup Patel
0 siblings, 0 replies; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:31 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> Add fw_counter_write_value API for platform specific firmware events
> which separates setting the counter's initial value from starting the
> counter. This is required so that the fw_event_data array can be reused
> to save the event data received.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> include/sbi/sbi_pmu.h | 11 ++++++++---
> lib/sbi/sbi_pmu.c | 25 +++++++++++++------------
> 2 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index 3232e14..53f2700 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -51,14 +51,19 @@ struct sbi_pmu_device {
> */
> uint64_t (*fw_counter_read_value)(uint32_t counter_index);
>
> + /**
> + * Write value to custom firmware counter
> + * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
> + */
> + void (*fw_counter_write_value)(uint32_t counter_index,
> + uint64_t value);
> +
> /**
> * 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,
> - uint64_t event_data,
> - uint64_t init_val, bool init_val_update);
> + uint64_t event_data);
>
> /**
> * Stop custom firmware counter
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index d083198..00a2c3e 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -376,7 +376,6 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
> uint64_t event_data, uint64_t ival,
> bool ival_update)
> {
> - int ret;
> u32 hartid = current_hartid();
>
> if ((event_code >= SBI_PMU_FW_MAX &&
> @@ -386,19 +385,22 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
>
> if (SBI_PMU_FW_PLATFORM == event_code) {
> if (!pmu_dev ||
> + !pmu_dev->fw_counter_write_value ||
> !pmu_dev->fw_counter_start) {
> return SBI_EINVAL;
> }
>
> - ret = pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
> - event_data,
> - ival, ival_update);
> - if (ret)
> - return ret;
> + if (ival_update)
> + pmu_dev->fw_counter_write_value(cidx - num_hw_ctrs,
> + ival);
> +
> + return pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
> + event_data);
> + } else {
> + if (ival_update)
> + fw_counters_data[hartid][cidx - num_hw_ctrs] = ival;
> }
>
> - if (ival_update)
> - fw_counters_data[hartid][cidx - num_hw_ctrs] = ival;
> fw_counters_started[hartid] |= BIT(cidx - num_hw_ctrs);
>
> return 0;
> @@ -762,10 +764,9 @@ skip_match:
> if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
> if (SBI_PMU_FW_PLATFORM == event_code &&
> pmu_dev && pmu_dev->fw_counter_start) {
> - ret = pmu_dev->fw_counter_start(
> - ctr_idx - num_hw_ctrs, event_data,
> - fw_counters_data[hartid][ctr_idx - num_hw_ctrs],
> - true);
> + ret = pmu_dev->fw_counter_start(ctr_idx -
> + num_hw_ctrs,
> + event_data);
> if (ret)
> return ret;
> }
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 8/8] lib: sbi_pmu: Add hartid parameter PMU device ops
2023-03-09 5:51 ` [PATCH v3 8/8] lib: sbi_pmu: Add hartid parameter PMU device ops Mayuresh Chitale
@ 2023-03-09 6:32 ` Anup Patel
0 siblings, 0 replies; 21+ messages in thread
From: Anup Patel @ 2023-03-09 6:32 UTC (permalink / raw)
To: opensbi
On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> Platform specific firmware event handler may leverage the hartid to program
> per hart specific registers for a given counter.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> include/sbi/sbi_pmu.h | 14 ++++++++------
> lib/sbi/sbi_pmu.c | 25 +++++++++++++++----------
> 2 files changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/include/sbi/sbi_pmu.h b/include/sbi/sbi_pmu.h
> index 53f2700..16f6877 100644
> --- a/include/sbi/sbi_pmu.h
> +++ b/include/sbi/sbi_pmu.h
> @@ -31,13 +31,14 @@ struct sbi_pmu_device {
> /**
> * Validate event code of custom firmware event
> */
> - int (*fw_event_validate_encoding)(uint64_t event_data);
> + int (*fw_event_validate_encoding)(uint32_t hartid, uint64_t event_data);
>
> /**
> * Match custom firmware counter with custom firmware event
> * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
> */
> - bool (*fw_counter_match_encoding)(uint32_t counter_index,
> + bool (*fw_counter_match_encoding)(uint32_t hartid,
> + uint32_t counter_index,
> uint64_t event_data);
>
> /**
> @@ -49,27 +50,28 @@ struct sbi_pmu_device {
> * 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);
> + uint64_t (*fw_counter_read_value)(uint32_t hartid,
> + uint32_t counter_index);
>
> /**
> * Write value to custom firmware counter
> * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
> */
> - void (*fw_counter_write_value)(uint32_t counter_index,
> + void (*fw_counter_write_value)(uint32_t hartid, uint32_t counter_index,
> uint64_t value);
>
> /**
> * Start custom firmware counter
> * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
> */
> - int (*fw_counter_start)(uint32_t counter_index,
> + int (*fw_counter_start)(uint32_t hartid, uint32_t counter_index,
> uint64_t event_data);
>
> /**
> * Stop custom firmware counter
> * Note: 0 <= counter_index < SBI_PMU_FW_CTR_MAX
> */
> - int (*fw_counter_stop)(uint32_t counter_index);
> + int (*fw_counter_stop)(uint32_t hartid, uint32_t counter_index);
>
> /**
> * Custom enable irq for hardware counter
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index 00a2c3e..74d6912 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -116,6 +116,7 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
> uint32_t event_idx_code = get_cidx_code(event_idx);
> uint32_t event_idx_code_max = -1;
> uint32_t cache_ops_result, cache_ops_id, cache_id;
> + u32 hartid = current_hartid();
>
> switch(event_idx_type) {
> case SBI_PMU_EVENT_TYPE_HW:
> @@ -129,7 +130,8 @@ static int pmu_event_validate(unsigned long event_idx, uint64_t edata)
>
> if (SBI_PMU_FW_PLATFORM == event_idx_code &&
> pmu_dev && pmu_dev->fw_event_validate_encoding)
> - return pmu_dev->fw_event_validate_encoding(edata);
> + return pmu_dev->fw_event_validate_encoding(hartid,
> + edata);
> else
> event_idx_code_max = SBI_PMU_FW_MAX;
> break;
> @@ -199,7 +201,8 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
>
> if (SBI_PMU_FW_PLATFORM == event_code) {
> if (pmu_dev && pmu_dev->fw_counter_read_value)
> - *cval = pmu_dev->fw_counter_read_value(cidx -
> + *cval = pmu_dev->fw_counter_read_value(hartid,
> + cidx -
> num_hw_ctrs);
> else
> *cval = 0;
> @@ -391,10 +394,11 @@ static int pmu_ctr_start_fw(uint32_t cidx, uint32_t event_code,
> }
>
> if (ival_update)
> - pmu_dev->fw_counter_write_value(cidx - num_hw_ctrs,
> + pmu_dev->fw_counter_write_value(hartid,
> + cidx - num_hw_ctrs,
> ival);
>
> - return pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
> + return pmu_dev->fw_counter_start(hartid, cidx - num_hw_ctrs,
> event_data);
> } else {
> if (ival_update)
> @@ -467,6 +471,7 @@ static int pmu_ctr_stop_hw(uint32_t cidx)
>
> static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
> {
> + u32 hartid = current_hartid();
> int ret;
>
> if ((event_code >= SBI_PMU_FW_MAX &&
> @@ -476,7 +481,7 @@ static int pmu_ctr_stop_fw(uint32_t cidx, uint32_t event_code)
>
> if (SBI_PMU_FW_PLATFORM == event_code &&
> pmu_dev && pmu_dev->fw_counter_stop) {
> - ret = pmu_dev->fw_counter_stop(cidx - num_hw_ctrs);
> + ret = pmu_dev->fw_counter_stop(hartid, cidx - num_hw_ctrs);
> if (ret)
> return ret;
> }
> @@ -697,8 +702,9 @@ static int pmu_ctr_find_fw(unsigned long cbase, unsigned long cmask,
> continue;
> if (SBI_PMU_FW_PLATFORM == event_code &&
> pmu_dev && pmu_dev->fw_counter_match_encoding) {
> - if (!pmu_dev->fw_counter_match_encoding(cidx - num_hw_ctrs,
> - edata))
> + if (!pmu_dev->fw_counter_match_encoding(hartid,
> + cidx - num_hw_ctrs,
> + edata))
> continue;
> }
>
> @@ -764,9 +770,8 @@ skip_match:
> if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
> if (SBI_PMU_FW_PLATFORM == event_code &&
> pmu_dev && pmu_dev->fw_counter_start) {
> - ret = pmu_dev->fw_counter_start(ctr_idx -
> - num_hw_ctrs,
> - event_data);
> + ret = pmu_dev->fw_counter_start(hartid,
> + ctr_idx - num_hw_ctrs, event_data);
> if (ret)
> return ret;
> }
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events
2023-03-09 5:51 ` [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events Mayuresh Chitale
2023-03-09 6:19 ` Anup Patel
@ 2023-03-09 9:16 ` Andrew Jones
2023-03-09 12:30 ` mchitale
1 sibling, 1 reply; 21+ messages in thread
From: Andrew Jones @ 2023-03-09 9:16 UTC (permalink / raw)
To: opensbi
On Thu, Mar 09, 2023 at 11:21:07AM +0530, Mayuresh Chitale wrote:
> We reserve space for SBI implementation specific custom firmware
> events which can be used by M-mode firmwares and HS-mode hypervisors
> for their own use. This reserved space is intentionally large to
> ensure that SBI implementation has enough space to accommodate
> platform specific firmware events as well.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> ---
> include/sbi/sbi_ecall_interface.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
> index d448212..8d1cb97 100644
> --- a/include/sbi/sbi_ecall_interface.h
> +++ b/include/sbi/sbi_ecall_interface.h
> @@ -185,6 +185,17 @@ enum sbi_pmu_fw_event_code_id {
> SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
> SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
> SBI_PMU_FW_MAX,
> + /*
> + * Event codes 22 to 255 are reserved for future use.
> + * Event codes 256 to 65534 are reserved for SBI implementation
> + * specific custom firmware events.
> + */
> + SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
> + /*
> + * Event code 0xFFFF is used for platform specific firmware
> + * events where the event data contains any event specific information.
> + */
The above three lines need to be spaced over one space. And, then, "the
stars will align", which should improve our luck!
Thanks,
drew
> + SBI_PMU_FW_PLATFORM = 0xFFFF,
> };
>
> /** SBI PMU event idx type */
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value
2023-03-09 5:51 ` [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value Mayuresh Chitale
2023-03-09 6:19 ` Anup Patel
@ 2023-03-09 9:19 ` Andrew Jones
1 sibling, 0 replies; 21+ messages in thread
From: Andrew Jones @ 2023-03-09 9:19 UTC (permalink / raw)
To: opensbi
On Thu, Mar 09, 2023 at 11:21:08AM +0530, Mayuresh Chitale wrote:
> Rename and reuse fw_counter_value arrary to save both the counter values
array
> for the SBI firmware events and event data for the SBI platform specific
> firmware events.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
> lib/sbi/sbi_pmu.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
Otherwise
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events
2023-03-09 9:16 ` Andrew Jones
@ 2023-03-09 12:30 ` mchitale
0 siblings, 0 replies; 21+ messages in thread
From: mchitale @ 2023-03-09 12:30 UTC (permalink / raw)
To: opensbi
On Thu, 2023-03-09 at 10:16 +0100, Andrew Jones wrote:
> On Thu, Mar 09, 2023 at 11:21:07AM +0530, Mayuresh Chitale wrote:
> > We reserve space for SBI implementation specific custom firmware
> > events which can be used by M-mode firmwares and HS-mode
> > hypervisors
> > for their own use. This reserved space is intentionally large to
> > ensure that SBI implementation has enough space to accommodate
> > platform specific firmware events as well.
> >
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > Reviewed-by: Atish Patra <atishp@rivosinc.com>
> > ---
> > include/sbi/sbi_ecall_interface.h | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/include/sbi/sbi_ecall_interface.h
> > b/include/sbi/sbi_ecall_interface.h
> > index d448212..8d1cb97 100644
> > --- a/include/sbi/sbi_ecall_interface.h
> > +++ b/include/sbi/sbi_ecall_interface.h
> > @@ -185,6 +185,17 @@ enum sbi_pmu_fw_event_code_id {
> > SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
> > SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
> > SBI_PMU_FW_MAX,
> > + /*
> > + * Event codes 22 to 255 are reserved for future use.
> > + * Event codes 256 to 65534 are reserved for SBI implementation
> > + * specific custom firmware events.
> > + */
> > + SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
> > + /*
> > + * Event code 0xFFFF is used for platform specific firmware
> > + * events where the event data contains any event specific
> > information.
> > + */
>
> The above three lines need to be spaced over one space. And, then,
> "the
> stars will align", which should improve our luck!
Ok :). Will fix it.
>
> Thanks,
> drew
>
> > + SBI_PMU_FW_PLATFORM = 0xFFFF,
> > };
> >
> > /** SBI PMU event idx type */
> > --
> > 2.34.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 6/8] lib: sbi_pmu: Use dedicated event code for platform firmware events
2023-03-09 6:28 ` Anup Patel
@ 2023-03-09 12:33 ` mchitale
0 siblings, 0 replies; 21+ messages in thread
From: mchitale @ 2023-03-09 12:33 UTC (permalink / raw)
To: opensbi
On Thu, 2023-03-09 at 11:58 +0530, Anup Patel wrote:
> On Thu, Mar 9, 2023 at 11:21?AM Mayuresh Chitale
> <mchitale@ventanamicro.com> wrote:
> > For all platform specific firmware event operations use the
> > dedicated
> > event code (0xFFFF) when matching against the input firmware event.
> > Furthermore save the real platform specific firmware event code
> > received as
> > the event data for future use.
> >
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > ---
> > lib/sbi/sbi_pmu.c | 65 +++++++++++++++++++++++++++++++++++++----
> > ------
> > 1 file changed, 52 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index 1169ef2..d083198 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -122,7 +122,12 @@ static int pmu_event_validate(unsigned long
> > event_idx, uint64_t edata)
> > event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
> > break;
> > case SBI_PMU_EVENT_TYPE_FW:
> > - if (SBI_PMU_FW_MAX <= event_idx_code &&
> > + if ((event_idx_code >= SBI_PMU_FW_MAX &&
> > + event_idx_code <= SBI_PMU_FW_RESERVED_MAX) ||
> > + event_idx_code > SBI_PMU_FW_PLATFORM)
> > + return SBI_EINVAL;
> > +
> > + if (SBI_PMU_FW_PLATFORM == event_idx_code &&
> > pmu_dev && pmu_dev->fw_event_validate_encoding)
> > return pmu_dev-
> > >fw_event_validate_encoding(edata);
> > else
> > @@ -187,12 +192,19 @@ 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_data[hartid][cidx - num_hw_ctrs] =
> > - pmu_dev->fw_counter_read_value(cidx -
> > num_hw_ctrs);
> > + if ((event_code >= SBI_PMU_FW_MAX &&
> > + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> > + event_code > SBI_PMU_FW_PLATFORM)
> > + return SBI_EINVAL;
> >
> > - *cval = fw_counters_data[hartid][cidx - num_hw_ctrs];
> > + if (SBI_PMU_FW_PLATFORM == event_code) {
> > + if (pmu_dev && pmu_dev->fw_counter_read_value)
> > + *cval = pmu_dev->fw_counter_read_value(cidx
> > -
> > + num_
> > hw_ctrs);
> > + else
> > + *cval = 0;
> > + } else
> > + *cval = fw_counters_data[hartid][cidx -
> > num_hw_ctrs];
> >
> > return 0;
> > }
> > @@ -367,8 +379,17 @@ static int pmu_ctr_start_fw(uint32_t cidx,
> > uint32_t event_code,
> > int ret;
> > u32 hartid = current_hartid();
> >
> > - if (SBI_PMU_FW_MAX <= event_code &&
> > - pmu_dev && pmu_dev->fw_counter_start) {
> > + if ((event_code >= SBI_PMU_FW_MAX &&
> > + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> > + event_code > SBI_PMU_FW_PLATFORM)
> > + return SBI_EINVAL;
> > +
> > + if (SBI_PMU_FW_PLATFORM == event_code) {
> > + if (!pmu_dev ||
> > + !pmu_dev->fw_counter_start) {
> > + return SBI_EINVAL;
> > + }
> > +
> > ret = pmu_dev->fw_counter_start(cidx - num_hw_ctrs,
> > event_data,
> > ival, ival_update);
> > @@ -386,12 +407,13 @@ static int pmu_ctr_start_fw(uint32_t cidx,
> > uint32_t event_code,
> > int sbi_pmu_ctr_start(unsigned long cbase, unsigned long cmask,
> > unsigned long flags, uint64_t ival)
> > {
> > + u32 hartid = current_hartid();
>
> This is an unrelated change. Probably belongs to a different patch.
When comparing with the platform fw event code we need the hartid to
get the event data that was saved for that hart and counter index.
>
> > int event_idx_type;
> > uint32_t event_code;
> > int ret = SBI_EINVAL;
> > bool bUpdate = false;
> > int i, cidx;
> > - uint64_t edata = 0;
> > + uint64_t edata;
> >
> > if ((cbase + sbi_fls(cmask)) >= total_ctrs)
> > return ret;
> > @@ -405,9 +427,13 @@ int sbi_pmu_ctr_start(unsigned long cbase,
> > unsigned long cmask,
> > if (event_idx_type < 0)
> > /* Continue the start operation for other
> > counters */
> > continue;
> > - else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW)
> > + else if (event_idx_type == SBI_PMU_EVENT_TYPE_FW) {
> > + edata = (event_code == SBI_PMU_FW_PLATFORM)
> > ?
> > + fw_counters_data[hartid][cidx -
> > num_hw_ctrs]
> > + : 0x0;
> > ret = pmu_ctr_start_fw(cidx, event_code,
> > edata, ival,
> > bUpdate);
> > + }
> > else
> > ret = pmu_ctr_start_hw(cidx, ival,
> > bUpdate);
> > }
> > @@ -441,7 +467,12 @@ static int pmu_ctr_stop_fw(uint32_t cidx,
> > uint32_t event_code)
> > {
> > int ret;
> >
> > - if (SBI_PMU_FW_MAX <= event_code &&
> > + if ((event_code >= SBI_PMU_FW_MAX &&
> > + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> > + event_code > SBI_PMU_FW_PLATFORM)
> > + return SBI_EINVAL;
> > +
> > + if (SBI_PMU_FW_PLATFORM == event_code &&
> > pmu_dev && pmu_dev->fw_counter_stop) {
> > ret = pmu_dev->fw_counter_stop(cidx - num_hw_ctrs);
> > if (ret)
> > @@ -651,13 +682,18 @@ static int pmu_ctr_find_fw(unsigned long
> > cbase, unsigned long cmask,
> > {
> > int i, cidx;
> >
> > + if ((event_code >= SBI_PMU_FW_MAX &&
> > + event_code <= SBI_PMU_FW_RESERVED_MAX) ||
> > + event_code > SBI_PMU_FW_PLATFORM)
> > + return SBI_EINVAL;
> > +
> > 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 &&
> > + if (SBI_PMU_FW_PLATFORM == event_code &&
> > pmu_dev && pmu_dev->fw_counter_match_encoding)
> > {
> > if (!pmu_dev-
> > >fw_counter_match_encoding(cidx - num_hw_ctrs,
> > eda
> > ta))
> > @@ -702,6 +738,9 @@ 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,
> > event_code,
> > hartid, event_data);
> > + if (event_code == SBI_PMU_FW_PLATFORM)
> > + fw_counters_data[hartid][ctr_idx -
> > num_hw_ctrs] =
> > + eve
> > nt_data;
> > } else {
> > ctr_idx = pmu_ctr_find_hw(cidx_base, cidx_mask,
> > flags, event_idx,
> > event_data);
> > @@ -721,7 +760,7 @@ skip_match:
> > if (flags & SBI_PMU_CFG_FLAG_CLEAR_VALUE)
> > fw_counters_data[hartid][ctr_idx -
> > num_hw_ctrs] = 0;
> > if (flags & SBI_PMU_CFG_FLAG_AUTO_START) {
> > - if (SBI_PMU_FW_MAX <= event_code &&
> > + if (SBI_PMU_FW_PLATFORM == event_code &&
> > pmu_dev && pmu_dev->fw_counter_start) {
> > ret = pmu_dev->fw_counter_start(
> > ctr_idx - num_hw_ctrs,
> > event_data,
> > --
> > 2.34.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
>
> Otherwise, it looks good to me.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Anup
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-03-09 12:33 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-09 5:51 [PATCH v3 0/8] SBI PMU firmware counters and events improvement Mayuresh Chitale
2023-03-09 5:51 ` [PATCH v3 1/8] lib: sbi_pmu: add callback for counter width Mayuresh Chitale
2023-03-09 6:18 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 2/8] lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi Mayuresh Chitale
2023-03-09 6:18 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 3/8] lib: sbi_pmu: Reserve space for implementation specific firmware events Mayuresh Chitale
2023-03-09 6:19 ` Anup Patel
2023-03-09 9:16 ` Andrew Jones
2023-03-09 12:30 ` mchitale
2023-03-09 5:51 ` [PATCH v3 4/8] lib: sbi_pmu: Rename fw_counter_value Mayuresh Chitale
2023-03-09 6:19 ` Anup Patel
2023-03-09 9:19 ` Andrew Jones
2023-03-09 5:51 ` [PATCH v3 5/8] lib: sbi_pmu: Update sbi_pmu dev ops Mayuresh Chitale
2023-03-09 6:21 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 6/8] lib: sbi_pmu: Use dedicated event code for platform firmware events Mayuresh Chitale
2023-03-09 6:28 ` Anup Patel
2023-03-09 12:33 ` mchitale
2023-03-09 5:51 ` [PATCH v3 7/8] lib: sbi_pmu: Introduce fw_counter_write_value API Mayuresh Chitale
2023-03-09 6:31 ` Anup Patel
2023-03-09 5:51 ` [PATCH v3 8/8] lib: sbi_pmu: Add hartid parameter PMU device ops Mayuresh Chitale
2023-03-09 6:32 ` Anup Patel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox