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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.11.2024 10:09, Jan Beulich wrote:
> 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.

I think I now can: memnodemap is set using vmap_contig(). The VESA frame buffer
unmap, neglecting to check VA alignment, will have wrongly unmapped memnodemap.

Jan