| Message ID | 20191002144156.2174-2-vincenzo.frascino@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: vdso32: Address various issues | expand |
On Wed, Oct 02, 2019 at 03:41:51PM +0100, Vincenzo Frascino wrote: > Cc: Will Deacon <will@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Reported-by: Will Deacon <will@kernel.org> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > Tested-by: Catalin Marinas <catalin.marinas@arm.com> If you changed the patch, please drop my reviewed/tested-by. > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index 84a3d502c5a5..dfa6a5cb99e4 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -53,20 +53,12 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable) > endif > endif > > +COMPATCC ?= $(CROSS_COMPILE_COMPAT)gcc > +export COMPATCC > + > ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) > - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) > - > - ifeq ($(CONFIG_CC_IS_CLANG), y) > - $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built) > - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) > - $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built) > - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) > - $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT) > - else > - export CROSS_COMPILE_COMPAT > - export CONFIG_COMPAT_VDSO := y > - compat_vdso := -DCONFIG_COMPAT_VDSO=1 > - endif > + export CONFIG_COMPAT_VDSO := y > + compat_vdso := -DCONFIG_COMPAT_VDSO=1 > endif With this change, if I don't have any CROSS_COMPILE_COMPAT in my environment, the kernel fails to build because COMPATCC becomes gcc which cannot build the vdso32. What I really want is not to warn me, nor fail to build the kernel when I don't care about the compat vDSO (e.g. I have a 64-bit only machine). What saved us before was the COMPATCC_IS_ARM_GCC check and a selection of the GENERIC_COMPAT_VDSO dependent on this check. This was now dropped from the previous version of the patch. We could add something like COMPATCC_CAN_LINK.
On Thu, Oct 03, 2019 at 05:00:41PM +0100, Catalin Marinas wrote: > On Wed, Oct 02, 2019 at 03:41:51PM +0100, Vincenzo Frascino wrote: > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > index 84a3d502c5a5..dfa6a5cb99e4 100644 > > --- a/arch/arm64/Makefile > > +++ b/arch/arm64/Makefile > > @@ -53,20 +53,12 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable) > > endif > > endif > > > > +COMPATCC ?= $(CROSS_COMPILE_COMPAT)gcc > > +export COMPATCC > > + > > ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) > > - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) > > - > > - ifeq ($(CONFIG_CC_IS_CLANG), y) > > - $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built) > > - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) > > - $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built) > > - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) > > - $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT) > > - else > > - export CROSS_COMPILE_COMPAT > > - export CONFIG_COMPAT_VDSO := y > > - compat_vdso := -DCONFIG_COMPAT_VDSO=1 > > - endif > > + export CONFIG_COMPAT_VDSO := y > > + compat_vdso := -DCONFIG_COMPAT_VDSO=1 > > endif > > With this change, if I don't have any CROSS_COMPILE_COMPAT in my > environment, the kernel fails to build because COMPATCC becomes gcc > which cannot build the vdso32. What I really want is not to warn me, nor > fail to build the kernel when I don't care about the compat vDSO (e.g. I > have a 64-bit only machine). > > What saved us before was the COMPATCC_IS_ARM_GCC check and a selection > of the GENERIC_COMPAT_VDSO dependent on this check. This was now dropped > from the previous version of the patch. We could add something like > COMPATCC_CAN_LINK. Ah, the COMPATCC_CAN_LINK idea doesn't help because an x86 gcc can still link. Maybe only enable CONFIG_COMPAT_VDSO above if CROSS_COMPILE_COMPAT was set. You could move the COMPATCC setting and export under the same 'if' block in the Makefile.
On 03/10/2019 17:00, Catalin Marinas wrote: > On Wed, Oct 02, 2019 at 03:41:51PM +0100, Vincenzo Frascino wrote: >> Cc: Will Deacon <will@kernel.org> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Reported-by: Will Deacon <will@kernel.org> >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> >> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> >> Tested-by: Catalin Marinas <catalin.marinas@arm.com> > > If you changed the patch, please drop my reviewed/tested-by. > Sorry, forgot it in this patch. >> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile >> index 84a3d502c5a5..dfa6a5cb99e4 100644 >> --- a/arch/arm64/Makefile >> +++ b/arch/arm64/Makefile >> @@ -53,20 +53,12 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable) >> endif >> endif >> >> +COMPATCC ?= $(CROSS_COMPILE_COMPAT)gcc >> +export COMPATCC >> + >> ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) >> - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) >> - >> - ifeq ($(CONFIG_CC_IS_CLANG), y) >> - $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built) >> - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) >> - $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built) >> - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) >> - $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT) >> - else >> - export CROSS_COMPILE_COMPAT >> - export CONFIG_COMPAT_VDSO := y >> - compat_vdso := -DCONFIG_COMPAT_VDSO=1 >> - endif >> + export CONFIG_COMPAT_VDSO := y >> + compat_vdso := -DCONFIG_COMPAT_VDSO=1 >> endif > > With this change, if I don't have any CROSS_COMPILE_COMPAT in my > environment, the kernel fails to build because COMPATCC becomes gcc > which cannot build the vdso32. What I really want is not to warn me, nor > fail to build the kernel when I don't care about the compat vDSO (e.g. I > have a 64-bit only machine). > > What saved us before was the COMPATCC_IS_ARM_GCC check and a selection > of the GENERIC_COMPAT_VDSO dependent on this check. This was now dropped > from the previous version of the patch. We could add something like > COMPATCC_CAN_LINK. > Seems not solving the problem, I am trying with COMPATCC_EMPTY as test -z $CROSS_COMPILER_COMPAT.
On 03/10/2019 17:16, Catalin Marinas wrote: > On Thu, Oct 03, 2019 at 05:00:41PM +0100, Catalin Marinas wrote: >> On Wed, Oct 02, 2019 at 03:41:51PM +0100, Vincenzo Frascino wrote: >>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile >>> index 84a3d502c5a5..dfa6a5cb99e4 100644 >>> --- a/arch/arm64/Makefile >>> +++ b/arch/arm64/Makefile >>> @@ -53,20 +53,12 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable) >>> endif >>> endif >>> >>> +COMPATCC ?= $(CROSS_COMPILE_COMPAT)gcc >>> +export COMPATCC >>> + >>> ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) >>> - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) >>> - >>> - ifeq ($(CONFIG_CC_IS_CLANG), y) >>> - $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built) >>> - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) >>> - $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built) >>> - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) >>> - $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT) >>> - else >>> - export CROSS_COMPILE_COMPAT >>> - export CONFIG_COMPAT_VDSO := y >>> - compat_vdso := -DCONFIG_COMPAT_VDSO=1 >>> - endif >>> + export CONFIG_COMPAT_VDSO := y >>> + compat_vdso := -DCONFIG_COMPAT_VDSO=1 >>> endif >> >> With this change, if I don't have any CROSS_COMPILE_COMPAT in my >> environment, the kernel fails to build because COMPATCC becomes gcc >> which cannot build the vdso32. What I really want is not to warn me, nor >> fail to build the kernel when I don't care about the compat vDSO (e.g. I >> have a 64-bit only machine). >> >> What saved us before was the COMPATCC_IS_ARM_GCC check and a selection >> of the GENERIC_COMPAT_VDSO dependent on this check. This was now dropped >> from the previous version of the patch. We could add something like >> COMPATCC_CAN_LINK. > > Ah, the COMPATCC_CAN_LINK idea doesn't help because an x86 gcc can still > link. Maybe only enable CONFIG_COMPAT_VDSO above if CROSS_COMPILE_COMPAT > was set. You could move the COMPATCC setting and export under the > same 'if' block in the Makefile. > Right, I did not see this before my previous reply. If I do that I get what follows, still investigating: make[1]: Entering directory '/data1/Projects/LinuxKernel/linux-out' $CROSS_COMPILE_COMPAT is [] CLEAN arch/arm64/crypto CLEAN arch/arm64/kernel/vdso /bin/sh: 0: Illegal option -r
Hi Catalin, On 03/10/2019 17:16, Catalin Marinas wrote: > On Thu, Oct 03, 2019 at 05:00:41PM +0100, Catalin Marinas wrote: >> On Wed, Oct 02, 2019 at 03:41:51PM +0100, Vincenzo Frascino wrote: >>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile >>> index 84a3d502c5a5..dfa6a5cb99e4 100644 >>> --- a/arch/arm64/Makefile >>> +++ b/arch/arm64/Makefile >>> @@ -53,20 +53,12 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable) >>> endif >>> endif >>> >>> +COMPATCC ?= $(CROSS_COMPILE_COMPAT)gcc >>> +export COMPATCC >>> + >>> ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) >>> - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) >>> - >>> - ifeq ($(CONFIG_CC_IS_CLANG), y) >>> - $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built) >>> - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) >>> - $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built) >>> - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) >>> - $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT) >>> - else >>> - export CROSS_COMPILE_COMPAT >>> - export CONFIG_COMPAT_VDSO := y >>> - compat_vdso := -DCONFIG_COMPAT_VDSO=1 >>> - endif >>> + export CONFIG_COMPAT_VDSO := y >>> + compat_vdso := -DCONFIG_COMPAT_VDSO=1 >>> endif >> >> With this change, if I don't have any CROSS_COMPILE_COMPAT in my >> environment, the kernel fails to build because COMPATCC becomes gcc >> which cannot build the vdso32. What I really want is not to warn me, nor >> fail to build the kernel when I don't care about the compat vDSO (e.g. I >> have a 64-bit only machine). >> >> What saved us before was the COMPATCC_IS_ARM_GCC check and a selection >> of the GENERIC_COMPAT_VDSO dependent on this check. This was now dropped >> from the previous version of the patch. We could add something like >> COMPATCC_CAN_LINK. > > Ah, the COMPATCC_CAN_LINK idea doesn't help because an x86 gcc can still > link. Maybe only enable CONFIG_COMPAT_VDSO above if CROSS_COMPILE_COMPAT > was set. You could move the COMPATCC setting and export under the > same 'if' block in the Makefile. > The snippet below the scissors solves the problem for me. I am going to re-post a new version of the patches that contains it. --->8--- diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 37c610963eee..b4cc8fb5344c 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -110,7 +110,7 @@ config ARM64 select GENERIC_STRNLEN_USER select GENERIC_TIME_VSYSCALL select GENERIC_GETTIMEOFDAY - select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT) + select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT && !COMPATCC_EMPTY) select HANDLE_DOMAIN_IRQ select HARDIRQS_SW_RESEND select HAVE_PCI @@ -313,6 +313,9 @@ config KASAN_SHADOW_OFFSET default 0xeffffff900000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS default 0xffffffffffffffff +config COMPATCC_EMPTY + def_bool $(success,test -z "${CROSS_COMPILE_COMPAT}") + source "arch/arm64/Kconfig.platforms" menu "Kernel Features"
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 84a3d502c5a5..dfa6a5cb99e4 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -53,20 +53,12 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable) endif endif +COMPATCC ?= $(CROSS_COMPILE_COMPAT)gcc +export COMPATCC + ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y) - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) - - ifeq ($(CONFIG_CC_IS_CLANG), y) - $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built) - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),) - $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built) - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),) - $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT) - else - export CROSS_COMPILE_COMPAT - export CONFIG_COMPAT_VDSO := y - compat_vdso := -DCONFIG_COMPAT_VDSO=1 - endif + export CONFIG_COMPAT_VDSO := y + compat_vdso := -DCONFIG_COMPAT_VDSO=1 endif KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \ diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 1fba0776ed40..19e0d3115ffe 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -8,8 +8,6 @@ ARCH_REL_TYPE_ABS := R_ARM_JUMP_SLOT|R_ARM_GLOB_DAT|R_ARM_ABS32 include $(srctree)/lib/vdso/Makefile -COMPATCC := $(CROSS_COMPILE_COMPAT)gcc - # Same as cc-*option, but using COMPATCC instead of CC cc32-option = $(call try-run,\ $(COMPATCC) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))