Re: [Xen-devel] [PATCH v2 10/15] xen/passthrough: iommu: Basic support of device tree assignment

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Jan,

On 02/24/2014 10:47 AM, Jan Beulich wrote:
>>>> On 23.02.14 at 23:16, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
>> Add IOMMU helpers to support device tree assignment/deassignment. This patch
>> introduces 2 new fields in the dt_device_node:
>>     - is_protected: Does the device is protected by an IOMMU
>>     - next_assigned: Pointer to the next device assigned to the same
>>     domain
>>
>> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
>>
>> ---
>>     Changes in v2:
>>         - Patch added
>> ---
>>  xen/common/device_tree.c              |    4 ++
>>  xen/drivers/passthrough/Makefile      |    1 +
>>  xen/drivers/passthrough/device_tree.c |  106 
>> +++++++++++++++++++++++++++++++++
>>  xen/drivers/passthrough/iommu.c       |   10 ++++
>>  xen/include/xen/device_tree.h         |   14 +++++
>>  xen/include/xen/hvm/iommu.h           |    6 ++
>>  xen/include/xen/iommu.h               |   16 +++++
> 
> No matter how small the changes to generic IOMMU code, you should
> Cc the maintainers.

Oops, sorry.

>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -123,6 +123,12 @@ int iommu_domain_init(struct domain *d)
>>      if ( ret )
>>          return ret;
>>  
>> +#if HAS_DEVICE_TREE
>> +    ret = iommu_dt_domain_init(d);
>> +    if ( ret )
>> +        return ret;
>> +#endif
> 
> Why can this not be part of arch_iommu_domain_init()?

I didn't think about this solution. I will move in
arch_iommu_domain_init in the next version.

>> @@ -198,6 +204,10 @@ void iommu_domain_destroy(struct domain *d)
>>      if ( need_iommu(d) )
>>          iommu_teardown(d);
>>  
>> +#ifdef HAS_DEVICE_TREE
>> +    iommu_dt_domain_destroy(d);
>> +#endif
>> +
>>      arch_iommu_domain_destroy(d);
> 
> And the former one here part of the latter?

Ok.

>> @@ -28,6 +29,11 @@ struct hvm_iommu {
>>  
>>      /* iommu_ops */
>>      const struct iommu_ops *platform_ops;
>> +
>> +    #ifdef HAS_DEVICE_TREE
>> +    /* List of DT devices assigned to this domain */
>> +    struct list_head dt_devices;
>> +    #endif
> 
> Indentation.

Will do.

Cheers,

-- 
Julien Grall

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