[PATCH 2/8] drivers/perf: riscv: Fix Platform firmware event data

[Alexandre Ghiti]

Hi Atish,

On 19/11/2024 21:29, Atish Patra wrote:
> 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 <atishp at rivosinc.com>
> ---
>   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 98f631b051db..9be38b05f4ad 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -158,6 +158,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 cb98efa9b106..50cbdbf66bb7 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -508,7 +508,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;
>   
>   	/*
> @@ -529,21 +528,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:
>   			/*
> @@ -552,7 +550,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;
>

It seems independent from the other patches, so I guess we should take 
it for 6.13 rcX.

Let me know if that's not the case.

Thanks,

Alex