| Message ID | 20230814043134.367280-1-mchitale@ventanamicro.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | drivers/perf: Risc-V: Update platform specific firmware event handling | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 174e8ac0272d |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 2 this patch: 2 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 25 this patch: 25 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 12 this patch: 12 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 53 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Mon, Aug 14, 2023 at 10:01:34AM +0530, Mayuresh Chitale wrote: > + /* > + * For Risc-V platform specific firmware events This is a nit, but it my OCD is triggered every time I read it - please don't invent another style of writing the name of the architecture, both here and in the commit message. If you respin for some other reason, could you please change these to one of the established styles? Thanks, Conor. > + * Event code - 0xFFFF > + * Event data - raw event encoding > + */ > + ret = SBI_PMU_EVENT_TYPE_FW << 16 | RISCV_PLAT_FW_EVENT; > *econfig = raw_config_val; > + break;
On Mon, Aug 14, 2023 at 10:01:34AM +0530, Mayuresh Chitale wrote: > SBI specification [1] reserves an event code for platform specific > firmware events. Update the driver to use the new reserved event code > for platform specific firmware events. Also update the raw event parsing > to use bits 63:62 instead of only bit 63 to distinguish between the raw > event types. The platform specific firmware events must be specified in > the perf command as in the below example: > perf stat -e rc000000000000001 <command> > where [63:62] = 0x3 indicates a platform specific firmware event > > Link: https://github.com/riscv-non-isa/riscv-sbi-doc/commit/c4bfdf9 [1] > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> Hmm, another question - is this actually in a frozen version of the SBI spec? I might've got distracted during the last PRS meeting and be unaware, but I had it in my head that it was not frozen.
On Tue, Aug 15, 2023 at 6:19 AM Conor Dooley <conor@kernel.org> wrote: > > On Mon, Aug 14, 2023 at 10:01:34AM +0530, Mayuresh Chitale wrote: > > SBI specification [1] reserves an event code for platform specific > > firmware events. Update the driver to use the new reserved event code > > for platform specific firmware events. Also update the raw event parsing > > to use bits 63:62 instead of only bit 63 to distinguish between the raw > > event types. The platform specific firmware events must be specified in > > the perf command as in the below example: > > perf stat -e rc000000000000001 <command> > > where [63:62] = 0x3 indicates a platform specific firmware event > > > > Link: https://github.com/riscv-non-isa/riscv-sbi-doc/commit/c4bfdf9 [1] > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > > Hmm, another question - is this actually in a frozen version of the SBI > spec? I might've got distracted during the last PRS meeting and be > unaware, but I had it in my head that it was not frozen. Yes. It is not frozen yet. We are in the process of getting all the RVI approvals to freeze the spec.
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 5b4a1bf5f439..001387354c2e 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -138,6 +138,7 @@ union sbi_pmu_ctr_info { #define RISCV_PMU_RAW_EVENT_MASK GENMASK_ULL(47, 0) #define RISCV_PMU_RAW_EVENT_IDX 0x20000 +#define RISCV_PLAT_FW_EVENT 0xFFFF /** General pmu event codes specified in SBI PMU extension */ enum sbi_pmu_hw_generic_events_t { diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c index 4163ff517471..f94a3f0a469b 100644 --- a/drivers/perf/riscv_pmu_sbi.c +++ b/drivers/perf/riscv_pmu_sbi.c @@ -414,7 +414,6 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) { u32 type = event->attr.type; u64 config = event->attr.config; - int bSoftware; u64 raw_config_val; int ret; @@ -429,18 +428,32 @@ static int pmu_sbi_event_map(struct perf_event *event, u64 *econfig) break; case PERF_TYPE_RAW: /* - * As per SBI specification, the upper 16 bits must be unused for - * a raw event. Use the MSB (63b) to distinguish between hardware - * raw event and firmware events. + * As per SBI specification, the upper 16 bits must be unused + * for a raw event. Hence 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 */ - bSoftware = config >> 63; raw_config_val = config & RISCV_PMU_RAW_EVENT_MASK; - if (bSoftware) { + switch (config >> 62) { + case 0: + ret = RISCV_PMU_RAW_EVENT_IDX; + *econfig = raw_config_val; + break; + case 2: ret = (raw_config_val & 0xFFFF) | (SBI_PMU_EVENT_TYPE_FW << 16); - } else { - ret = RISCV_PMU_RAW_EVENT_IDX; + break; + case 3: + /* + * For Risc-V platform specific firmware events + * Event code - 0xFFFF + * Event data - raw event encoding + */ + ret = SBI_PMU_EVENT_TYPE_FW << 16 | RISCV_PLAT_FW_EVENT; *econfig = raw_config_val; + break; } break; default:
SBI specification [1] reserves an event code for platform specific firmware events. Update the driver to use the new reserved event code for platform specific firmware events. Also update the raw event parsing to use bits 63:62 instead of only bit 63 to distinguish between the raw event types. The platform specific firmware events must be specified in the perf command as in the below example: perf stat -e rc000000000000001 <command> where [63:62] = 0x3 indicates a platform specific firmware event Link: https://github.com/riscv-non-isa/riscv-sbi-doc/commit/c4bfdf9 [1] Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> --- arch/riscv/include/asm/sbi.h | 1 + drivers/perf/riscv_pmu_sbi.c | 29 +++++++++++++++++++++-------- 2 files changed, 22 insertions(+), 8 deletions(-)