Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-12 Thread peterz
On Wed, Aug 12, 2020 at 10:18:32AM +0200, pet...@infradead.org wrote: > > trace_hardirqs_restore+0x59/0x80 kernel/trace/trace_preemptirq.c:106 > > rcu_irq_enter_irqson+0x43/0x70 kernel/rcu/tree.c:1074 > > trace_irq_enable_rcuidle+0x87/0x120 > > include/trace/events/preemptirq.h:40

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-12 Thread peterz
On Wed, Aug 12, 2020 at 10:06:50AM +0200, Marco Elver wrote: > On Tue, Aug 11, 2020 at 10:17PM +0200, pet...@infradead.org wrote: > > On Tue, Aug 11, 2020 at 11:46:51AM +0200, pet...@infradead.org wrote: > > > > > So let me once again see if I can't find a better solution for this all. > > >

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread peterz
On Tue, Aug 11, 2020 at 11:46:51AM +0200, pet...@infradead.org wrote: > So let me once again see if I can't find a better solution for this all. > Clearly it needs one :/ So the below boots without triggering the debug code from Marco -- it should allow nesting local_irq_save/restore under

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread peterz
On Tue, Aug 11, 2020 at 11:20:54AM +0200, pet...@infradead.org wrote: > On Tue, Aug 11, 2020 at 10:38:50AM +0200, Jürgen Groß wrote: > > In case you don't want to do it I can send the patch for the Xen > > variants. > > I might've opened a whole new can of worms here. I'm not sure we > can/want

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread peterz
On Tue, Aug 11, 2020 at 10:38:50AM +0200, Jürgen Groß wrote: > In case you don't want to do it I can send the patch for the Xen > variants. I might've opened a whole new can of worms here. I'm not sure we can/want to fix the entire fallout this release :/ Let me ponder this a little, because the

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread Jürgen Groß
On 11.08.20 10:12, Peter Zijlstra wrote: On Tue, Aug 11, 2020 at 09:57:55AM +0200, Jürgen Groß wrote: On 11.08.20 09:41, Peter Zijlstra wrote: On Fri, Aug 07, 2020 at 05:19:03PM +0200, Marco Elver wrote: My hypothesis here is simply that kvm_wait() may be called in a place where we get the

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread Jürgen Groß
On 11.08.20 10:12, Peter Zijlstra wrote: On Tue, Aug 11, 2020 at 09:57:55AM +0200, Jürgen Groß wrote: On 11.08.20 09:41, Peter Zijlstra wrote: On Fri, Aug 07, 2020 at 05:19:03PM +0200, Marco Elver wrote: My hypothesis here is simply that kvm_wait() may be called in a place where we get the

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread Peter Zijlstra
On Tue, Aug 11, 2020 at 09:57:55AM +0200, Jürgen Groß wrote: > On 11.08.20 09:41, Peter Zijlstra wrote: > > On Fri, Aug 07, 2020 at 05:19:03PM +0200, Marco Elver wrote: > > > > > My hypothesis here is simply that kvm_wait() may be called in a place > > > where we get the same case I mentioned to

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread Jürgen Groß
On 11.08.20 09:41, Peter Zijlstra wrote: On Fri, Aug 07, 2020 at 05:19:03PM +0200, Marco Elver wrote: My hypothesis here is simply that kvm_wait() may be called in a place where we get the same case I mentioned to Peter, raw_local_irq_save(); /* or other IRQs off without tracing */

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread Peter Zijlstra
On Fri, Aug 07, 2020 at 05:19:03PM +0200, Marco Elver wrote: > My hypothesis here is simply that kvm_wait() may be called in a place > where we get the same case I mentioned to Peter, > > raw_local_irq_save(); /* or other IRQs off without tracing */ > ... > kvm_wait() /* IRQ

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-11 Thread Jürgen Groß
On 11.08.20 09:00, Marco Elver wrote: On Fri, 7 Aug 2020 at 17:19, Marco Elver wrote: On Fri, Aug 07, 2020 at 02:08PM +0200, Marco Elver wrote: On Fri, 7 Aug 2020 at 14:04, Jürgen Groß wrote: On 07.08.20 13:38, Marco Elver wrote: On Fri, Aug 07, 2020 at 12:35PM +0200, Jürgen Groß wrote:

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-07 Thread Jürgen Groß
On 07.08.20 13:38, Marco Elver wrote: On Fri, Aug 07, 2020 at 12:35PM +0200, Jürgen Groß wrote: On 07.08.20 11:50, Marco Elver wrote: On Fri, Aug 07, 2020 at 11:24AM +0200, Jürgen Groß wrote: On 07.08.20 11:01, Marco Elver wrote: On Thu, 6 Aug 2020 at 18:06, Marco Elver wrote: On Thu, 6

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-07 Thread Jürgen Groß
On 07.08.20 11:50, Marco Elver wrote: On Fri, Aug 07, 2020 at 11:24AM +0200, Jürgen Groß wrote: On 07.08.20 11:01, Marco Elver wrote: On Thu, 6 Aug 2020 at 18:06, Marco Elver wrote: On Thu, 6 Aug 2020 at 15:17, Marco Elver wrote: On Thu, Aug 06, 2020 at 01:32PM +0200, pet...@infradead.org

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-07 Thread Jürgen Groß
On 07.08.20 11:01, Marco Elver wrote: On Thu, 6 Aug 2020 at 18:06, Marco Elver wrote: On Thu, 6 Aug 2020 at 15:17, Marco Elver wrote: On Thu, Aug 06, 2020 at 01:32PM +0200, pet...@infradead.org wrote: On Thu, Aug 06, 2020 at 09:47:23AM +0200, Marco Elver wrote: Testing my hypothesis that

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-06 Thread peterz
On Thu, Aug 06, 2020 at 09:47:23AM +0200, Marco Elver wrote: > Testing my hypothesis that raw then nested non-raw > local_irq_save/restore() breaks IRQ state tracking -- see the reproducer > below. This is at least 1 case I can think of that we're bound to hit. Aaargh! > diff --git a/init/main.c

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-05 Thread Jürgen Groß
On 05.08.20 16:12, pet...@infradead.org wrote: On Wed, Aug 05, 2020 at 03:59:40PM +0200, Marco Elver wrote: On Wed, Aug 05, 2020 at 03:42PM +0200, pet...@infradead.org wrote: Shouldn't we __always_inline those? They're going to be really small. I can send a v2, and you can choose. For

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-05 Thread peterz
On Wed, Aug 05, 2020 at 04:12:37PM +0200, pet...@infradead.org wrote: > On Wed, Aug 05, 2020 at 03:59:40PM +0200, Marco Elver wrote: > > On Wed, Aug 05, 2020 at 03:42PM +0200, pet...@infradead.org wrote: > > > > Shouldn't we __always_inline those? They're going to be really small. > > > > I can

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-05 Thread peterz
On Wed, Aug 05, 2020 at 03:59:40PM +0200, Marco Elver wrote: > On Wed, Aug 05, 2020 at 03:42PM +0200, pet...@infradead.org wrote: > > Shouldn't we __always_inline those? They're going to be really small. > > I can send a v2, and you can choose. For reference, though: > > 86271ee0

Re: [PATCH] x86/paravirt: Add missing noinstr to arch_local*() helpers

2020-08-05 Thread peterz
On Wed, Aug 05, 2020 at 03:26:29PM +0200, Marco Elver wrote: > Add missing noinstr to arch_local*() helpers, as they may be called from > noinstr code. > > On a KCSAN config with CONFIG_PARAVIRT=y, syzbot stumbled across corrupt Cute, so I've been working on adding objtool support for this a