linux-kernel - Re: [PATCH] coresight: docs: Remove target sink from examples

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250311135704.GK9682@e132581.arm.com>
Date: Tue, 11 Mar 2025 13:57:04 +0000
From: Leo Yan <leo.yan@....com>
To: James Clark <james.clark@...aro.org>
Cc: coresight@...ts.linaro.org, yeoreum.yun@....com,
	Mike Leach <mike.leach@...aro.org>,
	Jonathan Corbet <corbet@....net>, Leo Yan <leo.yan@...ux.dev>,
	Namhyung Kim <namhyung@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] coresight: docs: Remove target sink from examples

On Tue, Mar 11, 2025 at 10:51:39AM +0000, James Clark wrote:

[...]

> > > +Sink selection
> > > +~~~~~~~~~~~~~~
> > > +
> > > +An appropriate sink will be selected automatically for use with Perf, but since
> > > +there will typically be more than one sink, the name of the sink to use may be
> > > +specified as a special config option prefixed with '@'.
> > > +
> > > +The available sinks are listed in sysFS under
> > >   ($SYSFS)/bus/event_source/devices/cs_etm/sinks/::
> > > 
> > >          root@...alhost:/sys/bus/event_source/devices/cs_etm/sinks# ls
> > >          tmc_etf0  tmc_etr0  tpiu0
> > 
> > Just a minor comment.  To reflect the latest hardware, it is good to
> > mention the TRBE case, users should not and cannot specify TRBE as the
> 
> Is that strictly true? From looking at the code I think you could pick one
> TRBE sink as long as you are only tracing from a single ETM. Although yeah
> it would be pointless.

My testing result shows perf reports error when specifying trbe as sink:

  # perf record -e cs_etm/@...e0/ -- ls
  failed to mmap with 12 (Cannot allocate memory)

But I can make success for the command:

  # perf record -C 0 -e cs_etm/@...e0/ -- ls
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.062 MB perf.data ]

This makes sense to me that if a perf session tries to enable trace for
multiple CPUs but only a CPU sink is supported, the driver should
report error to remind users the command does not work.

> > sink name.  The driver will give priority for TRBE by default unless
> > users specify other sink types.
> > 
> 
> IMO mentioning TRBE would be overly wordy and not really add anything.
> Removing the sink from all the base examples is exactly to make TRBE work
> without going into detail about why. And the advanced section doesn't
> mention TRBE because manually picking it is never right.

The section "Sink selection" applies _only_ to traditional sinks, not
to TRBE.  I am not sure if we should clarify a bit for this.  I don't
have strong opinion on it, as it is a trade-off between providing
necessary info and avoiding overstatement.

So this patch is fine for me:

Reviewed-by: Leo Yan <leo.yan@....com>