| Message ID | 20231213233605.661251-28-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | kmsan: Enable on s390 | expand |
On Thu, Dec 14, 2023 at 12:36 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > KMSAN generates the following false positives on s390x: > > [ 6.063666] DEBUG_LOCKS_WARN_ON(lockdep_hardirqs_enabled()) > [ ...] > [ 6.577050] Call Trace: > [ 6.619637] [<000000000690d2de>] check_flags+0x1fe/0x210 > [ 6.665411] ([<000000000690d2da>] check_flags+0x1fa/0x210) > [ 6.707478] [<00000000006cec1a>] lock_acquire+0x2ca/0xce0 > [ 6.749959] [<00000000069820ea>] _raw_spin_lock_irqsave+0xea/0x190 > [ 6.794912] [<00000000041fc988>] __stack_depot_save+0x218/0x5b0 > [ 6.838420] [<000000000197affe>] __msan_poison_alloca+0xfe/0x1a0 > [ 6.882985] [<0000000007c5827c>] start_kernel+0x70c/0xd50 > [ 6.927454] [<0000000000100036>] startup_continue+0x36/0x40 > > Between trace_hardirqs_on() and `stosm __mask, 3` lockdep thinks that > interrupts are on, but on the CPU they are still off. KMSAN > instrumentation takes spinlocks, giving lockdep a chance to see and > complain about this discrepancy. > > KMSAN instrumentation is inserted in order to poison the __mask > variable. Disable instrumentation in the respective functions. They are > very small and it's easy to see that no important metadata updates are > lost because of this. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Reviewed-by: Alexander Potapenko <glider@google.com>
On Thu, Dec 14, 2023 at 12:24:47AM +0100, Ilya Leoshkevich wrote: > KMSAN generates the following false positives on s390x: > > [ 6.063666] DEBUG_LOCKS_WARN_ON(lockdep_hardirqs_enabled()) > [ ...] > [ 6.577050] Call Trace: > [ 6.619637] [<000000000690d2de>] check_flags+0x1fe/0x210 > [ 6.665411] ([<000000000690d2da>] check_flags+0x1fa/0x210) > [ 6.707478] [<00000000006cec1a>] lock_acquire+0x2ca/0xce0 > [ 6.749959] [<00000000069820ea>] _raw_spin_lock_irqsave+0xea/0x190 > [ 6.794912] [<00000000041fc988>] __stack_depot_save+0x218/0x5b0 > [ 6.838420] [<000000000197affe>] __msan_poison_alloca+0xfe/0x1a0 > [ 6.882985] [<0000000007c5827c>] start_kernel+0x70c/0xd50 > [ 6.927454] [<0000000000100036>] startup_continue+0x36/0x40 > > Between trace_hardirqs_on() and `stosm __mask, 3` lockdep thinks that > interrupts are on, but on the CPU they are still off. KMSAN > instrumentation takes spinlocks, giving lockdep a chance to see and > complain about this discrepancy. > > KMSAN instrumentation is inserted in order to poison the __mask > variable. Disable instrumentation in the respective functions. They are > very small and it's easy to see that no important metadata updates are > lost because of this. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > arch/s390/include/asm/irqflags.h | 18 +++++++++++++++--- > drivers/s390/char/sclp.c | 2 +- > 2 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/arch/s390/include/asm/irqflags.h b/arch/s390/include/asm/irqflags.h > index 02427b205c11..7353a88b2ae2 100644 > --- a/arch/s390/include/asm/irqflags.h > +++ b/arch/s390/include/asm/irqflags.h > @@ -37,12 +37,19 @@ static __always_inline void __arch_local_irq_ssm(unsigned long flags) > asm volatile("ssm %0" : : "Q" (flags) : "memory"); > } > > -static __always_inline unsigned long arch_local_save_flags(void) > +#ifdef CONFIG_KMSAN > +#define ARCH_LOCAL_IRQ_ATTRIBUTES \ > + noinline notrace __no_sanitize_memory __maybe_unused > +#else > +#define ARCH_LOCAL_IRQ_ATTRIBUTES __always_inline > +#endif > + > +static ARCH_LOCAL_IRQ_ATTRIBUTES unsigned long arch_local_save_flags(void) > { Please change this to lower case and long single lines, so it matches the more common patterns: #ifdef CONFIG_KMSAN #define __arch_local_irq_attributes noinline notrace __no_sanitize_memory __maybe_unused #else #define __arch_local_irq_attributes __always_inline #endif static __arch_local_irq_attributes unsigned long arch_local_save_flags(void) ...
diff --git a/arch/s390/include/asm/irqflags.h b/arch/s390/include/asm/irqflags.h index 02427b205c11..7353a88b2ae2 100644 --- a/arch/s390/include/asm/irqflags.h +++ b/arch/s390/include/asm/irqflags.h @@ -37,12 +37,19 @@ static __always_inline void __arch_local_irq_ssm(unsigned long flags) asm volatile("ssm %0" : : "Q" (flags) : "memory"); } -static __always_inline unsigned long arch_local_save_flags(void) +#ifdef CONFIG_KMSAN +#define ARCH_LOCAL_IRQ_ATTRIBUTES \ + noinline notrace __no_sanitize_memory __maybe_unused +#else +#define ARCH_LOCAL_IRQ_ATTRIBUTES __always_inline +#endif + +static ARCH_LOCAL_IRQ_ATTRIBUTES unsigned long arch_local_save_flags(void) { return __arch_local_irq_stnsm(0xff); } -static __always_inline unsigned long arch_local_irq_save(void) +static ARCH_LOCAL_IRQ_ATTRIBUTES unsigned long arch_local_irq_save(void) { return __arch_local_irq_stnsm(0xfc); } @@ -52,7 +59,12 @@ static __always_inline void arch_local_irq_disable(void) arch_local_irq_save(); } -static __always_inline void arch_local_irq_enable(void) +static ARCH_LOCAL_IRQ_ATTRIBUTES void arch_local_irq_enable_external(void) +{ + __arch_local_irq_stosm(0x01); +} + +static ARCH_LOCAL_IRQ_ATTRIBUTES void arch_local_irq_enable(void) { __arch_local_irq_stosm(0x03); } diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c index d53ee34d398f..fb1d9949adca 100644 --- a/drivers/s390/char/sclp.c +++ b/drivers/s390/char/sclp.c @@ -736,7 +736,7 @@ sclp_sync_wait(void) cr0_sync.val = cr0.val & ~CR0_IRQ_SUBCLASS_MASK; cr0_sync.val |= 1UL << (63 - 54); local_ctl_load(0, &cr0_sync); - __arch_local_irq_stosm(0x01); + arch_local_irq_enable_external(); /* Loop until driver state indicates finished request */ while (sclp_running_state != sclp_running_state_idle) { /* Check for expired request timer */
KMSAN generates the following false positives on s390x: [ 6.063666] DEBUG_LOCKS_WARN_ON(lockdep_hardirqs_enabled()) [ ...] [ 6.577050] Call Trace: [ 6.619637] [<000000000690d2de>] check_flags+0x1fe/0x210 [ 6.665411] ([<000000000690d2da>] check_flags+0x1fa/0x210) [ 6.707478] [<00000000006cec1a>] lock_acquire+0x2ca/0xce0 [ 6.749959] [<00000000069820ea>] _raw_spin_lock_irqsave+0xea/0x190 [ 6.794912] [<00000000041fc988>] __stack_depot_save+0x218/0x5b0 [ 6.838420] [<000000000197affe>] __msan_poison_alloca+0xfe/0x1a0 [ 6.882985] [<0000000007c5827c>] start_kernel+0x70c/0xd50 [ 6.927454] [<0000000000100036>] startup_continue+0x36/0x40 Between trace_hardirqs_on() and `stosm __mask, 3` lockdep thinks that interrupts are on, but on the CPU they are still off. KMSAN instrumentation takes spinlocks, giving lockdep a chance to see and complain about this discrepancy. KMSAN instrumentation is inserted in order to poison the __mask variable. Disable instrumentation in the respective functions. They are very small and it's easy to see that no important metadata updates are lost because of this. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- arch/s390/include/asm/irqflags.h | 18 +++++++++++++++--- drivers/s390/char/sclp.c | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-)