Re: [PATCH v1 1/9] perf: arm_spe: Introduce 'lds' capacity

[Leo Yan <leo.yan@arm.com>]

On 9/4/2024 1:35 PM, Will Deacon wrote:

>>> 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.
> 
> I don't think we shyould expose UAPI from the driver to detect potential
> hardware bugs. Let's add it when we know it's useful for something instead.

I understand your concern.

>>>> 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.
> 
> What exactly do you mean by this?

Sometimes, users might ask if the data source is supported in Arm SPE. With exposing
the LDS bit, it would be helpful for us to check if the feature is supported. For
example, when a user uses the perf tool to record Arm SPE data, the LDS bit is stored
in the perf metadata, and we can check its value during post-analysis.
 
> As far as I can tell:
> 
>   - Data source packets are either present or absent depending on LDS
>   - You need CPU-specific information to decode them it they are present
> 
> So it's neither necessary nor sufficient to expose the LDS bit to
> userspace.

We can live without exposing LDS bit currently. I will drop this change in next spin.

Just head up, later I think we might still need to expose capacity bits (or the
PMSIDR_EL1 register) for new features. As you said, we can do it when it is necessary.

Thanks for suggestion!

Leo