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

From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: al.grant@arm.com, Denis Nikitin <denik@chromium.org>,
	Anshuman Khandual <Anshuman.Khandual@arm.com>,
	"coresight@lists.linaro.org" <coresight@lists.linaro.org>,
	Leo Yan <leo.yan@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 11:06:42 -0700	[thread overview]
Message-ID: <20210104180642.GA2702940@xps15> (raw)
In-Reply-To: <98445ac9-9ef9-e377-034c-c65351348359@arm.com>

Hi Suzuki,

On Mon, Jan 04, 2021 at 05:33:50PM +0000, Suzuki K Poulose wrote:
> 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.

Looking at my (extensive) patch log this morning I was wondering what to do
about that patchset.  Yes, please respin it on rc2 and I'll look at it.

Reading through the above conversation with Leo, should I expect your next
patchset to replace your perf tools changes with Leo's work?  Alternatively and
knowing Leo is working on it I'd be happy with just the kernel changes. 

Thanks,
Mathieu

> 
> Kind regards
> Suzuki

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