[Intel-gfx] uABI / Removing DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig

[Steven Rostedt]

On Wed, 19 Dec 2018 12:08:18 +0200
Joonas Lahtinen <joonas.lahtinen at linux.intel.com> wrote:

> To me, it seems almost as if folks are too preoccupied with thinking if
> we somehow can do this through tracepoints, to stop and actually think
> if we should.

Regardless of whether it should or shouldn't, one solution to this is
to make the trace event in question record basically nothing but a
pointer.

DECLARE_EVENT_CLASS(i915_hw_request,
	    TP_PROTO(struct i915_request *rq),
	    TP_ARGS(rq),

	    TP_STRUCT__entry(
				__field(void *, rq)
			     ),

	    TP_fast_assign(
			__entry->rq = rq;
			   ),

	    TP_printk("rq=%p", __entry->rq)
);

Define the events from that:

DEFINE_EVENT(i915_hw_request, i915_request_submit,
	     TP_PROTO(struct i915_request *rq),
	     TP_ARGS(rq)
);


No tool can use that information. But for those that want to, you make a
separate module that you can load that has:

DECLARE_EVENT_CLASS(i915_specific_hw_request,
	    TP_PROTO(struct i915_request *rq),
	    TP_ARGS(rq),

	    TP_STRUCT__entry(
			     __field(u32, dev)
			     __field(u32, hw_id)
			     __field(u64, ctx)
			     __field(u16, class)
			     __field(u16, instance)
			     __field(u32, seqno)
			     __field(u32, global)
			     ),

	    TP_fast_assign(
			   __entry->dev = rq->i915->drm.primary->index;
			   __entry->hw_id = rq->gem_context->hw_id;
			   __entry->class = rq->engine->uabi_class;
			   __entry->instance = rq->engine->instance;
			   __entry->ctx = rq->fence.context;
			   __entry->seqno = rq->fence.seqno;
			   __entry->global = rq->global_seqno;
			   ),

	    TP_printk("dev=%u, engine=%u:%u, hw_id=%u, ctx=%llu, seqno=%u, global=%u",
		      __entry->dev, __entry->class, __entry->instance,
		      __entry->hw_id, __entry->ctx, __entry->seqno,
		      __entry->global)
);

DEFINE_EVENT_FN(i915_specific_hw_request, i915_specific_request_submit,
	     TP_PROTO(struct i915_request *rq),
	     TP_ARGS(rq),
	     start_i915_request, stop_i915_request)
);


In the module:

static void do_i915_hw_request(void *data, struct i915_request *rq)
{
	trace_i915_specific_request_submit(rq);
}

static bool i915_trace_enabled;

static void enable_disable_tracepoints(struct work_struct *work)
{
	if (i915_trace_enabled)
		/* Can not enable tracepoints from a tracepoint callback */
		register_trace_i915_hw_request(do_i915_hw_request, NULL);
	else
		unregister_trace_i915_hw_request(do_i915_hw_request, NULL);
}

int start_i915_request(void)
{
	i915_trace_enabled = true;
	schedule_work(work_call_enable_disable_tracepoints);
}

int stop_i915_request(void)
{
	i915_trace_enabled = false;
	schedule_work(work_call_enable_disable_tracepoints);
}


Note, the above can be racy, but I'm just trying to show a possible
solution. Which is to have the tracepoint there but exposing no useful
data (event the pointer shown will be mangled by the KASLR security).
But if someone wants a tool, they can create a new tracepoint that is
called by a handler of this tracepoint (yes its a double tracepoint).

The above work, handle would enable/disable the upstream tracepoint
when the other is enabled. Or that may not even be needed, just have
the tool enable both tracepoints when it wants the second one.

-- Steve