Re: [PATCH v1 1/2] vdso: Add VDSO_UNALIGNED_PACKED config option for ARMv5

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] vdso: VDSO_UNALIGNED_PACKED default omits non-ARMv5 strict-alignment targets
--

commit 1a3f0256f7d442ba6867246bd156ba9784b5b9a5
Author: Ian Rogers <irogers@google.com>

vdso: Add VDSO_UNALIGNED_PACKED config option for ARMv5

This commit adds CONFIG_VDSO_UNALIGNED_PACKED, defaulted to CPU_32v5, to
restore the packed struct implementation of __get_unaligned_t() and
__put_unaligned_t() on ARMv5 where GCC may emit out-of-line memcpy() calls
for __builtin_memcpy().

> diff --git a/lib/vdso/Kconfig b/lib/vdso/Kconfig
> index 597f5f0f96819..709eb193999fd 100644
> --- a/lib/vdso/Kconfig
> +++ b/lib/vdso/Kconfig
> @@ -23,3 +23,11 @@ config VDSO_GETRANDOM
>  	select VDSO_DATASTORE
>  	help
>  	  Selected by architectures that support vDSO getrandom().
> +
> +config VDSO_UNALIGNED_PACKED
> +	bool
> +	default CPU_32v5
> +	help
> +	  Use packed structs rather than memcpy() in include/vdso/unaligned.h
> +	  for unaligned memory accesses. Enabled on ARMv5 where GCC may emit
> +	  out-of-line memcpy() calls instead of inlining byte accesses.

[Severity: Medium]
Does defaulting VDSO_UNALIGNED_PACKED only to CPU_32v5 leave out other
strict-alignment targets that experience the same GCC __builtin_memcpy()
regression from commit a339671db64b ("vdso: Switch get/put_unaligned() from
packed struct to memcpy()")?

include/linux/unaligned.h includes include/vdso/unaligned.h to define
get_unaligned() and put_unaligned() across the entire kernel and boot
decompressors. On strict-alignment targets, GCC cannot fold
__builtin_memcpy() into a scalar access when the pointer alignment is
smaller than the type alignment, resulting in stack spills or out-of-line
memcpy() calls instead of the register shifts generated for a __packed
struct access.

In arch/arm/Makefile, ARMv3, ARMv4, and ARMv4T targets use separate Kconfig
symbols without selecting CPU_32v5, and !CONFIG_MMU builds pass
-mno-unaligned-access:

    ifeq ($(CONFIG_MMU),)
    CFLAGS_ABI	+=-mno-unaligned-access
    endif
    ...
    arch-$(CONFIG_CPU_32v5)		:=-march=armv5te
    arch-$(CONFIG_CPU_32v4T)	:=-march=armv4t
    arch-$(CONFIG_CPU_32v4)		:=-march=armv4
    arch-$(CONFIG_CPU_32v3)		:=-march=armv3m

In arch/arm/Kconfig, hardware unaligned access support is distinguished via
HAVE_EFFICIENT_UNALIGNED_ACCESS:

    select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU

Should VDSO_UNALIGNED_PACKED default to !HAVE_EFFICIENT_UNALIGNED_ACCESS (or
could include/vdso/unaligned.h use a single __packed __may_alias definition)
so that ARMv3, ARMv4, ARMv4T, ARM !MMU, and other strict-alignment
architectures are covered as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019201257.3805356-1-irogers@google.com?part=1