Some fixes to support an issue reported by Denis Nikitin where decoding
trace that contains different EL1 and EL2 kernels can crash or go into
an infinite loop because the wrong kernel maps are used for the decode.

This still doesn't support distinguishing guest and host userspace,
we'd still have to fix the timestamps and do a bit more work to
correlate that. And I've removed PERF_RECORD_MISC_HYPERVISOR as a
possible outcome of cs_etm__cpu_mode(). As far as I know this could
never have been returned anyway because machine__is_host(machine) was
always true due to session.machines.host being hard coded. And I'm not
sure of the relevance of the difference between PERF_RECORD_MISC_KERNEL
and PERF_RECORD_MISC_HYPERVISOR in this scenario.

The first commit is a tidy up, second fixes a bug that I found when
comparing the exception level and thread of branch records, the third
is the main fix, and the last commit is some extra error checking. 

Applies to acme/perf-tools (4e111f0cf0)

James Clark (4):
  perf cs-etm: Only track threads instead of PID and TIDs
  perf cs-etm: Use previous thread for branch sample source IP
  perf cs-etm: Track exception level
  perf cs-etm: Add exception level consistency check

 .../perf/util/cs-etm-decoder/cs-etm-decoder.c |  13 +-
 .../perf/util/cs-etm-decoder/cs-etm-decoder.h |   4 +-
 tools/perf/util/cs-etm.c                      | 220 +++++++++---------
 tools/perf/util/cs-etm.h                      |   5 +-
 4 files changed, 126 insertions(+), 116 deletions(-)

-- 
2.34.1

Em Wed, May 24, 2023 at 02:19:54PM +0100, James Clark escreveu:
> Some fixes to support an issue reported by Denis Nikitin where decoding
> trace that contains different EL1 and EL2 kernels can crash or go into
> an infinite loop because the wrong kernel maps are used for the decode.
> 
> This still doesn't support distinguishing guest and host userspace,
> we'd still have to fix the timestamps and do a bit more work to
> correlate that. And I've removed PERF_RECORD_MISC_HYPERVISOR as a
> possible outcome of cs_etm__cpu_mode(). As far as I know this could
> never have been returned anyway because machine__is_host(machine) was
> always true due to session.machines.host being hard coded. And I'm not
> sure of the relevance of the difference between PERF_RECORD_MISC_KERNEL
> and PERF_RECORD_MISC_HYPERVISOR in this scenario.
> 
> The first commit is a tidy up, second fixes a bug that I found when
> comparing the exception level and thread of branch records, the third
> is the main fix, and the last commit is some extra error checking. 
> 
> Applies to acme/perf-tools (4e111f0cf0)

So there seems to be agreement the first two patches can be applied? May
I go ahead and do that now?

- Arnaldo
 
> James Clark (4):
>   perf cs-etm: Only track threads instead of PID and TIDs
>   perf cs-etm: Use previous thread for branch sample source IP
>   perf cs-etm: Track exception level
>   perf cs-etm: Add exception level consistency check
> 
>  .../perf/util/cs-etm-decoder/cs-etm-decoder.c |  13 +-
>  .../perf/util/cs-etm-decoder/cs-etm-decoder.h |   4 +-
>  tools/perf/util/cs-etm.c                      | 220 +++++++++---------
>  tools/perf/util/cs-etm.h                      |   5 +-
>  4 files changed, 126 insertions(+), 116 deletions(-)
> 
> -- 
> 2.34.1
> 

-- 

- Arnaldo

Hi Arnaldo,

On Mon, Jun 05, 2023 at 04:44:45PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 24, 2023 at 02:19:54PM +0100, James Clark escreveu:
> > Some fixes to support an issue reported by Denis Nikitin where decoding
> > trace that contains different EL1 and EL2 kernels can crash or go into
> > an infinite loop because the wrong kernel maps are used for the decode.
> > 
> > This still doesn't support distinguishing guest and host userspace,
> > we'd still have to fix the timestamps and do a bit more work to
> > correlate that. And I've removed PERF_RECORD_MISC_HYPERVISOR as a
> > possible outcome of cs_etm__cpu_mode(). As far as I know this could
> > never have been returned anyway because machine__is_host(machine) was
> > always true due to session.machines.host being hard coded. And I'm not
> > sure of the relevance of the difference between PERF_RECORD_MISC_KERNEL
> > and PERF_RECORD_MISC_HYPERVISOR in this scenario.
> > 
> > The first commit is a tidy up, second fixes a bug that I found when
> > comparing the exception level and thread of branch records, the third
> > is the main fix, and the last commit is some extra error checking. 
> > 
> > Applies to acme/perf-tools (4e111f0cf0)
> 
> So there seems to be agreement the first two patches can be applied? May
> I go ahead and do that now?

Could you pick up the first patch in this series?

I would like ask James to refine a bit for the second patch.

Thanks,
Leo

Em Tue, Jun 06, 2023 at 08:46:48AM +0800, Leo Yan escreveu:
> Hi Arnaldo,
> 
> On Mon, Jun 05, 2023 at 04:44:45PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, May 24, 2023 at 02:19:54PM +0100, James Clark escreveu:
> > > Some fixes to support an issue reported by Denis Nikitin where decoding
> > > trace that contains different EL1 and EL2 kernels can crash or go into
> > > an infinite loop because the wrong kernel maps are used for the decode.
> > > 
> > > This still doesn't support distinguishing guest and host userspace,
> > > we'd still have to fix the timestamps and do a bit more work to
> > > correlate that. And I've removed PERF_RECORD_MISC_HYPERVISOR as a
> > > possible outcome of cs_etm__cpu_mode(). As far as I know this could
> > > never have been returned anyway because machine__is_host(machine) was
> > > always true due to session.machines.host being hard coded. And I'm not
> > > sure of the relevance of the difference between PERF_RECORD_MISC_KERNEL
> > > and PERF_RECORD_MISC_HYPERVISOR in this scenario.
> > > 
> > > The first commit is a tidy up, second fixes a bug that I found when
> > > comparing the exception level and thread of branch records, the third
> > > is the main fix, and the last commit is some extra error checking. 
> > > 
> > > Applies to acme/perf-tools (4e111f0cf0)
> > 
> > So there seems to be agreement the first two patches can be applied? May
> > I go ahead and do that now?
> 
> Could you pick up the first patch in this series?
> 
> I would like ask James to refine a bit for the second patch.

Ok, left just the first patch on my local perf-tools-next branch, will
go public when tested.

- Arnaldo