Re: [PATCH v4 12/22] iommu: Add I/O ASID allocator
Hi Jonathan, Thanks for the review, comments inline below. I saw Jean already took in changes based on your review in his sva/api tree. This is just some additions. On Tue, 18 Jun 2019 17:50:59 +0100 Jonathan Cameron wrote: > On Sun, 9 Jun 2019 06:44:12 -0700 > 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 is primarily used by IOMMU subsystem but in rare > > occasions drivers would like to allocate PASIDs for devices that > > aren't managed by an IOMMU, using the same ID space as IOMMU. > > > > There are two types of allocators: > > 1. default allocator - Always available, uses an XArray to track > > 2. custom allocators - Can be registered at runtime, take precedence > > over the default allocator. > > > > Custom allocators have these attributes: > > - provides platform specific alloc()/free() functions with private > > data. > > - allocation results lookup are not provided by the allocator, > > lookup request must be done by the IOASID framework by its own > > XArray. > > - allocators can be unregistered at runtime, either fallback to the > > next custom allocator or to the default allocator. > > What is the usecase for having a 'stack' of custom allocators? > mainly for hotplug. If a vIOMMU provides custom allocation service to a guest and got hot removed we can use other custom allocators from the stack. > > - custom allocators can share the same set of alloc()/free() > > helpers, in this case they also share the same IOASID space, thus > > the same XArray. > > - switching between allocators requires all outstanding IOASIDs to > > be freed unless the two allocators share the same alloc()/free() > > helpers. > In general this approach has a lot of features where the > justification is missing from this particular patch. It may be > useful to add some more background to this intro? > Good point. Here are the use cases as complexity grow. 1. native (host kernel) PASID/IOASID allocation, this will only use default allocator, users are IOMMU driver or any device drivers. 2. guest with a single vIOMMU. vIOMMU driver will register a custom allocator which eventually calls into the host to allocate system-wide PASID. 3. guest with two or more vIOMMUs but share the same backend to allocate system-wide PASID. IOASID code detects the sharing based on alloc/free ops. 4. guest with two or more vIOMMUs each allocate guest-wide PASIDs. 5. In case of hot plug of vIOMMU, the active custom allocator is chosen from a list established during registration. > > > > Signed-off-by: Jean-Philippe Brucker > > Signed-off-by: Jacob Pan > > Link: https://lkml.org/lkml/2019/4/26/462 > > Various comments inline. Given the several cups of coffee this took > to review I may well have misunderstood everything ;) > > Jonathan > > --- > > drivers/iommu/Kconfig | 8 + > > drivers/iommu/Makefile | 1 + > > drivers/iommu/ioasid.c | 427 > > + > > include/linux/ioasid.h | 74 + 4 files changed, 510 > > insertions(+) create mode 100644 drivers/iommu/ioasid.c > > create mode 100644 include/linux/ioasid.h > > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > > index 83664db..c40c4b5 100644 > > --- a/drivers/iommu/Kconfig > > +++ b/drivers/iommu/Kconfig > > @@ -3,6 +3,13 @@ > > config IOMMU_IOVA > > tristate > > > > +# The IOASID allocator may also be used by non-IOMMU_API users > > +config IOASID > > + tristate > > + help > > + Enable the I/O Address Space ID allocator. A single ID > > space shared > > + between different users. > > + > > # IOMMU_API always gets selected by whoever wants it. > > config IOMMU_API > > bool > > @@ -207,6 +214,7 @@ config INTEL_IOMMU_SVM > > depends on INTEL_IOMMU && X86 > > select PCI_PASID > > select MMU_NOTIFIER > > + select IOASID > > help > > Shared Virtual Memory (SVM) provides a facility for > > devices to access DMA resources through process address space by > > diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile > > index 8c71a15..0efac6f 100644 > > --- a/drivers/iommu/Makefile > > +++ b/drivers/iommu/Makefile > > @@ -7,6 +7,7 @@ obj-$(CONFIG_IOMMU_DMA) +=
Re: [PATCH v4 12/22] iommu: Add I/O ASID allocator
On Sun, 9 Jun 2019 06:44:12 -0700
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 is primarily used by IOMMU subsystem but in rare occasions
> drivers would like to allocate PASIDs for devices that aren't managed by
> an IOMMU, using the same ID space as IOMMU.
>
> There are two types of allocators:
> 1. default allocator - Always available, uses an XArray to track
> 2. custom allocators - Can be registered at runtime, take precedence
> over the default allocator.
>
> Custom allocators have these attributes:
> - provides platform specific alloc()/free() functions with private data.
> - allocation results lookup are not provided by the allocator, lookup
> request must be done by the IOASID framework by its own XArray.
> - allocators can be unregistered at runtime, either fallback to the next
> custom allocator or to the default allocator.
What is the usecase for having a 'stack' of custom allocators?
> - custom allocators can share the same set of alloc()/free() helpers, in
> this case they also share the same IOASID space, thus the same XArray.
> - switching between allocators requires all outstanding IOASIDs to be
> freed unless the two allocators share the same alloc()/free() helpers.
In general this approach has a lot of features where the justification is
missing from this particular patch. It may be useful to add some
more background to this intro?
>
> Signed-off-by: Jean-Philippe Brucker
> Signed-off-by: Jacob Pan
> Link: https://lkml.org/lkml/2019/4/26/462
Various comments inline. Given the several cups of coffee this took to
review I may well have misunderstood everything ;)
Jonathan
> ---
> drivers/iommu/Kconfig | 8 +
> drivers/iommu/Makefile | 1 +
> drivers/iommu/ioasid.c | 427
> +
> include/linux/ioasid.h | 74 +
> 4 files changed, 510 insertions(+)
> create mode 100644 drivers/iommu/ioasid.c
> create mode 100644 include/linux/ioasid.h
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 83664db..c40c4b5 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -3,6 +3,13 @@
> config IOMMU_IOVA
> tristate
>
> +# The IOASID allocator may also be used by non-IOMMU_API users
> +config IOASID
> + tristate
> + help
> + Enable the I/O Address Space ID allocator. A single ID space shared
> + between different users.
> +
> # IOMMU_API always gets selected by whoever wants it.
> config IOMMU_API
> bool
> @@ -207,6 +214,7 @@ config INTEL_IOMMU_SVM
> depends on INTEL_IOMMU && X86
> select PCI_PASID
> select MMU_NOTIFIER
> + select IOASID
> help
> Shared Virtual Memory (SVM) provides a facility for devices
> to access DMA resources through process address space by
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 8c71a15..0efac6f 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_IOMMU_DMA) += dma-iommu.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_IOASID) += ioasid.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/ioasid.c b/drivers/iommu/ioasid.c
> new file mode 100644
> index 000..0919b70
> --- /dev/null
> +++ b/drivers/iommu/ioasid.c
> @@ -0,0 +1,427 @@
> +// 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.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct ioasid_data {
> + ioasid_t id;
> + struct ioasid_set *set;
> + void *private;
> + struct rcu_head rcu;
> +};
> +
> +/*
> + * struct ioasid_allocator_data - Internal data structure to hold information
> + * about an allocator. There are two types of allocators:
> + *
> + * -
[PATCH v4 12/22] iommu: 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 is primarily used by IOMMU subsystem but in rare occasions
drivers would like to allocate PASIDs for devices that aren't managed by
an IOMMU, using the same ID space as IOMMU.
There are two types of allocators:
1. default allocator - Always available, uses an XArray to track
2. custom allocators - Can be registered at runtime, take precedence
over the default allocator.
Custom allocators have these attributes:
- provides platform specific alloc()/free() functions with private data.
- allocation results lookup are not provided by the allocator, lookup
request must be done by the IOASID framework by its own XArray.
- allocators can be unregistered at runtime, either fallback to the next
custom allocator or to the default allocator.
- custom allocators can share the same set of alloc()/free() helpers, in
this case they also share the same IOASID space, thus the same XArray.
- switching between allocators requires all outstanding IOASIDs to be
freed unless the two allocators share the same alloc()/free() helpers.
Signed-off-by: Jean-Philippe Brucker
Signed-off-by: Jacob Pan
Link: https://lkml.org/lkml/2019/4/26/462
---
drivers/iommu/Kconfig | 8 +
drivers/iommu/Makefile | 1 +
drivers/iommu/ioasid.c | 427 +
include/linux/ioasid.h | 74 +
4 files changed, 510 insertions(+)
create mode 100644 drivers/iommu/ioasid.c
create mode 100644 include/linux/ioasid.h
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 83664db..c40c4b5 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -3,6 +3,13 @@
config IOMMU_IOVA
tristate
+# The IOASID allocator may also be used by non-IOMMU_API users
+config IOASID
+ tristate
+ help
+ Enable the I/O Address Space ID allocator. A single ID space shared
+ between different users.
+
# IOMMU_API always gets selected by whoever wants it.
config IOMMU_API
bool
@@ -207,6 +214,7 @@ config INTEL_IOMMU_SVM
depends on INTEL_IOMMU && X86
select PCI_PASID
select MMU_NOTIFIER
+ select IOASID
help
Shared Virtual Memory (SVM) provides a facility for devices
to access DMA resources through process address space by
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 8c71a15..0efac6f 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_IOMMU_DMA) += dma-iommu.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_IOASID) += ioasid.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/ioasid.c b/drivers/iommu/ioasid.c
new file mode 100644
index 000..0919b70
--- /dev/null
+++ b/drivers/iommu/ioasid.c
@@ -0,0 +1,427 @@
+// 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.
+ */
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+#include
+#include
+
+struct ioasid_data {
+ ioasid_t id;
+ struct ioasid_set *set;
+ void *private;
+ struct rcu_head rcu;
+};
+
+/*
+ * struct ioasid_allocator_data - Internal data structure to hold information
+ * about an allocator. There are two types of allocators:
+ *
+ * - Default allocator always has its own XArray to track the IOASIDs
allocated.
+ * - Custom allocators may share allocation helpers with different private
data.
+ * Custom allocators share the same helper functions also share the same
+ * XArray.
+ * Rules:
+ * 1. Default allocator is always available, not dynamically registered. This
is
+ *to prevent race conditions with early boot code that want to register
+ *custom allocators or allocate IOASIDs.
+ * 2. Custom allocators take precedence over the default allocator.
+ * 3. When all custom allocators sharing the same helper functions are
+ *unregistered (e.g. due
