RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci-owner@xxxxxxxxxxxxxxx]
> On Behalf Of Alex Williamson
> Sent: Wednesday, September 25, 2013 10:16 PM
> To: Bhushan Bharat-R65777
> Cc: joro@xxxxxxxxxx; benh@xxxxxxxxxxxxxxxxxxx; galak@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx; linux-
> pci@xxxxxxxxxxxxxxx; agraf@xxxxxxx; Wood Scott-B07421; iommu@xxxxxxxxxxxx
> foundation.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
> 
> On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
> > This api return the iommu domain to which the device is attached.
> > The iommu_domain is required for making API calls related to iommu.
> > Follow up patches which use this API to know iommu maping.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx>
> > ---
> >  drivers/iommu/iommu.c |   10 ++++++++++
> >  include/linux/iommu.h |    7 +++++++
> >  2 files changed, 17 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index
> > fbe9ca7..6ac5f50 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -696,6 +696,16 @@ void iommu_detach_device(struct iommu_domain
> > *domain, struct device *dev)  }
> > EXPORT_SYMBOL_GPL(iommu_detach_device);
> >
> > +struct iommu_domain *iommu_get_dev_domain(struct device *dev) {
> > +	struct iommu_ops *ops = dev->bus->iommu_ops;
> > +
> > +	if (unlikely(ops == NULL || ops->get_dev_iommu_domain == NULL))
> > +		return NULL;
> > +
> > +	return ops->get_dev_iommu_domain(dev); }
> > +EXPORT_SYMBOL_GPL(iommu_get_dev_domain);
> 
> What prevents this from racing iommu_domain_free()?  There's no references
> acquired, so there's no reason for the caller to assume the pointer is valid.

Sorry for late query, somehow this email went into a folder and escaped;

Just to be sure, there is not lock at generic "struct iommu_domain", but IP specific structure (link FSL domain) linked in iommu_domain->priv have a lock, so we need to ensure this race in FSL iommu code (say drivers/iommu/fsl_pamu_domain.c), right?

Thanks
-Bharat

> 
> >  /*
> >   * IOMMU groups are really the natrual working unit of the IOMMU, but
> >   * the IOMMU API works on domains and devices.  Bridge that gap by
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
> > 7ea319e..fa046bd 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -127,6 +127,7 @@ struct iommu_ops {
> >  	int (*domain_set_windows)(struct iommu_domain *domain, u32 w_count);
> >  	/* Get the numer of window per domain */
> >  	u32 (*domain_get_windows)(struct iommu_domain *domain);
> > +	struct iommu_domain *(*get_dev_iommu_domain)(struct device *dev);
> >
> >  	unsigned long pgsize_bitmap;
> >  };
> > @@ -190,6 +191,7 @@ extern int iommu_domain_window_enable(struct iommu_domain
> *domain, u32 wnd_nr,
> >  				      phys_addr_t offset, u64 size,
> >  				      int prot);
> >  extern void iommu_domain_window_disable(struct iommu_domain *domain,
> > u32 wnd_nr);
> > +extern struct iommu_domain *iommu_get_dev_domain(struct device *dev);
> >  /**
> >   * report_iommu_fault() - report about an IOMMU fault to the IOMMU framework
> >   * @domain: the iommu domain where the fault has happened @@ -284,6
> > +286,11 @@ static inline void iommu_domain_window_disable(struct
> > iommu_domain *domain,  {  }
> >
> > +static inline struct iommu_domain *iommu_get_dev_domain(struct device
> > +*dev) {
> > +	return NULL;
> > +}
> > +
> >  static inline phys_addr_t iommu_iova_to_phys(struct iommu_domain
> > *domain, dma_addr_t iova)  {
> >  	return 0;
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body
> of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at
> http://vger.kernel.org/majordomo-info.html

¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_