[PATCH] arm64: Allow disabling of the compat vDSO

[PATCH] arm64: Allow disabling of the compat vDSO

[Catalin Marinas]

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(-)

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"

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Nick Desaulniers]

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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Catalin Marinas]

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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Nick Desaulniers]

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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Vincenzo Frascino]

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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Vincenzo Frascino]

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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Catalin Marinas]

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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Catalin Marinas]

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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[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

Re: [PATCH] arm64: Allow disabling of the compat vDSO

[Nick Desaulniers]

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