Hi all,
this is the third full mmiotrace patch set for review (a bit late, sorry).

These patches do not apply to any git tree that I know of, but after Ingo

pushes a new version of his sched-devel/latest tree, these patches should

be subtractable from there.
I created these three patches by reorganizing patches in the

sched-devel/latest tree to get all the mmiotrace patches to the top,

and the end result is identical to the original, though I had to solve

some conflicts along the way. I included my last 5+1 patches, too.
This first patch adds the bulk of the new code, only introducing new files.

The second patch establishes the interfaces to the ftrace framework, and

the third patch hooks everything up into the Kbuild system, page fault

handler, and ioremap functions.
All three patches went through checkpatch.pl, getting complaints only

about the use of 'volatile', which is used to duplicate the parameter

signature of iounmap() for the functions mmiotrace_iounmap() and

iounmap_trace_core() in the first patch.
New development since preview 2, March 9th:

- Abandoning the binary user space interface that used relay, and

- moving to ftrace framework interfaces, which

- produces the trace log in text format rather than binary, and

- made the special user space logging program obsolete.

- lots of cleanups, e.g. reference_kmmio() removed

- refactor dis/arm_kmmio_fault_page() and add check for page levels

- work around recursive probe hits on SMP, trying to survive

- added user documentation

- If HOTPLUG_CPU, disable all but one CPU when mmiotrace is activated,

- and also restore the CPUs when mmiotrace is deactivated.

- detect and print buffer overrun counts (losing events)
Some of the new code depends on the new features in the ftrace framework,

many thanks to Steven for those.
The current state is that mmiotrace seems to work fine, also on SMP,

but on SMP there's a chance to miss events due to CPUs racing. At last

the log produced via ftrace framework is up-to-spec. Inser...
[ message continues ]

On Mon, 28 Apr 2008 21:45:04 +0300
Ok, looks like I won't be able to do more testing than what I did today.

The sched-devel/latest tree has changes that break the out-of-tree DRM,

so I cannot test with Nouveau, and most likely the proprietary driver

will not work either. A very nice test would be to enable mmiotracing

as early in boot as convinient, but I don't know how to achieve that

with the ftrace framework.
I did notice two bugs in mmiotrace while looking at ioremap.c, so

I will follow here with three patches.
Tested with the patches and sched-devel/latest of

Tue, 29 Apr 2008 11:47:55 +0000

using testmmiotrace.ko and things looked fine.
Thanks.
--

Pekka Paalanen

http://www.iki.fi/pq/

--

On Tue, 29 Apr 2008 23:40:21 +0300
Hi,
I did get the nvidia blob to work, with some help, and tried mmiotrace

on it. Worked perfectly. The machine is Thinkpad T61 with a Core 2 Duo.

Mmiotrace disabled and enabled the extra core without a glitch, and did

not even drop any events with the default ftrace buffer size. Previously

while tracing Nouveau, I had to increase the buffer size, which worked

well, too.
I did a second trace run after doing

echo 1 > /debug/tracing/trace_entries

and as expected, it lost a lot of events, but showed no problems. Then

I disabled mmiotrace while in X, the 2nd cpu core came back online, and

everything was good. Losing events is logged into the kernel log once

per mmiotrace activation, and into the trace whenever noticed.
(On athlon64 3000+, x86_64:)

Earlier I tried unaligned accesses with testmmiotrace.ko, by mapping

from an odd start address and doing 8, 16 and 32 bit reads and writes.

There were no crashes, and at a page boundary it resulted in a recursive

probe hit, which was resolved by disarming the both pages. Only one page

is re-armed, so the other page is left "blind", but did not seem to

harm stability.
Conclusion: green light from me!
--

Pekka Paalanen

http://www.iki.fi/pq/

--

From ecd0591bcefb2bbd934de25cadd0f7cb5dd2b7e1 Mon Sep 17 00:00:00 2001

From: Pekka Paalanen <pq@iki.fi>

Date: Tue, 29 Apr 2008 22:56:26 +0300

Subject: [PATCH] mmiotrace: rename kmmio_probe::user_data to :private.
Signed-off-by: Pekka Paalanen <pq@iki.fi>

---

 arch/x86/mm/mmio-mod.c    |    4 ++--

 include/linux/mmiotrace.h |    2 +-

 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c

index 3b04a01..ed0e0e9 100644

--- a/arch/x86/mm/mmio-mod.c

+++ b/arch/x86/mm/mmio-mod.c

@@ -191,7 +191,7 @@ static void pre(struct kmmio_probe *p, struct pt_regs *regs,

 	struct mmiotrace_rw *my_trace = &get_cpu_var(cpu_trace);

 	const unsigned long instptr = instruction_pointer(regs);

 	const enum reason_type type = get_ins_type(instptr);

-	struct remap_trace *trace = p->user_data;

+	struct remap_trace *trace = p->private;
 	/* it doesn't make sense to have more than one active trace per cpu */

 	if (my_reason->active_traces)

@@ -299,7 +299,7 @@ static void ioremap_trace_core(resource_size_t offset, unsigned long size,

 			.len = size,

 			.pre_handler = pre,

 			.post_handler = post,

-			.user_data = trace

+			.private = trace

 		},

 		.phys = offset,

 		.id = atomic_inc_return(&next_id)

diff --git a/include/linux/mmiotrace.h b/include/linux/mmiotrace.h

index 5cbbc37..61d19e1 100644

--- a/include/linux/mmiotrace.h

+++ b/include/linux/mmiotrace.h

@@ -18,7 +18,7 @@ struct kmmio_probe {

 	unsigned long len; /* length of the probe region */

 	kmmio_pre_handler_t pre_handler; /* Called before addr is executed. */

 	kmmio_post_handler_t post_handler; /* Called after addr is executed */

-	void *user_data;

+	void *private;

 };
 /* kmmio is active by some kmmio_probes? */

--

1.5.3.7
--

thanks Pekka, applied all 3 patches.
	Ingo

--

From 8261c3eeead01739d18875797816637621d17c2d Mon Sep 17 00:00:00 2001

From: Pekka Paalanen <pq@iki.fi>

Date: Thu, 1 May 2008 16:15:11 +0300

Subject: [PATCH] x86 mmiotrace: page level is unsigned
Fixes some sparse warnings.
Signed-off-by: Pekka Paalanen <pq@iki.fi>

---
There are three other sparse warnings, too, but two are false

positives and one is a code style issue. I do not know how to annotate

those away, or if it is even possible, so I will probably write a

proper question some time after the merge window closes about how to

deal with those.
 arch/x86/mm/kmmio.c    |   13 +++++++------

 arch/x86/mm/mmio-mod.c |    2 +-

 2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/arch/x86/mm/kmmio.c b/arch/x86/mm/kmmio.c

index c12be07..93d8203 100644

--- a/arch/x86/mm/kmmio.c

+++ b/arch/x86/mm/kmmio.c

@@ -105,11 +105,12 @@ static struct kmmio_fault_page *get_kmmio_fault_page(unsigned long page)

 	return NULL;

 }
-static void set_page_present(unsigned long addr, bool present, int *pglevel)

+static void set_page_present(unsigned long addr, bool present,

+							unsigned int *pglevel)

 {

 	pteval_t pteval;

 	pmdval_t pmdval;

-	int level;

+	unsigned int level;

 	pmd_t *pmd;

 	pte_t *pte = lookup_address(addr, &level);
@@ -146,15 +147,15 @@ static void set_page_present(unsigned long addr, bool present, int *pglevel)

 }
 /** Mark the given page as not present. Access to it will trigger a fault. */

-static void arm_kmmio_fault_page(unsigned long page, int *page_level)

+static void arm_kmmio_fault_page(unsigned long page, unsigned int *pglevel)

 {

-	set_page_present(page & PAGE_MASK, false, page_level);

+	set_page_present(page & PAGE_MASK, false, pglevel);

 }
 /** Mark the given page as present. */

-static void disarm_kmmio_fault_page(unsigned long page, int *page_level)

+static void disarm_kmmio_fault_page(unsigned long page, unsigned int *pglevel)

 {

-	set_page_present(page & PAGE_MASK, true, page_level);

+	set_page_...
[ message continues ]

From 921daf84218f4d2b35007f7d867f3301bb59173d Mon Sep 17 00:00:00 2001

From: Pekka Paalanen <pq@iki.fi>

Date: Tue, 29 Apr 2008 22:52:31 +0300

Subject: [PATCH] x86 mmiotrace: use resource_size_t for phys addresses
Signed-off-by: Pekka Paalanen <pq@iki.fi>

---

 arch/x86/mm/mmio-mod.c         |   11 ++++++-----

 include/linux/mmiotrace.h      |   14 +++++++-------

 kernel/trace/trace_mmiotrace.c |   20 ++++++++++++--------

 3 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/arch/x86/mm/mmio-mod.c b/arch/x86/mm/mmio-mod.c

index 278998c..3b04a01 100644

--- a/arch/x86/mm/mmio-mod.c

+++ b/arch/x86/mm/mmio-mod.c

@@ -48,7 +48,7 @@ struct trap_reason {

 struct remap_trace {

 	struct list_head list;

 	struct kmmio_probe probe;

-	unsigned long phys;

+	resource_size_t phys;

 	unsigned long id;

 };
@@ -275,7 +275,7 @@ static void post(struct kmmio_probe *p, unsigned long condition,

 	put_cpu_var(pf_reason);

 }
-static void ioremap_trace_core(unsigned long offset, unsigned long size,

+static void ioremap_trace_core(resource_size_t offset, unsigned long size,

 							void __iomem *addr)

 {

 	static atomic_t next_id;

@@ -319,13 +319,14 @@ not_enabled:

 	spin_unlock_irq(&trace_lock);

 }
-void

-mmiotrace_ioremap(unsigned long offset, unsigned long size, void __iomem *addr)

+void mmiotrace_ioremap(resource_size_t offset, unsigned long size,

+						void __iomem *addr)

 {

 	if (!is_enabled()) /* recheck and proper locking in *_core() */

 		return;
-	pr_debug(NAME "ioremap_*(0x%lx, 0x%lx) = %p\n", offset, size, addr);

+	pr_debug(NAME "ioremap_*(0x%llx, 0x%lx) = %p\n",

+				(unsigned long long)offset, size, addr);

 	if ((filter_offset) && (offset != filter_offset))

 		return;

 	ioremap_trace_core(offset, size, addr);

diff --git a/include/linux/mmiotrace.h b/include/linux/mmiotrace.h

index de8e912..5cbbc37 100644

--- a/include/linux/mmiotrace.h

+++ b/include/linux/mmiotrace.h

@@ -2,7 +2,6 @@

 #define MMIOTRACE_H
 #include <linux...
[ message continues ]

From c306ab1bc1f40e74f7015a498784e1a39ec6229b Mon Sep 17 00:00:00 2001

From: Pekka Paalanen <pq@iki.fi>

Date: Tue, 29 Apr 2008 22:39:40 +0300

Subject: [PATCH] x86 mmiotrace: fix page-unaligned ioremaps
mmiotrace_ioremap() expects to receive the original unaligned map phys address

and size. Also fix {un,}register_kmmio_probe() to deal properly with

unaligned size.
Signed-off-by: Pekka Paalanen <pq@iki.fi>

---

 arch/x86/mm/ioremap.c  |    4 +++-

 arch/x86/mm/kmmio.c    |   13 +++++++++++--

 arch/x86/mm/mmio-mod.c |    1 +

 3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c

index ce73980..923621d 100644

--- a/arch/x86/mm/ioremap.c

+++ b/arch/x86/mm/ioremap.c

@@ -123,6 +123,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,

 {

 	unsigned long pfn, offset, vaddr;

 	resource_size_t last_addr;

+	const resource_size_t unaligned_phys_addr = phys_addr;

+	const unsigned long unaligned_size = size;

 	struct vm_struct *area;

 	unsigned long new_prot_val;

 	pgprot_t prot;

@@ -235,7 +237,7 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,

 	}
 	ret_addr = (void __iomem *) (vaddr + offset);

-	mmiotrace_ioremap(phys_addr, size, ret_addr);

+	mmiotrace_ioremap(unaligned_phys_addr, unaligned_size, ret_addr);
 	return ret_addr;

 }

diff --git a/arch/x86/mm/kmmio.c b/arch/x86/mm/kmmio.c

index 40567d8..c12be07 100644

--- a/arch/x86/mm/kmmio.c

+++ b/arch/x86/mm/kmmio.c

@@ -352,11 +352,19 @@ static void release_kmmio_fault_page(unsigned long page,

 	}

 }
+/*

+ * With page-unaligned ioremaps, one or two armed pages may contain

+ * addresses from outside the intended mapping. Events for these addresses

+ * are currently silently dropped. The events may result only from programming

+ * mistakes by accessing addresses before the beginning or past the end of a

+ * mapping.

+ */

 int register_kmmio_probe(struct kmmio_probe *p)

 {

 	unsigned long flags;

 	int ret = 0;

 	unsig...
[ message continues ]

might not be able to release a new sched-devel.git/x86.git tonight - had

to filter out lots of badness today from new patches and i dont trust

the quality of the current lot yet. (nothing mmiotrace related for that

matter)
	Ingo

--

On Mon, 28 Apr 2008 21:45:04 +0300
 arch/x86/Kconfig.debug |   28 ++++++++++++++++++++++++++++

 arch/x86/mm/Makefile   |    5 +++++

 arch/x86/mm/fault.c    |   13 +++++++++++++

 arch/x86/mm/ioremap.c  |    9 ++++++++-

 arch/x86/mm/pageattr.c |    1 +

 5 files changed, 55 insertions(+), 1 deletions(-)
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug

index 1f237d2..fb0794e 100644

--- a/arch/x86/Kconfig.debug

+++ b/arch/x86/Kconfig.debug

@@ -170,6 +170,34 @@ config IOMMU_LEAK

 	  Add a simple leak tracer to the IOMMU code. This is useful when you

 	  are debugging a buggy device driver that leaks IOMMU mappings.
+config MMIOTRACE_HOOKS

+	bool

+

+config MMIOTRACE

+	bool "Memory mapped IO tracing"

+	depends on DEBUG_KERNEL && PCI

+	select TRACING

+	select MMIOTRACE_HOOKS

+	default y

+	help

+	  Mmiotrace traces Memory Mapped I/O access and is meant for

+	  debugging and reverse engineering. It is called from the ioremap

+	  implementation and works via page faults. Tracing is disabled by

+	  default and can be enabled at run-time.

+

+	  See Documentation/tracers/mmiotrace.txt.

+	  If you are not helping to develop drivers, say N.

+

+config MMIOTRACE_TEST

+	tristate "Test module for mmiotrace"

+	depends on MMIOTRACE && m

+	help

+	  This is a dumb module for testing mmiotrace. It is very dangerous

+	  as it will write garbage to IO memory starting at a given address.

+	  However, it should be safe to use on e.g. unused portion of VRAM.

+

+	  Say N, unless you absolutely know what you are doing.

+

 #

 # IO delay types:

 #

diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile

index b7b3e4c..07dab50 100644

--- a/arch/x86/mm/Makefile

+++ b/arch/x86/mm/Makefile

@@ -8,6 +8,11 @@ obj-$(CONFIG_X86_PTDUMP)	+= dump_pagetables.o
 obj-$(CONFIG_HIGHMEM)		+= highmem_32.o
+obj-$(CONFIG_MMIOTRACE_HOOKS)	+= kmmio.o

+obj-$(CONFIG_MMIOTRACE)		+= mmiotrace.o

+mmiotrace-y			:= pf_in.o mmio-mod.o

+obj-$(CONFIG_MMIOTRACE_TEST)	+= testmmiotrace.o

...
[ message continues ]

Whoops, sorry, this is of course titled RFC 3/3.
On Mon, 28 Apr 2008 21:58:54 +0300
--

Pekka Paalanen

http://www.iki.fi/pq/

--

On Mon, 28 Apr 2008 21:45:04 +0300
 kernel/trace/Makefile          |    1 +

 kernel/trace/trace.c           |   66 +++++++---

 kernel/trace/trace.h           |   29 ++++

 kernel/trace/trace_mmiotrace.c |  291 ++++++++++++++++++++++++++++++++++++++++

 4 files changed, 370 insertions(+), 17 deletions(-)
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile

index 7aec123..71d17de 100644

--- a/kernel/trace/Makefile

+++ b/kernel/trace/Makefile

@@ -19,5 +19,6 @@ obj-$(CONFIG_FTRACE) += trace_functions.o

 obj-$(CONFIG_IRQSOFF_TRACER) += trace_irqsoff.o

 obj-$(CONFIG_PREEMPT_TRACER) += trace_irqsoff.o

 obj-$(CONFIG_SCHED_TRACER) += trace_sched_wakeup.o

+obj-$(CONFIG_MMIOTRACE) += trace_mmiotrace.o
 libftrace-y := ftrace.o

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c

index cbe476f..06c85b3 100644

--- a/kernel/trace/trace.c

+++ b/kernel/trace/trace.c

@@ -54,7 +54,7 @@ static int tracing_disabled = 1;
 static unsigned long tracing_pages_allocated;
-static long

+long

 ns2usecs(cycle_t nsec)

 {

 	nsec += 500;

@@ -173,18 +173,6 @@ unsigned long nsecs_to_usecs(unsigned long nsecs)

 	return nsecs / 1000;

 }
-enum trace_type {

-	__TRACE_FIRST_TYPE = 0,

-

-	TRACE_FN,

-	TRACE_CTX,

-	TRACE_WAKE,

-	TRACE_STACK,

-	TRACE_SPECIAL,

-

-	__TRACE_LAST_TYPE

-};

-

 /*

  * trace_flag_type is an enumeration that holds different

  * states when a trace occurs. These are:

@@ -313,7 +301,7 @@ void *head_page(struct trace_array_cpu *data)

  * buffer (@s). Then the output may be either used by

  * the sequencer or pulled into another buffer.

  */

-static int

+int

 trace_seq_printf(struct trace_seq *s, const char *fmt, ...)

 {

 	int len = (PAGE_SIZE - 1) - s->len;

@@ -328,7 +316,7 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)

 	va_end(ap);
 	/* If we can't write it all, don't bother writing anything */

-	if (ret > len)

+	if (ret >= len)

 		return 0;
 	s->len += ret;

@@ -809,7 +797,7 @@ tracing_generic_entry_update(struct tr...
[ message continues ]