Re: [RFC] mmiotrace full patch, preview 2

[Pekka Paalanen <pq@iki.fi>]

On Fri, 28 Mar 2008 00:13:48 +0100
"Vegard Nossum" <vegard.nossum@gmail.com> wrote:

> I may of course be wrong, but... Shouldn't the post_kmmio_handler(),
> called from the die notifier chain, check for the DR_STEP condition?
> This makes sure that the function is not called in the cases where the
> source of the debug exception was not a single-stepping event. Though
> I guess you'll also have other checks in place to notice that the
> interrupt was not the one you were expecting. I guess a little extra
> safety won't hurt though?
> 
> On Sun, Mar 9, 2008 at 3:40 PM, Pekka Paalanen <pq@iki.fi> wrote:
> >  +/*
> >  + * Interrupts are disabled on entry as trap1 is an interrupt gate
> >  + * and they remain disabled thorough out this function.
> >  + * This must always get called as the pair to kmmio_handler().
> >  + */
> >  +static int post_kmmio_handler(unsigned long condition, struct pt_regs *regs)
> >  +{
> >  +       int ret = 0;
> >  +       struct kmmio_context *ctx = &get_cpu_var(kmmio_ctx);
> 
> if (!(condition & DR_STEP))
>     return;

I guess that would be appropriate. I think it should go into this
function:

+static int kmmio_die_notifier(struct notifier_block *nb, unsigned long val,
+								void *args)
+{
+	struct die_args *arg = args;
+
+	if (val == DIE_DEBUG)
+		if (post_kmmio_handler(arg->err, arg->regs) == 1)
+			return NOTIFY_STOP;
+
+	return NOTIFY_DONE;
+}

On the other hand I am thinking of not using the die notifier chain
at all and adding a direct call from do_debug() or something.
This is the last dynamic hook remaining from the out-of-tree module
era of mmiotrace.

I guess with the notifier list there is a possibility that another
module intercepts my single step trap, so that this is never called,
which would leave mmiotrace half blind, and also trigger a recursive
probe hit.

btw. what if someone uses kmemcheck and mmiotrace at the same time?
Mmiotrace will not fiddle with any other pages than returned via
__ioremap(), but can kmemcheck "hide" the same mmio pages?
Also keeping in mind, that some day I'd like to make mmiotrace able
to catch mmio accesses originating in user space.


Thanks.

-- 
Pekka Paalanen
http://www.iki.fi/pq/