[PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Masahiro Yamada]

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

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Randy Dunlap]

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

-- 
~Randy

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Masahiro Yamada]

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)





-- 
Best Regards
Masahiro Yamada

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Masahiro Yamada]

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

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Anshuman Khandual]

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

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Mark Rutland]

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

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Mark Rutland]

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.

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Masahiro Yamada]

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


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] arm64: remove redundant 'if HAVE_ARCH_KASAN' in Kconfig

[Masahiro Yamada]

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




-- 
Best Regards
Masahiro Yamada