linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes
@ 2016-08-02 17:23 Eric Auger
  2016-08-02 17:23 ` [PATCH v12 01/11] genirq/msi: export msi_get_domain_info Eric Auger
                   ` (10 more replies)
  0 siblings, 11 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

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-rc7-passthrough-v12

History:
v11 -> v12
- rework kernel-docs, misc renamings and style issue fixing
- remove WARN_ON in msi_compose around iommu_msi_msg_pa_to_va,
  instead return the error
- new code structure in "genirq/msi: Map/unmap the MSI doorbells on
  msi_domain_alloc/free_irqs"
- introduce new msi_desc flags and let free_msi_irqs do the deallocation
- irq_get_msi_doorbell_info returns NULL in case of error
- clarify case where MSI controller stands inbetween the device and the
  IOMMU

v10 -> v11:
- restored irq_chip msi_doorbell_info since lookup function introduced
  in v10 (taking the chip_data as parameter) did not work for ITS and
  most probably for other irqchips/
- changed the registration API
- eventually tested with GICv3 ITS

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 (11):
  genirq/msi: export msi_get_domain_info
  genirq/msi: msi_compose wrapper
  genirq: Introduce irq_get_msi_doorbell_info
  genirq/msi: Allow MSI doorbell (un)registration
  genirq/msi: msi_doorbell_calc_pages
  genirq/msi: msi_doorbell_safe
  irqchip/gic-v2m: Register the MSI global doorbell
  irqchip/gicv3-its: Register the MSI global doorbell
  genirq/msi: Introduce msi_desc flags
  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    |  35 ++++++--
 drivers/irqchip/irq-gic-v3-its.c |  67 ++++++++++----
 drivers/pci/msi.c                |   2 +-
 include/linux/irq.h              |  23 ++++-
 include/linux/msi-doorbell.h     |  82 +++++++++++++++++
 include/linux/msi.h              |  14 +++
 kernel/irq/Kconfig               |   4 +
 kernel/irq/Makefile              |   1 +
 kernel/irq/msi-doorbell.c        | 138 +++++++++++++++++++++++++++++
 kernel/irq/msi.c                 | 187 +++++++++++++++++++++++++++++++++++++--
 11 files changed, 517 insertions(+), 37 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] 15+ messages in thread

* [PATCH v12 01/11] genirq/msi: export msi_get_domain_info
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 02/11] genirq/msi: msi_compose wrapper Eric Auger
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

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] 15+ messages in thread

* [PATCH v12 02/11] genirq/msi: msi_compose wrapper
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
  2016-08-02 17:23 ` [PATCH v12 01/11] genirq/msi: export msi_get_domain_info Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-09  9:19   ` Thomas Gleixner
  2016-08-02 17:23 ` [PATCH v12 03/11] genirq: Introduce irq_get_msi_doorbell_info Eric Auger
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

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] 15+ messages in thread

* [PATCH v12 03/11] genirq: Introduce irq_get_msi_doorbell_info
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
  2016-08-02 17:23 ` [PATCH v12 01/11] genirq/msi: export msi_get_domain_info Eric Auger
  2016-08-02 17:23 ` [PATCH v12 02/11] genirq/msi: msi_compose wrapper Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 04/11] genirq/msi: Allow MSI doorbell (un)registration Eric Auger
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

From: Eric Auger <eric.auger@linaro.org>

The purpose is to be able to retrieve the MSI doorbells of an irqchip.
This is now needed since on some platforms those doorbells must be
iommu mapped (in case the MSIs transit through an IOMMU that do not
bypass those transactions).

The assumption is there is a maximum of one doorbell region per cpu. The
doorbell can be global or per cpu.

A doorbell region is characterized by its physical address base, size,
IOMMU protection flag and whether it implements IRQ remapping (aka.
IRQ translation). Those characteristics are shared among all doorbells.

irq_get_msi_doorbell_info callback enables to retrieve the doorbells of
the irqchip.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---
v11 -> v12:
- remove tail comments and do proper kernel doc ones
- align struct members
- rename msi_doorbell_info into irq_get_msi_doorbell_info
- remove line break

v10 -> v11:
- disappeared in V10 and restored now. struct irq_chip_msi_doorbell_info
  is identifical to the one in V10 (union, irq_remapping field).

v7 -> v8:
- size and prot now are shared among all doorbells
- doorbells now directly points to a percpu phys_addr_t

v7: creation
---
 include/linux/irq.h | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 4d758a7..27a6cd0 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -313,6 +313,26 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
 }
 
 /**
+ * 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?
+ * @irq_remapping: is irq_remapping implemented?
+ * @size: size of the doorbell
+ * @prot: iommu protection flag
+ */
+struct msi_doorbell_info {
+	union {
+		phys_addr_t __percpu	*percpu_doorbells;
+		phys_addr_t		global_doorbell;
+	};
+	bool	doorbell_is_percpu;
+	bool	irq_remapping;
+	size_t	size;
+	int	prot;
+};
+
+/**
  * struct irq_chip - hardware interrupt chip descriptor
  *
  * @name:		name for /proc/interrupts
@@ -349,6 +369,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
  * @irq_get_irqchip_state:	return the internal state of an interrupt
  * @irq_set_irqchip_state:	set the internal state of a interrupt
  * @irq_set_vcpu_affinity:	optional to target a vCPU in a virtual machine
+ * @irq_get_msi_doorbell_info:	return the MSI doorbell info
  * @ipi_send_single:	send a single IPI to destination cpus
  * @ipi_send_mask:	send an IPI to destination cpus in cpumask
  * @flags:		chip specific flags
@@ -394,7 +415,7 @@ struct irq_chip {
 	int		(*irq_set_irqchip_state)(struct irq_data *data, enum irqchip_irq_state which, bool state);
 
 	int		(*irq_set_vcpu_affinity)(struct irq_data *data, void *vcpu_info);
-
+	struct msi_doorbell_info *(*irq_get_msi_doorbell_info)(struct irq_data *data);
 	void		(*ipi_send_single)(struct irq_data *data, unsigned int cpu);
 	void		(*ipi_send_mask)(struct irq_data *data, const struct cpumask *dest);
 
-- 
1.9.1

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

* [PATCH v12 04/11] genirq/msi: Allow MSI doorbell (un)registration
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (2 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 03/11] genirq: Introduce irq_get_msi_doorbell_info Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 05/11] genirq/msi: msi_doorbell_calc_pages Eric Auger
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

This new API aims at allowing irqchips to allocate & register
the MSI doorbells likely to be iommu mapped.

Later on, other services will be added allowing the VFIO layer
to query information based on all registered doorbells.

We count the number of doorbells that do not implement IRQ
remapping. They will be considered as unsafe with respect to MSI
assignment.

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

---
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
---
 drivers/iommu/Kconfig        |  1 +
 include/linux/msi-doorbell.h | 55 +++++++++++++++++++++++++++++++++++
 kernel/irq/Kconfig           |  4 +++
 kernel/irq/Makefile          |  1 +
 kernel/irq/msi-doorbell.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 129 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..a2a033b
--- /dev/null
+++ b/include/linux/msi-doorbell.h
@@ -0,0 +1,55 @@
+/*
+ * 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>
+
+#ifdef CONFIG_MSI_DOORBELL
+
+/**
+ * msi_doorbell_register_global - allocate and register a global doorbell
+ * @base: physical base address of the global doorbell
+ * @size: size of the global doorbell
+ * @prot: protection/memory attributes
+ * @irq_remapping: is irq_remapping implemented for this doorbell
+ *
+ * Return: the newly allocated doorbell info handle or NULL if allocation
+ * failed
+ */
+struct msi_doorbell_info *
+msi_doorbell_register_global(phys_addr_t base, size_t size,
+			     int prot, bool irq_remapping);
+
+/**
+ * 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);
+
+#else
+
+static inline struct msi_doorbell_info *
+msi_doorbell_register_global(phys_addr_t base, size_t size,
+			     int prot, bool irq_remapping)
+{
+	return NULL;
+}
+
+static inline void
+msi_doorbell_unregister_global(struct msi_doorbell_info *db) {}
+
+#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..e3e0d95
--- /dev/null
+++ b/kernel/irq/msi-doorbell.c
@@ -0,0 +1,68 @@
+/*
+ * 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 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);
+
+struct msi_doorbell_info *
+msi_doorbell_register_global(phys_addr_t base, size_t size,
+			     int prot, bool irq_remapping)
+{
+	struct msi_doorbell *db;
+
+	db = kzalloc(sizeof(*db), GFP_KERNEL);
+	if (!db)
+		return NULL;
+
+	db->info.global_doorbell = base;
+	db->info.size = size;
+	db->info.prot = prot;
+	db->info.irq_remapping = irq_remapping;
+
+	mutex_lock(&msi_doorbell_mutex);
+	list_add(&db->next, &msi_doorbell_list);
+	if (!db->info.irq_remapping)
+		nb_unsafe_doorbells++;
+	mutex_unlock(&msi_doorbell_mutex);
+	return &db->info;
+}
+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.irq_remapping)
+		nb_unsafe_doorbells--;
+	mutex_unlock(&msi_doorbell_mutex);
+	kfree(db);
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global);
-- 
1.9.1

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

* [PATCH v12 05/11] genirq/msi: msi_doorbell_calc_pages
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (3 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 04/11] genirq/msi: Allow MSI doorbell (un)registration Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 06/11] genirq/msi: msi_doorbell_safe Eric Auger
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

msi_doorbell_calc_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>

---
v11 -> v12:
- fix style issues: remove useless line break, remove pointless braces,
  fix kernel-doc comments
- reword commit message
- rename msi_doorbell_pages into msi_doorbell_calc_pages
- rename static compute* functions

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

diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
index a2a033b..bbedb3b 100644
--- a/include/linux/msi-doorbell.h
+++ b/include/linux/msi-doorbell.h
@@ -38,6 +38,15 @@ msi_doorbell_register_global(phys_addr_t base, size_t size,
  */
 void msi_doorbell_unregister_global(struct msi_doorbell_info *db);
 
+/**
+ * msi_doorbell_calc_pages - compute the number of pages
+ * requested to map all the registered doorbells
+ * @order: iommu page order
+ *
+ * Return: the number of requested pages
+ */
+int msi_doorbell_calc_pages(unsigned int order);
+
 #else
 
 static inline struct msi_doorbell_info *
@@ -50,6 +59,11 @@ msi_doorbell_register_global(phys_addr_t base, size_t size,
 static inline void
 msi_doorbell_unregister_global(struct msi_doorbell_info *db) {}
 
+static inline int msi_doorbell_calc_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 e3e0d95..5c6c2aa 100644
--- a/kernel/irq/msi-doorbell.c
+++ b/kernel/irq/msi-doorbell.c
@@ -66,3 +66,67 @@ void msi_doorbell_unregister_global(struct msi_doorbell_info *dbinfo)
 	kfree(db);
 }
 EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global);
+
+/**
+ * calc_region_reqs - compute the number of pages requested to map a region
+ *
+ * @addr: physical base address of the region
+ * @size: size of the region
+ * @order: the page order
+ *
+ * Return: the number of requested pages to map this region
+ */
+static int calc_region_reqs(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;
+}
+
+/**
+ * calc_dbinfo_reqs - compute the number of pages requested to map a given
+ * MSI doorbell
+ *
+ * @dbi: doorbell info descriptor
+ * @order: page order
+ *
+ * Return: the number of requested pages to map this doorbell
+ */
+static int calc_dbinfo_reqs(struct msi_doorbell_info *dbi, unsigned int order)
+{
+	int ret = 0;
+
+	if (!dbi->doorbell_is_percpu) {
+		ret = calc_region_reqs(dbi->global_doorbell, dbi->size, order);
+	} else {
+		phys_addr_t __percpu *pbase;
+		int cpu;
+
+		for_each_possible_cpu(cpu) {
+			pbase = per_cpu_ptr(dbi->percpu_doorbells, cpu);
+			ret += calc_region_reqs(*pbase, dbi->size, order);
+		}
+	}
+	return ret;
+}
+
+int msi_doorbell_calc_pages(unsigned int order)
+{
+	struct msi_doorbell *db;
+	int ret = 0;
+
+	mutex_lock(&msi_doorbell_mutex);
+	list_for_each_entry(db, &msi_doorbell_list, next)
+		ret += calc_dbinfo_reqs(&db->info, order);
+
+	mutex_unlock(&msi_doorbell_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_calc_pages);
-- 
1.9.1

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

* [PATCH v12 06/11] genirq/msi: msi_doorbell_safe
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (4 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 05/11] genirq/msi: msi_doorbell_calc_pages Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 07/11] irqchip/gic-v2m: Register the MSI global doorbell Eric Auger
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

msi_doorbell_safe returns whether all the registered doorbells
implement irq_remapping.

IRQ remapping is the x86 terminology (IRQ translation used on ARM).
The MSI controller topology is safe if all the registered doorbells
implement IRQ remapping.

This safety notion is used on ARM when assigning PCIe devices. If
any of the MSI doorbell is unsafe, the MSI assignment gets considered
unsafe.

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

---

v11 -> v12:
- reword the commit message
---
 include/linux/msi-doorbell.h | 13 +++++++++++++
 kernel/irq/msi-doorbell.c    |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
index bbedb3b..696e0bd 100644
--- a/include/linux/msi-doorbell.h
+++ b/include/linux/msi-doorbell.h
@@ -47,6 +47,14 @@ void msi_doorbell_unregister_global(struct msi_doorbell_info *db);
  */
 int msi_doorbell_calc_pages(unsigned int order);
 
+/**
+ * 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 struct msi_doorbell_info *
@@ -64,6 +72,11 @@ static inline int msi_doorbell_calc_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 5c6c2aa..f8f0d24 100644
--- a/kernel/irq/msi-doorbell.c
+++ b/kernel/irq/msi-doorbell.c
@@ -130,3 +130,9 @@ int msi_doorbell_calc_pages(unsigned int order)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(msi_doorbell_calc_pages);
+
+bool msi_doorbell_safe(void)
+{
+	return !nb_unsafe_doorbells;
+}
+EXPORT_SYMBOL_GPL(msi_doorbell_safe);
-- 
1.9.1

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

* [PATCH v12 07/11] irqchip/gic-v2m: Register the MSI global doorbell
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (5 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 06/11] genirq/msi: msi_doorbell_safe Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 08/11] irqchip/gicv3-its: " Eric Auger
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

Use the msi-doorbell API to register the global doorbell
and implement the irq_get_msi_doorbell_info

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

---
v11 -> v12:
- use irq_get_msi_doorbell_info new name
- simplify error handling

v10 -> v11:
- use the new registration API and re-implement the msi_doorbell_info
  ops

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 | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index ad0d296..37d8437 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:
@@ -68,6 +70,7 @@ struct v2m_data {
 	u32 spi_offset;		/* offset to be subtracted from SPI number */
 	unsigned long *bm;	/* MSI vector bitmap */
 	u32 flags;		/* v2m flags for specific implementation */
+	struct msi_doorbell_info *doorbell_info; /* MSI doorbell */
 };
 
 static void gicv2m_mask_msi_irq(struct irq_data *d)
@@ -109,13 +112,22 @@ static void gicv2m_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 		msg->data -= v2m->spi_offset;
 }
 
+static struct msi_doorbell_info *
+gicv2m_irq_get_msi_doorbell_info(struct irq_data *data)
+{
+	struct v2m_data *v2m = irq_data_get_irq_chip_data(data);
+
+	return v2m->doorbell_info;
+}
+
 static struct irq_chip gicv2m_irq_chip = {
-	.name			= "GICv2m",
-	.irq_mask		= irq_chip_mask_parent,
-	.irq_unmask		= irq_chip_unmask_parent,
-	.irq_eoi		= irq_chip_eoi_parent,
-	.irq_set_affinity	= irq_chip_set_affinity_parent,
-	.irq_compose_msi_msg	= gicv2m_compose_msi_msg,
+	.name				= "GICv2m",
+	.irq_mask			= irq_chip_mask_parent,
+	.irq_unmask			= irq_chip_unmask_parent,
+	.irq_eoi			= irq_chip_eoi_parent,
+	.irq_set_affinity		= irq_chip_set_affinity_parent,
+	.irq_compose_msi_msg		= gicv2m_compose_msi_msg,
+	.irq_get_msi_doorbell_info	= gicv2m_irq_get_msi_doorbell_info,
 };
 
 static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
@@ -366,12 +378,21 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
 		goto err_iounmap;
 	}
 
+	v2m->doorbell_info =
+		msi_doorbell_register_global(v2m->res.start, sizeof(u32),
+					     IOMMU_WRITE | IOMMU_MMIO, false);
+	if (!v2m->doorbell_info) {
+		ret = -ENOMEM;
+		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] 15+ messages in thread

* [PATCH v12 08/11] irqchip/gicv3-its: Register the MSI global doorbell
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (6 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 07/11] irqchip/gic-v2m: Register the MSI global doorbell Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 09/11] genirq/msi: Introduce msi_desc flags Eric Auger
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

This patch adds the registration of the MSI global doorbell in
gicv3-its driver plus the implementation for irq_chip
irq_get_msi_doorbell_info ops.

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

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

---

v11 -> v12:
- use new irq_get_msi_doorbell_info name
- simplify error handling

v10 -> v11:
- adapt to new doorbell registration API and implement msi_doorbell_info
---
 drivers/irqchip/irq-gic-v3-its.c | 67 ++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 19 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 5eb1f9e..9543aac 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>
@@ -71,19 +73,20 @@ struct its_baser {
  * list of devices writing to it.
  */
 struct its_node {
-	raw_spinlock_t		lock;
-	struct list_head	entry;
-	void __iomem		*base;
-	unsigned long		phys_base;
-	struct its_cmd_block	*cmd_base;
-	struct its_cmd_block	*cmd_write;
-	struct its_baser	tables[GITS_BASER_NR_REGS];
-	struct its_collection	*collections;
-	struct list_head	its_device_list;
-	u64			flags;
-	u32			ite_size;
-	u32			device_ids;
-	int			numa_node;
+	raw_spinlock_t			lock;
+	struct list_head		entry;
+	void __iomem			*base;
+	unsigned long			phys_base;
+	struct its_cmd_block		*cmd_base;
+	struct its_cmd_block		*cmd_write;
+	struct its_baser		tables[GITS_BASER_NR_REGS];
+	struct its_collection		*collections;
+	struct list_head		its_device_list;
+	u64				flags;
+	u32				ite_size;
+	u32				device_ids;
+	int				numa_node;
+	struct msi_doorbell_info	*doorbell_info;
 };
 
 #define ITS_ITT_ALIGN		SZ_256
@@ -656,13 +659,24 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
 	msg->data		= its_get_event_id(d);
 }
 
+static struct msi_doorbell_info *
+its_irq_get_msi_doorbell_info(struct irq_data *d)
+{
+	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+	struct its_node *its = its_dev->its;
+
+	return its->doorbell_info;
+}
+
+
 static struct irq_chip its_irq_chip = {
-	.name			= "ITS",
-	.irq_mask		= its_mask_irq,
-	.irq_unmask		= its_unmask_irq,
-	.irq_eoi		= irq_chip_eoi_parent,
-	.irq_set_affinity	= its_set_affinity,
-	.irq_compose_msi_msg	= its_irq_compose_msi_msg,
+	.name				= "ITS",
+	.irq_mask			= its_mask_irq,
+	.irq_unmask			= its_unmask_irq,
+	.irq_eoi			= irq_chip_eoi_parent,
+	.irq_set_affinity		= its_set_affinity,
+	.irq_compose_msi_msg		= its_irq_compose_msi_msg,
+	.irq_get_msi_doorbell_info	= its_irq_get_msi_doorbell_info,
 };
 
 /*
@@ -1607,6 +1621,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 +1629,24 @@ static int __init its_probe(struct device_node *node,
 			goto out_free_tables;
 		}
 
+		translater = its->phys_base + GITS_TRANSLATER;
+		its->doorbell_info =
+			msi_doorbell_register_global(translater, sizeof(u32),
+						     IOMMU_WRITE | IOMMU_MMIO,
+						     true);
+
+		if (!its->doorbell_info)  {
+			err = -ENOMEM;
+			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_global(its->doorbell_info);
 			goto out_free_tables;
 		}
 
-- 
1.9.1

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

* [PATCH v12 09/11] genirq/msi: Introduce msi_desc flags
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (7 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 08/11] irqchip/gicv3-its: " Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-09  6:52   ` Auger Eric
  2016-08-02 17:23 ` [PATCH v12 10/11] genirq/msi: Map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Eric Auger
  2016-08-02 17:23 ` [PATCH v12 11/11] genirq/msi: Use the MSI doorbell's IOVA when requested Eric Auger
  10 siblings, 1 reply; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

This new flags member is meant to store additional information about
the msi descriptor, starting with allocation status information.

MSI_DESC_FLAG_ALLOCATED bit tells the associated base IRQ is allocated.
This information is currently used at deallocation time. We also
introduce MSI_DESC_FLAG_FUNCTIONAL telling the MSIs are functional.

For the time being ALLOCATED and FUNCTIONAL are set at the same time
but this is going to change in subsequent patch. Indeed in some situations
some additional tasks need to be carried out for the MSI to be functional.
For instance the MSI doorbell may need to be mapped in an IOMMU.

FUNCTIONAL value already gets used when enumerating the usable MSIs in
msix_capability_init.

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

---

v12: new
---
 drivers/pci/msi.c   |  2 +-
 include/linux/msi.h | 14 ++++++++++++++
 kernel/irq/msi.c    |  7 ++++++-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index a080f44..d7733ea 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -793,7 +793,7 @@ out_avail:
 		int avail = 0;
 
 		for_each_pci_msi_entry(entry, dev) {
-			if (entry->irq != 0)
+			if (entry->flags & MSI_DESC_FLAG_FUNCTIONAL)
 				avail++;
 		}
 		if (avail != 0)
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 8b425c6..18f894f 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -47,6 +47,7 @@ struct fsl_mc_msi_desc {
  * @nvec_used:	The number of vectors used
  * @dev:	Pointer to the device which uses this descriptor
  * @msg:	The last set MSI message cached for reuse
+ * @flags:	flags to describe the MSI descriptor status or features
  *
  * @masked:	[PCI MSI/X] Mask bits
  * @is_msix:	[PCI MSI/X] True if MSI-X
@@ -67,6 +68,7 @@ struct msi_desc {
 	unsigned int			nvec_used;
 	struct device			*dev;
 	struct msi_msg			msg;
+	u32						flags;
 
 	union {
 		/* PCI MSI/X specific data */
@@ -99,6 +101,18 @@ struct msi_desc {
 	};
 };
 
+/* Flags for msi_desc */
+enum {
+	/* the base IRQ is allocated */
+	MSI_DESC_FLAG_ALLOCATED	=	(1 << 0),
+	/**
+	 * the MSI is functional; in some cases the fact the base IRQ is
+	 * allocated is not sufficient for the MSIs to be functional: for
+	 * example the MSI doorbell(s) may need to be IOMMU mapped.
+	 */
+	MSI_DESC_FLAG_FUNCTIONAL =	(1 << 1),
+};
+
 /* Helpers to hide struct msi_desc implementation details */
 #define msi_desc_to_dev(desc)		((desc)->dev)
 #define dev_to_msi_list(dev)		(&(dev)->msi_list)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 72bf4d6..9b93766 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -361,6 +361,9 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
 			return ret;
 		}
 
+		desc->flags |= MSI_DESC_FLAG_ALLOCATED;
+		desc->flags |= MSI_DESC_FLAG_FUNCTIONAL;
+
 		for (i = 0; i < desc->nvec_used; i++)
 			irq_set_msi_desc_off(virq, i, desc);
 	}
@@ -395,9 +398,11 @@ void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
 		 * enough that there is no IRQ associated to this
 		 * entry. If that's the case, don't do anything.
 		 */
-		if (desc->irq) {
+		if (desc->flags & MSI_DESC_FLAG_ALLOCATED) {
 			irq_domain_free_irqs(desc->irq, desc->nvec_used);
 			desc->irq = 0;
+			desc->flags &= ~MSI_DESC_FLAG_ALLOCATED;
+			desc->flags &= ~MSI_DESC_FLAG_FUNCTIONAL;
 		}
 	}
 }
-- 
1.9.1

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

* [PATCH v12 10/11] genirq/msi: Map/unmap the MSI doorbells on msi_domain_alloc/free_irqs
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (8 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 09/11] genirq/msi: Introduce msi_desc flags Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  2016-08-02 17:23 ` [PATCH v12 11/11] genirq/msi: Use the MSI doorbell's IOVA when requested Eric Auger
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

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.

If we detect the device sending MSIs is in front of an IOMMU that do not
bypass MSIs but we can't find any doorbell to map we fail. This means we
currently do not support MSI controllers inbetween the device and the
IOMMU. In the future, those controllers, typically integrated into the
PCI host controller may also register a doorbell declared as not mappable.

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

---
v11 -> v12:
- introduce intermediate helpers:
  msi_get_doorbell_info, msi_map_global_doorbell, msi_map_percpu_doorbell
- add kernel-doc comments
- remove desc->irq reset and cleanup in case of failure and set
  MSI_DESC_FLAG_FUNCTION instead
- add comments

v10 -> v11:
- restore v9 version based on irq_chip msi_doorbell_info

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 | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 148 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 9b93766..6a3cb14 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,131 @@ int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
 }
 
 /**
+ * msi_get_doorbell_info - return the MSI doorbell descriptor corresponding
+ * to an irq data
+ * @data: irq data handle
+ *
+ * Return: the doorbell descriptor pointer if any, NULL if none, an ERR_PTR
+ * otherwise
+ */
+static struct msi_doorbell_info *msi_get_doorbell_info(struct irq_data *data)
+{
+	struct irq_chip *chip;
+
+	while (data) {
+		chip = irq_data_get_irq_chip(data);
+		if (chip->irq_get_msi_doorbell_info)
+			break;
+		data = data->parent_data;
+	}
+
+	if (!data)
+		return NULL;
+
+	return chip->irq_get_msi_doorbell_info(data);
+}
+
+/**
+ * msi_map_global_doorbell - iommu map/unmap the global doorbell physical
+ * address
+ * @domain: iommu domain the mapping is associated to
+ * @dbi: doorbell descriptor
+ * @map: true if map operation, false if unmap operation
+ *
+ * Return: 0 on success or an error code
+ */
+static int msi_map_global_doorbell(struct iommu_domain *domain,
+			       const struct msi_doorbell_info *dbi, bool map)
+{
+	dma_addr_t iova;
+	int ret = 0;
+
+	if (map)
+		ret = iommu_msi_get_doorbell_iova(domain, dbi->global_doorbell,
+						  dbi->size, dbi->prot, &iova);
+	else
+		iommu_msi_put_doorbell_iova(domain, dbi->global_doorbell);
+	return ret;
+}
+
+/**
+ * msi_map_percpu_doorbell - iommu map/unmap the percpu doorbell physical
+ * addresses
+ * @domain: iommu domain the mapping is associated to
+ * @dbi: doorbell descriptor
+ * @map: true if map operation, false if unmap operation
+ *
+ * Return: 0 on success or an error code
+ */
+static int msi_map_percpu_doorbell(struct iommu_domain *domain,
+			       const struct msi_doorbell_info *dbi, bool map)
+{
+	int cpu, ret;
+
+	for_each_possible_cpu(cpu) {
+		phys_addr_t __percpu *db_addr;
+		dma_addr_t iova;
+
+		db_addr = per_cpu_ptr(dbi->percpu_doorbells, cpu);
+
+		if (map) {
+			ret = iommu_msi_get_doorbell_iova(domain, *db_addr,
+							  dbi->size, dbi->prot,
+							  &iova);
+			if (ret)
+				return ret;
+		} else {
+			iommu_msi_put_doorbell_iova(domain, *db_addr);
+		}
+	}
+	return 0;
+}
+
+/**
+ * msi_handle_doorbell_mappings - IOMMU map/unmap any MSI doorbell associated
+ * to the irq data handle
+ * @data: irq data handle
+ * @map: true if map operation, false if unmap operation
+ *
+ * In case the irq data corresponds to an MSI sent by a device in front of
+ * an IOMMU and this latter does not bypass MSI transactions,
+ * traverse the irq domain hierarchy to retrieve the MSI doorbells and
+ * iommu_map/unmap them according to @map boolean.
+ *
+ * Return 0 on success or if no action is required, or an error code
+ */
+static int msi_handle_doorbell_mappings(struct irq_data *data, bool map)
+{
+	const struct msi_doorbell_info *dbi;
+	struct iommu_domain *domain;
+	struct device *dev;
+
+	/* Is the MSI address translated by an IOMMU? */
+	dev = msi_desc_to_dev(irq_data_get_msi_desc(data));
+	domain = iommu_msi_domain(dev);
+	if (!domain)
+		return 0;
+
+	/**
+	 * Do we find a doorbell to IOMMU map?
+	 * If we don't either the doorbell registration failed, or
+	 * the actual MSI controller did not register its doorbell:
+	 * either the MSI controller is behind the IOMMU and the MSI
+	 * controller should have registered its doorbell; or the MSI
+	 * controller is inbetween the device and the IOMMU. We currently
+	 * do not support this case.
+	 */
+	dbi = msi_get_doorbell_info(data);
+	if (!dbi)
+		return -ENODEV;
+
+	if (dbi->doorbell_is_percpu)
+		return msi_map_percpu_doorbell(domain, dbi, map);
+	else
+		return msi_map_global_doorbell(domain, dbi, map);
+}
+
+/**
  * 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
@@ -354,18 +482,23 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
 					       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;
+			goto error;
 		}
 
 		desc->flags |= MSI_DESC_FLAG_ALLOCATED;
-		desc->flags |= MSI_DESC_FLAG_FUNCTIONAL;
 
 		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)
+				goto error;
+		}
+
+		desc->flags |= MSI_DESC_FLAG_FUNCTIONAL;
 	}
 
 	if (ops->msi_finish)
@@ -380,6 +513,12 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
 	}
 
 	return 0;
+error:
+	if (ops->handle_error)
+		ret = ops->handle_error(domain, desc, ret);
+	if (ops->msi_finish)
+		ops->msi_finish(&arg, ret);
+	return ret;
 }
 
 /**
@@ -399,6 +538,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->flags & MSI_DESC_FLAG_ALLOCATED) {
+			struct irq_data *d = irq_get_irq_data(desc->irq);
+
+			msi_handle_doorbell_mappings(d, false);
 			irq_domain_free_irqs(desc->irq, desc->nvec_used);
 			desc->irq = 0;
 			desc->flags &= ~MSI_DESC_FLAG_ALLOCATED;
-- 
1.9.1

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

* [PATCH v12 11/11] genirq/msi: Use the MSI doorbell's IOVA when requested
  2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
                   ` (9 preceding siblings ...)
  2016-08-02 17:23 ` [PATCH v12 10/11] genirq/msi: Map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Eric Auger
@ 2016-08-02 17:23 ` Eric Auger
  10 siblings, 0 replies; 15+ messages in thread
From: Eric Auger @ 2016-08-02 17:23 UTC (permalink / raw)
  To: eric.auger, eric.auger.pro, christoffer.dall, marc.zyngier,
	robin.murphy, alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: kvm, drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

Currently the msi_compose() uses the doorbell's physical address.

However in case the MSI doorbell is accessed through an IOMMU that
does not bypass the MSI transactions we need to use an IOVA instead.

iommu_msi_msg_pa_to_va helper, part of the iommu-msi API, detects
the above case and swaps the MSI doorbell physical address with a
pre-allocated/mapped IOVA.

That way the device will send the MSI with this IOVA and the address
will be translated by the IOMMU into the target MSI doorbell PA.

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

---
v11 -> v12:
- remove WARN_ON. iommu domain cannot dissapear while msi still are enabled
  so the userspace cannot induce a kernel panic.

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 6a3cb14..02c1c71 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));
+		ret = iommu_msi_msg_pa_to_va(dev, msg);
+	}
 
 	return ret;
 }
-- 
1.9.1

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

* Re: [PATCH v12 09/11] genirq/msi: Introduce msi_desc flags
  2016-08-02 17:23 ` [PATCH v12 09/11] genirq/msi: Introduce msi_desc flags Eric Auger
@ 2016-08-09  6:52   ` Auger Eric
  0 siblings, 0 replies; 15+ messages in thread
From: Auger Eric @ 2016-08-09  6:52 UTC (permalink / raw)
  To: eric.auger.pro, christoffer.dall, marc.zyngier, robin.murphy,
	alex.williamson, will.deacon, joro, tglx, jason,
	linux-arm-kernel
  Cc: drjones, kvm, Jean-Philippe.Brucker, Manish.Jaggi, p.fedin,
	linux-kernel, Bharat.Bhushan, iommu, pranav.sawargaonkar,
	dennis.chen, robert.richter, yehuday

Hi,

On 02/08/2016 19:23, Eric Auger wrote:
> This new flags member is meant to store additional information about
> the msi descriptor, starting with allocation status information.
> 
> MSI_DESC_FLAG_ALLOCATED bit tells the associated base IRQ is allocated.
> This information is currently used at deallocation time. We also
> introduce MSI_DESC_FLAG_FUNCTIONAL telling the MSIs are functional.
> 
> For the time being ALLOCATED and FUNCTIONAL are set at the same time
> but this is going to change in subsequent patch. Indeed in some situations
> some additional tasks need to be carried out for the MSI to be functional.
> For instance the MSI doorbell may need to be mapped in an IOMMU.
> 
> FUNCTIONAL value already gets used when enumerating the usable MSIs in
> msix_capability_init.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> v12: new
> ---
>  drivers/pci/msi.c   |  2 +-
>  include/linux/msi.h | 14 ++++++++++++++
>  kernel/irq/msi.c    |  7 ++++++-
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index a080f44..d7733ea 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -793,7 +793,7 @@ out_avail:
>  		int avail = 0;
>  
>  		for_each_pci_msi_entry(entry, dev) {
> -			if (entry->irq != 0)
> +			if (entry->flags & MSI_DESC_FLAG_FUNCTIONAL)
>  				avail++;
>  		}
>  		if (avail != 0)
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 8b425c6..18f894f 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -47,6 +47,7 @@ struct fsl_mc_msi_desc {
>   * @nvec_used:	The number of vectors used
>   * @dev:	Pointer to the device which uses this descriptor
>   * @msg:	The last set MSI message cached for reuse
> + * @flags:	flags to describe the MSI descriptor status or features
>   *
>   * @masked:	[PCI MSI/X] Mask bits
>   * @is_msix:	[PCI MSI/X] True if MSI-X
> @@ -67,6 +68,7 @@ struct msi_desc {
>  	unsigned int			nvec_used;
>  	struct device			*dev;
>  	struct msi_msg			msg;
> +	u32						flags;
I will fix this bad alignment on next version

>  
>  	union {
>  		/* PCI MSI/X specific data */
> @@ -99,6 +101,18 @@ struct msi_desc {
>  	};
>  };
>  
> +/* Flags for msi_desc */
> +enum {
> +	/* the base IRQ is allocated */
> +	MSI_DESC_FLAG_ALLOCATED	=	(1 << 0),
> +	/**
> +	 * the MSI is functional; in some cases the fact the base IRQ is
> +	 * allocated is not sufficient for the MSIs to be functional: for
> +	 * example the MSI doorbell(s) may need to be IOMMU mapped.
> +	 */
> +	MSI_DESC_FLAG_FUNCTIONAL =	(1 << 1),
> +};
> +
>  /* Helpers to hide struct msi_desc implementation details */
>  #define msi_desc_to_dev(desc)		((desc)->dev)
>  #define dev_to_msi_list(dev)		(&(dev)->msi_list)
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index 72bf4d6..9b93766 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -361,6 +361,9 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
>  			return ret;
>  		}
>  
> +		desc->flags |= MSI_DESC_FLAG_ALLOCATED;
> +		desc->flags |= MSI_DESC_FLAG_FUNCTIONAL;
> +
>  		for (i = 0; i < desc->nvec_used; i++)
>  			irq_set_msi_desc_off(virq, i, desc);
>  	}
> @@ -395,9 +398,11 @@ void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
>  		 * enough that there is no IRQ associated to this
>  		 * entry. If that's the case, don't do anything.
>  		 */
> -		if (desc->irq) {
> +		if (desc->flags & MSI_DESC_FLAG_ALLOCATED) {
>  			irq_domain_free_irqs(desc->irq, desc->nvec_used);
>  			desc->irq = 0;
> +			desc->flags &= ~MSI_DESC_FLAG_ALLOCATED;
> +			desc->flags &= ~MSI_DESC_FLAG_FUNCTIONAL;
Also I will combine those settings

Thanks

Eric
>  		}
>  	}
>  }
> 

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

* Re: [PATCH v12 02/11] genirq/msi: msi_compose wrapper
  2016-08-02 17:23 ` [PATCH v12 02/11] genirq/msi: msi_compose wrapper Eric Auger
@ 2016-08-09  9:19   ` Thomas Gleixner
  2016-08-10  8:48     ` Auger Eric
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2016-08-09  9:19 UTC (permalink / raw)
  To: Eric Auger
  Cc: eric.auger.pro, christoffer.dall, marc.zyngier, robin.murphy,
	alex.williamson, will.deacon, joro, jason, linux-arm-kernel, kvm,
	drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

On Tue, 2 Aug 2016, Eric Auger wrote:

> 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.

I just can't find how you do that. After applying the series the

> +	if (erase)
> +		memset(msg, 0, sizeof(*msg));

branch is still just a memset(). The wrapper is fine for the compose side, but
having the extra argument just to wrap the memset() for no gain is silly.

Thanks,

	tglx

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

* Re: [PATCH v12 02/11] genirq/msi: msi_compose wrapper
  2016-08-09  9:19   ` Thomas Gleixner
@ 2016-08-10  8:48     ` Auger Eric
  0 siblings, 0 replies; 15+ messages in thread
From: Auger Eric @ 2016-08-10  8:48 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: eric.auger.pro, christoffer.dall, marc.zyngier, robin.murphy,
	alex.williamson, will.deacon, joro, jason, linux-arm-kernel, kvm,
	drjones, linux-kernel, Bharat.Bhushan, pranav.sawargaonkar,
	p.fedin, iommu, Jean-Philippe.Brucker, yehuday, Manish.Jaggi,
	robert.richter, dennis.chen

Hi Thomas,

On 09/08/2016 11:19, Thomas Gleixner wrote:
> On Tue, 2 Aug 2016, Eric Auger wrote:
> 
>> 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.
> 
> I just can't find how you do that. After applying the series the
> 
>> +	if (erase)
>> +		memset(msg, 0, sizeof(*msg));
> 
> branch is still just a memset(). The wrapper is fine for the compose side, but
> having the extra argument just to wrap the memset() for no gain is silly.

Yes you're right: this was true in the first releases of the series
where the iommu mapping/unmapping were done at composition & erase time.
Now the mapping/unmapping is done on msi_domain_alloc/free_irqs, this is
not mandated anymore. I will keep the wrapper for the compose side and
remove the rest + update the commit message accordingly.

Thank you for your time.

Eric
> 
> Thanks,
> 
> 	tglx
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

end of thread, other threads:[~2016-08-10 18:16 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-02 17:23 [PATCH v12 00/11] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger
2016-08-02 17:23 ` [PATCH v12 01/11] genirq/msi: export msi_get_domain_info Eric Auger
2016-08-02 17:23 ` [PATCH v12 02/11] genirq/msi: msi_compose wrapper Eric Auger
2016-08-09  9:19   ` Thomas Gleixner
2016-08-10  8:48     ` Auger Eric
2016-08-02 17:23 ` [PATCH v12 03/11] genirq: Introduce irq_get_msi_doorbell_info Eric Auger
2016-08-02 17:23 ` [PATCH v12 04/11] genirq/msi: Allow MSI doorbell (un)registration Eric Auger
2016-08-02 17:23 ` [PATCH v12 05/11] genirq/msi: msi_doorbell_calc_pages Eric Auger
2016-08-02 17:23 ` [PATCH v12 06/11] genirq/msi: msi_doorbell_safe Eric Auger
2016-08-02 17:23 ` [PATCH v12 07/11] irqchip/gic-v2m: Register the MSI global doorbell Eric Auger
2016-08-02 17:23 ` [PATCH v12 08/11] irqchip/gicv3-its: " Eric Auger
2016-08-02 17:23 ` [PATCH v12 09/11] genirq/msi: Introduce msi_desc flags Eric Auger
2016-08-09  6:52   ` Auger Eric
2016-08-02 17:23 ` [PATCH v12 10/11] genirq/msi: Map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Eric Auger
2016-08-02 17:23 ` [PATCH v12 11/11] 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).