On Wed, 20 Sep 2017 17:12:58 +0100 Mark Brown broonie@kernel.org wrote:
On Wed, Sep 20, 2017 at 04:49:44PM +0100, Kim Phillips wrote:
Tor Jeremiassen tor@ti.com wrote:
project [4]. Instead, this patch set includes the necessary code and build settings to interfaces to the decoder library, as well as a "stub" or
"null"
library for the case when the perf tool is built without the library.
I seriously doubt this would be acceptable upstream: they prefer to have all code fully-inclusive. Do we have a plan for somehow upstreaming the library, or some other means for working around this restriction?
I can't see why people would insist on having the library be upstreamed, it's not something that exists solely for the kernel. CoreSight is something that any ARM system can use if it's got appropriate hardware built in, the goal is for people to be able to share the decoding code between all kinds of tools running on many OSs. perf is one such application but far from the only one, and it's still usable without the trace decode.
For record, sure, but this series is about report AFAICT (I still can't tell where it applies cleanly).
If it helps to clarify my position, I'm not saying the ETM trace decoder / OpenCSD library / master branch should necessarily be converted to move *away* from where it is, and live *solely* in the kernel tree's perf tool sources: I'm asking what if the maintainers didn't want to have to depend on external libraries for Coresight report support.
If there were no reasonable possibility of the library being used outside of the kernel or it were so fundamental to building kernels that it were essential then it'd be possible people would feel a need to duplicate it, but even then we'd probably do as we do with dtc and have a copy of the code rather than first class kernel code. As it is it's an optional feature and seems closer to things like some of the kselftests which happily use external libraries installed on the system.
I don't know how optional or not Coresight report is - I'll leave that up to the upstream maintainers, but, I will say that perf report with Intel PT input currently runs on Arm perf binaries, and there is no option to opt-out of it, so the upstream maintainers, sure, whilst being a little Intel-centric, nevertheless made the decision that any perf binary should be able to decode a perf.data file from another arch.
FWIW, Intel PT also has an independent decoder project:
https://github.com/01org/processor-trace
Upon quick perusal, it bears almost no common code with the Intel PT decoder present in the perf tool upstream source (tools/perf/util/intel-pt-decoder/). I'm guessing the upstream perf decoder was done separately to better interleave with the perf buffer handling and event generation callbacks? Not sure, but it's evidence two versions of a single common h/w trace decoder exist IRL.
That same project is also available as a package on my distro:
libipt-dev/zesty 1.5-1ubuntu1 amd64 Intel Processor Trace Decoder Library -- development files
Yet it still didn't qualify to be an optional perf tool dependency: the string 'libipt' doesn't occur in the kernel source tree.
I still think this conversation will conclude once the upstream perf tool maintainers are consulted, as it's ultimately their decision.
Kim