Re: [PATCH v3 17/23] IOMMU/x86: prefill newly allocate page tables

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.02.2022 06:01, Tian, Kevin wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Tuesday, January 11, 2022 12:35 AM
>>
>> Page tables are used for two purposes after allocation: They either
>> start out all empty, or they get filled to replace a superpage.
>> Subsequently, to replace all empty or fully contiguous page tables,
>> contiguous sub-regions will be recorded within individual page tables.
>> Install the initial set of markers immediately after allocation. Make
>> sure to retain these markers when further populating a page table in
>> preparation for it to replace a superpage.
>>
>> The markers are simply 4-bit fields holding the order value of
>> contiguous entries. To demonstrate this, if a page table had just 16
>> entries, this would be the initial (fully contiguous) set of markers:
>>
>> index  0 1 2 3 4 5 6 7 8 9 A B C D E F
>> marker 4 0 1 0 2 0 1 0 3 0 1 0 2 0 1 0
>>
>> "Contiguous" here means not only present entries with successively
>> increasing MFNs, each one suitably aligned for its slot, but also a
>> respective number of all non-present entries.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>, with a nit:

Thanks.

>> @@ -478,7 +478,28 @@ struct page_info *iommu_alloc_pgtable(st
>>          return NULL;
>>
>>      p = __map_domain_page(pg);
>> -    clear_page(p);
>> +
>> +    if ( contig_mask )
>> +    {
>> +        unsigned int i, shift = find_first_set_bit(contig_mask);
>> +
>> +        ASSERT(((PAGE_SHIFT - 3) & (contig_mask >> shift)) == PAGE_SHIFT - 
>> 3);
>> +
>> +        p[0] = (PAGE_SHIFT - 3ull) << shift;
>> +        p[1] = 0;
>> +        p[2] = 1ull << shift;
>> +        p[3] = 0;
>> +
>> +        for ( i = 4; i < PAGE_SIZE / 8; i += 4 )
>> +        {
>> +            p[i + 0] = (find_first_set_bit(i) + 0ull) << shift;
>> +            p[i + 1] = 0;
>> +            p[i + 2] = 1ull << shift;
>> +            p[i + 3] = 0;
>> +        }
> 
> some comment similar to what commit msg describes can improve
> the readability here.

I wouldn't want to replicate what pt-contig-markers.h describes, so
maybe a comment referring there would do?

Jan