On Wed, Jul 09, 2025 at 11:36:27AM +0200, Geert Uytterhoeven wrote: > Hi Kirill, > > On Wed, 9 Jul 2025 at 11:31, Kirill A. Shutemov > <kirill.shutemov@xxxxxxxxxxxxxxx> wrote: > > On Tue, Jul 08, 2025 at 07:40:35PM -0700, Sohil Mehta wrote: > > > > @@ -664,14 +673,23 @@ static enum kernel_gp_hint get_kernel_gp_address(struct pt_regs *regs, > > > > return GP_NO_HINT; > > > > > > > > #ifdef CONFIG_X86_64 > > > > > > Might as well get rid of the #ifdef in C code, if possible. > > > > > > if (!IS_ENABLED(CONFIG_X86_64) > > > return GP_CANONICAL; > > > > > > or combine it with the next check. > > > > I tried this before. It triggers compiler error on 32-bit: > > > > arch/x86/kernel/traps.c:673:16: error: shift count >= width of type [-Werror,-Wshift-count-overflow] > > 673 | if (*addr >= ~__VIRTUAL_MASK) > > | ^~~~~~~~~~~~~~ > > > > __VIRTUAL_MASK is not usable on 32-bit configs. > > arch/x86/include/asm/page_32_types.h:#define __VIRTUAL_MASK_SHIFT 32 > arch/x86/include/asm/page_32_types.h:#define __VIRTUAL_MASK_SHIFT 32 > arch/x86/include/asm/page_64_types.h:#define __VIRTUAL_MASK_SHIFT > (pgtable_l5_enabled() ? 56 : 47) > arch/x86/include/asm/page_types.h:#define __VIRTUAL_MASK > ((1UL << __VIRTUAL_MASK_SHIFT) - 1) > > Given __VIRTUAL_MASK_SHIFT is 32 on 32-bit platforms, perhaps > __VIRTUAL_MASK should just be changed to shift 1ULL instead? > Or better, use GENMASK(__VIRTUAL_MASK_SHIFT - 1, 0), so the > resulting type is still unsigned long. Making __VIRTUAL_MASK unsigned long long is no-go. Virtual address are unsigned long. I guess GENMASK() would work. I think re-defining __VIRTUAL_MASK is out-of-scope for the patchset. Feel free to prepare a separate patch to do it. -- Kiryl Shutsemau / Kirill A. Shutemov
