Re: [PATCH v5 2/9] iommu/arm: Add iommu_dt_xlate()

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 10/24/23 14:39, Julien Grall wrote:
> Hi Stewart,
> 
> On 04/10/2023 15:55, Stewart Hildebrand wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>
>> Move code for processing DT IOMMU specifier to a separate helper.
>> This helper will be re-used for adding PCI devices by the subsequent
>> patches as we will need exact the same actions for processing
>> DT PCI-IOMMU specifier.
>>
>> While at it introduce NO_IOMMU to avoid magic "1".
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>> ---
>> v4->v5:
>> * rebase on top of "dynamic node programming using overlay dtbo" series
>> * move #define NO_IOMMU 1 to header
>> * s/these/this/ inside comment
>>
>> v3->v4:
>> * make dt_phandle_args *iommu_spec const
>> * move !ops->add_device check to helper
>>
>> v2->v3:
>> * no change
>>
>> v1->v2:
>> * no change
>>
>> downstream->v1:
>> * trivial rebase
>> * s/dt_iommu_xlate/iommu_dt_xlate/
>>
>> (cherry picked from commit c26bab0415ca303df86aba1d06ef8edc713734d3 from
>>   the downstream branch poc/pci-passthrough from
>>   https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git)
>> ---
>>   xen/drivers/passthrough/device_tree.c | 48 +++++++++++++++++----------
>>   xen/include/xen/iommu.h               |  2 ++
>>   2 files changed, 32 insertions(+), 18 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/device_tree.c 
>> b/xen/drivers/passthrough/device_tree.c
>> index 075fb25a3706..159ace9856c9 100644
>> --- a/xen/drivers/passthrough/device_tree.c
>> +++ b/xen/drivers/passthrough/device_tree.c
>> @@ -137,6 +137,30 @@ int iommu_release_dt_devices(struct domain *d)
>>       return 0;
>>   }
>>
>> +static int iommu_dt_xlate(struct device *dev,
>> +                          const struct dt_phandle_args *iommu_spec)
>> +{
>> +    const struct iommu_ops *ops = iommu_get_ops();
> 
> NIT: Rather than calling iommu_get_ops(), how about passing the ops as a
> parameter of iommu_dt_xlate()?

Yes, will do.

> 
> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Thanks!

> 
> Cheers,
> 
> -- 
> Julien Grall