Re: [PATCH 1/3] iommu/fsl: Factor out PCI specific code.
On Sun, Oct 13, 2013 at 02:02:32AM +0530, Varun Sethi wrote:
> Factor out PCI specific code in the PAMU driver.
>
> Signed-off-by: Varun Sethi
> ---
> drivers/iommu/fsl_pamu_domain.c | 81
> +++
> 1 file changed, 40 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
> index c857c30..e02e1de 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -677,13 +677,9 @@ static int handle_attach_device(struct fsl_dma_domain
> *dma_domain,
> return ret;
> }
>
> -static int fsl_pamu_attach_device(struct iommu_domain *domain,
> - struct device *dev)
> +static void check_for_pci_dma_device(struct device **dev)
"check_for_pci_dma_device()" doesn't give a good clue about what the
function returns. And why return something via a reference parameter
when you could return it directly?
> {
> - struct fsl_dma_domain *dma_domain = domain->priv;
> - const u32 *liodn;
> - u32 liodn_cnt;
> - int len, ret = 0;
> +#ifdef CONFIG_PCI
> struct pci_dev *pdev = NULL;
> struct pci_controller *pci_ctl;
This is sort of a goofy looking function. It would read much better as
something like this:
struct device *dma_dev = dev;
#ifdef CONFIG_PCI
if (...) {
dma_dev = ...;
}
#endif
return dma_dev;
Does this need to care about reference counting when you return a pointer
to a different device?
Bjorn
>
> @@ -691,25 +687,38 @@ static int fsl_pamu_attach_device(struct iommu_domain
> *domain,
>* Use LIODN of the PCI controller while attaching a
>* PCI device.
>*/
> - if (dev->bus == &pci_bus_type) {
> - pdev = to_pci_dev(dev);
> + if ((*dev)->bus == &pci_bus_type) {
> + pdev = to_pci_dev(*dev);
> pci_ctl = pci_bus_to_host(pdev->bus);
> /*
>* make dev point to pci controller device
>* so we can get the LIODN programmed by
>* u-boot.
>*/
> - dev = pci_ctl->parent;
> + *dev = pci_ctl->parent;
> }
> +#endif
> +}
>
> - liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
> +static int fsl_pamu_attach_device(struct iommu_domain *domain,
> + struct device *dev)
> +{
> + struct fsl_dma_domain *dma_domain = domain->priv;
> + struct device *dma_dev = dev;
> + const u32 *liodn;
> + u32 liodn_cnt;
> + int len, ret = 0;
> +
> + check_for_pci_dma_device(&dma_dev);
> +
> + liodn = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
> if (liodn) {
> liodn_cnt = len / sizeof(u32);
> ret = handle_attach_device(dma_domain, dev,
>liodn, liodn_cnt);
> } else {
> pr_debug("missing fsl,liodn property at %s\n",
> - dev->of_node->full_name);
> + dma_dev->of_node->full_name);
> ret = -EINVAL;
> }
>
> @@ -720,32 +729,18 @@ static void fsl_pamu_detach_device(struct iommu_domain
> *domain,
> struct device *dev)
> {
> struct fsl_dma_domain *dma_domain = domain->priv;
> + struct device *dma_dev = dev;
> const u32 *prop;
> int len;
> - struct pci_dev *pdev = NULL;
> - struct pci_controller *pci_ctl;
>
> - /*
> - * Use LIODN of the PCI controller while detaching a
> - * PCI device.
> - */
> - if (dev->bus == &pci_bus_type) {
> - pdev = to_pci_dev(dev);
> - pci_ctl = pci_bus_to_host(pdev->bus);
> - /*
> - * make dev point to pci controller device
> - * so we can get the LIODN programmed by
> - * u-boot.
> - */
> - dev = pci_ctl->parent;
> - }
> + check_for_pci_dma_device(&dma_dev);
>
> - prop = of_get_property(dev->of_node, "fsl,liodn", &len);
> + prop = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
> if (prop)
> detach_device(dev, dma_domain);
> else
> pr_debug("missing fsl,liodn property at %s\n",
> - dev->of_node->full_name);
> + dma_dev->of_node->full_name);
> }
>
> static int configure_domain_geometry(struct iommu_domain *domain, void
> *data)
> @@ -905,6 +900,7 @@ static struct iommu_group *get_device_iommu_group(struct
> device *dev)
> return group;
> }
>
> +#ifdef CONFIG_PCI
> static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl)
> {
> u32 version;
> @@ -945,13 +941,18 @@ static struct iommu_group
> *get_shared_pci_device_group(struct pci_dev *pdev)
> return NULL;
> }
>
> -static struct iommu_group *get_pci_device_group(struct pci_dev *pdev)
> +static struct iomm
Re: [PATCH 2/2] ARM: SMMU: return NULL on error in arm_smmu_iova_to_phys
On Mon, Oct 14, 2013 at 04:17:51PM +0100, Antonios Motakis wrote:
> On Mon, Oct 14, 2013 at 2:48 PM, Will Deacon wrote:
> > On Fri, Oct 11, 2013 at 02:24:47PM +0100, Antonios Motakis wrote:
> >> The return value of arm_smmu_iova_to_phys is directly passed to the
> >> user of the IOMMU API via iommu_iova_to_phys; however the ARM SMMU
> >> driver returns -EINVAL on error, which is not consistent with the
> >> rest of the drivers implementing the IOMMU API. VFIO also relies on
> >> the call returning NULL when a page has not been mapped already.
> >>
> >> Signed-off-by: Antonios Motakis
> >> ---
> >> drivers/iommu/arm-smmu.c | 5 +
> >> 1 file changed, 1 insertion(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >> index 8b71332..fe81b20 100644
> >> --- a/drivers/iommu/arm-smmu.c
> >> +++ b/drivers/iommu/arm-smmu.c
> >> @@ -1480,10 +1480,7 @@ static phys_addr_t arm_smmu_iova_to_phys(struct
> >> iommu_domain *domain,
> >>
> >> err_unlock:
> >> spin_unlock(&smmu_domain->lock);
> >> - dev_warn(smmu->dev,
> >> - "invalid (corrupt?) page tables detected for iova 0x%llx\n",
> >> - (unsigned long long)iova);
> >> - return -EINVAL;
> >> + return NULL;
> >
> > Why are you removing the warning message?
>
> VFIO will exercise this code path every time when mapping DMA memory.
> This is normal and VFIO *expects* the function to fail - it is only if
> the function succeeds that VFIO needs to back down from the DMA
> mapping and fail.
>
> This means that there would be a warning every time a VFIO user maps
> some memory for DMA use, even though nothing went wrong.
Ok, in which case it might be worth reworking arm_smmu_iova_to_phys to treat
{pgd,pud,pmd,pte}_none different from {pgd,pud,pmd,pte}_bad.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] ARM: SMMU: add devices attached to the SMMU to an IOMMU group
On Mon, Oct 14, 2013 at 04:13:15PM +0100, Antonios Motakis wrote:
> On Mon, Oct 14, 2013 at 2:48 PM, Will Deacon wrote:
> > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > > index 0f45a48..8b71332 100644
> > > --- a/drivers/iommu/arm-smmu.c
> > > +++ b/drivers/iommu/arm-smmu.c
> > > @@ -1502,6 +1502,8 @@ static int arm_smmu_add_device(struct device *dev)
> > > {
> > > struct arm_smmu_device *child, *parent, *smmu;
> > > struct arm_smmu_master *master = NULL;
> > > + struct iommu_group *group;
> > > + int ret;
> > >
> > > spin_lock(&arm_smmu_devices_lock);
> > > list_for_each_entry(parent, &arm_smmu_devices, list) {
> > > @@ -1534,13 +1536,27 @@ static int arm_smmu_add_device(struct device *dev)
> > > if (!master)
> > > return -ENODEV;
> > >
> > > + group = iommu_group_get(dev);
> >
> > I'm not especially familiar with IOMMU groups (I understand them as the
> > minimum translation granularity, which would mean single StreamID for the
> > ARM SMMU), but under what circumstances would you expect to receive a
> > non-NULL group here? I can't see any other code adding devices to groups
> > (outside of other drivers)...
> >
>
> You are right, only other IOMMU drivers will add a device to a group.
> There was a discussion about this when I posted a similar patch for
> the Exynos System MMU driver, see
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/185675.html
>
> The idea is to check in the case of add_device() being called multiple
> times, which is not the case most of the time, but still a sane
> safeguard.
Ok, but it feels a bit weird. The current code (arm_smmu_add_device)
basically does a bunch of sanity checking against the DT data in order to
find where the master sits in the device topology. Then it updates
dev->archdata.iommu to point at the relevant SMMU instance.
So, the interesting case is where the device was previously associated with
a *different* IOMMU. In that case, the current code clobbers the iommu field
with the new smmu, whereas the new code could end up getting very confused
with respect to IOMMU groups.
A better way is probably to check that dev->archdata.iommu is NULL before we
assign to it. If not, then spit out a warning and return an error. That
would also mean you could get rid of the group get/put calls.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] ARM: SMMU: return NULL on error in arm_smmu_iova_to_phys
On Mon, Oct 14, 2013 at 2:48 PM, Will Deacon wrote: > On Fri, Oct 11, 2013 at 02:24:47PM +0100, Antonios Motakis wrote: >> The return value of arm_smmu_iova_to_phys is directly passed to the >> user of the IOMMU API via iommu_iova_to_phys; however the ARM SMMU >> driver returns -EINVAL on error, which is not consistent with the >> rest of the drivers implementing the IOMMU API. VFIO also relies on >> the call returning NULL when a page has not been mapped already. >> >> Signed-off-by: Antonios Motakis >> --- >> drivers/iommu/arm-smmu.c | 5 + >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index 8b71332..fe81b20 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -1480,10 +1480,7 @@ static phys_addr_t arm_smmu_iova_to_phys(struct >> iommu_domain *domain, >> >> err_unlock: >> spin_unlock(&smmu_domain->lock); >> - dev_warn(smmu->dev, >> - "invalid (corrupt?) page tables detected for iova 0x%llx\n", >> - (unsigned long long)iova); >> - return -EINVAL; >> + return NULL; > > Why are you removing the warning message? VFIO will exercise this code path every time when mapping DMA memory. This is normal and VFIO *expects* the function to fail - it is only if the function succeeds that VFIO needs to back down from the DMA mapping and fail. This means that there would be a warning every time a VFIO user maps some memory for DMA use, even though nothing went wrong. > > Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] ARM: SMMU: add devices attached to the SMMU to an IOMMU group
On Mon, Oct 14, 2013 at 2:48 PM, Will Deacon wrote:
>
> Hi Antonios,
>
> On Fri, Oct 11, 2013 at 02:24:46PM +0100, Antonios Motakis wrote:
> > IOMMU groups are expected by certain users of the IOMMU API,
> > e.g. VFIO. Add new devices found by the SMMU driver to an IOMMU
> > group to satisfy those users.
>
> Cheers for looking at this: VFIO has been on my TODO list for some time.
>
> > Signed-off-by: Antonios Motakis
> > ---
> > drivers/iommu/arm-smmu.c | 18 +-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index 0f45a48..8b71332 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -1502,6 +1502,8 @@ static int arm_smmu_add_device(struct device *dev)
> > {
> > struct arm_smmu_device *child, *parent, *smmu;
> > struct arm_smmu_master *master = NULL;
> > + struct iommu_group *group;
> > + int ret;
> >
> > spin_lock(&arm_smmu_devices_lock);
> > list_for_each_entry(parent, &arm_smmu_devices, list) {
> > @@ -1534,13 +1536,27 @@ static int arm_smmu_add_device(struct device *dev)
> > if (!master)
> > return -ENODEV;
> >
> > + group = iommu_group_get(dev);
>
> I'm not especially familiar with IOMMU groups (I understand them as the
> minimum translation granularity, which would mean single StreamID for the
> ARM SMMU), but under what circumstances would you expect to receive a
> non-NULL group here? I can't see any other code adding devices to groups
> (outside of other drivers)...
>
Hi Will,
You are right, only other IOMMU drivers will add a device to a group.
There was a discussion about this when I posted a similar patch for
the Exynos System MMU driver, see
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/185675.html
The idea is to check in the case of add_device() being called multiple
times, which is not the case most of the time, but still a sane
safeguard.
>
> Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
On Mon, 2013-10-14 at 12:58 +, Sethi Varun-B16395 wrote: > > > -Original Message- > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > Sent: Friday, October 11, 2013 2:12 AM > > To: Sethi Varun-B16395 > > Cc: Bhushan Bharat-R65777; ag...@suse.de; Wood Scott-B07421; linux- > > p...@vger.kernel.org; ga...@kernel.crashing.org; linux- > > ker...@vger.kernel.org; iommu@lists.linux-foundation.org; > > b...@kernel.crashing.org; linuxppc-...@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 +, Sethi Varun-B16395 wrote: > > > > > > > -Original Message- > > > > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > > > > boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson > > > > Sent: Tuesday, October 08, 2013 8:43 AM > > > > To: Bhushan Bharat-R65777 > > > > Cc: ag...@suse.de; Wood Scott-B07421; linux-...@vger.kernel.org; > > > > ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; > > > > iommu@lists.linux-foundation.org; b...@kernel.crashing.org; > > > > linuxppc- d...@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 +, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > -Original Message- > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > Sent: Friday, October 04, 2013 11:42 PM > > > > > > To: Bhushan Bharat-R65777 > > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > > ga...@kernel.crashing.org; linux- ker...@vger.kernel.org; > > > > > > linuxppc-...@lists.ozlabs.org; linux- p...@vger.kernel.org; > > > > > > ag...@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 +, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > > > > > -Original Message- > > > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > > > Sent: Friday, October 04, 2013 10:43 PM > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > > > > ga...@kernel.crashing.org; linux- ker...@vger.kernel.org; > > > > > > > > linuxppc-...@lists.ozlabs.org; linux- p...@vger.kernel.org; > > > > > > > > ag...@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 +, Bhushan Bharat-R65777 > > wrote: > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > > > From: Alex Williamson > > > > > > > > > > [mailto:alex.william...@redhat.com] > > > > > > > > > > Sent: Friday, October 04, 2013 9:15 PM > > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > > > > > > ga...@kernel.crashing.org; linux- > > > > > > > > > > ker...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; > > > > > > > > > > linux- p...@vger.kernel.org; ag...@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 +, Bhushan Bharat-R65777 > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > > > > > From: linux-pci-ow...@vger.kernel.org > > > > > > > > > > > > [mailto:linux-pci-ow...@vger.kernel.org] > > > > > > > > > > > > On Behalf Of Alex Williamson > > > > > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > > > > > > > > ga...@kernel.crashing.org; linux- > > > > > > > > > > > > ker...@vger.kernel.org; > > > > > > > > > > > > linuxppc-...@lists.ozlabs.org; > > > > > > > > > > > > linux- p...@vger.kernel.org; ag...@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. > > > > > > > > > > > > > > > > > >
RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device
> -Original Message- > From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Friday, October 11, 2013 2:12 AM > To: Sethi Varun-B16395 > Cc: Bhushan Bharat-R65777; ag...@suse.de; Wood Scott-B07421; linux- > p...@vger.kernel.org; ga...@kernel.crashing.org; linux- > ker...@vger.kernel.org; iommu@lists.linux-foundation.org; > b...@kernel.crashing.org; linuxppc-...@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 +, Sethi Varun-B16395 wrote: > > > > > -Original Message- > > > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > > > boun...@lists.linux-foundation.org] On Behalf Of Alex Williamson > > > Sent: Tuesday, October 08, 2013 8:43 AM > > > To: Bhushan Bharat-R65777 > > > Cc: ag...@suse.de; Wood Scott-B07421; linux-...@vger.kernel.org; > > > ga...@kernel.crashing.org; linux-ker...@vger.kernel.org; > > > iommu@lists.linux-foundation.org; b...@kernel.crashing.org; > > > linuxppc- d...@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 +, Bhushan Bharat-R65777 wrote: > > > > > > > > > -Original Message- > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > Sent: Friday, October 04, 2013 11:42 PM > > > > > To: Bhushan Bharat-R65777 > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > ga...@kernel.crashing.org; linux- ker...@vger.kernel.org; > > > > > linuxppc-...@lists.ozlabs.org; linux- p...@vger.kernel.org; > > > > > ag...@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 +, Bhushan Bharat-R65777 wrote: > > > > > > > > > > > > > -Original Message- > > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > > Sent: Friday, October 04, 2013 10:43 PM > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > > > ga...@kernel.crashing.org; linux- ker...@vger.kernel.org; > > > > > > > linuxppc-...@lists.ozlabs.org; linux- p...@vger.kernel.org; > > > > > > > ag...@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 +, Bhushan Bharat-R65777 > wrote: > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > > From: Alex Williamson > > > > > > > > > [mailto:alex.william...@redhat.com] > > > > > > > > > Sent: Friday, October 04, 2013 9:15 PM > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > > > > > ga...@kernel.crashing.org; linux- > > > > > > > > > ker...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; > > > > > > > > > linux- p...@vger.kernel.org; ag...@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 +, Bhushan Bharat-R65777 > > > wrote: > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > > > > From: linux-pci-ow...@vger.kernel.org > > > > > > > > > > > [mailto:linux-pci-ow...@vger.kernel.org] > > > > > > > > > > > On Behalf Of Alex Williamson > > > > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM > > > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > > > Cc: j...@8bytes.org; b...@kernel.crashing.org; > > > > > > > > > > > ga...@kernel.crashing.org; linux- > > > > > > > > > > > ker...@vger.kernel.org; > > > > > > > > > > > linuxppc-...@lists.ozlabs.org; > > > > > > > > > > > linux- p...@vger.kernel.org; ag...@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 > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > drivers/iommu/iommu.c | 10 ++ > > > > > > > > > > > > include/linux/iommu.h |7 +++ > > > > > > > > > > > > 2 files ch
Re: [PATCH 2/2] ARM: SMMU: return NULL on error in arm_smmu_iova_to_phys
On Fri, Oct 11, 2013 at 02:24:47PM +0100, Antonios Motakis wrote: > The return value of arm_smmu_iova_to_phys is directly passed to the > user of the IOMMU API via iommu_iova_to_phys; however the ARM SMMU > driver returns -EINVAL on error, which is not consistent with the > rest of the drivers implementing the IOMMU API. VFIO also relies on > the call returning NULL when a page has not been mapped already. > > Signed-off-by: Antonios Motakis > --- > drivers/iommu/arm-smmu.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 8b71332..fe81b20 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -1480,10 +1480,7 @@ static phys_addr_t arm_smmu_iova_to_phys(struct > iommu_domain *domain, > > err_unlock: > spin_unlock(&smmu_domain->lock); > - dev_warn(smmu->dev, > - "invalid (corrupt?) page tables detected for iova 0x%llx\n", > - (unsigned long long)iova); > - return -EINVAL; > + return NULL; Why are you removing the warning message? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] ARM: SMMU: add devices attached to the SMMU to an IOMMU group
Hi Antonios,
On Fri, Oct 11, 2013 at 02:24:46PM +0100, Antonios Motakis wrote:
> IOMMU groups are expected by certain users of the IOMMU API,
> e.g. VFIO. Add new devices found by the SMMU driver to an IOMMU
> group to satisfy those users.
Cheers for looking at this: VFIO has been on my TODO list for some time.
> Signed-off-by: Antonios Motakis
> ---
> drivers/iommu/arm-smmu.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 0f45a48..8b71332 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1502,6 +1502,8 @@ static int arm_smmu_add_device(struct device *dev)
> {
> struct arm_smmu_device *child, *parent, *smmu;
> struct arm_smmu_master *master = NULL;
> + struct iommu_group *group;
> + int ret;
>
> spin_lock(&arm_smmu_devices_lock);
> list_for_each_entry(parent, &arm_smmu_devices, list) {
> @@ -1534,13 +1536,27 @@ static int arm_smmu_add_device(struct device *dev)
> if (!master)
> return -ENODEV;
>
> + group = iommu_group_get(dev);
I'm not especially familiar with IOMMU groups (I understand them as the
minimum translation granularity, which would mean single StreamID for the
ARM SMMU), but under what circumstances would you expect to receive a
non-NULL group here? I can't see any other code adding devices to groups
(outside of other drivers)...
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 20/20] iommu/exynos: add devices attached to the System MMU to an IOMMU group
On Thu, 10 Oct 2013 14:54:29 -0600, Alex Williamson wrote:
> On Mon, 2013-10-07 at 10:58 +0900, Cho KyongHo wrote:
> > Patch written by Antonios Motakis :
> >
> > IOMMU groups are expected by certain users of the IOMMU API,
> > e.g. VFIO. Since each device is behind its own System MMU, we
> > can allocate a new IOMMU group for each device.
> >
> > Reviewd-by: Cho KyongHo
> > Signed-off-by: Antonios Motakis
> > ---
> > drivers/iommu/exynos-iommu.c | 28
> > 1 files changed, 28 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index 5025338..24505a0 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> > @@ -1028,6 +1028,32 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct
> > iommu_domain *domain,
> > return phys;
> > }
> >
> > +static int exynos_iommu_add_device(struct device *dev)
> > +{
> > + struct iommu_group *group;
> > + int ret;
> > +
> > + group = iommu_group_get(dev);
>
> Seems reasonable, my only nit would be whether it's really an error to
> get a group back from the above call. If devices are always isolated
> and IOMMU groups are always singleton, it would be an error to find one
> already associated with the device. Right? Thanks,
>
Do you mean that calling iommu_group_add_device() with the group that is
returned by the above iommu_group_get() will return -EEXIST?
I didn't think about that.
> Alex
Thank you.
KyongHo.
>
> > +
> > + if (!group) {
> > + group = iommu_group_alloc();
> > + if (IS_ERR(group)) {
> > + dev_err(dev, "Failed to allocate IOMMU group\n");
> > + return PTR_ERR(group);
> > + }
> > + }
> > +
> > + ret = iommu_group_add_device(group, dev);
> > + iommu_group_put(group);
> > +
> > + return ret;
> > +}
> > +
> > +static void exynos_iommu_remove_device(struct device *dev)
> > +{
> > + iommu_group_remove_device(dev);
> > +}
> > +
> > static struct iommu_ops exynos_iommu_ops = {
> > .domain_init = &exynos_iommu_domain_init,
> > .domain_destroy = &exynos_iommu_domain_destroy,
> > @@ -1036,6 +1062,8 @@ static struct iommu_ops exynos_iommu_ops = {
> > .map = &exynos_iommu_map,
> > .unmap = &exynos_iommu_unmap,
> > .iova_to_phys = &exynos_iommu_iova_to_phys,
> > + .add_device = &exynos_iommu_add_device,
> > + .remove_device = &exynos_iommu_remove_device,
> > .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
> > };
> >
>
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
IO_PAGE_FAULTS when enabling IOMMU in coreboot for ASUS F2A85-M
Dear Linux folks, Rudolf Marek pushed a patch for review to enable the IOMMU in coreboot for the ASUS F2A85-M [1]. With this patch applied, Linux, I think 3.10-rc1, shows IO_PAGE_FAULT messages. $ dmesg […] [0.00] ACPI: IVRS bf144e10 00070 (v02 AMD AMDIOMMU 0001 AMD ) [0.00] ACPI: SSDT bf144e80 0051F (v02AMD ALIB 0001 MSFT 0400) [0.00] ACPI: SSDT bf1453a0 006B2 (v01 AMDPOWERNOW 0001 AMD 0001) [0.00] ACPI: SSDT bf145a52 00045 (v02 CORE COREBOOT 002A CORE 002A) […] [0.465114] [Firmware Bug]: ACPI: no secondary bus range in _CRS […] [0.567330] pci :00:00.0: >[1022:1410] type 00 class 0x06 [0.567364] pci :00:00.2: >[1022:1419] type 00 class 0x080600 [0.567427] pci :00:01.0: >[1002:9993] type 00 class 0x03000 […] [0.597731] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT] [0.597899] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.PIBR._PRT] [0.597933] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.SBR0._PRT] [0.597972] ACPI: PCI Interrupt Routing Table [\_SB_.PCI0.SBR1._PRT] [0.598073] pci:00: >Requesting ACPI _OSC control (0x1d) [0.603808] pci:00: >ACPI _OSC request failed (AE_NOT_FOUND), returned control mask: 0x1d [0.612397] ACPI _OSC control for PCIe not granted, disabling ASPM [0.620508] Freeing initrd memory: 14876k freed […] [0.882674] pci :00:01.0: >Boot video device [0.882876] PCI: CLS 64 bytes, default 64 [0.897088] AMD-Vi: Enabling IOMMU at :00:00.2 cap 0x40 extended features: PreF PPR GT IA [0.905816] pci :00:00.2: >irq 40 for MSI/MSI-X [0.917457] AMD-Vi: Lazy IO/TLB flushing enabled [0.922076] PCI-DMA: Using software bounce buffering for IO (SWIOTLB) [0.928500] software IO TLB [mem 0xbb13d000-0xbf13cfff] (64MB) mapped at [8800bb13d000-8800bf13cfff] [0.938535] LVT offset 0 assigned for vector 0x400 [0.943338] perf: AMD IBS detected (0x00ff) [0.948037] audit: initializing netlink socket (disabled) [0.953432] type=2000 audit(1369659616.800:1): initialized [0.977011] HugeTLB registered 2 MB page size, pre-allocated 0 pages […] [7.881938] radeon :00:01.0: >VRAM: 512M 0x - 0x1FFF (512M used) [7.881941] radeon :00:01.0: >GTT: 512M 0x2000 - 0x3FFF […] [7.885516] radeon :00:01.0: >irq 48 for MSI/MSI-X [7.885525] radeon :00:01.0: >radeon: using MSI. […] [8.276775] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ae000 flags=0x0010] [8.287363] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001acc00 flags=0x0010] [8.297945] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ae200 flags=0x0010] [8.308527] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ae080 flags=0x0010] [8.319109] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ae240 flags=0x0010] [8.329694] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001accc0 flags=0x0010] [8.340276] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ace80 flags=0x0010] [8.350858] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001acd80 flags=0x0010] [8.361441] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ae280 flags=0x0010] [8.372022] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ae180 flags=0x0010] [8.382605] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ace00 flags=0x0010] [8.393188] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001acdc0 flags=0x0010] [8.403770] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ace40 flags=0x0010] [8.414353] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001ae1c0 flags=0x0010] [8.424936] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001acc40 flags=0x0010] [8.435518] AMD-Vi: Event logged [IO_PAGE_FAULT device=00:01.0 domain=0x0003 address=0x000f001acc80 flags=0x0010] [8.446100] AMD-Vi: Event lo
