Hi Robin,
-----Original Message----- From: Robin Murphy [mailto:robin.murphy@arm.com] Sent: 26 January 2021 13:51 To: Shameerali Kolothum Thodi shameerali.kolothum.thodi@huawei.com Cc: linux-kernel@vger.kernel.org; iommu@lists.linux-foundation.org; jean-philippe@linaro.org; will@kernel.org; linuxarm@openeuler.org; Zengtao (B) prime.zeng@hisilicon.com Subject: Re: [PATCH] iommu: Check dev->iommu in iommu_dev_xxx functions
On Tue, 26 Jan 2021 13:06:29 +0000 Shameer Kolothum shameerali.kolothum.thodi@huawei.com wrote:
The device iommu probe/attach might have failed leaving dev->iommu to NULL and device drivers may still invoke these functions resulting a crash in iommu vendor driver code. Hence make sure we check that.
Signed-off-by: Shameer Kolothum shameerali.kolothum.thodi@huawei.com
drivers/iommu/iommu.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index ffeebda8d6de..cb68153c5cc0 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2867,7 +2867,7 @@ bool iommu_dev_has_feature(struct device *dev, enum iommu_dev_features feat) { const struct iommu_ops *ops = dev->bus->iommu_ops;
- if (ops && ops->dev_has_feat)
- if (dev->iommu && ops && ops->dev_has_feat) return ops->dev_has_feat(dev, feat);
Might make sense to make these more self-contained, e.g.:
if (dev->iommu && dev->iommu->ops->foo) dev->iommu->ops->foo()
Right. Does that mean adding ops to "struct dev_iommu" or retrieve ops like below,
if (dev->iommu && dev->iommu->iommu_dev->ops->foo) dev->iommu->iommu_dev->ops->foo()
Sorry, not clear to me.
Thanks, Shameer