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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.06.14 at 14:24, <julien.grall@xxxxxxxxxx> wrote:
> On 06/17/2014 02:04 PM, Jan Beulich wrote:
>>>>> On 17.06.14 at 14:38, <julien.grall@xxxxxxxxxx> wrote:
>>> On 06/17/2014 10:29 AM, Jan Beulich wrote:
>>>>>>> On 17.06.14 at 11:18, <julien.grall@xxxxxxxxxx> wrote:
>>>>> 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).
>>>
>>> I think this is important to handle toolstack failure (such as crash)
>>> just in case. Hence it doesn't add much code for this purpose.
>> 
>> If you think this is necessary, then there's no reason to make this
>> ARM-specific (which in turn would eliminate the need for this to sit
>> in an arch hook).
> 
> We will have to do DT/PCI specific as I don't use the same way to know
> which device is assigned to which guest.

Which sounds wrong from a design pov. But again, it's the tool stack
maintainers' call in the end.

Jan


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