On Thu, Dec 21, 2023 at 1:04 PM Michael Ellerman <m...@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 <l...@intel.com> > Closes: > https://lore.kernel.org/oe-kbuild-all/202311061940.4pbrm44u-...@intel.com/ > Signed-off-by: Michael Ellerman <m...@ellerman.id.au> Clearly a working solution! Acked-by: Linus Walleij <linus.wall...@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