Re: [PATCH v4 13/21] IOMMU/x86: prefill newly allocate page tables

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.05.2022 14:22, Roger Pau Monné wrote:
> On Fri, May 20, 2022 at 01:13:28PM +0200, Jan Beulich wrote:
>> On 20.05.2022 13:11, Jan Beulich wrote:
>>> On 20.05.2022 12:47, Roger Pau Monné wrote:
>>>> On Thu, May 19, 2022 at 02:12:04PM +0200, Jan Beulich wrote:
>>>>> On 06.05.2022 13:16, Roger Pau Monné wrote:
>>>>>> On Mon, Apr 25, 2022 at 10:40:55AM +0200, Jan Beulich wrote:
>>>>>>> --- a/xen/drivers/passthrough/amd/iommu_map.c
>>>>>>> +++ b/xen/drivers/passthrough/amd/iommu_map.c
>>>>>>> @@ -115,7 +115,19 @@ static void set_iommu_ptes_present(unsig
>>>>>>>  
>>>>>>>      while ( nr_ptes-- )
>>>>>>>      {
>>>>>>> -        set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
>>>>>>> +        ASSERT(!pde->next_level);
>>>>>>> +        ASSERT(!pde->u);
>>>>>>> +
>>>>>>> +        if ( pde > table )
>>>>>>> +            ASSERT(pde->ign0 == find_first_set_bit(pde - table));
>>>>>>> +        else
>>>>>>> +            ASSERT(pde->ign0 == PAGE_SHIFT - 3);
>>>>>>
>>>>>> I think PAGETABLE_ORDER would be clearer here.
>>>>>
>>>>> I disagree - PAGETABLE_ORDER is a CPU-side concept. It's not used anywhere
>>>>> in IOMMU code afaics.
>>>>
>>>> Isn't PAGE_SHIFT also a CPU-side concept in the same way?  I'm not
>>>> sure what's the rule for declaring that PAGE_SHIFT is fine to use in
>>>> IOMMU code  but not PAGETABLE_ORDER.
>>>
>>> Hmm, yes and no. But for consistency with other IOMMU code I may want
>>> to switch to PAGE_SHIFT_4K.
>>
>> Except that, with the plan to re-use pt_update_contig_markers() for CPU-
>> side re-coalescing, there I'd prefer to stick to PAGE_SHIFT.
> 
> Then can PAGETABLE_ORDER be used instead of PAGE_SHIFT - 3?

pt_update_contig_markers() isn't IOMMU code; since I've said I'd switch
to PAGE_SHIFT_4K in IOMMU code I'm having a hard time seeing how I could
at the same time start using PAGETABLE_ORDER there.

What I maybe could do is use PTE_PER_TABLE_SHIFT in AMD code and
LEVEL_STRIDE in VT-d one. Yet I'm not sure that would be fully correct/
consistent, ...

> IMO it makes the code quite easier to understand.

... or in fact helping readability.

Jan