Re: [PATCH v2 1/4] x86/mm: introduce helpers to detect super page alignment

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.11.2024 13:29, Roger Pau Monne wrote:
> --- a/xen/arch/x86/include/asm/page.h
> +++ b/xen/arch/x86/include/asm/page.h
> @@ -200,6 +200,12 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa, 
> unsigned int flags)
>  #define l4_table_offset(a)         \
>      (((a) >> L4_PAGETABLE_SHIFT) & (L4_PAGETABLE_ENTRIES - 1))
>  
> +/* Check if an address is aligned for a given slot level. */
> +#define SLOT_IS_ALIGNED(v, m, s) \
> +    IS_ALIGNED(PFN_DOWN(v) | mfn_x(m), (1UL << ((s) - PAGE_SHIFT)) - 1)

The check involving an address and an MFN, I think the comment would better
also reflect this. "Check if a (va,mfn) tuple is suitably aligned to be
mapped by a large page at a given page table level"?

As to the name of this helper macro - "SLOT" can mean about anything when
not further qualified. If the macro was local to ...

> +#define IS_L3E_ALIGNED(v, m) SLOT_IS_ALIGNED(v, m, L3_PAGETABLE_SHIFT)
> +#define IS_L2E_ALIGNED(v, m) SLOT_IS_ALIGNED(v, m, L2_PAGETABLE_SHIFT)
> +
>  /* Convert a pointer to a page-table entry into pagetable slot index. */
>  #define pgentry_ptr_to_slot(_p)    \
>      (((unsigned long)(_p) & ~PAGE_MASK) / sizeof(*(_p)))
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c

... this (sole) file using the derived ones, that might be acceptable. If
it's to remain in page.h, how about e.g. IS_LnE_ALIGNED()?

I further wonder whether it wouldn't be neater if just the level was passed
into the helper. Doing so wouldn't even require token concatenation (which
iirc both you and Andrew don't like in situations like this one), as the
mask can be calculated from just level and PAGETABLE_ORDER. At which point
it may even make sense to leave out the wrapper macros.

Jan