| Message ID | 20240714105848.1844400-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig | expand |
On 7/14/24 3:58 AM, Masahiro Yamada wrote: > The condition 'select HAVE_ARCH_KASAN' is always true because > there is 'select HAVE_ARCH_KASAN' statement above. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Randy Dunlap <rdunlap@infradead.org> Thanks. > --- > > arch/arm64/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index c87d16b12e9b..d37cbfc3031e 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -167,9 +167,9 @@ config ARM64 > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_JUMP_LABEL_RELATIVE > select HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > + select HAVE_ARCH_KASAN_VMALLOC > + select HAVE_ARCH_KASAN_SW_TAGS > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > # Some instrumentation may be unsound, hence EXPERT > select HAVE_ARCH_KCSAN if EXPERT > select HAVE_ARCH_KFENCE
On Sun, Jul 14, 2024 at 7:58 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > The condition 'select HAVE_ARCH_KASAN' is always true because > there is 'select HAVE_ARCH_KASAN' statement above. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > arch/arm64/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index c87d16b12e9b..d37cbfc3031e 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -167,9 +167,9 @@ config ARM64 > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_JUMP_LABEL_RELATIVE > select HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > + select HAVE_ARCH_KASAN_VMALLOC > + select HAVE_ARCH_KASAN_SW_TAGS > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > # Some instrumentation may be unsound, hence EXPERT > select HAVE_ARCH_KCSAN if EXPERT > select HAVE_ARCH_KFENCE > -- > 2.43.0 > I expect this patch will go through the arm64 tree. (I just sent it to the kbuild ML by mistake)
On Sun, Jul 14, 2024 at 7:58 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > The condition 'select HAVE_ARCH_KASAN' is always true because This is a typo. I meant, The condition 'if HAVE_ARCH_KASAN' is always true because > there is 'select HAVE_ARCH_KASAN' statement above. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > arch/arm64/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index c87d16b12e9b..d37cbfc3031e 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -167,9 +167,9 @@ config ARM64 > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_JUMP_LABEL_RELATIVE > select HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > + select HAVE_ARCH_KASAN_VMALLOC > + select HAVE_ARCH_KASAN_SW_TAGS > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > # Some instrumentation may be unsound, hence EXPERT > select HAVE_ARCH_KCSAN if EXPERT > select HAVE_ARCH_KFENCE > -- > 2.43.0 > -- Best Regards Masahiro Yamada
On 7/14/24 16:28, Masahiro Yamada wrote: > The condition 'select HAVE_ARCH_KASAN' is always true because > there is 'select HAVE_ARCH_KASAN' statement above. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > arch/arm64/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index c87d16b12e9b..d37cbfc3031e 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -167,9 +167,9 @@ config ARM64 > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_JUMP_LABEL_RELATIVE > select HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > + select HAVE_ARCH_KASAN_VMALLOC > + select HAVE_ARCH_KASAN_SW_TAGS > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > # Some instrumentation may be unsound, hence EXPERT > select HAVE_ARCH_KCSAN if EXPERT > select HAVE_ARCH_KFENCE There is another similar instance with HAVE_FUNCTION_GRAPH_TRACER as well. Just wondering if the following change should be folded in here ? --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -210,8 +210,8 @@ config ARM64 select HAVE_FTRACE_MCOUNT_RECORD select HAVE_FUNCTION_TRACER select HAVE_FUNCTION_ERROR_INJECTION - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_GRAPH_TRACER + select HAVE_FUNCTION_GRAPH_RETVAL select HAVE_GCC_PLUGINS select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && \ HW_PERF_EVENTS && HAVE_PERF_EVENTS_NMI
On Sun, Jul 14, 2024 at 07:58:46PM +0900, Masahiro Yamada wrote: > The condition 'select HAVE_ARCH_KASAN' is always true because > there is 'select HAVE_ARCH_KASAN' statement above. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Looks like we forgot to simplify this in commit: 0383808e4d99ac31 ("arm64: kasan: Reduce minimum shadow alignment and enable 5 level paging") ... where we began selecting HAVE_ARCH_KASAN unconditionally. FWIW: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > > arch/arm64/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index c87d16b12e9b..d37cbfc3031e 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -167,9 +167,9 @@ config ARM64 > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_JUMP_LABEL_RELATIVE > select HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > + select HAVE_ARCH_KASAN_VMALLOC > + select HAVE_ARCH_KASAN_SW_TAGS > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > # Some instrumentation may be unsound, hence EXPERT > select HAVE_ARCH_KCSAN if EXPERT > select HAVE_ARCH_KFENCE > -- > 2.43.0 > >
On Mon, Jul 15, 2024 at 08:11:08AM +0530, Anshuman Khandual wrote: > On 7/14/24 16:28, Masahiro Yamada wrote: > > The condition 'select HAVE_ARCH_KASAN' is always true because > > there is 'select HAVE_ARCH_KASAN' statement above. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > arch/arm64/Kconfig | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index c87d16b12e9b..d37cbfc3031e 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -167,9 +167,9 @@ config ARM64 > > select HAVE_ARCH_JUMP_LABEL > > select HAVE_ARCH_JUMP_LABEL_RELATIVE > > select HAVE_ARCH_KASAN > > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > > + select HAVE_ARCH_KASAN_VMALLOC > > + select HAVE_ARCH_KASAN_SW_TAGS > > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > > # Some instrumentation may be unsound, hence EXPERT > > select HAVE_ARCH_KCSAN if EXPERT > > select HAVE_ARCH_KFENCE > > There is another similar instance with HAVE_FUNCTION_GRAPH_TRACER as well. > Just wondering if the following change should be folded in here ? > > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -210,8 +210,8 @@ config ARM64 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_TRACER > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_RETVAL > select HAVE_GCC_PLUGINS > select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && \ > HW_PERF_EVENTS && HAVE_PERF_EVENTS_NMI That looks like a sensible cleanup, but I think it'd be better as a separate patch. It looks like that has always been redundant since it was introduced in commit 3646970322464c21 ("arm64: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL") ... would you mind spinning a patch? Mark.
On Mon, Jul 15, 2024 at 10:29 PM Mark Rutland <mark.rutland@arm.com> wrote: > > On Sun, Jul 14, 2024 at 07:58:46PM +0900, Masahiro Yamada wrote: > > The condition 'select HAVE_ARCH_KASAN' is always true because > > there is 'select HAVE_ARCH_KASAN' statement above. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Looks like we forgot to simplify this in commit: > > 0383808e4d99ac31 ("arm64: kasan: Reduce minimum shadow alignment and enable 5 level paging") > > ... where we began selecting HAVE_ARCH_KASAN unconditionally. Good point. This may be worth recording in the description. I will send v2 with a typo fixed. Thanks. > > FWIW: > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > Mark. > > > --- > > > > arch/arm64/Kconfig | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index c87d16b12e9b..d37cbfc3031e 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -167,9 +167,9 @@ config ARM64 > > select HAVE_ARCH_JUMP_LABEL > > select HAVE_ARCH_JUMP_LABEL_RELATIVE > > select HAVE_ARCH_KASAN > > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > > + select HAVE_ARCH_KASAN_VMALLOC > > + select HAVE_ARCH_KASAN_SW_TAGS > > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > > # Some instrumentation may be unsound, hence EXPERT > > select HAVE_ARCH_KCSAN if EXPERT > > select HAVE_ARCH_KFENCE > > -- > > 2.43.0 > > > > >
On Mon, Jul 15, 2024 at 10:44 PM Mark Rutland <mark.rutland@arm.com> wrote: > > On Mon, Jul 15, 2024 at 08:11:08AM +0530, Anshuman Khandual wrote: > > On 7/14/24 16:28, Masahiro Yamada wrote: > > > The condition 'select HAVE_ARCH_KASAN' is always true because > > > there is 'select HAVE_ARCH_KASAN' statement above. > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > --- > > > > > > arch/arm64/Kconfig | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > > index c87d16b12e9b..d37cbfc3031e 100644 > > > --- a/arch/arm64/Kconfig > > > +++ b/arch/arm64/Kconfig > > > @@ -167,9 +167,9 @@ config ARM64 > > > select HAVE_ARCH_JUMP_LABEL > > > select HAVE_ARCH_JUMP_LABEL_RELATIVE > > > select HAVE_ARCH_KASAN > > > - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > > > - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN > > > - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) > > > + select HAVE_ARCH_KASAN_VMALLOC > > > + select HAVE_ARCH_KASAN_SW_TAGS > > > + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE > > > # Some instrumentation may be unsound, hence EXPERT > > > select HAVE_ARCH_KCSAN if EXPERT > > > select HAVE_ARCH_KFENCE > > > > There is another similar instance with HAVE_FUNCTION_GRAPH_TRACER as well. > > Just wondering if the following change should be folded in here ? > > > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -210,8 +210,8 @@ config ARM64 > > select HAVE_FTRACE_MCOUNT_RECORD > > select HAVE_FUNCTION_TRACER > > select HAVE_FUNCTION_ERROR_INJECTION > > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > > select HAVE_FUNCTION_GRAPH_TRACER > > + select HAVE_FUNCTION_GRAPH_RETVAL > > select HAVE_GCC_PLUGINS > > select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && \ > > HW_PERF_EVENTS && HAVE_PERF_EVENTS_NMI > > That looks like a sensible cleanup, but I think it'd be better as a > separate patch. > > It looks like that has always been redundant since it was introduced in > commit > > 3646970322464c21 ("arm64: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL") > > ... would you mind spinning a patch? > > Mark. Agree. We can have a separate patch. I can see more redundant 'if' and 'depends on'. For example, KERNEL_MODE_NEON is always 'y'. config KERNEL_MODE_NEON def_bool y So, I can see redundancy in select ARCH_HAS_KERNEL_FPU_SUPPORT if KERNEL_MODE_NEON and config EFI bool "UEFI runtime support" depends on OF && !CPU_BIG_ENDIAN depends on KERNEL_MODE_NEON
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c87d16b12e9b..d37cbfc3031e 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -167,9 +167,9 @@ config ARM64 select HAVE_ARCH_JUMP_LABEL select HAVE_ARCH_JUMP_LABEL_RELATIVE select HAVE_ARCH_KASAN - select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN - select HAVE_ARCH_KASAN_SW_TAGS if HAVE_ARCH_KASAN - select HAVE_ARCH_KASAN_HW_TAGS if (HAVE_ARCH_KASAN && ARM64_MTE) + select HAVE_ARCH_KASAN_VMALLOC + select HAVE_ARCH_KASAN_SW_TAGS + select HAVE_ARCH_KASAN_HW_TAGS if ARM64_MTE # Some instrumentation may be unsound, hence EXPERT select HAVE_ARCH_KCSAN if EXPERT select HAVE_ARCH_KFENCE
The condition 'select HAVE_ARCH_KASAN' is always true because there is 'select HAVE_ARCH_KASAN' statement above. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- arch/arm64/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)