Re: [PATCH v2 4/6] drm/msm: Add support to create a local pagetable
On Tue, Jul 07, 2020 at 12:36:42PM +0100, Robin Murphy wrote: > On 2020-06-26 21:04, Jordan Crouse wrote: > >Add support to create a io-pgtable for use by targets that support > >per-instance pagetables. In order to support per-instance pagetables the > >GPU SMMU device needs to have the qcom,adreno-smmu compatible string and > >split pagetables and auxiliary domains need to be supported and enabled. > > > >Signed-off-by: Jordan Crouse > >--- > > > > drivers/gpu/drm/msm/msm_gpummu.c | 2 +- > > drivers/gpu/drm/msm/msm_iommu.c | 180 ++- > > drivers/gpu/drm/msm/msm_mmu.h| 16 ++- > > 3 files changed, 195 insertions(+), 3 deletions(-) > > > >diff --git a/drivers/gpu/drm/msm/msm_gpummu.c > >b/drivers/gpu/drm/msm/msm_gpummu.c > >index 310a31b05faa..aab121f4beb7 100644 > >--- a/drivers/gpu/drm/msm/msm_gpummu.c > >+++ b/drivers/gpu/drm/msm/msm_gpummu.c > >@@ -102,7 +102,7 @@ struct msm_mmu *msm_gpummu_new(struct device *dev, > >struct msm_gpu *gpu) > > } > > gpummu->gpu = gpu; > >-msm_mmu_init(&gpummu->base, dev, &funcs); > >+msm_mmu_init(&gpummu->base, dev, &funcs, MSM_MMU_GPUMMU); > > return &gpummu->base; > > } > >diff --git a/drivers/gpu/drm/msm/msm_iommu.c > >b/drivers/gpu/drm/msm/msm_iommu.c > >index 1b6635504069..f455c597f76d 100644 > >--- a/drivers/gpu/drm/msm/msm_iommu.c > >+++ b/drivers/gpu/drm/msm/msm_iommu.c > >@@ -4,15 +4,192 @@ > > * Author: Rob Clark > > */ > >+#include > > #include "msm_drv.h" > > #include "msm_mmu.h" > > struct msm_iommu { > > struct msm_mmu base; > > struct iommu_domain *domain; > >+struct iommu_domain *aux_domain; > > }; > >+ > > #define to_msm_iommu(x) container_of(x, struct msm_iommu, base) > >+struct msm_iommu_pagetable { > >+struct msm_mmu base; > >+struct msm_mmu *parent; > >+struct io_pgtable_ops *pgtbl_ops; > >+phys_addr_t ttbr; > >+u32 asid; > >+}; > >+ > >+static struct msm_iommu_pagetable *to_pagetable(struct msm_mmu *mmu) > >+{ > >+return container_of(mmu, struct msm_iommu_pagetable, base); > >+} > >+ > >+static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova, > >+size_t size) > >+{ > >+struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); > >+struct io_pgtable_ops *ops = pagetable->pgtbl_ops; > >+size_t unmapped = 0; > >+ > >+/* Unmap the block one page at a time */ > >+while (size) { > >+unmapped += ops->unmap(ops, iova, 4096, NULL); > >+iova += 4096; > >+size -= 4096; > >+} > >+ > >+iommu_flush_tlb_all(to_msm_iommu(pagetable->parent)->domain); > >+ > >+return (unmapped == size) ? 0 : -EINVAL; > >+} > > Remember in patch #1 when you said "Then 'domain' can be used like any other > iommu domain to map and unmap iova addresses in the pagetable."? > > This appears to be very much not that :/ The code changed but the commit log stayed the same. I'll reword. Jordan > Robin. > > >+ > >+static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova, > >+struct sg_table *sgt, size_t len, int prot) > >+{ > >+struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); > >+struct io_pgtable_ops *ops = pagetable->pgtbl_ops; > >+struct scatterlist *sg; > >+size_t mapped = 0; > >+u64 addr = iova; > >+unsigned int i; > >+ > >+for_each_sg(sgt->sgl, sg, sgt->nents, i) { > >+size_t size = sg->length; > >+phys_addr_t phys = sg_phys(sg); > >+ > >+/* Map the block one page at a time */ > >+while (size) { > >+if (ops->map(ops, addr, phys, 4096, prot)) { > >+msm_iommu_pagetable_unmap(mmu, iova, mapped); > >+return -EINVAL; > >+} > >+ > >+phys += 4096; > >+addr += 4096; > >+size -= 4096; > >+mapped += 4096; > >+} > >+} > >+ > >+return 0; > >+} > >+ > >+static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu) > >+{ > >+struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); > >+ > >+free_io_pgtable_ops(pagetable->pgtbl_ops); > >+kfree(pagetable); > >+} > >+ > >+/* > >+ * Given a parent device, create and return an aux domain. This will enable > >the > >+ * TTBR0 region > >+ */ > >+static struct iommu_domain *msm_iommu_get_aux_domain(struct msm_mmu *parent) > >+{ > >+struct msm_iommu *iommu = to_msm_iommu(parent); > >+struct iommu_domain *domain; > >+int ret; > >+ > >+if (iommu->aux_domain) > >+return iommu->aux_domain; > >+ > >+if (!iommu_dev_has_feature(parent->dev, IOMMU_DEV_FEAT_AUX)) > >+return ERR_PTR(-ENODEV); > >+ > >+domain = iommu_domain_alloc(&platform_bus_type); > >+if (!domain) > >+return ERR_PTR(-ENODEV); > >+ > >+ret = iommu_aux_attach_device(domain, parent->dev); > >+if (ret) { > >+
Re: [Freedreno] [PATCH v2 4/6] drm/msm: Add support to create a local pagetable
On Tue, Jul 7, 2020 at 4:36 AM Robin Murphy wrote: > > On 2020-06-26 21:04, Jordan Crouse wrote: > > Add support to create a io-pgtable for use by targets that support > > per-instance pagetables. In order to support per-instance pagetables the > > GPU SMMU device needs to have the qcom,adreno-smmu compatible string and > > split pagetables and auxiliary domains need to be supported and enabled. > > > > Signed-off-by: Jordan Crouse > > --- > > > > drivers/gpu/drm/msm/msm_gpummu.c | 2 +- > > drivers/gpu/drm/msm/msm_iommu.c | 180 ++- > > drivers/gpu/drm/msm/msm_mmu.h| 16 ++- > > 3 files changed, 195 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gpummu.c > > b/drivers/gpu/drm/msm/msm_gpummu.c > > index 310a31b05faa..aab121f4beb7 100644 > > --- a/drivers/gpu/drm/msm/msm_gpummu.c > > +++ b/drivers/gpu/drm/msm/msm_gpummu.c > > @@ -102,7 +102,7 @@ struct msm_mmu *msm_gpummu_new(struct device *dev, > > struct msm_gpu *gpu) > > } > > > > gpummu->gpu = gpu; > > - msm_mmu_init(&gpummu->base, dev, &funcs); > > + msm_mmu_init(&gpummu->base, dev, &funcs, MSM_MMU_GPUMMU); > > > > return &gpummu->base; > > } > > diff --git a/drivers/gpu/drm/msm/msm_iommu.c > > b/drivers/gpu/drm/msm/msm_iommu.c > > index 1b6635504069..f455c597f76d 100644 > > --- a/drivers/gpu/drm/msm/msm_iommu.c > > +++ b/drivers/gpu/drm/msm/msm_iommu.c > > @@ -4,15 +4,192 @@ > >* Author: Rob Clark > >*/ > > > > +#include > > #include "msm_drv.h" > > #include "msm_mmu.h" > > > > struct msm_iommu { > > struct msm_mmu base; > > struct iommu_domain *domain; > > + struct iommu_domain *aux_domain; > > }; > > + > > #define to_msm_iommu(x) container_of(x, struct msm_iommu, base) > > > > +struct msm_iommu_pagetable { > > + struct msm_mmu base; > > + struct msm_mmu *parent; > > + struct io_pgtable_ops *pgtbl_ops; > > + phys_addr_t ttbr; > > + u32 asid; > > +}; > > + > > +static struct msm_iommu_pagetable *to_pagetable(struct msm_mmu *mmu) > > +{ > > + return container_of(mmu, struct msm_iommu_pagetable, base); > > +} > > + > > +static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova, > > + size_t size) > > +{ > > + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); > > + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; > > + size_t unmapped = 0; > > + > > + /* Unmap the block one page at a time */ > > + while (size) { > > + unmapped += ops->unmap(ops, iova, 4096, NULL); > > + iova += 4096; > > + size -= 4096; > > + } > > + > > + iommu_flush_tlb_all(to_msm_iommu(pagetable->parent)->domain); > > + > > + return (unmapped == size) ? 0 : -EINVAL; > > +} > > Remember in patch #1 when you said "Then 'domain' can be used like any > other iommu domain to map and unmap iova addresses in the pagetable."? > > This appears to be very much not that :/ > I guess that comment is a bit stale.. the original plan was to create an iommu_domain per set of pgtables, but at some point we realized that by using the io-pgtable helpers directly, we would inflict a lot less GPU-crazy on the iommu drivers BR, -R > Robin. > > > + > > +static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova, > > + struct sg_table *sgt, size_t len, int prot) > > +{ > > + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); > > + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; > > + struct scatterlist *sg; > > + size_t mapped = 0; > > + u64 addr = iova; > > + unsigned int i; > > + > > + for_each_sg(sgt->sgl, sg, sgt->nents, i) { > > + size_t size = sg->length; > > + phys_addr_t phys = sg_phys(sg); > > + > > + /* Map the block one page at a time */ > > + while (size) { > > + if (ops->map(ops, addr, phys, 4096, prot)) { > > + msm_iommu_pagetable_unmap(mmu, iova, mapped); > > + return -EINVAL; > > + } > > + > > + phys += 4096; > > + addr += 4096; > > + size -= 4096; > > + mapped += 4096; > > + } > > + } > > + > > + return 0; > > +} > > + > > +static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu) > > +{ > > + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); > > + > > + free_io_pgtable_ops(pagetable->pgtbl_ops); > > + kfree(pagetable); > > +} > > + > > +/* > > + * Given a parent device, create and return an aux domain. This will > > enable the > > + * TTBR0 region > > + */ > > +static struct iommu_domain *msm_iommu_get_aux_domain(struct msm_mmu > > *parent) > > +{ > > + struct msm_iommu *iommu = to_msm_iommu(parent); > > + struct iommu_domain *domain; > > + int ret; > > + > > + if (iommu->aux_domain) > > +
Re: [PATCH v2 4/6] drm/msm: Add support to create a local pagetable
On 2020-06-26 21:04, Jordan Crouse wrote: Add support to create a io-pgtable for use by targets that support per-instance pagetables. In order to support per-instance pagetables the GPU SMMU device needs to have the qcom,adreno-smmu compatible string and split pagetables and auxiliary domains need to be supported and enabled. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpummu.c | 2 +- drivers/gpu/drm/msm/msm_iommu.c | 180 ++- drivers/gpu/drm/msm/msm_mmu.h| 16 ++- 3 files changed, 195 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gpummu.c b/drivers/gpu/drm/msm/msm_gpummu.c index 310a31b05faa..aab121f4beb7 100644 --- a/drivers/gpu/drm/msm/msm_gpummu.c +++ b/drivers/gpu/drm/msm/msm_gpummu.c @@ -102,7 +102,7 @@ struct msm_mmu *msm_gpummu_new(struct device *dev, struct msm_gpu *gpu) } gpummu->gpu = gpu; - msm_mmu_init(&gpummu->base, dev, &funcs); + msm_mmu_init(&gpummu->base, dev, &funcs, MSM_MMU_GPUMMU); return &gpummu->base; } diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index 1b6635504069..f455c597f76d 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -4,15 +4,192 @@ * Author: Rob Clark */ +#include #include "msm_drv.h" #include "msm_mmu.h" struct msm_iommu { struct msm_mmu base; struct iommu_domain *domain; + struct iommu_domain *aux_domain; }; + #define to_msm_iommu(x) container_of(x, struct msm_iommu, base) +struct msm_iommu_pagetable { + struct msm_mmu base; + struct msm_mmu *parent; + struct io_pgtable_ops *pgtbl_ops; + phys_addr_t ttbr; + u32 asid; +}; + +static struct msm_iommu_pagetable *to_pagetable(struct msm_mmu *mmu) +{ + return container_of(mmu, struct msm_iommu_pagetable, base); +} + +static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova, + size_t size) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; + size_t unmapped = 0; + + /* Unmap the block one page at a time */ + while (size) { + unmapped += ops->unmap(ops, iova, 4096, NULL); + iova += 4096; + size -= 4096; + } + + iommu_flush_tlb_all(to_msm_iommu(pagetable->parent)->domain); + + return (unmapped == size) ? 0 : -EINVAL; +} Remember in patch #1 when you said "Then 'domain' can be used like any other iommu domain to map and unmap iova addresses in the pagetable."? This appears to be very much not that :/ Robin. + +static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova, + struct sg_table *sgt, size_t len, int prot) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; + struct scatterlist *sg; + size_t mapped = 0; + u64 addr = iova; + unsigned int i; + + for_each_sg(sgt->sgl, sg, sgt->nents, i) { + size_t size = sg->length; + phys_addr_t phys = sg_phys(sg); + + /* Map the block one page at a time */ + while (size) { + if (ops->map(ops, addr, phys, 4096, prot)) { + msm_iommu_pagetable_unmap(mmu, iova, mapped); + return -EINVAL; + } + + phys += 4096; + addr += 4096; + size -= 4096; + mapped += 4096; + } + } + + return 0; +} + +static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + + free_io_pgtable_ops(pagetable->pgtbl_ops); + kfree(pagetable); +} + +/* + * Given a parent device, create and return an aux domain. This will enable the + * TTBR0 region + */ +static struct iommu_domain *msm_iommu_get_aux_domain(struct msm_mmu *parent) +{ + struct msm_iommu *iommu = to_msm_iommu(parent); + struct iommu_domain *domain; + int ret; + + if (iommu->aux_domain) + return iommu->aux_domain; + + if (!iommu_dev_has_feature(parent->dev, IOMMU_DEV_FEAT_AUX)) + return ERR_PTR(-ENODEV); + + domain = iommu_domain_alloc(&platform_bus_type); + if (!domain) + return ERR_PTR(-ENODEV); + + ret = iommu_aux_attach_device(domain, parent->dev); + if (ret) { + iommu_domain_free(domain); + return ERR_PTR(ret); + } + + iommu->aux_domain = domain; + return domain; +} + +int msm_iommu_pagetable_params(struct msm_mmu *mmu, + phys_addr_t *ttbr, int *asid) +{ + struct msm_iommu_pagetable *pagetable; + + if (mmu->type != MSM_MMU_IOMMU_PAGETABLE) + return
[PATCH v2 4/6] drm/msm: Add support to create a local pagetable
Add support to create a io-pgtable for use by targets that support per-instance pagetables. In order to support per-instance pagetables the GPU SMMU device needs to have the qcom,adreno-smmu compatible string and split pagetables and auxiliary domains need to be supported and enabled. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpummu.c | 2 +- drivers/gpu/drm/msm/msm_iommu.c | 180 ++- drivers/gpu/drm/msm/msm_mmu.h| 16 ++- 3 files changed, 195 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gpummu.c b/drivers/gpu/drm/msm/msm_gpummu.c index 310a31b05faa..aab121f4beb7 100644 --- a/drivers/gpu/drm/msm/msm_gpummu.c +++ b/drivers/gpu/drm/msm/msm_gpummu.c @@ -102,7 +102,7 @@ struct msm_mmu *msm_gpummu_new(struct device *dev, struct msm_gpu *gpu) } gpummu->gpu = gpu; - msm_mmu_init(&gpummu->base, dev, &funcs); + msm_mmu_init(&gpummu->base, dev, &funcs, MSM_MMU_GPUMMU); return &gpummu->base; } diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index 1b6635504069..f455c597f76d 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -4,15 +4,192 @@ * Author: Rob Clark */ +#include #include "msm_drv.h" #include "msm_mmu.h" struct msm_iommu { struct msm_mmu base; struct iommu_domain *domain; + struct iommu_domain *aux_domain; }; + #define to_msm_iommu(x) container_of(x, struct msm_iommu, base) +struct msm_iommu_pagetable { + struct msm_mmu base; + struct msm_mmu *parent; + struct io_pgtable_ops *pgtbl_ops; + phys_addr_t ttbr; + u32 asid; +}; + +static struct msm_iommu_pagetable *to_pagetable(struct msm_mmu *mmu) +{ + return container_of(mmu, struct msm_iommu_pagetable, base); +} + +static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova, + size_t size) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; + size_t unmapped = 0; + + /* Unmap the block one page at a time */ + while (size) { + unmapped += ops->unmap(ops, iova, 4096, NULL); + iova += 4096; + size -= 4096; + } + + iommu_flush_tlb_all(to_msm_iommu(pagetable->parent)->domain); + + return (unmapped == size) ? 0 : -EINVAL; +} + +static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova, + struct sg_table *sgt, size_t len, int prot) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + struct io_pgtable_ops *ops = pagetable->pgtbl_ops; + struct scatterlist *sg; + size_t mapped = 0; + u64 addr = iova; + unsigned int i; + + for_each_sg(sgt->sgl, sg, sgt->nents, i) { + size_t size = sg->length; + phys_addr_t phys = sg_phys(sg); + + /* Map the block one page at a time */ + while (size) { + if (ops->map(ops, addr, phys, 4096, prot)) { + msm_iommu_pagetable_unmap(mmu, iova, mapped); + return -EINVAL; + } + + phys += 4096; + addr += 4096; + size -= 4096; + mapped += 4096; + } + } + + return 0; +} + +static void msm_iommu_pagetable_destroy(struct msm_mmu *mmu) +{ + struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); + + free_io_pgtable_ops(pagetable->pgtbl_ops); + kfree(pagetable); +} + +/* + * Given a parent device, create and return an aux domain. This will enable the + * TTBR0 region + */ +static struct iommu_domain *msm_iommu_get_aux_domain(struct msm_mmu *parent) +{ + struct msm_iommu *iommu = to_msm_iommu(parent); + struct iommu_domain *domain; + int ret; + + if (iommu->aux_domain) + return iommu->aux_domain; + + if (!iommu_dev_has_feature(parent->dev, IOMMU_DEV_FEAT_AUX)) + return ERR_PTR(-ENODEV); + + domain = iommu_domain_alloc(&platform_bus_type); + if (!domain) + return ERR_PTR(-ENODEV); + + ret = iommu_aux_attach_device(domain, parent->dev); + if (ret) { + iommu_domain_free(domain); + return ERR_PTR(ret); + } + + iommu->aux_domain = domain; + return domain; +} + +int msm_iommu_pagetable_params(struct msm_mmu *mmu, + phys_addr_t *ttbr, int *asid) +{ + struct msm_iommu_pagetable *pagetable; + + if (mmu->type != MSM_MMU_IOMMU_PAGETABLE) + return -EINVAL; + + pagetable = to_pagetable(mmu); + + if (ttbr) + *ttbr = pagetable->ttbr; + + if (asid) + *asid = pagetable->asid; + + return 0; +} + +static const struct msm_mmu_funcs pagetable_funcs = { +