[PATCH] xtensa: enable KCSAN

[PATCH] xtensa: enable KCSAN

[Max Filippov]

Prefix arch-specific barrier macros with '__' to make use of instrumented
generic macros.
Prefix arch-specific bitops with 'arch_' to make use of instrumented
generic functions.
Provide stubs for 64-bit atomics when building with KCSAN.
Disable KCSAN instrumentation in arch/xtensa/boot.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 arch/xtensa/Kconfig               |  1 +
 arch/xtensa/boot/lib/Makefile     |  1 +
 arch/xtensa/include/asm/barrier.h |  6 ++--
 arch/xtensa/include/asm/bitops.h  | 10 +++---
 arch/xtensa/lib/Makefile          |  2 ++
 arch/xtensa/lib/kcsan-stubs.c     | 54 +++++++++++++++++++++++++++++++
 6 files changed, 67 insertions(+), 7 deletions(-)
 create mode 100644 arch/xtensa/lib/kcsan-stubs.c

diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 797355c142b3..c87f5ab493d9 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -29,6 +29,7 @@ config XTENSA
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
 	select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL
+	select HAVE_ARCH_KCSAN
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_CONTEXT_TRACKING
diff --git a/arch/xtensa/boot/lib/Makefile b/arch/xtensa/boot/lib/Makefile
index e3d717c7bfa1..162d10af36f3 100644
--- a/arch/xtensa/boot/lib/Makefile
+++ b/arch/xtensa/boot/lib/Makefile
@@ -16,6 +16,7 @@ CFLAGS_REMOVE_inffast.o = -pg
 endif
 
 KASAN_SANITIZE := n
+KCSAN_SANITIZE := n
 
 CFLAGS_REMOVE_inflate.o += -fstack-protector -fstack-protector-strong
 CFLAGS_REMOVE_zmem.o += -fstack-protector -fstack-protector-strong
diff --git a/arch/xtensa/include/asm/barrier.h b/arch/xtensa/include/asm/barrier.h
index d6f8d4ddc2bc..a22d4bb08159 100644
--- a/arch/xtensa/include/asm/barrier.h
+++ b/arch/xtensa/include/asm/barrier.h
@@ -11,9 +11,9 @@
 
 #include <asm/core.h>
 
-#define mb()  ({ __asm__ __volatile__("memw" : : : "memory"); })
-#define rmb() barrier()
-#define wmb() mb()
+#define __mb()  ({ __asm__ __volatile__("memw" : : : "memory"); })
+#define __rmb() barrier()
+#define __wmb() mb()
 
 #if XCHAL_HAVE_S32C1I
 #define __smp_mb__before_atomic()		barrier()
diff --git a/arch/xtensa/include/asm/bitops.h b/arch/xtensa/include/asm/bitops.h
index cd225896c40f..e02ec5833389 100644
--- a/arch/xtensa/include/asm/bitops.h
+++ b/arch/xtensa/include/asm/bitops.h
@@ -99,7 +99,7 @@ static inline unsigned long __fls(unsigned long word)
 #if XCHAL_HAVE_EXCLUSIVE
 
 #define BIT_OP(op, insn, inv)						\
-static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
+static inline void arch_##op##_bit(unsigned int bit, volatile unsigned long *p)\
 {									\
 	unsigned long tmp;						\
 	unsigned long mask = 1UL << (bit & 31);				\
@@ -119,7 +119,7 @@ static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
 
 #define TEST_AND_BIT_OP(op, insn, inv)					\
 static inline int							\
-test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)	\
+arch_test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)	\
 {									\
 	unsigned long tmp, value;					\
 	unsigned long mask = 1UL << (bit & 31);				\
@@ -142,7 +142,7 @@ test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)	\
 #elif XCHAL_HAVE_S32C1I
 
 #define BIT_OP(op, insn, inv)						\
-static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
+static inline void arch_##op##_bit(unsigned int bit, volatile unsigned long *p)\
 {									\
 	unsigned long tmp, value;					\
 	unsigned long mask = 1UL << (bit & 31);				\
@@ -163,7 +163,7 @@ static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
 
 #define TEST_AND_BIT_OP(op, insn, inv)					\
 static inline int							\
-test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)	\
+arch_test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)	\
 {									\
 	unsigned long tmp, value;					\
 	unsigned long mask = 1UL << (bit & 31);				\
@@ -205,6 +205,8 @@ BIT_OPS(change, "xor", )
 #undef BIT_OP
 #undef TEST_AND_BIT_OP
 
+#include <asm-generic/bitops/instrumented-atomic.h>
+
 #include <asm-generic/bitops/le.h>
 
 #include <asm-generic/bitops/ext2-atomic-setbit.h>
diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
index 5848c133f7ea..d4e9c397e3fd 100644
--- a/arch/xtensa/lib/Makefile
+++ b/arch/xtensa/lib/Makefile
@@ -8,3 +8,5 @@ lib-y	+= memcopy.o memset.o checksum.o \
 	   divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o \
 	   usercopy.o strncpy_user.o strnlen_user.o
 lib-$(CONFIG_PCI) += pci-auto.o
+lib-$(CONFIG_KCSAN) += kcsan-stubs.o
+KCSAN_SANITIZE_kcsan-stubs.o := n
diff --git a/arch/xtensa/lib/kcsan-stubs.c b/arch/xtensa/lib/kcsan-stubs.c
new file mode 100644
index 000000000000..2b08faa62b86
--- /dev/null
+++ b/arch/xtensa/lib/kcsan-stubs.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bug.h>
+#include <linux/types.h>
+
+void __atomic_store_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
+
+u64 __atomic_load_8(const volatile void *p, int i)
+{
+	BUG();
+}
+
+u64 __atomic_exchange_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
+
+bool __atomic_compare_exchange_8(volatile void *p1, void *p2, u64 v, bool b, int i1, int i2)
+{
+	BUG();
+}
+
+u64 __atomic_fetch_add_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
+
+u64 __atomic_fetch_sub_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
+
+u64 __atomic_fetch_and_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
+
+u64 __atomic_fetch_or_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
+
+u64 __atomic_fetch_xor_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
+
+u64 __atomic_fetch_nand_8(volatile void *p, u64 v, int i)
+{
+	BUG();
+}
-- 
2.30.2

Re: [PATCH] xtensa: enable KCSAN

[Marco Elver]

Nice to see this happen!

On Sat, 16 Apr 2022 at 10:14, Max Filippov <jcmvbkbc@gmail.com> wrote:
>
> Prefix arch-specific barrier macros with '__' to make use of instrumented
> generic macros.
> Prefix arch-specific bitops with 'arch_' to make use of instrumented
> generic functions.

> Provide stubs for 64-bit atomics when building with KCSAN.

The stubs are the only thing I don't understand. More elaboration on
why this is required would be useful (maybe there's another way to
solve?).

> Disable KCSAN instrumentation in arch/xtensa/boot.

Given you went for barrier instrumentation, I assume you tested with a
CONFIG_KCSAN_STRICT=y config? Did the kcsan_test pass?

> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  arch/xtensa/Kconfig               |  1 +
>  arch/xtensa/boot/lib/Makefile     |  1 +
>  arch/xtensa/include/asm/barrier.h |  6 ++--
>  arch/xtensa/include/asm/bitops.h  | 10 +++---
>  arch/xtensa/lib/Makefile          |  2 ++
>  arch/xtensa/lib/kcsan-stubs.c     | 54 +++++++++++++++++++++++++++++++
>  6 files changed, 67 insertions(+), 7 deletions(-)
>  create mode 100644 arch/xtensa/lib/kcsan-stubs.c
>
> diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> index 797355c142b3..c87f5ab493d9 100644
> --- a/arch/xtensa/Kconfig
> +++ b/arch/xtensa/Kconfig
> @@ -29,6 +29,7 @@ config XTENSA
>         select HAVE_ARCH_AUDITSYSCALL
>         select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
>         select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL
> +       select HAVE_ARCH_KCSAN
>         select HAVE_ARCH_SECCOMP_FILTER
>         select HAVE_ARCH_TRACEHOOK
>         select HAVE_CONTEXT_TRACKING
> diff --git a/arch/xtensa/boot/lib/Makefile b/arch/xtensa/boot/lib/Makefile
> index e3d717c7bfa1..162d10af36f3 100644
> --- a/arch/xtensa/boot/lib/Makefile
> +++ b/arch/xtensa/boot/lib/Makefile
> @@ -16,6 +16,7 @@ CFLAGS_REMOVE_inffast.o = -pg
>  endif
>
>  KASAN_SANITIZE := n
> +KCSAN_SANITIZE := n
>
>  CFLAGS_REMOVE_inflate.o += -fstack-protector -fstack-protector-strong
>  CFLAGS_REMOVE_zmem.o += -fstack-protector -fstack-protector-strong
> diff --git a/arch/xtensa/include/asm/barrier.h b/arch/xtensa/include/asm/barrier.h
> index d6f8d4ddc2bc..a22d4bb08159 100644
> --- a/arch/xtensa/include/asm/barrier.h
> +++ b/arch/xtensa/include/asm/barrier.h
> @@ -11,9 +11,9 @@
>
>  #include <asm/core.h>
>
> -#define mb()  ({ __asm__ __volatile__("memw" : : : "memory"); })
> -#define rmb() barrier()
> -#define wmb() mb()
> +#define __mb()  ({ __asm__ __volatile__("memw" : : : "memory"); })
> +#define __rmb() barrier()
> +#define __wmb() mb()
>
>  #if XCHAL_HAVE_S32C1I
>  #define __smp_mb__before_atomic()              barrier()
> diff --git a/arch/xtensa/include/asm/bitops.h b/arch/xtensa/include/asm/bitops.h
> index cd225896c40f..e02ec5833389 100644
> --- a/arch/xtensa/include/asm/bitops.h
> +++ b/arch/xtensa/include/asm/bitops.h
> @@ -99,7 +99,7 @@ static inline unsigned long __fls(unsigned long word)
>  #if XCHAL_HAVE_EXCLUSIVE
>
>  #define BIT_OP(op, insn, inv)                                          \
> -static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
> +static inline void arch_##op##_bit(unsigned int bit, volatile unsigned long *p)\
>  {                                                                      \
>         unsigned long tmp;                                              \
>         unsigned long mask = 1UL << (bit & 31);                         \
> @@ -119,7 +119,7 @@ static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
>
>  #define TEST_AND_BIT_OP(op, insn, inv)                                 \
>  static inline int                                                      \
> -test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)       \
> +arch_test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)  \
>  {                                                                      \
>         unsigned long tmp, value;                                       \
>         unsigned long mask = 1UL << (bit & 31);                         \
> @@ -142,7 +142,7 @@ test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)    \
>  #elif XCHAL_HAVE_S32C1I
>
>  #define BIT_OP(op, insn, inv)                                          \
> -static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
> +static inline void arch_##op##_bit(unsigned int bit, volatile unsigned long *p)\
>  {                                                                      \
>         unsigned long tmp, value;                                       \
>         unsigned long mask = 1UL << (bit & 31);                         \
> @@ -163,7 +163,7 @@ static inline void op##_bit(unsigned int bit, volatile unsigned long *p)\
>
>  #define TEST_AND_BIT_OP(op, insn, inv)                                 \
>  static inline int                                                      \
> -test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)       \
> +arch_test_and_##op##_bit(unsigned int bit, volatile unsigned long *p)  \
>  {                                                                      \
>         unsigned long tmp, value;                                       \
>         unsigned long mask = 1UL << (bit & 31);                         \
> @@ -205,6 +205,8 @@ BIT_OPS(change, "xor", )
>  #undef BIT_OP
>  #undef TEST_AND_BIT_OP
>
> +#include <asm-generic/bitops/instrumented-atomic.h>
> +
>  #include <asm-generic/bitops/le.h>
>
>  #include <asm-generic/bitops/ext2-atomic-setbit.h>
> diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
> index 5848c133f7ea..d4e9c397e3fd 100644
> --- a/arch/xtensa/lib/Makefile
> +++ b/arch/xtensa/lib/Makefile
> @@ -8,3 +8,5 @@ lib-y   += memcopy.o memset.o checksum.o \
>            divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o \
>            usercopy.o strncpy_user.o strnlen_user.o
>  lib-$(CONFIG_PCI) += pci-auto.o
> +lib-$(CONFIG_KCSAN) += kcsan-stubs.o
> +KCSAN_SANITIZE_kcsan-stubs.o := n
> diff --git a/arch/xtensa/lib/kcsan-stubs.c b/arch/xtensa/lib/kcsan-stubs.c
> new file mode 100644
> index 000000000000..2b08faa62b86
> --- /dev/null
> +++ b/arch/xtensa/lib/kcsan-stubs.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bug.h>
> +#include <linux/types.h>
> +
> +void __atomic_store_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_load_8(const volatile void *p, int i)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_exchange_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> +
> +bool __atomic_compare_exchange_8(volatile void *p1, void *p2, u64 v, bool b, int i1, int i2)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_fetch_add_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_fetch_sub_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_fetch_and_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_fetch_or_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_fetch_xor_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> +
> +u64 __atomic_fetch_nand_8(volatile void *p, u64 v, int i)
> +{
> +       BUG();
> +}
> --
> 2.30.2
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20220416081355.2155050-1-jcmvbkbc%40gmail.com.

Re: [PATCH] xtensa: enable KCSAN

[Max Filippov]

Hi Marco,

On Tue, Apr 19, 2022 at 3:16 AM Marco Elver <elver@google.com> wrote:
>
> Nice to see this happen!
>
> On Sat, 16 Apr 2022 at 10:14, Max Filippov <jcmvbkbc@gmail.com> wrote:
> > Provide stubs for 64-bit atomics when building with KCSAN.
>
> The stubs are the only thing I don't understand. More elaboration on
> why this is required would be useful (maybe there's another way to
> solve?).

It doesn't build without it, because the compiler left function calls
in the code:

xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic32_compare_exchange_val':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_load_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_load':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_load_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_store_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_store':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_store_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_exchange_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_exchange':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_exchange_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_fetch_add_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_fetch_add':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_fetch_add_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_fetch_sub_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_fetch_sub':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_fetch_sub_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_fetch_and_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_fetch_and':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_fetch_and_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_fetch_or_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_fetch_or':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_fetch_or_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_fetch_xor_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_fetch_xor':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_fetch_xor_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_fetch_nand_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_fetch_nand':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_fetch_nand_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_compare_exchange_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_compare_exchange_strong':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_compare_exchange_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_compare_exchange_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
`__tsan_atomic64_compare_exchange_weak':
kernel/kcsan/core.c:1262: undefined reference to `__atomic_compare_exchange_8'
xtensa-de233_fpu-elf-ld: kernel/kcsan/core.c:1262: undefined reference
to `__atomic_compare_exchange_8'

None of these functions are called because xtensa doesn't have
64-bit atomic ops.

I guess that another way to fix it would be making
DEFINE_TSAN_ATOMIC_OPS(64);
conditional and not enabling it when building for xtensa.

> > Disable KCSAN instrumentation in arch/xtensa/boot.
>
> Given you went for barrier instrumentation, I assume you tested with a
> CONFIG_KCSAN_STRICT=y config?

Yes.

> Did the kcsan_test pass?

current results are the following on QEMU:

     # test_missing_barrier: EXPECTATION FAILED at
kernel/kcsan/kcsan_test.c:1313
     Expected match_expect to be true, but is false
     # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
kernel/kcsan/kcsan_test.c:1356
     Expected match_expect to be true, but is false
 # kcsan: pass:27 fail:2 skip:0 total:29
 # Totals: pass:193 fail:4 skip:0 total:197

and the following on the real hardware:

    # test_concurrent_races: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:762
    Expected match_expect to be true, but is false
    # test_write_write_struct_part: EXPECTATION FAILED at
kernel/kcsan/kcsan_test.c:910
    Expected match_expect to be true, but is false
    # test_assert_exclusive_access_writer: EXPECTATION FAILED at
kernel/kcsan/kcsan_test.c:1077
    Expected match_expect_access_writer to be true, but is false
    # test_assert_exclusive_bits_change: EXPECTATION FAILED at
kernel/kcsan/kcsan_test.c:1098
    Expected match_expect to be true, but is false
    # test_assert_exclusive_writer_scoped: EXPECTATION FAILED at
kernel/kcsan/kcsan_test.c:1136
    Expected match_expect_start to be true, but is false
    # test_missing_barrier: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:1313
    Expected match_expect to be true, but is false
    # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
kernel/kcsan/kcsan_test.c:1356
    Expected match_expect to be true, but is false
# kcsan: pass:22 fail:7 skip:0 total:29
# Totals: pass:177 fail:20 skip:0 total:197

-- 
Thanks.
-- Max

Re: [PATCH] xtensa: enable KCSAN

[Marco Elver]

On Tue, Apr 19, 2022 at 07:59PM -0700, Max Filippov wrote:
[...]
> > The stubs are the only thing I don't understand. More elaboration on
> > why this is required would be useful (maybe there's another way to
> > solve?).
> 
> It doesn't build without it, because the compiler left function calls
> in the code:
> 
> xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
> `__tsan_atomic32_compare_exchange_val':
> kernel/kcsan/core.c:1262: undefined reference to `__atomic_load_8'
> xtensa-de233_fpu-elf-ld: kernel/kcsan/core.o: in function
> `__tsan_atomic64_load':
[...]
> 
> None of these functions are called because xtensa doesn't have
> 64-bit atomic ops.
> 
> I guess that another way to fix it would be making
> DEFINE_TSAN_ATOMIC_OPS(64);
> conditional and not enabling it when building for xtensa.

I see - however, it seems the kernel provides 64-bit atomics to xtensa
using lib/atomic64.c:

	arch/xtensa/Kconfig:    select GENERIC_ATOMIC64

So the right thing to do might be to implement the builtin atomics using
the kernel's atomic64_* primitives. However, granted, the builtin
atomics might not be needed on xtensa (depending on configuration).
Their existence is due to some compiler instrumentation emitting
builtin-atomics (Clang's GCOV), folks using them accidentally and
blaming KCSAN (also https://paulmck.livejournal.com/64970.html).

So I think it's fair to leave them to BUG() until somebody complains (at
which point they need to be implemented). I leave it to you.

> > > Disable KCSAN instrumentation in arch/xtensa/boot.
> >
> > Given you went for barrier instrumentation, I assume you tested with a
> > CONFIG_KCSAN_STRICT=y config?
> 
> Yes.
> 
> > Did the kcsan_test pass?
> 
> current results are the following on QEMU:
> 
>      # test_missing_barrier: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1313
>      Expected match_expect to be true, but is false
>      # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1356
>      Expected match_expect to be true, but is false
>  # kcsan: pass:27 fail:2 skip:0 total:29
>  # Totals: pass:193 fail:4 skip:0 total:197
> 
> and the following on the real hardware:
> 
>     # test_concurrent_races: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:762
>     Expected match_expect to be true, but is false
>     # test_write_write_struct_part: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:910
>     Expected match_expect to be true, but is false
>     # test_assert_exclusive_access_writer: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1077
>     Expected match_expect_access_writer to be true, but is false
>     # test_assert_exclusive_bits_change: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1098
>     Expected match_expect to be true, but is false
>     # test_assert_exclusive_writer_scoped: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1136
>     Expected match_expect_start to be true, but is false
>     # test_missing_barrier: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:1313
>     Expected match_expect to be true, but is false
>     # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
> kernel/kcsan/kcsan_test.c:1356
>     Expected match_expect to be true, but is false
> # kcsan: pass:22 fail:7 skip:0 total:29
> # Totals: pass:177 fail:20 skip:0 total:197

Each test case is run with varying number of threads - am I correctly
inferring that out of all test cases, usually only one such run failed,
and runs with different number of threads (of the same test case)
succeeded?

If that's the case, I think we can say that it works, and the failures
are due to flakiness with either higher or lower threads counts. I know
that some test cases might still be flaky under QEMU TCG because of how
it does concurrent execution of different CPU cores.

Thanks,
-- Marco

Re: [PATCH] xtensa: enable KCSAN

[Max Filippov]

On Wed, Apr 20, 2022 at 2:04 AM Marco Elver <elver@google.com> wrote:
> So the right thing to do might be to implement the builtin atomics using
> the kernel's atomic64_* primitives. However, granted, the builtin
> atomics might not be needed on xtensa (depending on configuration).
> Their existence is due to some compiler instrumentation emitting
> builtin-atomics (Clang's GCOV), folks using them accidentally and
> blaming KCSAN (also https://paulmck.livejournal.com/64970.html).
>
> So I think it's fair to leave them to BUG() until somebody complains (at
> which point they need to be implemented). I leave it to you.

Sure, that was my plan.

> > > Did the kcsan_test pass?
> >
> > current results are the following on QEMU:
> >
> >      # test_missing_barrier: EXPECTATION FAILED at
> > kernel/kcsan/kcsan_test.c:1313
> >      Expected match_expect to be true, but is false
> >      # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
> > kernel/kcsan/kcsan_test.c:1356
> >      Expected match_expect to be true, but is false
> >  # kcsan: pass:27 fail:2 skip:0 total:29
> >  # Totals: pass:193 fail:4 skip:0 total:197
> >
> > and the following on the real hardware:
> >
> >     # test_concurrent_races: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:762
> >     Expected match_expect to be true, but is false
> >     # test_write_write_struct_part: EXPECTATION FAILED at
> > kernel/kcsan/kcsan_test.c:910
> >     Expected match_expect to be true, but is false
> >     # test_assert_exclusive_access_writer: EXPECTATION FAILED at
> > kernel/kcsan/kcsan_test.c:1077
> >     Expected match_expect_access_writer to be true, but is false
> >     # test_assert_exclusive_bits_change: EXPECTATION FAILED at
> > kernel/kcsan/kcsan_test.c:1098
> >     Expected match_expect to be true, but is false
> >     # test_assert_exclusive_writer_scoped: EXPECTATION FAILED at
> > kernel/kcsan/kcsan_test.c:1136
> >     Expected match_expect_start to be true, but is false
> >     # test_missing_barrier: EXPECTATION FAILED at kernel/kcsan/kcsan_test.c:1313
> >     Expected match_expect to be true, but is false
> >     # test_atomic_builtins_missing_barrier: EXPECTATION FAILED at
> > kernel/kcsan/kcsan_test.c:1356
> >     Expected match_expect to be true, but is false
> > # kcsan: pass:22 fail:7 skip:0 total:29
> > # Totals: pass:177 fail:20 skip:0 total:197
>
> Each test case is run with varying number of threads - am I correctly
> inferring that out of all test cases, usually only one such run failed,
> and runs with different number of threads (of the same test case)
> succeeded?

For most of the failures -- yes.
For the test_missing_barrier and test_atomic_builtins_missing_barrier
on the hardware it was the opposite: only one subtest succeeded while
all others failed. Does it mean that the xtensa memory model is
insufficiently weak?

> If that's the case, I think we can say that it works, and the failures
> are due to flakiness with either higher or lower threads counts. I know
> that some test cases might still be flaky under QEMU TCG because of how
> it does concurrent execution of different CPU cores.

Thanks for taking a look.
I'll post v2 with a couple additional minor changes.

-- 
Thanks.
-- Max

Re: [PATCH] xtensa: enable KCSAN

[Marco Elver]

On Wed, 20 Apr 2022 at 20:11, Max Filippov <jcmvbkbc@gmail.com> wrote:

> > Each test case is run with varying number of threads - am I correctly
> > inferring that out of all test cases, usually only one such run failed,
> > and runs with different number of threads (of the same test case)
> > succeeded?
>
> For most of the failures -- yes.
> For the test_missing_barrier and test_atomic_builtins_missing_barrier
> on the hardware it was the opposite: only one subtest succeeded while
> all others failed. Does it mean that the xtensa memory model is
> insufficiently weak?

No - KCSAN's weak memory modeling and detection of missing barriers
doesn't care what the HW does, it only approximates the LKMM. If the
test_barrier_nothreads case passed, there's nothing wrong with barrier
instrumentation. Regarding the test case failures, if at least 1
passed I'm guessing it's just flaky (not enough concurrency, or
unexpected barriers due to too many interrupts which can happen if we
enter the scheduler).

Unfortunately I don't know xtensa and if this is normal, but modulo
flakiness, I think it's fine. (I've made a note to try and deflake the
test if I can find time to try the xtensa version.)