Re: [Freedreno] [PATCH 11/14] drm/msm: Add support for iommu-sva PASIDs

[Jean-Philippe Brucker]

On 21/02/18 22:59, Jordan Crouse wrote:
[...]> +static int install_pasid_cb(int pasid, u64 ttbr, u32 asid, void *data)
> +{
> + struct pasid_entry *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +
> + if (!entry)
> + return -ENOMEM;
> +
> + entry->pasid = pasid;
> + entry->ttbr = ttbr;
> + entry->asid = asid;
> +
> + /* FIXME: Assume that we'll never have a pasid conflict? */

I think a conflict would be a bug on the IOMMU side. Users should not have
to check this. Then again, I have a few WARNs on the SMMUv3 context table
code that uncovered nasty bugs during development.

Thanks,
Jean
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [PATCH 11/14] drm/msm: Add support for iommu-sva PASIDs

[Jordan Crouse]

The IOMMU core can support creating multiple pagetables
for a specific domai and making them available to a client
driver that has the means to manage the pagetable itself.

PASIDs are unique indexes to a software created pagetable with
the same format and characteristics as the parent IOMMU device.
The IOMMU driver allocates the pagetable and tracks it with a
unique token (PASID) - it does not touch the actual hardware.
 The client driver is expected to be able to manage the pagetables
and do something interesting with them.

Some flavors of the MSM GPU are able to allow each DRM instance
to have its own pagetable (and virtual memory space) and switch them
asynchronously at the beginning of a command.  This protects against
accidental or malicious corruption or copying of buffers from other
instances.

The first step is to add a MMU implementation that can allocate a
PASID and set up a msm_mmu struct to abstract (most) of the details
from the rest of the system.

Signed-off-by: Jordan Crouse 
---
 drivers/gpu/drm/msm/msm_iommu.c | 184 
 drivers/gpu/drm/msm/msm_mmu.h   |   6 ++
 2 files changed, 190 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index fdbe1a8372f0..de8669c9a5a1 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -15,6 +15,9 @@
  * this program.  If not, see .
  */
 
+#include 
+#include 
+
 #include "msm_drv.h"
 #include "msm_mmu.h"
 
@@ -34,12 +37,29 @@ static int msm_fault_handler(struct iommu_domain *domain, 
struct device *dev,
return 0;
 }
 
+static bool msm_iommu_check_per_instance(struct msm_iommu *iommu)
+{
+   int val;
+
+   if (!IS_ENABLED(CONFIG_IOMMU_SVA))
+   return false;
+
+   if (iommu_domain_get_attr(iommu->domain, DOMAIN_ATTR_ENABLE_TTBR1,
+   ))
+   return false;
+
+   return val ? true : false;
+}
+
 static int msm_iommu_attach(struct msm_mmu *mmu, const char * const *names,
int cnt)
 {
struct msm_iommu *iommu = to_msm_iommu(mmu);
int ret;
 
+   if (msm_iommu_check_per_instance(iommu))
+   msm_mmu_set_feature(mmu, MMU_FEATURE_PER_INSTANCE_TABLES);
+
pm_runtime_get_sync(mmu->dev);
ret = iommu_attach_device(iommu->domain, mmu->dev);
pm_runtime_put_sync(mmu->dev);
@@ -112,3 +132,167 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct 
iommu_domain *domain)
 
return >base;
 }
+
+struct pasid_entry {
+   int pasid;
+   u64 ttbr;
+   u32 asid;
+   struct hlist_node node;
+};
+
+DECLARE_HASHTABLE(pasid_table, 4);
+
+static int install_pasid_cb(int pasid, u64 ttbr, u32 asid, void *data)
+{
+   struct pasid_entry *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+
+   if (!entry)
+   return -ENOMEM;
+
+   entry->pasid = pasid;
+   entry->ttbr = ttbr;
+   entry->asid = asid;
+
+   /* FIXME: Assume that we'll never have a pasid conflict? */
+   /* FIXME: locks? RCU? */
+   hash_add(pasid_table, >node, pasid);
+   return 0;
+}
+
+static void remove_pasid_cb(int pasid, void *data)
+{
+   struct pasid_entry *entry;
+
+   hash_for_each_possible(pasid_table, entry, node, pasid) {
+   if (pasid == entry->pasid) {
+   hash_del(>node);
+   kfree(entry);
+   return;
+   }
+   }
+}
+
+struct msm_iommu_pasid {
+   struct msm_mmu base;
+   int pasid;
+   u64 ttbr;
+   u32 asid;
+};
+#define to_msm_iommu_pasid(x) container_of(x, struct msm_iommu_pasid, base)
+
+static int msm_iommu_pasid_attach(struct msm_mmu *mmu,
+   const char * const *names, int cnt)
+{
+   return 0;
+}
+
+static int msm_iommu_pasid_map(struct msm_mmu *mmu, uint64_t iova,
+   struct sg_table *sgt, unsigned len, int prot)
+{
+   struct msm_iommu_pasid *pasid = to_msm_iommu_pasid(mmu);
+   int ret;
+
+   ret = iommu_sva_map_sg(pasid->pasid, iova, sgt->sgl, sgt->nents, prot);
+   WARN_ON(ret < 0);
+
+   return (ret == len) ? 0 : -EINVAL;
+}
+
+static int msm_iommu_pasid_unmap(struct msm_mmu *mmu, uint64_t iova,
+   struct sg_table *sgt, unsigned len)
+{
+   struct msm_iommu_pasid *pasid = to_msm_iommu_pasid(mmu);
+
+   iommu_sva_unmap(pasid->pasid, iova, len);
+
+   return 0;
+}
+
+static void msm_iommu_pasid_detach(struct msm_mmu *mmu,
+   const char * const *names, int cnt)
+{
+}
+
+static void msm_iommu_pasid_destroy(struct msm_mmu *mmu)
+{
+   struct msm_iommu_pasid *pasid = to_msm_iommu_pasid(mmu);
+
+   iommu_sva_free_pasid(pasid->pasid);
+   kfree(pasid);
+}
+
+static const struct msm_mmu_funcs pasid_funcs = {
+   .attach = msm_iommu_pasid_attach,
+   .detach = msm_iommu_pasid_detach,
+