Re: [PATCH v8 09/14] x86/mm: LAM compatible non-canonical definition

[Sean Christopherson <seanjc@google.com>]

On Fri, Jan 16, 2026, Andrey Ryabinin wrote:
> On 1/12/26 6:28 PM, Maciej Wieczor-Retman wrote:
> > diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
> > index bcf5cad3da36..b7940fa49e64 100644
> > --- a/arch/x86/include/asm/page.h
> > +++ b/arch/x86/include/asm/page.h
> > @@ -82,9 +82,22 @@ static __always_inline void *pfn_to_kaddr(unsigned long pfn)
> >  	return __va(pfn << PAGE_SHIFT);
> >  }
> >  
> > +#ifdef CONFIG_KASAN_SW_TAGS
> > +#define CANONICAL_MASK(vaddr_bits) (BIT_ULL(63) | BIT_ULL((vaddr_bits) - 1))
> 
> why is the choice of CANONICAL_MASK() gated at compile time? Shouldn’t this be a
> runtime decision based on whether LAM is enabled or not on the running system?
>  
> > +#else
> > +#define CANONICAL_MASK(vaddr_bits) GENMASK_ULL(63, vaddr_bits)
> > +#endif
> > +
> > +/*
> > + * To make an address canonical either set or clear the bits defined by the
> > + * CANONICAL_MASK(). Clear the bits for userspace addresses if the top address
> > + * bit is a zero. Set the bits for kernel addresses if the top address bit is a
> > + * one.
> > + */
> >  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
> 
> +Cc KVM

Thanks!

> This is used extensively in KVM code. As far as I can tell, it may be used to
> determine whether a guest virtual address is canonical or not.

Yep, KVM uses this both to check canonical addresses and to force a canonical
address (Intel and AMD disagree on the MSR_IA32_SYSENTER_{EIP,ESP} semantics in
64-bit mode) for guest addresses.  This change will break KVM badly if KASAN_SW_TAGS=y.

> case, the result should depend on whether LAM is enabled for the guest, not
> the host (and certainly not a host's compile-time option).

Ya, KVM could roll its own versions, but IMO these super low level helpers should
do exactly what they say.  E.g. at a glance, I'm not sure pt_event_addr_filters_sync()
should be subjected to KASAN_SW_TAGS either.  If that's true, then AFAICT the
_only_ code that wants the LAM-aware behavior is copy_from_kernel_nofault_allowed(),
so maybe just handle it there?  Not sure that's a great long-term maintenance
story either though.