Re: [PATCH 4/7] perf cs-etm: Don't use hard coded config bits when setting up ETMCR

James Clark
2025-12-02T11:53:40Z

On 02/12/2025 11:43 am, Leo Yan wrote:
> On Mon, Dec 01, 2025 at 04:41:07PM +0000, Coresight ML wrote:
>
> [...]
>
>> @@ -746,7 +779,7 @@ static void cs_etm_get_metadata(struct perf_cpu cpu, u32 *offset,
>> case CS_ETMV3:
>> magic = __perf_cs_etmv3_magic;
>> /* Get configuration register */
>> - info->priv[*offset + CS_ETM_ETMCR] = cs_etm_get_config(itr);
>> + info->priv[*offset + CS_ETM_ETMCR] = cs_etm_guess_etmcr(itr);
>
> I still think cs_etm_get_config() is better than cs_etm_guess_etmcr().
>
> For ETMv3, we directly pass CONFIG to the kernel, and after validation
> in the dirver, then the value will be set to ETMCR. If we already know
> the config value is consistent between user space and kernel, why
> introduce a redundant "guess" operation here?
>
> Thanks,
> Leo

Because userspace doesn't always come up with the same value as the
driver. For example right now in ETM3, ETMCR_RETURN_STACK isn't set
depending on certain conditions that userspace doesn't know about. ETM4
has the same for TRCCONFIGR_RS and maybe some others. In the future,
other versions of the driver could do different things as long as we
don't break decoding.

I didn't want the function name to imply it was doing something it
wasn't as that confused me a little bit. It's definitely not "getting"
the value. Maybe "guess" isn't the best it could be, but it's not far off.