Message-ID: <0c6d3625-228a-4cb0-b75f-57f1d4069ced@arm.com>
Date: Sat, 31 Aug 2024 12:37:29 +0100
From: Leo Yan <leo.yan@....com>
To: Will Deacon <will@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
 Mark Rutland <mark.rutland@....com>,
 Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach
 <mike.leach@...aro.org>, James Clark <james.clark@...aro.org>,
 John Garry <john.g.garry@...cle.com>, Namhyung Kim <namhyung@...nel.org>,
 Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
 "Liang, Kan" <kan.liang@...ux.intel.com>,
 Jonathan Cameron <jonathan.cameron@...wei.com>,
 Yicong Yang <yangyicong@...ilicon.com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 coresight@...ts.linaro.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v1 1/9] perf: arm_spe: Introduce 'lds' capacity

On 8/30/2024 2:09 PM, Will Deacon wrote:

[...]

>>>> @@ -160,6 +162,7 @@ static ssize_t arm_spe_pmu_cap_show(struct device *dev,
>>>>
>>>>   static struct attribute *arm_spe_pmu_cap_attr[] = {
>>>>        SPE_CAP_EXT_ATTR_ENTRY(arch_inst, SPE_PMU_CAP_ARCH_INST),
>>>> +     SPE_CAP_EXT_ATTR_ENTRY(lds, SPE_PMU_CAP_LDS),
>>>>        SPE_CAP_EXT_ATTR_ENTRY(ernd, SPE_PMU_CAP_ERND),
>>>>        SPE_CAP_EXT_ATTR_ENTRY(count_size, SPE_PMU_CAP_CNT_SZ),
>>>>        SPE_CAP_EXT_ATTR_ENTRY(min_interval, SPE_PMU_CAP_MIN_IVAL),
>>>
>>> What will userspace do with this? I don't think you can turn LDS on/off,
>>> so either you'll get the data source packet or you won't.
>>
>> Yes, LDS bit does not work as a switch.
>>
>> The tool in the userspace will record the LDS bit into the metadata. During
>> decoding phase, it reads out the LDS from metadata. Based on it, the perf
>> tool can know if the data source is supported or not, if yes then decode the
>> data source packet.
> 
> Why not just decode a data source packet when you see it? i.e. assume LDS
> is always set.

The current tool works this way to directly decode a data source packet.

However, as Arm ARM section D17.2.4 "Data Source packet" describes, the loaded
data source is implementation dependent, the data source payload format also
is implementation defined.

We are halfway here in using the LDS bit to determine if the data source is
implemented. However, we lack information on the data source format
implementation. As a first step, we can use the LDS bit for sanity checking in
the tool to detect any potential silicon implementation issues. Once we have
an architectural definition for the data source format, we can extend the tool
accordingly.

>> Another point is how to decide the data source packet format. Now we maintain
>> a CPU list for tracking CPU variants which support data source trace. For long
>> term, I would like the tool can based on hardware feature (e.g. a ID register
>> in Arm SPE) to decide the data source format, so far it is absent. This is why
>> LDS bit + CPU list is a more reliable way. See some discussion [1].
> 
> Huh. Why would you have a CPU in the list if it _doesn't_ have LDS?

Yeah, this is what we don't expect - we can verify the implementation based on
LDS bit.

E.g. if users ask data source related questions, we can use LDS bit (saved in
the perf metadata) to confirm the feature has been implemented in a silicon.

> If we have to resort to per-CPU decoding, then that's even more of a reason>
not to have the LDS cap imo.

This series converts the Arm SPE information into per-CPU metadata, including
the LDS bit. Consequently, the decoding process retrieves CPU metadata for
per-CPU decoding, making it easy to determine if a CPU supports the data source.

We have platforms that not all CPUs support Arm SPE, for example, the CPU0 and
CPU1 don't support Arm SPE, CPU2~CPU5 share a Arm SPE PMU event, CPU6~CPU7
share another Arm SPE PMU event. In this case, per CPU metadata can be easily
for checking hardware capacity (include LDS bit) in the decoding.

Thanks,
Leo

Powered by blists - more mailing lists