From fc58db9aeb15e89b69ff5e9abc69ecf9e5f888ed Mon Sep 17 00:00:00 2001 From: Atish Patra Date: Thu, 12 Dec 2024 16:09:32 -0800 Subject: [PATCH 1/3] drivers/perf: riscv: Fix Platform firmware event data Platform firmware event data field is allowed to be 62 bits for Linux as uppper most two bits are reserved to indicate SBI fw or platform specific firmware events. However, the event data field is masked as per the hardware raw event mask which is not correct. Fix the platform firmware event data field with proper mask. Fixes: f0c9363db2dd ("perf/riscv-sbi: Add platform specific firmware event handling") Signed-off-by: Atish Patra Link: https://lore.kernel.org/r/20241212-pmu_event_fixes_v2-v2-1-813e8a4f5962@rivosinc.com Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/sbi.h | 1 + drivers/perf/riscv_pmu_sbi.c | 12 +++++------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 6c82318065cfd4..3d250824178bd5 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -159,6 +159,7 @@ struct riscv_pmu_snapshot_data { }; #define RISCV_PMU_RAW_EVENT_MASK GENMASK_ULL(47, 0) +#define RISCV_PMU_PLAT_FW_EVENT_MASK GENMASK_ULL(61, 0) #define RISCV_PMU_RAW_EVENT_IDX 0x20000 #define RISCV_PLAT_FW_EVENT 0xFFFF diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c index 1aa303f76cc7af..3473ba02abf3cb 100644 --- a/drivers/perf/riscv_pmu_sbi.c +++ b/drivers/perf/riscv_pmu_sbi.c @@ -507,7 +507,6 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) { u32 type = event->attr.type; u64 config = event->attr.config; - u64 raw_config_val; int ret; /* @@ -528,21 +527,20 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) case PERF_TYPE_RAW: /* * As per SBI specification, the upper 16 bits must be unused - * for a raw event. + * for a hardware raw event. * Bits 63:62 are used to distinguish between raw events * 00 - Hardware raw event * 10 - SBI firmware events * 11 - Risc-V platform specific firmware event */ - raw_config_val = config & RISCV_PMU_RAW_EVENT_MASK; + switch (config >> 62) { case 0: ret = RISCV_PMU_RAW_EVENT_IDX; - *econfig = raw_config_val; + *econfig = config & RISCV_PMU_RAW_EVENT_MASK; break; case 2: - ret = (raw_config_val & 0xFFFF) | - (SBI_PMU_EVENT_TYPE_FW << 16); + ret = (config & 0xFFFF) | (SBI_PMU_EVENT_TYPE_FW << 16); break; case 3: /* @@ -551,7 +549,7 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) * Event data - raw event encoding */ ret = SBI_PMU_EVENT_TYPE_FW << 16 | RISCV_PLAT_FW_EVENT; - *econfig = raw_config_val; + *econfig = config & RISCV_PMU_PLAT_FW_EVENT_MASK; break; } break; From 2c206cdede567f53035c622e846678a996f39d69 Mon Sep 17 00:00:00 2001 From: Atish Patra Date: Thu, 12 Dec 2024 16:09:33 -0800 Subject: [PATCH 2/3] drivers/perf: riscv: Return error for default case If the upper two bits has an invalid valid (0x1), the event mapping is not reliable as it returns an uninitialized variable. Return appropriate value for the default case. Fixes: f0c9363db2dd ("perf/riscv-sbi: Add platform specific firmware event handling") Signed-off-by: Atish Patra Link: https://lore.kernel.org/r/20241212-pmu_event_fixes_v2-v2-2-813e8a4f5962@rivosinc.com Signed-off-by: Palmer Dabbelt --- drivers/perf/riscv_pmu_sbi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c index 3473ba02abf3cb..da3651d329069c 100644 --- a/drivers/perf/riscv_pmu_sbi.c +++ b/drivers/perf/riscv_pmu_sbi.c @@ -507,7 +507,7 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) { u32 type = event->attr.type; u64 config = event->attr.config; - int ret; + int ret = -ENOENT; /* * Ensure we are finished checking standard hardware events for @@ -551,10 +551,11 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) ret = SBI_PMU_EVENT_TYPE_FW << 16 | RISCV_PLAT_FW_EVENT; *econfig = config & RISCV_PMU_PLAT_FW_EVENT_MASK; break; + default: + break; } break; default: - ret = -ENOENT; break; } From 3aff4cdbe506652da77570baccad623511628250 Mon Sep 17 00:00:00 2001 From: Atish Patra Date: Thu, 12 Dec 2024 16:09:34 -0800 Subject: [PATCH 3/3] drivers/perf: riscv: Do not allow invalid raw event config The SBI specification allows only lower 48bits of hpmeventX to be configured via SBI PMU. Currently, the driver masks of the higher bits but doesn't return an error. This will lead to an additional SBI call for config matching which should return for an invalid event error in most of the cases. However, if a platform(i.e Rocket and sifive cores) implements a bitmap of all bits in the event encoding this will lead to an incorrect event being programmed leading to user confusion. Report the error to the user if higher bits are set during the event mapping itself to avoid the confusion and save an additional SBI call. Suggested-by: Samuel Holland Signed-off-by: Atish Patra Link: https://lore.kernel.org/r/20241212-pmu_event_fixes_v2-v2-3-813e8a4f5962@rivosinc.com Signed-off-by: Palmer Dabbelt --- drivers/perf/riscv_pmu_sbi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c index da3651d329069c..194c153e5d71c6 100644 --- a/drivers/perf/riscv_pmu_sbi.c +++ b/drivers/perf/riscv_pmu_sbi.c @@ -536,8 +536,11 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) switch (config >> 62) { case 0: - ret = RISCV_PMU_RAW_EVENT_IDX; - *econfig = config & RISCV_PMU_RAW_EVENT_MASK; + /* Return error any bits [48-63] is set as it is not allowed by the spec */ + if (!(config & ~RISCV_PMU_RAW_EVENT_MASK)) { + *econfig = config & RISCV_PMU_RAW_EVENT_MASK; + ret = RISCV_PMU_RAW_EVENT_IDX; + } break; case 2: ret = (config & 0xFFFF) | (SBI_PMU_EVENT_TYPE_FW << 16);