Re: [PATCH v4 17/21] AMD/IOMMU: replace all-contiguous page tables by superpage mappings

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.05.2022 17:31, Roger Pau Monné wrote:
> On Mon, Apr 25, 2022 at 10:43:16AM +0200, Jan Beulich wrote:
>> @@ -94,11 +95,15 @@ static union amd_iommu_pte set_iommu_pte
>>           old.iw != iw || old.ir != ir )
>>      {
>>          set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
>> -        pt_update_contig_markers(&table->raw, pfn_to_pde_idx(dfn, level),
>> -                                 level, PTE_kind_leaf);
>> +        *contig = pt_update_contig_markers(&table->raw,
>> +                                           pfn_to_pde_idx(dfn, level),
>> +                                           level, PTE_kind_leaf);
>>      }
>>      else
>> +    {
>>          old.pr = false; /* signal "no change" to the caller */
>> +        *contig = false;
> 
> So we assume that any caller getting contig == true must have acted
> and coalesced the page table?

Yes, except that I wouldn't use "must", but "would". It's not a
requirement after all, functionality-wise all will be fine without
re-coalescing.

> Might be worth a comment, to note that the function assumes that a
> previous return of contig == true will have coalesced the page table
> and hence a "no change" PTE write is not expected to happen on a
> contig page table.

I'm not convinced, as there's effectively only one caller,
amd_iommu_map_page(). I also don't see why "no change" would be a
problem. "No change" can't result in a fully contiguous page table
if the page table wasn't fully contiguous already before (at which
point it would have been replaced by a superpage).

Jan