[RFC,3/3] iommu/tegra: smmu: Support read-only mapping

Message ID 1371707384-30037-4-git-send-email-hdoyu@nvidia.com
State New
Headers show

Commit Message

Hiroshi Doyu June 20, 2013, 5:49 a.m.
Support read-only mapping via struct dma_attrs.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

Comments

Kyungmin Park June 20, 2013, 6:50 a.m. | #1
On Thu, Jun 20, 2013 at 2:49 PM, Hiroshi Doyu <hdoyu@nvidia.com> wrote:
> Support read-only mapping via struct dma_attrs.
>
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
>  drivers/iommu/tegra-smmu.c | 41 +++++++++++++++++++++++++++++------------
>  1 file changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index fab1f19..3aff4cd 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -862,12 +862,13 @@ static size_t __smmu_iommu_unmap_largepage(struct smmu_as *as, dma_addr_t iova)
>  }
>
>  static int __smmu_iommu_map_pfn(struct smmu_as *as, dma_addr_t iova,
> -                                unsigned long pfn)
> +                               unsigned long pfn, int prot)
Can you find 'prot' is used at other arch? In previous patch, you cast
it as 'int' but below code cast it as 'struct dma_attr' again.
doesn't it better to use 'struct dma_attr' as parameter to avoid
double cast? Of course you have to modify existing APIs to use 'struct
dma_attr'.

Thank you,
Kyungmin Park
>  {
>         struct smmu_device *smmu = as->smmu;
>         unsigned long *pte;
>         unsigned int *count;
>         struct page *page;
> +       int attrs = as->pte_attr;
>
>         pte = locate_pte(as, iova, true, &page, &count);
>         if (WARN_ON(!pte))
> @@ -875,7 +876,11 @@ static int __smmu_iommu_map_pfn(struct smmu_as *as, dma_addr_t iova,
>
>         if (*pte == _PTE_VACANT(iova))
>                 (*count)++;
> -       *pte = SMMU_PFN_TO_PTE(pfn, as->pte_attr);
> +
> +       if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
> +               attrs &= ~_WRITABLE;
> +
> +       *pte = SMMU_PFN_TO_PTE(pfn, attrs);
>         FLUSH_CPU_DCACHE(pte, page, sizeof(*pte));
>         flush_ptc_and_tlb(smmu, as, iova, pte, page, 0);
>         put_signature(as, iova, pfn);
> @@ -883,23 +888,27 @@ static int __smmu_iommu_map_pfn(struct smmu_as *as, dma_addr_t iova,
>  }
>
>  static int __smmu_iommu_map_page(struct smmu_as *as, dma_addr_t iova,
> -                                phys_addr_t pa)
> +                                phys_addr_t pa, int prot)
>  {
>         unsigned long pfn = __phys_to_pfn(pa);
>
> -       return __smmu_iommu_map_pfn(as, iova, pfn);
> +       return __smmu_iommu_map_pfn(as, iova, pfn, prot);
>  }
>
>  static int __smmu_iommu_map_largepage(struct smmu_as *as, dma_addr_t iova,
> -                                phys_addr_t pa)
> +                                phys_addr_t pa, int prot)
>  {
>         unsigned long pdn = SMMU_ADDR_TO_PDN(iova);
>         unsigned long *pdir = (unsigned long *)page_address(as->pdir_page);
> +       int attrs = _PDE_ATTR;
>
>         if (pdir[pdn] != _PDE_VACANT(pdn))
>                 return -EINVAL;
>
> -       pdir[pdn] = SMMU_ADDR_TO_PDN(pa) << 10 | _PDE_ATTR;
> +       if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
> +               attrs &= ~_WRITABLE;
> +
> +       pdir[pdn] = SMMU_ADDR_TO_PDN(pa) << 10 | attrs;
>         FLUSH_CPU_DCACHE(&pdir[pdn], as->pdir_page, sizeof pdir[pdn]);
>         flush_ptc_and_tlb(as->smmu, as, iova, &pdir[pdn], as->pdir_page, 1);
>
> @@ -912,7 +921,8 @@ static int smmu_iommu_map(struct iommu_domain *domain, unsigned long iova,
>         struct smmu_as *as = domain->priv;
>         unsigned long flags;
>         int err;
> -       int (*fn)(struct smmu_as *as, dma_addr_t iova, phys_addr_t pa);
> +       int (*fn)(struct smmu_as *as, dma_addr_t iova, phys_addr_t pa,
> +                 int prot);
>
>         dev_dbg(as->smmu->dev, "[%d] %08lx:%08x\n", as->asid, iova, pa);
>
> @@ -929,7 +939,7 @@ static int smmu_iommu_map(struct iommu_domain *domain, unsigned long iova,
>         }
>
>         spin_lock_irqsave(&as->lock, flags);
> -       err = fn(as, iova, pa);
> +       err = fn(as, iova, pa, prot);
>         spin_unlock_irqrestore(&as->lock, flags);
>         return err;
>  }
> @@ -943,6 +953,10 @@ static int smmu_iommu_map_pages(struct iommu_domain *domain, unsigned long iova,
>         unsigned long *pdir = page_address(as->pdir_page);
>         int err = 0;
>         bool flush_all = (total > SZ_512) ? true : false;
> +       int attrs = as->pte_attr;
> +
> +       if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
> +               attrs &= ~_WRITABLE;
>
>         spin_lock_irqsave(&as->lock, flags);
>
> @@ -977,8 +991,7 @@ static int smmu_iommu_map_pages(struct iommu_domain *domain, unsigned long iova,
>                         if (*pte == _PTE_VACANT(iova + i * PAGE_SIZE))
>                                 (*rest)++;
>
> -                       *pte = SMMU_PFN_TO_PTE(page_to_pfn(pages[i]),
> -                                              as->pte_attr);
> +                       *pte = SMMU_PFN_TO_PTE(page_to_pfn(pages[i]), attrs);
>                 }
>
>                 pte = &ptbl[ptn];
> @@ -1010,6 +1023,10 @@ static int smmu_iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
>         bool flush_all = (nents * PAGE_SIZE > SZ_512) ? true : false;
>         struct smmu_as *as = domain->priv;
>         struct smmu_device *smmu = as->smmu;
> +       int attrs = as->pte_attr;
> +
> +       if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
> +               attrs &= ~_WRITABLE;
>
>         for (count = 0, s = sgl; count < nents; s = sg_next(s)) {
>                 phys_addr_t phys = page_to_phys(sg_page(s));
> @@ -1053,7 +1070,7 @@ static int smmu_iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
>                                         (*rest)++;
>                                 }
>
> -                               *pte = SMMU_PFN_TO_PTE(pfn + i, as->pte_attr);
> +                               *pte = SMMU_PFN_TO_PTE(pfn + i, attrs);
>                         }
>
>                         pte = &ptbl[ptn];
> @@ -1191,7 +1208,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
>                 struct page *page;
>
>                 page = as->smmu->avp_vector_page;
> -               __smmu_iommu_map_pfn(as, 0, page_to_pfn(page));
> +               __smmu_iommu_map_pfn(as, 0, page_to_pfn(page), 0);
>
>                 pr_debug("Reserve \"page zero\" \
>                         for AVP vectors using a common dummy\n");
> --
> 1.8.1.5
>
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
Hiroshi Doyu June 20, 2013, 7:27 a.m. | #2
Hi Kyungmin,

Kyungmin Park <kmpark@infradead.org> wrote @ Thu, 20 Jun 2013 08:50:14 +0200:

> On Thu, Jun 20, 2013 at 2:49 PM, Hiroshi Doyu <hdoyu@nvidia.com> wrote:
> > Support read-only mapping via struct dma_attrs.
> >
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >  drivers/iommu/tegra-smmu.c | 41 +++++++++++++++++++++++++++++------------
> >  1 file changed, 29 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> > index fab1f19..3aff4cd 100644
> > --- a/drivers/iommu/tegra-smmu.c
> > +++ b/drivers/iommu/tegra-smmu.c
> > @@ -862,12 +862,13 @@ static size_t __smmu_iommu_unmap_largepage(struct smmu_as *as, dma_addr_t iova)
> >  }
> >
> >  static int __smmu_iommu_map_pfn(struct smmu_as *as, dma_addr_t iova,
> > -                                unsigned long pfn)
> > +                               unsigned long pfn, int prot)
> Can you find 'prot' is used at other arch? In previous patch, you cast

{amd,intel}_iommu_map() take the IOMMU standard flags below and
translate it into each H/W dependent bits.

#define IOMMU_READ	(1)
#define IOMMU_WRITE	(2)
#define IOMMU_CACHE	(4) /* DMA cache coherency */

The others like OMAP/MSM pass their H/W dependent bits all the way. I
think that they don't use IOMMU via DMA mapping API.

> it as 'int' but below code cast it as 'struct dma_attr' again.
> doesn't it better to use 'struct dma_attr' as parameter to avoid
> double cast? Of course you have to modify existing APIs to use 'struct
> dma_attr'.

If DMA mapping API is considered as the standard frontend of
IOMMU(API), that may be an option.

Patch hide | download patch | download mbox

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index fab1f19..3aff4cd 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -862,12 +862,13 @@  static size_t __smmu_iommu_unmap_largepage(struct smmu_as *as, dma_addr_t iova)
 }
 
 static int __smmu_iommu_map_pfn(struct smmu_as *as, dma_addr_t iova,
-				 unsigned long pfn)
+				unsigned long pfn, int prot)
 {
 	struct smmu_device *smmu = as->smmu;
 	unsigned long *pte;
 	unsigned int *count;
 	struct page *page;
+	int attrs = as->pte_attr;
 
 	pte = locate_pte(as, iova, true, &page, &count);
 	if (WARN_ON(!pte))
@@ -875,7 +876,11 @@  static int __smmu_iommu_map_pfn(struct smmu_as *as, dma_addr_t iova,
 
 	if (*pte == _PTE_VACANT(iova))
 		(*count)++;
-	*pte = SMMU_PFN_TO_PTE(pfn, as->pte_attr);
+
+	if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
+		attrs &= ~_WRITABLE;
+
+	*pte = SMMU_PFN_TO_PTE(pfn, attrs);
 	FLUSH_CPU_DCACHE(pte, page, sizeof(*pte));
 	flush_ptc_and_tlb(smmu, as, iova, pte, page, 0);
 	put_signature(as, iova, pfn);
@@ -883,23 +888,27 @@  static int __smmu_iommu_map_pfn(struct smmu_as *as, dma_addr_t iova,
 }
 
 static int __smmu_iommu_map_page(struct smmu_as *as, dma_addr_t iova,
-				 phys_addr_t pa)
+				 phys_addr_t pa, int prot)
 {
 	unsigned long pfn = __phys_to_pfn(pa);
 
-	return __smmu_iommu_map_pfn(as, iova, pfn);
+	return __smmu_iommu_map_pfn(as, iova, pfn, prot);
 }
 
 static int __smmu_iommu_map_largepage(struct smmu_as *as, dma_addr_t iova,
-				 phys_addr_t pa)
+				 phys_addr_t pa, int prot)
 {
 	unsigned long pdn = SMMU_ADDR_TO_PDN(iova);
 	unsigned long *pdir = (unsigned long *)page_address(as->pdir_page);
+	int attrs = _PDE_ATTR;
 
 	if (pdir[pdn] != _PDE_VACANT(pdn))
 		return -EINVAL;
 
-	pdir[pdn] = SMMU_ADDR_TO_PDN(pa) << 10 | _PDE_ATTR;
+	if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
+		attrs &= ~_WRITABLE;
+
+	pdir[pdn] = SMMU_ADDR_TO_PDN(pa) << 10 | attrs;
 	FLUSH_CPU_DCACHE(&pdir[pdn], as->pdir_page, sizeof pdir[pdn]);
 	flush_ptc_and_tlb(as->smmu, as, iova, &pdir[pdn], as->pdir_page, 1);
 
@@ -912,7 +921,8 @@  static int smmu_iommu_map(struct iommu_domain *domain, unsigned long iova,
 	struct smmu_as *as = domain->priv;
 	unsigned long flags;
 	int err;
-	int (*fn)(struct smmu_as *as, dma_addr_t iova, phys_addr_t pa);
+	int (*fn)(struct smmu_as *as, dma_addr_t iova, phys_addr_t pa,
+		  int prot);
 
 	dev_dbg(as->smmu->dev, "[%d] %08lx:%08x\n", as->asid, iova, pa);
 
@@ -929,7 +939,7 @@  static int smmu_iommu_map(struct iommu_domain *domain, unsigned long iova,
 	}
 
 	spin_lock_irqsave(&as->lock, flags);
-	err = fn(as, iova, pa);
+	err = fn(as, iova, pa, prot);
 	spin_unlock_irqrestore(&as->lock, flags);
 	return err;
 }
@@ -943,6 +953,10 @@  static int smmu_iommu_map_pages(struct iommu_domain *domain, unsigned long iova,
 	unsigned long *pdir = page_address(as->pdir_page);
 	int err = 0;
 	bool flush_all = (total > SZ_512) ? true : false;
+	int attrs = as->pte_attr;
+
+	if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
+		attrs &= ~_WRITABLE;
 
 	spin_lock_irqsave(&as->lock, flags);
 
@@ -977,8 +991,7 @@  static int smmu_iommu_map_pages(struct iommu_domain *domain, unsigned long iova,
 			if (*pte == _PTE_VACANT(iova + i * PAGE_SIZE))
 				(*rest)++;
 
-			*pte = SMMU_PFN_TO_PTE(page_to_pfn(pages[i]),
-					       as->pte_attr);
+			*pte = SMMU_PFN_TO_PTE(page_to_pfn(pages[i]), attrs);
 		}
 
 		pte = &ptbl[ptn];
@@ -1010,6 +1023,10 @@  static int smmu_iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
 	bool flush_all = (nents * PAGE_SIZE > SZ_512) ? true : false;
 	struct smmu_as *as = domain->priv;
 	struct smmu_device *smmu = as->smmu;
+	int attrs = as->pte_attr;
+
+	if (dma_get_attr(DMA_ATTR_READ_ONLY, (struct dma_attrs *)prot))
+		attrs &= ~_WRITABLE;
 
 	for (count = 0, s = sgl; count < nents; s = sg_next(s)) {
 		phys_addr_t phys = page_to_phys(sg_page(s));
@@ -1053,7 +1070,7 @@  static int smmu_iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
 					(*rest)++;
 				}
 
-				*pte = SMMU_PFN_TO_PTE(pfn + i, as->pte_attr);
+				*pte = SMMU_PFN_TO_PTE(pfn + i, attrs);
 			}
 
 			pte = &ptbl[ptn];
@@ -1191,7 +1208,7 @@  static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 		struct page *page;
 
 		page = as->smmu->avp_vector_page;
-		__smmu_iommu_map_pfn(as, 0, page_to_pfn(page));
+		__smmu_iommu_map_pfn(as, 0, page_to_pfn(page), 0);
 
 		pr_debug("Reserve \"page zero\" \
 			for AVP vectors using a common dummy\n");