Message ID | 1379575763-2091-3-git-send-email-Bharat.Bhushan@freescale.com |
---|---|
State | Not Applicable |
Headers | show |
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@freescale.com> > --- > 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. > /* > * 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] > On Behalf Of Alex Williamson > Sent: Wednesday, September 25, 2013 10:16 PM > To: Bhushan Bharat-R65777 > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > 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@freescale.com> > > --- > > 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@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Bhushan Bharat-R65777 > Sent: Friday, October 04, 2013 3:24 PM > To: 'Alex Williamson' > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > foundation.org > Subject: RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > > > > -----Original Message----- > > From: linux-pci-owner@vger.kernel.org > > [mailto:linux-pci-owner@vger.kernel.org] > > On Behalf Of Alex Williamson > > Sent: Wednesday, September 25, 2013 10:16 PM > > To: Bhushan Bharat-R65777 > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- 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@freescale.com> > > > --- > > > 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? Further thinking of this, there are more problems here: - Like MSI subsystem will call iommu_get_dev_domain(), which will take a lock, find the domain pointer, release the lock, and return the domain - Now if domain in freed up - While MSI subsystem tries to do work on domain (like get_attribute/set_attribute etc) ??? So can we do like iommu_get_dev_domain() will return domain with the lock held, and iommu_put_dev_domain() will release the lock? And iommu_get_dev_domain() must always be followed by iommu_get_dev_domain() Thanks -Bharat > > 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@vger.kernel.org More majordomo > > info at http://vger.kernel.org/majordomo-info.html
On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote: > > > -----Original Message----- > > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org] > > On Behalf Of Alex Williamson > > Sent: Wednesday, September 25, 2013 10:16 PM > > To: Bhushan Bharat-R65777 > > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > 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@freescale.com> > > > --- > > > 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? No, it's not sufficient to make sure that your use of the interface is race free. The interface itself needs to be designed so that it's difficult to use incorrectly. That's not the case here. This is a backdoor to get the iommu domain from the iommu driver regardless of who is using it or how. The iommu domain is created and managed by vfio, so shouldn't we be looking at how to do this through vfio? It seems like you'd want to use your device to get a vfio group reference, from which you could do something with the vfio external user interface and get the iommu domain reference. Thanks, Alex > > > /* > > > * 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@vger.kernel.org More majordomo info at > > http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Friday, October 04, 2013 9:15 PM > To: Bhushan Bharat-R65777 > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > foundation.org > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote: > > > > > -----Original Message----- > > > From: linux-pci-owner@vger.kernel.org > > > [mailto:linux-pci-owner@vger.kernel.org] > > > On Behalf Of Alex Williamson > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > To: Bhushan Bharat-R65777 > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- 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@freescale.com> > > > > --- > > > > 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? > > No, it's not sufficient to make sure that your use of the interface is race > free. The interface itself needs to be designed so that it's difficult to use > incorrectly. So we can define iommu_get_dev_domain()/iommu_put_dev_domain(); iommu_get_dev_domain() will return domain with the lock held, and iommu_put_dev_domain() will release the lock? And iommu_get_dev_domain() must always be followed by iommu_get_dev_domain(). > That's not the case here. This is a backdoor to get the iommu > domain from the iommu driver regardless of who is using it or how. The iommu > domain is created and managed by vfio, so shouldn't we be looking at how to do > this through vfio? Let me first describe what we are doing here: During initialization:- - vfio talks to MSI system to know the MSI-page and size - vfio then interacts with iommu to map the MSI-page in iommu (IOVA is decided by userspace and physical address is the MSI-page) - So the IOVA subwindow mapping is created in iommu and yes VFIO know about this mapping. Now do SET_IRQ(MSI/MSIX) ioctl: - calls pci_enable_msix()/pci_enable_msi_block(): which is supposed to set MSI address/data in device. - So in current implementation (this patchset) msi-subsystem gets the IOVA from iommu via this defined interface. - Are you saying that rather than getting this from iommu, we should get this from vfio? What difference does this make? Thanks -Bharat > It seems like you'd want to use your device to get a vfio > group reference, from which you could do something with the vfio external user > interface and get the iommu domain reference. Thanks, > > Alex > > > > > /* > > > > * 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@vger.kernel.org More majordomo > > > info at http://vger.kernel.org/majordomo-info.html > > > > >
On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote: > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Friday, October 04, 2013 9:15 PM > > To: Bhushan Bharat-R65777 > > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > foundation.org > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > -----Original Message----- > > > > From: linux-pci-owner@vger.kernel.org > > > > [mailto:linux-pci-owner@vger.kernel.org] > > > > On Behalf Of Alex Williamson > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > To: Bhushan Bharat-R65777 > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- 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@freescale.com> > > > > > --- > > > > > 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? > > > > No, it's not sufficient to make sure that your use of the interface is race > > free. The interface itself needs to be designed so that it's difficult to use > > incorrectly. > > So we can define iommu_get_dev_domain()/iommu_put_dev_domain(); > iommu_get_dev_domain() will return domain with the lock held, and > iommu_put_dev_domain() will release the lock? And > iommu_get_dev_domain() must always be followed by > iommu_get_dev_domain(). What lock? get/put are generally used for reference counting, not locking in the kernel. > > That's not the case here. This is a backdoor to get the iommu > > domain from the iommu driver regardless of who is using it or how. The iommu > > domain is created and managed by vfio, so shouldn't we be looking at how to do > > this through vfio? > > Let me first describe what we are doing here: > During initialization:- > - vfio talks to MSI system to know the MSI-page and size > - vfio then interacts with iommu to map the MSI-page in iommu (IOVA is decided by userspace and physical address is the MSI-page) > - So the IOVA subwindow mapping is created in iommu and yes VFIO know about this mapping. > > Now do SET_IRQ(MSI/MSIX) ioctl: > - calls pci_enable_msix()/pci_enable_msi_block(): which is supposed to set MSI address/data in device. > - So in current implementation (this patchset) msi-subsystem gets the IOVA from iommu via this defined interface. > - Are you saying that rather than getting this from iommu, we should get this from vfio? What difference does this make? Yes, you just said above that vfio knows the msi to iova mapping, so why go outside of vfio to find it later? The difference is one case you can have a proper reference to data structures to make sure the pointer you get back actually has meaning at the time you're using it vs the code here where you're defining an API that returns a meaningless value because you can't check or enforce that an arbitrary caller is using it correctly. It's not maintainable. Thanks, Alex > > It seems like you'd want to use your device to get a vfio > > group reference, from which you could do something with the vfio external user > > interface and get the iommu domain reference. Thanks, > > > > Alex > > > > > > > /* > > > > > * 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@vger.kernel.org More majordomo > > > > info at http://vger.kernel.org/majordomo-info.html > > > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Friday, October 04, 2013 10:43 PM > To: Bhushan Bharat-R65777 > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > foundation.org > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote: > > > > > -----Original Message----- > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > Sent: Friday, October 04, 2013 9:15 PM > > > To: Bhushan Bharat-R65777 > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > device > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > -----Original Message----- > > > > > From: linux-pci-owner@vger.kernel.org > > > > > [mailto:linux-pci-owner@vger.kernel.org] > > > > > On Behalf Of Alex Williamson > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > To: Bhushan Bharat-R65777 > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > 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@freescale.com> > > > > > > --- > > > > > > 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? > > > > > > No, it's not sufficient to make sure that your use of the interface > > > is race free. The interface itself needs to be designed so that > > > it's difficult to use incorrectly. > > > > So we can define iommu_get_dev_domain()/iommu_put_dev_domain(); > > iommu_get_dev_domain() will return domain with the lock held, and > > iommu_put_dev_domain() will release the lock? And > > iommu_get_dev_domain() must always be followed by > > iommu_get_dev_domain(). > > What lock? get/put are generally used for reference counting, not locking in > the kernel. > > > > That's not the case here. This is a backdoor to get the iommu > > > domain from the iommu driver regardless of who is using it or how. > > > The iommu domain is created and managed by vfio, so shouldn't we be > > > looking at how to do this through vfio? > > > > Let me first describe what we are doing here: > > During initialization:- > > - vfio talks to MSI system to know the MSI-page and size > > - vfio then interacts with iommu to map the MSI-page in iommu (IOVA > > is decided by userspace and physical address is the MSI-page) > > - So the IOVA subwindow mapping is created in iommu and yes VFIO know about > this mapping. > > > > Now do SET_IRQ(MSI/MSIX) ioctl: > > - calls pci_enable_msix()/pci_enable_msi_block(): which is supposed to set > MSI address/data in device. > > - So in current implementation (this patchset) msi-subsystem gets the IOVA > from iommu via this defined interface. > > - Are you saying that rather than getting this from iommu, we should get this > from vfio? What difference does this make? > > Yes, you just said above that vfio knows the msi to iova mapping, so why go > outside of vfio to find it later? The difference is one case you can have a > proper reference to data structures to make sure the pointer you get back > actually has meaning at the time you're using it vs the code here where you're > defining an API that returns a meaningless value With FSL-PAMU we will always get consistant data from iommu or vfio-data structure. > because you can't check or > enforce that an arbitrary caller is using it correctly. I am not sure what is arbitrary caller? pdev is known to vfio, so vfio will only make pci_enable_msix()/pci_enable_msi_block() for this pdev. If anyother code makes then it is some other unexpectedly thing happening in system, no? > It's not maintainable. > Thanks, I do not have any issue with this as well, can you also describe the type of API you are envisioning; I can think of defining some function in vfio.c/vfio_iommu*.c, make them global and declare then in include/Linux/vfio.h And include <Linux/vfio.h> in caller file (arch/powerpc/kernel/msi.c) Thanks -Bharat > > Alex > > > > It seems like you'd want to use your device to get a vfio group > > > reference, from which you could do something with the vfio external > > > user interface and get the iommu domain reference. Thanks, > > > > > > Alex > > > > > > > > > /* > > > > > > * 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@vger.kernel.org More > > > > > majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > > > > > > > > > > > > > >
On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote: > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Friday, October 04, 2013 10:43 PM > > To: Bhushan Bharat-R65777 > > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > foundation.org > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > -----Original Message----- > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > Sent: Friday, October 04, 2013 9:15 PM > > > > To: Bhushan Bharat-R65777 > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > device > > > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > -----Original Message----- > > > > > > From: linux-pci-owner@vger.kernel.org > > > > > > [mailto:linux-pci-owner@vger.kernel.org] > > > > > > On Behalf Of Alex Williamson > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > > To: Bhushan Bharat-R65777 > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > 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@freescale.com> > > > > > > > --- > > > > > > > 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? > > > > > > > > No, it's not sufficient to make sure that your use of the interface > > > > is race free. The interface itself needs to be designed so that > > > > it's difficult to use incorrectly. > > > > > > So we can define iommu_get_dev_domain()/iommu_put_dev_domain(); > > > iommu_get_dev_domain() will return domain with the lock held, and > > > iommu_put_dev_domain() will release the lock? And > > > iommu_get_dev_domain() must always be followed by > > > iommu_get_dev_domain(). > > > > What lock? get/put are generally used for reference counting, not locking in > > the kernel. > > > > > > That's not the case here. This is a backdoor to get the iommu > > > > domain from the iommu driver regardless of who is using it or how. > > > > The iommu domain is created and managed by vfio, so shouldn't we be > > > > looking at how to do this through vfio? > > > > > > Let me first describe what we are doing here: > > > During initialization:- > > > - vfio talks to MSI system to know the MSI-page and size > > > - vfio then interacts with iommu to map the MSI-page in iommu (IOVA > > > is decided by userspace and physical address is the MSI-page) > > > - So the IOVA subwindow mapping is created in iommu and yes VFIO know about > > this mapping. > > > > > > Now do SET_IRQ(MSI/MSIX) ioctl: > > > - calls pci_enable_msix()/pci_enable_msi_block(): which is supposed to set > > MSI address/data in device. > > > - So in current implementation (this patchset) msi-subsystem gets the IOVA > > from iommu via this defined interface. > > > - Are you saying that rather than getting this from iommu, we should get this > > from vfio? What difference does this make? > > > > Yes, you just said above that vfio knows the msi to iova mapping, so why go > > outside of vfio to find it later? The difference is one case you can have a > > proper reference to data structures to make sure the pointer you get back > > actually has meaning at the time you're using it vs the code here where you're > > defining an API that returns a meaningless value > > With FSL-PAMU we will always get consistant data from iommu or vfio-data structure. Great, but you're trying to add a generic API to the IOMMU subsystem that's difficult to use correctly. The fact that you use it correctly does not justify the API. > > because you can't check or > > enforce that an arbitrary caller is using it correctly. > > I am not sure what is arbitrary caller? pdev is known to vfio, so vfio > will only make pci_enable_msix()/pci_enable_msi_block() for this pdev. > If anyother code makes then it is some other unexpectedly thing > happening in system, no? What's proposed here is a generic IOMMU API. Anybody can call this. What if the host SCSI driver decides to go get the iommu domain for it's device (or any other device)? Does that fit your usage model? > > It's not maintainable. > > Thanks, > > I do not have any issue with this as well, can you also describe the type of API you are envisioning; > I can think of defining some function in vfio.c/vfio_iommu*.c, make them global and declare then in include/Linux/vfio.h > And include <Linux/vfio.h> in caller file (arch/powerpc/kernel/msi.c) Do you really want module dependencies between vfio and your core kernel MSI setup? Look at the vfio external user interface that we've already defined. That allows other components of the kernel to get a proper reference to a vfio group. From there you can work out how to get what you want. Another alternative is that vfio could register an MSI to IOVA mapping with architecture code when the mapping is created. The MSI setup path could then do a lookup in architecture code for the mapping. You could even store the MSI to IOVA mapping in VFIO and create an interface where SET_IRQ passes that mapping into setup code. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQWxleCBXaWxsaWFtc29u IFttYWlsdG86YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb21dDQo+IFNlbnQ6IEZyaWRheSwgT2N0 b2JlciAwNCwgMjAxMyAxMTo0MiBQTQ0KPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+IENj OiBqb3JvQDhieXRlcy5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsgZ2FsYWtAa2VybmVs LmNyYXNoaW5nLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBj LWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0NCj4gcGNpQHZnZXIua2VybmVsLm9yZzsgYWdy YWZAc3VzZS5kZTsgV29vZCBTY290dC1CMDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LQ0KPiBmb3Vu ZGF0aW9uLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDIvN10gaW9tbXU6IGFkZCBhcGkgdG8g Z2V0IGlvbW11X2RvbWFpbiBvZiBhIGRldmljZQ0KPiANCj4gT24gRnJpLCAyMDEzLTEwLTA0IGF0 IDE3OjIzICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3Nzcgd3JvdGU6DQo+ID4NCj4gPiA+IC0t LS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBBbGV4IFdpbGxpYW1zb24gW21h aWx0bzphbGV4LndpbGxpYW1zb25AcmVkaGF0LmNvbV0NCj4gPiA+IFNlbnQ6IEZyaWRheSwgT2N0 b2JlciAwNCwgMjAxMyAxMDo0MyBQTQ0KPiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0K PiA+ID4gQ2M6IGpvcm9AOGJ5dGVzLm9yZzsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOw0KPiA+ ID4gZ2FsYWtAa2VybmVsLmNyYXNoaW5nLm9yZzsgbGludXgtIGtlcm5lbEB2Z2VyLmtlcm5lbC5v cmc7DQo+ID4gPiBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgbGludXgtIHBjaUB2Z2Vy Lmtlcm5lbC5vcmc7DQo+ID4gPiBhZ3JhZkBzdXNlLmRlOyBXb29kIFNjb3R0LUIwNzQyMTsgaW9t bXVAbGlzdHMubGludXgtIGZvdW5kYXRpb24ub3JnDQo+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENI IDIvN10gaW9tbXU6IGFkZCBhcGkgdG8gZ2V0IGlvbW11X2RvbWFpbiBvZiBhDQo+ID4gPiBkZXZp Y2UNCj4gPiA+DQo+ID4gPiBPbiBGcmksIDIwMTMtMTAtMDQgYXQgMTY6NDcgKzAwMDAsIEJodXNo YW4gQmhhcmF0LVI2NTc3NyB3cm90ZToNCj4gPiA+ID4NCj4gPiA+ID4gPiAtLS0tLU9yaWdpbmFs IE1lc3NhZ2UtLS0tLQ0KPiA+ID4gPiA+IEZyb206IEFsZXggV2lsbGlhbXNvbiBbbWFpbHRvOmFs ZXgud2lsbGlhbXNvbkByZWRoYXQuY29tXQ0KPiA+ID4gPiA+IFNlbnQ6IEZyaWRheSwgT2N0b2Jl ciAwNCwgMjAxMyA5OjE1IFBNDQo+ID4gPiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0K PiA+ID4gPiA+IENjOiBqb3JvQDhieXRlcy5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsN Cj4gPiA+ID4gPiBnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnOyBsaW51eC0ga2VybmVsQHZnZXIu a2VybmVsLm9yZzsNCj4gPiA+ID4gPiBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgbGlu dXgtIHBjaUB2Z2VyLmtlcm5lbC5vcmc7DQo+ID4gPiA+ID4gYWdyYWZAc3VzZS5kZTsgV29vZCBT Y290dC1CMDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LQ0KPiA+ID4gPiA+IGZvdW5kYXRpb24ub3Jn DQo+ID4gPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCAyLzddIGlvbW11OiBhZGQgYXBpIHRvIGdl dCBpb21tdV9kb21haW4gb2YgYQ0KPiA+ID4gPiA+IGRldmljZQ0KPiA+ID4gPiA+DQo+ID4gPiA+ ID4gT24gRnJpLCAyMDEzLTEwLTA0IGF0IDA5OjU0ICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3 Nzcgd3JvdGU6DQo+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiAtLS0tLU9yaWdpbmFsIE1lc3Nh Z2UtLS0tLQ0KPiA+ID4gPiA+ID4gPiBGcm9tOiBsaW51eC1wY2ktb3duZXJAdmdlci5rZXJuZWwu b3JnDQo+ID4gPiA+ID4gPiA+IFttYWlsdG86bGludXgtcGNpLW93bmVyQHZnZXIua2VybmVsLm9y Z10NCj4gPiA+ID4gPiA+ID4gT24gQmVoYWxmIE9mIEFsZXggV2lsbGlhbXNvbg0KPiA+ID4gPiA+ ID4gPiBTZW50OiBXZWRuZXNkYXksIFNlcHRlbWJlciAyNSwgMjAxMyAxMDoxNiBQTQ0KPiA+ID4g PiA+ID4gPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4gPiA+ID4gPiA+IENjOiBqb3Jv QDhieXRlcy5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsNCj4gPiA+ID4gPiA+ID4gZ2Fs YWtAa2VybmVsLmNyYXNoaW5nLm9yZzsgbGludXgtIGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+ ID4gPiA+ID4gPiA+IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0gcGNpQHZn ZXIua2VybmVsLm9yZzsNCj4gPiA+ID4gPiA+ID4gYWdyYWZAc3VzZS5kZTsgV29vZCBTY290dC1C MDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LQ0KPiA+ID4gPiA+ID4gPiBmb3VuZGF0aW9uLm9yZzsg Qmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4gPiA+ID4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg Mi83XSBpb21tdTogYWRkIGFwaSB0byBnZXQgaW9tbXVfZG9tYWluDQo+ID4gPiA+ID4gPiA+IG9m IGEgZGV2aWNlDQo+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+IE9uIFRodSwgMjAxMy0wOS0x OSBhdCAxMjo1OSArMDUzMCwgQmhhcmF0IEJodXNoYW4gd3JvdGU6DQo+ID4gPiA+ID4gPiA+ID4g VGhpcyBhcGkgcmV0dXJuIHRoZSBpb21tdSBkb21haW4gdG8gd2hpY2ggdGhlIGRldmljZSBpcyBh dHRhY2hlZC4NCj4gPiA+ID4gPiA+ID4gPiBUaGUgaW9tbXVfZG9tYWluIGlzIHJlcXVpcmVkIGZv ciBtYWtpbmcgQVBJIGNhbGxzIHJlbGF0ZWQgdG8NCj4gaW9tbXUuDQo+ID4gPiA+ID4gPiA+ID4g Rm9sbG93IHVwIHBhdGNoZXMgd2hpY2ggdXNlIHRoaXMgQVBJIHRvIGtub3cgaW9tbXUgbWFwaW5n Lg0KPiA+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4gU2lnbmVkLW9mZi1ieTogQmhhcmF0 IEJodXNoYW4NCj4gPiA+ID4gPiA+ID4gPiA8YmhhcmF0LmJodXNoYW5AZnJlZXNjYWxlLmNvbT4N Cj4gPiA+ID4gPiA+ID4gPiAtLS0NCj4gPiA+ID4gPiA+ID4gPiAgZHJpdmVycy9pb21tdS9pb21t dS5jIHwgICAxMCArKysrKysrKysrDQo+ID4gPiA+ID4gPiA+ID4gIGluY2x1ZGUvbGludXgvaW9t bXUuaCB8ICAgIDcgKysrKysrKw0KPiA+ID4gPiA+ID4gPiA+ICAyIGZpbGVzIGNoYW5nZWQsIDE3 IGluc2VydGlvbnMoKyksIDAgZGVsZXRpb25zKC0pDQo+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4g PiA+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9pb21tdS9pb21tdS5jIGIvZHJpdmVycy9pb21t dS9pb21tdS5jDQo+ID4gPiA+ID4gPiA+ID4gaW5kZXgNCj4gPiA+ID4gPiA+ID4gPiBmYmU5Y2E3 Li42YWM1ZjUwIDEwMDY0NA0KPiA+ID4gPiA+ID4gPiA+IC0tLSBhL2RyaXZlcnMvaW9tbXUvaW9t bXUuYw0KPiA+ID4gPiA+ID4gPiA+ICsrKyBiL2RyaXZlcnMvaW9tbXUvaW9tbXUuYw0KPiA+ID4g PiA+ID4gPiA+IEBAIC02OTYsNiArNjk2LDE2IEBAIHZvaWQgaW9tbXVfZGV0YWNoX2RldmljZShz dHJ1Y3QNCj4gPiA+ID4gPiA+ID4gPiBpb21tdV9kb21haW4gKmRvbWFpbiwgc3RydWN0IGRldmlj ZSAqZGV2KSAgfQ0KPiA+ID4gPiA+ID4gPiA+IEVYUE9SVF9TWU1CT0xfR1BMKGlvbW11X2RldGFj aF9kZXZpY2UpOw0KPiA+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4gK3N0cnVjdCBpb21t dV9kb21haW4gKmlvbW11X2dldF9kZXZfZG9tYWluKHN0cnVjdCBkZXZpY2UgKmRldikgew0KPiA+ ID4gPiA+ID4gPiA+ICsJc3RydWN0IGlvbW11X29wcyAqb3BzID0gZGV2LT5idXMtPmlvbW11X29w czsNCj4gPiA+ID4gPiA+ID4gPiArDQo+ID4gPiA+ID4gPiA+ID4gKwlpZiAodW5saWtlbHkob3Bz ID09IE5VTEwgfHwgb3BzLT5nZXRfZGV2X2lvbW11X2RvbWFpbiA9PQ0KPiBOVUxMKSkNCj4gPiA+ ID4gPiA+ID4gPiArCQlyZXR1cm4gTlVMTDsNCj4gPiA+ID4gPiA+ID4gPiArDQo+ID4gPiA+ID4g PiA+ID4gKwlyZXR1cm4gb3BzLT5nZXRfZGV2X2lvbW11X2RvbWFpbihkZXYpOyB9DQo+ID4gPiA+ ID4gPiA+ID4gK0VYUE9SVF9TWU1CT0xfR1BMKGlvbW11X2dldF9kZXZfZG9tYWluKTsNCj4gPiA+ ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gV2hhdCBwcmV2ZW50cyB0aGlzIGZyb20gcmFjaW5nIGlv bW11X2RvbWFpbl9mcmVlKCk/ICBUaGVyZSdzDQo+ID4gPiA+ID4gPiA+IG5vIHJlZmVyZW5jZXMg YWNxdWlyZWQsIHNvIHRoZXJlJ3Mgbm8gcmVhc29uIGZvciB0aGUgY2FsbGVyDQo+ID4gPiA+ID4g PiA+IHRvIGFzc3VtZSB0aGUNCj4gPiA+ID4gPiBwb2ludGVyIGlzIHZhbGlkLg0KPiA+ID4gPiA+ ID4NCj4gPiA+ID4gPiA+IFNvcnJ5IGZvciBsYXRlIHF1ZXJ5LCBzb21laG93IHRoaXMgZW1haWwg d2VudCBpbnRvIGEgZm9sZGVyDQo+ID4gPiA+ID4gPiBhbmQgZXNjYXBlZDsNCj4gPiA+ID4gPiA+ DQo+ID4gPiA+ID4gPiBKdXN0IHRvIGJlIHN1cmUsIHRoZXJlIGlzIG5vdCBsb2NrIGF0IGdlbmVy aWMgInN0cnVjdA0KPiA+ID4gPiA+ID4gaW9tbXVfZG9tYWluIiwgYnV0IElQDQo+ID4gPiA+ID4g c3BlY2lmaWMgc3RydWN0dXJlIChsaW5rIEZTTCBkb21haW4pIGxpbmtlZCBpbg0KPiA+ID4gPiA+ IGlvbW11X2RvbWFpbi0+cHJpdiBoYXZlIGEgbG9jaywgc28gd2UgbmVlZCB0byBlbnN1cmUgdGhp cyByYWNlDQo+ID4gPiA+ID4gaW4gRlNMIGlvbW11IGNvZGUgKHNheSBkcml2ZXJzL2lvbW11L2Zz bF9wYW11X2RvbWFpbi5jKSwgcmlnaHQ/DQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBObywgaXQncyBu b3Qgc3VmZmljaWVudCB0byBtYWtlIHN1cmUgdGhhdCB5b3VyIHVzZSBvZiB0aGUNCj4gPiA+ID4g PiBpbnRlcmZhY2UgaXMgcmFjZSBmcmVlLiAgVGhlIGludGVyZmFjZSBpdHNlbGYgbmVlZHMgdG8g YmUNCj4gPiA+ID4gPiBkZXNpZ25lZCBzbyB0aGF0IGl0J3MgZGlmZmljdWx0IHRvIHVzZSBpbmNv cnJlY3RseS4NCj4gPiA+ID4NCj4gPiA+ID4gU28gd2UgY2FuIGRlZmluZSBpb21tdV9nZXRfZGV2 X2RvbWFpbigpL2lvbW11X3B1dF9kZXZfZG9tYWluKCk7DQo+ID4gPiA+IGlvbW11X2dldF9kZXZf ZG9tYWluKCkgd2lsbCByZXR1cm4gZG9tYWluIHdpdGggdGhlIGxvY2sgaGVsZCwgYW5kDQo+ID4g PiA+IGlvbW11X3B1dF9kZXZfZG9tYWluKCkgd2lsbCByZWxlYXNlIHRoZSBsb2NrPyBBbmQNCj4g PiA+ID4gaW9tbXVfZ2V0X2Rldl9kb21haW4oKSBtdXN0IGFsd2F5cyBiZSBmb2xsb3dlZCBieQ0K PiA+ID4gPiBpb21tdV9nZXRfZGV2X2RvbWFpbigpLg0KPiA+ID4NCj4gPiA+IFdoYXQgbG9jaz8g IGdldC9wdXQgYXJlIGdlbmVyYWxseSB1c2VkIGZvciByZWZlcmVuY2UgY291bnRpbmcsIG5vdA0K PiA+ID4gbG9ja2luZyBpbiB0aGUga2VybmVsLg0KPiA+ID4NCj4gPiA+ID4gPiBUaGF0J3Mgbm90 IHRoZSBjYXNlIGhlcmUuICBUaGlzIGlzIGEgYmFja2Rvb3IgdG8gZ2V0IHRoZSBpb21tdQ0KPiA+ ID4gPiA+IGRvbWFpbiBmcm9tIHRoZSBpb21tdSBkcml2ZXIgcmVnYXJkbGVzcyBvZiB3aG8gaXMg dXNpbmcgaXQgb3IgaG93Lg0KPiA+ID4gPiA+IFRoZSBpb21tdSBkb21haW4gaXMgY3JlYXRlZCBh bmQgbWFuYWdlZCBieSB2ZmlvLCBzbyBzaG91bGRuJ3Qgd2UNCj4gPiA+ID4gPiBiZSBsb29raW5n IGF0IGhvdyB0byBkbyB0aGlzIHRocm91Z2ggdmZpbz8NCj4gPiA+ID4NCj4gPiA+ID4gTGV0IG1l IGZpcnN0IGRlc2NyaWJlIHdoYXQgd2UgYXJlIGRvaW5nIGhlcmU6DQo+ID4gPiA+IER1cmluZyBp bml0aWFsaXphdGlvbjotDQo+ID4gPiA+ICAtIHZmaW8gdGFsa3MgdG8gTVNJIHN5c3RlbSB0byBr bm93IHRoZSBNU0ktcGFnZSBhbmQgc2l6ZQ0KPiA+ID4gPiAgLSB2ZmlvIHRoZW4gaW50ZXJhY3Rz IHdpdGggaW9tbXUgdG8gbWFwIHRoZSBNU0ktcGFnZSBpbiBpb21tdQ0KPiA+ID4gPiAoSU9WQSBp cyBkZWNpZGVkIGJ5IHVzZXJzcGFjZSBhbmQgcGh5c2ljYWwgYWRkcmVzcyBpcyB0aGUNCj4gPiA+ ID4gTVNJLXBhZ2UpDQo+ID4gPiA+ICAtIFNvIHRoZSBJT1ZBIHN1YndpbmRvdyBtYXBwaW5nIGlz IGNyZWF0ZWQgaW4gaW9tbXUgYW5kIHllcyBWRklPDQo+ID4gPiA+IGtub3cgYWJvdXQNCj4gPiA+ IHRoaXMgbWFwcGluZy4NCj4gPiA+ID4NCj4gPiA+ID4gTm93IGRvIFNFVF9JUlEoTVNJL01TSVgp IGlvY3RsOg0KPiA+ID4gPiAgLSBjYWxscyBwY2lfZW5hYmxlX21zaXgoKS9wY2lfZW5hYmxlX21z aV9ibG9jaygpOiB3aGljaCBpcw0KPiA+ID4gPiBzdXBwb3NlZCB0byBzZXQNCj4gPiA+IE1TSSBh ZGRyZXNzL2RhdGEgaW4gZGV2aWNlLg0KPiA+ID4gPiAgLSBTbyBpbiBjdXJyZW50IGltcGxlbWVu dGF0aW9uICh0aGlzIHBhdGNoc2V0KSBtc2ktc3Vic3lzdGVtIGdldHMNCj4gPiA+ID4gdGhlIElP VkENCj4gPiA+IGZyb20gaW9tbXUgdmlhIHRoaXMgZGVmaW5lZCBpbnRlcmZhY2UuDQo+ID4gPiA+ ICAtIEFyZSB5b3Ugc2F5aW5nIHRoYXQgcmF0aGVyIHRoYW4gZ2V0dGluZyB0aGlzIGZyb20gaW9t bXUsIHdlDQo+ID4gPiA+IHNob3VsZCBnZXQgdGhpcw0KPiA+ID4gZnJvbSB2ZmlvPyBXaGF0IGRp ZmZlcmVuY2UgZG9lcyB0aGlzIG1ha2U/DQo+ID4gPg0KPiA+ID4gWWVzLCB5b3UganVzdCBzYWlk IGFib3ZlIHRoYXQgdmZpbyBrbm93cyB0aGUgbXNpIHRvIGlvdmEgbWFwcGluZywgc28NCj4gPiA+ IHdoeSBnbyBvdXRzaWRlIG9mIHZmaW8gdG8gZmluZCBpdCBsYXRlcj8gIFRoZSBkaWZmZXJlbmNl IGlzIG9uZSBjYXNlDQo+ID4gPiB5b3UgY2FuIGhhdmUgYSBwcm9wZXIgcmVmZXJlbmNlIHRvIGRh dGEgc3RydWN0dXJlcyB0byBtYWtlIHN1cmUgdGhlDQo+ID4gPiBwb2ludGVyIHlvdSBnZXQgYmFj ayBhY3R1YWxseSBoYXMgbWVhbmluZyBhdCB0aGUgdGltZSB5b3UncmUgdXNpbmcNCj4gPiA+IGl0 IHZzIHRoZSBjb2RlIGhlcmUgd2hlcmUgeW91J3JlIGRlZmluaW5nIGFuIEFQSSB0aGF0IHJldHVy bnMgYQ0KPiA+ID4gbWVhbmluZ2xlc3MgdmFsdWUNCj4gPg0KPiA+IFdpdGggRlNMLVBBTVUgd2Ug d2lsbCBhbHdheXMgZ2V0IGNvbnNpc3RhbnQgZGF0YSBmcm9tIGlvbW11IG9yIHZmaW8tZGF0YQ0K PiBzdHJ1Y3R1cmUuDQo+IA0KPiBHcmVhdCwgYnV0IHlvdSdyZSB0cnlpbmcgdG8gYWRkIGEgZ2Vu ZXJpYyBBUEkgdG8gdGhlIElPTU1VIHN1YnN5c3RlbSB0aGF0J3MNCj4gZGlmZmljdWx0IHRvIHVz ZSBjb3JyZWN0bHkuICBUaGUgZmFjdCB0aGF0IHlvdSB1c2UgaXQgY29ycmVjdGx5IGRvZXMgbm90 IGp1c3RpZnkNCj4gdGhlIEFQSS4NCj4gDQo+ID4gPiBiZWNhdXNlIHlvdSBjYW4ndCBjaGVjayBv cg0KPiA+ID4gZW5mb3JjZSB0aGF0IGFuIGFyYml0cmFyeSBjYWxsZXIgaXMgdXNpbmcgaXQgY29y cmVjdGx5Lg0KPiA+DQo+ID4gSSBhbSBub3Qgc3VyZSB3aGF0IGlzIGFyYml0cmFyeSBjYWxsZXI/ IHBkZXYgaXMga25vd24gdG8gdmZpbywgc28gdmZpbw0KPiA+IHdpbGwgb25seSBtYWtlIHBjaV9l bmFibGVfbXNpeCgpL3BjaV9lbmFibGVfbXNpX2Jsb2NrKCkgZm9yIHRoaXMgcGRldi4NCj4gPiBJ ZiBhbnlvdGhlciBjb2RlIG1ha2VzIHRoZW4gaXQgaXMgc29tZSBvdGhlciB1bmV4cGVjdGVkbHkg dGhpbmcNCj4gPiBoYXBwZW5pbmcgaW4gc3lzdGVtLCBubz8NCj4gDQo+IFdoYXQncyBwcm9wb3Nl ZCBoZXJlIGlzIGEgZ2VuZXJpYyBJT01NVSBBUEkuICBBbnlib2R5IGNhbiBjYWxsIHRoaXMuDQo+ IFdoYXQgaWYgdGhlIGhvc3QgU0NTSSBkcml2ZXIgZGVjaWRlcyB0byBnbyBnZXQgdGhlIGlvbW11 IGRvbWFpbiBmb3IgaXQncyBkZXZpY2UNCj4gKG9yIGFueSBvdGhlciBkZXZpY2UpPyAgRG9lcyB0 aGF0IGZpdCB5b3VyIHVzYWdlIG1vZGVsPw0KPiANCj4gPiA+ICBJdCdzIG5vdCBtYWludGFpbmFi bGUuDQo+ID4gPiBUaGFua3MsDQo+ID4NCj4gPiBJIGRvIG5vdCBoYXZlIGFueSBpc3N1ZSB3aXRo IHRoaXMgYXMgd2VsbCwgY2FuIHlvdSBhbHNvIGRlc2NyaWJlIHRoZQ0KPiA+IHR5cGUgb2YgQVBJ IHlvdSBhcmUgZW52aXNpb25pbmc7IEkgY2FuIHRoaW5rIG9mIGRlZmluaW5nIHNvbWUgZnVuY3Rp b24NCj4gPiBpbiB2ZmlvLmMvdmZpb19pb21tdSouYywgbWFrZSB0aGVtIGdsb2JhbCBhbmQgZGVj bGFyZSB0aGVuIGluDQo+ID4gaW5jbHVkZS9MaW51eC92ZmlvLmggQW5kIGluY2x1ZGUgPExpbnV4 L3ZmaW8uaD4gaW4gY2FsbGVyIGZpbGUNCj4gPiAoYXJjaC9wb3dlcnBjL2tlcm5lbC9tc2kuYykN Cj4gDQo+IERvIHlvdSByZWFsbHkgd2FudCBtb2R1bGUgZGVwZW5kZW5jaWVzIGJldHdlZW4gdmZp byBhbmQgeW91ciBjb3JlIGtlcm5lbCBNU0kNCj4gc2V0dXA/ICBMb29rIGF0IHRoZSB2ZmlvIGV4 dGVybmFsIHVzZXIgaW50ZXJmYWNlIHRoYXQgd2UndmUgYWxyZWFkeSBkZWZpbmVkLg0KPiBUaGF0 IGFsbG93cyBvdGhlciBjb21wb25lbnRzIG9mIHRoZSBrZXJuZWwgdG8gZ2V0IGEgcHJvcGVyIHJl ZmVyZW5jZSB0byBhIHZmaW8NCj4gZ3JvdXAuICBGcm9tIHRoZXJlIHlvdSBjYW4gd29yayBvdXQg aG93IHRvIGdldCB3aGF0IHlvdSB3YW50LiAgQW5vdGhlcg0KPiBhbHRlcm5hdGl2ZSBpcyB0aGF0 IHZmaW8gY291bGQgcmVnaXN0ZXIgYW4gTVNJIHRvIElPVkEgbWFwcGluZyB3aXRoIGFyY2hpdGVj dHVyZQ0KPiBjb2RlIHdoZW4gdGhlIG1hcHBpbmcgaXMgY3JlYXRlZC4gIFRoZSBNU0kgc2V0dXAg cGF0aCBjb3VsZCB0aGVuIGRvIGEgbG9va3VwIGluDQo+IGFyY2hpdGVjdHVyZSBjb2RlIGZvciB0 aGUgbWFwcGluZy4gIFlvdSBjb3VsZCBldmVuIHN0b3JlIHRoZSBNU0kgdG8gSU9WQSBtYXBwaW5n DQo+IGluIFZGSU8gYW5kIGNyZWF0ZSBhbiBpbnRlcmZhY2Ugd2hlcmUgU0VUX0lSUSBwYXNzZXMg dGhhdCBtYXBwaW5nIGludG8gc2V0dXANCj4gY29kZS4NCg0KT2ssIFdoYXQgSSB3YW50IGlzIHRv IGdldCBJT1ZBIGFzc29jaWF0ZWQgd2l0aCBhIHBoeXNpY2FsIGFkZHJlc3MgKHBoeXNpY2FsIGFk ZHJlc3Mgb2YgTVNJLWJhbmspLg0KQW5kIGN1cnJlbnRseSBJIGRvIG5vdCBzZWUgYSB3YXkgdG8g a25vdyBJT1ZBIG9mIGEgcGh5c2ljYWwgYWRkcmVzcyBhbmQgZG9pbmcgYWxsIHRoaXMgZG9tYWlu IGdldCBhbmQgdGhlbiBzZWFyY2ggdGhyb3VnaCBhbGwgb2YgaW9tbXUtd2luZG93cyBvZiB0aGF0 IGRvbWFpbi4NCg0KV2hhdCBpZiB3ZSBhZGQgYW4gaW9tbXUtQVBJIHdoaWNoIGNhbiByZXR1cm4g dGhlIElPVkEgbWFwcGluZyBvZiBhIHBoeXNpY2FsIGFkZHJlc3MuIEN1cnJlbnQgdXNlIGNhc2Ug aXMgc2V0dGluZyB1cCBNU0kncyBmb3IgYXBlcnR1cmUgdHlwZSBvZiBJT01NVSBhbHNvIGdldHRp bmcgYSBwaHlzX3RvX2lvdmEoKSBtYXBwaW5nIGlzIGluZGVwZW5kZW50IG9mIFZGSU8sIHlvdXIg dGhvdWdodD8NCg0KVGhhbmtzDQotQmhhcmF0DQoNCj4gVGhhbmtzLA0KPiANCj4gQWxleA0KPiAN Cg0K -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-10-07 at 05:46 +0000, Bhushan Bharat-R65777 wrote: > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Friday, October 04, 2013 11:42 PM > > To: Bhushan Bharat-R65777 > > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux- > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > foundation.org > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > > > On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > -----Original Message----- > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > Sent: Friday, October 04, 2013 10:43 PM > > > > To: Bhushan Bharat-R65777 > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > device > > > > > > > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > -----Original Message----- > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > > Sent: Friday, October 04, 2013 9:15 PM > > > > > > To: Bhushan Bharat-R65777 > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > foundation.org > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > > > device > > > > > > > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: linux-pci-owner@vger.kernel.org > > > > > > > > [mailto:linux-pci-owner@vger.kernel.org] > > > > > > > > On Behalf Of Alex Williamson > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > > > 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@freescale.com> > > > > > > > > > --- > > > > > > > > > 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? > > > > > > > > > > > > No, it's not sufficient to make sure that your use of the > > > > > > interface is race free. The interface itself needs to be > > > > > > designed so that it's difficult to use incorrectly. > > > > > > > > > > So we can define iommu_get_dev_domain()/iommu_put_dev_domain(); > > > > > iommu_get_dev_domain() will return domain with the lock held, and > > > > > iommu_put_dev_domain() will release the lock? And > > > > > iommu_get_dev_domain() must always be followed by > > > > > iommu_get_dev_domain(). > > > > > > > > What lock? get/put are generally used for reference counting, not > > > > locking in the kernel. > > > > > > > > > > That's not the case here. This is a backdoor to get the iommu > > > > > > domain from the iommu driver regardless of who is using it or how. > > > > > > The iommu domain is created and managed by vfio, so shouldn't we > > > > > > be looking at how to do this through vfio? > > > > > > > > > > Let me first describe what we are doing here: > > > > > During initialization:- > > > > > - vfio talks to MSI system to know the MSI-page and size > > > > > - vfio then interacts with iommu to map the MSI-page in iommu > > > > > (IOVA is decided by userspace and physical address is the > > > > > MSI-page) > > > > > - So the IOVA subwindow mapping is created in iommu and yes VFIO > > > > > know about > > > > this mapping. > > > > > > > > > > Now do SET_IRQ(MSI/MSIX) ioctl: > > > > > - calls pci_enable_msix()/pci_enable_msi_block(): which is > > > > > supposed to set > > > > MSI address/data in device. > > > > > - So in current implementation (this patchset) msi-subsystem gets > > > > > the IOVA > > > > from iommu via this defined interface. > > > > > - Are you saying that rather than getting this from iommu, we > > > > > should get this > > > > from vfio? What difference does this make? > > > > > > > > Yes, you just said above that vfio knows the msi to iova mapping, so > > > > why go outside of vfio to find it later? The difference is one case > > > > you can have a proper reference to data structures to make sure the > > > > pointer you get back actually has meaning at the time you're using > > > > it vs the code here where you're defining an API that returns a > > > > meaningless value > > > > > > With FSL-PAMU we will always get consistant data from iommu or vfio-data > > structure. > > > > Great, but you're trying to add a generic API to the IOMMU subsystem that's > > difficult to use correctly. The fact that you use it correctly does not justify > > the API. > > > > > > because you can't check or > > > > enforce that an arbitrary caller is using it correctly. > > > > > > I am not sure what is arbitrary caller? pdev is known to vfio, so vfio > > > will only make pci_enable_msix()/pci_enable_msi_block() for this pdev. > > > If anyother code makes then it is some other unexpectedly thing > > > happening in system, no? > > > > What's proposed here is a generic IOMMU API. Anybody can call this. > > What if the host SCSI driver decides to go get the iommu domain for it's device > > (or any other device)? Does that fit your usage model? > > > > > > It's not maintainable. > > > > Thanks, > > > > > > I do not have any issue with this as well, can you also describe the > > > type of API you are envisioning; I can think of defining some function > > > in vfio.c/vfio_iommu*.c, make them global and declare then in > > > include/Linux/vfio.h And include <Linux/vfio.h> in caller file > > > (arch/powerpc/kernel/msi.c) > > > > Do you really want module dependencies between vfio and your core kernel MSI > > setup? Look at the vfio external user interface that we've already defined. > > That allows other components of the kernel to get a proper reference to a vfio > > group. From there you can work out how to get what you want. Another > > alternative is that vfio could register an MSI to IOVA mapping with architecture > > code when the mapping is created. The MSI setup path could then do a lookup in > > architecture code for the mapping. You could even store the MSI to IOVA mapping > > in VFIO and create an interface where SET_IRQ passes that mapping into setup > > code. > > Ok, What I want is to get IOVA associated with a physical address > (physical address of MSI-bank). > And currently I do not see a way to know IOVA of a physical address > and doing all this domain get and then search through all of > iommu-windows of that domain. > > What if we add an iommu-API which can return the IOVA mapping of a > physical address. Current use case is setting up MSI's for aperture > type of IOMMU also getting a phys_to_iova() mapping is independent of > VFIO, your thought? A physical address can be mapped to multiple IOVAs, so the interface seems flawed by design. It also has the same problem as above, it's a backdoor that can be called asynchronous to the owner of the domain, so what reason is there to believe the result? It just replaces an iommu_domain pointer with an IOVA. VFIO knows this mapping, so why are we trying to go behind its back and ask the IOMMU? Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiA+ID4gRG8geW91IHJlYWxseSB3YW50IG1vZHVsZSBkZXBlbmRlbmNpZXMgYmV0d2VlbiB2Zmlv IGFuZCB5b3VyIGNvcmUNCj4gPiA+IGtlcm5lbCBNU0kgc2V0dXA/ICBMb29rIGF0IHRoZSB2Zmlv IGV4dGVybmFsIHVzZXIgaW50ZXJmYWNlIHRoYXQgd2UndmUNCj4gYWxyZWFkeSBkZWZpbmVkLg0K PiA+ID4gVGhhdCBhbGxvd3Mgb3RoZXIgY29tcG9uZW50cyBvZiB0aGUga2VybmVsIHRvIGdldCBh IHByb3BlciByZWZlcmVuY2UNCj4gPiA+IHRvIGEgdmZpbyBncm91cC4gIEZyb20gdGhlcmUgeW91 IGNhbiB3b3JrIG91dCBob3cgdG8gZ2V0IHdoYXQgeW91DQo+ID4gPiB3YW50LiAgQW5vdGhlciBh bHRlcm5hdGl2ZSBpcyB0aGF0IHZmaW8gY291bGQgcmVnaXN0ZXIgYW4gTVNJIHRvDQo+ID4gPiBJ T1ZBIG1hcHBpbmcgd2l0aCBhcmNoaXRlY3R1cmUgY29kZSB3aGVuIHRoZSBtYXBwaW5nIGlzIGNy ZWF0ZWQuDQo+ID4gPiBUaGUgTVNJIHNldHVwIHBhdGggY291bGQgdGhlbiBkbyBhIGxvb2t1cCBp biBhcmNoaXRlY3R1cmUgY29kZSBmb3INCj4gPiA+IHRoZSBtYXBwaW5nLiAgWW91IGNvdWxkIGV2 ZW4gc3RvcmUgdGhlIE1TSSB0byBJT1ZBIG1hcHBpbmcgaW4gVkZJTw0KPiA+ID4gYW5kIGNyZWF0 ZSBhbiBpbnRlcmZhY2Ugd2hlcmUgU0VUX0lSUSBwYXNzZXMgdGhhdCBtYXBwaW5nIGludG8gc2V0 dXAgY29kZS4NCj4gPg0KPiA+IE9rLCBXaGF0IEkgd2FudCBpcyB0byBnZXQgSU9WQSBhc3NvY2lh dGVkIHdpdGggYSBwaHlzaWNhbCBhZGRyZXNzDQo+ID4gKHBoeXNpY2FsIGFkZHJlc3Mgb2YgTVNJ LWJhbmspLg0KPiA+IEFuZCBjdXJyZW50bHkgSSBkbyBub3Qgc2VlIGEgd2F5IHRvIGtub3cgSU9W QSBvZiBhIHBoeXNpY2FsIGFkZHJlc3MNCj4gPiBhbmQgZG9pbmcgYWxsIHRoaXMgZG9tYWluIGdl dCBhbmQgdGhlbiBzZWFyY2ggdGhyb3VnaCBhbGwgb2YNCj4gPiBpb21tdS13aW5kb3dzIG9mIHRo YXQgZG9tYWluLg0KPiA+DQo+ID4gV2hhdCBpZiB3ZSBhZGQgYW4gaW9tbXUtQVBJIHdoaWNoIGNh biByZXR1cm4gdGhlIElPVkEgbWFwcGluZyBvZiBhDQo+ID4gcGh5c2ljYWwgYWRkcmVzcy4gQ3Vy cmVudCB1c2UgY2FzZSBpcyBzZXR0aW5nIHVwIE1TSSdzIGZvciBhcGVydHVyZQ0KPiA+IHR5cGUg b2YgSU9NTVUgYWxzbyBnZXR0aW5nIGEgcGh5c190b19pb3ZhKCkgbWFwcGluZyBpcyBpbmRlcGVu ZGVudCBvZg0KPiA+IFZGSU8sIHlvdXIgdGhvdWdodD8NCj4gDQo+IEEgcGh5c2ljYWwgYWRkcmVz cyBjYW4gYmUgbWFwcGVkIHRvIG11bHRpcGxlIElPVkFzLCBzbyB0aGUgaW50ZXJmYWNlIHNlZW1z DQo+IGZsYXdlZCBieSBkZXNpZ24uICBJdCBhbHNvIGhhcyB0aGUgc2FtZSBwcm9ibGVtIGFzIGFi b3ZlLCBpdCdzIGEgYmFja2Rvb3IgdGhhdA0KPiBjYW4gYmUgY2FsbGVkIGFzeW5jaHJvbm91cyB0 byB0aGUgb3duZXIgb2YgdGhlIGRvbWFpbiwgc28gd2hhdCByZWFzb24gaXMgdGhlcmUNCj4gdG8g YmVsaWV2ZSB0aGUgcmVzdWx0PyAgSXQganVzdCByZXBsYWNlcyBhbiBpb21tdV9kb21haW4gcG9p bnRlciB3aXRoIGFuIElPVkEuDQo+IFZGSU8ga25vd3MgdGhpcyBtYXBwaW5nLCBzbyB3aHkgYXJl IHdlIHRyeWluZyB0byBnbyBiZWhpbmQgaXRzIGJhY2sgYW5kIGFzayB0aGUNCj4gSU9NTVU/DQpJ T01NVSBpcyB0aGUgZmluYWwgcGxhY2Ugd2hlcmUgbWFwcGluZyBpcyBjcmVhdGVkLCBzbyBtYXkg YmUgdG9kYXkgaXQgaXMgY2FsbGluZyBvbiBiZWhhbGYgb2YgVkZJTywgdG9tb3Jyb3cgaXQgY2Fu IGJlIGZvciBub3JtYWwgTGludXggb3Igc29tZSBvdGhlciBpbnRlcmZhY2UuIEJ1dCBJIGFtIGZp bmUgdG8gZGlyZWN0bHkgdGFsayB0byB2ZmlvIGFuZCB3aWxsIG5vdCB0cnkgdG8gc29sdmUgYSBw cm9ibGVtIHdoaWNoIGRvZXMgbm90IGV4aXN0cyB0b2RheS4NCg0KTVNJIHN1YnN5c3RlbSBrbm93 cyBwZGV2IChwY2kgZGV2aWNlKSBhbmQgcGh5c2ljYWwgYWRkcmVzcywgdGhlbiB3aGF0IGludGVy ZmFjZSBpdCB3aWxsIHVzZSB0byBnZXQgdGhlIElPVkEgZnJvbSBWRklPPw0KDQpUaGFua3MNCi1C aGFyYXQNCg0KPiAgVGhhbmtzLA0KPiANCj4gQWxleA0KPiANCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu- > bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson > Sent: Tuesday, October 08, 2013 8:43 AM > To: Bhushan Bharat-R65777 > Cc: agraf@suse.de; Wood Scott-B07421; linux-pci@vger.kernel.org; > galak@kernel.crashing.org; linux-kernel@vger.kernel.org; > iommu@lists.linux-foundation.org; benh@kernel.crashing.org; linuxppc- > dev@lists.ozlabs.org > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > On Mon, 2013-10-07 at 05:46 +0000, Bhushan Bharat-R65777 wrote: > > > > > -----Original Message----- > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > Sent: Friday, October 04, 2013 11:42 PM > > > To: Bhushan Bharat-R65777 > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > device > > > > > > On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > -----Original Message----- > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > Sent: Friday, October 04, 2013 10:43 PM > > > > > To: Bhushan Bharat-R65777 > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > foundation.org > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > > device > > > > > > > > > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > > > Sent: Friday, October 04, 2013 9:15 PM > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > > foundation.org > > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain > > > > > > > of a device > > > > > > > > > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 > wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > From: linux-pci-owner@vger.kernel.org > > > > > > > > > [mailto:linux-pci-owner@vger.kernel.org] > > > > > > > > > On Behalf Of Alex Williamson > > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > > > galak@kernel.crashing.org; linux- > > > > > > > > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; > > > > > > > > > linux- pci@vger.kernel.org; agraf@suse.de; Wood > > > > > > > > > Scott-B07421; iommu@lists.linux- 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@freescale.com> > > > > > > > > > > --- > > > > > > > > > > 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? > > > > > > > > > > > > > > No, it's not sufficient to make sure that your use of the > > > > > > > interface is race free. The interface itself needs to be > > > > > > > designed so that it's difficult to use incorrectly. > > > > > > > > > > > > So we can define > > > > > > iommu_get_dev_domain()/iommu_put_dev_domain(); > > > > > > iommu_get_dev_domain() will return domain with the lock held, > > > > > > and > > > > > > iommu_put_dev_domain() will release the lock? And > > > > > > iommu_get_dev_domain() must always be followed by > > > > > > iommu_get_dev_domain(). > > > > > > > > > > What lock? get/put are generally used for reference counting, > > > > > not locking in the kernel. > > > > > > > > > > > > That's not the case here. This is a backdoor to get the > > > > > > > iommu domain from the iommu driver regardless of who is using > it or how. > > > > > > > The iommu domain is created and managed by vfio, so > > > > > > > shouldn't we be looking at how to do this through vfio? > > > > > > > > > > > > Let me first describe what we are doing here: > > > > > > During initialization:- > > > > > > - vfio talks to MSI system to know the MSI-page and size > > > > > > - vfio then interacts with iommu to map the MSI-page in iommu > > > > > > (IOVA is decided by userspace and physical address is the > > > > > > MSI-page) > > > > > > - So the IOVA subwindow mapping is created in iommu and yes > > > > > > VFIO know about > > > > > this mapping. > > > > > > > > > > > > Now do SET_IRQ(MSI/MSIX) ioctl: > > > > > > - calls pci_enable_msix()/pci_enable_msi_block(): which is > > > > > > supposed to set > > > > > MSI address/data in device. > > > > > > - So in current implementation (this patchset) msi-subsystem > > > > > > gets the IOVA > > > > > from iommu via this defined interface. > > > > > > - Are you saying that rather than getting this from iommu, we > > > > > > should get this > > > > > from vfio? What difference does this make? > > > > > > > > > > Yes, you just said above that vfio knows the msi to iova > > > > > mapping, so why go outside of vfio to find it later? The > > > > > difference is one case you can have a proper reference to data > > > > > structures to make sure the pointer you get back actually has > > > > > meaning at the time you're using it vs the code here where > > > > > you're defining an API that returns a meaningless value > > > > > > > > With FSL-PAMU we will always get consistant data from iommu or > > > > vfio-data > > > structure. > > > > > > Great, but you're trying to add a generic API to the IOMMU subsystem > > > that's difficult to use correctly. The fact that you use it > > > correctly does not justify the API. > > > > > > > > because you can't check or > > > > > enforce that an arbitrary caller is using it correctly. > > > > > > > > I am not sure what is arbitrary caller? pdev is known to vfio, so > > > > vfio will only make pci_enable_msix()/pci_enable_msi_block() for > this pdev. > > > > If anyother code makes then it is some other unexpectedly thing > > > > happening in system, no? > > > > > > What's proposed here is a generic IOMMU API. Anybody can call this. > > > What if the host SCSI driver decides to go get the iommu domain for > > > it's device (or any other device)? Does that fit your usage model? > > > > > > > > It's not maintainable. > > > > > Thanks, > > > > > > > > I do not have any issue with this as well, can you also describe > > > > the type of API you are envisioning; I can think of defining some > > > > function in vfio.c/vfio_iommu*.c, make them global and declare > > > > then in include/Linux/vfio.h And include <Linux/vfio.h> in caller > > > > file > > > > (arch/powerpc/kernel/msi.c) > > > > > > Do you really want module dependencies between vfio and your core > > > kernel MSI setup? Look at the vfio external user interface that > we've already defined. > > > That allows other components of the kernel to get a proper reference > > > to a vfio group. From there you can work out how to get what you > > > want. Another alternative is that vfio could register an MSI to > > > IOVA mapping with architecture code when the mapping is created. > > > The MSI setup path could then do a lookup in architecture code for > > > the mapping. You could even store the MSI to IOVA mapping in VFIO > > > and create an interface where SET_IRQ passes that mapping into setup > code. [Sethi Varun-B16395] What you are suggesting is that the MSI setup path queries the vfio subsystem for the mapping, rather than directly querying the iommu subsystem. So, say if we add an interface for getting MSI to IOVA mapping in the msi setup path, wouldn't this again be specific to vfio? I reckon that this interface would again ppc machine specific interface. -Varun -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2013-10-10 at 20:09 +0000, Sethi Varun-B16395 wrote: > > > -----Original Message----- > > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu- > > bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson > > Sent: Tuesday, October 08, 2013 8:43 AM > > To: Bhushan Bharat-R65777 > > Cc: agraf@suse.de; Wood Scott-B07421; linux-pci@vger.kernel.org; > > galak@kernel.crashing.org; linux-kernel@vger.kernel.org; > > iommu@lists.linux-foundation.org; benh@kernel.crashing.org; linuxppc- > > dev@lists.ozlabs.org > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > > > On Mon, 2013-10-07 at 05:46 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > -----Original Message----- > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > Sent: Friday, October 04, 2013 11:42 PM > > > > To: Bhushan Bharat-R65777 > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > device > > > > > > > > On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > -----Original Message----- > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > > Sent: Friday, October 04, 2013 10:43 PM > > > > > > To: Bhushan Bharat-R65777 > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > foundation.org > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > > > device > > > > > > > > > > > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > > > > Sent: Friday, October 04, 2013 9:15 PM > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > > > foundation.org > > > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain > > > > > > > > of a device > > > > > > > > > > > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 > > wrote: > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: linux-pci-owner@vger.kernel.org > > > > > > > > > > [mailto:linux-pci-owner@vger.kernel.org] > > > > > > > > > > On Behalf Of Alex Williamson > > > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > > > > galak@kernel.crashing.org; linux- > > > > > > > > > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; > > > > > > > > > > linux- pci@vger.kernel.org; agraf@suse.de; Wood > > > > > > > > > > Scott-B07421; iommu@lists.linux- 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@freescale.com> > > > > > > > > > > > --- > > > > > > > > > > > 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? > > > > > > > > > > > > > > > > No, it's not sufficient to make sure that your use of the > > > > > > > > interface is race free. The interface itself needs to be > > > > > > > > designed so that it's difficult to use incorrectly. > > > > > > > > > > > > > > So we can define > > > > > > > iommu_get_dev_domain()/iommu_put_dev_domain(); > > > > > > > iommu_get_dev_domain() will return domain with the lock held, > > > > > > > and > > > > > > > iommu_put_dev_domain() will release the lock? And > > > > > > > iommu_get_dev_domain() must always be followed by > > > > > > > iommu_get_dev_domain(). > > > > > > > > > > > > What lock? get/put are generally used for reference counting, > > > > > > not locking in the kernel. > > > > > > > > > > > > > > That's not the case here. This is a backdoor to get the > > > > > > > > iommu domain from the iommu driver regardless of who is using > > it or how. > > > > > > > > The iommu domain is created and managed by vfio, so > > > > > > > > shouldn't we be looking at how to do this through vfio? > > > > > > > > > > > > > > Let me first describe what we are doing here: > > > > > > > During initialization:- > > > > > > > - vfio talks to MSI system to know the MSI-page and size > > > > > > > - vfio then interacts with iommu to map the MSI-page in iommu > > > > > > > (IOVA is decided by userspace and physical address is the > > > > > > > MSI-page) > > > > > > > - So the IOVA subwindow mapping is created in iommu and yes > > > > > > > VFIO know about > > > > > > this mapping. > > > > > > > > > > > > > > Now do SET_IRQ(MSI/MSIX) ioctl: > > > > > > > - calls pci_enable_msix()/pci_enable_msi_block(): which is > > > > > > > supposed to set > > > > > > MSI address/data in device. > > > > > > > - So in current implementation (this patchset) msi-subsystem > > > > > > > gets the IOVA > > > > > > from iommu via this defined interface. > > > > > > > - Are you saying that rather than getting this from iommu, we > > > > > > > should get this > > > > > > from vfio? What difference does this make? > > > > > > > > > > > > Yes, you just said above that vfio knows the msi to iova > > > > > > mapping, so why go outside of vfio to find it later? The > > > > > > difference is one case you can have a proper reference to data > > > > > > structures to make sure the pointer you get back actually has > > > > > > meaning at the time you're using it vs the code here where > > > > > > you're defining an API that returns a meaningless value > > > > > > > > > > With FSL-PAMU we will always get consistant data from iommu or > > > > > vfio-data > > > > structure. > > > > > > > > Great, but you're trying to add a generic API to the IOMMU subsystem > > > > that's difficult to use correctly. The fact that you use it > > > > correctly does not justify the API. > > > > > > > > > > because you can't check or > > > > > > enforce that an arbitrary caller is using it correctly. > > > > > > > > > > I am not sure what is arbitrary caller? pdev is known to vfio, so > > > > > vfio will only make pci_enable_msix()/pci_enable_msi_block() for > > this pdev. > > > > > If anyother code makes then it is some other unexpectedly thing > > > > > happening in system, no? > > > > > > > > What's proposed here is a generic IOMMU API. Anybody can call this. > > > > What if the host SCSI driver decides to go get the iommu domain for > > > > it's device (or any other device)? Does that fit your usage model? > > > > > > > > > > It's not maintainable. > > > > > > Thanks, > > > > > > > > > > I do not have any issue with this as well, can you also describe > > > > > the type of API you are envisioning; I can think of defining some > > > > > function in vfio.c/vfio_iommu*.c, make them global and declare > > > > > then in include/Linux/vfio.h And include <Linux/vfio.h> in caller > > > > > file > > > > > (arch/powerpc/kernel/msi.c) > > > > > > > > Do you really want module dependencies between vfio and your core > > > > kernel MSI setup? Look at the vfio external user interface that > > we've already defined. > > > > That allows other components of the kernel to get a proper reference > > > > to a vfio group. From there you can work out how to get what you > > > > want. Another alternative is that vfio could register an MSI to > > > > IOVA mapping with architecture code when the mapping is created. > > > > The MSI setup path could then do a lookup in architecture code for > > > > the mapping. You could even store the MSI to IOVA mapping in VFIO > > > > and create an interface where SET_IRQ passes that mapping into setup > > code. > [Sethi Varun-B16395] What you are suggesting is that the MSI setup > path queries the vfio subsystem for the mapping, rather than directly > querying the iommu subsystem. So, say if we add an interface for > getting MSI to IOVA mapping in the msi setup path, wouldn't this again > be specific to vfio? I reckon that this interface would again ppc > machine specific interface. Sure, if this is a generic MSI setup path then clearly vfio should not be involved. But by that same notion, if this is a generic MSI setup path, how can the proposed solutions guarantee that the iommu_domain or iova returned is still valid in all cases? It's racy. If the caller trying to setup MSI has the information needed, why doesn't it pass it in as part of the setup? Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQWxleCBXaWxsaWFtc29u IFttYWlsdG86YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb21dDQo+IFNlbnQ6IEZyaWRheSwgT2N0 b2JlciAxMSwgMjAxMyAyOjEyIEFNDQo+IFRvOiBTZXRoaSBWYXJ1bi1CMTYzOTUNCj4gQ2M6IEJo dXNoYW4gQmhhcmF0LVI2NTc3NzsgYWdyYWZAc3VzZS5kZTsgV29vZCBTY290dC1CMDc0MjE7IGxp bnV4LQ0KPiBwY2lAdmdlci5rZXJuZWwub3JnOyBnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnOyBs aW51eC0NCj4ga2VybmVsQHZnZXIua2VybmVsLm9yZzsgaW9tbXVAbGlzdHMubGludXgtZm91bmRh dGlvbi5vcmc7DQo+IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsgbGludXhwcGMtZGV2QGxpc3Rz Lm96bGFicy5vcmcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAyLzddIGlvbW11OiBhZGQgYXBpIHRv IGdldCBpb21tdV9kb21haW4gb2YgYSBkZXZpY2UNCj4gDQo+IE9uIFRodSwgMjAxMy0xMC0xMCBh dCAyMDowOSArMDAwMCwgU2V0aGkgVmFydW4tQjE2Mzk1IHdyb3RlOg0KPiA+DQo+ID4gPiAtLS0t LU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+ID4gRnJvbTogaW9tbXUtYm91bmNlc0BsaXN0cy5s aW51eC1mb3VuZGF0aW9uLm9yZyBbbWFpbHRvOmlvbW11LQ0KPiA+ID4gYm91bmNlc0BsaXN0cy5s aW51eC1mb3VuZGF0aW9uLm9yZ10gT24gQmVoYWxmIE9mIEFsZXggV2lsbGlhbXNvbg0KPiA+ID4g U2VudDogVHVlc2RheSwgT2N0b2JlciAwOCwgMjAxMyA4OjQzIEFNDQo+ID4gPiBUbzogQmh1c2hh biBCaGFyYXQtUjY1Nzc3DQo+ID4gPiBDYzogYWdyYWZAc3VzZS5kZTsgV29vZCBTY290dC1CMDc0 MjE7IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7DQo+ID4gPiBnYWxha0BrZXJuZWwuY3Jhc2hp bmcub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOw0KPiA+ID4gaW9tbXVAbGlzdHMu bGludXgtZm91bmRhdGlvbi5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsNCj4gPiA+IGxp bnV4cHBjLSBkZXZAbGlzdHMub3psYWJzLm9yZw0KPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCAy LzddIGlvbW11OiBhZGQgYXBpIHRvIGdldCBpb21tdV9kb21haW4gb2YgYQ0KPiA+ID4gZGV2aWNl DQo+ID4gPg0KPiA+ID4gT24gTW9uLCAyMDEzLTEwLTA3IGF0IDA1OjQ2ICswMDAwLCBCaHVzaGFu IEJoYXJhdC1SNjU3Nzcgd3JvdGU6DQo+ID4gPiA+DQo+ID4gPiA+ID4gLS0tLS1PcmlnaW5hbCBN ZXNzYWdlLS0tLS0NCj4gPiA+ID4gPiBGcm9tOiBBbGV4IFdpbGxpYW1zb24gW21haWx0bzphbGV4 LndpbGxpYW1zb25AcmVkaGF0LmNvbV0NCj4gPiA+ID4gPiBTZW50OiBGcmlkYXksIE9jdG9iZXIg MDQsIDIwMTMgMTE6NDIgUE0NCj4gPiA+ID4gPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ ID4gPiA+ID4gQ2M6IGpvcm9AOGJ5dGVzLm9yZzsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOw0K PiA+ID4gPiA+IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IGxpbnV4LSBrZXJuZWxAdmdlci5r ZXJuZWwub3JnOw0KPiA+ID4gPiA+IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51 eC0gcGNpQHZnZXIua2VybmVsLm9yZzsNCj4gPiA+ID4gPiBhZ3JhZkBzdXNlLmRlOyBXb29kIFNj b3R0LUIwNzQyMTsgaW9tbXVAbGlzdHMubGludXgtDQo+ID4gPiA+ID4gZm91bmRhdGlvbi5vcmcN Cj4gPiA+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENIIDIvN10gaW9tbXU6IGFkZCBhcGkgdG8gZ2V0 IGlvbW11X2RvbWFpbiBvZiBhDQo+ID4gPiA+ID4gZGV2aWNlDQo+ID4gPiA+ID4NCj4gPiA+ID4g PiBPbiBGcmksIDIwMTMtMTAtMDQgYXQgMTc6MjMgKzAwMDAsIEJodXNoYW4gQmhhcmF0LVI2NTc3 NyB3cm90ZToNCj4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2Fn ZS0tLS0tDQo+ID4gPiA+ID4gPiA+IEZyb206IEFsZXggV2lsbGlhbXNvbiBbbWFpbHRvOmFsZXgu d2lsbGlhbXNvbkByZWRoYXQuY29tXQ0KPiA+ID4gPiA+ID4gPiBTZW50OiBGcmlkYXksIE9jdG9i ZXIgMDQsIDIwMTMgMTA6NDMgUE0NCj4gPiA+ID4gPiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2 NTc3Nw0KPiA+ID4gPiA+ID4gPiBDYzogam9yb0A4Ynl0ZXMub3JnOyBiZW5oQGtlcm5lbC5jcmFz aGluZy5vcmc7DQo+ID4gPiA+ID4gPiA+IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IGxpbnV4 LSBrZXJuZWxAdmdlci5rZXJuZWwub3JnOw0KPiA+ID4gPiA+ID4gPiBsaW51eHBwYy1kZXZAbGlz dHMub3psYWJzLm9yZzsgbGludXgtIHBjaUB2Z2VyLmtlcm5lbC5vcmc7DQo+ID4gPiA+ID4gPiA+ IGFncmFmQHN1c2UuZGU7IFdvb2QgU2NvdHQtQjA3NDIxOyBpb21tdUBsaXN0cy5saW51eC0NCj4g PiA+ID4gPiA+ID4gZm91bmRhdGlvbi5vcmcNCj4gPiA+ID4gPiA+ID4gU3ViamVjdDogUmU6IFtQ QVRDSCAyLzddIGlvbW11OiBhZGQgYXBpIHRvIGdldCBpb21tdV9kb21haW4NCj4gPiA+ID4gPiA+ ID4gb2YgYSBkZXZpY2UNCj4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gT24gRnJpLCAyMDEz LTEwLTA0IGF0IDE2OjQ3ICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3NzcNCj4gd3JvdGU6DQo+ ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0t LS0tDQo+ID4gPiA+ID4gPiA+ID4gPiBGcm9tOiBBbGV4IFdpbGxpYW1zb24NCj4gPiA+ID4gPiA+ ID4gPiA+IFttYWlsdG86YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb21dDQo+ID4gPiA+ID4gPiA+ ID4gPiBTZW50OiBGcmlkYXksIE9jdG9iZXIgMDQsIDIwMTMgOToxNSBQTQ0KPiA+ID4gPiA+ID4g PiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0KPiA+ID4gPiA+ID4gPiA+ID4gQ2M6IGpv cm9AOGJ5dGVzLm9yZzsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOw0KPiA+ID4gPiA+ID4gPiA+ ID4gZ2FsYWtAa2VybmVsLmNyYXNoaW5nLm9yZzsgbGludXgtDQo+ID4gPiA+ID4gPiA+ID4gPiBr ZXJuZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsNCj4g PiA+ID4gPiA+ID4gPiA+IGxpbnV4LSBwY2lAdmdlci5rZXJuZWwub3JnOyBhZ3JhZkBzdXNlLmRl OyBXb29kDQo+ID4gPiA+ID4gPiA+ID4gPiBTY290dC1CMDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4 LSBmb3VuZGF0aW9uLm9yZw0KPiA+ID4gPiA+ID4gPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCAy LzddIGlvbW11OiBhZGQgYXBpIHRvIGdldA0KPiA+ID4gPiA+ID4gPiA+ID4gaW9tbXVfZG9tYWlu IG9mIGEgZGV2aWNlDQo+ID4gPiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiA+ID4gT24gRnJp LCAyMDEzLTEwLTA0IGF0IDA5OjU0ICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3NzcNCj4gPiA+ IHdyb3RlOg0KPiA+ID4gPiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+IC0tLS0t T3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gRnJvbTogbGludXgt cGNpLW93bmVyQHZnZXIua2VybmVsLm9yZw0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+IFttYWlsdG86 bGludXgtcGNpLW93bmVyQHZnZXIua2VybmVsLm9yZ10NCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiBP biBCZWhhbGYgT2YgQWxleCBXaWxsaWFtc29uDQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gU2VudDog V2VkbmVzZGF5LCBTZXB0ZW1iZXIgMjUsIDIwMTMgMTA6MTYgUE0NCj4gPiA+ID4gPiA+ID4gPiA+ ID4gPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gQ2M6 IGpvcm9AOGJ5dGVzLm9yZzsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOw0KPiA+ID4gPiA+ID4g PiA+ID4gPiA+IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IGxpbnV4LQ0KPiA+ID4gPiA+ID4g PiA+ID4gPiA+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4g bGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gbGlu dXgtIHBjaUB2Z2VyLmtlcm5lbC5vcmc7IGFncmFmQHN1c2UuZGU7IFdvb2QNCj4gPiA+ID4gPiA+ ID4gPiA+ID4gPiBTY290dC1CMDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LSBmb3VuZGF0aW9uLm9y ZzsNCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiBCaHVzaGFuDQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4g QmhhcmF0LVI2NTc3Nw0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg Mi83XSBpb21tdTogYWRkIGFwaSB0byBnZXQNCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiBpb21tdV9k b21haW4gb2YgYSBkZXZpY2UNCj4gPiA+ID4gPiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiA+ ID4gPiA+IE9uIFRodSwgMjAxMy0wOS0xOSBhdCAxMjo1OSArMDUzMCwgQmhhcmF0IEJodXNoYW4N Cj4gd3JvdGU6DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gPiBUaGlzIGFwaSByZXR1cm4gdGhlIGlv bW11IGRvbWFpbiB0byB3aGljaCB0aGUNCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiA+IGRldmljZSBp cw0KPiA+ID4gYXR0YWNoZWQuDQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gPiBUaGUgaW9tbXVfZG9t YWluIGlzIHJlcXVpcmVkIGZvciBtYWtpbmcgQVBJIGNhbGxzDQo+ID4gPiA+ID4gPiA+ID4gPiA+ ID4gPiByZWxhdGVkIHRvDQo+ID4gPiA+ID4gaW9tbXUuDQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4g PiBGb2xsb3cgdXAgcGF0Y2hlcyB3aGljaCB1c2UgdGhpcyBBUEkgdG8ga25vdyBpb21tdQ0KPiA+ ID4gbWFwaW5nLg0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPiA+ID4g PiA+IFNpZ25lZC1vZmYtYnk6IEJoYXJhdCBCaHVzaGFuDQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4g PiA8YmhhcmF0LmJodXNoYW5AZnJlZXNjYWxlLmNvbT4NCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiA+ IC0tLQ0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+ID4gIGRyaXZlcnMvaW9tbXUvaW9tbXUuYyB8ICAg MTAgKysrKysrKysrKw0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+ID4gIGluY2x1ZGUvbGludXgvaW9t bXUuaCB8ICAgIDcgKysrKysrKw0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+ID4gIDIgZmlsZXMgY2hh bmdlZCwgMTcgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkNCj4gPiA+ID4gPiA+ID4gPiA+ ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9pb21t dS9pb21tdS5jDQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gPiBiL2RyaXZlcnMvaW9tbXUvaW9tbXUu YyBpbmRleA0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+ID4gZmJlOWNhNy4uNmFjNWY1MCAxMDA2NDQN Cj4gPiA+ID4gPiA+ID4gPiA+ID4gPiA+IC0tLSBhL2RyaXZlcnMvaW9tbXUvaW9tbXUuYw0KPiA+ ID4gPiA+ID4gPiA+ID4gPiA+ID4gKysrIGIvZHJpdmVycy9pb21tdS9pb21tdS5jDQo+ID4gPiA+ ID4gPiA+ID4gPiA+ID4gPiBAQCAtNjk2LDYgKzY5NiwxNiBAQCB2b2lkDQo+ID4gPiA+ID4gPiA+ ID4gPiA+ID4gPiBpb21tdV9kZXRhY2hfZGV2aWNlKHN0cnVjdCBpb21tdV9kb21haW4gKmRvbWFp biwNCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiA+IHN0cnVjdCBkZXZpY2UgKmRldikgIH0NCj4gPiA+ ID4gPiA+ID4gPiA+ID4gPiA+IEVYUE9SVF9TWU1CT0xfR1BMKGlvbW11X2RldGFjaF9kZXZpY2Up Ow0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiA+ICtzdHJ1 Y3QgaW9tbXVfZG9tYWluICppb21tdV9nZXRfZGV2X2RvbWFpbihzdHJ1Y3QNCj4gPiA+IGRldmlj ZSAqZGV2KSB7DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gPiArCXN0cnVjdCBpb21tdV9vcHMgKm9w cyA9IGRldi0+YnVzLT5pb21tdV9vcHM7DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gPiArDQo+ID4g PiA+ID4gPiA+ID4gPiA+ID4gPiArCWlmICh1bmxpa2VseShvcHMgPT0gTlVMTCB8fA0KPiA+ID4g PiA+ID4gPiA+ID4gPiA+ID4gK29wcy0+Z2V0X2Rldl9pb21tdV9kb21haW4gPT0NCj4gPiA+ID4g PiBOVUxMKSkNCj4gPiA+ID4gPiA+ID4gPiA+ID4gPiA+ICsJCXJldHVybiBOVUxMOw0KPiA+ID4g PiA+ID4gPiA+ID4gPiA+ID4gKw0KPiA+ID4gPiA+ID4gPiA+ID4gPiA+ID4gKwlyZXR1cm4gb3Bz LT5nZXRfZGV2X2lvbW11X2RvbWFpbihkZXYpOyB9DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gPiAr RVhQT1JUX1NZTUJPTF9HUEwoaW9tbXVfZ2V0X2Rldl9kb21haW4pOw0KPiA+ID4gPiA+ID4gPiA+ ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gV2hhdCBwcmV2ZW50cyB0aGlzIGZyb20gcmFj aW5nIGlvbW11X2RvbWFpbl9mcmVlKCk/DQo+ID4gPiA+ID4gPiA+ID4gPiA+ID4gVGhlcmUncyBu byByZWZlcmVuY2VzIGFjcXVpcmVkLCBzbyB0aGVyZSdzIG5vIHJlYXNvbg0KPiA+ID4gPiA+ID4g PiA+ID4gPiA+IGZvciB0aGUgY2FsbGVyIHRvIGFzc3VtZSB0aGUNCj4gPiA+ID4gPiA+ID4gPiA+ IHBvaW50ZXIgaXMgdmFsaWQuDQo+ID4gPiA+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4g PiA+IFNvcnJ5IGZvciBsYXRlIHF1ZXJ5LCBzb21laG93IHRoaXMgZW1haWwgd2VudCBpbnRvIGEN Cj4gPiA+ID4gPiA+ID4gPiA+ID4gZm9sZGVyIGFuZCBlc2NhcGVkOw0KPiA+ID4gPiA+ID4gPiA+ ID4gPg0KPiA+ID4gPiA+ID4gPiA+ID4gPiBKdXN0IHRvIGJlIHN1cmUsIHRoZXJlIGlzIG5vdCBs b2NrIGF0IGdlbmVyaWMgInN0cnVjdA0KPiA+ID4gPiA+ID4gPiA+ID4gPiBpb21tdV9kb21haW4i LCBidXQgSVANCj4gPiA+ID4gPiA+ID4gPiA+IHNwZWNpZmljIHN0cnVjdHVyZSAobGluayBGU0wg ZG9tYWluKSBsaW5rZWQgaW4NCj4gPiA+ID4gPiA+ID4gPiA+IGlvbW11X2RvbWFpbi0+cHJpdiBo YXZlIGEgbG9jaywgc28gd2UgbmVlZCB0byBlbnN1cmUNCj4gPiA+ID4gPiA+ID4gPiA+IHRoaXMg cmFjZSBpbiBGU0wgaW9tbXUgY29kZSAoc2F5DQo+ID4gPiA+ID4gPiA+ID4gPiBkcml2ZXJzL2lv bW11L2ZzbF9wYW11X2RvbWFpbi5jKSwNCj4gPiA+IHJpZ2h0Pw0KPiA+ID4gPiA+ID4gPiA+ID4N Cj4gPiA+ID4gPiA+ID4gPiA+IE5vLCBpdCdzIG5vdCBzdWZmaWNpZW50IHRvIG1ha2Ugc3VyZSB0 aGF0IHlvdXIgdXNlIG9mDQo+ID4gPiA+ID4gPiA+ID4gPiB0aGUgaW50ZXJmYWNlIGlzIHJhY2Ug ZnJlZS4gIFRoZSBpbnRlcmZhY2UgaXRzZWxmIG5lZWRzDQo+ID4gPiA+ID4gPiA+ID4gPiB0byBi ZSBkZXNpZ25lZCBzbyB0aGF0IGl0J3MgZGlmZmljdWx0IHRvIHVzZSBpbmNvcnJlY3RseS4NCj4g PiA+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiA+IFNvIHdlIGNhbiBkZWZpbmUNCj4gPiA+ID4g PiA+ID4gPiBpb21tdV9nZXRfZGV2X2RvbWFpbigpL2lvbW11X3B1dF9kZXZfZG9tYWluKCk7DQo+ ID4gPiA+ID4gPiA+ID4gaW9tbXVfZ2V0X2Rldl9kb21haW4oKSB3aWxsIHJldHVybiBkb21haW4g d2l0aCB0aGUgbG9jaw0KPiA+ID4gPiA+ID4gPiA+IGhlbGQsIGFuZA0KPiA+ID4gPiA+ID4gPiA+ IGlvbW11X3B1dF9kZXZfZG9tYWluKCkgd2lsbCByZWxlYXNlIHRoZSBsb2NrPyBBbmQNCj4gPiA+ ID4gPiA+ID4gPiBpb21tdV9nZXRfZGV2X2RvbWFpbigpIG11c3QgYWx3YXlzIGJlIGZvbGxvd2Vk IGJ5DQo+ID4gPiA+ID4gPiA+ID4gaW9tbXVfZ2V0X2Rldl9kb21haW4oKS4NCj4gPiA+ID4gPiA+ ID4NCj4gPiA+ID4gPiA+ID4gV2hhdCBsb2NrPyAgZ2V0L3B1dCBhcmUgZ2VuZXJhbGx5IHVzZWQg Zm9yIHJlZmVyZW5jZQ0KPiA+ID4gPiA+ID4gPiBjb3VudGluZywgbm90IGxvY2tpbmcgaW4gdGhl IGtlcm5lbC4NCj4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPiA+IFRoYXQncyBub3QgdGhl IGNhc2UgaGVyZS4gIFRoaXMgaXMgYSBiYWNrZG9vciB0byBnZXQgdGhlDQo+ID4gPiA+ID4gPiA+ ID4gPiBpb21tdSBkb21haW4gZnJvbSB0aGUgaW9tbXUgZHJpdmVyIHJlZ2FyZGxlc3Mgb2Ygd2hv IGlzDQo+ID4gPiA+ID4gPiA+ID4gPiB1c2luZw0KPiA+ID4gaXQgb3IgaG93Lg0KPiA+ID4gPiA+ ID4gPiA+ID4gVGhlIGlvbW11IGRvbWFpbiBpcyBjcmVhdGVkIGFuZCBtYW5hZ2VkIGJ5IHZmaW8s IHNvDQo+ID4gPiA+ID4gPiA+ID4gPiBzaG91bGRuJ3Qgd2UgYmUgbG9va2luZyBhdCBob3cgdG8g ZG8gdGhpcyB0aHJvdWdoIHZmaW8/DQo+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPiBM ZXQgbWUgZmlyc3QgZGVzY3JpYmUgd2hhdCB3ZSBhcmUgZG9pbmcgaGVyZToNCj4gPiA+ID4gPiA+ ID4gPiBEdXJpbmcgaW5pdGlhbGl6YXRpb246LQ0KPiA+ID4gPiA+ID4gPiA+ICAtIHZmaW8gdGFs a3MgdG8gTVNJIHN5c3RlbSB0byBrbm93IHRoZSBNU0ktcGFnZSBhbmQgc2l6ZQ0KPiA+ID4gPiA+ ID4gPiA+ICAtIHZmaW8gdGhlbiBpbnRlcmFjdHMgd2l0aCBpb21tdSB0byBtYXAgdGhlIE1TSS1w YWdlIGluDQo+ID4gPiA+ID4gPiA+ID4gaW9tbXUgKElPVkEgaXMgZGVjaWRlZCBieSB1c2Vyc3Bh Y2UgYW5kIHBoeXNpY2FsIGFkZHJlc3MNCj4gPiA+ID4gPiA+ID4gPiBpcyB0aGUNCj4gPiA+ID4g PiA+ID4gPiBNU0ktcGFnZSkNCj4gPiA+ID4gPiA+ID4gPiAgLSBTbyB0aGUgSU9WQSBzdWJ3aW5k b3cgbWFwcGluZyBpcyBjcmVhdGVkIGluIGlvbW11IGFuZA0KPiA+ID4gPiA+ID4gPiA+IHllcyBW RklPIGtub3cgYWJvdXQNCj4gPiA+ID4gPiA+ID4gdGhpcyBtYXBwaW5nLg0KPiA+ID4gPiA+ID4g PiA+DQo+ID4gPiA+ID4gPiA+ID4gTm93IGRvIFNFVF9JUlEoTVNJL01TSVgpIGlvY3RsOg0KPiA+ ID4gPiA+ID4gPiA+ICAtIGNhbGxzIHBjaV9lbmFibGVfbXNpeCgpL3BjaV9lbmFibGVfbXNpX2Js b2NrKCk6IHdoaWNoDQo+ID4gPiA+ID4gPiA+ID4gaXMgc3VwcG9zZWQgdG8gc2V0DQo+ID4gPiA+ ID4gPiA+IE1TSSBhZGRyZXNzL2RhdGEgaW4gZGV2aWNlLg0KPiA+ID4gPiA+ID4gPiA+ICAtIFNv IGluIGN1cnJlbnQgaW1wbGVtZW50YXRpb24gKHRoaXMgcGF0Y2hzZXQpDQo+ID4gPiA+ID4gPiA+ ID4gbXNpLXN1YnN5c3RlbSBnZXRzIHRoZSBJT1ZBDQo+ID4gPiA+ID4gPiA+IGZyb20gaW9tbXUg dmlhIHRoaXMgZGVmaW5lZCBpbnRlcmZhY2UuDQo+ID4gPiA+ID4gPiA+ID4gIC0gQXJlIHlvdSBz YXlpbmcgdGhhdCByYXRoZXIgdGhhbiBnZXR0aW5nIHRoaXMgZnJvbQ0KPiA+ID4gPiA+ID4gPiA+ IGlvbW11LCB3ZSBzaG91bGQgZ2V0IHRoaXMNCj4gPiA+ID4gPiA+ID4gZnJvbSB2ZmlvPyBXaGF0 IGRpZmZlcmVuY2UgZG9lcyB0aGlzIG1ha2U/DQo+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+ IFllcywgeW91IGp1c3Qgc2FpZCBhYm92ZSB0aGF0IHZmaW8ga25vd3MgdGhlIG1zaSB0byBpb3Zh DQo+ID4gPiA+ID4gPiA+IG1hcHBpbmcsIHNvIHdoeSBnbyBvdXRzaWRlIG9mIHZmaW8gdG8gZmlu ZCBpdCBsYXRlcj8gIFRoZQ0KPiA+ID4gPiA+ID4gPiBkaWZmZXJlbmNlIGlzIG9uZSBjYXNlIHlv dSBjYW4gaGF2ZSBhIHByb3BlciByZWZlcmVuY2UgdG8NCj4gPiA+ID4gPiA+ID4gZGF0YSBzdHJ1 Y3R1cmVzIHRvIG1ha2Ugc3VyZSB0aGUgcG9pbnRlciB5b3UgZ2V0IGJhY2sNCj4gPiA+ID4gPiA+ ID4gYWN0dWFsbHkgaGFzIG1lYW5pbmcgYXQgdGhlIHRpbWUgeW91J3JlIHVzaW5nIGl0IHZzIHRo ZSBjb2RlDQo+ID4gPiA+ID4gPiA+IGhlcmUgd2hlcmUgeW91J3JlIGRlZmluaW5nIGFuIEFQSSB0 aGF0IHJldHVybnMgYSBtZWFuaW5nbGVzcw0KPiA+ID4gPiA+ID4gPiB2YWx1ZQ0KPiA+ID4gPiA+ ID4NCj4gPiA+ID4gPiA+IFdpdGggRlNMLVBBTVUgd2Ugd2lsbCBhbHdheXMgZ2V0IGNvbnNpc3Rh bnQgZGF0YSBmcm9tIGlvbW11IG9yDQo+ID4gPiA+ID4gPiB2ZmlvLWRhdGENCj4gPiA+ID4gPiBz dHJ1Y3R1cmUuDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBHcmVhdCwgYnV0IHlvdSdyZSB0cnlpbmcg dG8gYWRkIGEgZ2VuZXJpYyBBUEkgdG8gdGhlIElPTU1VDQo+ID4gPiA+ID4gc3Vic3lzdGVtIHRo YXQncyBkaWZmaWN1bHQgdG8gdXNlIGNvcnJlY3RseS4gIFRoZSBmYWN0IHRoYXQgeW91DQo+ID4g PiA+ID4gdXNlIGl0IGNvcnJlY3RseSBkb2VzIG5vdCBqdXN0aWZ5IHRoZSBBUEkuDQo+ID4gPiA+ ID4NCj4gPiA+ID4gPiA+ID4gYmVjYXVzZSB5b3UgY2FuJ3QgY2hlY2sgb3INCj4gPiA+ID4gPiA+ ID4gZW5mb3JjZSB0aGF0IGFuIGFyYml0cmFyeSBjYWxsZXIgaXMgdXNpbmcgaXQgY29ycmVjdGx5 Lg0KPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+IEkgYW0gbm90IHN1cmUgd2hhdCBpcyBhcmJpdHJh cnkgY2FsbGVyPyBwZGV2IGlzIGtub3duIHRvIHZmaW8sDQo+ID4gPiA+ID4gPiBzbyB2ZmlvIHdp bGwgb25seSBtYWtlDQo+ID4gPiA+ID4gPiBwY2lfZW5hYmxlX21zaXgoKS9wY2lfZW5hYmxlX21z aV9ibG9jaygpIGZvcg0KPiA+ID4gdGhpcyBwZGV2Lg0KPiA+ID4gPiA+ID4gSWYgYW55b3RoZXIg Y29kZSBtYWtlcyB0aGVuIGl0IGlzIHNvbWUgb3RoZXIgdW5leHBlY3RlZGx5DQo+ID4gPiA+ID4g PiB0aGluZyBoYXBwZW5pbmcgaW4gc3lzdGVtLCBubz8NCj4gPiA+ID4gPg0KPiA+ID4gPiA+IFdo YXQncyBwcm9wb3NlZCBoZXJlIGlzIGEgZ2VuZXJpYyBJT01NVSBBUEkuICBBbnlib2R5IGNhbiBj YWxsDQo+IHRoaXMuDQo+ID4gPiA+ID4gV2hhdCBpZiB0aGUgaG9zdCBTQ1NJIGRyaXZlciBkZWNp ZGVzIHRvIGdvIGdldCB0aGUgaW9tbXUgZG9tYWluDQo+ID4gPiA+ID4gZm9yIGl0J3MgZGV2aWNl IChvciBhbnkgb3RoZXIgZGV2aWNlKT8gIERvZXMgdGhhdCBmaXQgeW91ciB1c2FnZQ0KPiBtb2Rl bD8NCj4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiAgSXQncyBub3QgbWFpbnRhaW5hYmxlLg0KPiA+ ID4gPiA+ID4gPiBUaGFua3MsDQo+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gSSBkbyBub3QgaGF2 ZSBhbnkgaXNzdWUgd2l0aCB0aGlzIGFzIHdlbGwsIGNhbiB5b3UgYWxzbw0KPiA+ID4gPiA+ID4g ZGVzY3JpYmUgdGhlIHR5cGUgb2YgQVBJIHlvdSBhcmUgZW52aXNpb25pbmc7IEkgY2FuIHRoaW5r IG9mDQo+ID4gPiA+ID4gPiBkZWZpbmluZyBzb21lIGZ1bmN0aW9uIGluIHZmaW8uYy92ZmlvX2lv bW11Ki5jLCBtYWtlIHRoZW0NCj4gPiA+ID4gPiA+IGdsb2JhbCBhbmQgZGVjbGFyZSB0aGVuIGlu IGluY2x1ZGUvTGludXgvdmZpby5oIEFuZCBpbmNsdWRlDQo+ID4gPiA+ID4gPiA8TGludXgvdmZp by5oPiBpbiBjYWxsZXIgZmlsZQ0KPiA+ID4gPiA+ID4gKGFyY2gvcG93ZXJwYy9rZXJuZWwvbXNp LmMpDQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBEbyB5b3UgcmVhbGx5IHdhbnQgbW9kdWxlIGRlcGVu ZGVuY2llcyBiZXR3ZWVuIHZmaW8gYW5kIHlvdXINCj4gPiA+ID4gPiBjb3JlIGtlcm5lbCBNU0kg c2V0dXA/ICBMb29rIGF0IHRoZSB2ZmlvIGV4dGVybmFsIHVzZXIgaW50ZXJmYWNlDQo+ID4gPiA+ ID4gdGhhdA0KPiA+ID4gd2UndmUgYWxyZWFkeSBkZWZpbmVkLg0KPiA+ID4gPiA+IFRoYXQgYWxs b3dzIG90aGVyIGNvbXBvbmVudHMgb2YgdGhlIGtlcm5lbCB0byBnZXQgYSBwcm9wZXINCj4gPiA+ ID4gPiByZWZlcmVuY2UgdG8gYSB2ZmlvIGdyb3VwLiAgRnJvbSB0aGVyZSB5b3UgY2FuIHdvcmsg b3V0IGhvdyB0bw0KPiA+ID4gPiA+IGdldCB3aGF0IHlvdSB3YW50LiAgQW5vdGhlciBhbHRlcm5h dGl2ZSBpcyB0aGF0IHZmaW8gY291bGQNCj4gPiA+ID4gPiByZWdpc3RlciBhbiBNU0kgdG8gSU9W QSBtYXBwaW5nIHdpdGggYXJjaGl0ZWN0dXJlIGNvZGUgd2hlbiB0aGUNCj4gbWFwcGluZyBpcyBj cmVhdGVkLg0KPiA+ID4gPiA+IFRoZSBNU0kgc2V0dXAgcGF0aCBjb3VsZCB0aGVuIGRvIGEgbG9v a3VwIGluIGFyY2hpdGVjdHVyZSBjb2RlDQo+ID4gPiA+ID4gZm9yIHRoZSBtYXBwaW5nLiAgWW91 IGNvdWxkIGV2ZW4gc3RvcmUgdGhlIE1TSSB0byBJT1ZBIG1hcHBpbmcNCj4gPiA+ID4gPiBpbiBW RklPIGFuZCBjcmVhdGUgYW4gaW50ZXJmYWNlIHdoZXJlIFNFVF9JUlEgcGFzc2VzIHRoYXQNCj4g PiA+ID4gPiBtYXBwaW5nIGludG8gc2V0dXANCj4gPiA+IGNvZGUuDQo+ID4gW1NldGhpIFZhcnVu LUIxNjM5NV0gV2hhdCB5b3UgYXJlIHN1Z2dlc3RpbmcgaXMgdGhhdCB0aGUgTVNJIHNldHVwDQo+ ID4gcGF0aCBxdWVyaWVzIHRoZSB2ZmlvIHN1YnN5c3RlbSBmb3IgdGhlIG1hcHBpbmcsIHJhdGhl ciB0aGFuIGRpcmVjdGx5DQo+ID4gcXVlcnlpbmcgdGhlIGlvbW11IHN1YnN5c3RlbS4gU28sIHNh eSBpZiB3ZSBhZGQgYW4gaW50ZXJmYWNlIGZvcg0KPiA+IGdldHRpbmcgTVNJIHRvIElPVkEgbWFw cGluZyBpbiB0aGUgbXNpIHNldHVwIHBhdGgsIHdvdWxkbid0IHRoaXMgYWdhaW4NCj4gPiBiZSBz cGVjaWZpYyB0byB2ZmlvPyBJIHJlY2tvbiB0aGF0IHRoaXMgaW50ZXJmYWNlIHdvdWxkIGFnYWlu IHBwYw0KPiA+IG1hY2hpbmUgc3BlY2lmaWMgaW50ZXJmYWNlLg0KPiANCj4gU3VyZSwgaWYgdGhp cyBpcyBhIGdlbmVyaWMgTVNJIHNldHVwIHBhdGggdGhlbiBjbGVhcmx5IHZmaW8gc2hvdWxkIG5v dCBiZQ0KPiBpbnZvbHZlZC4gIEJ1dCBieSB0aGF0IHNhbWUgbm90aW9uLCBpZiB0aGlzIGlzIGEg Z2VuZXJpYyBNU0kgc2V0dXAgcGF0aCwNCj4gaG93IGNhbiB0aGUgcHJvcG9zZWQgc29sdXRpb25z IGd1YXJhbnRlZSB0aGF0IHRoZSBpb21tdV9kb21haW4gb3IgaW92YQ0KPiByZXR1cm5lZCBpcyBz dGlsbCB2YWxpZCBpbiBhbGwgY2FzZXM/ICBJdCdzIHJhY3kuICBJZiB0aGUgY2FsbGVyIHRyeWlu Zw0KPiB0byBzZXR1cCBNU0kgaGFzIHRoZSBpbmZvcm1hdGlvbiBuZWVkZWQsIHdoeSBkb2Vzbid0 IGl0IHBhc3MgaXQgaW4gYXMNCj4gcGFydCBvZiB0aGUgc2V0dXA/ICBUaGFua3MsDQpbU2V0aGkg VmFydW4tQjE2Mzk1XSBBZ3JlZWQsIHRoZSBwcm9wb3NlZCBpbnRlcmZhY2UgaXMgbm90IGNsZWFu LiBCdXQsIHdlIHN0aWxsIG5lZWQgYW4gaW50ZXJmYWNlIHRocm91Z2ggd2hpY2ggTVNJIGRyaXZl ciBjYW4gY2FsbCBpbiB0byB0aGUgdmZpbyBsYXllci4NCg0KLVZhcnVuDQoNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-10-14 at 12:58 +0000, Sethi Varun-B16395 wrote: > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Friday, October 11, 2013 2:12 AM > > To: Sethi Varun-B16395 > > Cc: Bhushan Bharat-R65777; agraf@suse.de; Wood Scott-B07421; linux- > > pci@vger.kernel.org; galak@kernel.crashing.org; linux- > > kernel@vger.kernel.org; iommu@lists.linux-foundation.org; > > benh@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device > > > > On Thu, 2013-10-10 at 20:09 +0000, Sethi Varun-B16395 wrote: > > > > > > > -----Original Message----- > > > > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu- > > > > bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson > > > > Sent: Tuesday, October 08, 2013 8:43 AM > > > > To: Bhushan Bharat-R65777 > > > > Cc: agraf@suse.de; Wood Scott-B07421; linux-pci@vger.kernel.org; > > > > galak@kernel.crashing.org; linux-kernel@vger.kernel.org; > > > > iommu@lists.linux-foundation.org; benh@kernel.crashing.org; > > > > linuxppc- dev@lists.ozlabs.org > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > device > > > > > > > > On Mon, 2013-10-07 at 05:46 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > -----Original Message----- > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > > Sent: Friday, October 04, 2013 11:42 PM > > > > > > To: Bhushan Bharat-R65777 > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > foundation.org > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a > > > > > > device > > > > > > > > > > > > On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > > > > Sent: Friday, October 04, 2013 10:43 PM > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org; > > > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org; > > > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- > > > > > > > > foundation.org > > > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain > > > > > > > > of a device > > > > > > > > > > > > > > > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 > > wrote: > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: Alex Williamson > > > > > > > > > > [mailto:alex.williamson@redhat.com] > > > > > > > > > > Sent: Friday, October 04, 2013 9:15 PM > > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > > > > galak@kernel.crashing.org; linux- > > > > > > > > > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; > > > > > > > > > > linux- pci@vger.kernel.org; agraf@suse.de; Wood > > > > > > > > > > Scott-B07421; iommu@lists.linux- foundation.org > > > > > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get > > > > > > > > > > iommu_domain of a device > > > > > > > > > > > > > > > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > > > From: linux-pci-owner@vger.kernel.org > > > > > > > > > > > > [mailto:linux-pci-owner@vger.kernel.org] > > > > > > > > > > > > On Behalf Of Alex Williamson > > > > > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org; > > > > > > > > > > > > galak@kernel.crashing.org; linux- > > > > > > > > > > > > kernel@vger.kernel.org; > > > > > > > > > > > > linuxppc-dev@lists.ozlabs.org; > > > > > > > > > > > > linux- pci@vger.kernel.org; agraf@suse.de; Wood > > > > > > > > > > > > Scott-B07421; iommu@lists.linux- 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@freescale.com> > > > > > > > > > > > > > --- > > > > > > > > > > > > > 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? > > > > > > > > > > > > > > > > > > > > No, it's not sufficient to make sure that your use of > > > > > > > > > > the interface is race free. The interface itself needs > > > > > > > > > > to be designed so that it's difficult to use incorrectly. > > > > > > > > > > > > > > > > > > So we can define > > > > > > > > > iommu_get_dev_domain()/iommu_put_dev_domain(); > > > > > > > > > iommu_get_dev_domain() will return domain with the lock > > > > > > > > > held, and > > > > > > > > > iommu_put_dev_domain() will release the lock? And > > > > > > > > > iommu_get_dev_domain() must always be followed by > > > > > > > > > iommu_get_dev_domain(). > > > > > > > > > > > > > > > > What lock? get/put are generally used for reference > > > > > > > > counting, not locking in the kernel. > > > > > > > > > > > > > > > > > > That's not the case here. This is a backdoor to get the > > > > > > > > > > iommu domain from the iommu driver regardless of who is > > > > > > > > > > using > > > > it or how. > > > > > > > > > > The iommu domain is created and managed by vfio, so > > > > > > > > > > shouldn't we be looking at how to do this through vfio? > > > > > > > > > > > > > > > > > > Let me first describe what we are doing here: > > > > > > > > > During initialization:- > > > > > > > > > - vfio talks to MSI system to know the MSI-page and size > > > > > > > > > - vfio then interacts with iommu to map the MSI-page in > > > > > > > > > iommu (IOVA is decided by userspace and physical address > > > > > > > > > is the > > > > > > > > > MSI-page) > > > > > > > > > - So the IOVA subwindow mapping is created in iommu and > > > > > > > > > yes VFIO know about > > > > > > > > this mapping. > > > > > > > > > > > > > > > > > > Now do SET_IRQ(MSI/MSIX) ioctl: > > > > > > > > > - calls pci_enable_msix()/pci_enable_msi_block(): which > > > > > > > > > is supposed to set > > > > > > > > MSI address/data in device. > > > > > > > > > - So in current implementation (this patchset) > > > > > > > > > msi-subsystem gets the IOVA > > > > > > > > from iommu via this defined interface. > > > > > > > > > - Are you saying that rather than getting this from > > > > > > > > > iommu, we should get this > > > > > > > > from vfio? What difference does this make? > > > > > > > > > > > > > > > > Yes, you just said above that vfio knows the msi to iova > > > > > > > > mapping, so why go outside of vfio to find it later? The > > > > > > > > difference is one case you can have a proper reference to > > > > > > > > data structures to make sure the pointer you get back > > > > > > > > actually has meaning at the time you're using it vs the code > > > > > > > > here where you're defining an API that returns a meaningless > > > > > > > > value > > > > > > > > > > > > > > With FSL-PAMU we will always get consistant data from iommu or > > > > > > > vfio-data > > > > > > structure. > > > > > > > > > > > > Great, but you're trying to add a generic API to the IOMMU > > > > > > subsystem that's difficult to use correctly. The fact that you > > > > > > use it correctly does not justify the API. > > > > > > > > > > > > > > because you can't check or > > > > > > > > enforce that an arbitrary caller is using it correctly. > > > > > > > > > > > > > > I am not sure what is arbitrary caller? pdev is known to vfio, > > > > > > > so vfio will only make > > > > > > > pci_enable_msix()/pci_enable_msi_block() for > > > > this pdev. > > > > > > > If anyother code makes then it is some other unexpectedly > > > > > > > thing happening in system, no? > > > > > > > > > > > > What's proposed here is a generic IOMMU API. Anybody can call > > this. > > > > > > What if the host SCSI driver decides to go get the iommu domain > > > > > > for it's device (or any other device)? Does that fit your usage > > model? > > > > > > > > > > > > > > It's not maintainable. > > > > > > > > Thanks, > > > > > > > > > > > > > > I do not have any issue with this as well, can you also > > > > > > > describe the type of API you are envisioning; I can think of > > > > > > > defining some function in vfio.c/vfio_iommu*.c, make them > > > > > > > global and declare then in include/Linux/vfio.h And include > > > > > > > <Linux/vfio.h> in caller file > > > > > > > (arch/powerpc/kernel/msi.c) > > > > > > > > > > > > Do you really want module dependencies between vfio and your > > > > > > core kernel MSI setup? Look at the vfio external user interface > > > > > > that > > > > we've already defined. > > > > > > That allows other components of the kernel to get a proper > > > > > > reference to a vfio group. From there you can work out how to > > > > > > get what you want. Another alternative is that vfio could > > > > > > register an MSI to IOVA mapping with architecture code when the > > mapping is created. > > > > > > The MSI setup path could then do a lookup in architecture code > > > > > > for the mapping. You could even store the MSI to IOVA mapping > > > > > > in VFIO and create an interface where SET_IRQ passes that > > > > > > mapping into setup > > > > code. > > > [Sethi Varun-B16395] What you are suggesting is that the MSI setup > > > path queries the vfio subsystem for the mapping, rather than directly > > > querying the iommu subsystem. So, say if we add an interface for > > > getting MSI to IOVA mapping in the msi setup path, wouldn't this again > > > be specific to vfio? I reckon that this interface would again ppc > > > machine specific interface. > > > > Sure, if this is a generic MSI setup path then clearly vfio should not be > > involved. But by that same notion, if this is a generic MSI setup path, > > how can the proposed solutions guarantee that the iommu_domain or iova > > returned is still valid in all cases? It's racy. If the caller trying > > to setup MSI has the information needed, why doesn't it pass it in as > > part of the setup? Thanks, > [Sethi Varun-B16395] Agreed, the proposed interface is not clean. But, we still need an interface through which MSI driver can call in to the vfio layer. Or one that allows vfio to pass in the iova when the interrupt is being setup. I'm afraid any kind of reverse interface where MSI calls back into vfio is going to be racy. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); /* * 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;
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@freescale.com> --- drivers/iommu/iommu.c | 10 ++++++++++ include/linux/iommu.h | 7 +++++++ 2 files changed, 17 insertions(+), 0 deletions(-)