[PATCH] powerpc: Split PAGE_SHIFT/SIZE into vdso/page.h

[Linus Walleij]

On Thu, Dec 21, 2023 at 1:04 PM Michael Ellerman <mpe at ellerman.id.au> wrote:

> The VDSO needs PAGE_SHIFT/SIZE defined, so it includes asm/page.h.
>
> However when COMPAT=y the VDSO is built 32-bit, even though the kernel
> is 64-bit. That can lead to odd warnings because some kernel constants
> are 64-bit, but unsigned long is 32-bit, for example:
>
>     VDSO32C arch/powerpc/kernel/vdso/vgettimeofday-32.o
>   In file included from <built-in>:4:
>   In file included from /home/michael/linux/lib/vdso/gettimeofday.c:5:
>   In file included from ../include/vdso/datapage.h:137:
>   In file included from ../arch/powerpc/include/asm/vdso/gettimeofday.h:7:
>   ../arch/powerpc/include/asm/page.h:230:9: warning: result of comparison of constant 13835058055282163712 with expression of type 'unsigned long' is always true
>     230 |         return __pa(kaddr) >> PAGE_SHIFT;
>         |                ^~~~~~~~~~~
>
> Fix it by moving the PAGE_SHIFT/SIZE constants into a separate header,
> which can be included by the VDSO, and also by the existing kernel
> headers. That avoids exposing the rest of the header to the non-standard
> build environment of the compat VDSO.
>
> The particular warning above was introduced by commit 58b6fed89ab0
> ("powerpc: Make virt_to_pfn() a static inline"), though it is not at
> fault, it just exposed the fact that the VDSO was including parts of
> page.h that weren't needed or appropriate for the VDSO.
>
> Don't copy the comment about page sizes, it just risks becoming
> outdated, that information is better available in the Kconfig
> dependencies and help text.
>
> Fixes: 58b6fed89ab0 ("powerpc: Make virt_to_pfn() a static inline")
> Reported-by: kernel test robot <lkp at intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202311061940.4pBrm44u-lkp@intel.com/
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>

Clearly a working solution!
Acked-by: Linus Walleij <linus.walleij at linaro.org>

Just a note:

> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
(...)
> +#include <asm/vdso/page.h>

(...)
> +++ b/arch/powerpc/include/asm/vdso/page.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _ASM_POWERPC_VDSO_PAGE_H
> +#define _ASM_POWERPC_VDSO_PAGE_H
> +
> +#include <vdso/const.h>

Now the whole kernel includes <vdso/const.h>, is this necessary?

Yours,
Linus Walleij