Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu

[Madhavan Srinivasan <maddy@linux.ibm.com>]

On 6/2/20 8:26 AM, Alexey Kardashevskiy wrote:
> The bhrb_filter_map ("The  Branch  History  Rolling  Buffer") callback is
> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
> generic_compat_pmu which does not have this callback and crashed occur.
>
> This add a NULL pointer check for bhrb_filter_map() which behaves as if
> the callback returned an error.
>
> This does not add the same check for config_bhrb() as the only caller
> checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.

Changes looks fine.
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>

The commit be80e758d0c2e ('powerpc/perf: Add generic compat mode pmu 
driver')
which introduced generic_compat_pmu was merged in v5.2.  So we need to
CC stable starting from 5.2 :( .  My bad,  sorry.

Maddy

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 3dcfecf858f3..36870569bf9c 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event *event, int ef_flags)
>   	ret = 0;
>    out:
>   	if (has_branch_stack(event)) {
> -		power_pmu_bhrb_enable(event);
> -		cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
> -					event->attr.branch_sample_type);
> +		u64 bhrb_filter = -1;
> +
> +		if (ppmu->bhrb_filter_map)
> +			bhrb_filter = ppmu->bhrb_filter_map(
> +				event->attr.branch_sample_type);
> +
> +		if (bhrb_filter != -1) {
> +			cpuhw->bhrb_filter = bhrb_filter;
> +			power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
> +		}
>   	}
>
>   	perf_pmu_enable(event->pmu);
> @@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct perf_event *event)
>   	int n;
>   	int err;
>   	struct cpu_hw_events *cpuhw;
> -	u64 bhrb_filter;
>
>   	if (!ppmu)
>   		return -ENOENT;
> @@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct perf_event *event)
>   	err = power_check_constraints(cpuhw, events, cflags, n + 1);
>
>   	if (has_branch_stack(event)) {
> -		bhrb_filter = ppmu->bhrb_filter_map(
> +		u64 bhrb_filter = -1;
> +
> +		if (ppmu->bhrb_filter_map)
> +			bhrb_filter = ppmu->bhrb_filter_map(
>   					event->attr.branch_sample_type);
>
>   		if (bhrb_filter == -1) {