linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
@ 2017-11-27  7:51 Karol Herbst
  2017-12-12 10:55 ` [tip:x86/urgent] " tip-bot for Karol Herbst
  0 siblings, 1 reply; 10+ messages in thread
From: Karol Herbst @ 2017-11-27  7:51 UTC (permalink / raw)
  To: nouveau, linux-kernel
  Cc: Karol Herbst, Steven Rostedt, Ingo Molnar, Thomas Gleixner,
	Pekka Paalanen, x86

If something calls ioremap with an address not aligned to PAGE_SIZE, the
returned address might be not aligned as well. This led to a probe
registered on exactly the returned address, but the entire page was armed
for mmiotracing.

On calling iounmap the address passed to unregister_kmmio_probe was
PAGE_SIZE aligned by the caller leading to a complete freeze of the
machine.

We should always page align addresses while (un)registerung mappings,
because the mmiotracer works on top of pages, not mappings. We still keep
track of the probes based on their real addresses and lengths though,
because the mmiotrace still needs to know what are mapped memory regions.

Also move the call to mmiotrace_iounmap prior page aligning the address,
so that all probes are unregistered properly, otherwise the kernel ends up
failing memory allocations randomly after disabling the mmiotracer.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: nouveau@lists.freedesktop.org
Cc: x86@kernel.org
Acked-by: Pekka Paalanen <ppaalanen@gmail.com>
Tested-by: Lyude <lyude@redhat.com>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
---
 arch/x86/mm/ioremap.c |  4 ++--
 arch/x86/mm/kmmio.c   | 12 +++++++-----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 34f0e1847dd6..5d4c358778dd 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -349,11 +349,11 @@ void iounmap(volatile void __iomem *addr)
 		return;
 	}
 
+	mmiotrace_iounmap(addr);
+
 	addr = (volatile void __iomem *)
 		(PAGE_MASK & (unsigned long __force)addr);
 
-	mmiotrace_iounmap(addr);
-
 	/* Use the vm area unlocked, assuming the caller
 	   ensures there isn't another iounmap for the same address
 	   in parallel. Reuse of the virtual address is prevented by
diff --git a/arch/x86/mm/kmmio.c b/arch/x86/mm/kmmio.c
index c21c2ed04612..58477ec3d66d 100644
--- a/arch/x86/mm/kmmio.c
+++ b/arch/x86/mm/kmmio.c
@@ -435,17 +435,18 @@ int register_kmmio_probe(struct kmmio_probe *p)
 	unsigned long flags;
 	int ret = 0;
 	unsigned long size = 0;
+	unsigned long addr = p->addr & PAGE_MASK;
 	const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
 	unsigned int l;
 	pte_t *pte;
 
 	spin_lock_irqsave(&kmmio_lock, flags);
-	if (get_kmmio_probe(p->addr)) {
+	if (get_kmmio_probe(addr)) {
 		ret = -EEXIST;
 		goto out;
 	}
 
-	pte = lookup_address(p->addr, &l);
+	pte = lookup_address(addr, &l);
 	if (!pte) {
 		ret = -EINVAL;
 		goto out;
@@ -454,7 +455,7 @@ int register_kmmio_probe(struct kmmio_probe *p)
 	kmmio_count++;
 	list_add_rcu(&p->list, &kmmio_probes);
 	while (size < size_lim) {
-		if (add_kmmio_fault_page(p->addr + size))
+		if (add_kmmio_fault_page(addr + size))
 			pr_err("Unable to set page fault.\n");
 		size += page_level_size(l);
 	}
@@ -528,19 +529,20 @@ void unregister_kmmio_probe(struct kmmio_probe *p)
 {
 	unsigned long flags;
 	unsigned long size = 0;
+	unsigned long addr = p->addr & PAGE_MASK;
 	const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
 	struct kmmio_fault_page *release_list = NULL;
 	struct kmmio_delayed_release *drelease;
 	unsigned int l;
 	pte_t *pte;
 
-	pte = lookup_address(p->addr, &l);
+	pte = lookup_address(addr, &l);
 	if (!pte)
 		return;
 
 	spin_lock_irqsave(&kmmio_lock, flags);
 	while (size < size_lim) {
-		release_kmmio_fault_page(p->addr + size, &release_list);
+		release_kmmio_fault_page(addr + size, &release_list);
 		size += page_level_size(l);
 	}
 	list_del_rcu(&p->list);
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-11-27  7:51 [PATCH] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses Karol Herbst
@ 2017-12-12 10:55 ` tip-bot for Karol Herbst
  2017-12-12 13:49   ` Peter Zijlstra
  0 siblings, 1 reply; 10+ messages in thread
From: tip-bot for Karol Herbst @ 2017-12-12 10:55 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, torvalds, tglx, kherbst, lyude, ppaalanen, mingo, rostedt,
	peterz, linux-kernel

Commit-ID:  6d60ce384d1d5ca32b595244db4077a419acc687
Gitweb:     https://git.kernel.org/tip/6d60ce384d1d5ca32b595244db4077a419acc687
Author:     Karol Herbst <kherbst@redhat.com>
AuthorDate: Mon, 27 Nov 2017 08:51:39 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 11 Dec 2017 15:35:18 +0100

x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

If something calls ioremap() with an address not aligned to PAGE_SIZE, the
returned address might be not aligned as well. This led to a probe
registered on exactly the returned address, but the entire page was armed
for mmiotracing.

On calling iounmap() the address passed to unregister_kmmio_probe() was
PAGE_SIZE aligned by the caller leading to a complete freeze of the
machine.

We should always page align addresses while (un)registerung mappings,
because the mmiotracer works on top of pages, not mappings. We still keep
track of the probes based on their real addresses and lengths though,
because the mmiotrace still needs to know what are mapped memory regions.

Also move the call to mmiotrace_iounmap() prior page aligning the address,
so that all probes are unregistered properly, otherwise the kernel ends up
failing memory allocations randomly after disabling the mmiotracer.

Tested-by: Lyude <lyude@redhat.com>
Signed-off-by: Karol Herbst <kherbst@redhat.com>
Acked-by: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: nouveau@lists.freedesktop.org
Link: http://lkml.kernel.org/r/20171127075139.4928-1-kherbst@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/ioremap.c |  4 ++--
 arch/x86/mm/kmmio.c   | 12 +++++++-----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 6e4573b..c45b6ec 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -404,11 +404,11 @@ void iounmap(volatile void __iomem *addr)
 		return;
 	}
 
+	mmiotrace_iounmap(addr);
+
 	addr = (volatile void __iomem *)
 		(PAGE_MASK & (unsigned long __force)addr);
 
-	mmiotrace_iounmap(addr);
-
 	/* Use the vm area unlocked, assuming the caller
 	   ensures there isn't another iounmap for the same address
 	   in parallel. Reuse of the virtual address is prevented by
diff --git a/arch/x86/mm/kmmio.c b/arch/x86/mm/kmmio.c
index c21c2ed..58477ec 100644
--- a/arch/x86/mm/kmmio.c
+++ b/arch/x86/mm/kmmio.c
@@ -435,17 +435,18 @@ int register_kmmio_probe(struct kmmio_probe *p)
 	unsigned long flags;
 	int ret = 0;
 	unsigned long size = 0;
+	unsigned long addr = p->addr & PAGE_MASK;
 	const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
 	unsigned int l;
 	pte_t *pte;
 
 	spin_lock_irqsave(&kmmio_lock, flags);
-	if (get_kmmio_probe(p->addr)) {
+	if (get_kmmio_probe(addr)) {
 		ret = -EEXIST;
 		goto out;
 	}
 
-	pte = lookup_address(p->addr, &l);
+	pte = lookup_address(addr, &l);
 	if (!pte) {
 		ret = -EINVAL;
 		goto out;
@@ -454,7 +455,7 @@ int register_kmmio_probe(struct kmmio_probe *p)
 	kmmio_count++;
 	list_add_rcu(&p->list, &kmmio_probes);
 	while (size < size_lim) {
-		if (add_kmmio_fault_page(p->addr + size))
+		if (add_kmmio_fault_page(addr + size))
 			pr_err("Unable to set page fault.\n");
 		size += page_level_size(l);
 	}
@@ -528,19 +529,20 @@ void unregister_kmmio_probe(struct kmmio_probe *p)
 {
 	unsigned long flags;
 	unsigned long size = 0;
+	unsigned long addr = p->addr & PAGE_MASK;
 	const unsigned long size_lim = p->len + (p->addr & ~PAGE_MASK);
 	struct kmmio_fault_page *release_list = NULL;
 	struct kmmio_delayed_release *drelease;
 	unsigned int l;
 	pte_t *pte;
 
-	pte = lookup_address(p->addr, &l);
+	pte = lookup_address(addr, &l);
 	if (!pte)
 		return;
 
 	spin_lock_irqsave(&kmmio_lock, flags);
 	while (size < size_lim) {
-		release_kmmio_fault_page(p->addr + size, &release_list);
+		release_kmmio_fault_page(addr + size, &release_list);
 		size += page_level_size(l);
 	}
 	list_del_rcu(&p->list);

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 10:55 ` [tip:x86/urgent] " tip-bot for Karol Herbst
@ 2017-12-12 13:49   ` Peter Zijlstra
  2017-12-12 14:04     ` Ingo Molnar
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2017-12-12 13:49 UTC (permalink / raw)
  To: linux-kernel, ppaalanen, lyude, rostedt, mingo, tglx, kherbst,
	torvalds, hpa
  Cc: linux-tip-commits

On Tue, Dec 12, 2017 at 02:55:30AM -0800, tip-bot for Karol Herbst wrote:
> Commit-ID:  6d60ce384d1d5ca32b595244db4077a419acc687
> Gitweb:     https://git.kernel.org/tip/6d60ce384d1d5ca32b595244db4077a419acc687
> Author:     Karol Herbst <kherbst@redhat.com>
> AuthorDate: Mon, 27 Nov 2017 08:51:39 +0100
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Mon, 11 Dec 2017 15:35:18 +0100
> 
> x86/mm/kmmio: Fix mmiotrace for page unaligned addresses

OK, let me hijack this thread since apparently people use and care about
mmiotrace.

I was recently auditing the x86 tlb flushing and ran across this
'thing'. Can someone please explain to me how this is supposed to work
and how its not completely broken?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 13:49   ` Peter Zijlstra
@ 2017-12-12 14:04     ` Ingo Molnar
  2017-12-12 14:21       ` Ilia Mirkin
  2017-12-12 14:32       ` Karol Herbst
  0 siblings, 2 replies; 10+ messages in thread
From: Ingo Molnar @ 2017-12-12 14:04 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, ppaalanen, lyude, rostedt, tglx, kherbst, torvalds,
	hpa, linux-tip-commits, Karol Herbst


* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Dec 12, 2017 at 02:55:30AM -0800, tip-bot for Karol Herbst wrote:
> > Commit-ID:  6d60ce384d1d5ca32b595244db4077a419acc687
> > Gitweb:     https://git.kernel.org/tip/6d60ce384d1d5ca32b595244db4077a419acc687
> > Author:     Karol Herbst <kherbst@redhat.com>
> > AuthorDate: Mon, 27 Nov 2017 08:51:39 +0100
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Mon, 11 Dec 2017 15:35:18 +0100
> > 
> > x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
> 
> OK, let me hijack this thread since apparently people use and care about
> mmiotrace.
> 
> I was recently auditing the x86 tlb flushing and ran across this
> 'thing'. Can someone please explain to me how this is supposed to work
> and how its not completely broken?

(I have Cc:-ed other gents as well.)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 14:04     ` Ingo Molnar
@ 2017-12-12 14:21       ` Ilia Mirkin
  2017-12-12 14:43         ` Peter Zijlstra
  2017-12-12 14:32       ` Karol Herbst
  1 sibling, 1 reply; 10+ messages in thread
From: Ilia Mirkin @ 2017-12-12 14:21 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Peter Zijlstra, linux-kernel, Pekka Paalanen, Lyude,
	Steven Rostedt, Thomas Gleixner, Karol Herbst, Linus Torvalds,
	hpa, linux-tip-commits, Karol Herbst

On Tue, Dec 12, 2017 at 9:04 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
>> On Tue, Dec 12, 2017 at 02:55:30AM -0800, tip-bot for Karol Herbst wrote:
>> > Commit-ID:  6d60ce384d1d5ca32b595244db4077a419acc687
>> > Gitweb:     https://git.kernel.org/tip/6d60ce384d1d5ca32b595244db4077a419acc687
>> > Author:     Karol Herbst <kherbst@redhat.com>
>> > AuthorDate: Mon, 27 Nov 2017 08:51:39 +0100
>> > Committer:  Ingo Molnar <mingo@kernel.org>
>> > CommitDate: Mon, 11 Dec 2017 15:35:18 +0100
>> >
>> > x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
>>
>> OK, let me hijack this thread since apparently people use and care about
>> mmiotrace.
>>
>> I was recently auditing the x86 tlb flushing and ran across this
>> 'thing'. Can someone please explain to me how this is supposed to work
>> and how its not completely broken?

The "thing" being mmiotrace, or the "thing" being page-unaligned addresses?

If the former, its primary use-case is for snooping on the NVIDIA
proprietary GPU driver in order to figure out how to drive the
underlying hardware. The driver does ioremap's to get at PCI space,
which mmiotrace "steals" and provides pages without a present bit set,
along with a fault handler. When the fault handler is hit, it
reinstates the faulting page, and single-steps the faulting
instruction reading the before/after regs to determine what happened
(doesn't work universally, but enough for instructions used for PCI
MMIO accesses). See mmio-mod.c::pre and post (the latter is called
from the debug handler).

You may be interested in reading
Documentation/trace/mmiotrace.txt::How Mmiotrace Works

Cheers,

  -ilia

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 14:04     ` Ingo Molnar
  2017-12-12 14:21       ` Ilia Mirkin
@ 2017-12-12 14:32       ` Karol Herbst
  1 sibling, 0 replies; 10+ messages in thread
From: Karol Herbst @ 2017-12-12 14:32 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Peter Zijlstra, linux-kernel, Pekka Paalanen, Lyude Paul,
	Steven Rostedt, Thomas Gleixner, torvalds, hpa,
	linux-tip-commits, Karol Herbst

Hi Peter,

the basic idea is to detect if a driver accesses a memory region
mapped through ioremap. This is super usefull for reverse engineering
closed source drivers like the Nvidia GPU driver.

So here is what it does:
1. on ioremap the entire memory region mapped is registered in the
mmiotracer and marked as not presen, which basically leads to page
faults on acces
2. mmiotrace is the registered page fault handler for those pages and
while handling the page (which basically means marking them as presen,
because they were never missing in the first place) it parses the
current instruction to detect if it was a read or write and writes
relevant information into a file. This includes address accessed,
value read/written, type of instruction
3. after single stepping, the page is marked as not present again
4. on unmap time, mmiotrace unregisteres those regions and marks them as present

this is more or less the basic idea.

And to answer your question how it is not completely broken: I don't
know. It works for us (more or less, we can't parse repeat
instructions as one example what does not work) and if we come across
issues we try to fix them on the way.

Anyway, this is a super useful tool to record and debug what a driver
is doing with hardware and helps tracking down a lot of this,
especially for Nouveau.

I hope that helps.

On Tue, Dec 12, 2017 at 3:04 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
>> On Tue, Dec 12, 2017 at 02:55:30AM -0800, tip-bot for Karol Herbst wrote:
>> > Commit-ID:  6d60ce384d1d5ca32b595244db4077a419acc687
>> > Gitweb:     https://git.kernel.org/tip/6d60ce384d1d5ca32b595244db4077a419acc687
>> > Author:     Karol Herbst <kherbst@redhat.com>
>> > AuthorDate: Mon, 27 Nov 2017 08:51:39 +0100
>> > Committer:  Ingo Molnar <mingo@kernel.org>
>> > CommitDate: Mon, 11 Dec 2017 15:35:18 +0100
>> >
>> > x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
>>
>> OK, let me hijack this thread since apparently people use and care about
>> mmiotrace.
>>
>> I was recently auditing the x86 tlb flushing and ran across this
>> 'thing'. Can someone please explain to me how this is supposed to work
>> and how its not completely broken?
>
> (I have Cc:-ed other gents as well.)
>
> Thanks,
>
>         Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 14:21       ` Ilia Mirkin
@ 2017-12-12 14:43         ` Peter Zijlstra
  2017-12-12 14:47           ` Ilia Mirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2017-12-12 14:43 UTC (permalink / raw)
  To: Ilia Mirkin
  Cc: Ingo Molnar, linux-kernel, Pekka Paalanen, Lyude, Steven Rostedt,
	Thomas Gleixner, Karol Herbst, Linus Torvalds, hpa,
	linux-tip-commits, Karol Herbst

On Tue, Dec 12, 2017 at 09:21:10AM -0500, Ilia Mirkin wrote:
> The "thing" being mmiotrace, or the "thing" being page-unaligned addresses?

mmiotrace

> If the former, its primary use-case is for snooping on the NVIDIA
> proprietary GPU driver in order to figure out how to drive the
> underlying hardware. The driver does ioremap's to get at PCI space,
> which mmiotrace "steals" and provides pages without a present bit set,
> along with a fault handler. When the fault handler is hit, it
> reinstates the faulting page, and single-steps the faulting
> instruction

At which point you have valid page-tables and another CPU can access
that page too.

> reading the before/after regs to determine what happened
> (doesn't work universally, but enough for instructions used for PCI
> MMIO accesses). See mmio-mod.c::pre and post (the latter is called
> from the debug handler).

And after that you only invalidate the TLBs for the CPU that took the
initial fault, leaving possibly stale TLBs on other CPUs.


So this 'thing' has huge gaping SMP holes in.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 14:43         ` Peter Zijlstra
@ 2017-12-12 14:47           ` Ilia Mirkin
  2017-12-12 14:51             ` Peter Zijlstra
  0 siblings, 1 reply; 10+ messages in thread
From: Ilia Mirkin @ 2017-12-12 14:47 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, linux-kernel, Pekka Paalanen, Lyude, Steven Rostedt,
	Thomas Gleixner, Karol Herbst, Linus Torvalds, hpa,
	linux-tip-commits, Karol Herbst

On Tue, Dec 12, 2017 at 9:43 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Dec 12, 2017 at 09:21:10AM -0500, Ilia Mirkin wrote:
>> The "thing" being mmiotrace, or the "thing" being page-unaligned addresses?
>
> mmiotrace
>
>> If the former, its primary use-case is for snooping on the NVIDIA
>> proprietary GPU driver in order to figure out how to drive the
>> underlying hardware. The driver does ioremap's to get at PCI space,
>> which mmiotrace "steals" and provides pages without a present bit set,
>> along with a fault handler. When the fault handler is hit, it
>> reinstates the faulting page, and single-steps the faulting
>> instruction
>
> At which point you have valid page-tables and another CPU can access
> that page too.
>
>> reading the before/after regs to determine what happened
>> (doesn't work universally, but enough for instructions used for PCI
>> MMIO accesses). See mmio-mod.c::pre and post (the latter is called
>> from the debug handler).
>
> And after that you only invalidate the TLBs for the CPU that took the
> initial fault, leaving possibly stale TLBs on other CPUs.
>
>
> So this 'thing' has huge gaping SMP holes in.

Sure does! Probably why the following happens when mmiotrace is enabled:

void enable_mmiotrace(void)
{
        mutex_lock(&mmiotrace_mutex);
        if (is_enabled())
                goto out;

        if (nommiotrace)
                pr_info("MMIO tracing disabled.\n");
        kmmio_init();
        enter_uniprocessor();
        spin_lock_irq(&trace_lock);
        atomic_inc(&mmiotrace_enabled);
        spin_unlock_irq(&trace_lock);
        pr_info("enabled.\n");
out:
        mutex_unlock(&mmiotrace_mutex);
}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 14:47           ` Ilia Mirkin
@ 2017-12-12 14:51             ` Peter Zijlstra
  2017-12-13 16:31               ` Steven Rostedt
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2017-12-12 14:51 UTC (permalink / raw)
  To: Ilia Mirkin
  Cc: Ingo Molnar, linux-kernel, Pekka Paalanen, Lyude, Steven Rostedt,
	Thomas Gleixner, Karol Herbst, Linus Torvalds, hpa,
	linux-tip-commits, Karol Herbst

On Tue, Dec 12, 2017 at 09:47:05AM -0500, Ilia Mirkin wrote:
> > So this 'thing' has huge gaping SMP holes in.
> 
> Sure does! Probably why the following happens when mmiotrace is enabled:
> 
> void enable_mmiotrace(void)
> {
>         mutex_lock(&mmiotrace_mutex);
>         if (is_enabled())
>                 goto out;
> 
>         if (nommiotrace)
>                 pr_info("MMIO tracing disabled.\n");
>         kmmio_init();
>         enter_uniprocessor();

	^^^^^

Ah! I completely missed that. OK, that makes it much less broken :-)

If I don't forget, I'll add some comments to this file to clarify that.

Thanks!

>         spin_lock_irq(&trace_lock);
>         atomic_inc(&mmiotrace_enabled);
>         spin_unlock_irq(&trace_lock);
>         pr_info("enabled.\n");
> out:
>         mutex_unlock(&mmiotrace_mutex);
> }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tip:x86/urgent] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses
  2017-12-12 14:51             ` Peter Zijlstra
@ 2017-12-13 16:31               ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2017-12-13 16:31 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ilia Mirkin, Ingo Molnar, linux-kernel, Pekka Paalanen, Lyude,
	Thomas Gleixner, Karol Herbst, Linus Torvalds, hpa,
	linux-tip-commits, Karol Herbst

On Tue, 12 Dec 2017 15:51:05 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Dec 12, 2017 at 09:47:05AM -0500, Ilia Mirkin wrote:
> > > So this 'thing' has huge gaping SMP holes in.  
> > 
> > Sure does! Probably why the following happens when mmiotrace is enabled:
> > 
> > void enable_mmiotrace(void)
> > {
> >         mutex_lock(&mmiotrace_mutex);
> >         if (is_enabled())
> >                 goto out;
> > 
> >         if (nommiotrace)
> >                 pr_info("MMIO tracing disabled.\n");
> >         kmmio_init();
> >         enter_uniprocessor();  
> 
> 	^^^^^
> 
> Ah! I completely missed that. OK, that makes it much less broken :-)

/me just saw this thread.

Once I saw your initial complaint, my first thought was "I think he
doesn't know that it causes the system to turn into a uniprocessor
first".

My ftrace tests have a test that switches to each tracer, and this
mmiotrace catches the most bugs after an rc1 release. The bugs are
triggered by switching to and from uniprocessor mode. Hopefully, with
the new hotplug code, that will be a thing of the past.

> 
> If I don't forget, I'll add some comments to this file to clarify that.

Great, thanks!

-- Steve

> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-12-13 16:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27  7:51 [PATCH] x86/mm/kmmio: Fix mmiotrace for page unaligned addresses Karol Herbst
2017-12-12 10:55 ` [tip:x86/urgent] " tip-bot for Karol Herbst
2017-12-12 13:49   ` Peter Zijlstra
2017-12-12 14:04     ` Ingo Molnar
2017-12-12 14:21       ` Ilia Mirkin
2017-12-12 14:43         ` Peter Zijlstra
2017-12-12 14:47           ` Ilia Mirkin
2017-12-12 14:51             ` Peter Zijlstra
2017-12-13 16:31               ` Steven Rostedt
2017-12-12 14:32       ` Karol Herbst

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).