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

From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Leo Yan <leo.yan@linaro.org>, Denis Nikitin <denik@chromium.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: al.grant@arm.com, Anshuman Khandual <Anshuman.Khandual@arm.com>,
	"coresight@lists.linaro.org" <coresight@lists.linaro.org>,
	Daniel Kiss <Daniel.Kiss@arm.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"mike.leach@linaro.org" <mike.leach@linaro.org>
Subject: Re: [PATCH 3/3] rfc: perf: cs_etm: Detect pid in VMID for kernel running at EL2
Date: Mon, 4 Jan 2021 17:33:50 +0000	[thread overview]
Message-ID: <98445ac9-9ef9-e377-034c-c65351348359@arm.com> (raw)
In-Reply-To: <20201223080532.GA26191@leoy-ThinkPad-X240s>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel