[PATCH 1/2] powerpc/perf: Infrastructure to support checking of attr.config*

[Paul A. Clarke]

On Wed, Feb 24, 2021 at 07:58:39PM +0530, Madhavan Srinivasan wrote:
> Introduce code to support the checking of attr.config* for
> values which are reserved for a given platform.
> Performance Monitoring Unit (PMU) configuration registers
> have fileds that are reserved and specific values to bit fields

s/fileds/fields/

> as reserved. Writing a none zero values in these fields

Should the previous sentences say something like "required values
for specific bit fields" or "specific bit fields that are reserved"?

s/none zero/non-zero/

> or writing invalid value to bit fields will have unknown
> behaviours.
> 
> Patch here add a generic call-back function "check_attr_config"

s/add/adds/ or "This patch adds ..." or just "Add ...".

> in "struct power_pmu", to be called in event_init to
> check for attr.config* values for a given platform.
> "check_attr_config" is valid only for raw event type.
> 
> Suggested-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> Signed-off-by: Madhavan Srinivasan <maddy at linux.ibm.com>
> ---
>  arch/powerpc/include/asm/perf_event_server.h |  6 ++++++
>  arch/powerpc/perf/core-book3s.c              | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index 00e7e671bb4b..dde97d7d9253 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -67,6 +67,12 @@ struct power_pmu {
>  	 * the pmu supports extended perf regs capability
>  	 */
>  	int		capabilities;
> +	/*
> +	 * Function to check event code for values which are
> +	 * reserved. Function takes struct perf_event as input,
> +	 * since event code could be spread in attr.config*
> +	 */
> +	int		(*check_attr_config)(struct perf_event *ev);
>  };
> 
>  /*
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 6817331e22ff..679d67506299 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1958,6 +1958,18 @@ static int power_pmu_event_init(struct perf_event *event)
> 
>  		if (ppmu->blacklist_ev && is_event_blacklisted(ev))
>  			return -EINVAL;
> +		/*
> +		 * PMU config registers have fileds that are
> +		 * reserved and spacific values to bit fileds be reserved.

s/spacific/specific/
s/fileds/fields/
Same comment about "specific values to bit fields be reserved", and
rewording that to be more clear.

> +		 * This call-back will check the event code for same.
> +		 *
> +		 * Event type hardware and hw_cache will not value
> +		 * invalid values in the event code which is not true
> +		 * for raw event type.

I confess I don't understand what this means. (But it could be just me!)

> +		 */
> +		if (ppmu->check_attr_config &&
> +		    ppmu->check_attr_config(event))
> +			return -EINVAL;
>  		break;
>  	default:
>  		return -ENOENT;
> -- 

PC