Re: [PATCH 16/37] iommu: Add generic PASID table library

2018-02-28 Thread Jean-Philippe Brucker 
On 27/02/18 18:51, Jacob Pan wrote:
[...]
>> +struct iommu_pasid_table_ops *
>> +iommu_alloc_pasid_ops(enum iommu_pasid_table_fmt fmt,
>> +  struct iommu_pasid_table_cfg *cfg, void
>> *cookie) +{
> I guess you don't need to pass in cookie here.

The cookie is stored in the table driver and passed back to the IOMMU
driver when invalidating a PASID table entry

>> +struct iommu_pasid_table *table;
>> +const struct iommu_pasid_init_fns *fns;
>> +
>> +if (fmt >= PASID_TABLE_NUM_FMTS)
>> +return NULL;
>> +
>> +fns = pasid_table_init_fns[fmt];
>> +if (!fns)
>> +return NULL;
>> +
>> +table = fns->alloc(cfg, cookie);
>> +if (!table)
>> +return NULL;
>> +
>> +table->fmt = fmt;
>> +table->cookie = cookie;
>> +table->cfg = *cfg;
>> +
> the ops is already IOMMU model specific, why do you need to pass cfg
> back?

The table driver needs some config information at runtime. Callbacks such
as iommu_pasid_table_ops::alloc_shared_entry() receive the
iommu_pasid_table_ops instance as argument. They can then get the
iommu_pasid_table structure with container_of() and retrieve the config
stored in table->cfg.

>> +return >ops;
> If there is no common code that uses these ops, I don't see the benefit
> of having these APIs. Or the plan is to consolidate even further such
> that referene to pasid table can be attached at per iommu_domain etc,
> but that would be model specific choice.

I don't plan to consolidate further. This API is for multiple IOMMU
drivers with different transports implementing the same PASID table
formats. For example my vSVA implementation uses this API in virtio-iommu
for assigning PASID tables to the guest (All fairly experimental at this
point. I initially intended to assign just the page directories, but
passing the whole PASID table seemed more popular.)

In the future there might be other vendor IOMMUs implementing the same
PASID table formats, just like there are currently 6 IOMMU drivers using
the page-table code implemented by the io-pgtable.c lib (which I copied in
this patch).

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 16/37] iommu: Add generic PASID table library

2018-02-27 Thread Jacob Pan 
On Mon, 12 Feb 2018 18:33:31 +
Jean-Philippe Brucker  wrote:

> Add a small API within the IOMMU subsystem to handle different
> formats of PASID tables. It uses the same principle as io-pgtable:
> 
> * The IOMMU driver registers a PASID table with some invalidation
>   callbacks.
> * The pasid-table lib allocates a set of tables of the right format,
> and returns an iommu_pasid_table_ops structure.
> * The IOMMU driver allocates entries and writes them using the
> provided ops.
> * The pasid-table lib calls the IOMMU driver back for invalidation
> when necessary.
> * The IOMMU driver unregisters the ops which frees the tables when
>   finished.
> 
> An example user will be Arm SMMU in a subsequent patch.
> 
> Signed-off-by: Jean-Philippe Brucker 
> ---
>  drivers/iommu/Kconfig   |   8 +++
>  drivers/iommu/Makefile  |   1 +
>  drivers/iommu/iommu-pasid.c |  53 +
>  drivers/iommu/iommu-pasid.h | 142
>  4 files changed, 204
> insertions(+) create mode 100644 drivers/iommu/iommu-pasid.c
>  create mode 100644 drivers/iommu/iommu-pasid.h
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index e751bb9958ba..8add90ba9b75 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -60,6 +60,14 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
>  
>  endmenu
>  
> +menu "Generic PASID table support"
> +
> +# Selected by the actual PASID table implementations
> +config IOMMU_PASID_TABLE
> + bool
> +
> +endmenu
> +
>  config IOMMU_IOVA
>   tristate
>  
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index f4324e29035e..338e59c93131 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_IOMMU_FAULT) += io-pgfault.o
>  obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
>  obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
>  obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
> +obj-$(CONFIG_IOMMU_PASID_TABLE) += iommu-pasid.o
>  obj-$(CONFIG_IOMMU_IOVA) += iova.o
>  obj-$(CONFIG_OF_IOMMU)   += of_iommu.o
>  obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
> diff --git a/drivers/iommu/iommu-pasid.c b/drivers/iommu/iommu-pasid.c
> new file mode 100644
> index ..6b21d369d514
> --- /dev/null
> +++ b/drivers/iommu/iommu-pasid.c
> @@ -0,0 +1,53 @@
> +/*
> + * PASID table management for the IOMMU
> + *
> + * Copyright (C) 2018 ARM Ltd.
> + * Author: Jean-Philippe Brucker 
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include 
> +
> +#include "iommu-pasid.h"
> +
> +static const struct iommu_pasid_init_fns *
> +pasid_table_init_fns[PASID_TABLE_NUM_FMTS] = {
> +};
> +
> +struct iommu_pasid_table_ops *
> +iommu_alloc_pasid_ops(enum iommu_pasid_table_fmt fmt,
> +   struct iommu_pasid_table_cfg *cfg, void
> *cookie) +{
I guess you don't need to pass in cookie here.
> + struct iommu_pasid_table *table;
> + const struct iommu_pasid_init_fns *fns;
> +
> + if (fmt >= PASID_TABLE_NUM_FMTS)
> + return NULL;
> +
> + fns = pasid_table_init_fns[fmt];
> + if (!fns)
> + return NULL;
> +
> + table = fns->alloc(cfg, cookie);
> + if (!table)
> + return NULL;
> +
> + table->fmt = fmt;
> + table->cookie = cookie;
> + table->cfg = *cfg;
> +
the ops is already IOMMU model specific, why do you need to pass cfg
back?
> + return >ops;
If there is no common code that uses these ops, I don't see the benefit
of having these APIs. Or the plan is to consolidate even further such
that referene to pasid table can be attached at per iommu_domain etc,
but that would be model specific choice.

Jacob 
> +}
> +
> +void iommu_free_pasid_ops(struct iommu_pasid_table_ops *ops)
> +{
> + struct iommu_pasid_table *table;
> +
> + if (!ops)
> + return;
> +
> + table = container_of(ops, struct iommu_pasid_table, ops);
> + iommu_pasid_flush_all(table);
> + pasid_table_init_fns[table->fmt]->free(table);
> +}
> diff --git a/drivers/iommu/iommu-pasid.h b/drivers/iommu/iommu-pasid.h
> new file mode 100644
> index ..40a27d35c1e0
> --- /dev/null
> +++ b/drivers/iommu/iommu-pasid.h
> @@ -0,0 +1,142 @@
> +/*
> + * PASID table management for the IOMMU
> + *
> + * Copyright (C) 2017 ARM Ltd.
> + * Author: Jean-Philippe Brucker 
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +#ifndef __IOMMU_PASID_H
> +#define __IOMMU_PASID_H
> +
> +#include 
> +#include "io-pgtable.h"
> +
> +struct mm_struct;
> +
> +enum iommu_pasid_table_fmt {
> + PASID_TABLE_NUM_FMTS,
> +};
> +
> +/**
> + * iommu_pasid_entry - Entry of a PASID table
> + *
> + * @token:   architecture-specific data needed to uniquely
> identify the
> + *   entry. Most notably used for TLB invalidation
> + */
> +struct iommu_pasid_entry {
> + u64

[PATCH 16/37] iommu: Add generic PASID table library

2018-02-12 Thread Jean-Philippe Brucker 
Add a small API within the IOMMU subsystem to handle different formats of
PASID tables. It uses the same principle as io-pgtable:

* The IOMMU driver registers a PASID table with some invalidation
  callbacks.
* The pasid-table lib allocates a set of tables of the right format, and
  returns an iommu_pasid_table_ops structure.
* The IOMMU driver allocates entries and writes them using the provided
  ops.
* The pasid-table lib calls the IOMMU driver back for invalidation when
  necessary.
* The IOMMU driver unregisters the ops which frees the tables when
  finished.

An example user will be Arm SMMU in a subsequent patch.

Signed-off-by: Jean-Philippe Brucker 
---
 drivers/iommu/Kconfig   |   8 +++
 drivers/iommu/Makefile  |   1 +
 drivers/iommu/iommu-pasid.c |  53 +
 drivers/iommu/iommu-pasid.h | 142 
 4 files changed, 204 insertions(+)
 create mode 100644 drivers/iommu/iommu-pasid.c
 create mode 100644 drivers/iommu/iommu-pasid.h

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index e751bb9958ba..8add90ba9b75 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -60,6 +60,14 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
 
 endmenu
 
+menu "Generic PASID table support"
+
+# Selected by the actual PASID table implementations
+config IOMMU_PASID_TABLE
+   bool
+
+endmenu
+
 config IOMMU_IOVA
tristate
 
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index f4324e29035e..338e59c93131 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_IOMMU_FAULT) += io-pgfault.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
+obj-$(CONFIG_IOMMU_PASID_TABLE) += iommu-pasid.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU) += of_iommu.o
 obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
diff --git a/drivers/iommu/iommu-pasid.c b/drivers/iommu/iommu-pasid.c
new file mode 100644
index ..6b21d369d514
--- /dev/null
+++ b/drivers/iommu/iommu-pasid.c
@@ -0,0 +1,53 @@
+/*
+ * PASID table management for the IOMMU
+ *
+ * Copyright (C) 2018 ARM Ltd.
+ * Author: Jean-Philippe Brucker 
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+
+#include "iommu-pasid.h"
+
+static const struct iommu_pasid_init_fns *
+pasid_table_init_fns[PASID_TABLE_NUM_FMTS] = {
+};
+
+struct iommu_pasid_table_ops *
+iommu_alloc_pasid_ops(enum iommu_pasid_table_fmt fmt,
+ struct iommu_pasid_table_cfg *cfg, void *cookie)
+{
+   struct iommu_pasid_table *table;
+   const struct iommu_pasid_init_fns *fns;
+
+   if (fmt >= PASID_TABLE_NUM_FMTS)
+   return NULL;
+
+   fns = pasid_table_init_fns[fmt];
+   if (!fns)
+   return NULL;
+
+   table = fns->alloc(cfg, cookie);
+   if (!table)
+   return NULL;
+
+   table->fmt = fmt;
+   table->cookie = cookie;
+   table->cfg = *cfg;
+
+   return >ops;
+}
+
+void iommu_free_pasid_ops(struct iommu_pasid_table_ops *ops)
+{
+   struct iommu_pasid_table *table;
+
+   if (!ops)
+   return;
+
+   table = container_of(ops, struct iommu_pasid_table, ops);
+   iommu_pasid_flush_all(table);
+   pasid_table_init_fns[table->fmt]->free(table);
+}
diff --git a/drivers/iommu/iommu-pasid.h b/drivers/iommu/iommu-pasid.h
new file mode 100644
index ..40a27d35c1e0
--- /dev/null
+++ b/drivers/iommu/iommu-pasid.h
@@ -0,0 +1,142 @@
+/*
+ * PASID table management for the IOMMU
+ *
+ * Copyright (C) 2017 ARM Ltd.
+ * Author: Jean-Philippe Brucker 
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+#ifndef __IOMMU_PASID_H
+#define __IOMMU_PASID_H
+
+#include 
+#include "io-pgtable.h"
+
+struct mm_struct;
+
+enum iommu_pasid_table_fmt {
+   PASID_TABLE_NUM_FMTS,
+};
+
+/**
+ * iommu_pasid_entry - Entry of a PASID table
+ *
+ * @token: architecture-specific data needed to uniquely identify the
+ * entry. Most notably used for TLB invalidation
+ */
+struct iommu_pasid_entry {
+   u64 tag;
+};
+
+/**
+ * iommu_pasid_table_ops - Operations on a PASID table
+ *
+ * @alloc_shared_entry:allocate an entry for sharing an mm (SVA)
+ * Returns the pointer to a new entry or an error
+ * @alloc_priv_entry:  allocate an entry for map/unmap operations
+ * Returns the pointer to a new entry or an error
+ * @free_entry:free an entry obtained with alloc_entry
+ * @set_entry: write PASID table entry
+ * @clear_entry:   clear PASID table entry
+ */
+struct iommu_pasid_table_ops {
+   struct iommu_pasid_entry *
+   (*alloc_shared_entry)(struct iommu_pasid_table_ops *ops,
+ struct mm_struct *mm);
+