Re: [PATCH 4/5] kbuild: unify vdso_install rules
On Mon, Oct 09, 2023 at 09:42:09PM +0900, Masahiro Yamada wrote: > arch/arm/Makefile | 7 +--- > arch/arm/vdso/Makefile | 25 -- Acked-by: Russell King (Oracle) Thanks! -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 4/5] kbuild: unify vdso_install rules
On Mon, Oct 09, 2023 at 09:42:09PM +0900, Masahiro Yamada wrote: > Currently, there is no standard implementation for vdso_install, > leading to various issues: [...] > arch/arm64/Makefile| 9 ++ > arch/arm64/kernel/vdso/Makefile| 10 -- > arch/arm64/kernel/vdso32/Makefile | 10 -- For arm64: Acked-by: Catalin Marinas
Re: [PATCH 4/5] kbuild: unify vdso_install rules
On Wed, Oct 11, 2023 at 8:53 PM Masahiro Yamada wrote: > > On Wed, Oct 11, 2023 at 11:24 AM Guo Ren wrote: > > > > On Mon, Oct 9, 2023 at 8:42 PM Masahiro Yamada wrote: > > > > --- a/arch/riscv/Makefile > > > +++ b/arch/riscv/Makefile > > > @@ -131,12 +131,6 @@ endif > > > libs-y += arch/riscv/lib/ > > > libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > -PHONY += vdso_install > > > -vdso_install: > > > - $(Q)$(MAKE) $(build)=arch/riscv/kernel/vdso $@ > > > - $(if $(CONFIG_COMPAT),$(Q)$(MAKE) \ > > > - $(build)=arch/riscv/kernel/compat_vdso compat_$@) > > > - > > > ifeq ($(KBUILD_EXTMOD),) > > > ifeq ($(CONFIG_MMU),y) > > > prepare: vdso_prepare > > > @@ -148,6 +142,9 @@ vdso_prepare: prepare0 > > > endif > > > endif > > > > > > +vdso-install-y += arch/riscv/kernel/vdso/vdso.so.dbg > > > +vdso-install-$(CONFIG_COMPAT) += > > > arch/riscv/kernel/compat_vdso/compat_vdso.so.dbg:../compat_vdso/compat_vdso.so > > Why do we need ":../compat_vdso/compat_vdso.so" here? > > > > > All architectures except riscv install vdso files > to /lib/modules/$(uname -r)/vdso/. > > > > See the following code in arch/riscv/kernel/compat_vdso/Makefile: > > > quiet_cmd_compat_vdso_install = INSTALL $@ > cmd_compat_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/compat_vdso/$@ > > > > > Riscv copies the compat vdso to > /lib/modules/$(uname -r)/compat_vdso/. > > > > This commit preserves the current installation path as-is. > > If the riscv maintainers agree, we can change the > installation destination to /lib/modules/$(uname -r)/vdso/ > for consistency. Yes, but it should be another patch. Thx for the clarification. Reviewed-by: Guo Ren > > > > -- > Best Regards > Masahiro Yamada -- Best Regards Guo Ren
Re: [PATCH 4/5] kbuild: unify vdso_install rules
On Mon 09 Oct 2023 21:42:09 GMT, Masahiro Yamada wrote:
> Currently, there is no standard implementation for vdso_install,
> leading to various issues:
>
> 1. Code duplication
>
> Many architectures duplicate similar code just for copying files
> to the install destination.
>
> Some architectures (arm, sparc, x86) create build-id symlinks,
> introducing more code duplication.
>
> 2. Accidental updates of in-tree build artifacts
>
> The vdso_install rule depends on the vdso files to install.
> It may update in-tree build artifacts. This can be problematic,
> as explained in commit 19514fc665ff ("arm, kbuild: make
> "make install" not depend on vmlinux").
>
> 3. Broken code in some architectures
>
> Makefile code is often copied from one architecture to another
> without proper adaptation or testing.
>
> The previous commits removed broken code from csky, UML, and parisc.
>
> Another issue is that 'make vdso_install' for ARCH=s390 installs
> vdso64, but not vdso32.
>
> To address these problems, this commit introduces the generic vdso_install.
>
> Architectures that support vdso_install need to define vdso-install-y
> in arch/*/Makefile.
>
> vdso-install-y lists the files to install. For example, arch/x86/Makefile
> looks like this:
>
> vdso-install-$(CONFIG_X86_64) += arch/x86/entry/vdso/vdso64.so.dbg
> vdso-install-$(CONFIG_X86_X32_ABI) +=
> arch/x86/entry/vdso/vdsox32.so.dbg
> vdso-install-$(CONFIG_X86_32) += arch/x86/entry/vdso/vdso32.so.dbg
> vdso-install-$(CONFIG_IA32_EMULATION) += arch/x86/entry/vdso/vdso32.so.dbg
>
> These files will be installed to $(MODLIB)/vdso/ with the .dbg suffix,
> if exists, stripped away.
>
> vdso-install-y can optionally take the second field after the colon
> separator. This is needed because some architectures install vdso
> files as a different base name.
>
> The following is a snippet from arch/arm64/Makefile.
>
> vdso-install-$(CONFIG_COMPAT_VDSO) +=
> arch/arm64/kernel/vdso32/vdso.so.dbg:vdso32.so
>
> This will rename vdso.so.dbg to vdso32.so during installation. If such
> architectures change their implementation so that the file names match,
> this workaround will go away.
>
> Signed-off-by: Masahiro Yamada
> ---
Thanks for cleaning this up!
Reviewed-by: Nicolas Schier
Re: [PATCH 4/5] kbuild: unify vdso_install rules
On Wed, Oct 11, 2023 at 11:24 AM Guo Ren wrote: > > On Mon, Oct 9, 2023 at 8:42 PM Masahiro Yamada wrote: > > --- a/arch/riscv/Makefile > > +++ b/arch/riscv/Makefile > > @@ -131,12 +131,6 @@ endif > > libs-y += arch/riscv/lib/ > > libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > -PHONY += vdso_install > > -vdso_install: > > - $(Q)$(MAKE) $(build)=arch/riscv/kernel/vdso $@ > > - $(if $(CONFIG_COMPAT),$(Q)$(MAKE) \ > > - $(build)=arch/riscv/kernel/compat_vdso compat_$@) > > - > > ifeq ($(KBUILD_EXTMOD),) > > ifeq ($(CONFIG_MMU),y) > > prepare: vdso_prepare > > @@ -148,6 +142,9 @@ vdso_prepare: prepare0 > > endif > > endif > > > > +vdso-install-y += arch/riscv/kernel/vdso/vdso.so.dbg > > +vdso-install-$(CONFIG_COMPAT) += > > arch/riscv/kernel/compat_vdso/compat_vdso.so.dbg:../compat_vdso/compat_vdso.so > Why do we need ":../compat_vdso/compat_vdso.so" here? All architectures except riscv install vdso files to /lib/modules/$(uname -r)/vdso/. See the following code in arch/riscv/kernel/compat_vdso/Makefile: quiet_cmd_compat_vdso_install = INSTALL $@ cmd_compat_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/compat_vdso/$@ Riscv copies the compat vdso to /lib/modules/$(uname -r)/compat_vdso/. This commit preserves the current installation path as-is. If the riscv maintainers agree, we can change the installation destination to /lib/modules/$(uname -r)/vdso/ for consistency. -- Best Regards Masahiro Yamada
Re: [PATCH 4/5] kbuild: unify vdso_install rules
On Mon, Oct 9, 2023 at 8:42 PM Masahiro Yamada wrote:
>
> Currently, there is no standard implementation for vdso_install,
> leading to various issues:
>
> 1. Code duplication
>
> Many architectures duplicate similar code just for copying files
> to the install destination.
>
> Some architectures (arm, sparc, x86) create build-id symlinks,
> introducing more code duplication.
>
> 2. Accidental updates of in-tree build artifacts
>
> The vdso_install rule depends on the vdso files to install.
> It may update in-tree build artifacts. This can be problematic,
> as explained in commit 19514fc665ff ("arm, kbuild: make
> "make install" not depend on vmlinux").
>
> 3. Broken code in some architectures
>
> Makefile code is often copied from one architecture to another
> without proper adaptation or testing.
>
> The previous commits removed broken code from csky, UML, and parisc.
>
> Another issue is that 'make vdso_install' for ARCH=s390 installs
> vdso64, but not vdso32.
>
> To address these problems, this commit introduces the generic vdso_install.
>
> Architectures that support vdso_install need to define vdso-install-y
> in arch/*/Makefile.
>
> vdso-install-y lists the files to install. For example, arch/x86/Makefile
> looks like this:
>
> vdso-install-$(CONFIG_X86_64) += arch/x86/entry/vdso/vdso64.so.dbg
> vdso-install-$(CONFIG_X86_X32_ABI) +=
> arch/x86/entry/vdso/vdsox32.so.dbg
> vdso-install-$(CONFIG_X86_32) += arch/x86/entry/vdso/vdso32.so.dbg
> vdso-install-$(CONFIG_IA32_EMULATION) += arch/x86/entry/vdso/vdso32.so.dbg
>
> These files will be installed to $(MODLIB)/vdso/ with the .dbg suffix,
> if exists, stripped away.
>
> vdso-install-y can optionally take the second field after the colon
> separator. This is needed because some architectures install vdso
> files as a different base name.
>
> The following is a snippet from arch/arm64/Makefile.
>
> vdso-install-$(CONFIG_COMPAT_VDSO) +=
> arch/arm64/kernel/vdso32/vdso.so.dbg:vdso32.so
>
> This will rename vdso.so.dbg to vdso32.so during installation. If such
> architectures change their implementation so that the file names match,
> this workaround will go away.
>
> Signed-off-by: Masahiro Yamada
> ---
>
> Makefile | 9 ++
> arch/arm/Makefile | 7 +---
> arch/arm/vdso/Makefile | 25 --
> arch/arm64/Makefile| 9 ++
> arch/arm64/kernel/vdso/Makefile| 10 --
> arch/arm64/kernel/vdso32/Makefile | 10 --
> arch/loongarch/Makefile| 4 +--
> arch/loongarch/vdso/Makefile | 10 --
> arch/riscv/Makefile| 9 ++
> arch/riscv/kernel/compat_vdso/Makefile | 10 --
> arch/riscv/kernel/vdso/Makefile| 10 --
> arch/s390/Makefile | 6 ++--
> arch/s390/kernel/vdso32/Makefile | 10 --
> arch/s390/kernel/vdso64/Makefile | 10 --
> arch/sparc/Makefile| 5 ++-
> arch/sparc/vdso/Makefile | 27
> arch/x86/Makefile | 7 ++--
> arch/x86/entry/vdso/Makefile | 27
> scripts/Makefile.vdsoinst | 45 ++
> 19 files changed, 71 insertions(+), 179 deletions(-)
> create mode 100644 scripts/Makefile.vdsoinst
>
> diff --git a/Makefile b/Makefile
> index 373649c7374e..2170d56630e8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1317,6 +1317,14 @@ scripts_unifdef: scripts_basic
> quiet_cmd_install = INSTALL $(INSTALL_PATH)
>cmd_install = unset sub_make_done; $(srctree)/scripts/install.sh
>
> +# ---
> +# vDSO install
> +
> +PHONY += vdso_install
> +vdso_install: export INSTALL_FILES = $(vdso-install-y)
> +vdso_install:
> + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vdsoinst
> +
> # ---
> # Tools
>
> @@ -1560,6 +1568,7 @@ help:
> @echo '* vmlinux - Build the bare kernel'
> @echo '* modules - Build all modules'
> @echo ' modules_install - Install all modules to INSTALL_MOD_PATH
> (default: /)'
> + @echo ' vdso_install- Install unstripped vdso to
> INSTALL_MOD_PATH (default: /)'
> @echo ' dir/- Build all files in dir and below'
> @echo ' dir/file.[ois] - Build specified target only'
> @echo ' dir/file.ll - Build the LLVM assembly file'
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 547e5856eaa0..5ba42f69f8ce 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -304,11 +304,7 @@ $(INSTALL_TARGETS): KBUILD_IMAGE = $(boot)/$(patsubst
> %install,%Image,$@)
> $(INSTALL_TARGETS):
> $(call cmd,install)
>
> -PHONY += vdso_install
>
Re: [PATCH 4/5] kbuild: unify vdso_install rules
Masahiro Yamada writes:
> Currently, there is no standard implementation for vdso_install,
> leading to various issues:
>
> 1. Code duplication
>
> Many architectures duplicate similar code just for copying files
> to the install destination.
>
> Some architectures (arm, sparc, x86) create build-id symlinks,
> introducing more code duplication.
>
> 2. Accidental updates of in-tree build artifacts
>
> The vdso_install rule depends on the vdso files to install.
> It may update in-tree build artifacts. This can be problematic,
> as explained in commit 19514fc665ff ("arm, kbuild: make
> "make install" not depend on vmlinux").
>
> 3. Broken code in some architectures
>
> Makefile code is often copied from one architecture to another
> without proper adaptation or testing.
>
> The previous commits removed broken code from csky, UML, and parisc.
>
> Another issue is that 'make vdso_install' for ARCH=s390 installs
> vdso64, but not vdso32.
>
> To address these problems, this commit introduces the generic vdso_install.
>
> Architectures that support vdso_install need to define vdso-install-y
> in arch/*/Makefile.
>
> vdso-install-y lists the files to install. For example, arch/x86/Makefile
> looks like this:
>
> vdso-install-$(CONFIG_X86_64) += arch/x86/entry/vdso/vdso64.so.dbg
> vdso-install-$(CONFIG_X86_X32_ABI) +=
> arch/x86/entry/vdso/vdsox32.so.dbg
> vdso-install-$(CONFIG_X86_32) += arch/x86/entry/vdso/vdso32.so.dbg
> vdso-install-$(CONFIG_IA32_EMULATION) += arch/x86/entry/vdso/vdso32.so.dbg
>
> These files will be installed to $(MODLIB)/vdso/ with the .dbg suffix,
> if exists, stripped away.
>
> vdso-install-y can optionally take the second field after the colon
> separator. This is needed because some architectures install vdso
> files as a different base name.
>
> The following is a snippet from arch/arm64/Makefile.
>
> vdso-install-$(CONFIG_COMPAT_VDSO) +=
> arch/arm64/kernel/vdso32/vdso.so.dbg:vdso32.so
>
> This will rename vdso.so.dbg to vdso32.so during installation. If such
> architectures change their implementation so that the file names match,
> this workaround will go away.
>
> Signed-off-by: Masahiro Yamada
> ---
Acked-by: Sven Schnelle # s390
[PATCH 4/5] kbuild: unify vdso_install rules
Currently, there is no standard implementation for vdso_install,
leading to various issues:
1. Code duplication
Many architectures duplicate similar code just for copying files
to the install destination.
Some architectures (arm, sparc, x86) create build-id symlinks,
introducing more code duplication.
2. Accidental updates of in-tree build artifacts
The vdso_install rule depends on the vdso files to install.
It may update in-tree build artifacts. This can be problematic,
as explained in commit 19514fc665ff ("arm, kbuild: make
"make install" not depend on vmlinux").
3. Broken code in some architectures
Makefile code is often copied from one architecture to another
without proper adaptation or testing.
The previous commits removed broken code from csky, UML, and parisc.
Another issue is that 'make vdso_install' for ARCH=s390 installs
vdso64, but not vdso32.
To address these problems, this commit introduces the generic vdso_install.
Architectures that support vdso_install need to define vdso-install-y
in arch/*/Makefile.
vdso-install-y lists the files to install. For example, arch/x86/Makefile
looks like this:
vdso-install-$(CONFIG_X86_64) += arch/x86/entry/vdso/vdso64.so.dbg
vdso-install-$(CONFIG_X86_X32_ABI) += arch/x86/entry/vdso/vdsox32.so.dbg
vdso-install-$(CONFIG_X86_32) += arch/x86/entry/vdso/vdso32.so.dbg
vdso-install-$(CONFIG_IA32_EMULATION) += arch/x86/entry/vdso/vdso32.so.dbg
These files will be installed to $(MODLIB)/vdso/ with the .dbg suffix,
if exists, stripped away.
vdso-install-y can optionally take the second field after the colon
separator. This is needed because some architectures install vdso
files as a different base name.
The following is a snippet from arch/arm64/Makefile.
vdso-install-$(CONFIG_COMPAT_VDSO) +=
arch/arm64/kernel/vdso32/vdso.so.dbg:vdso32.so
This will rename vdso.so.dbg to vdso32.so during installation. If such
architectures change their implementation so that the file names match,
this workaround will go away.
Signed-off-by: Masahiro Yamada
---
Makefile | 9 ++
arch/arm/Makefile | 7 +---
arch/arm/vdso/Makefile | 25 --
arch/arm64/Makefile| 9 ++
arch/arm64/kernel/vdso/Makefile| 10 --
arch/arm64/kernel/vdso32/Makefile | 10 --
arch/loongarch/Makefile| 4 +--
arch/loongarch/vdso/Makefile | 10 --
arch/riscv/Makefile| 9 ++
arch/riscv/kernel/compat_vdso/Makefile | 10 --
arch/riscv/kernel/vdso/Makefile| 10 --
arch/s390/Makefile | 6 ++--
arch/s390/kernel/vdso32/Makefile | 10 --
arch/s390/kernel/vdso64/Makefile | 10 --
arch/sparc/Makefile| 5 ++-
arch/sparc/vdso/Makefile | 27
arch/x86/Makefile | 7 ++--
arch/x86/entry/vdso/Makefile | 27
scripts/Makefile.vdsoinst | 45 ++
19 files changed, 71 insertions(+), 179 deletions(-)
create mode 100644 scripts/Makefile.vdsoinst
diff --git a/Makefile b/Makefile
index 373649c7374e..2170d56630e8 100644
--- a/Makefile
+++ b/Makefile
@@ -1317,6 +1317,14 @@ scripts_unifdef: scripts_basic
quiet_cmd_install = INSTALL $(INSTALL_PATH)
cmd_install = unset sub_make_done; $(srctree)/scripts/install.sh
+# ---
+# vDSO install
+
+PHONY += vdso_install
+vdso_install: export INSTALL_FILES = $(vdso-install-y)
+vdso_install:
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vdsoinst
+
# ---
# Tools
@@ -1560,6 +1568,7 @@ help:
@echo '* vmlinux - Build the bare kernel'
@echo '* modules - Build all modules'
@echo ' modules_install - Install all modules to INSTALL_MOD_PATH
(default: /)'
+ @echo ' vdso_install- Install unstripped vdso to INSTALL_MOD_PATH
(default: /)'
@echo ' dir/- Build all files in dir and below'
@echo ' dir/file.[ois] - Build specified target only'
@echo ' dir/file.ll - Build the LLVM assembly file'
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 547e5856eaa0..5ba42f69f8ce 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -304,11 +304,7 @@ $(INSTALL_TARGETS): KBUILD_IMAGE = $(boot)/$(patsubst
%install,%Image,$@)
$(INSTALL_TARGETS):
$(call cmd,install)
-PHONY += vdso_install
-vdso_install:
-ifeq ($(CONFIG_VDSO),y)
- $(Q)$(MAKE) $(build)=arch/arm/vdso $@
-endif
+vdso-install-$(CONFIG_VDSO) += arch/arm/vdso/vdso.so.dbg
# My testing targets (bypasses dependencies)
bp:; $(Q)$(MAKE) $(build)=$(boot) $(boot)/bootpImage
@@ -331,7 +327,6 @@ define
