Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
On 30/04/2019 21:24, Jacob Pan wrote: > On Thu, 25 Apr 2019 11:41:05 +0100 > Jean-Philippe Brucker wrote: > >> On 25/04/2019 11:17, Auger Eric wrote: +/** + * ioasid_alloc - Allocate an IOASID + * @set: the IOASID set + * @min: the minimum ID (inclusive) + * @max: the maximum ID (exclusive) + * @private: data private to the caller + * + * Allocate an ID between @min and @max (or %0 and %INT_MAX). Return the >>> I would remove "(or %0 and %INT_MAX)". >> >> Agreed, those where the default values of idr, but the xarray doesn't >> define a default max value. By the way, I do think squashing patches 6 >> and 7 would be better (keeping my SOB but you can change the author). >> > I will squash 6 and 7 in v3. I will just add my SOB but keep the > author if that is OK. Sure, that works Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
On Fri, 26 Apr 2019 05:21:15 -0700 Christoph Hellwig wrote: > On Fri, Apr 26, 2019 at 12:47:43PM +0100, Jean-Philippe Brucker wrote: > > >> On Tue, Apr 23, 2019 at 04:31:06PM -0700, Jacob Pan wrote: > > >>> The allocator doesn't really belong in drivers/iommu because > > >>> some drivers would like to allocate PASIDs for devices that > > >>> aren't managed by an IOMMU, using the same ID space as IOMMU. > > >>> It doesn't really belong in drivers/pci either since platform > > >>> device also support PASID. Add the allocator in > > >>> drivers/base. > > >> > > >> I'd still add it to drivers/iommu, just selectable separately > > >> from the core iommu code.. > > > Perhaps I misunderstood. If a driver wants to use IOASIDs w/o > > > iommu subsystem even turned on, how could selecting from the core > > > iommu code help? Could you elaborate on "selectable"? > > > > How about doing the same as CONFIG_IOMMU_IOVA? The code is in > > drivers/iommu but can be selected by non-IOMMU_API users, > > independently of CONFIG_IOMMU_SUPPORT. It's true that this > > allocator will mostly be used by IOMMU drivers. > > That is exactly what I meant! Make sense, will do that in the next round. Thanks! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
On Fri, Apr 26, 2019 at 12:47:43PM +0100, Jean-Philippe Brucker wrote: > >> On Tue, Apr 23, 2019 at 04:31:06PM -0700, Jacob Pan wrote: > >>> The allocator doesn't really belong in drivers/iommu because some > >>> drivers would like to allocate PASIDs for devices that aren't > >>> managed by an IOMMU, using the same ID space as IOMMU. It doesn't > >>> really belong in drivers/pci either since platform device also > >>> support PASID. Add the allocator in drivers/base. > >> > >> I'd still add it to drivers/iommu, just selectable separately from the > >> core iommu code.. > > Perhaps I misunderstood. If a driver wants to use IOASIDs w/o iommu > > subsystem even turned on, how could selecting from the core iommu code > > help? Could you elaborate on "selectable"? > > How about doing the same as CONFIG_IOMMU_IOVA? The code is in > drivers/iommu but can be selected by non-IOMMU_API users, independently > of CONFIG_IOMMU_SUPPORT. It's true that this allocator will mostly be > used by IOMMU drivers. That is exactly what I meant! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
On 25/04/2019 19:19, Jacob Pan wrote: > Hi Christoph, > > On Tue, 23 Apr 2019 23:19:03 -0700 > Christoph Hellwig wrote: > >> On Tue, Apr 23, 2019 at 04:31:06PM -0700, Jacob Pan wrote: >>> The allocator doesn't really belong in drivers/iommu because some >>> drivers would like to allocate PASIDs for devices that aren't >>> managed by an IOMMU, using the same ID space as IOMMU. It doesn't >>> really belong in drivers/pci either since platform device also >>> support PASID. Add the allocator in drivers/base. >> >> I'd still add it to drivers/iommu, just selectable separately from the >> core iommu code.. > Perhaps I misunderstood. If a driver wants to use IOASIDs w/o iommu > subsystem even turned on, how could selecting from the core iommu code > help? Could you elaborate on "selectable"? How about doing the same as CONFIG_IOMMU_IOVA? The code is in drivers/iommu but can be selected by non-IOMMU_API users, independently of CONFIG_IOMMU_SUPPORT. It's true that this allocator will mostly be used by IOMMU drivers. > From VT-d's perspective, PASIDs are only used with IOMMU on. Jean > knows other use cases. I know of one: the AMD GPU driver may use IOASID for context IDs, even if IOMMU is disabled. As I understand it, if IOMMU is enabled they need to use the same allocator as IOMMU since it's the same ID space. And I think it's more convenient to use the same allocation code in the GPU driver regardless of CONFIG_IOMMU_SUPPORT. See the previous discussion at https://www.spinics.net/lists/iommu/msg31200.html Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
Hi Christoph, On Tue, 23 Apr 2019 23:19:03 -0700 Christoph Hellwig wrote: > On Tue, Apr 23, 2019 at 04:31:06PM -0700, Jacob Pan wrote: > > The allocator doesn't really belong in drivers/iommu because some > > drivers would like to allocate PASIDs for devices that aren't > > managed by an IOMMU, using the same ID space as IOMMU. It doesn't > > really belong in drivers/pci either since platform device also > > support PASID. Add the allocator in drivers/base. > > I'd still add it to drivers/iommu, just selectable separately from the > core iommu code.. Perhaps I misunderstood. If a driver wants to use IOASIDs w/o iommu subsystem even turned on, how could selecting from the core iommu code help? Could you elaborate on "selectable"? >From VT-d's perspective, PASIDs are only used with IOMMU on. Jean knows other use cases. Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
On 25/04/2019 11:17, Auger Eric wrote: >> +/** >> + * ioasid_alloc - Allocate an IOASID >> + * @set: the IOASID set >> + * @min: the minimum ID (inclusive) >> + * @max: the maximum ID (exclusive) >> + * @private: data private to the caller >> + * >> + * Allocate an ID between @min and @max (or %0 and %INT_MAX). Return the > I would remove "(or %0 and %INT_MAX)". Agreed, those where the default values of idr, but the xarray doesn't define a default max value. By the way, I do think squashing patches 6 and 7 would be better (keeping my SOB but you can change the author). >> +typedef int (*ioasid_iter_t)(ioasid_t ioasid, void *private, void *data); > I don't see it used in this series. There used to be a "ioasid_for_each()", which isn't needed by anyone at the moment. This can be removed. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
Hi Jean-Philippe, Jacob,
On 4/24/19 1:31 AM, Jacob Pan wrote:
> From: Jean-Philippe Brucker
>
> Some devices might support multiple DMA address spaces, in particular
> those that have the PCI PASID feature. PASID (Process Address Space ID)
> allows to share process address spaces with devices (SVA), partition a
> device into VM-assignable entities (VFIO mdev) or simply provide
> multiple DMA address space to kernel drivers. Add a global PASID
> allocator usable by different drivers at the same time. Name it I/O ASID
> to avoid confusion with ASIDs allocated by arch code, which are usually
> a separate ID space.
>
> The IOASID space is global. Each device can have its own PASID space,
> but by convention the IOMMU ended up having a global PASID space, so
> that with SVA, each mm_struct is associated to a single PASID.
>
> The allocator doesn't really belong in drivers/iommu because some
> drivers would like to allocate PASIDs for devices that aren't managed by
> an IOMMU, using the same ID space as IOMMU. It doesn't really belong in
> drivers/pci either since platform device also support PASID. Add the
> allocator in drivers/base.
>
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/base/Kconfig | 6 +++
> drivers/base/Makefile | 1 +
> drivers/base/ioasid.c | 106
> +
> include/linux/ioasid.h | 40 +++
> 4 files changed, 153 insertions(+)
> create mode 100644 drivers/base/ioasid.c
> create mode 100644 include/linux/ioasid.h
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 059700e..47c1348 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -182,6 +182,12 @@ config DMA_SHARED_BUFFER
> APIs extension; the file's descriptor can then be passed on to other
> driver.
>
> +config IOASID
> + bool
> + help
> + Enable the I/O Address Space ID allocator. A single ID space shared
> + between different users.
> +
> config DMA_FENCE_TRACE
> bool "Enable verbose DMA_FENCE_TRACE messages"
> depends on DMA_SHARED_BUFFER
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 1574520..aafa2ac 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_PINCTRL) += pinctrl.o
> obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
> obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> +obj-$(CONFIG_IOASID) += ioasid.o
>
> obj-y+= test/
>
> diff --git a/drivers/base/ioasid.c b/drivers/base/ioasid.c
> new file mode 100644
> index 000..cf122b2
> --- /dev/null
> +++ b/drivers/base/ioasid.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * I/O Address Space ID allocator. There is one global IOASID space, split
> into
> + * subsets. Users create a subset with DECLARE_IOASID_SET, then allocate and
> + * free IOASIDs with ioasid_alloc and ioasid_free.
> + */
> +#include
> +#include
> +#include
> +#include
> +
> +struct ioasid_data {
> + ioasid_t id;
> + struct ioasid_set *set;
> + void *private;
> + struct rcu_head rcu;
> +};
> +
> +static DEFINE_IDR(ioasid_idr);
> +
> +/**
> + * ioasid_alloc - Allocate an IOASID
> + * @set: the IOASID set
> + * @min: the minimum ID (inclusive)
> + * @max: the maximum ID (exclusive)
> + * @private: data private to the caller
> + *
> + * Allocate an ID between @min and @max (or %0 and %INT_MAX). Return the
I would remove "(or %0 and %INT_MAX)".
> + * allocated ID on success, or INVALID_IOASID on failure. The @private
> pointer
> + * is stored internally and can be retrieved with ioasid_find().
> + */
> +ioasid_t ioasid_alloc(struct ioasid_set *set, ioasid_t min, ioasid_t max,
> + void *private)
> +{
> + int id = -1;
> + struct ioasid_data *data;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return INVALID_IOASID;
> +
> + data->set = set;
> + data->private = private;
> +
> + idr_preload(GFP_KERNEL);
> + idr_lock(_idr);
> + data->id = id = idr_alloc(_idr, data, min, max, GFP_ATOMIC);
> + idr_unlock(_idr);
> + idr_preload_end();
> +
> + if (id < 0) {
> + kfree(data);
> + return INVALID_IOASID;
> + }
> + return id;
> +}
> +EXPORT_SYMBOL_GPL(ioasid_alloc);
> +
> +/**
> + * ioasid_free - Free an IOASID
> + * @ioasid: the ID to remove
> + */
> +void ioasid_free(ioasid_t ioasid)
> +{
> + struct ioasid_data *ioasid_data;
> +
> + idr_lock(_idr);
> + ioasid_data = idr_remove(_idr, ioasid);
> + idr_unlock(_idr);
> +
> + if (ioasid_data)
> + kfree_rcu(ioasid_data, rcu);
> +}
> +EXPORT_SYMBOL_GPL(ioasid_free);
> +
> +/**
> + * ioasid_find - Find IOASID data
> + * @set: the IOASID set
> + * @ioasid: the IOASID to find
> + * @getter: function to call on the found object
> + *
> + * The optional getter
Re: [PATCH v2 06/19] drivers core: Add I/O ASID allocator
On Tue, Apr 23, 2019 at 04:31:06PM -0700, Jacob Pan wrote: > The allocator doesn't really belong in drivers/iommu because some > drivers would like to allocate PASIDs for devices that aren't managed by > an IOMMU, using the same ID space as IOMMU. It doesn't really belong in > drivers/pci either since platform device also support PASID. Add the > allocator in drivers/base. I'd still add it to drivers/iommu, just selectable separately from the core iommu code.. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 06/19] drivers core: Add I/O ASID allocator
From: Jean-Philippe Brucker
Some devices might support multiple DMA address spaces, in particular
those that have the PCI PASID feature. PASID (Process Address Space ID)
allows to share process address spaces with devices (SVA), partition a
device into VM-assignable entities (VFIO mdev) or simply provide
multiple DMA address space to kernel drivers. Add a global PASID
allocator usable by different drivers at the same time. Name it I/O ASID
to avoid confusion with ASIDs allocated by arch code, which are usually
a separate ID space.
The IOASID space is global. Each device can have its own PASID space,
but by convention the IOMMU ended up having a global PASID space, so
that with SVA, each mm_struct is associated to a single PASID.
The allocator doesn't really belong in drivers/iommu because some
drivers would like to allocate PASIDs for devices that aren't managed by
an IOMMU, using the same ID space as IOMMU. It doesn't really belong in
drivers/pci either since platform device also support PASID. Add the
allocator in drivers/base.
Signed-off-by: Jean-Philippe Brucker
---
drivers/base/Kconfig | 6 +++
drivers/base/Makefile | 1 +
drivers/base/ioasid.c | 106 +
include/linux/ioasid.h | 40 +++
4 files changed, 153 insertions(+)
create mode 100644 drivers/base/ioasid.c
create mode 100644 include/linux/ioasid.h
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 059700e..47c1348 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -182,6 +182,12 @@ config DMA_SHARED_BUFFER
APIs extension; the file's descriptor can then be passed on to other
driver.
+config IOASID
+ bool
+ help
+ Enable the I/O Address Space ID allocator. A single ID space shared
+ between different users.
+
config DMA_FENCE_TRACE
bool "Enable verbose DMA_FENCE_TRACE messages"
depends on DMA_SHARED_BUFFER
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 1574520..aafa2ac 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_PINCTRL) += pinctrl.o
obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
+obj-$(CONFIG_IOASID) += ioasid.o
obj-y += test/
diff --git a/drivers/base/ioasid.c b/drivers/base/ioasid.c
new file mode 100644
index 000..cf122b2
--- /dev/null
+++ b/drivers/base/ioasid.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * I/O Address Space ID allocator. There is one global IOASID space, split into
+ * subsets. Users create a subset with DECLARE_IOASID_SET, then allocate and
+ * free IOASIDs with ioasid_alloc and ioasid_free.
+ */
+#include
+#include
+#include
+#include
+
+struct ioasid_data {
+ ioasid_t id;
+ struct ioasid_set *set;
+ void *private;
+ struct rcu_head rcu;
+};
+
+static DEFINE_IDR(ioasid_idr);
+
+/**
+ * ioasid_alloc - Allocate an IOASID
+ * @set: the IOASID set
+ * @min: the minimum ID (inclusive)
+ * @max: the maximum ID (exclusive)
+ * @private: data private to the caller
+ *
+ * Allocate an ID between @min and @max (or %0 and %INT_MAX). Return the
+ * allocated ID on success, or INVALID_IOASID on failure. The @private pointer
+ * is stored internally and can be retrieved with ioasid_find().
+ */
+ioasid_t ioasid_alloc(struct ioasid_set *set, ioasid_t min, ioasid_t max,
+ void *private)
+{
+ int id = -1;
+ struct ioasid_data *data;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return INVALID_IOASID;
+
+ data->set = set;
+ data->private = private;
+
+ idr_preload(GFP_KERNEL);
+ idr_lock(_idr);
+ data->id = id = idr_alloc(_idr, data, min, max, GFP_ATOMIC);
+ idr_unlock(_idr);
+ idr_preload_end();
+
+ if (id < 0) {
+ kfree(data);
+ return INVALID_IOASID;
+ }
+ return id;
+}
+EXPORT_SYMBOL_GPL(ioasid_alloc);
+
+/**
+ * ioasid_free - Free an IOASID
+ * @ioasid: the ID to remove
+ */
+void ioasid_free(ioasid_t ioasid)
+{
+ struct ioasid_data *ioasid_data;
+
+ idr_lock(_idr);
+ ioasid_data = idr_remove(_idr, ioasid);
+ idr_unlock(_idr);
+
+ if (ioasid_data)
+ kfree_rcu(ioasid_data, rcu);
+}
+EXPORT_SYMBOL_GPL(ioasid_free);
+
+/**
+ * ioasid_find - Find IOASID data
+ * @set: the IOASID set
+ * @ioasid: the IOASID to find
+ * @getter: function to call on the found object
+ *
+ * The optional getter function allows to take a reference to the found object
+ * under the rcu lock. The function can also check if the object is still
valid:
+ * if @getter returns false, then the object is invalid and NULL is returned.
+ *
+ * If the IOASID has been allocated for this set, return the private pointer
+ * passed to ioasid_alloc. Otherwise
