Re: [PATCH 3/3] rfc: perf: cs_etm: Detect pid in VMID for kernel running at EL2

Suzuki K Poulose
2021-01-04T17:35:50Z

Hi Leo,

On 12/23/20 8:05 AM, Leo Yan wrote:
> Hi Denis,
>
> On Tue, Dec 22, 2020 at 01:57:41AM -0800, Denis Nikitin wrote:
>
> [...]
>
>>>> Below is the drafted patch for support PID format in the metadata and
>>>> I tested for "perf record" and "perf script" command and can work as
>>>> expected.
>>>>
>>>> P.s. I uploaded the patches into the github [1] and gave some minor
>>>> refactoring for your last patch "perf cs-etm: Detect pid in VMID for
>>>> kernel running at EL2".
>>>
>> I have tested the patches on Chrome OS EL2 kernel and they worked fine for
>> me.
>
> Thanks a lot for the testing!
>
>> Note that "perf cs-etm: Add PID format into metadata" patch breaks perf
>> backward compatibility. It may cause a problem in off-target decoding if
>> there is a version skew in perf.
>> I saw a discussion about perf compatibility in
>> https://lists.linaro.org/pipermail/coresight/2020-November/005326.html.
>> I understand that perf doesn't guarantee backward compatibility but in fact
>> incompatibility issues occur rarely. I think if there is an (easy) way to
>> do it the compatibility breakage should be avoided.
>
> Agreed. After reading the code and I think it's possible to do an extra
> checking the length of auxtrace info structure so that can know if
> the item CS_ETMV4_PID_FMT is valid or not. Thus we needs to write a
> parity function of intel_pt_has() for perf cs-etm; I will try to rework
> this patch.
>
>> This is a critical fix for Chrome OS. Please let me know what you think.
>
> So are you asking to upstream the changes to mainline kernel? You
> could see this patch is owned by Suzuki and I only proposed for one
> patch for perf related change.
>
> Suzuki, could you give some update for the plan of this patch set?
> I can help to prepare the perf patch based on Suzuki's plan.

I am happy for you to pick up the perf tool changes as needed. I believe
there are no changes required for the patches, 1 & 2, which adds the basic
kernel and perf tool support. As I have mentioned in the description of this
patch, this is clearly an RFC and is a hack. So I am happy that you can
fix this properly in perf tool decoding.

Mathieu,

Are you happy with the proposed series to solve this issue ? I could respin
the series on the latest upstream tree if you like.

Kind regards
Suzuki