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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.11.2024 12:31, Roger Pau Monne wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5232,6 +5232,12 @@ int map_pages_to_xen(
>      }                                          \
>  } while (0)
>  
> +/* Check if a (virt, mfn) tuple is aligned for a given slot level. */
> +#define IS_LnE_ALIGNED(v, m, n) \
> +    IS_ALIGNED(PFN_DOWN(v) | mfn_x(m), (1UL << (PAGETABLE_ORDER * (n - 1))) 
> - 1)
> +#define IS_L2E_ALIGNED(v, m) IS_LnE_ALIGNED(v, m, 2)
> +#define IS_L3E_ALIGNED(v, m) IS_LnE_ALIGNED(v, m, 3)
> +
>      L3T_INIT(current_l3page);
>  
>      while ( nr_mfns != 0 )
> @@ -5249,9 +5255,7 @@ int map_pages_to_xen(
>          L3T_LOCK(current_l3page);
>          ol3e = *pl3e;
>  
> -        if ( cpu_has_page1gb &&
> -             !(((virt >> PAGE_SHIFT) | mfn_x(mfn)) &
> -               ((1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1)) &&
> +        if ( cpu_has_page1gb && IS_L3E_ALIGNED(virt, mfn) &&
>               nr_mfns >= (1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) &&
>               !(flags & (_PAGE_PAT | MAP_SMALL_PAGES)) )
>          {
> @@ -5370,8 +5374,7 @@ int map_pages_to_xen(
>          if ( !pl2e )
>              goto out;
>  
> -        if ( ((((virt >> PAGE_SHIFT) | mfn_x(mfn)) &
> -               ((1u << PAGETABLE_ORDER) - 1)) == 0) &&
> +        if ( IS_L2E_ALIGNED(virt, mfn) &&
>               (nr_mfns >= (1u << PAGETABLE_ORDER)) &&
>               !(flags & (_PAGE_PAT|MAP_SMALL_PAGES)) )
>          {
> @@ -5541,9 +5544,7 @@ int map_pages_to_xen(

About 50 lines up from here there's another place where I think
IS_L2E_ALIGNED() wants using. That becomes noticeable with the remark
I'm about to make on patch 2, on this ...

>   check_l3:
>          if ( cpu_has_page1gb &&
>               (flags == PAGE_HYPERVISOR) &&
> -             ((nr_mfns == 0) ||
> -              !(((virt >> PAGE_SHIFT) | mfn_x(mfn)) &
> -                ((1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1))) )
> +             ((nr_mfns == 0) || IS_L3E_ALIGNED(virt, mfn)) )
>          {

... similar construct (checking "flags == PAGE_HYPERVISOR"). Not sure
if I should also make this kind of an adjustment while committing ...

Jan