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

Message ID 20180508181700.5169-7-digetx@gmail.com
State New
Headers show
Series
  • Tegra GART driver clean up and optimization
Related show

Commit Message

Dmitry Osipenko May 8, 2018, 6:16 p.m.
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@gmail.com>
---
 drivers/iommu/tegra-gart.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Comments

Robin Murphy May 11, 2018, 11:34 a.m. | #1
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@gmail.com>
> ---
>   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.

Robin.

> +
> +	return 0;
> +}
> +
>   static int tegra_gart_suspend(struct device *dev)
>   {
>   	struct gart_device *gart = dev_get_drvdata(dev);
> @@ -462,6 +492,7 @@ static int tegra_gart_probe(struct platform_device *pdev)
>   	}
>   
>   	iommu_device_set_ops(&gart->iommu, &gart_iommu_ops);
> +	iommu_device_set_fwnode(&gart->iommu, dev->fwnode);
>   
>   	ret = iommu_device_register(&gart->iommu);
>   	if (ret) {
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Osipenko May 11, 2018, 3:34 p.m. | #2
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@gmail.com>
>> ---
>>   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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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;
+
+	return 0;
+}
+
 static int tegra_gart_suspend(struct device *dev)
 {
 	struct gart_device *gart = dev_get_drvdata(dev);
@@ -462,6 +492,7 @@  static int tegra_gart_probe(struct platform_device *pdev)
 	}
 
 	iommu_device_set_ops(&gart->iommu, &gart_iommu_ops);
+	iommu_device_set_fwnode(&gart->iommu, dev->fwnode);
 
 	ret = iommu_device_register(&gart->iommu);
 	if (ret) {