* [PATCH] arm64: do not descend to vdso directories twice
@ 2020-12-18 2:45 ` Masahiro Yamada
0 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-12-18 2:45 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, linux-arm-kernel
Cc: Michael Ellerman, Masahiro Yamada, linux-kernel
arm64 descends into each vdso directory twice; first in vdso_prepare,
second during the ordinary build process.
PPC mimicked it and uncovered a problem [1]. In the first descend,
Kbuild directly visits the vdso directories, therefore it does not
inherit subdir-ccflags-y from upper directories.
This means the command line parameters may differ between the two.
If it happens, the offset values in the generated headers might be
different from real offsets of vdso.so in the kernel.
This potential danger should be avoided. The vdso directories are
built in the vdso_prepare stage, so the second descend is unneeded.
[1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
arch/arm64/Makefile | 10 ++++++----
arch/arm64/kernel/Makefile | 5 +++--
arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
arch/arm64/kernel/vdso/Makefile | 1 -
arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
arch/arm64/kernel/vdso32/Makefile | 1 -
6 files changed, 9 insertions(+), 8 deletions(-)
rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 6a87d592bd00..f18d20a68170 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
# this hack.
prepare: vdso_prepare
vdso_prepare: prepare0
- $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
- $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
- $(build)=arch/arm64/kernel/vdso32 \
- include/generated/vdso32-offsets.h)
+ $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
+ include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
+ifdef CONFIG_COMPAT_VDSO
+ $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
+ include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
+endif
endif
define archhelp
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 86364ab6f13f..42f6ad2c7eac 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
obj-$(CONFIG_ARM64_MTE) += mte.o
+obj-y += vdso-wrap.o
+obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
-obj-y += vdso/ probes/
-obj-$(CONFIG_COMPAT_VDSO) += vdso32/
+obj-y += probes/
head-y := head.o
extra-y += $(head-y) vmlinux.lds
diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
similarity index 100%
rename from arch/arm64/kernel/vdso/vdso.S
rename to arch/arm64/kernel/vdso-wrap.S
diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index a8f8e409e2bf..85222f64f394 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -45,7 +45,6 @@ endif
# Disable gcov profiling for VDSO code
GCOV_PROFILE := n
-obj-y += vdso.o
targets += vdso.lds
CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
similarity index 100%
rename from arch/arm64/kernel/vdso32/vdso.S
rename to arch/arm64/kernel/vdso32-wrap.S
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index a1e0f91e6cea..789ad420f16b 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
-obj-y += vdso.o
targets += vdso.lds
CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] arm64: do not descend to vdso directories twice
@ 2020-12-18 2:45 ` Masahiro Yamada
0 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-12-18 2:45 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, linux-arm-kernel
Cc: Michael Ellerman, Masahiro Yamada, linux-kernel
arm64 descends into each vdso directory twice; first in vdso_prepare,
second during the ordinary build process.
PPC mimicked it and uncovered a problem [1]. In the first descend,
Kbuild directly visits the vdso directories, therefore it does not
inherit subdir-ccflags-y from upper directories.
This means the command line parameters may differ between the two.
If it happens, the offset values in the generated headers might be
different from real offsets of vdso.so in the kernel.
This potential danger should be avoided. The vdso directories are
built in the vdso_prepare stage, so the second descend is unneeded.
[1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
arch/arm64/Makefile | 10 ++++++----
arch/arm64/kernel/Makefile | 5 +++--
arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
arch/arm64/kernel/vdso/Makefile | 1 -
arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
arch/arm64/kernel/vdso32/Makefile | 1 -
6 files changed, 9 insertions(+), 8 deletions(-)
rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 6a87d592bd00..f18d20a68170 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
# this hack.
prepare: vdso_prepare
vdso_prepare: prepare0
- $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
- $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
- $(build)=arch/arm64/kernel/vdso32 \
- include/generated/vdso32-offsets.h)
+ $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
+ include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
+ifdef CONFIG_COMPAT_VDSO
+ $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
+ include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
+endif
endif
define archhelp
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 86364ab6f13f..42f6ad2c7eac 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
obj-$(CONFIG_ARM64_MTE) += mte.o
+obj-y += vdso-wrap.o
+obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
-obj-y += vdso/ probes/
-obj-$(CONFIG_COMPAT_VDSO) += vdso32/
+obj-y += probes/
head-y := head.o
extra-y += $(head-y) vmlinux.lds
diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
similarity index 100%
rename from arch/arm64/kernel/vdso/vdso.S
rename to arch/arm64/kernel/vdso-wrap.S
diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index a8f8e409e2bf..85222f64f394 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -45,7 +45,6 @@ endif
# Disable gcov profiling for VDSO code
GCOV_PROFILE := n
-obj-y += vdso.o
targets += vdso.lds
CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
similarity index 100%
rename from arch/arm64/kernel/vdso32/vdso.S
rename to arch/arm64/kernel/vdso32-wrap.S
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index a1e0f91e6cea..789ad420f16b 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
-obj-y += vdso.o
targets += vdso.lds
CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
--
2.27.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
2020-12-18 2:45 ` Masahiro Yamada
@ 2020-12-21 14:39 ` Vincenzo Frascino
-1 siblings, 0 replies; 10+ messages in thread
From: Vincenzo Frascino @ 2020-12-21 14:39 UTC (permalink / raw)
To: Masahiro Yamada, Catalin Marinas, Will Deacon, linux-arm-kernel
Cc: Michael Ellerman, linux-kernel
Hi Masahiro,
On 12/18/20 2:45 AM, Masahiro Yamada wrote:
> arm64 descends into each vdso directory twice; first in vdso_prepare,
> second during the ordinary build process.
>
> PPC mimicked it and uncovered a problem [1]. In the first descend,
> Kbuild directly visits the vdso directories, therefore it does not
> inherit subdir-ccflags-y from upper directories.
>
> This means the command line parameters may differ between the two.
> If it happens, the offset values in the generated headers might be
> different from real offsets of vdso.so in the kernel.
>
> This potential danger should be avoided. The vdso directories are
> built in the vdso_prepare stage, so the second descend is unneeded.
>
> [1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
>
I could not reproduce the problem you are reporting on arm64. Could you please
provide some steps?
In my case the vDSO library is not rebuilt as a result of the procedure reported
in the email you linked at [1].
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> arch/arm64/Makefile | 10 ++++++----
> arch/arm64/kernel/Makefile | 5 +++--
> arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
> arch/arm64/kernel/vdso/Makefile | 1 -
> arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
> arch/arm64/kernel/vdso32/Makefile | 1 -
> 6 files changed, 9 insertions(+), 8 deletions(-)
> rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
> rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 6a87d592bd00..f18d20a68170 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
> # this hack.
> prepare: vdso_prepare
> vdso_prepare: prepare0
> - $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
> - $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
> - $(build)=arch/arm64/kernel/vdso32 \
> - include/generated/vdso32-offsets.h)
> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
> + include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
> +ifdef CONFIG_COMPAT_VDSO
> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
> + include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
> +endif
> endif
The reason why it is currently done in two phases (a bit hacky as per comment)
is because vdso-offsets.h is required to be generated before compiling kernel/.
Please refer to the comment in arch/arm64/Makefile.
Could you explain how your change satisfies the dependency?
>
> define archhelp
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 86364ab6f13f..42f6ad2c7eac 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
> obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
> obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
> obj-$(CONFIG_ARM64_MTE) += mte.o
> +obj-y += vdso-wrap.o
> +obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
>
> -obj-y += vdso/ probes/
> -obj-$(CONFIG_COMPAT_VDSO) += vdso32/
> +obj-y += probes/
> head-y := head.o
> extra-y += $(head-y) vmlinux.lds
>
> diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
> similarity index 100%
> rename from arch/arm64/kernel/vdso/vdso.S
> rename to arch/arm64/kernel/vdso-wrap.S
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index a8f8e409e2bf..85222f64f394 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -45,7 +45,6 @@ endif
> # Disable gcov profiling for VDSO code
> GCOV_PROFILE := n
>
> -obj-y += vdso.o
> targets += vdso.lds
> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>
> diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
> similarity index 100%
> rename from arch/arm64/kernel/vdso32/vdso.S
> rename to arch/arm64/kernel/vdso32-wrap.S
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index a1e0f91e6cea..789ad420f16b 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
> asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
> obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
>
> -obj-y += vdso.o
> targets += vdso.lds
> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>
>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
@ 2020-12-21 14:39 ` Vincenzo Frascino
0 siblings, 0 replies; 10+ messages in thread
From: Vincenzo Frascino @ 2020-12-21 14:39 UTC (permalink / raw)
To: Masahiro Yamada, Catalin Marinas, Will Deacon, linux-arm-kernel
Cc: Michael Ellerman, linux-kernel
Hi Masahiro,
On 12/18/20 2:45 AM, Masahiro Yamada wrote:
> arm64 descends into each vdso directory twice; first in vdso_prepare,
> second during the ordinary build process.
>
> PPC mimicked it and uncovered a problem [1]. In the first descend,
> Kbuild directly visits the vdso directories, therefore it does not
> inherit subdir-ccflags-y from upper directories.
>
> This means the command line parameters may differ between the two.
> If it happens, the offset values in the generated headers might be
> different from real offsets of vdso.so in the kernel.
>
> This potential danger should be avoided. The vdso directories are
> built in the vdso_prepare stage, so the second descend is unneeded.
>
> [1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
>
I could not reproduce the problem you are reporting on arm64. Could you please
provide some steps?
In my case the vDSO library is not rebuilt as a result of the procedure reported
in the email you linked at [1].
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> arch/arm64/Makefile | 10 ++++++----
> arch/arm64/kernel/Makefile | 5 +++--
> arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
> arch/arm64/kernel/vdso/Makefile | 1 -
> arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
> arch/arm64/kernel/vdso32/Makefile | 1 -
> 6 files changed, 9 insertions(+), 8 deletions(-)
> rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
> rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 6a87d592bd00..f18d20a68170 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
> # this hack.
> prepare: vdso_prepare
> vdso_prepare: prepare0
> - $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
> - $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
> - $(build)=arch/arm64/kernel/vdso32 \
> - include/generated/vdso32-offsets.h)
> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
> + include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
> +ifdef CONFIG_COMPAT_VDSO
> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
> + include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
> +endif
> endif
The reason why it is currently done in two phases (a bit hacky as per comment)
is because vdso-offsets.h is required to be generated before compiling kernel/.
Please refer to the comment in arch/arm64/Makefile.
Could you explain how your change satisfies the dependency?
>
> define archhelp
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 86364ab6f13f..42f6ad2c7eac 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
> obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
> obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
> obj-$(CONFIG_ARM64_MTE) += mte.o
> +obj-y += vdso-wrap.o
> +obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
>
> -obj-y += vdso/ probes/
> -obj-$(CONFIG_COMPAT_VDSO) += vdso32/
> +obj-y += probes/
> head-y := head.o
> extra-y += $(head-y) vmlinux.lds
>
> diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
> similarity index 100%
> rename from arch/arm64/kernel/vdso/vdso.S
> rename to arch/arm64/kernel/vdso-wrap.S
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index a8f8e409e2bf..85222f64f394 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -45,7 +45,6 @@ endif
> # Disable gcov profiling for VDSO code
> GCOV_PROFILE := n
>
> -obj-y += vdso.o
> targets += vdso.lds
> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>
> diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
> similarity index 100%
> rename from arch/arm64/kernel/vdso32/vdso.S
> rename to arch/arm64/kernel/vdso32-wrap.S
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index a1e0f91e6cea..789ad420f16b 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
> asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
> obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
>
> -obj-y += vdso.o
> targets += vdso.lds
> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>
>
--
Regards,
Vincenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
2020-12-21 14:39 ` Vincenzo Frascino
@ 2020-12-21 16:03 ` Masahiro Yamada
-1 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-12-21 16:03 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Michael Ellerman,
Linux Kernel Mailing List
On Mon, Dec 21, 2020 at 11:36 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Masahiro,
>
> On 12/18/20 2:45 AM, Masahiro Yamada wrote:
> > arm64 descends into each vdso directory twice; first in vdso_prepare,
> > second during the ordinary build process.
> >
> > PPC mimicked it and uncovered a problem [1]. In the first descend,
> > Kbuild directly visits the vdso directories, therefore it does not
> > inherit subdir-ccflags-y from upper directories.
> >
> > This means the command line parameters may differ between the two.
> > If it happens, the offset values in the generated headers might be
> > different from real offsets of vdso.so in the kernel.
> >
> > This potential danger should be avoided. The vdso directories are
> > built in the vdso_prepare stage, so the second descend is unneeded.
> >
> > [1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
> >
>
> I could not reproduce the problem you are reporting on arm64. Could you please
> provide some steps?
As far as I see in arm64 Makefiles, you cannot reproduce it.
So, this is a _potential_ problem.
When somebody adds subdir-ccflags-y to arch/arm64/Kbuild or
arch/arm64/kernel/Makefile,
the real issue will suddenly come up.
arm64 is the first arch that implemented vdso-offsets.h
arch maintainers tend to copy Makefiles from existing ones.
Now PPC is the second with almost the same Makefile implementation.
This time, powerpc hit the problem because arch/powerpc/Kbuild
has subdir-ccflags-y.
I do not want to be bothered every time arch maintainers hit this issue.
So, I want to fix arm64 first,
then powerpc will eventually copy the correct Makefiles.
> In my case the vDSO library is not rebuilt as a result of the procedure reported
> in the email you linked at [1].
>
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > arch/arm64/Makefile | 10 ++++++----
> > arch/arm64/kernel/Makefile | 5 +++--
> > arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
> > arch/arm64/kernel/vdso/Makefile | 1 -
> > arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
> > arch/arm64/kernel/vdso32/Makefile | 1 -
> > 6 files changed, 9 insertions(+), 8 deletions(-)
> > rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
> > rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
> >
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 6a87d592bd00..f18d20a68170 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
> > # this hack.
> > prepare: vdso_prepare
> > vdso_prepare: prepare0
> > - $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
> > - $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
> > - $(build)=arch/arm64/kernel/vdso32 \
> > - include/generated/vdso32-offsets.h)
> > + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
> > + include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
> > +ifdef CONFIG_COMPAT_VDSO
> > + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
> > + include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
> > +endif
> > endif
>
> The reason why it is currently done in two phases (a bit hacky as per comment)
> is because vdso-offsets.h is required to be generated before compiling kernel/.
> Please refer to the comment in arch/arm64/Makefile.
Yes, I know.
>
> Could you explain how your change satisfies the dependency?
vdso_prepare completes before Kbuild starts ordinary descending.
So *.so files and vdso-offsets.h exist
when Kbuild builds arch/arm64/kernel/.
> >
> > define archhelp
> > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> > index 86364ab6f13f..42f6ad2c7eac 100644
> > --- a/arch/arm64/kernel/Makefile
> > +++ b/arch/arm64/kernel/Makefile
> > @@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
> > obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
> > obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
> > obj-$(CONFIG_ARM64_MTE) += mte.o
> > +obj-y += vdso-wrap.o
> > +obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
> >
> > -obj-y += vdso/ probes/
> > -obj-$(CONFIG_COMPAT_VDSO) += vdso32/
> > +obj-y += probes/
> > head-y := head.o
> > extra-y += $(head-y) vmlinux.lds
> >
> > diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
> > similarity index 100%
> > rename from arch/arm64/kernel/vdso/vdso.S
> > rename to arch/arm64/kernel/vdso-wrap.S
> > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> > index a8f8e409e2bf..85222f64f394 100644
> > --- a/arch/arm64/kernel/vdso/Makefile
> > +++ b/arch/arm64/kernel/vdso/Makefile
> > @@ -45,7 +45,6 @@ endif
> > # Disable gcov profiling for VDSO code
> > GCOV_PROFILE := n
> >
> > -obj-y += vdso.o
> > targets += vdso.lds
> > CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
> >
> > diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
> > similarity index 100%
> > rename from arch/arm64/kernel/vdso32/vdso.S
> > rename to arch/arm64/kernel/vdso32-wrap.S
> > diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> > index a1e0f91e6cea..789ad420f16b 100644
> > --- a/arch/arm64/kernel/vdso32/Makefile
> > +++ b/arch/arm64/kernel/vdso32/Makefile
> > @@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
> > asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
> > obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
> >
> > -obj-y += vdso.o
> > targets += vdso.lds
> > CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
> >
> >
>
> --
> Regards,
> Vincenzo
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
@ 2020-12-21 16:03 ` Masahiro Yamada
0 siblings, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2020-12-21 16:03 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: Catalin Marinas, Will Deacon, Linux Kernel Mailing List,
linux-arm-kernel, Michael Ellerman
On Mon, Dec 21, 2020 at 11:36 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Masahiro,
>
> On 12/18/20 2:45 AM, Masahiro Yamada wrote:
> > arm64 descends into each vdso directory twice; first in vdso_prepare,
> > second during the ordinary build process.
> >
> > PPC mimicked it and uncovered a problem [1]. In the first descend,
> > Kbuild directly visits the vdso directories, therefore it does not
> > inherit subdir-ccflags-y from upper directories.
> >
> > This means the command line parameters may differ between the two.
> > If it happens, the offset values in the generated headers might be
> > different from real offsets of vdso.so in the kernel.
> >
> > This potential danger should be avoided. The vdso directories are
> > built in the vdso_prepare stage, so the second descend is unneeded.
> >
> > [1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
> >
>
> I could not reproduce the problem you are reporting on arm64. Could you please
> provide some steps?
As far as I see in arm64 Makefiles, you cannot reproduce it.
So, this is a _potential_ problem.
When somebody adds subdir-ccflags-y to arch/arm64/Kbuild or
arch/arm64/kernel/Makefile,
the real issue will suddenly come up.
arm64 is the first arch that implemented vdso-offsets.h
arch maintainers tend to copy Makefiles from existing ones.
Now PPC is the second with almost the same Makefile implementation.
This time, powerpc hit the problem because arch/powerpc/Kbuild
has subdir-ccflags-y.
I do not want to be bothered every time arch maintainers hit this issue.
So, I want to fix arm64 first,
then powerpc will eventually copy the correct Makefiles.
> In my case the vDSO library is not rebuilt as a result of the procedure reported
> in the email you linked at [1].
>
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > arch/arm64/Makefile | 10 ++++++----
> > arch/arm64/kernel/Makefile | 5 +++--
> > arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
> > arch/arm64/kernel/vdso/Makefile | 1 -
> > arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
> > arch/arm64/kernel/vdso32/Makefile | 1 -
> > 6 files changed, 9 insertions(+), 8 deletions(-)
> > rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
> > rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
> >
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 6a87d592bd00..f18d20a68170 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
> > # this hack.
> > prepare: vdso_prepare
> > vdso_prepare: prepare0
> > - $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
> > - $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
> > - $(build)=arch/arm64/kernel/vdso32 \
> > - include/generated/vdso32-offsets.h)
> > + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
> > + include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
> > +ifdef CONFIG_COMPAT_VDSO
> > + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
> > + include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
> > +endif
> > endif
>
> The reason why it is currently done in two phases (a bit hacky as per comment)
> is because vdso-offsets.h is required to be generated before compiling kernel/.
> Please refer to the comment in arch/arm64/Makefile.
Yes, I know.
>
> Could you explain how your change satisfies the dependency?
vdso_prepare completes before Kbuild starts ordinary descending.
So *.so files and vdso-offsets.h exist
when Kbuild builds arch/arm64/kernel/.
> >
> > define archhelp
> > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> > index 86364ab6f13f..42f6ad2c7eac 100644
> > --- a/arch/arm64/kernel/Makefile
> > +++ b/arch/arm64/kernel/Makefile
> > @@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
> > obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
> > obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
> > obj-$(CONFIG_ARM64_MTE) += mte.o
> > +obj-y += vdso-wrap.o
> > +obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
> >
> > -obj-y += vdso/ probes/
> > -obj-$(CONFIG_COMPAT_VDSO) += vdso32/
> > +obj-y += probes/
> > head-y := head.o
> > extra-y += $(head-y) vmlinux.lds
> >
> > diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
> > similarity index 100%
> > rename from arch/arm64/kernel/vdso/vdso.S
> > rename to arch/arm64/kernel/vdso-wrap.S
> > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> > index a8f8e409e2bf..85222f64f394 100644
> > --- a/arch/arm64/kernel/vdso/Makefile
> > +++ b/arch/arm64/kernel/vdso/Makefile
> > @@ -45,7 +45,6 @@ endif
> > # Disable gcov profiling for VDSO code
> > GCOV_PROFILE := n
> >
> > -obj-y += vdso.o
> > targets += vdso.lds
> > CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
> >
> > diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
> > similarity index 100%
> > rename from arch/arm64/kernel/vdso32/vdso.S
> > rename to arch/arm64/kernel/vdso32-wrap.S
> > diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> > index a1e0f91e6cea..789ad420f16b 100644
> > --- a/arch/arm64/kernel/vdso32/Makefile
> > +++ b/arch/arm64/kernel/vdso32/Makefile
> > @@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
> > asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
> > obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
> >
> > -obj-y += vdso.o
> > targets += vdso.lds
> > CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
> >
> >
>
> --
> Regards,
> Vincenzo
--
Best Regards
Masahiro Yamada
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
2020-12-21 16:03 ` Masahiro Yamada
@ 2020-12-22 14:34 ` Vincenzo Frascino
-1 siblings, 0 replies; 10+ messages in thread
From: Vincenzo Frascino @ 2020-12-22 14:34 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Michael Ellerman,
Linux Kernel Mailing List
On 12/21/20 4:03 PM, Masahiro Yamada wrote:
> On Mon, Dec 21, 2020 at 11:36 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> Hi Masahiro,
>>
>> On 12/18/20 2:45 AM, Masahiro Yamada wrote:
>>> arm64 descends into each vdso directory twice; first in vdso_prepare,
>>> second during the ordinary build process.
>>>
>>> PPC mimicked it and uncovered a problem [1]. In the first descend,
>>> Kbuild directly visits the vdso directories, therefore it does not
>>> inherit subdir-ccflags-y from upper directories.
>>>
>>> This means the command line parameters may differ between the two.
>>> If it happens, the offset values in the generated headers might be
>>> different from real offsets of vdso.so in the kernel.
>>>
>>> This potential danger should be avoided. The vdso directories are
>>> built in the vdso_prepare stage, so the second descend is unneeded.
>>>
>>> [1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
>>>
>>
>> I could not reproduce the problem you are reporting on arm64. Could you please
>> provide some steps?
>
>
> As far as I see in arm64 Makefiles, you cannot reproduce it.
> So, this is a _potential_ problem.
>
> When somebody adds subdir-ccflags-y to arch/arm64/Kbuild or
> arch/arm64/kernel/Makefile,
> the real issue will suddenly come up.
>
>
> arm64 is the first arch that implemented vdso-offsets.h
>
> arch maintainers tend to copy Makefiles from existing ones.
>
> Now PPC is the second with almost the same Makefile implementation.
>
> This time, powerpc hit the problem because arch/powerpc/Kbuild
> has subdir-ccflags-y.
>
> I do not want to be bothered every time arch maintainers hit this issue.
> So, I want to fix arm64 first,
> then powerpc will eventually copy the correct Makefiles.
>
>
Ok this seems a problem related to a top down approach in the ccflags
propagation, which as it stands does not affect arm64, hence IMHO we should not
fix what is not broken, but I do not have a strong opinion on this.
>
>
>> In my case the vDSO library is not rebuilt as a result of the procedure reported
>> in the email you linked at [1].
>>
>>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>>> ---
>>>
>>> arch/arm64/Makefile | 10 ++++++----
>>> arch/arm64/kernel/Makefile | 5 +++--
>>> arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
>>> arch/arm64/kernel/vdso/Makefile | 1 -
>>> arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
>>> arch/arm64/kernel/vdso32/Makefile | 1 -
>>> 6 files changed, 9 insertions(+), 8 deletions(-)
>>> rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
>>> rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
>>>
>>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
>>> index 6a87d592bd00..f18d20a68170 100644
>>> --- a/arch/arm64/Makefile
>>> +++ b/arch/arm64/Makefile
>>> @@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
>>> # this hack.
>>> prepare: vdso_prepare
>>> vdso_prepare: prepare0
>>> - $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
>>> - $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
>>> - $(build)=arch/arm64/kernel/vdso32 \
>>> - include/generated/vdso32-offsets.h)
>>> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
>>> + include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
>>> +ifdef CONFIG_COMPAT_VDSO
>>> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
>>> + include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
>>> +endif
>>> endif
>>
>> The reason why it is currently done in two phases (a bit hacky as per comment)
>> is because vdso-offsets.h is required to be generated before compiling kernel/.
>> Please refer to the comment in arch/arm64/Makefile.
>
> Yes, I know.
>
>>
>> Could you explain how your change satisfies the dependency?
>
> vdso_prepare completes before Kbuild starts ordinary descending.
> So *.so files and vdso-offsets.h exist
> when Kbuild builds arch/arm64/kernel/.
>
>
This is not immediately obvious, could you please mention it in the commit message?
Nit: Could you please re-organize the code as follows in order to limit the
changes (untested)?
$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h \
arch/arm64/kernel/vdso/vdso.so
$(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
$(build)=arch/arm64/kernel/vdso32 \
include/generated/vdso32-offsets.h \
arch/arm64/kernel/vdso32/vdso.so)
>
>
>
>
>>>
>>> define archhelp
>>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>>> index 86364ab6f13f..42f6ad2c7eac 100644
>>> --- a/arch/arm64/kernel/Makefile
>>> +++ b/arch/arm64/kernel/Makefile
>>> @@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
>>> obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
>>> obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
>>> obj-$(CONFIG_ARM64_MTE) += mte.o
>>> +obj-y += vdso-wrap.o
>>> +obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
>>>
>>> -obj-y += vdso/ probes/
>>> -obj-$(CONFIG_COMPAT_VDSO) += vdso32/
>>> +obj-y += probes/
>>> head-y := head.o
>>> extra-y += $(head-y) vmlinux.lds
>>>
>>> diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
>>> similarity index 100%
>>> rename from arch/arm64/kernel/vdso/vdso.S
>>> rename to arch/arm64/kernel/vdso-wrap.S
>>> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
>>> index a8f8e409e2bf..85222f64f394 100644
>>> --- a/arch/arm64/kernel/vdso/Makefile
>>> +++ b/arch/arm64/kernel/vdso/Makefile
>>> @@ -45,7 +45,6 @@ endif
>>> # Disable gcov profiling for VDSO code
>>> GCOV_PROFILE := n
>>>
>>> -obj-y += vdso.o
>>> targets += vdso.lds
>>> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>>>
>>> diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
>>> similarity index 100%
>>> rename from arch/arm64/kernel/vdso32/vdso.S
>>> rename to arch/arm64/kernel/vdso32-wrap.S
>>> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
>>> index a1e0f91e6cea..789ad420f16b 100644
>>> --- a/arch/arm64/kernel/vdso32/Makefile
>>> +++ b/arch/arm64/kernel/vdso32/Makefile
>>> @@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
>>> asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
>>> obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
>>>
>>> -obj-y += vdso.o
>>> targets += vdso.lds
>>> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>>>
>>>
>>
>> --
>> Regards,
>> Vincenzo
>
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Regards,
Vincenzo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
@ 2020-12-22 14:34 ` Vincenzo Frascino
0 siblings, 0 replies; 10+ messages in thread
From: Vincenzo Frascino @ 2020-12-22 14:34 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Catalin Marinas, Will Deacon, Linux Kernel Mailing List,
linux-arm-kernel, Michael Ellerman
On 12/21/20 4:03 PM, Masahiro Yamada wrote:
> On Mon, Dec 21, 2020 at 11:36 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> Hi Masahiro,
>>
>> On 12/18/20 2:45 AM, Masahiro Yamada wrote:
>>> arm64 descends into each vdso directory twice; first in vdso_prepare,
>>> second during the ordinary build process.
>>>
>>> PPC mimicked it and uncovered a problem [1]. In the first descend,
>>> Kbuild directly visits the vdso directories, therefore it does not
>>> inherit subdir-ccflags-y from upper directories.
>>>
>>> This means the command line parameters may differ between the two.
>>> If it happens, the offset values in the generated headers might be
>>> different from real offsets of vdso.so in the kernel.
>>>
>>> This potential danger should be avoided. The vdso directories are
>>> built in the vdso_prepare stage, so the second descend is unneeded.
>>>
>>> [1]: https://lore.kernel.org/linux-kbuild/CAK7LNARAkJ3_-4gX0VA2UkapbOftuzfSTVMBbgbw=HD8n7N+7w@mail.gmail.com/T/#ma10dcb961fda13f36d42d58fa6cb2da988b7e73a
>>>
>>
>> I could not reproduce the problem you are reporting on arm64. Could you please
>> provide some steps?
>
>
> As far as I see in arm64 Makefiles, you cannot reproduce it.
> So, this is a _potential_ problem.
>
> When somebody adds subdir-ccflags-y to arch/arm64/Kbuild or
> arch/arm64/kernel/Makefile,
> the real issue will suddenly come up.
>
>
> arm64 is the first arch that implemented vdso-offsets.h
>
> arch maintainers tend to copy Makefiles from existing ones.
>
> Now PPC is the second with almost the same Makefile implementation.
>
> This time, powerpc hit the problem because arch/powerpc/Kbuild
> has subdir-ccflags-y.
>
> I do not want to be bothered every time arch maintainers hit this issue.
> So, I want to fix arm64 first,
> then powerpc will eventually copy the correct Makefiles.
>
>
Ok this seems a problem related to a top down approach in the ccflags
propagation, which as it stands does not affect arm64, hence IMHO we should not
fix what is not broken, but I do not have a strong opinion on this.
>
>
>> In my case the vDSO library is not rebuilt as a result of the procedure reported
>> in the email you linked at [1].
>>
>>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>>> ---
>>>
>>> arch/arm64/Makefile | 10 ++++++----
>>> arch/arm64/kernel/Makefile | 5 +++--
>>> arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} | 0
>>> arch/arm64/kernel/vdso/Makefile | 1 -
>>> arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} | 0
>>> arch/arm64/kernel/vdso32/Makefile | 1 -
>>> 6 files changed, 9 insertions(+), 8 deletions(-)
>>> rename arch/arm64/kernel/{vdso/vdso.S => vdso-wrap.S} (100%)
>>> rename arch/arm64/kernel/{vdso32/vdso.S => vdso32-wrap.S} (100%)
>>>
>>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
>>> index 6a87d592bd00..f18d20a68170 100644
>>> --- a/arch/arm64/Makefile
>>> +++ b/arch/arm64/Makefile
>>> @@ -179,10 +179,12 @@ ifeq ($(KBUILD_EXTMOD),)
>>> # this hack.
>>> prepare: vdso_prepare
>>> vdso_prepare: prepare0
>>> - $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h
>>> - $(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
>>> - $(build)=arch/arm64/kernel/vdso32 \
>>> - include/generated/vdso32-offsets.h)
>>> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso \
>>> + include/generated/vdso-offsets.h arch/arm64/kernel/vdso/vdso.so
>>> +ifdef CONFIG_COMPAT_VDSO
>>> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 \
>>> + include/generated/vdso32-offsets.h arch/arm64/kernel/vdso32/vdso.so
>>> +endif
>>> endif
>>
>> The reason why it is currently done in two phases (a bit hacky as per comment)
>> is because vdso-offsets.h is required to be generated before compiling kernel/.
>> Please refer to the comment in arch/arm64/Makefile.
>
> Yes, I know.
>
>>
>> Could you explain how your change satisfies the dependency?
>
> vdso_prepare completes before Kbuild starts ordinary descending.
> So *.so files and vdso-offsets.h exist
> when Kbuild builds arch/arm64/kernel/.
>
>
This is not immediately obvious, could you please mention it in the commit message?
Nit: Could you please re-organize the code as follows in order to limit the
changes (untested)?
$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso include/generated/vdso-offsets.h \
arch/arm64/kernel/vdso/vdso.so
$(if $(CONFIG_COMPAT_VDSO),$(Q)$(MAKE) \
$(build)=arch/arm64/kernel/vdso32 \
include/generated/vdso32-offsets.h \
arch/arm64/kernel/vdso32/vdso.so)
>
>
>
>
>>>
>>> define archhelp
>>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>>> index 86364ab6f13f..42f6ad2c7eac 100644
>>> --- a/arch/arm64/kernel/Makefile
>>> +++ b/arch/arm64/kernel/Makefile
>>> @@ -59,9 +59,10 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
>>> obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
>>> obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
>>> obj-$(CONFIG_ARM64_MTE) += mte.o
>>> +obj-y += vdso-wrap.o
>>> +obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o
>>>
>>> -obj-y += vdso/ probes/
>>> -obj-$(CONFIG_COMPAT_VDSO) += vdso32/
>>> +obj-y += probes/
>>> head-y := head.o
>>> extra-y += $(head-y) vmlinux.lds
>>>
>>> diff --git a/arch/arm64/kernel/vdso/vdso.S b/arch/arm64/kernel/vdso-wrap.S
>>> similarity index 100%
>>> rename from arch/arm64/kernel/vdso/vdso.S
>>> rename to arch/arm64/kernel/vdso-wrap.S
>>> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
>>> index a8f8e409e2bf..85222f64f394 100644
>>> --- a/arch/arm64/kernel/vdso/Makefile
>>> +++ b/arch/arm64/kernel/vdso/Makefile
>>> @@ -45,7 +45,6 @@ endif
>>> # Disable gcov profiling for VDSO code
>>> GCOV_PROFILE := n
>>>
>>> -obj-y += vdso.o
>>> targets += vdso.lds
>>> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>>>
>>> diff --git a/arch/arm64/kernel/vdso32/vdso.S b/arch/arm64/kernel/vdso32-wrap.S
>>> similarity index 100%
>>> rename from arch/arm64/kernel/vdso32/vdso.S
>>> rename to arch/arm64/kernel/vdso32-wrap.S
>>> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
>>> index a1e0f91e6cea..789ad420f16b 100644
>>> --- a/arch/arm64/kernel/vdso32/Makefile
>>> +++ b/arch/arm64/kernel/vdso32/Makefile
>>> @@ -155,7 +155,6 @@ c-obj-vdso-gettimeofday := $(addprefix $(obj)/, $(c-obj-vdso-gettimeofday))
>>> asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
>>> obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
>>>
>>> -obj-y += vdso.o
>>> targets += vdso.lds
>>> CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
>>>
>>>
>>
>> --
>> Regards,
>> Vincenzo
>
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Regards,
Vincenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
2020-12-18 2:45 ` Masahiro Yamada
@ 2021-01-20 13:01 ` Will Deacon
-1 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2021-01-20 13:01 UTC (permalink / raw)
To: Catalin Marinas, Masahiro Yamada, linux-arm-kernel
Cc: kernel-team, Will Deacon, Michael Ellerman, linux-kernel
On Fri, 18 Dec 2020 11:45:40 +0900, Masahiro Yamada wrote:
> arm64 descends into each vdso directory twice; first in vdso_prepare,
> second during the ordinary build process.
>
> PPC mimicked it and uncovered a problem [1]. In the first descend,
> Kbuild directly visits the vdso directories, therefore it does not
> inherit subdir-ccflags-y from upper directories.
>
> [...]
Applied to arm64 (for-next/vdso), thanks!
[1/1] arm64: do not descend to vdso directories twice
https://git.kernel.org/arm64/c/a5b8ca97fbf8
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice
@ 2021-01-20 13:01 ` Will Deacon
0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2021-01-20 13:01 UTC (permalink / raw)
To: Catalin Marinas, Masahiro Yamada, linux-arm-kernel
Cc: Will Deacon, kernel-team, linux-kernel, Michael Ellerman
On Fri, 18 Dec 2020 11:45:40 +0900, Masahiro Yamada wrote:
> arm64 descends into each vdso directory twice; first in vdso_prepare,
> second during the ordinary build process.
>
> PPC mimicked it and uncovered a problem [1]. In the first descend,
> Kbuild directly visits the vdso directories, therefore it does not
> inherit subdir-ccflags-y from upper directories.
>
> [...]
Applied to arm64 (for-next/vdso), thanks!
[1/1] arm64: do not descend to vdso directories twice
https://git.kernel.org/arm64/c/a5b8ca97fbf8
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-20 14:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 2:45 [PATCH] arm64: do not descend to vdso directories twice Masahiro Yamada
2020-12-18 2:45 ` Masahiro Yamada
2020-12-21 14:39 ` Vincenzo Frascino
2020-12-21 14:39 ` Vincenzo Frascino
2020-12-21 16:03 ` Masahiro Yamada
2020-12-21 16:03 ` Masahiro Yamada
2020-12-22 14:34 ` Vincenzo Frascino
2020-12-22 14:34 ` Vincenzo Frascino
2021-01-20 13:01 ` Will Deacon
2021-01-20 13:01 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.