linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes
@ 2016-06-07 16:01 Eric Auger
  2016-06-07 16:01 ` [PATCH v10 1/9] genirq/msi: export msi_get_domain_info Eric Auger
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

This series implements the MSI address mapping/unmapping in the MSI layer.
IOMMU binding happens on pci_enable_msi since this function can sleep and
return errors. On msi_domain_set_affinity, msi_domain_(de)activate, which
are not allowed to sleep, we simply look for the already existing binding.

Irqchips likely to be downstream to iommus (not bypassing MSIs) are supposed
to register their MSI doorbells. This make possible to retrieve their
characteristics, detect whether MSI assignment is safe and report to the
userspace the size/alignment of the guest PA window to provision for MSI
mapping.

A new MSI domain info flag value is introduced to report whether the msi
domain implements IRQ remapping. GIC v3 ITS is the first MSI controller
advertising it. This flag value will be used by VFIO subsystem to
determine whether MSI forwarding is safe.

More details & context can be found at:
http://www.linaro.org/blog/core-dump/kvm-pciemsi-passthrough-armarm64/

Best Regards

Eric

Git: complete series available at
https://github.com/eauger/linux/tree/v4.7-rc2-passthrough-v10

History:

v9 -> v10:
- was forced to introduce important changes on parts that were reviewed
  already :-( I took the initiative to replace the irqchip's
  get_doorbell_info callback by a new API, msi-doorbell).
  the new API makes possible to register, lookup doorbells and also compute
  the total requirements and IRQ safety flag used by VFIO.
- also added code in GICv3 ITS to register a global doorbell.

v8 -> v9:
- use a union in irq_chip_msi_doorbell_info + boolean telling whether the
  doorbell is percpu
- decouple irq_data parsing from the actual mapping/unmapping in
  msi_handle_doorbell_mappings
- fix misc style issues

v7 -> v8:
take into account Marc's comments:
- use iommu_msi_msg_pa_to_va with new proto
- change in irq_chip_msi_doorbell_info struct definition:
  prot and size became shared between all doorbells and phys_addr_t __percpu
- cleanups in v2m irqchip
- eventually did not touch MSI_FLAG_IRQ_REMAPPING naming
- On msi_handle_doorbell_mappings, stop on the first irqchip where doorbells
  can be found
- fix resource deallocation on mapping failure in msi_domain_alloc_irqs

v6 -> v7:
- do alloc/map handling on pci_enable_msi and search on msi_(de)domain_activate
- add msi_doorbell_info callback in irq-chip to retrieve the characteristics
  of doorbells

RFC v5 -> patch v6:
- split to ease the review process
- rebase on default iommu domain code (irq_data_to_msi_mapping_domain
  checks IOMMU_DOMAIN_DMA type)
- fix unmap sequence on msi_domain_set_affinity (reported by Marc):
  unmap the previous doorbell when the new one has been mapped & written to
  the device, ie. irq_chip_write_msi_msg.
- "msi: msi_compose wrapper removed" following change above
- add size parameter to iommu_get_reserved_iova API following Marc's request

RFC v4 -> RFC v5:
- take into account Thomas' comments on MSI related patches
  - split "msi: IOMMU map the doorbell address when needed"
  - increase readability and add comments
  - fix style issues
 - split "iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute"
 - platform ITS now advertises IOMMU_CAP_INTR_REMAP
 - fix compilation issue with CONFIG_IOMMU API unset
 - arm-smmu-v3 now advertises DOMAIN_ATTR_MSI_MAPPING

RFC v3 -> v4:
- Move doorbell mapping/unmapping in msi.c
- fix ref count issue on set_affinity: in case of a change in the address
  the previous address is decremented
- doorbell map/unmap now is done on msi composition. Should allow the use
  case for platform MSI controllers
- create dma-reserved-iommu.h/c exposing/implementing a new API dedicated
  to reserved IOVA management (looking like dma-iommu glue)
- series reordering to ease the review:
  - first part is related to IOMMU
  - second related to MSI sub-system
  - third related to VFIO (except arm-smmu IOMMU_CAP_INTR_REMAP removal)
- expose the number of requested IOVA pages through VFIO_IOMMU_GET_INFO
  [this partially addresses Marc's comments on iommu_get/put_single_reserved
   size/alignment problematic - which I did not ignore - but I don't know
   how much I can do at the moment]

RFC v2 -> RFC v3:
- should fix wrong handling of some CONFIG combinations:
  CONFIG_IOVA, CONFIG_IOMMU_API, CONFIG_PCI_MSI_IRQ_DOMAIN
- fix MSI_FLAG_IRQ_REMAPPING setting in GICv3 ITS (although not tested)

PATCH v1 -> RFC v2:
- reverted to RFC since it looks more reasonable ;-) the code is split
  between VFIO, IOMMU, MSI controller and I am not sure I did the right
  choices. Also API need to be further discussed.
- iova API usage in arm-smmu.c.
- MSI controller natively programs the MSI addr with either the PA or IOVA.
  This is not done anymore in vfio-pci driver as suggested by Alex.
- check irq remapping capability of the group

RFC v1 [2] -> PATCH v1:
- use the existing dma map/unmap ioctl interface with a flag to register a
  reserved IOVA range. Use the legacy Rb to store this special vfio_dma.
- a single reserved IOVA contiguous region now is allowed
- use of an RB tree indexed by PA to store allocated reserved slots
- use of a vfio_domain iova_domain to manage iova allocation within the
  window provided by the userspace
- vfio alloc_map/unmap_free take a vfio_group handle
- vfio_group handle is cached in vfio_pci_device
- add ref counting to bindings
- user modality enabled at the end of the series


Eric Auger (9):
  genirq/msi: export msi_get_domain_info
  genirq/msi: msi_compose wrapper
  genirq/msi-doorbell: creation
  genirq/msi-doorbell: msi_doorbell_pages
  genirq/msi-doorbell: msi_doorbell_safe
  irqchip/gicv2m: register the MSI global doorbell
  irqchip/gicv3-its: register the MSI global doorbell
  genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs
  genirq/msi: use the MSI doorbell's IOVA when requested

 drivers/iommu/Kconfig            |   1 +
 drivers/irqchip/irq-gic-v2m.c    |  10 ++-
 drivers/irqchip/irq-gic-v3-its.c |  14 ++++
 include/linux/msi-doorbell.h     | 105 +++++++++++++++++++++++++++
 kernel/irq/Kconfig               |   4 ++
 kernel/irq/Makefile              |   1 +
 kernel/irq/msi-doorbell.c        | 148 +++++++++++++++++++++++++++++++++++++++
 kernel/irq/msi.c                 | 134 ++++++++++++++++++++++++++++++++---
 8 files changed, 405 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/msi-doorbell.h
 create mode 100644 kernel/irq/msi-doorbell.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v10 1/9] genirq/msi: export msi_get_domain_info
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-07 16:01 ` [PATCH v10 2/9] genirq/msi: msi_compose wrapper Eric Auger
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

We plan to use msi_get_domain_info in VFIO module so let's export it.

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

---

v2 -> v3:
- remove static implementation in case CONFIG_PCI_MSI_IRQ_DOMAIN is not set
---
 kernel/irq/msi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 38e89ce..9b0ba4a 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -400,5 +400,6 @@ struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
 {
 	return (struct msi_domain_info *)domain->host_data;
 }
+EXPORT_SYMBOL_GPL(msi_get_domain_info);
 
 #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 2/9] genirq/msi: msi_compose wrapper
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
  2016-06-07 16:01 ` [PATCH v10 1/9] genirq/msi: export msi_get_domain_info Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-07 16:01 ` [PATCH v10 3/9] genirq/msi-doorbell: creation Eric Auger
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

Currently the MSI message is composed by directly calling
irq_chip_compose_msi_msg and erased by setting the memory to zero.

On some platforms, we will need to complexify this composition to
properly handle MSI emission through IOMMU. Also we will need to track
when the MSI message is erased.

We propose to introduce a common wrapper for actual composition and
erasure, msi_compose.

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

---
v4 -> v5:
- just introduce the msi-compose wrapper without adding new
  functionalities

v3 -> v4:
- that code was formely in irq-gic-common.c
  "irqchip/gicv2m/v3-its-pci-msi: IOMMU map the MSI frame when needed"
  also the [un]mapping was done in irq_write_msi_msg; now done on compose

v2 -> v3:
- protect iova/addr manipulation with CONFIG_ARCH_DMA_ADDR_T_64BIT and
  CONFIG_PHYS_ADDR_T_64BIT
- only expose gic_pci_msi_domain_write_msg in case CONFIG_IOMMU_API &
  CONFIG_PCI_MSI_IRQ_DOMAIN are set.
- gic_set/unset_msi_addr duly become static
---
 kernel/irq/msi.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 9b0ba4a..72bf4d6 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -55,6 +55,19 @@ static inline void irq_chip_write_msi_msg(struct irq_data *data,
 	data->chip->irq_write_msi_msg(data, msg);
 }
 
+static int msi_compose(struct irq_data *irq_data,
+		       struct msi_msg *msg, bool erase)
+{
+	int ret = 0;
+
+	if (erase)
+		memset(msg, 0, sizeof(*msg));
+	else
+		ret = irq_chip_compose_msi_msg(irq_data, msg);
+
+	return ret;
+}
+
 /**
  * msi_domain_set_affinity - Generic affinity setter function for MSI domains
  * @irq_data:	The irq data associated to the interrupt
@@ -73,7 +86,7 @@ int msi_domain_set_affinity(struct irq_data *irq_data,
 
 	ret = parent->chip->irq_set_affinity(parent, mask, force);
 	if (ret >= 0 && ret != IRQ_SET_MASK_OK_DONE) {
-		BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
+		BUG_ON(msi_compose(irq_data, &msg, false));
 		irq_chip_write_msi_msg(irq_data, &msg);
 	}
 
@@ -85,7 +98,7 @@ static void msi_domain_activate(struct irq_domain *domain,
 {
 	struct msi_msg msg;
 
-	BUG_ON(irq_chip_compose_msi_msg(irq_data, &msg));
+	BUG_ON(msi_compose(irq_data, &msg, false));
 	irq_chip_write_msi_msg(irq_data, &msg);
 }
 
@@ -94,7 +107,7 @@ static void msi_domain_deactivate(struct irq_domain *domain,
 {
 	struct msi_msg msg;
 
-	memset(&msg, 0, sizeof(msg));
+	msi_compose(irq_data, &msg, true);
 	irq_chip_write_msi_msg(irq_data, &msg);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 3/9] genirq/msi-doorbell: creation
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
  2016-06-07 16:01 ` [PATCH v10 1/9] genirq/msi: export msi_get_domain_info Eric Auger
  2016-06-07 16:01 ` [PATCH v10 2/9] genirq/msi: msi_compose wrapper Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-07 16:01 ` [PATCH v10 4/9] genirq/msi-doorbell: msi_doorbell_pages Eric Auger
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

This new API aims at managing MSI doorbells likely to be iommu
mapped. The API is bound to be used at least by irqchip drivers,
the MSI layer and the VFIO sub-system.

Irqchips likely to be downstream to IOMMUs (that do not bypass MSI
transactions) need to register their MSI doorbells so that the MSI
layer can look them up and iommu map then when needed.

We currently introduce the irq_chip_msi_doorbell_info struct that
contains all the info characterizing a doorbell plus 3 functions that
allows to (un)register, and lookup MSI doorbells stored in a global
list.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/iommu/Kconfig        |  1 +
 include/linux/msi-doorbell.h | 79 ++++++++++++++++++++++++++++++++++++++++++
 kernel/irq/Kconfig           |  4 +++
 kernel/irq/Makefile          |  1 +
 kernel/irq/msi-doorbell.c    | 82 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 167 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 5ea1610..ba54146 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -78,6 +78,7 @@ config IOMMU_DMA
 config IOMMU_MSI
 	bool
 	select IOMMU_DMA
+	select MSI_DOORBELL
 
 config FSL_PAMU
 	bool "Freescale IOMMU support"
diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
new file mode 100644
index 0000000..bbc7321
--- /dev/null
+++ b/include/linux/msi-doorbell.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2016 Eric Auger
+ *
+ * Eric Auger <eric.auger@linaro.org>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef _LINUX_MSI_DOORBELL_H
+#define _LINUX_MSI_DOORBELL_H
+
+#include <linux/irq.h>
+
+/* Describe all the MSI doorbell regions for an irqchip */
+struct irq_chip_msi_doorbell_info {
+	union {
+		phys_addr_t __percpu *percpu_doorbells;
+		phys_addr_t global_doorbell;
+	};
+	bool doorbell_is_percpu;
+	bool irq_remapping;	/* is irq_remapping implemented? */
+	size_t size;		/* size of a each doorbell */
+	int prot;		/* iommu protection flag */
+};
+
+#ifdef CONFIG_MSI_DOORBELL
+
+/**
+ * msi_doorbell_register_global: allocate and register a global doorbell
+ *
+ * @chip_data: chip_data pointer
+ * @addr: physical address of the global doorbell
+ * @size: size of the global doorbell
+ * @prot: protection/memory attributes
+ * @irq_remapping: is irq_remapping implemented for this doorbell
+ */
+int msi_doorbell_register_global(void *chip_data, phys_addr_t addr,
+				 size_t size, int prot, bool irq_remapping);
+
+/**
+ * msi_doorbell_unregister: remove a doorbell from the list of registered
+ * doorbells
+ * @chip_data: chip_data pointer
+ */
+void msi_doorbell_unregister(void *chip_data);
+
+/**
+ * msi_doorbell_lookup: return the doorbell info associated to @chip_data
+ * doorbell info
+ *
+ * @chip_data: chip_data pointer
+ * return NULL if no registered doorbell for that chip_data, or the actual
+ * doorbell info pointer upon success
+ */
+struct irq_chip_msi_doorbell_info *
+msi_doorbell_lookup(void *chip_data);
+
+#else
+
+static inline int
+msi_doorbell_register_global(void *chip_data, phys_addr_t addr,
+			     size_t size, int prot, bool irq_remapping)
+{
+	return -ENOENT;
+}
+
+static inline void msi_doorbell_unregister(void *chip_data);
+
+static inline struct irq_chip_msi_doorbell_info *
+msi_doorbell_lookup(void *chip_data)
+{
+	return NULL;
+}
+
+#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 2ee42e9..be02dfd 100644
--- a/kernel/irq/Makefile
+++ b/kernel/irq/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_GENERIC_IRQ_MIGRATION) += cpuhotplug.o
 obj-$(CONFIG_PM_SLEEP) += pm.o
 obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o
 obj-$(CONFIG_GENERIC_IRQ_IPI) += ipi.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..84ecca1
--- /dev/null
+++ b/kernel/irq/msi-doorbell.c
@@ -0,0 +1,82 @@
+/*
+ * linux/kernel/irq/msi-doorbell.c
+ *
+ * Copyright (C) 2016 Linaro
+ * Author: Eric Auger <eric.auger@linaro.org>
+ *
+ * This file is licensed under GPLv2.
+ *
+ * This file contains common code to manage MSI doorbells likely
+ * to be iommu mapped. Typically meaningful on ARM.
+ */
+
+#include <linux/slab.h>
+#include <linux/irq.h>
+#include <linux/msi-doorbell.h>
+
+struct irqchip_doorbell {
+	struct irq_chip_msi_doorbell_info info;
+	void *chip_data;
+	struct list_head next;
+};
+
+static LIST_HEAD(irqchip_doorbell_list);
+static DEFINE_MUTEX(irqchip_doorbell_mutex);
+
+int msi_doorbell_register_global(void *chip_data, phys_addr_t addr,
+				 size_t size, int prot, bool irq_remapping)
+{
+	struct irqchip_doorbell *db;
+
+	db = kmalloc(sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return -ENOMEM;
+
+	db->chip_data = chip_data;
+	db->info.doorbell_is_percpu = false;
+	db->info.global_doorbell = addr;
+	db->info.size = size;
+	db->info.prot = prot;
+	db->info.irq_remapping = irq_remapping;
+
+	mutex_lock(&irqchip_doorbell_mutex);
+	list_add(&db->next, &irqchip_doorbell_list);
+	mutex_unlock(&irqchip_doorbell_mutex);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_register_global);
+
+void msi_doorbell_unregister(void *chip_data)
+{
+	struct irqchip_doorbell *db, *tmp;
+
+	mutex_lock(&irqchip_doorbell_mutex);
+	list_for_each_entry_safe(db, tmp, &irqchip_doorbell_list, next) {
+		if (db->chip_data == chip_data) {
+			list_del(&db->next);
+			kfree(db);
+			break;
+		}
+	}
+	mutex_unlock(&irqchip_doorbell_mutex);
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_unregister);
+
+struct irq_chip_msi_doorbell_info *
+msi_doorbell_lookup(void *chip_data)
+{
+	struct irqchip_doorbell *db;
+	struct irq_chip_msi_doorbell_info *dbinfo = NULL;
+
+	mutex_lock(&irqchip_doorbell_mutex);
+	list_for_each_entry(db, &irqchip_doorbell_list, next) {
+		if (db->chip_data == chip_data) {
+			dbinfo = &db->info;
+			break;
+		}
+	}
+	mutex_unlock(&irqchip_doorbell_mutex);
+	return dbinfo;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_lookup);
+
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 4/9] genirq/msi-doorbell: msi_doorbell_pages
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (2 preceding siblings ...)
  2016-06-07 16:01 ` [PATCH v10 3/9] genirq/msi-doorbell: creation Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-07 16:01 ` [PATCH v10 5/9] genirq/msi-doorbell: msi_doorbell_safe Eric Auger
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

msi_doorbell_pages sum up the number of iommu pages of a given order
requested to map all the registered doorbells. This function will allow
to dimension the intermediate physical address (IPA) aperture requested
to map the MSI doorbells.

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

---

v10: creation
---
 include/linux/msi-doorbell.h | 14 ++++++++++++
 kernel/irq/msi-doorbell.c    | 52 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
index bbc7321..abdaaa0 100644
--- a/include/linux/msi-doorbell.h
+++ b/include/linux/msi-doorbell.h
@@ -57,6 +57,14 @@ void msi_doorbell_unregister(void *chip_data);
 struct irq_chip_msi_doorbell_info *
 msi_doorbell_lookup(void *chip_data);
 
+/**
+ * msi_doorbell_pages: compute the number of iommu pages of size 1 << order
+ * requested to map all the registered doorbells
+ *
+ * @order: iommu page order
+ */
+int msi_doorbell_pages(unsigned int order);
+
 #else
 
 static inline int
@@ -74,6 +82,12 @@ msi_doorbell_lookup(void *chip_data)
 	return NULL;
 }
 
+static inline int
+msi_doorbell_pages(unsigned int order)
+{
+	return 0;
+}
+
 #endif /* CONFIG_MSI_DOORBELL */
 
 #endif
diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c
index 84ecca1..65ca9292 100644
--- a/kernel/irq/msi-doorbell.c
+++ b/kernel/irq/msi-doorbell.c
@@ -80,3 +80,55 @@ msi_doorbell_lookup(void *chip_data)
 }
 EXPORT_SYMBOL_GPL(msi_doorbell_lookup);
 
+static int compute_db_mapping_requirements(phys_addr_t addr, size_t size,
+					   unsigned int order)
+{
+	phys_addr_t offset, granule;
+	unsigned int nb_pages;
+
+	granule = (uint64_t)(1 << order);
+	offset = addr & (granule - 1);
+	size = ALIGN(size + offset, granule);
+	nb_pages = size >> order;
+
+	return nb_pages;
+}
+
+static int
+compute_dbinfo_mapping_requirements(struct irq_chip_msi_doorbell_info *dbinfo,
+				    unsigned int order)
+{
+	int ret = 0;
+
+	if (!dbinfo->doorbell_is_percpu) {
+		ret = compute_db_mapping_requirements(dbinfo->global_doorbell,
+						      dbinfo->size, order);
+	} else {
+		phys_addr_t __percpu *pbase;
+		int cpu;
+
+		for_each_possible_cpu(cpu) {
+			pbase = per_cpu_ptr(dbinfo->percpu_doorbells, cpu);
+			ret += compute_db_mapping_requirements(*pbase,
+							       dbinfo->size,
+							       order);
+		}
+	}
+	return ret;
+}
+
+int msi_doorbell_pages(unsigned int order)
+{
+	struct irqchip_doorbell *db;
+	int ret = 0;
+
+	mutex_lock(&irqchip_doorbell_mutex);
+	list_for_each_entry(db, &irqchip_doorbell_list, next) {
+		ret += compute_dbinfo_mapping_requirements(&db->info, order);
+	}
+	mutex_unlock(&irqchip_doorbell_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_pages);
+
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 5/9] genirq/msi-doorbell: msi_doorbell_safe
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (3 preceding siblings ...)
  2016-06-07 16:01 ` [PATCH v10 4/9] genirq/msi-doorbell: msi_doorbell_pages Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-07 16:01 ` [PATCH v10 6/9] irqchip/gicv2m: register the MSI global doorbell Eric Auger
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

msi_doorbell_safe returns whether all the registered doorbells
implement irq_remapping.

When assigning a PCIe device whose host controller is upstream to
an IOMMU, we currently do not know whether the MSI controller is
upstream or downstream to the IOMMU.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/linux/msi-doorbell.h | 12 ++++++++++++
 kernel/irq/msi-doorbell.c    | 14 ++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
index abdaaa0..648f239 100644
--- a/include/linux/msi-doorbell.h
+++ b/include/linux/msi-doorbell.h
@@ -65,6 +65,13 @@ msi_doorbell_lookup(void *chip_data);
  */
 int msi_doorbell_pages(unsigned int order);
 
+/**
+ * msi_doorbell_safe: return whether all registered doorbells
+ * do implement irq_remapping and are safe to assign (coarse safety
+ * assessment)
+ */
+bool msi_doorbell_safe(void);
+
 #else
 
 static inline int
@@ -88,6 +95,11 @@ msi_doorbell_pages(unsigned int order)
 	return 0;
 }
 
+static inline bool
+msi_doorbell_safe(void)
+{
+	return true;
+}
 #endif /* CONFIG_MSI_DOORBELL */
 
 #endif
diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c
index 65ca9292..69121c8 100644
--- a/kernel/irq/msi-doorbell.c
+++ b/kernel/irq/msi-doorbell.c
@@ -132,3 +132,17 @@ int msi_doorbell_pages(unsigned int order)
 }
 EXPORT_SYMBOL_GPL(msi_doorbell_pages);
 
+bool msi_doorbell_safe(void)
+{
+	struct irqchip_doorbell *db;
+	bool irq_remapping = true;
+
+	mutex_lock(&irqchip_doorbell_mutex);
+	list_for_each_entry(db, &irqchip_doorbell_list, next) {
+		irq_remapping &= db->info.irq_remapping;
+	}
+	mutex_unlock(&irqchip_doorbell_mutex);
+
+	return irq_remapping;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_safe);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 6/9] irqchip/gicv2m: register the MSI global doorbell
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (4 preceding siblings ...)
  2016-06-07 16:01 ` [PATCH v10 5/9] genirq/msi-doorbell: msi_doorbell_safe Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-07 16:01 ` [PATCH v10 7/9] irqchip/gicv3-its: " Eric Auger
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

Use the msi-doorbell API to register the global doorbell.

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

---

v9 -> v10:
- introduce the registration concept in place of msi_doorbell_info
  callback

v8 -> v9:
- use global_doorbell instead of percpu_doorbells

v7 -> v8:
- gicv2m_msi_doorbell_info does not return a pointer to const
- remove spurious !v2m check
- add IOMMU_MMIO flag

v7: creation
---
 drivers/irqchip/irq-gic-v2m.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index ad0d296..11e039b 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -24,6 +24,8 @@
 #include <linux/of_pci.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/iommu.h>
+#include <linux/msi-doorbell.h>
 
 /*
 * MSI_TYPER:
@@ -366,12 +368,18 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
 		goto err_iounmap;
 	}
 
+	ret = msi_doorbell_register_global(v2m, v2m->res.start, sizeof(u32),
+					   IOMMU_WRITE | IOMMU_MMIO, false);
+	if (ret)
+		goto err_free_bm;
+
 	list_add_tail(&v2m->entry, &v2m_nodes);
 
 	pr_info("range%pR, SPI[%d:%d]\n", res,
 		v2m->spi_start, (v2m->spi_start + v2m->nr_spis - 1));
 	return 0;
-
+err_free_bm:
+	kfree(v2m->bm);
 err_iounmap:
 	iounmap(v2m->base);
 err_free_v2m:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 7/9] irqchip/gicv3-its: register the MSI global doorbell
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (5 preceding siblings ...)
  2016-06-07 16:01 ` [PATCH v10 6/9] irqchip/gicv2m: register the MSI global doorbell Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-17 16:33   ` Jean-Philippe Brucker
  2016-06-07 16:01 ` [PATCH v10 8/9] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Eric Auger
  2016-06-07 16:01 ` [PATCH v10 9/9] genirq/msi: use the MSI doorbell's IOVA when requested Eric Auger
  8 siblings, 1 reply; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

This patch adds the registration of the MSI global doorbell in
gicv3-its driver.

This will allow the msi layer to iommu_map this doorbell when
requested.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 5eb1f9e..ed9dfce 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -29,6 +29,8 @@
 #include <linux/of_platform.h>
 #include <linux/percpu.h>
 #include <linux/slab.h>
+#include <linux/iommu.h>
+#include <linux/msi-doorbell.h>
 
 #include <linux/irqchip.h>
 #include <linux/irqchip/arm-gic-v3.h>
@@ -1607,6 +1609,7 @@ static int __init its_probe(struct device_node *node,
 
 	if (of_property_read_bool(node, "msi-controller")) {
 		struct msi_domain_info *info;
+		phys_addr_t translater;
 
 		info = kzalloc(sizeof(*info), GFP_KERNEL);
 		if (!info) {
@@ -1614,10 +1617,21 @@ static int __init its_probe(struct device_node *node,
 			goto out_free_tables;
 		}
 
+		translater = its->phys_base + GITS_TRANSLATER;
+		err = msi_doorbell_register_global(its, translater,
+						sizeof(u32),
+						IOMMU_WRITE | IOMMU_MMIO, true);
+		if (err)  {
+			kfree(info);
+			goto out_free_tables;
+		}
+
+
 		inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
 		if (!inner_domain) {
 			err = -ENOMEM;
 			kfree(info);
+			msi_doorbell_unregister(its);
 			goto out_free_tables;
 		}
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 8/9] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (6 preceding siblings ...)
  2016-06-07 16:01 ` [PATCH v10 7/9] irqchip/gicv3-its: " Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  2016-06-07 16:01 ` [PATCH v10 9/9] genirq/msi: use the MSI doorbell's IOVA when requested Eric Auger
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

This patch handles the iommu mapping of MSI doorbells that require to
be mapped in an iommu domain. This happens on msi_domain_alloc/free_irqs
since this is called in code that can sleep (pci_enable/disable_msi):
iommu_map/unmap is not stated as atomic. On msi_domain_(de)activate and
msi_domain_set_affinity, which must be atomic, we just lookup for this
pre-allocated/mapped IOVA.

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

---
v9 -> v10:
- use irqchip API to lookup for the chip_data's doorbell

v8 -> v9:
- decouple irq_data parsing from the actual mapping/unmapping

v7 -> v8:
- new percpu pointer type
- exit from the irq domain hierarchy parsing on first map/unmap success
- reset desc->irq to 0 on mapping failure

v7: creation
---
 kernel/irq/msi.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 98 insertions(+), 8 deletions(-)

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 72bf4d6..a78db5f 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -14,6 +14,9 @@
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/msi.h>
+#include <linux/msi-iommu.h>
+#include <linux/iommu.h>
+#include <linux/msi-doorbell.h>
 
 /* Temparory solution for building, will be removed later */
 #include <linux/pci.h>
@@ -322,6 +325,71 @@ int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
 }
 
 /**
+ * msi_handle_doorbell_mappings: in case the irq data corresponds to an
+ * MSI that requires iommu mapping, traverse the irq domain hierarchy
+ * to retrieve the doorbells to handle and iommu_map/unmap them according
+ * to @map boolean.
+ *
+ * @data: irq data handle
+ * @map: mapping if true, unmapping if false
+ */
+static int msi_handle_doorbell_mappings(struct irq_data *data, bool map)
+{
+	const struct irq_chip_msi_doorbell_info *dbinfo;
+	struct iommu_domain *domain;
+	struct device *dev;
+	dma_addr_t iova;
+	void *chip_data;
+	int ret = 0, cpu;
+
+	while (data) {
+		dev = msi_desc_to_dev(irq_data_get_msi_desc(data));
+		domain = iommu_msi_domain(dev);
+		if (domain) {
+			chip_data = irq_data_get_irq_chip_data(data);
+			dbinfo = msi_doorbell_lookup(chip_data);
+			if (dbinfo)
+				break;
+		}
+		data = data->parent_data;
+	}
+
+	if (!data)
+		return 0;
+
+	if (!dbinfo->doorbell_is_percpu) {
+		if (!map) {
+			iommu_msi_put_doorbell_iova(domain,
+						    dbinfo->global_doorbell);
+			return 0;
+		}
+		return iommu_msi_get_doorbell_iova(domain,
+						   dbinfo->global_doorbell,
+						   dbinfo->size, dbinfo->prot,
+						   &iova);
+	}
+
+	/* percpu doorbells */
+	for_each_possible_cpu(cpu) {
+		phys_addr_t __percpu *db_addr =
+			per_cpu_ptr(dbinfo->percpu_doorbells, cpu);
+
+		if (!map) {
+			iommu_msi_put_doorbell_iova(domain, *db_addr);
+		} else {
+
+			ret = iommu_msi_get_doorbell_iova(domain, *db_addr,
+							  dbinfo->size,
+							  dbinfo->prot, &iova);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+/**
  * msi_domain_alloc_irqs - Allocate interrupts from a MSI interrupt domain
  * @domain:	The domain to allocate from
  * @dev:	Pointer to device struct of the device for which the interrupts
@@ -352,17 +420,29 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
 
 		virq = __irq_domain_alloc_irqs(domain, virq, desc->nvec_used,
 					       dev_to_node(dev), &arg, false);
-		if (virq < 0) {
-			ret = -ENOSPC;
-			if (ops->handle_error)
-				ret = ops->handle_error(domain, desc, ret);
-			if (ops->msi_finish)
-				ops->msi_finish(&arg, ret);
-			return ret;
-		}
+		if (virq < 0)
+			goto error;
 
 		for (i = 0; i < desc->nvec_used; i++)
 			irq_set_msi_desc_off(virq, i, desc);
+
+		for (i = 0; i < desc->nvec_used; i++) {
+			struct irq_data *d = irq_get_irq_data(virq + i);
+
+			ret = msi_handle_doorbell_mappings(d, true);
+			if (ret)
+				break;
+		}
+		if (ret) {
+			for (; i >= 0; i--) {
+				struct irq_data *d = irq_get_irq_data(virq + i);
+
+				msi_handle_doorbell_mappings(d, false);
+			}
+			irq_domain_free_irqs(virq, desc->nvec_used);
+			desc->irq = 0;
+			goto error;
+		}
 	}
 
 	if (ops->msi_finish)
@@ -377,6 +457,13 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
 	}
 
 	return 0;
+error:
+	ret = -ENOSPC;
+	if (ops->handle_error)
+		ret = ops->handle_error(domain, desc, ret);
+	if (ops->msi_finish)
+		ops->msi_finish(&arg, ret);
+	return ret;
 }
 
 /**
@@ -396,6 +483,9 @@ void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
 		 * entry. If that's the case, don't do anything.
 		 */
 		if (desc->irq) {
+			msi_handle_doorbell_mappings(
+				irq_get_irq_data(desc->irq),
+				false);
 			irq_domain_free_irqs(desc->irq, desc->nvec_used);
 			desc->irq = 0;
 		}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v10 9/9] genirq/msi: use the MSI doorbell's IOVA when requested
  2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (7 preceding siblings ...)
  2016-06-07 16:01 ` [PATCH v10 8/9] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Eric Auger
@ 2016-06-07 16:01 ` Eric Auger
  8 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2016-06-07 16:01 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, robin.murphy, alex.williamson,
	will.deacon, joro, tglx, jason, marc.zyngier, christoffer.dall,
	linux-arm-kernel
  Cc: linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, Jean-Philippe.Brucker, julien.grall, yehuday

On MSI message composition we now use the MSI doorbell's IOVA in
place of the doorbell's PA in case the device is upstream to an
IOMMU that requires MSI addresses to be mapped. The doorbell's
allocation and mapping happened on an early stage (pci_enable_msi).

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

---
v8 -> v9:
- Braces on both sides of the 'else' in msi_compose

v7 -> v8:
- use iommu_msi_msg_pa_to_va
- add WARN_ON

v6 -> v7:
- allocation/mapping is done at an earlier stage. We now just perform
  the iova lookup. So it is safe now to be called in a code that cannot
  sleep. iommu_msi_set_doorbell_iova is moved in the dma-reserved-iommu
  API: I think it cleans things up with respect to various #ifdef CONFIGS.

v5:
- use macros to increase the readability
- add comments
- fix a typo that caused a compilation error if CONFIG_IOMMU_API
  is not set
---
 kernel/irq/msi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index a78db5f..e731949 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -63,10 +63,18 @@ static int msi_compose(struct irq_data *irq_data,
 {
 	int ret = 0;
 
-	if (erase)
+	if (erase) {
 		memset(msg, 0, sizeof(*msg));
-	else
+	} else {
+		struct device *dev;
+
 		ret = irq_chip_compose_msi_msg(irq_data, msg);
+		if (ret)
+			return ret;
+
+		dev = msi_desc_to_dev(irq_data_get_msi_desc(irq_data));
+		WARN_ON(iommu_msi_msg_pa_to_va(dev, msg));
+	}
 
 	return ret;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 7/9] irqchip/gicv3-its: register the MSI global doorbell
  2016-06-07 16:01 ` [PATCH v10 7/9] irqchip/gicv3-its: " Eric Auger
@ 2016-06-17 16:33   ` Jean-Philippe Brucker
  2016-06-19 16:11     ` Auger Eric
  0 siblings, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2016-06-17 16:33 UTC (permalink / raw)
  To: Eric Auger
  Cc: eric.auger.pro, robin.murphy, alex.williamson, will.deacon, joro,
	tglx, jason, marc.zyngier, christoffer.dall, linux-arm-kernel,
	linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, julien.grall, yehuday

Hi Eric,

On Tue, Jun 07, 2016 at 04:01:26PM +0000, Eric Auger wrote:
> This patch adds the registration of the MSI global doorbell in
> gicv3-its driver.
> 
> This will allow the msi layer to iommu_map this doorbell when
> requested.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 5eb1f9e..ed9dfce 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -29,6 +29,8 @@
>  #include <linux/of_platform.h>
>  #include <linux/percpu.h>
>  #include <linux/slab.h>
> +#include <linux/iommu.h>
> +#include <linux/msi-doorbell.h>
>  
>  #include <linux/irqchip.h>
>  #include <linux/irqchip/arm-gic-v3.h>
> @@ -1607,6 +1609,7 @@ static int __init its_probe(struct device_node *node,
>  
>  	if (of_property_read_bool(node, "msi-controller")) {
>  		struct msi_domain_info *info;
> +		phys_addr_t translater;
>  
>  		info = kzalloc(sizeof(*info), GFP_KERNEL);
>  		if (!info) {
> @@ -1614,10 +1617,21 @@ static int __init its_probe(struct device_node *node,
>  			goto out_free_tables;
>  		}
>  
> +		translater = its->phys_base + GITS_TRANSLATER;
> +		err = msi_doorbell_register_global(its, translater,
> +						sizeof(u32),
> +						IOMMU_WRITE | IOMMU_MMIO, true);

This doesn't work :(

First we have its_probe registering the global mapping with
doorbell->chip_data = its, which is a pointer to an its_node structure.

But when enabling MSIs for a device, its_irq_domain_alloc puts a pointer
to an *its_device* into irq_data->chip_data. This seems to be a
per-device structure, allocated by its_msi_prepare.

The following call to msi_doorbell_lookup won't ever succeed, because it
will compare its_node to its_device. I can't figure out how to fix it
cleanly at the moment.

Jean-Philippe

> +		if (err)  {
> +			kfree(info);
> +			goto out_free_tables;
> +		}
> +
> +
>  		inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
>  		if (!inner_domain) {
>  			err = -ENOMEM;
>  			kfree(info);
> +			msi_doorbell_unregister(its);
>  			goto out_free_tables;
>  		}
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v10 7/9] irqchip/gicv3-its: register the MSI global doorbell
  2016-06-17 16:33   ` Jean-Philippe Brucker
@ 2016-06-19 16:11     ` Auger Eric
  0 siblings, 0 replies; 12+ messages in thread
From: Auger Eric @ 2016-06-19 16:11 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: eric.auger.pro, robin.murphy, alex.williamson, will.deacon, joro,
	tglx, jason, marc.zyngier, christoffer.dall, linux-arm-kernel,
	linux-kernel, Bharat.Bhushan, pranav.sawargaonkar, p.fedin,
	iommu, julien.grall, yehuday

Hi Jean-Philippe,

On 17/06/2016 18:33, Jean-Philippe Brucker wrote:
> Hi Eric,
> 
> On Tue, Jun 07, 2016 at 04:01:26PM +0000, Eric Auger wrote:
>> This patch adds the registration of the MSI global doorbell in
>> gicv3-its driver.
>>
>> This will allow the msi layer to iommu_map this doorbell when
>> requested.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>  drivers/irqchip/irq-gic-v3-its.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 5eb1f9e..ed9dfce 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -29,6 +29,8 @@
>>  #include <linux/of_platform.h>
>>  #include <linux/percpu.h>
>>  #include <linux/slab.h>
>> +#include <linux/iommu.h>
>> +#include <linux/msi-doorbell.h>
>>  
>>  #include <linux/irqchip.h>
>>  #include <linux/irqchip/arm-gic-v3.h>
>> @@ -1607,6 +1609,7 @@ static int __init its_probe(struct device_node *node,
>>  
>>  	if (of_property_read_bool(node, "msi-controller")) {
>>  		struct msi_domain_info *info;
>> +		phys_addr_t translater;
>>  
>>  		info = kzalloc(sizeof(*info), GFP_KERNEL);
>>  		if (!info) {
>> @@ -1614,10 +1617,21 @@ static int __init its_probe(struct device_node *node,
>>  			goto out_free_tables;
>>  		}
>>  
>> +		translater = its->phys_base + GITS_TRANSLATER;
>> +		err = msi_doorbell_register_global(its, translater,
>> +						sizeof(u32),
>> +						IOMMU_WRITE | IOMMU_MMIO, true);
> 
> This doesn't work :(

Sorry to hear that.

Thank you for testing with ITS. As mentioned in the part 3 cover letter,
I could not test assignment with this MSI controller; I now have access
to it so I am going to test it from next release onwards.
> 
> First we have its_probe registering the global mapping with
> doorbell->chip_data = its, which is a pointer to an its_node structure.
> 
> But when enabling MSIs for a device, its_irq_domain_alloc puts a pointer
> to an *its_device* into irq_data->chip_data. This seems to be a
> per-device structure, allocated by its_msi_prepare.
Hum OK, I missed the fact the chip_data was overwritten on
its_irq_domain_alloc. I will investigate what we have as other alternatives.

Thank you for the time spent on debugging that ;-)

Best Regards

Eric


> 
> The following call to msi_doorbell_lookup won't ever succeed, because it
> will compare its_node to its_device. I can't figure out how to fix it
> cleanly at the moment.
> 
> Jean-Philippe
> 
>> +		if (err)  {
>> +			kfree(info);
>> +			goto out_free_tables;
>> +		}
>> +
>> +
>>  		inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
>>  		if (!inner_domain) {
>>  			err = -ENOMEM;
>>  			kfree(info);
>> +			msi_doorbell_unregister(its);
>>  			goto out_free_tables;
>>  		}
>>  
>> -- 
>> 1.9.1
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-06-19 16:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07 16:01 [PATCH v10 0/9] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
2016-06-07 16:01 ` [PATCH v10 1/9] genirq/msi: export msi_get_domain_info Eric Auger
2016-06-07 16:01 ` [PATCH v10 2/9] genirq/msi: msi_compose wrapper Eric Auger
2016-06-07 16:01 ` [PATCH v10 3/9] genirq/msi-doorbell: creation Eric Auger
2016-06-07 16:01 ` [PATCH v10 4/9] genirq/msi-doorbell: msi_doorbell_pages Eric Auger
2016-06-07 16:01 ` [PATCH v10 5/9] genirq/msi-doorbell: msi_doorbell_safe Eric Auger
2016-06-07 16:01 ` [PATCH v10 6/9] irqchip/gicv2m: register the MSI global doorbell Eric Auger
2016-06-07 16:01 ` [PATCH v10 7/9] irqchip/gicv3-its: " Eric Auger
2016-06-17 16:33   ` Jean-Philippe Brucker
2016-06-19 16:11     ` Auger Eric
2016-06-07 16:01 ` [PATCH v10 8/9] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Eric Auger
2016-06-07 16:01 ` [PATCH v10 9/9] genirq/msi: use the MSI doorbell's IOVA when requested Eric Auger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).