Re: [Xen-devel] [RFC 11/19] xen/passthrough: Call arch_iommu_domain_destroy before calling iommu_teardown

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.06.14 at 11:18, <julien.grall@xxxxxxxxxx> wrote:
> Hi Ian,
> 
> On 17/06/14 09:07, Jan Beulich wrote:
>>>>> On 16.06.14 at 18:17, <julien.grall@xxxxxxxxxx> wrote:
>>> --- a/xen/drivers/passthrough/iommu.c
>>> +++ b/xen/drivers/passthrough/iommu.c
>>> @@ -219,10 +219,10 @@ void iommu_domain_destroy(struct domain *d)
>>>       if ( !iommu_enabled || !hd->platform_ops )
>>>           return;
>>>
>>> +    arch_iommu_domain_destroy(d);
>>> +
>>>       if ( need_iommu(d) )
>>>           iommu_teardown(d);
>>> -
>>> -    arch_iommu_domain_destroy(d);
>>
>> At the first glance this doesn't look right, including the explanation
>> you gave (why would devices still be assigned to a guest at this
>> point).
> 
> Because the toolstack may forget to deassign a device. How do you handle 
> this case in x86? In the SMMU case, this will mean a memory leak and 
> misconfiguration of the registers.

Proper tool stack behavior is required (and not just here).

>> And it's rather hard to properly decide with the series here
>> depending on two other series, i.e. there not being a
>> arch_iommu_domain_destroy() at all in current staging.
> 
> Are you sure? The other series doesn't deal with the IOMMU stuff. This 
> change has been pushed upstream a month ago (see commit 4905b35c " 
> iommu: introduce arch specific code").

Oops, indeed - I'm sorry, I looked at a stale branch. Looking at the
correct code I still think the current order is the correct one, and if
you need to take extra steps you ought to do so from the .teardown
hook.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel