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
[off-list ref] wrote:

quoted

On Tue, Jul 08, 2025 at 07:40:35PM -0700, Sohil Mehta wrote:

quoted

quoted

@@ -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