Re: [PATCH v3 2/4] x86/mm: skip super-page alignment checks for non-present entries

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.11.2024 15:57, Roger Pau Monne wrote:
> @@ -5517,7 +5524,8 @@ int map_pages_to_xen(
>          L3T_LOCK(current_l3page);
>          ol3e = *pl3e;
>  
> -        if ( cpu_has_page1gb && IS_L3E_ALIGNED(virt, mfn) &&
> +        if ( cpu_has_page1gb &&
> +             (!(flags & _PAGE_PRESENT) || IS_L3E_ALIGNED(virt, mfn)) &&
>               nr_mfns >= (1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) &&
>               !(flags & (_PAGE_PAT | MAP_SMALL_PAGES)) )
>          {
> @@ -5636,7 +5644,7 @@ int map_pages_to_xen(
>          if ( !pl2e )
>              goto out;
>  
> -        if ( IS_L2E_ALIGNED(virt, mfn) &&
> +        if ( (!(flags & _PAGE_PRESENT) || IS_L2E_ALIGNED(virt, mfn)) &&
>               (nr_mfns >= (1u << PAGETABLE_ORDER)) &&
>               !(flags & (_PAGE_PAT|MAP_SMALL_PAGES)) )
>          {

Upon inspecting Andrew's report of crashes I noticed that this can't be quite
right. We can't entirely skip the alignment check when non-present mappings
are requested; we merely need to limit the check to the VA. In a reply to
the 1st v2 I actually had it arranged to match that requirement:

        if ( (cpu_has_page1gb || !(flags & _PAGE_PRESENT)) &&
             IS_L3E_ALIGNED(virt, flags & _PAGE_PRESENT ? mfn : _mfn(0)) &&
             nr_mfns >= (1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) &&
             !(flags & (_PAGE_PAT | MAP_SMALL_PAGES)) )

Yet then I didn't pay attention to the difference when reviewing the 2nd v2
(that versioning issue of course isn't helping here either).

I'm afraid I can't (yet) connect the observed bad behavior with this issue,
though.

Jan