Re: [PATCH v1 6/9] iommu/tegra: gart: Ignore devices without IOMMU phandle in DT

[Dmitry Osipenko]

Hi Robin,

On 11.05.2018 14:34, Robin Murphy wrote:
> Hi Dmitry,
> 
> On 08/05/18 19:16, Dmitry Osipenko wrote:
>> GART can't handle all devices, ignore devices that aren't related to GART.
>> Device tree must explicitly assign GART IOMMU to the devices.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> ---
>> Â drivers/iommu/tegra-gart.c | 33 ++++++++++++++++++++++++++++++++-
>> Â 1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
>> index 39305224c48d..5b2d27620350 100644
>> --- a/drivers/iommu/tegra-gart.c
>> +++ b/drivers/iommu/tegra-gart.c
>> @@ -366,6 +366,26 @@ static void gart_iommu_remove_device(struct device *dev)
>> ÂÂÂÂÂ iommu_device_unlink(&gart_handle->iommu, dev);
>> Â }
>> Â +static int gart_iommu_check_device(struct gart_device *gart,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device *dev);
>> +
>> +struct iommu_group *gart_iommu_device_group(struct device *dev)
>> +{
>> +ÂÂÂ int err;
>> +
>> +ÂÂÂ err = gart_iommu_check_device(gart_handle, dev);
>> +ÂÂÂ if (err)
>> +ÂÂÂÂÂÂÂ return ERR_PTR(err);
>> +
>> +ÂÂÂ return generic_device_group(dev);
>> +}
>> +
>> +static int gart_iommu_of_xlate(struct device *dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct of_phandle_args *args)
>> +{
>> +ÂÂÂ return 0;
>> +}
>> +
>> Â static const struct iommu_ops gart_iommu_ops = {
>> ÂÂÂÂÂ .capableÂÂÂ = gart_iommu_capable,
>> ÂÂÂÂÂ .domain_allocÂÂÂ = gart_iommu_domain_alloc,
>> @@ -374,14 +394,24 @@ static const struct iommu_ops gart_iommu_ops = {
>> ÂÂÂÂÂ .detach_devÂÂÂ = gart_iommu_detach_dev,
>> ÂÂÂÂÂ .add_deviceÂÂÂ = gart_iommu_add_device,
>> ÂÂÂÂÂ .remove_deviceÂÂÂ = gart_iommu_remove_device,
>> -ÂÂÂ .device_groupÂÂÂ = generic_device_group,
>> +ÂÂÂ .device_groupÂÂÂ = gart_iommu_device_group,
>> ÂÂÂÂÂ .mapÂÂÂÂÂÂÂ = gart_iommu_map,
>> ÂÂÂÂÂ .map_sgÂÂÂÂÂÂÂ = default_iommu_map_sg,
>> ÂÂÂÂÂ .unmapÂÂÂÂÂÂÂ = gart_iommu_unmap,
>> ÂÂÂÂÂ .iova_to_physÂÂÂ = gart_iommu_iova_to_phys,
>> ÂÂÂÂÂ .pgsize_bitmapÂÂÂ = GART_IOMMU_PGSIZES,
>> +ÂÂÂ .of_xlateÂÂÂ = gart_iommu_of_xlate,
>> Â };
>> Â +static int gart_iommu_check_device(struct gart_device *gart,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device *dev)
>> +{
>> +ÂÂÂ if (!dev->iommu_fwspec || dev->iommu_fwspec->ops != &gart_iommu_ops)
>> +ÂÂÂÂÂÂÂ return -ENODEV;
> 
> Conceptually, it would be better to verify this in .add_device *before* calling
> iommu_group_get_for_dev(). That would also align with what other drivers do, and
> let you save introducing the .device_group callback until the real
> implementation in the later patch.

Indeed, thank you for the suggestion.