search for: raw_local_irq_restor

...othesis 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 state tracing gets confused */ >> ... >> raw_local_irq_restore(); >> >> and therefore, using raw variants in kvm_wait() works. It's also safe >> because it doesn't call any other libraries that would result in corrupt > > Yes, this is definitely an issue. > > Tracing, we also musn't call into tracing when using raw_...

...othesis 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 state tracing gets confused */ >> ... >> raw_local_irq_restore(); >> >> and therefore, using raw variants in kvm_wait() works. It's also safe >> because it doesn't call any other libraries that would result in corrupt > > Yes, this is definitely an issue. > > Tracing, we also musn't call into tracing when using raw_...

...ay 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 state tracing gets confused */ >> ... >> raw_local_irq_restore(); >> >> and therefore, using raw variants in kvm_wait() works. It's also safe >> because it doesn't call any other libraries that would result in corrupt >> IRQ state AFAIK. > > Just to follow-up, it'd still be nice to fix this. Suggestions? > > I...

On Wed, Aug 05, 2020 at 03:59:40PM +0200, Marco Elver wrote: > On Wed, Aug 05, 2020 at 03:42PM +0200, peterz at 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: > > ffffffff86271ee0 <arch_local_save_flags>: > ffffffff86271ee0: 0f 1f 44 00 00

On Wed, Aug 05, 2020 at 03:59:40PM +0200, Marco Elver wrote: > On Wed, Aug 05, 2020 at 03:42PM +0200, peterz at 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: > > ffffffff86271ee0 <arch_local_save_flags>: > ffffffff86271ee0: 0f 1f 44 00 00

Hi folks, Here is the result of the latest work on the pvops front, after the x86 arch merge. From the functionality point of view, almost nothing was changed, except for proper vsmp support - which was discussed, but not implemented before - and the introduction of smp_ops in x86_64, which eased the merging of the smp header. Speaking of the merge, a significant part (although not majority) of

Hi folks, Here is the result of the latest work on the pvops front, after the x86 arch merge. From the functionality point of view, almost nothing was changed, except for proper vsmp support - which was discussed, but not implemented before - and the introduction of smp_ops in x86_64, which eased the merging of the smp header. Speaking of the merge, a significant part (although not majority) of

...and unconditionally disable IRQs and trace them disabled. > + lockdep_assert_irqs_disabled(); /* Pass. */ > + local_irq_restore(flags2); But here, we restore IRQ state to 'disabled' and explicitly trace it disabled *again* (which is a bit daft, but whatever). > + } > + raw_local_irq_restore(flags1); This then restores the IRQ state to enable, but no tracing. > + } > + lockdep_assert_irqs_enabled(); /* FAIL! */ And we're out of sync... :/ /me goes ponder things... How's something like this then? --- include/linux/sched.h | 3 --- kernel/kcsan/core.c | 62 ++++++...

...o 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 state tracing gets confused */ > ... > raw_local_irq_restore(); > > and therefore, using raw variants in kvm_wait() works. It's also safe > because it doesn't call any other libraries that would result in corrupt Yes, this is definitely an issue. Tracing, we also musn't call into tracing when using raw_local_irq_*(). Because then we...

On Tue, Aug 11, 2020 at 11:20:54AM +0200, peterz at 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 to fix the entire fallout this release :/ > > Let me

On Tue, Aug 11, 2020 at 11:20:54AM +0200, peterz at 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 to fix the entire fallout this release :/ > > Let me

In your sequences in asm/paravirt.h, you explicitly save the caller-save regs: static inline void raw_local_irq_restore(unsigned long f) { __asm__ __volatile__(paravirt_alt("pushl %%ecx; pushl %%edx\n\t" "pushl %1; call *%0\n\t" "popl %1; popl %%edx; popl %%ecx", PARAVIRT_RESTORE_FLAGS) : : "m" (paravirt_ops.restore_fl), "a"(f)...

...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 state tracing gets confused */ > > > ... > > > raw_local_irq_restore(); > > > > > > and therefore, using raw variants in kvm_wait() works. It's also safe > > > because it doesn't call any other libraries that would result in corrupt > > > > Yes, this is definitely an issue. > > > > Tracing, we also musn&...

...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 state tracing gets confused */ >>>> ... >>>> raw_local_irq_restore(); >>>> >>>> and therefore, using raw variants in kvm_wait() works. It's also safe >>>> because it doesn't call any other libraries that would result in corrupt >>> >>> Yes, this is definitely an issue. >>> >>> Tracing...

...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 state tracing gets confused */ >>>> ... >>>> raw_local_irq_restore(); >>>> >>>> and therefore, using raw variants in kvm_wait() works. It's also safe >>>> because it doesn't call any other libraries that would result in corrupt >>> >>> Yes, this is definitely an issue. >>> >>> Tracing...

...0fc62e>] set_huge_pte_at+0xe/0x12 [ 86.404071] [<ffffffff810fec42>] hugetlb_fault+0x380/0x510 [ 86.404071] [<ffffffff810eba12>] handle_mm_fault+0xbd/0x84d [ 86.404071] [<ffffffff8100670f>] ? xen_restore_fl_direct_end+0x0/0x1 [ 86.404071] [<ffffffff812aad54>] ? raw_local_irq_restore+0x10/0x12 [ 86.404071] [<ffffffff8146c468>] do_page_fault+0x250/0x265 [ 86.404071] [<ffffffff814697f5>] page_fault+0x25/0x30 [ 86.404071] Code: ff ff 05 be 00 ca 00 e8 aa ff ff ff 44 8b 2d ba 00 ca 00 e8 48 6f 02 00 ff c8 0f 94 c0 0f b6 c0 46 8d 2c 28 44 89 2d a2 00 ca 00 <4...

...(x) paravirt_ops.write_cr3(x) + +#define read_cr4() paravirt_ops.read_cr4() +#define read_cr4_safe(x) paravirt_ops.read_cr4_safe() +#define write_cr4(x) paravirt_ops.write_cr4(x) + +static inline unsigned long __raw_local_save_flags(void) +{ + return paravirt_ops.save_fl(); +} + +static inline void raw_local_irq_restore(unsigned long flags) +{ + return paravirt_ops.restore_fl(flags); +} + +static inline void raw_local_irq_disable(void) +{ + paravirt_ops.irq_disable(); +} + +static inline void raw_local_irq_enable(void) +{ + paravirt_ops.irq_enable(); +} + +static inline unsigned long __raw_local_irq_save(void) +{...

...local_save_flags(void) -{ - unsigned long flags; - - __asm__ __volatile__( - "pushfl ; popl %0" - : "=g" (flags) - : /* no input */ - ); - - return flags; -} - #define raw_local_save_flags(flags) \ do { (flags) = __raw_local_save_flags(); } while (0) - -static inline void raw_local_irq_restore(unsigned long flags) -{ - __asm__ __volatile__( - "pushl %0 ; popfl" - : /* no output */ - :"g" (flags) - :"memory", "cc" - ); -} - -static inline void raw_local_irq_disable(void) -{ - __asm__ __volatile__("cli" : : : "memory"); -} -...

...local_save_flags(void) -{ - unsigned long flags; - - __asm__ __volatile__( - "pushfl ; popl %0" - : "=g" (flags) - : /* no input */ - ); - - return flags; -} - #define raw_local_save_flags(flags) \ do { (flags) = __raw_local_save_flags(); } while (0) - -static inline void raw_local_irq_restore(unsigned long flags) -{ - __asm__ __volatile__( - "pushl %0 ; popfl" - : /* no output */ - :"g" (flags) - :"memory", "cc" - ); -} - -static inline void raw_local_irq_disable(void) -{ - __asm__ __volatile__("cli" : : : "memory"); -} -...

...avirt.h> +#else #ifndef __ASSEMBLY__ static inline unsigned long __raw_local_save_flags(void) @@ -24,9 +27,6 @@ static inline unsigned long __raw_local_ return flags; } - -#define raw_local_save_flags(flags) \ - do { (flags) = __raw_local_save_flags(); } while (0) static inline void raw_local_irq_restore(unsigned long flags) { @@ -66,18 +66,6 @@ static inline void halt(void) __asm__ __volatile__("hlt": : :"memory"); } -static inline int raw_irqs_disabled_flags(unsigned long flags) -{ - return !(flags & (1 << 9)); -} - -static inline int raw_irqs_disabled(void) -...