* [PATCH] arm64: do not descend to vdso directories twice
@ 2020-12-18 2:45 Masahiro Yamada
2020-12-21 14:39 ` Vincenzo Frascino
2021-01-20 13:01 ` Will Deacon
0 siblings, 2 replies; 5+ 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] 5+ messages in thread* Re: [PATCH] arm64: do not descend to vdso directories twice 2020-12-18 2:45 [PATCH] arm64: do not descend to vdso directories twice Masahiro Yamada @ 2020-12-21 14:39 ` Vincenzo Frascino 2020-12-21 16:03 ` Masahiro Yamada 2021-01-20 13:01 ` Will Deacon 1 sibling, 1 reply; 5+ 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] 5+ 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 2020-12-22 14:34 ` Vincenzo Frascino 0 siblings, 1 reply; 5+ 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] 5+ 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 0 siblings, 0 replies; 5+ 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] 5+ messages in thread
* Re: [PATCH] arm64: do not descend to vdso directories twice 2020-12-18 2:45 [PATCH] arm64: do not descend to vdso directories twice Masahiro Yamada 2020-12-21 14:39 ` Vincenzo Frascino @ 2021-01-20 13:01 ` Will Deacon 1 sibling, 0 replies; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2021-01-20 14:19 UTC | newest] Thread overview: 5+ 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-21 14:39 ` Vincenzo Frascino 2020-12-21 16:03 ` Masahiro Yamada 2020-12-22 14:34 ` Vincenzo Frascino 2021-01-20 13:01 ` Will Deacon
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).