Messages in this thread | | | From | Shameerali Kolothum Thodi <> | Subject | RE: [PATCH] iommu: Check dev->iommu in iommu_dev_xxx functions | Date | Tue, 26 Jan 2021 16:40:45 +0000 |
| |
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
| |