diff mbox series

[kconfig,RFC] getting things like CONFIG_ARCH_HAS_SYSCALL_WRAPPER without bothering with selects

Message ID YvLmPl8sgR2q3WgE@ZenIV (mailing list archive)
State New
Headers show
Series [kconfig,RFC] getting things like CONFIG_ARCH_HAS_SYSCALL_WRAPPER without bothering with selects | expand

Commit Message

Al Viro Aug. 9, 2022, 10:57 p.m. UTC
Sometimes we have situations when we want linux/foo.h
include asm/bar.h if the latter exists; one example is
includes of asm/syscall_wrapper.h.  Right now there are two
ways to do that:
	1) ARCH_HAS_BAR_H, explicitly selected by architectures
that have asm/bar.h and include guarded by #ifdef CONFIG_ARCH_HAS_BAR_H
	2) include/asm-default/bar.h, either empty or with
something like #define __ARCH_HAS_NO_BAR_H, with mandatory-y += bar.h
in include/asm-generic/Kbuild and unconditional includes of asm/bar.h,
possibly followed by #ifdef __ARCH_HAS_NO_BAR_H ... #endif

However, kconfig could do (1) without selects - something like
bool ARCH_HAS_BAR_H
	def_bool $(header-exists,bar.h)

Does anybody see problems with the patch below?

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---

Comments

Linus Torvalds Aug. 9, 2022, 11:52 p.m. UTC | #1
On Tue, Aug 9, 2022 at 3:57 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> However, kconfig could do (1) without selects - something like
> bool ARCH_HAS_BAR_H
>         def_bool $(header-exists,bar.h)
>
> Does anybody see problems with the patch below?

I like the concept, but I'm not entirely sure how generic it is. How
many of those "arch has this header" do we really end up having?

Just looking at some of the "ARCH_HAS_XYZ" options, very few strike me
as being about a header file existing.

I'm not saying you picked the _only_ such case, but it doesn't seem
_entirely_ impossible that you did..

Looking at some of the other cases, they might be more interested in
not "does it have a header file", but "can I grep for this symbol
name"?

And that kind of "do you expose symbol XYZ in header ABC" query might
work for this ARCH_HAS_SYSCALL_WRAPPER case too?

So that seems like it might potentially be a more general-purpose
Kconfig helper. Hmm?

                    Linus
Masahiro Yamada Aug. 10, 2022, 2:31 a.m. UTC | #2
On Wed, Aug 10, 2022 at 7:57 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>         Sometimes we have situations when we want linux/foo.h
> include asm/bar.h if the latter exists; one example is
> includes of asm/syscall_wrapper.h.  Right now there are two
> ways to do that:
>         1) ARCH_HAS_BAR_H, explicitly selected by architectures
> that have asm/bar.h and include guarded by #ifdef CONFIG_ARCH_HAS_BAR_H
>         2) include/asm-default/bar.h, either empty or with
> something like #define __ARCH_HAS_NO_BAR_H, with mandatory-y += bar.h
> in include/asm-generic/Kbuild and unconditional includes of asm/bar.h,
> possibly followed by #ifdef __ARCH_HAS_NO_BAR_H ... #endif
>
> However, kconfig could do (1) without selects - something like
> bool ARCH_HAS_BAR_H
>         def_bool $(header-exists,bar.h)
>
> Does anybody see problems with the patch below?




A problem is that Kconfig does not react to addition/removal
of asm/syscall_wrapper.h


For example, with the patch applied,
touch or rm arch/arm/include/asm/syscall_wrapper.h
and rebuild the arm kernel.


$ touch   arch/arm/include/asm/syscall_wrapper.h
$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-  -j8
  CALL    scripts/atomic/check-atomics.sh
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  Kernel: arch/arm/boot/Image is ready
  Kernel: arch/arm/boot/zImage is ready
$ rm     arch/arm/include/asm/syscall_wrapper.h
$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-  -j8
  CALL    scripts/atomic/check-atomics.sh
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  Kernel: arch/arm/boot/Image is ready
  Kernel: arch/arm/boot/zImage is ready


When the header is added or removed, the kernel must be
rebuilt with CONFIG_ARCH_HAS_SYSCALL_WRAPPER updated,
but Kconfig is actually not invoked at all.


The condition for triggering Kconfig is
generated in include/config/auto.conf.cmd
but I do not know how to handle this case correctly.




I think it is better to not have Kconfig think too much.
We do several compiler tests in Kconfig, but
that is because otherwise we cannot know
users' build environments.

We know everything in the source tree.
"test -e arch/$(ARCH)/include/asm/syscall_wrapper.h"
does not seem sensible to me.



Masahiro Yamada








>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1652a9800ebe..277437f329ce 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -42,7 +42,6 @@ config ARM64
>         select ARCH_HAS_STRICT_MODULE_RWX
>         select ARCH_HAS_SYNC_DMA_FOR_DEVICE
>         select ARCH_HAS_SYNC_DMA_FOR_CPU
> -       select ARCH_HAS_SYSCALL_WRAPPER
>         select ARCH_HAS_TEARDOWN_DMA_OPS if IOMMU_SUPPORT
>         select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>         select ARCH_HAS_VM_GET_PAGE_PROT
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 91c0b80a8bf0..86e905b8462c 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -78,7 +78,6 @@ config S390
>         select ARCH_HAS_SET_MEMORY
>         select ARCH_HAS_STRICT_KERNEL_RWX
>         select ARCH_HAS_STRICT_MODULE_RWX
> -       select ARCH_HAS_SYSCALL_WRAPPER
>         select ARCH_HAS_UBSAN_SANITIZE_ALL
>         select ARCH_HAS_VDSO_DATA
>         select ARCH_HAVE_NMI_SAFE_CMPXCHG
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index be0b95e51df6..64592d027a0d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -92,7 +92,6 @@ config X86
>         select ARCH_HAS_STRICT_KERNEL_RWX
>         select ARCH_HAS_STRICT_MODULE_RWX
>         select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
> -       select ARCH_HAS_SYSCALL_WRAPPER
>         select ARCH_HAS_UBSAN_SANITIZE_ALL
>         select ARCH_HAS_VM_GET_PAGE_PROT
>         select ARCH_HAS_DEBUG_WX
> diff --git a/init/Kconfig b/init/Kconfig
> index c7900e8975f1..ce88397c4e46 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2257,4 +2257,4 @@ config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
>  # kernel/time/posix-stubs.c. All these overrides need to be available in
>  # <asm/syscall_wrapper.h>.
>  config ARCH_HAS_SYSCALL_WRAPPER
> -       def_bool n
> +       def_bool $(header-exists,syscall_wrapper.h)
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 0496efd6e117..465bb836f82a 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -23,6 +23,10 @@ success = $(if-success,$(1),y,n)
>  # Return n if <command> exits with 0, y otherwise
>  failure = $(if-success,$(1),n,y)
>
> +# $(header-exists,<header>)
> +# Return y if arch/$(ARCH)/include/asm/<header> exists, n otherwise
> +header-exists = $(success,test -e $(srctree)/arch/$(ARCH)/include/asm/$(1))
> +
>  # $(cc-option,<flag>)
>  # Return y if the compiler supports <flag>, n otherwise
>  cc-option = $(success,mkdir .tmp_$$$$; trap "rm -rf .tmp_$$$$" EXIT; $(CC) -Werror $(CLANG_FLAGS) $(1) -c -x c /dev/null -o .tmp_$$$$/tmp.o)
diff mbox series

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1652a9800ebe..277437f329ce 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -42,7 +42,6 @@  config ARM64
 	select ARCH_HAS_STRICT_MODULE_RWX
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
 	select ARCH_HAS_SYNC_DMA_FOR_CPU
-	select ARCH_HAS_SYSCALL_WRAPPER
 	select ARCH_HAS_TEARDOWN_DMA_OPS if IOMMU_SUPPORT
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAS_VM_GET_PAGE_PROT
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 91c0b80a8bf0..86e905b8462c 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -78,7 +78,6 @@  config S390
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_STRICT_MODULE_RWX
-	select ARCH_HAS_SYSCALL_WRAPPER
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_HAS_VDSO_DATA
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e51df6..64592d027a0d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -92,7 +92,6 @@  config X86
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_STRICT_MODULE_RWX
 	select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
-	select ARCH_HAS_SYSCALL_WRAPPER
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_HAS_VM_GET_PAGE_PROT
 	select ARCH_HAS_DEBUG_WX
diff --git a/init/Kconfig b/init/Kconfig
index c7900e8975f1..ce88397c4e46 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2257,4 +2257,4 @@  config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
 # kernel/time/posix-stubs.c. All these overrides need to be available in
 # <asm/syscall_wrapper.h>.
 config ARCH_HAS_SYSCALL_WRAPPER
-	def_bool n
+	def_bool $(header-exists,syscall_wrapper.h)
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 0496efd6e117..465bb836f82a 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -23,6 +23,10 @@  success = $(if-success,$(1),y,n)
 # Return n if <command> exits with 0, y otherwise
 failure = $(if-success,$(1),n,y)
 
+# $(header-exists,<header>)
+# Return y if arch/$(ARCH)/include/asm/<header> exists, n otherwise
+header-exists = $(success,test -e $(srctree)/arch/$(ARCH)/include/asm/$(1))
+
 # $(cc-option,<flag>)
 # Return y if the compiler supports <flag>, n otherwise
 cc-option = $(success,mkdir .tmp_$$$$; trap "rm -rf .tmp_$$$$" EXIT; $(CC) -Werror $(CLANG_FLAGS) $(1) -c -x c /dev/null -o .tmp_$$$$/tmp.o)