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@xxxxxxx> 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@xxxxxxxxxx>
>>> Cc: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
>>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>>> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
>>
>> Thanks for the patch.
>> Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
>> 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