[PATCH V5 6/8] perf/tools: Extend branch type classification

[James Clark]

On 04/04/2022 05:50, Anshuman Khandual wrote:
> This updates the perf tool with generic branch type classification with new
> ABI extender place holder i.e PERF_BR_EXTEND_ABI, the new 4 bit branch type
> field i.e perf_branch_entry.new_type, new generic page fault related branch
> types and some arch specific branch types as added earlier in the kernel.
> 
> Cc: Peter Zijlstra <peterz at infradead.org>
> Cc: Ingo Molnar <mingo at redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme at kernel.org>
> Cc: Mark Rutland <mark.rutland at arm.com>
> Cc: Alexander Shishkin <alexander.shishkin at linux.intel.com>
> Cc: Jiri Olsa <jolsa at redhat.com>
> Cc: Namhyung Kim <namhyung at kernel.org>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Will Deacon <will at kernel.org>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-perf-users at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual at arm.com>
> ---
>  tools/include/uapi/linux/perf_event.h | 16 +++++++++++++++-
>  tools/perf/util/branch.c              |  3 ++-
>  tools/perf/util/branch.h              |  3 ++-
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index 26d8f0b5ac0d..d29280adc3c4 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h
> @@ -255,9 +255,22 @@ enum {
>  	PERF_BR_IRQ		= 12,	/* irq */
>  	PERF_BR_SERROR		= 13,	/* system error */
>  	PERF_BR_NO_TX		= 14,	/* not in transaction */
> +	PERF_BR_EXTEND_ABI	= 15,	/* extend ABI */
>  	PERF_BR_MAX,
>  };
>  
> +enum {
> +	PERF_BR_NEW_FAULT_ALGN		= 0,    /* Alignment fault */
> +	PERF_BR_NEW_FAULT_DATA		= 1,    /* Data fault */
> +	PERF_BR_NEW_FAULT_INST		= 2,    /* Inst fault */
> +	PERF_BR_NEW_ARCH_1		= 3,    /* Architecture specific */
> +	PERF_BR_NEW_ARCH_2		= 4,    /* Architecture specific */
> +	PERF_BR_NEW_ARCH_3		= 5,    /* Architecture specific */
> +	PERF_BR_NEW_ARCH_4		= 6,    /* Architecture specific */
> +	PERF_BR_NEW_ARCH_5		= 7,    /* Architecture specific */
> +	PERF_BR_NEW_MAX,
> +};
> +
>  #define PERF_SAMPLE_BRANCH_PLM_ALL \
>  	(PERF_SAMPLE_BRANCH_USER|\
>  	 PERF_SAMPLE_BRANCH_KERNEL|\
> @@ -1372,7 +1385,8 @@ struct perf_branch_entry {
>  		abort:1,    /* transaction abort */
>  		cycles:16,  /* cycle count to last branch */
>  		type:4,     /* branch type */
> -		reserved:40;
> +		new_type:4, /* additional branch type */
> +		reserved:36;
>  };
>  
>  union perf_sample_weight {
> diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
> index abc673347bee..4bd52de0527c 100644
> --- a/tools/perf/util/branch.c
> +++ b/tools/perf/util/branch.c
> @@ -53,7 +53,8 @@ const char *branch_type_name(int type)
>  		"ERET",
>  		"IRQ",
>  		"SERROR",
> -		"NO_TX"
> +		"NO_TX",
> +		"EXTEND_ABI"

Hi Anshuman,

I still think it's best to make this change a bit more transparent to the internals
of perf and to users. For example if they dump the file, they'll see "EXTEND_ABI"
printed out, but what they really want to see is whatever the branch type is,
including the new types.

The same goes for any function that accesses the branch type (if there is one),
it should just return the final type and skip the details about EXTEND_ABI.

If that was done I don't think you'd need to add that string because as it would
never get printed.

Thanks
James

>  	};
>  
>  	if (type >= 0 && type < PERF_BR_MAX)
> diff --git a/tools/perf/util/branch.h b/tools/perf/util/branch.h
> index 17b2ccc61094..37b6ed546c46 100644
> --- a/tools/perf/util/branch.h
> +++ b/tools/perf/util/branch.h
> @@ -24,7 +24,8 @@ struct branch_flags {
>  			u64 abort:1;
>  			u64 cycles:16;
>  			u64 type:4;
> -			u64 reserved:40;
> +			u64 new_type:4;
> +			u64 reserved:36;
>  		};
>  	};
>  };