Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0267c94e-7989-ca92-4175-d820d1d63a0c@linux.intel.com>
Date:   Thu, 11 Aug 2022 08:23:31 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.1.2
Subject: Re: [PATCH] perf/x86/intel/lbr: fix branch type encoding
To:     Stephane Eranian <eranian@google.com>, linux-kernel@vger.kernel.org
Cc:     peterz@infradead.org, kan.liang@intel.com, ak@linux.intel.com,
        acme@redhat.com, namhyung@kernel.org, irogers@google.com
References: <20220810210656.2799243-1-eranian@google.com>
Content-Language: en-US
From:   "Liang, Kan" <kan.liang@linux.intel.com>
In-Reply-To: <20220810210656.2799243-1-eranian@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2022-08-10 5:06 p.m., Stephane Eranian wrote:
> With architected LBR, the procesosr can record the type of each sampled taken
> branch. The type is encoded in 4-bit field in the LBR_INFO MSR of each entry.
> 
> The branch type must then extracted and saved in the perf_branch_entry in the
> perf_events sampling buffer. With the current code, the raw Intel encoding of
> the branch is exported to user tools. 

In the intel_pmu_lbr_filter(), the raw encoding will be converted into
the X86_BR_* format via arch_lbr_br_type_map[]. Then the
common_branch_type() will convert the X86_BR_* format to the generic
PERF_BR_* type and expose to user tools.

I double check the existing arch_lbr_br_type_map[] and branch_map[].
They should generate the same PERF_BR_* type as your arch_lbr_type_map[].

Is there a test case which I can use to reproduce the problem?

Thanks,
Kan

> Yet tools, such as perf, expected the
> branch type to be encoded using perf_events branch type enum
> (see tools/perf/util/branch.c). As a result of the discrepancy, the output of
> perf report -D shows bogus branch types.
> 
> Fix the problem by converting the Intel raw encoding into the perf_events
> branch type enum values. With that in place and with no changes to the tools,
> the branch types are now reported properly.
> 
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
>  arch/x86/events/intel/lbr.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 4f70fb6c2c1e..ef63d4d46b50 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -894,9 +894,23 @@ static DEFINE_STATIC_KEY_FALSE(x86_lbr_mispred);
>  static DEFINE_STATIC_KEY_FALSE(x86_lbr_cycles);
>  static DEFINE_STATIC_KEY_FALSE(x86_lbr_type);
>  
> -static __always_inline int get_lbr_br_type(u64 info)
> +/*
> + * Array index encodes IA32_LBR_x_INFO Branch Type Encodings
> + * as per Intel SDM Vol3b Branch Types section
> + */
> +static const int arch_lbr_type_map[]={
> +	[0] = PERF_BR_COND,
> +	[1] = PERF_BR_IND,
> +	[2] = PERF_BR_UNCOND,
> +	[3] = PERF_BR_IND_CALL,
> +	[4] = PERF_BR_CALL,
> +	[5] = PERF_BR_RET,
> +};
> +#define ARCH_LBR_TYPE_COUNT ARRAY_SIZE(arch_lbr_type_map)
> +
> +static __always_inline u16 get_lbr_br_type(u64 info)
>  {
> -	int type = 0;
> +	u16 type = 0;
>  
>  	if (static_branch_likely(&x86_lbr_type))
>  		type = (info & LBR_INFO_BR_TYPE) >> LBR_INFO_BR_TYPE_OFFSET;
> @@ -904,6 +918,21 @@ static __always_inline int get_lbr_br_type(u64 info)
>  	return type;
>  }
>  
> +/*
> + * The kernel cannot expose raw Intel branch type encodings because they are
> + * not generic. Instead, the function below  maps the encoding to the
> + * perf_events user visible branch types.
> + */
> +static __always_inline int get_lbr_br_type_mapping(u64 info)
> +{
> +	if (static_branch_likely(&x86_lbr_type)) {
> +		u16 raw_type = get_lbr_br_type(info);
> +		if (raw_type < ARCH_LBR_TYPE_COUNT)
> +			return arch_lbr_type_map[raw_type];
> +	}
> +	return PERF_BR_UNKNOWN;
> +}
> +
>  static __always_inline bool get_lbr_mispred(u64 info)
>  {
>  	bool mispred = 0;
> @@ -957,7 +986,7 @@ static void intel_pmu_store_lbr(struct cpu_hw_events *cpuc,
>  		e->in_tx	= !!(info & LBR_INFO_IN_TX);
>  		e->abort	= !!(info & LBR_INFO_ABORT);
>  		e->cycles	= get_lbr_cycles(info);
> -		e->type		= get_lbr_br_type(info);
> +		e->type		= get_lbr_br_type_mapping(info);
>  	}
>  
>  	cpuc->lbr_stack.nr = i;