Re: [PATCH v4 03/15] drivers/perf: riscv: Read upper bits of a firmware counter

On Wed, Feb 28, 2024 at 05:01:18PM -0800, Atish Patra wrote:

SBI v2.0 introduced a explicit function to read the upper 32 bits for any firmwar counter width that is longer than 32bits.

firmware

This is only applicable for RV32 where firmware counter can be 64 bit.

Acked-by: Palmer Dabbelt palmer@rivosinc.com Reviewed-by: Conor Dooley conor.dooley@microchip.com Reviewed-by: Anup Patel anup@brainfault.org Signed-off-by: Atish Patra atishp@rivosinc.com

---

drivers/perf/riscv_pmu_sbi.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c index 16acd4dcdb96..ea0fdb589f0d 100644 --- a/drivers/perf/riscv_pmu_sbi.c +++ b/drivers/perf/riscv_pmu_sbi.c @@ -35,6 +35,8 @@ PMU_FORMAT_ATTR(event, "config:0-47"); PMU_FORMAT_ATTR(firmware, "config:63"); +static bool sbi_v2_available;

static struct attribute *riscv_arch_formats_attr[] = { &format_attr_event.attr, &format_attr_firmware.attr, @@ -488,16 +490,23 @@ static u64 pmu_sbi_ctr_read(struct perf_event *event) struct hw_perf_event *hwc = &event->hw; int idx = hwc->idx; struct sbiret ret;

• union sbi_pmu_ctr_info info; u64 val = 0;

• union sbi_pmu_ctr_info info = pmu_ctr_list[idx];

if (pmu_sbi_is_fw_event(event)) { ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_FW_READ, hwc->idx, 0, 0, 0, 0, 0);

• if (!ret.error)
  

• 	val = ret.value;
  

• if (ret.error)
  

• 	return 0;
  

• val = ret.value;
  

• if (IS_ENABLED(CONFIG_32BIT) && sbi_v2_available && info.width >= 32) {
  

• 	ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_FW_READ_HI,
  

• 			hwc->idx, 0, 0, 0, 0, 0);
  

• 	if (!ret.error)
  

Getting an error here indicates a buggy SBI. Maybe we should have a warn-once?

• 		val |= ((u64)ret.value << 32);
  

• }
  

  } else {

• info = pmu_ctr_list[idx];
  

  val = riscv_pmu_ctr_read_csr(info.csr); if (IS_ENABLED(CONFIG_32BIT)) val = ((u64)riscv_pmu_ctr_read_csr(info.csr + 0x80)) << 31 | val;

@@ -1108,6 +1117,9 @@ static int __init pmu_sbi_devinit(void) return 0; }

• if (sbi_spec_version >= sbi_mk_version(2, 0))

• sbi_v2_available = true;
  

• ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_RISCV_STARTING, "perf/riscv/pmu:starting", pmu_sbi_starting_cpu, pmu_sbi_dying_cpu);

-- 2.34.1

Either way,

Reviewed-by: Andrew Jones ajones@ventanamicro.com