Re: [PATCH v2 1/5] IOMMU: make page table population preemptible

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 13/12/2013 15:41, Jan Beulich wrote:
>>>> On 13.12.13 at 16:09, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 13/12/2013 13:59, Jan Beulich wrote:
>>> @@ -290,15 +307,14 @@ static int assign_device(struct domain *
>>>                     rc);
>>>      }
>>>  
>>> -    if ( has_arch_pdevs(d) && !need_iommu(d) )
>>> + done:
>>> +    if ( !has_arch_pdevs(d) && need_iommu(d) )
>> We now have a case where, for the first device, we could set up
>> pagetables for a large domain, get an error with assignment, then tear
>> them all back down.  (-EBUSY from pci_get_pdev() looks like a good
>> non-fatal candidate for causing this behaviour)
>>
>> I am wondering whether this is better for worse than the race condition
>> where a guest couldn't use the device.  A guest could not reasonably
>> expect to use a device before the toolstack is done setting it up.   A
>> buggy toolstack could quite easily tie up a lot of Xen time creating and
>> destroying complete iommu pagetable sets.
> I don't think it's worth worrying about buggy tool stacks here -
> they should simply get fixed.
>
> Furthermore this change in operation ordering is only a (nice)
> side effect, the necessary cleanup seemed easier with the
> order changed. And said time window would have grown with
> the added preemption handling.
>
> Jan
>

All true.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>