Re: [PATCH v2 15/18] IOMMU/x86: prefill newly allocate page tables

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.12.2021 16:15, Roger Pau Monné wrote:
> On Tue, Dec 14, 2021 at 04:05:27PM +0100, Jan Beulich wrote:
>> On 14.12.2021 15:50, Roger Pau Monné wrote:
>>> On Fri, Sep 24, 2021 at 11:54:58AM +0200, Jan Beulich wrote:
>>>> --- a/xen/drivers/passthrough/amd/iommu_map.c
>>>> +++ b/xen/drivers/passthrough/amd/iommu_map.c
>>>> @@ -116,7 +116,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);
>>>
>>> You could even special case (pde - table) % 2 != 0, but this is debug
>>> only code, and it's possible a mod is more costly than
>>> find_first_set_bit.
>>
>> Not sure why I would want to special case anything that doesn't need
>> special casing. The pde == table case needs special care because there
>> find_first_set_bit() cannot be called.
> 
> Well in iommu_alloc_pgtable you already special case odd entries by
> explicitly setting the mask to 0 instead of using find_first_set_bit.

I don't consider this special casing; instead I'm unrolling the loop
4 times to simplify calculations not only for odd entries, but also
for those where index % 4 == 2. Unrolling the loop here just for the
assertions doesn't look very desirable.

Jan