| Message ID | 20190925130926.50674-1-catalin.marinas@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: Allow disabling of the compat vDSO | expand |
On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > The compat vDSO building requires a cross-compiler than produces AArch32 > binaries, defined via CONFIG_CROSS_COMPILE_COMPAT_VDSO or the > CROSS_COMPILE_COMPAT environment variable. If none of these is defined, > building the kernel always prints a warning as there is no way to > deselect the compat vDSO. > > Add an arm64 Kconfig entry to allow the deselection of the compat vDSO. > In addition, make it an EXPERT option, default n, until other issues > with the compat vDSO are solved (64-bit only kernel headers included in > user-space vDSO code, CC_IS_CLANG irrelevant to CROSS_COMPILE_COMPAT). CC_IS_CLANG might be because then CC can be reused with different flags, rather than providing a different cross compiler binary via config option. > > Fixes: bfe801ebe84f ("arm64: vdso: Enable vDSO compat support") > Cc: Will Deacon <will@kernel.org> > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Link: https://github.com/ClangBuiltLinux/linux/issues/595 Overall, this work is important to Android; the ARMv8-A series of mobile SoCs we see today have to support 32b and 64b (A32+A64?) for at least a few more years; we would like gettimeofday() and friends to be fast for 32b and 64b applications. > --- > > It looks like you can't really win with the current compat vDSO logic. > You either don't have a compat cross-compiler and you get a Makefile > warning or you have one and a get a compiler warning (or failure) > because of the 64-bit kernel headers included in 32-bit user-space code. > > "depends on BROKEN" for ARM64_COMPAT_VDSO also work for me instead of > EXPERT. I'll leave it up to Will to decide but I'd like at least this > patch in -rc2 (even better if everything else is fixed before the final > 5.4). > > Suggestions for future improvements of the compat vDSO handling: > > - replace the CROSS_COMPILE_COMPAT prefix with a full COMPATCC; maybe > check that it indeed produces 32-bit code > > - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which > only checks the native compiler When cross compiling, IIUC CC_IS_CLANG is referring to CC which is the cross compiler, which is different than HOSTCC which is the host compiler. HOSTCC is used mostly for things in scripts/ while CC is used to compile a majority of the kernel in a cross compile. > > - clean up the headers includes; vDSO should not include kernel-only > headers that may even contain code patched at run-time This is a big one; Clang validates the inline asm constraints for extended inline assembly, GCC does not for dead code. So Clang chokes on the inclusion of arm64 headers using extended inline assembly when being compiled for arm-linux-gnueabi. > > arch/arm64/Kconfig | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 866e05882799..83a9a78085c6 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -110,7 +110,6 @@ config ARM64 > select GENERIC_STRNLEN_USER > select GENERIC_TIME_VSYSCALL > select GENERIC_GETTIMEOFDAY > - select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT) > select HANDLE_DOMAIN_IRQ > select HARDIRQS_SW_RESEND > select HAVE_PCI > @@ -1185,6 +1184,15 @@ config KUSER_HELPERS > Say N here only if you are absolutely certain that you do not > need these helpers; otherwise, the safe option is to say Y. > > +config ARM64_COMPAT_VDSO > + bool "Enable the 32-bit vDSO" if EXPERT > + depends on !CPU_BIG_ENDIAN > + select GENERIC_COMPAT_VDSO > + help > + Enable the vDSO support for 32-bit applications. You would > + need to set the 32-bit cross-compiler prefix in > + CONFIG_CROSS_COMPILE_COMPAT_VDSO or the CROSS_COMPILE_COMPAT > + environment variable. > > menuconfig ARMV8_DEPRECATED > bool "Emulate deprecated/obsolete ARMv8 instructions" -- Thanks, ~Nick Desaulniers
On Wed, Sep 25, 2019 at 09:53:16AM -0700, Nick Desaulniers wrote: > On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > > > The compat vDSO building requires a cross-compiler than produces AArch32 > > binaries, defined via CONFIG_CROSS_COMPILE_COMPAT_VDSO or the > > CROSS_COMPILE_COMPAT environment variable. If none of these is defined, > > building the kernel always prints a warning as there is no way to > > deselect the compat vDSO. > > > > Add an arm64 Kconfig entry to allow the deselection of the compat vDSO. > > In addition, make it an EXPERT option, default n, until other issues > > with the compat vDSO are solved (64-bit only kernel headers included in > > user-space vDSO code, CC_IS_CLANG irrelevant to CROSS_COMPILE_COMPAT). > > CC_IS_CLANG might be because then CC can be reused with different > flags, rather than providing a different cross compiler binary via > config option. > > > > > Fixes: bfe801ebe84f ("arm64: vdso: Enable vDSO compat support") > > Cc: Will Deacon <will@kernel.org> > > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > > Thanks for the patch. > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/595 This is just a temporary hiding of the issue, not a complete fix. Vincenzo will do the fix later on. > Overall, this work is important to Android; the ARMv8-A series of > mobile SoCs we see today have to support 32b and 64b (A32+A64?) for at > least a few more years; we would like gettimeofday() and friends to be > fast for 32b and 64b applications. I agree, it just needs some tweaking and hopefully we get most of it fixed in 5.4. > > Suggestions for future improvements of the compat vDSO handling: > > > > - replace the CROSS_COMPILE_COMPAT prefix with a full COMPATCC; maybe > > check that it indeed produces 32-bit code > > > > - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which > > only checks the native compiler > > When cross compiling, IIUC CC_IS_CLANG is referring to CC which is the > cross compiler, which is different than HOSTCC which is the host > compiler. HOSTCC is used mostly for things in scripts/ while CC is > used to compile a majority of the kernel in a cross compile. We need the third compiler here for the compat vDSO (at least with gcc), COMPATCC. I'm tempted to just drop the CONFIG_CROSS_COMPILE_COMPAT_VDSO altogether and only rely on a COMPATCC. This way we can add COMPATCC_IS_CLANG etc. in the Kconfig checks directly. If clang can build both 32 and 64-bit with the same binary (just different options), we could maybe have COMPATCC default to CC and add a check on whether COMPATCC generates 32-bit binaries. > > - clean up the headers includes; vDSO should not include kernel-only > > headers that may even contain code patched at run-time > > This is a big one; Clang validates the inline asm constraints for > extended inline assembly, GCC does not for dead code. So Clang chokes > on the inclusion of arm64 headers using extended inline assembly when > being compiled for arm-linux-gnueabi. Whether clang or gcc, I'd like this fixed anyway. At some point we may inadvertently rely on some code which is patched at boot time for the kernel code but not for the vDSO. Thanks. -- Catalin
On Wed, Sep 25, 2019 at 10:08 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > This is just a temporary hiding of the issue, not a complete fix. Yep. > Vincenzo will do the fix later on. Appreciated, I'm happy to help discuss, review, and test. > > > - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which > > > only checks the native compiler > > > > When cross compiling, IIUC CC_IS_CLANG is referring to CC which is the > > cross compiler, which is different than HOSTCC which is the host > > compiler. HOSTCC is used mostly for things in scripts/ while CC is > > used to compile a majority of the kernel in a cross compile. > > We need the third compiler here for the compat vDSO (at least with gcc), > COMPATCC. I'm tempted to just drop the CONFIG_CROSS_COMPILE_COMPAT_VDSO > altogether and only rely on a COMPATCC. This way we can add > COMPATCC_IS_CLANG etc. in the Kconfig checks directly. Oh, man, yeah 3 compilers in that case: 1. HOSTCC 2. CC 3. COMPATCC > > If clang can build both 32 and 64-bit with the same binary (just > different options), we could maybe have COMPATCC default to CC and add a > check on whether COMPATCC generates 32-bit binaries. Cross compilation work differently between GCC and Clang from a developers perspective. In GCC, at ./configure time you select which architecture you'd like to cross compile for, and you get one binary that targets one architecture. You get a nice compiler that can do mostly static dispatch at the cost of needing multiple binaries in admittedly rare scenarios like the one we have here. Clang defaults to all backends enabled when invoking cmake (though there are options to enable certain backends; Sony for instance enables only x86_64 for their PS4 SDK (and thus I break their build frequently with my arm64 unit tests)). Clang can do all of the above with one binary. The codebase makes heavy use of OOP+virtual dispatch to run ISA specific and general code transformations (virtual dispatch is slower than static dispatch, but relative to what the compiler is actually doing, I suspect the effects are minimal. Folks are also heavily invested in researching "devirtualization"). With one clang binary, I can do: # implicitly uses the host's ISA, for me that's x86_64-linux-gnu $ clang foo.c $ clang -target aarch64-linux-gnu foo.c $ clang -target arm-linux-gnueabi foo.c Admittedly, it's not as simple for the kernel's case; the top level Makefile sets some flags to support invoking Clang from a non-standard location, and telling it where to find binutils because we can't assemble the kernel with LLVM's substitute for GAS). -- Thanks, ~Nick Desaulniers
On 9/25/19 6:31 PM, Nick Desaulniers wrote: > On Wed, Sep 25, 2019 at 10:08 AM Catalin Marinas > <catalin.marinas@arm.com> wrote: >> >> This is just a temporary hiding of the issue, not a complete fix. > > Yep. > >> Vincenzo will do the fix later on. > > Appreciated, I'm happy to help discuss, review, and test. > >>>> - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which >>>> only checks the native compiler >>> >>> When cross compiling, IIUC CC_IS_CLANG is referring to CC which is the >>> cross compiler, which is different than HOSTCC which is the host >>> compiler. HOSTCC is used mostly for things in scripts/ while CC is >>> used to compile a majority of the kernel in a cross compile. >> >> We need the third compiler here for the compat vDSO (at least with gcc), >> COMPATCC. I'm tempted to just drop the CONFIG_CROSS_COMPILE_COMPAT_VDSO >> altogether and only rely on a COMPATCC. This way we can add >> COMPATCC_IS_CLANG etc. in the Kconfig checks directly. > > Oh, man, yeah 3 compilers in that case: > 1. HOSTCC > 2. CC > 3. COMPATCC > The easier way I found to encapsulate the three compilers is using CONFIG_CROSS_COMPILE_COMPAT_VDSO, hence I would prefer to not drop it. In the case of gcc: ------------------- CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%) $(CROSS_COMPILE_COMPAT)gcc ... In the case of clang: --------------------- CLANG_TRIPLE ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%-"=%) clang --target=$(notdir $CLANG_TRIPLE) This will prevent the vdso32 library generation to depend from a fixed configuration that might change in future. Based on this work we will be able to add COMPAT_CC_IS_CLANG, COMPAT_CC_IS_GCC and COMPAT_CC_GCC_VERSION, COMPAT_CC_CLANG_VERSION which will help us in a more fine grain control of the compiler versions. The clang support will be added shortly after the header problems have been addressed, because without that and the possibility to have 32bit headers in arm64 would result in a broken target. >> >> If clang can build both 32 and 64-bit with the same binary (just >> different options), we could maybe have COMPATCC default to CC and add a >> check on whether COMPATCC generates 32-bit binaries. > > Cross compilation work differently between GCC and Clang from a > developers perspective. In GCC, at ./configure time you select which > architecture you'd like to cross compile for, and you get one binary > that targets one architecture. You get a nice compiler that can do > mostly static dispatch at the cost of needing multiple binaries in > admittedly rare scenarios like the one we have here. Clang defaults > to all backends enabled when invoking cmake (though there are options > to enable certain backends; Sony for instance enables only x86_64 for > their PS4 SDK (and thus I break their build frequently with my arm64 > unit tests)). > > Clang can do all of the above with one binary. The codebase makes > heavy use of OOP+virtual dispatch to run ISA specific and general code > transformations (virtual dispatch is slower than static dispatch, but > relative to what the compiler is actually doing, I suspect the effects > are minimal. Folks are also heavily invested in researching > "devirtualization"). With one clang binary, I can do: > > # implicitly uses the host's ISA, for me that's x86_64-linux-gnu > $ clang foo.c > $ clang -target aarch64-linux-gnu foo.c > $ clang -target arm-linux-gnueabi foo.c > > Admittedly, it's not as simple for the kernel's case; the top level > Makefile sets some flags to support invoking Clang from a non-standard > location, and telling it where to find binutils because we can't > assemble the kernel with LLVM's substitute for GAS). > Thank you for the explanation, if I understand it correctly the strategy proposed above should cover all the cased proposed. Please, let me know if i need to tweak something. The plan is to use binutils to build the vdso libraries with both the compilers. -- Regards, Vincenzo
On 9/25/19 6:08 PM, Catalin Marinas wrote: > On Wed, Sep 25, 2019 at 09:53:16AM -0700, Nick Desaulniers wrote: >> On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote: >>> >>> The compat vDSO building requires a cross-compiler than produces AArch32 >>> binaries, defined via CONFIG_CROSS_COMPILE_COMPAT_VDSO or the >>> CROSS_COMPILE_COMPAT environment variable. If none of these is defined, >>> building the kernel always prints a warning as there is no way to >>> deselect the compat vDSO. >>> >>> Add an arm64 Kconfig entry to allow the deselection of the compat vDSO. >>> In addition, make it an EXPERT option, default n, until other issues >>> with the compat vDSO are solved (64-bit only kernel headers included in >>> user-space vDSO code, CC_IS_CLANG irrelevant to CROSS_COMPILE_COMPAT). >> >> CC_IS_CLANG might be because then CC can be reused with different >> flags, rather than providing a different cross compiler binary via >> config option. >> >>> >>> Fixes: bfe801ebe84f ("arm64: vdso: Enable vDSO compat support") >>> Cc: Will Deacon <will@kernel.org> >>> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> >>> Cc: Thomas Gleixner <tglx@linutronix.de> >>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> >> >> Thanks for the patch. >> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> >> Link: https://github.com/ClangBuiltLinux/linux/issues/595 > > This is just a temporary hiding of the issue, not a complete fix. > Vincenzo will do the fix later on. > >> Overall, this work is important to Android; the ARMv8-A series of >> mobile SoCs we see today have to support 32b and 64b (A32+A64?) for at >> least a few more years; we would like gettimeofday() and friends to be >> fast for 32b and 64b applications. > > I agree, it just needs some tweaking and hopefully we get most of it > fixed in 5.4. > >>> Suggestions for future improvements of the compat vDSO handling: >>> >>> - replace the CROSS_COMPILE_COMPAT prefix with a full COMPATCC; maybe >>> check that it indeed produces 32-bit code >>> CROSS_COMPILE_COMPAT is called like this for symmetry with CROSS_COMPILE. >>> - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which >>> only checks the native compiler >> >> When cross compiling, IIUC CC_IS_CLANG is referring to CC which is the >> cross compiler, which is different than HOSTCC which is the host >> compiler. HOSTCC is used mostly for things in scripts/ while CC is >> used to compile a majority of the kernel in a cross compile. > > We need the third compiler here for the compat vDSO (at least with gcc), > COMPATCC. I'm tempted to just drop the CONFIG_CROSS_COMPILE_COMPAT_VDSO > altogether and only rely on a COMPATCC. This way we can add > COMPATCC_IS_CLANG etc. in the Kconfig checks directly. > > If clang can build both 32 and 64-bit with the same binary (just > different options), we could maybe have COMPATCC default to CC and add a > check on whether COMPATCC generates 32-bit binaries. > clang requires the --target option for specifying the 32bit triple. Basically $(TRIPLE)-gcc is equivalent to gcc --target $(TRIPLE). We need a configuration option to encode this. >>> - clean up the headers includes; vDSO should not include kernel-only >>> headers that may even contain code patched at run-time >> >> This is a big one; Clang validates the inline asm constraints for >> extended inline assembly, GCC does not for dead code. So Clang chokes >> on the inclusion of arm64 headers using extended inline assembly when >> being compiled for arm-linux-gnueabi. > > Whether clang or gcc, I'd like this fixed anyway. At some point we may > inadvertently rely on some code which is patched at boot time for the > kernel code but not for the vDSO. > Do we have any code of this kind in header files? The vDSO library uses only a subset of the headers (mainly Macros) hence all the unused symbols should be compiled out. Is your concern only theoretical or do you have an example on where this could be happening? > Thanks. > -- Regards, Vincenzo
On Thu, Sep 26, 2019 at 01:06:50AM +0100, Vincenzo Frascino wrote: > On 9/25/19 6:08 PM, Catalin Marinas wrote: > > On Wed, Sep 25, 2019 at 09:53:16AM -0700, Nick Desaulniers wrote: > >> On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > >>> Suggestions for future improvements of the compat vDSO handling: > >>> > >>> - replace the CROSS_COMPILE_COMPAT prefix with a full COMPATCC; maybe > >>> check that it indeed produces 32-bit code > > CROSS_COMPILE_COMPAT is called like this for symmetry with CROSS_COMPILE. Actually, what gets in the way is the CONFIG_CROSS_COMPILE_COMPAT_VDSO. We can keep CROSS_COMPILE_COMPAT together with COMPATCC initialised to $(CROSS_COMPILE_COMPAT)gcc. When we will be able to build the compat vDSO with clang, we just pass COMPATCC=clang on the make line and the kernel Makefile will figure out the --target option from CROSS_COMPILE_COMPAT (see how CLANG_FLAGS is handled). If we stick only to env variables or make cmd line (without Kconfig) for the compiler name, we can add a COMPATCC_IS_CLANG in the Kconfig directly and simply not allow the enabling the COMPAT_VDSO if we don't have the right compiler. This saves us warnings during build. > >>> - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which > >>> only checks the native compiler > >> > >> When cross compiling, IIUC CC_IS_CLANG is referring to CC which is the > >> cross compiler, which is different than HOSTCC which is the host > >> compiler. HOSTCC is used mostly for things in scripts/ while CC is > >> used to compile a majority of the kernel in a cross compile. > > > > We need the third compiler here for the compat vDSO (at least with gcc), > > COMPATCC. I'm tempted to just drop the CONFIG_CROSS_COMPILE_COMPAT_VDSO > > altogether and only rely on a COMPATCC. This way we can add > > COMPATCC_IS_CLANG etc. in the Kconfig checks directly. > > > > If clang can build both 32 and 64-bit with the same binary (just > > different options), we could maybe have COMPATCC default to CC and add a > > check on whether COMPATCC generates 32-bit binaries. > > clang requires the --target option for specifying the 32bit triple. > Basically $(TRIPLE)-gcc is equivalent to gcc --target $(TRIPLE). > We need a configuration option to encode this. Since we don't have a CONFIG_* option for the cross-compiler prefix, we shouldn't have one for the compat compiler either. If you want to build the compat vDSO with clang, just pass COMPATCC=clang together with CROSS_COMPILE_COMPAT. We can add Kconfig checks to actually verify that COMPATCC generates 32-bit binaries (e.g. COMPATCC_CAN_LINK32). > >>> - clean up the headers includes; vDSO should not include kernel-only > >>> headers that may even contain code patched at run-time > >> > >> This is a big one; Clang validates the inline asm constraints for > >> extended inline assembly, GCC does not for dead code. So Clang chokes > >> on the inclusion of arm64 headers using extended inline assembly when > >> being compiled for arm-linux-gnueabi. > > > > Whether clang or gcc, I'd like this fixed anyway. At some point we may > > inadvertently rely on some code which is patched at boot time for the > > kernel code but not for the vDSO. > > Do we have any code of this kind in header files? > > The vDSO library uses only a subset of the headers (mainly Macros) hence all the > unused symbols should be compiled out. Is your concern only theoretical or do > you have an example on where this could be happening? At the moment it's rather theoretical. -- Catalin
On Thu, Sep 26, 2019 at 08:47:18AM +0100, Catalin Marinas wrote: > On Thu, Sep 26, 2019 at 01:06:50AM +0100, Vincenzo Frascino wrote: > > On 9/25/19 6:08 PM, Catalin Marinas wrote: > > > On Wed, Sep 25, 2019 at 09:53:16AM -0700, Nick Desaulniers wrote: > > >> On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > >>> - clean up the headers includes; vDSO should not include kernel-only > > >>> headers that may even contain code patched at run-time > > >> > > >> This is a big one; Clang validates the inline asm constraints for > > >> extended inline assembly, GCC does not for dead code. So Clang chokes > > >> on the inclusion of arm64 headers using extended inline assembly when > > >> being compiled for arm-linux-gnueabi. > > > > > > Whether clang or gcc, I'd like this fixed anyway. At some point we may > > > inadvertently rely on some code which is patched at boot time for the > > > kernel code but not for the vDSO. > > > > Do we have any code of this kind in header files? > > > > The vDSO library uses only a subset of the headers (mainly Macros) hence all the > > unused symbols should be compiled out. Is your concern only theoretical or do > > you have an example on where this could be happening? > > At the moment it's rather theoretical. Actually, it's not. The moment the compat vdso Makefile needs the line below, we are doing it wrong: VDSO_CFLAGS += -D__uint128_t='void*' -- Catalin
On Thu, Sep 26, 2019 at 12:47 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Thu, Sep 26, 2019 at 01:06:50AM +0100, Vincenzo Frascino wrote: > > On 9/25/19 6:08 PM, Catalin Marinas wrote: > > > On Wed, Sep 25, 2019 at 09:53:16AM -0700, Nick Desaulniers wrote: > > >> On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > >>> Suggestions for future improvements of the compat vDSO handling: > > >>> > > >>> - replace the CROSS_COMPILE_COMPAT prefix with a full COMPATCC; maybe > > >>> check that it indeed produces 32-bit code > > > > CROSS_COMPILE_COMPAT is called like this for symmetry with CROSS_COMPILE. > > Actually, what gets in the way is the CONFIG_CROSS_COMPILE_COMPAT_VDSO. > We can keep CROSS_COMPILE_COMPAT together with COMPATCC initialised to > $(CROSS_COMPILE_COMPAT)gcc. When we will be able to build the compat > vDSO with clang, we just pass COMPATCC=clang on the make line and the > kernel Makefile will figure out the --target option from > CROSS_COMPILE_COMPAT (see how CLANG_FLAGS is handled). > > If we stick only to env variables or make cmd line (without Kconfig) for > the compiler name, we can add a COMPATCC_IS_CLANG in the Kconfig > directly and simply not allow the enabling the COMPAT_VDSO if we don't > have the right compiler. This saves us warnings during build. Yes, I think this would be a nice approach. -- Thanks, ~Nick Desaulniers
On Thu, Sep 26, 2019 at 8:51 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Thu, Sep 26, 2019 at 08:47:18AM +0100, Catalin Marinas wrote: > > On Thu, Sep 26, 2019 at 01:06:50AM +0100, Vincenzo Frascino wrote: > > > On 9/25/19 6:08 PM, Catalin Marinas wrote: > > > > On Wed, Sep 25, 2019 at 09:53:16AM -0700, Nick Desaulniers wrote: > > > >> On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > > >>> - clean up the headers includes; vDSO should not include kernel-only > > > >>> headers that may even contain code patched at run-time > > > >> > > > >> This is a big one; Clang validates the inline asm constraints for > > > >> extended inline assembly, GCC does not for dead code. So Clang chokes > > > >> on the inclusion of arm64 headers using extended inline assembly when > > > >> being compiled for arm-linux-gnueabi. This case is very much real (not sure if Vincenzo was asking me or Catalin), see report at the bottom of this comment: https://github.com/ClangBuiltLinux/linux/issues/595#issuecomment-509874891 > > > > > > > > Whether clang or gcc, I'd like this fixed anyway. At some point we may > > > > inadvertently rely on some code which is patched at boot time for the > > > > kernel code but not for the vDSO. > > > > > > Do we have any code of this kind in header files? > > > > > > The vDSO library uses only a subset of the headers (mainly Macros) hence all the > > > unused symbols should be compiled out. Is your concern only theoretical or do > > > you have an example on where this could be happening? > > > > At the moment it's rather theoretical. > > Actually, it's not. The moment the compat vdso Makefile needs the line > below, we are doing it wrong: > > VDSO_CFLAGS += -D__uint128_t='void*' *yikes* -- Thanks, ~Nick Desaulniers
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 866e05882799..83a9a78085c6 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -110,7 +110,6 @@ config ARM64 select GENERIC_STRNLEN_USER select GENERIC_TIME_VSYSCALL select GENERIC_GETTIMEOFDAY - select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT) select HANDLE_DOMAIN_IRQ select HARDIRQS_SW_RESEND select HAVE_PCI @@ -1185,6 +1184,15 @@ config KUSER_HELPERS Say N here only if you are absolutely certain that you do not need these helpers; otherwise, the safe option is to say Y. +config ARM64_COMPAT_VDSO + bool "Enable the 32-bit vDSO" if EXPERT + depends on !CPU_BIG_ENDIAN + select GENERIC_COMPAT_VDSO + help + Enable the vDSO support for 32-bit applications. You would + need to set the 32-bit cross-compiler prefix in + CONFIG_CROSS_COMPILE_COMPAT_VDSO or the CROSS_COMPILE_COMPAT + environment variable. menuconfig ARMV8_DEPRECATED bool "Emulate deprecated/obsolete ARMv8 instructions"
The compat vDSO building requires a cross-compiler than produces AArch32 binaries, defined via CONFIG_CROSS_COMPILE_COMPAT_VDSO or the CROSS_COMPILE_COMPAT environment variable. If none of these is defined, building the kernel always prints a warning as there is no way to deselect the compat vDSO. Add an arm64 Kconfig entry to allow the deselection of the compat vDSO. In addition, make it an EXPERT option, default n, until other issues with the compat vDSO are solved (64-bit only kernel headers included in user-space vDSO code, CC_IS_CLANG irrelevant to CROSS_COMPILE_COMPAT). Fixes: bfe801ebe84f ("arm64: vdso: Enable vDSO compat support") Cc: Will Deacon <will@kernel.org> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> --- It looks like you can't really win with the current compat vDSO logic. You either don't have a compat cross-compiler and you get a Makefile warning or you have one and a get a compiler warning (or failure) because of the 64-bit kernel headers included in 32-bit user-space code. "depends on BROKEN" for ARM64_COMPAT_VDSO also work for me instead of EXPERT. I'll leave it up to Will to decide but I'd like at least this patch in -rc2 (even better if everything else is fixed before the final 5.4). Suggestions for future improvements of the compat vDSO handling: - replace the CROSS_COMPILE_COMPAT prefix with a full COMPATCC; maybe check that it indeed produces 32-bit code - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which only checks the native compiler - clean up the headers includes; vDSO should not include kernel-only headers that may even contain code patched at run-time arch/arm64/Kconfig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)