linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
@ 2023-02-16  5:09 Rohan McLure
  2023-02-16  5:09 ` [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support Rohan McLure
  2023-02-16  7:12 ` [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems Christophe Leroy
  0 siblings, 2 replies; 9+ messages in thread
From: Rohan McLure @ 2023-02-16  5:09 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Rohan McLure, Max Filippov

KCSAN instruments calls to atomic builtins, and will in turn call these
builtins itself. As such, architectures supporting KCSAN must have
compiler support for these atomic primitives.

Since 32-bit systems are unlikely to have 64-bit compiler builtins,
provide a stub for each missing builtin, and use BUG() to assert
unreachability.

In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
locally. Move these definitions to be accessible to all 32-bit
architectures that do not provide the necessary builtins, with opt in
for PowerPC and xtensa.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
---
Previously issued as a part of a patch series adding KCSAN support to
64-bit.
Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t
v1: Remove __has_builtin check, as gcc is not obligated to inline
builtins detected using this check, but instead is permitted to supply
them in libatomic:
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
Instead, opt-in PPC32 and xtensa.
---
 arch/xtensa/lib/Makefile                              | 1 -
 kernel/kcsan/Makefile                                 | 2 ++
 arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
 3 files changed, 2 insertions(+), 1 deletion(-)
 rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)

diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
index 7ecef0519a27..d69356dc97df 100644
--- a/arch/xtensa/lib/Makefile
+++ b/arch/xtensa/lib/Makefile
@@ -8,5 +8,4 @@ lib-y	+= memcopy.o memset.o checksum.o \
 	   divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.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/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
index 8cf70f068d92..86dd713d8855 100644
--- a/kernel/kcsan/Makefile
+++ b/kernel/kcsan/Makefile
@@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
 	-fno-stack-protector -DDISABLE_BRANCH_PROFILING
 
 obj-y := core.o debugfs.o report.o
+obj-$(CONFIG_PPC32) += stubs.o
+obj-$(CONFIG_XTENSA) += stubs.o
 
 KCSAN_INSTRUMENT_BARRIERS_selftest.o := y
 obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o
diff --git a/arch/xtensa/lib/kcsan-stubs.c b/kernel/kcsan/stubs.c
similarity index 100%
rename from arch/xtensa/lib/kcsan-stubs.c
rename to kernel/kcsan/stubs.c
-- 
2.37.2


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

* [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support
  2023-02-16  5:09 [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems Rohan McLure
@ 2023-02-16  5:09 ` Rohan McLure
  2023-02-16  7:14   ` Christophe Leroy
  2023-02-16  7:12 ` [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems Christophe Leroy
  1 sibling, 1 reply; 9+ messages in thread
From: Rohan McLure @ 2023-02-16  5:09 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Rohan McLure

Enable HAVE_ARCH_KCSAN on all powerpc platforms, permitting use of the
kernel concurrency sanitiser through the CONFIG_KCSAN_* kconfig options.

Boots and passes selftests on 32-bit and 64-bit platforms. See
documentation in Documentation/dev-tools/kcsan.rst for more information.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
New patch
---
 arch/powerpc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2c9cdf1d8761..45771448d47a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -197,7 +197,7 @@ config PPC
 	select HAVE_ARCH_KASAN			if PPC_RADIX_MMU
 	select HAVE_ARCH_KASAN			if PPC_BOOK3E_64
 	select HAVE_ARCH_KASAN_VMALLOC		if HAVE_ARCH_KASAN
-	select HAVE_ARCH_KCSAN            	if PPC_BOOK3S_64
+	select HAVE_ARCH_KCSAN
 	select HAVE_ARCH_KFENCE			if ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
 	select HAVE_ARCH_KGDB
-- 
2.37.2


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

* Re: [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
  2023-02-16  5:09 [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems Rohan McLure
  2023-02-16  5:09 ` [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support Rohan McLure
@ 2023-02-16  7:12 ` Christophe Leroy
  2023-02-16  8:09   ` Marco Elver
  1 sibling, 1 reply; 9+ messages in thread
From: Christophe Leroy @ 2023-02-16  7:12 UTC (permalink / raw)
  To: Rohan McLure, linuxppc-dev@lists.ozlabs.org
  Cc: Marco Elver, Dmitry Vyukov, kasan-dev, Max Filippov



Le 16/02/2023 à 06:09, Rohan McLure a écrit :
> KCSAN instruments calls to atomic builtins, and will in turn call these
> builtins itself. As such, architectures supporting KCSAN must have
> compiler support for these atomic primitives.
> 
> Since 32-bit systems are unlikely to have 64-bit compiler builtins,
> provide a stub for each missing builtin, and use BUG() to assert
> unreachability.
> 
> In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
> locally. Move these definitions to be accessible to all 32-bit
> architectures that do not provide the necessary builtins, with opt in
> for PowerPC and xtensa.
> 
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>

This series should also be addressed to KCSAN Maintainers, shouldn't it ?

KCSAN
M:	Marco Elver <elver@google.com>
R:	Dmitry Vyukov <dvyukov@google.com>
L:	kasan-dev@googlegroups.com
S:	Maintained
F:	Documentation/dev-tools/kcsan.rst
F:	include/linux/kcsan*.h
F:	kernel/kcsan/
F:	lib/Kconfig.kcsan
F:	scripts/Makefile.kcsan


> ---
> Previously issued as a part of a patch series adding KCSAN support to
> 64-bit.
> Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t
> v1: Remove __has_builtin check, as gcc is not obligated to inline
> builtins detected using this check, but instead is permitted to supply
> them in libatomic:
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
> Instead, opt-in PPC32 and xtensa.
> ---
>   arch/xtensa/lib/Makefile                              | 1 -
>   kernel/kcsan/Makefile                                 | 2 ++
>   arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
>   3 files changed, 2 insertions(+), 1 deletion(-)
>   rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)
> 
> diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
> index 7ecef0519a27..d69356dc97df 100644
> --- a/arch/xtensa/lib/Makefile
> +++ b/arch/xtensa/lib/Makefile
> @@ -8,5 +8,4 @@ lib-y	+= memcopy.o memset.o checksum.o \
>   	   divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.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/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
> index 8cf70f068d92..86dd713d8855 100644
> --- a/kernel/kcsan/Makefile
> +++ b/kernel/kcsan/Makefile
> @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
>   	-fno-stack-protector -DDISABLE_BRANCH_PROFILING
>   
>   obj-y := core.o debugfs.o report.o
> +obj-$(CONFIG_PPC32) += stubs.o
> +obj-$(CONFIG_XTENSA) += stubs.o

Not sure it is acceptable to do it that way.

There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in 
KCSAN's Kconfig then PPC32 and XTENSA should select it.

>   
>   KCSAN_INSTRUMENT_BARRIERS_selftest.o := y
>   obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o
> diff --git a/arch/xtensa/lib/kcsan-stubs.c b/kernel/kcsan/stubs.c
> similarity index 100%
> rename from arch/xtensa/lib/kcsan-stubs.c
> rename to kernel/kcsan/stubs.c

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

* Re: [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support
  2023-02-16  5:09 ` [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support Rohan McLure
@ 2023-02-16  7:14   ` Christophe Leroy
  2023-02-16 23:23     ` Rohan McLure
  0 siblings, 1 reply; 9+ messages in thread
From: Christophe Leroy @ 2023-02-16  7:14 UTC (permalink / raw)
  To: Rohan McLure, linuxppc-dev@lists.ozlabs.org



Le 16/02/2023 à 06:09, Rohan McLure a écrit :
> Enable HAVE_ARCH_KCSAN on all powerpc platforms, permitting use of the
> kernel concurrency sanitiser through the CONFIG_KCSAN_* kconfig options.
> 
> Boots and passes selftests on 32-bit and 64-bit platforms. See
> documentation in Documentation/dev-tools/kcsan.rst for more information.
> 
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> New patch
> ---
>   arch/powerpc/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2c9cdf1d8761..45771448d47a 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -197,7 +197,7 @@ config PPC
>   	select HAVE_ARCH_KASAN			if PPC_RADIX_MMU
>   	select HAVE_ARCH_KASAN			if PPC_BOOK3E_64
>   	select HAVE_ARCH_KASAN_VMALLOC		if HAVE_ARCH_KASAN
> -	select HAVE_ARCH_KCSAN            	if PPC_BOOK3S_64
> +	select HAVE_ARCH_KCSAN

So that's a followup of a not yet posted version v5 of the other series ?
Why not just add patch 1 in that series and have KCSAN for all powerpc 
at once ?

>   	select HAVE_ARCH_KFENCE			if ARCH_SUPPORTS_DEBUG_PAGEALLOC
>   	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
>   	select HAVE_ARCH_KGDB

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

* Re: [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
  2023-02-16  7:12 ` [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems Christophe Leroy
@ 2023-02-16  8:09   ` Marco Elver
  2023-02-16 23:23     ` Rohan McLure
  0 siblings, 1 reply; 9+ messages in thread
From: Marco Elver @ 2023-02-16  8:09 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: kasan-dev, Max Filippov, Rohan McLure,
	linuxppc-dev@lists.ozlabs.org, Dmitry Vyukov

On Thu, Feb 16, 2023 at 07:12AM +0000, Christophe Leroy wrote:
> 
> 
> Le 16/02/2023 à 06:09, Rohan McLure a écrit :
> > KCSAN instruments calls to atomic builtins, and will in turn call these
> > builtins itself. As such, architectures supporting KCSAN must have
> > compiler support for these atomic primitives.
> > 
> > Since 32-bit systems are unlikely to have 64-bit compiler builtins,
> > provide a stub for each missing builtin, and use BUG() to assert
> > unreachability.
> > 
> > In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
> > locally. Move these definitions to be accessible to all 32-bit
> > architectures that do not provide the necessary builtins, with opt in
> > for PowerPC and xtensa.
> > 
> > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> > Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
> 
> This series should also be addressed to KCSAN Maintainers, shouldn't it ?
> 
> KCSAN
> M:	Marco Elver <elver@google.com>
> R:	Dmitry Vyukov <dvyukov@google.com>
> L:	kasan-dev@googlegroups.com
> S:	Maintained
> F:	Documentation/dev-tools/kcsan.rst
> F:	include/linux/kcsan*.h
> F:	kernel/kcsan/
> F:	lib/Kconfig.kcsan
> F:	scripts/Makefile.kcsan
> 
> 
> > ---
> > Previously issued as a part of a patch series adding KCSAN support to
> > 64-bit.
> > Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t
> > v1: Remove __has_builtin check, as gcc is not obligated to inline
> > builtins detected using this check, but instead is permitted to supply
> > them in libatomic:
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
> > Instead, opt-in PPC32 and xtensa.
> > ---
> >   arch/xtensa/lib/Makefile                              | 1 -
> >   kernel/kcsan/Makefile                                 | 2 ++
> >   arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
> >   3 files changed, 2 insertions(+), 1 deletion(-)
> >   rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)
> > 
> > diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
> > index 7ecef0519a27..d69356dc97df 100644
> > --- a/arch/xtensa/lib/Makefile
> > +++ b/arch/xtensa/lib/Makefile
> > @@ -8,5 +8,4 @@ lib-y	+= memcopy.o memset.o checksum.o \
> >   	   divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.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/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
> > index 8cf70f068d92..86dd713d8855 100644
> > --- a/kernel/kcsan/Makefile
> > +++ b/kernel/kcsan/Makefile
> > @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
> >   	-fno-stack-protector -DDISABLE_BRANCH_PROFILING
> >   
> >   obj-y := core.o debugfs.o report.o
> > +obj-$(CONFIG_PPC32) += stubs.o
> > +obj-$(CONFIG_XTENSA) += stubs.o
> 
> Not sure it is acceptable to do it that way.
> 
> There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in 
> KCSAN's Kconfig then PPC32 and XTENSA should select it.

The longer I think about it, since these stubs all BUG() anyway, perhaps
we ought to just avoid them altogether. If you delete all the stubs from
ppc and xtensa, but do this:

 | diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
 | index 54d077e1a2dc..8169d6dadd0e 100644
 | --- a/kernel/kcsan/core.c
 | +++ b/kernel/kcsan/core.c
 | @@ -1261,7 +1261,9 @@ static __always_inline void kcsan_atomic_builtin_memorder(int memorder)
 |  DEFINE_TSAN_ATOMIC_OPS(8);
 |  DEFINE_TSAN_ATOMIC_OPS(16);
 |  DEFINE_TSAN_ATOMIC_OPS(32);
 | +#ifdef CONFIG_64BIT
 |  DEFINE_TSAN_ATOMIC_OPS(64);
 | +#endif
 |  
 |  void __tsan_atomic_thread_fence(int memorder);
 |  void __tsan_atomic_thread_fence(int memorder)

Does that work?

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

* Re: [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support
  2023-02-16  7:14   ` Christophe Leroy
@ 2023-02-16 23:23     ` Rohan McLure
  2023-02-17  6:36       ` Christophe Leroy
  0 siblings, 1 reply; 9+ messages in thread
From: Rohan McLure @ 2023-02-16 23:23 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linuxppc-dev@lists.ozlabs.org

> On 16 Feb 2023, at 6:14 pm, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 16/02/2023 à 06:09, Rohan McLure a écrit :
>> Enable HAVE_ARCH_KCSAN on all powerpc platforms, permitting use of the
>> kernel concurrency sanitiser through the CONFIG_KCSAN_* kconfig options.
>> 
>> Boots and passes selftests on 32-bit and 64-bit platforms. See
>> documentation in Documentation/dev-tools/kcsan.rst for more information.
>> 
>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>> ---
>> New patch
>> ---
>>  arch/powerpc/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 2c9cdf1d8761..45771448d47a 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -197,7 +197,7 @@ config PPC
>>   select HAVE_ARCH_KASAN if PPC_RADIX_MMU
>>   select HAVE_ARCH_KASAN if PPC_BOOK3E_64
>>   select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN
>> - select HAVE_ARCH_KCSAN             if PPC_BOOK3S_64
>> + select HAVE_ARCH_KCSAN
> 
> So that's a followup of a not yet posted version v5 of the other series ?
> Why not just add patch 1 in that series and have KCSAN for all powerpc 
> at once ?

So the v3 was accepted upstream, likely to appear in 6.3. This patch series is
just to extend support to other platforms, once kcsan supports us.

Link: https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20230206021801.105268-1-rmclure@linux.ibm.com/

> 
>>   select HAVE_ARCH_KFENCE if ARCH_SUPPORTS_DEBUG_PAGEALLOC
>>   select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
>>   select HAVE_ARCH_KGDB



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

* Re: [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
  2023-02-16  8:09   ` Marco Elver
@ 2023-02-16 23:23     ` Rohan McLure
  0 siblings, 0 replies; 9+ messages in thread
From: Rohan McLure @ 2023-02-16 23:23 UTC (permalink / raw)
  To: Marco Elver
  Cc: Dmitry Vyukov, Max Filippov, linuxppc-dev@lists.ozlabs.org,
	kasan-dev

> On 16 Feb 2023, at 7:09 pm, Marco Elver <elver@google.com> wrote:
> 
> On Thu, Feb 16, 2023 at 07:12AM +0000, Christophe Leroy wrote:
>> 
>> 
>> Le 16/02/2023 à 06:09, Rohan McLure a écrit :
>>> KCSAN instruments calls to atomic builtins, and will in turn call these
>>> builtins itself. As such, architectures supporting KCSAN must have
>>> compiler support for these atomic primitives.
>>> 
>>> Since 32-bit systems are unlikely to have 64-bit compiler builtins,
>>> provide a stub for each missing builtin, and use BUG() to assert
>>> unreachability.
>>> 
>>> In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
>>> locally. Move these definitions to be accessible to all 32-bit
>>> architectures that do not provide the necessary builtins, with opt in
>>> for PowerPC and xtensa.
>>> 
>>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>>> Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
>> 
>> This series should also be addressed to KCSAN Maintainers, shouldn't it ?
>> 
>> KCSAN
>> M: Marco Elver <elver@google.com>
>> R: Dmitry Vyukov <dvyukov@google.com>
>> L: kasan-dev@googlegroups.com
>> S: Maintained
>> F: Documentation/dev-tools/kcsan.rst
>> F: include/linux/kcsan*.h
>> F: kernel/kcsan/
>> F: lib/Kconfig.kcsan
>> F: scripts/Makefile.kcsan
>> 
>> 
>>> ---
>>> Previously issued as a part of a patch series adding KCSAN support to
>>> 64-bit.
>>> Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t
>>> v1: Remove __has_builtin check, as gcc is not obligated to inline
>>> builtins detected using this check, but instead is permitted to supply
>>> them in libatomic:
>>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
>>> Instead, opt-in PPC32 and xtensa.
>>> ---
>>>  arch/xtensa/lib/Makefile                              | 1 -
>>>  kernel/kcsan/Makefile                                 | 2 ++
>>>  arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
>>>  3 files changed, 2 insertions(+), 1 deletion(-)
>>>  rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)
>>> 
>>> diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
>>> index 7ecef0519a27..d69356dc97df 100644
>>> --- a/arch/xtensa/lib/Makefile
>>> +++ b/arch/xtensa/lib/Makefile
>>> @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \
>>>      divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.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/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
>>> index 8cf70f068d92..86dd713d8855 100644
>>> --- a/kernel/kcsan/Makefile
>>> +++ b/kernel/kcsan/Makefile
>>> @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
>>>   -fno-stack-protector -DDISABLE_BRANCH_PROFILING
>>> 
>>>  obj-y := core.o debugfs.o report.o
>>> +obj-$(CONFIG_PPC32) += stubs.o
>>> +obj-$(CONFIG_XTENSA) += stubs.o
>> 
>> Not sure it is acceptable to do it that way.
>> 
>> There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in 
>> KCSAN's Kconfig then PPC32 and XTENSA should select it.
> 
> The longer I think about it, since these stubs all BUG() anyway, perhaps
> we ought to just avoid them altogether. If you delete all the stubs from
> ppc and xtensa, but do this:
> 
> | diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
> | index 54d077e1a2dc..8169d6dadd0e 100644
> | --- a/kernel/kcsan/core.c
> | +++ b/kernel/kcsan/core.c
> | @@ -1261,7 +1261,9 @@ static __always_inline void kcsan_atomic_builtin_memorder(int memorder)
> |  DEFINE_TSAN_ATOMIC_OPS(8);
> |  DEFINE_TSAN_ATOMIC_OPS(16);
> |  DEFINE_TSAN_ATOMIC_OPS(32);
> | +#ifdef CONFIG_64BIT
> |  DEFINE_TSAN_ATOMIC_OPS(64);
> | +#endif
> |  
> |  void __tsan_atomic_thread_fence(int memorder);
> |  void __tsan_atomic_thread_fence(int memorder)
> 
> Does that work?

This makes much more sense. Rather than assume that kcsan is the only
consumer of __atomic_*_8, and stubbing accordingly, we should just
remove its mention from relevant sub-archs.



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

* Re: [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support
  2023-02-16 23:23     ` Rohan McLure
@ 2023-02-17  6:36       ` Christophe Leroy
  2023-02-20  6:10         ` Michael Ellerman
  0 siblings, 1 reply; 9+ messages in thread
From: Christophe Leroy @ 2023-02-17  6:36 UTC (permalink / raw)
  To: Rohan McLure; +Cc: linuxppc-dev@lists.ozlabs.org



Le 17/02/2023 à 00:23, Rohan McLure a écrit :
>> On 16 Feb 2023, at 6:14 pm, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 16/02/2023 à 06:09, Rohan McLure a écrit :
>>> Enable HAVE_ARCH_KCSAN on all powerpc platforms, permitting use of the
>>> kernel concurrency sanitiser through the CONFIG_KCSAN_* kconfig options.
>>>
>>> Boots and passes selftests on 32-bit and 64-bit platforms. See
>>> documentation in Documentation/dev-tools/kcsan.rst for more information.
>>>
>>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>>> ---
>>> New patch
>>> ---
>>>   arch/powerpc/Kconfig | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index 2c9cdf1d8761..45771448d47a 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -197,7 +197,7 @@ config PPC
>>>    select HAVE_ARCH_KASAN if PPC_RADIX_MMU
>>>    select HAVE_ARCH_KASAN if PPC_BOOK3E_64
>>>    select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN
>>> - select HAVE_ARCH_KCSAN             if PPC_BOOK3S_64
>>> + select HAVE_ARCH_KCSAN
>>
>> So that's a followup of a not yet posted version v5 of the other series ?
>> Why not just add patch 1 in that series and have KCSAN for all powerpc
>> at once ?
> 
> So the v3 was accepted upstream, likely to appear in 6.3. This patch series is
> just to extend support to other platforms, once kcsan supports us.

Hum ... Ok.

I checked in checkpatch before writting that mail and saw that v4 was 
flagged "changes requested", so I didn't notice v3 was accepted.



> 
> Link: https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20230206021801.105268-1-rmclure@linux.ibm.com/
> 
>>
>>>    select HAVE_ARCH_KFENCE if ARCH_SUPPORTS_DEBUG_PAGEALLOC
>>>    select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
>>>    select HAVE_ARCH_KGDB
> 
> 

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

* Re: [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support
  2023-02-17  6:36       ` Christophe Leroy
@ 2023-02-20  6:10         ` Michael Ellerman
  0 siblings, 0 replies; 9+ messages in thread
From: Michael Ellerman @ 2023-02-20  6:10 UTC (permalink / raw)
  To: Christophe Leroy, Rohan McLure; +Cc: linuxppc-dev@lists.ozlabs.org

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 17/02/2023 à 00:23, Rohan McLure a écrit :
>>> On 16 Feb 2023, at 6:14 pm, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>> Le 16/02/2023 à 06:09, Rohan McLure a écrit :
>>>> Enable HAVE_ARCH_KCSAN on all powerpc platforms, permitting use of the
>>>> kernel concurrency sanitiser through the CONFIG_KCSAN_* kconfig options.
>>>>
>>>> Boots and passes selftests on 32-bit and 64-bit platforms. See
>>>> documentation in Documentation/dev-tools/kcsan.rst for more information.
>>>>
>>>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>>>> ---
>>>> New patch
>>>> ---
>>>>   arch/powerpc/Kconfig | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>>> index 2c9cdf1d8761..45771448d47a 100644
>>>> --- a/arch/powerpc/Kconfig
>>>> +++ b/arch/powerpc/Kconfig
>>>> @@ -197,7 +197,7 @@ config PPC
>>>>    select HAVE_ARCH_KASAN if PPC_RADIX_MMU
>>>>    select HAVE_ARCH_KASAN if PPC_BOOK3E_64
>>>>    select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN
>>>> - select HAVE_ARCH_KCSAN             if PPC_BOOK3S_64
>>>> + select HAVE_ARCH_KCSAN
>>>
>>> So that's a followup of a not yet posted version v5 of the other series ?
>>> Why not just add patch 1 in that series and have KCSAN for all powerpc
>>> at once ?
>> 
>> So the v3 was accepted upstream, likely to appear in 6.3. This patch series is
>> just to extend support to other platforms, once kcsan supports us.
>
> Hum ... Ok.
>
> I checked in checkpatch before writting that mail and saw that v4 was 
> flagged "changes requested", so I didn't notice v3 was accepted.

Sorry that's my fault. I talked to Rohan on chat and decided that taking
v3 was the best way to get something in before the merge window closed.

cheers

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

end of thread, other threads:[~2023-02-20  6:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-16  5:09 [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems Rohan McLure
2023-02-16  5:09 ` [PATCH 2/2] powerpc/{32,book3e}: kcsan: Extend KCSAN Support Rohan McLure
2023-02-16  7:14   ` Christophe Leroy
2023-02-16 23:23     ` Rohan McLure
2023-02-17  6:36       ` Christophe Leroy
2023-02-20  6:10         ` Michael Ellerman
2023-02-16  7:12 ` [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems Christophe Leroy
2023-02-16  8:09   ` Marco Elver
2023-02-16 23:23     ` Rohan McLure

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).