Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1496515 > unrolled thread

[PATCH v13 00/15] KVM PCIe/MSI passthrough on ARM/ARM64

Started byEric Auger <eric.auger@redhat.com>
First post2016-10-06 11:40 +0200
Last post2016-10-07 19:20 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v13 00/15] KVM PCIe/MSI passthrough on ARM/ARM64 Eric Auger <eric.auger@redhat.com> - 2016-10-06 11:40 +0200
    [PATCH v13 14/15] iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP Eric Auger <eric.auger@redhat.com> - 2016-10-06 11:40 +0200
    [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API Eric Auger <eric.auger@redhat.com> - 2016-10-06 11:40 +0200
      Re: [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API Alex Williamson <alex.williamson@redhat.com> - 2016-10-06 22:20 +0200
        Re: [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API Auger Eric <eric.auger@redhat.com> - 2016-10-07 19:20 +0200

#1496515 — [PATCH v13 00/15] KVM PCIe/MSI passthrough on ARM/ARM64

FromEric Auger <eric.auger@redhat.com>
Date2016-10-06 11:40 +0200
Subject[PATCH v13 00/15] KVM PCIe/MSI passthrough on ARM/ARM64
Message-ID<spcxk-68N-19@gated-at.bofh.it>
Following Robin's series [1], addressing MSI IOMMU mapping for devices
attached to a DMA ops domain, quite a lot of changes (and simplifications)
were induced with respect to the v12 iteration:

- msi-iommu API role now is handled at dma-iommu level
- MSI doorbell registration API still is used for security assessment
  and doorbell overall iommu page size computation.
- MSI layer part II is not needed anymore since mapping directly is
  done in the irqchip compose callback.

The VFIO user API and VFIO layer changes have not changed though. All the
patches now are in the same series.

Tested on AMD Overdrive (single GICv2m frame) with I350 VF assignment.

dependency:
the series depends on Robin's generic-v7 branch:
[1] [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
http://www.spinics.net/lists/arm-kernel/msg531110.html

Best Regards

Eric

Git: complete series available at
https://github.com/eauger/linux/tree/generic-v7-pcie-passthru-v13

the above branch includes a temporary patch to work around a ThunderX pci
bus reset crash (which I think unrelated to this series):
"vfio: pci: HACK! workaround thunderx pci_try_reset_bus crash"
Do not take this one for other platforms.

Eric Auger (14):
  iommu: Introduce DOMAIN_ATTR_MSI_GEOMETRY
  iommu/arm-smmu: Initialize the msi geometry
  genirq/msi: Introduce the MSI doorbell API
  genirq/msi: msi_doorbell_calc_pages
  irqchip/gic-v2m: Register the MSI doorbell
  irqchip/gicv3-its: Register the MSI doorbell
  vfio: Introduce a vfio_dma type field
  vfio/type1: vfio_find_dma accepting a type argument
  vfio/type1: Implement recursive vfio_find_dma_from_node
  vfio/type1: Handle unmap/unpin and replay for VFIO_IOVA_RESERVED slots
  vfio: Allow reserved msi iova registration
  vfio/type1: Check doorbell safety
  iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP
  vfio/type1: Return the MSI geometry through VFIO_IOMMU_GET_INFO
    capability chains

Robin Murphy (1):
  iommu/dma: Allow MSI-only cookies

 drivers/iommu/Kconfig            |   2 +
 drivers/iommu/arm-smmu-v3.c      |   5 +-
 drivers/iommu/arm-smmu.c         |   6 +-
 drivers/iommu/dma-iommu.c        |  40 +++++++
 drivers/iommu/iommu.c            |   5 +
 drivers/irqchip/irq-gic-v2m.c    |  11 +-
 drivers/irqchip/irq-gic-v3-its.c |  14 +++
 drivers/vfio/Kconfig             |   1 +
 drivers/vfio/vfio_iommu_type1.c  | 244 ++++++++++++++++++++++++++++++++++++---
 include/linux/dma-iommu.h        |   9 ++
 include/linux/iommu.h            |  14 +++
 include/linux/msi-doorbell.h     |  92 +++++++++++++++
 include/uapi/linux/vfio.h        |  42 ++++++-
 kernel/irq/Kconfig               |   4 +
 kernel/irq/Makefile              |   1 +
 kernel/irq/msi-doorbell.c        | 162 ++++++++++++++++++++++++++
 16 files changed, 633 insertions(+), 19 deletions(-)
 create mode 100644 include/linux/msi-doorbell.h
 create mode 100644 kernel/irq/msi-doorbell.c

-- 
1.9.1

[toc] | [next] | [standalone]

#1496516 — [PATCH v13 14/15] iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP

FromEric Auger <eric.auger@redhat.com>
Date2016-10-06 11:40 +0200
Subject[PATCH v13 14/15] iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP
Message-ID<spdjJ-6HB-63@gated-at.bofh.it>
In reply to#1496515
Do not advertise IOMMU_CAP_INTR_REMAP for arm-smmu(-v3). Indeed the
irq_remapping capability is abstracted on irqchip side for ARM as
opposed to Intel IOMMU featuring IRQ remapping HW.

So to check IRQ remapping capability, the msi domain needs to be
checked instead.

This commit affects platform and PCIe device assignment use cases
on any platform featuring an unsafe MSI controller (currently the
ARM GICv2m). For those platforms the VFIO module must be loaded with
allow_unsafe_interrupts set to 1.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v9 -> v10:
- reword the commit message (allow_unsafe_interrupts)
---
 drivers/iommu/arm-smmu-v3.c | 3 ++-
 drivers/iommu/arm-smmu.c    | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f82eec3..c0a34be 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1371,7 +1371,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 	case IOMMU_CAP_CACHE_COHERENCY:
 		return true;
 	case IOMMU_CAP_INTR_REMAP:
-		return true; /* MSIs are just memory writes */
+		/* interrupt translation handled at MSI controller level */
+		return false;
 	case IOMMU_CAP_NOEXEC:
 		return true;
 	default:
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 97ff1b4..0c0cd9e 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1361,7 +1361,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 		 */
 		return true;
 	case IOMMU_CAP_INTR_REMAP:
-		return true; /* MSIs are just memory writes */
+		/* interrupt translation handled at MSI controller level */
+		return false;
 	case IOMMU_CAP_NOEXEC:
 		return true;
 	default:
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1496520 — [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API

FromEric Auger <eric.auger@redhat.com>
Date2016-10-06 11:40 +0200
Subject[PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API
Message-ID<spdjJ-6HB-73@gated-at.bofh.it>
In reply to#1496515
We introduce a new msi-doorbell API that allows msi controllers
to allocate and register their doorbells. This is useful when
those doorbells are likely to be iommu mapped (typically on ARM).
The VFIO layer will need to gather information about those doorbells:
whether they are safe (ie. they implement irq remapping) and how
many IOMMU pages are requested to map all of them.

This patch first introduces the dedicated msi_doorbell_info struct
and the registration/unregistration functions.

A doorbell region is characterized by its physical address base, size,
and whether it its safe (ie. it implements IRQ remapping). A doorbell
can be per-cpu of global. We currently only care about global doorbells.

A function returns whether all doorbells are safe.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
v12 -> v13:
- directly select MSI_DOORBELL in ARM_SMMU and ARM_SMMU_V3 configs
- remove prot attribute
- move msi_doorbell_info struct definition in msi-doorbell.c
- change the commit title
- change proto of the registration function
- msi_doorbell_safe now in this patch

v11 -> v12:
- rename irqchip_doorbell into msi_doorbell, irqchip_doorbell_list
  into msi_doorbell_list and irqchip_doorbell_mutex into
  msi_doorbell_mutex
- fix style issues: align msi_doorbell struct members, kernel-doc comments
- use kzalloc
- use container_of in msi_doorbell_unregister_global
- compute nb_unsafe_doorbells on registration/unregistration
- registration simply returns NULL if allocation failed

v10 -> v11:
- remove void *chip_data argument from register/unregister function
- remove lookup funtions since we restored the struct irq_chip
  msi_doorbell_info ops to realize this function
- reword commit message and title

Conflicts:
	kernel/irq/Makefile

Conflicts:
	drivers/iommu/Kconfig
---
 drivers/iommu/Kconfig        |  2 +
 include/linux/msi-doorbell.h | 77 ++++++++++++++++++++++++++++++++++
 kernel/irq/Kconfig           |  4 ++
 kernel/irq/Makefile          |  1 +
 kernel/irq/msi-doorbell.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 182 insertions(+)
 create mode 100644 include/linux/msi-doorbell.h
 create mode 100644 kernel/irq/msi-doorbell.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 8ee54d7..0cc7fac 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -297,6 +297,7 @@ config SPAPR_TCE_IOMMU
 config ARM_SMMU
 	bool "ARM Ltd. System MMU (SMMU) Support"
 	depends on (ARM64 || ARM) && MMU
+	select MSI_DOORBELL
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU if ARM
@@ -310,6 +311,7 @@ config ARM_SMMU
 config ARM_SMMU_V3
 	bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
 	depends on ARM64
+	select MSI_DOORBELL
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select GENERIC_MSI_IRQ_DOMAIN
diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
new file mode 100644
index 0000000..c18a382
--- /dev/null
+++ b/include/linux/msi-doorbell.h
@@ -0,0 +1,77 @@
+/*
+ * API to register/query MSI doorbells likely to be IOMMU mapped
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _LINUX_MSI_DOORBELL_H
+#define _LINUX_MSI_DOORBELL_H
+
+struct msi_doorbell_info;
+
+#ifdef CONFIG_MSI_DOORBELL
+
+/**
+ * msi_doorbell_register - allocate and register a global doorbell
+ * @base: physical base address of the global doorbell
+ * @size: size of the global doorbell
+ * @prot: protection/memory attributes
+ * @safe: true is irq_remapping implemented for this doorbell
+ * @dbinfo: returned doorbell info
+ *
+ * Return: 0 on success, -ENOMEM on allocation failure
+ */
+int msi_doorbell_register_global(phys_addr_t base, size_t size,
+				 bool safe,
+				 struct msi_doorbell_info **dbinfo);
+
+/**
+ * msi_doorbell_unregister_global - unregister a global doorbell
+ * @db: doorbell info to unregister
+ *
+ * remove the doorbell descriptor from the list of registered doorbells
+ * and deallocates it
+ */
+void msi_doorbell_unregister_global(struct msi_doorbell_info *db);
+
+/**
+ * msi_doorbell_safe - return whether all registered doorbells are safe
+ *
+ * Safe doorbells are those which implement irq remapping
+ * Return: true if all doorbells are safe, false otherwise
+ */
+bool msi_doorbell_safe(void);
+
+#else
+
+static inline int
+msi_doorbell_register_global(phys_addr_t base, size_t size,
+			     int prot, bool safe,
+			     struct msi_doorbell_info **dbinfo)
+{
+	*dbinfo = NULL;
+	return 0;
+}
+
+static inline void
+msi_doorbell_unregister_global(struct msi_doorbell_info *db) {}
+
+static inline bool msi_doorbell_safe(void)
+{
+	return true;
+}
+#endif /* CONFIG_MSI_DOORBELL */
+
+#endif
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 3bbfd6a..d4faaaa 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -72,6 +72,10 @@ config GENERIC_IRQ_IPI
 config GENERIC_MSI_IRQ
 	bool
 
+# MSI doorbell support (for doorbell IOMMU mapping)
+config MSI_DOORBELL
+	bool
+
 # Generic MSI hierarchical interrupt domain support
 config GENERIC_MSI_IRQ_DOMAIN
 	bool
diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
index 1d3ee31..5b04dd1 100644
--- a/kernel/irq/Makefile
+++ b/kernel/irq/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_PM_SLEEP) += pm.o
 obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
 obj-$(CONFIG_GENERIC_IRQ_IPI) += ipi.o
 obj-$(CONFIG_SMP) += affinity.o
+obj-$(CONFIG_MSI_DOORBELL) += msi-doorbell.o
diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c
new file mode 100644
index 0000000..60a262a
--- /dev/null
+++ b/kernel/irq/msi-doorbell.c
@@ -0,0 +1,98 @@
+/*
+ * API to register/query MSI doorbells likely to be IOMMU mapped
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ * Author: Eric Auger <eric.auger@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/slab.h>
+#include <linux/irq.h>
+#include <linux/msi-doorbell.h>
+
+/**
+ * struct msi_doorbell_info - MSI doorbell region descriptor
+ * @percpu_doorbells: per cpu doorbell base address
+ * @global_doorbell: base address of the doorbell
+ * @doorbell_is_percpu: is the doorbell per cpu or global?
+ * @safe: true if irq remapping is implemented
+ * @size: size of the doorbell
+ */
+struct msi_doorbell_info {
+	union {
+		phys_addr_t __percpu    *percpu_doorbells;
+		phys_addr_t             global_doorbell;
+	};
+	bool    doorbell_is_percpu;
+	bool    safe;
+	size_t  size;
+};
+
+struct msi_doorbell {
+	struct msi_doorbell_info	info;
+	struct list_head		next;
+};
+
+/* list of registered MSI doorbells */
+static LIST_HEAD(msi_doorbell_list);
+
+/* counts the number of unsafe registered doorbells */
+static uint nb_unsafe_doorbells;
+
+/* protects the list and nb__unsafe_doorbells */
+static DEFINE_MUTEX(msi_doorbell_mutex);
+
+int msi_doorbell_register_global(phys_addr_t base, size_t size, bool safe,
+				 struct msi_doorbell_info **dbinfo)
+{
+	struct msi_doorbell *db;
+
+	db = kzalloc(sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return -ENOMEM;
+
+	db->info.global_doorbell = base;
+	db->info.size = size;
+	db->info.safe = safe;
+
+	mutex_lock(&msi_doorbell_mutex);
+	list_add(&db->next, &msi_doorbell_list);
+	if (!db->info.safe)
+		nb_unsafe_doorbells++;
+	mutex_unlock(&msi_doorbell_mutex);
+	*dbinfo = &db->info;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_register_global);
+
+void msi_doorbell_unregister_global(struct msi_doorbell_info *dbinfo)
+{
+	struct msi_doorbell *db;
+
+	db = container_of(dbinfo, struct msi_doorbell, info);
+
+	mutex_lock(&msi_doorbell_mutex);
+	list_del(&db->next);
+	if (!db->info.safe)
+		nb_unsafe_doorbells--;
+	mutex_unlock(&msi_doorbell_mutex);
+	kfree(db);
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global);
+
+bool msi_doorbell_safe(void)
+{
+	return !nb_unsafe_doorbells;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_safe);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1496847 — Re: [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API

FromAlex Williamson <alex.williamson@redhat.com>
Date2016-10-06 22:20 +0200
SubjectRe: [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API
Message-ID<spnj4-5PE-17@gated-at.bofh.it>
In reply to#1496520
On Thu,  6 Oct 2016 08:45:20 +0000
Eric Auger <eric.auger@redhat.com> wrote:

> We introduce a new msi-doorbell API that allows msi controllers
> to allocate and register their doorbells. This is useful when
> those doorbells are likely to be iommu mapped (typically on ARM).
> The VFIO layer will need to gather information about those doorbells:
> whether they are safe (ie. they implement irq remapping) and how
> many IOMMU pages are requested to map all of them.
> 
> This patch first introduces the dedicated msi_doorbell_info struct
> and the registration/unregistration functions.
> 
> A doorbell region is characterized by its physical address base, size,
> and whether it its safe (ie. it implements IRQ remapping). A doorbell
> can be per-cpu of global. We currently only care about global doorbells.
                 ^^ s/of/or/

> 
> A function returns whether all doorbells are safe.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v12 -> v13:
> - directly select MSI_DOORBELL in ARM_SMMU and ARM_SMMU_V3 configs
> - remove prot attribute
> - move msi_doorbell_info struct definition in msi-doorbell.c
> - change the commit title
> - change proto of the registration function
> - msi_doorbell_safe now in this patch
> 
> v11 -> v12:
> - rename irqchip_doorbell into msi_doorbell, irqchip_doorbell_list
>   into msi_doorbell_list and irqchip_doorbell_mutex into
>   msi_doorbell_mutex
> - fix style issues: align msi_doorbell struct members, kernel-doc comments
> - use kzalloc
> - use container_of in msi_doorbell_unregister_global
> - compute nb_unsafe_doorbells on registration/unregistration
> - registration simply returns NULL if allocation failed
> 
> v10 -> v11:
> - remove void *chip_data argument from register/unregister function
> - remove lookup funtions since we restored the struct irq_chip
>   msi_doorbell_info ops to realize this function
> - reword commit message and title
> 
> Conflicts:
> 	kernel/irq/Makefile
> 
> Conflicts:
> 	drivers/iommu/Kconfig
> ---
>  drivers/iommu/Kconfig        |  2 +
>  include/linux/msi-doorbell.h | 77 ++++++++++++++++++++++++++++++++++
>  kernel/irq/Kconfig           |  4 ++
>  kernel/irq/Makefile          |  1 +
>  kernel/irq/msi-doorbell.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 182 insertions(+)
>  create mode 100644 include/linux/msi-doorbell.h
>  create mode 100644 kernel/irq/msi-doorbell.c
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 8ee54d7..0cc7fac 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -297,6 +297,7 @@ config SPAPR_TCE_IOMMU
>  config ARM_SMMU
>  	bool "ARM Ltd. System MMU (SMMU) Support"
>  	depends on (ARM64 || ARM) && MMU
> +	select MSI_DOORBELL
>  	select IOMMU_API
>  	select IOMMU_IO_PGTABLE_LPAE
>  	select ARM_DMA_USE_IOMMU if ARM
> @@ -310,6 +311,7 @@ config ARM_SMMU
>  config ARM_SMMU_V3
>  	bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
>  	depends on ARM64
> +	select MSI_DOORBELL
>  	select IOMMU_API
>  	select IOMMU_IO_PGTABLE_LPAE
>  	select GENERIC_MSI_IRQ_DOMAIN
> diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
> new file mode 100644
> index 0000000..c18a382
> --- /dev/null
> +++ b/include/linux/msi-doorbell.h
> @@ -0,0 +1,77 @@
> +/*
> + * API to register/query MSI doorbells likely to be IOMMU mapped
> + *
> + * Copyright (C) 2016 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef _LINUX_MSI_DOORBELL_H
> +#define _LINUX_MSI_DOORBELL_H
> +
> +struct msi_doorbell_info;
> +
> +#ifdef CONFIG_MSI_DOORBELL
> +
> +/**
> + * msi_doorbell_register - allocate and register a global doorbell
> + * @base: physical base address of the global doorbell
> + * @size: size of the global doorbell
> + * @prot: protection/memory attributes
> + * @safe: true is irq_remapping implemented for this doorbell
> + * @dbinfo: returned doorbell info
> + *
> + * Return: 0 on success, -ENOMEM on allocation failure
> + */
> +int msi_doorbell_register_global(phys_addr_t base, size_t size,
> +				 bool safe,
> +				 struct msi_doorbell_info **dbinfo);
> +

Seems like alloc/free behavior vs register/unregister.  Also seems
cleaner to just return a struct msi_doorbell_info* and use PTR_ERR for
return codes.  These are of course superficial changes that could be
addressed in the future.

> +/**
> + * msi_doorbell_unregister_global - unregister a global doorbell
> + * @db: doorbell info to unregister
> + *
> + * remove the doorbell descriptor from the list of registered doorbells
> + * and deallocates it
> + */
> +void msi_doorbell_unregister_global(struct msi_doorbell_info *db);
> +
> +/**
> + * msi_doorbell_safe - return whether all registered doorbells are safe
> + *
> + * Safe doorbells are those which implement irq remapping
> + * Return: true if all doorbells are safe, false otherwise
> + */
> +bool msi_doorbell_safe(void);
> +
> +#else
> +
> +static inline int
> +msi_doorbell_register_global(phys_addr_t base, size_t size,
> +			     int prot, bool safe,
> +			     struct msi_doorbell_info **dbinfo)
> +{
> +	*dbinfo = NULL;
> +	return 0;

If we return a struct*

return NULL;

> +}
> +
> +static inline void
> +msi_doorbell_unregister_global(struct msi_doorbell_info *db) {}
> +
> +static inline bool msi_doorbell_safe(void)
> +{
> +	return true;
> +}

Is it?

> +#endif /* CONFIG_MSI_DOORBELL */
> +
> +#endif
> diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
> index 3bbfd6a..d4faaaa 100644
> --- a/kernel/irq/Kconfig
> +++ b/kernel/irq/Kconfig
> @@ -72,6 +72,10 @@ config GENERIC_IRQ_IPI
>  config GENERIC_MSI_IRQ
>  	bool
>  
> +# MSI doorbell support (for doorbell IOMMU mapping)
> +config MSI_DOORBELL
> +	bool
> +
>  # Generic MSI hierarchical interrupt domain support
>  config GENERIC_MSI_IRQ_DOMAIN
>  	bool
> diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
> index 1d3ee31..5b04dd1 100644
> --- a/kernel/irq/Makefile
> +++ b/kernel/irq/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_PM_SLEEP) += pm.o
>  obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
>  obj-$(CONFIG_GENERIC_IRQ_IPI) += ipi.o
>  obj-$(CONFIG_SMP) += affinity.o
> +obj-$(CONFIG_MSI_DOORBELL) += msi-doorbell.o
> diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c
> new file mode 100644
> index 0000000..60a262a
> --- /dev/null
> +++ b/kernel/irq/msi-doorbell.c
> @@ -0,0 +1,98 @@
> +/*
> + * API to register/query MSI doorbells likely to be IOMMU mapped
> + *
> + * Copyright (C) 2016 Red Hat, Inc.
> + * Author: Eric Auger <eric.auger@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/irq.h>
> +#include <linux/msi-doorbell.h>
> +
> +/**
> + * struct msi_doorbell_info - MSI doorbell region descriptor
> + * @percpu_doorbells: per cpu doorbell base address
> + * @global_doorbell: base address of the doorbell
> + * @doorbell_is_percpu: is the doorbell per cpu or global?
> + * @safe: true if irq remapping is implemented
> + * @size: size of the doorbell
> + */
> +struct msi_doorbell_info {
> +	union {
> +		phys_addr_t __percpu    *percpu_doorbells;
> +		phys_addr_t             global_doorbell;
> +	};
> +	bool    doorbell_is_percpu;
> +	bool    safe;
> +	size_t  size;
> +};
> +
> +struct msi_doorbell {
> +	struct msi_doorbell_info	info;
> +	struct list_head		next;
> +};
> +
> +/* list of registered MSI doorbells */
> +static LIST_HEAD(msi_doorbell_list);
> +
> +/* counts the number of unsafe registered doorbells */
> +static uint nb_unsafe_doorbells;
> +
> +/* protects the list and nb__unsafe_doorbells */

Extra underscore

> +static DEFINE_MUTEX(msi_doorbell_mutex);
> +
> +int msi_doorbell_register_global(phys_addr_t base, size_t size, bool safe,
> +				 struct msi_doorbell_info **dbinfo)
> +{
> +	struct msi_doorbell *db;
> +
> +	db = kzalloc(sizeof(*db), GFP_KERNEL);
> +	if (!db)
> +		return -ENOMEM;
> +
> +	db->info.global_doorbell = base;
> +	db->info.size = size;
> +	db->info.safe = safe;
> +
> +	mutex_lock(&msi_doorbell_mutex);
> +	list_add(&db->next, &msi_doorbell_list);
> +	if (!db->info.safe)
> +		nb_unsafe_doorbells++;
> +	mutex_unlock(&msi_doorbell_mutex);
> +	*dbinfo = &db->info;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(msi_doorbell_register_global);
> +
> +void msi_doorbell_unregister_global(struct msi_doorbell_info *dbinfo)
> +{
> +	struct msi_doorbell *db;
> +
> +	db = container_of(dbinfo, struct msi_doorbell, info);
> +
> +	mutex_lock(&msi_doorbell_mutex);
> +	list_del(&db->next);
> +	if (!db->info.safe)
> +		nb_unsafe_doorbells--;
> +	mutex_unlock(&msi_doorbell_mutex);
> +	kfree(db);
> +}
> +EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global);
> +
> +bool msi_doorbell_safe(void)
> +{
> +	return !nb_unsafe_doorbells;
> +}
> +EXPORT_SYMBOL_GPL(msi_doorbell_safe);

[toc] | [prev] | [next] | [standalone]

#1497380 — Re: [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API

FromAuger Eric <eric.auger@redhat.com>
Date2016-10-07 19:20 +0200
SubjectRe: [PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API
Message-ID<spGYq-3AS-33@gated-at.bofh.it>
In reply to#1496847
Hi Alex,

On 06/10/2016 22:17, Alex Williamson wrote:
> On Thu,  6 Oct 2016 08:45:20 +0000
> Eric Auger <eric.auger@redhat.com> wrote:
> 
>> We introduce a new msi-doorbell API that allows msi controllers
>> to allocate and register their doorbells. This is useful when
>> those doorbells are likely to be iommu mapped (typically on ARM).
>> The VFIO layer will need to gather information about those doorbells:
>> whether they are safe (ie. they implement irq remapping) and how
>> many IOMMU pages are requested to map all of them.
>>
>> This patch first introduces the dedicated msi_doorbell_info struct
>> and the registration/unregistration functions.
>>
>> A doorbell region is characterized by its physical address base, size,
>> and whether it its safe (ie. it implements IRQ remapping). A doorbell
>> can be per-cpu of global. We currently only care about global doorbells.
>                  ^^ s/of/or/
OK
> 
>>
>> A function returns whether all doorbells are safe.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v12 -> v13:
>> - directly select MSI_DOORBELL in ARM_SMMU and ARM_SMMU_V3 configs
>> - remove prot attribute
>> - move msi_doorbell_info struct definition in msi-doorbell.c
>> - change the commit title
>> - change proto of the registration function
>> - msi_doorbell_safe now in this patch
>>
>> v11 -> v12:
>> - rename irqchip_doorbell into msi_doorbell, irqchip_doorbell_list
>>   into msi_doorbell_list and irqchip_doorbell_mutex into
>>   msi_doorbell_mutex
>> - fix style issues: align msi_doorbell struct members, kernel-doc comments
>> - use kzalloc
>> - use container_of in msi_doorbell_unregister_global
>> - compute nb_unsafe_doorbells on registration/unregistration
>> - registration simply returns NULL if allocation failed
>>
>> v10 -> v11:
>> - remove void *chip_data argument from register/unregister function
>> - remove lookup funtions since we restored the struct irq_chip
>>   msi_doorbell_info ops to realize this function
>> - reword commit message and title
>>
>> Conflicts:
>> 	kernel/irq/Makefile
>>
>> Conflicts:
>> 	drivers/iommu/Kconfig
>> ---
>>  drivers/iommu/Kconfig        |  2 +
>>  include/linux/msi-doorbell.h | 77 ++++++++++++++++++++++++++++++++++
>>  kernel/irq/Kconfig           |  4 ++
>>  kernel/irq/Makefile          |  1 +
>>  kernel/irq/msi-doorbell.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 182 insertions(+)
>>  create mode 100644 include/linux/msi-doorbell.h
>>  create mode 100644 kernel/irq/msi-doorbell.c
>>
>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>> index 8ee54d7..0cc7fac 100644
>> --- a/drivers/iommu/Kconfig
>> +++ b/drivers/iommu/Kconfig
>> @@ -297,6 +297,7 @@ config SPAPR_TCE_IOMMU
>>  config ARM_SMMU
>>  	bool "ARM Ltd. System MMU (SMMU) Support"
>>  	depends on (ARM64 || ARM) && MMU
>> +	select MSI_DOORBELL
>>  	select IOMMU_API
>>  	select IOMMU_IO_PGTABLE_LPAE
>>  	select ARM_DMA_USE_IOMMU if ARM
>> @@ -310,6 +311,7 @@ config ARM_SMMU
>>  config ARM_SMMU_V3
>>  	bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
>>  	depends on ARM64
>> +	select MSI_DOORBELL
>>  	select IOMMU_API
>>  	select IOMMU_IO_PGTABLE_LPAE
>>  	select GENERIC_MSI_IRQ_DOMAIN
>> diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
>> new file mode 100644
>> index 0000000..c18a382
>> --- /dev/null
>> +++ b/include/linux/msi-doorbell.h
>> @@ -0,0 +1,77 @@
>> +/*
>> + * API to register/query MSI doorbells likely to be IOMMU mapped
>> + *
>> + * Copyright (C) 2016 Red Hat, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef _LINUX_MSI_DOORBELL_H
>> +#define _LINUX_MSI_DOORBELL_H
>> +
>> +struct msi_doorbell_info;
>> +
>> +#ifdef CONFIG_MSI_DOORBELL
>> +
>> +/**
>> + * msi_doorbell_register - allocate and register a global doorbell
>> + * @base: physical base address of the global doorbell
>> + * @size: size of the global doorbell
>> + * @prot: protection/memory attributes
>> + * @safe: true is irq_remapping implemented for this doorbell
>> + * @dbinfo: returned doorbell info
>> + *
>> + * Return: 0 on success, -ENOMEM on allocation failure
>> + */
>> +int msi_doorbell_register_global(phys_addr_t base, size_t size,
>> +				 bool safe,
>> +				 struct msi_doorbell_info **dbinfo);
>> +
> 
> Seems like alloc/free behavior vs register/unregister.  Also seems
> cleaner to just return a struct msi_doorbell_info* and use PTR_ERR for
> return codes.  These are of course superficial changes that could be
> addressed in the future.
Sure
> 
>> +/**
>> + * msi_doorbell_unregister_global - unregister a global doorbell
>> + * @db: doorbell info to unregister
>> + *
>> + * remove the doorbell descriptor from the list of registered doorbells
>> + * and deallocates it
>> + */
>> +void msi_doorbell_unregister_global(struct msi_doorbell_info *db);
>> +
>> +/**
>> + * msi_doorbell_safe - return whether all registered doorbells are safe
>> + *
>> + * Safe doorbells are those which implement irq remapping
>> + * Return: true if all doorbells are safe, false otherwise
>> + */
>> +bool msi_doorbell_safe(void);
>> +
>> +#else
>> +
>> +static inline int
>> +msi_doorbell_register_global(phys_addr_t base, size_t size,
>> +			     int prot, bool safe,
>> +			     struct msi_doorbell_info **dbinfo)
>> +{
>> +	*dbinfo = NULL;
>> +	return 0;
> 
> If we return a struct*
> 
> return NULL;
Yep
> 
>> +}
>> +
>> +static inline void
>> +msi_doorbell_unregister_global(struct msi_doorbell_info *db) {}
>> +
>> +static inline bool msi_doorbell_safe(void)
>> +{
>> +	return true;
>> +}
> 
> Is it?
Yes I will return false and change the safety check in vfio_iommu_type1.c

Thanks

Eric
> 
>> +#endif /* CONFIG_MSI_DOORBELL */
>> +
>> +#endif
>> diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
>> index 3bbfd6a..d4faaaa 100644
>> --- a/kernel/irq/Kconfig
>> +++ b/kernel/irq/Kconfig
>> @@ -72,6 +72,10 @@ config GENERIC_IRQ_IPI
>>  config GENERIC_MSI_IRQ
>>  	bool
>>  
>> +# MSI doorbell support (for doorbell IOMMU mapping)
>> +config MSI_DOORBELL
>> +	bool
>> +
>>  # Generic MSI hierarchical interrupt domain support
>>  config GENERIC_MSI_IRQ_DOMAIN
>>  	bool
>> diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
>> index 1d3ee31..5b04dd1 100644
>> --- a/kernel/irq/Makefile
>> +++ b/kernel/irq/Makefile
>> @@ -10,3 +10,4 @@ obj-$(CONFIG_PM_SLEEP) += pm.o
>>  obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
>>  obj-$(CONFIG_GENERIC_IRQ_IPI) += ipi.o
>>  obj-$(CONFIG_SMP) += affinity.o
>> +obj-$(CONFIG_MSI_DOORBELL) += msi-doorbell.o
>> diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c
>> new file mode 100644
>> index 0000000..60a262a
>> --- /dev/null
>> +++ b/kernel/irq/msi-doorbell.c
>> @@ -0,0 +1,98 @@
>> +/*
>> + * API to register/query MSI doorbells likely to be IOMMU mapped
>> + *
>> + * Copyright (C) 2016 Red Hat, Inc.
>> + * Author: Eric Auger <eric.auger@redhat.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/slab.h>
>> +#include <linux/irq.h>
>> +#include <linux/msi-doorbell.h>
>> +
>> +/**
>> + * struct msi_doorbell_info - MSI doorbell region descriptor
>> + * @percpu_doorbells: per cpu doorbell base address
>> + * @global_doorbell: base address of the doorbell
>> + * @doorbell_is_percpu: is the doorbell per cpu or global?
>> + * @safe: true if irq remapping is implemented
>> + * @size: size of the doorbell
>> + */
>> +struct msi_doorbell_info {
>> +	union {
>> +		phys_addr_t __percpu    *percpu_doorbells;
>> +		phys_addr_t             global_doorbell;
>> +	};
>> +	bool    doorbell_is_percpu;
>> +	bool    safe;
>> +	size_t  size;
>> +};
>> +
>> +struct msi_doorbell {
>> +	struct msi_doorbell_info	info;
>> +	struct list_head		next;
>> +};
>> +
>> +/* list of registered MSI doorbells */
>> +static LIST_HEAD(msi_doorbell_list);
>> +
>> +/* counts the number of unsafe registered doorbells */
>> +static uint nb_unsafe_doorbells;
>> +
>> +/* protects the list and nb__unsafe_doorbells */
> 
> Extra underscore
> 
>> +static DEFINE_MUTEX(msi_doorbell_mutex);
>> +
>> +int msi_doorbell_register_global(phys_addr_t base, size_t size, bool safe,
>> +				 struct msi_doorbell_info **dbinfo)
>> +{
>> +	struct msi_doorbell *db;
>> +
>> +	db = kzalloc(sizeof(*db), GFP_KERNEL);
>> +	if (!db)
>> +		return -ENOMEM;
>> +
>> +	db->info.global_doorbell = base;
>> +	db->info.size = size;
>> +	db->info.safe = safe;
>> +
>> +	mutex_lock(&msi_doorbell_mutex);
>> +	list_add(&db->next, &msi_doorbell_list);
>> +	if (!db->info.safe)
>> +		nb_unsafe_doorbells++;
>> +	mutex_unlock(&msi_doorbell_mutex);
>> +	*dbinfo = &db->info;
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(msi_doorbell_register_global);
>> +
>> +void msi_doorbell_unregister_global(struct msi_doorbell_info *dbinfo)
>> +{
>> +	struct msi_doorbell *db;
>> +
>> +	db = container_of(dbinfo, struct msi_doorbell, info);
>> +
>> +	mutex_lock(&msi_doorbell_mutex);
>> +	list_del(&db->next);
>> +	if (!db->info.safe)
>> +		nb_unsafe_doorbells--;
>> +	mutex_unlock(&msi_doorbell_mutex);
>> +	kfree(db);
>> +}
>> +EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global);
>> +
>> +bool msi_doorbell_safe(void)
>> +{
>> +	return !nb_unsafe_doorbells;
>> +}
>> +EXPORT_SYMBOL_GPL(msi_doorbell_safe);
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web