Date:   Fri, 10 Dec 2021 10:41:37 +0800
From:   Leo Yan <leo.yan@linaro.org>
To:     James Clark <james.clark@arm.com>
Cc:     mathieu.poirier@linaro.org, coresight@lists.linaro.org,
        suzuki.poulose@arm.com, Mike Leach <mike.leach@linaro.org>,
        John Garry <john.garry@huawei.com>,
        Will Deacon <will@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/3] perf cs-etm: Update deduction of TRCCONFIGR register
 for branch broadcast
Message-ID: <20211210024137.GB622826@leoy-ThinkPad-X240s>
References: <20211208160907.749482-1-james.clark@arm.com>
 <20211208160907.749482-3-james.clark@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211208160907.749482-3-james.clark@arm.com>
Precedence: bulk

On Wed, Dec 08, 2021 at 04:09:07PM +0000, James Clark wrote:
> Now that a config flag for branch broadcast has been added, take it into
> account when trying to deduce what the driver would have programmed the
> TRCCONFIGR register to.
> 
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/include/linux/coresight-pmu.h | 2 ++
>  tools/perf/arch/arm/util/cs-etm.c   | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h
> index 4ac5c081af93..6c2fd6cc5a98 100644
> --- a/tools/include/linux/coresight-pmu.h
> +++ b/tools/include/linux/coresight-pmu.h
> @@ -18,6 +18,7 @@
>   * ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and
>   * directly use below macros as config bits.
>   */
> +#define ETM_OPT_BRANCH_BROADCAST 8

I checked ETMv3 architecture spec (ARM IHI 0014Q), its bit 8 is "branch
output" for all branch address outputting.  I am not sure if it is the
same thing between ETMv3's "branch output" and ETMv4's "branch
broadcasting", but it makes sense for me to use bit 8 as an unified
config bit to control these two options for ETMv3 and ETMv4
respectively.

Just note, I understand this patch set is to enable branch broadcasting
for entire memory region rather than using any comparators (see
TRCBBCTLR) to limit branch broadcasting ranges.  This is fine for me and
we could enable ranges later.

Reviewed-by: Leo Yan <leo.yan@linaro.org>

>  #define ETM_OPT_CYCACC		12
>  #define ETM_OPT_CTXTID		14
>  #define ETM_OPT_CTXTID2		15
> @@ -25,6 +26,7 @@
>  #define ETM_OPT_RETSTK		29
>  
>  /* ETMv4 CONFIGR programming bits for the ETM OPTs */
> +#define ETM4_CFG_BIT_BB         3
>  #define ETM4_CFG_BIT_CYCACC	4
>  #define ETM4_CFG_BIT_CTXTID	6
>  #define ETM4_CFG_BIT_VMID	7
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index 293a23bf8be3..c7ef4e9b4a3a 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -527,6 +527,9 @@ static u64 cs_etmv4_get_config(struct auxtrace_record *itr)
>  	if (config_opts & BIT(ETM_OPT_CTXTID2))
>  		config |= BIT(ETM4_CFG_BIT_VMID) |
>  			  BIT(ETM4_CFG_BIT_VMID_OPT);
> +	if (config_opts & BIT(ETM_OPT_BRANCH_BROADCAST))
> +		config |= BIT(ETM4_CFG_BIT_BB);
> +
>  	return config;
>  }
>  
> -- 
> 2.28.0
>