Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1446435 > unrolled thread
| Started by | Eric Auger <eric.auger@redhat.com> |
|---|---|
| First post | 2016-07-19 15:10 +0200 |
| Last post | 2016-07-19 15:10 +0200 |
| Articles | 15 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v11 00/10] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes Eric Auger <eric.auger@redhat.com> - 2016-07-19 15:10 +0200
[PATCH v11 02/10] genirq/msi: msi_compose wrapper Eric Auger <eric.auger@redhat.com> - 2016-07-19 15:10 +0200
[PATCH v11 09/10] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Eric Auger <eric.auger@redhat.com> - 2016-07-19 15:10 +0200
Re: [PATCH v11 09/10] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Thomas Gleixner <tglx@linutronix.de> - 2016-07-20 11:10 +0200
Re: [PATCH v11 09/10] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs Auger Eric <eric.auger@redhat.com> - 2016-07-25 18:30 +0200
[PATCH v11 10/10] genirq/msi: use the MSI doorbell's IOVA when requested Eric Auger <eric.auger@redhat.com> - 2016-07-19 15:10 +0200
Re: [PATCH v11 10/10] genirq/msi: use the MSI doorbell's IOVA when requested Thomas Gleixner <tglx@linutronix.de> - 2016-07-20 11:20 +0200
Re: [PATCH v11 10/10] genirq/msi: use the MSI doorbell's IOVA when requested Auger Eric <eric.auger@redhat.com> - 2016-07-25 18:40 +0200
[PATCH v11 08/10] irqchip/gicv3-its: register the MSI global doorbell Eric Auger <eric.auger@redhat.com> - 2016-07-19 15:10 +0200
[PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe Eric Auger <eric.auger@redhat.com> - 2016-07-19 15:10 +0200
Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe Thomas Gleixner <tglx@linutronix.de> - 2016-07-20 10:20 +0200
Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe Auger Eric <eric.auger@redhat.com> - 2016-07-21 15:40 +0200
Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe Thomas Gleixner <tglx@linutronix.de> - 2016-07-22 14:50 +0200
Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe Auger Eric <eric.auger@redhat.com> - 2016-07-22 16:10 +0200
[PATCH v11 01/10] genirq/msi: export msi_get_domain_info Eric Auger <eric.auger@redhat.com> - 2016-07-19 15:10 +0200
| From | Eric Auger <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-19 15:10 +0200 |
| Subject | [PATCH v11 00/10] KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes |
| Message-ID | <rWCWB-4qE-3@gated-at.bofh.it> |
KVM PCIe/MSI passthrough on ARM/ARM64: kernel part 2/3: msi changes 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-v11 History: 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 (10): genirq/msi: export msi_get_domain_info genirq/msi: msi_compose wrapper genirq/irq: introduce msi_doorbell_info genirq/msi-doorbell: allow MSI doorbell (un)registration 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 | 23 ++++++- drivers/irqchip/irq-gic-v3-its.c | 28 ++++++++ include/linux/irq.h | 16 ++++- include/linux/msi-doorbell.h | 78 ++++++++++++++++++++++ kernel/irq/Kconfig | 4 ++ kernel/irq/Makefile | 1 + kernel/irq/msi-doorbell.c | 129 ++++++++++++++++++++++++++++++++++++ kernel/irq/msi.c | 137 +++++++++++++++++++++++++++++++++++---- 9 files changed, 404 insertions(+), 13 deletions(-) create mode 100644 include/linux/msi-doorbell.h create mode 100644 kernel/irq/msi-doorbell.c -- 1.9.1
[toc] | [next] | [standalone]
| From | Eric Auger <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-19 15:10 +0200 |
| Subject | [PATCH v11 02/10] genirq/msi: msi_compose wrapper |
| Message-ID | <rWCWC-4qE-21@gated-at.bofh.it> |
| In reply to | #1446435 |
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
[toc] | [prev] | [next] | [standalone]
| From | Eric Auger <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-19 15:10 +0200 |
| Subject | [PATCH v11 09/10] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs |
| Message-ID | <rWCWC-4qE-35@gated-at.bofh.it> |
| In reply to | #1446435 |
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>
---
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 | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 101 insertions(+), 8 deletions(-)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 72bf4d6..69b5b19 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,74 @@ 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 irq_chip *chip;
+ struct device *dev;
+ dma_addr_t iova;
+ 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 = irq_data_get_irq_chip(data);
+ if (chip->msi_doorbell_info)
+ break;
+ }
+ data = data->parent_data;
+ }
+
+ if (!data)
+ return 0;
+
+ dbinfo = chip->msi_doorbell_info(data);
+ if (!dbinfo)
+ return -EINVAL;
+
+ 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 +423,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 +460,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 +486,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
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-07-20 11:10 +0200 |
| Subject | Re: [PATCH v11 09/10] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs |
| Message-ID | <rWVFT-7RJ-15@gated-at.bofh.it> |
| In reply to | #1446442 |
On Tue, 19 Jul 2016, Eric Auger wrote:
> /**
> + * 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
> + */
Please run that through the kernel doc generator. It does not work that way.
The format is:
/**
* function_name - Short function description
* @arg1: Description of arg1
* @argument2: Description of argument2
*
* Long explanation including documentation of the return values.
*/
> +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 irq_chip *chip;
> + struct device *dev;
> + dma_addr_t iova;
> + 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 = irq_data_get_irq_chip(data);
> + if (chip->msi_doorbell_info)
> + break;
> + }
> + data = data->parent_data;
> + }
Please split that out into a seperate function
struct irq_data *msi_get_doorbell_info(data)
{
.....
if (chip->msi_doorbell_info)
return chip->msi_get_doorbell_info(data);
}
return NULL;
}
info = msi_get_doorbell_info(data);
.....
> + if (!data)
> + return 0;
> +
> + dbinfo = chip->msi_doorbell_info(data);
> + if (!dbinfo)
> + return -EINVAL;
> +
> + 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);
> + }
You can spare an indentation level with a helper function
if (!dbinfo->doorbell_is_percpu)
return msi_map_global_doorbell(domain, dbinfo);
> +
> + /* 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;
> + }
> + }
Same here:
for_each_possible_cpu(cpu) {
ret = msi_map_percpu_doorbell(domain, cpu);
if (ret)
return ret;
}
return 0;
Hmm?
> +
> + 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 +423,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;
How is that supposed to work? You clear desc->irq and then you call
ops->handle_error.
Why are you adding this extra stuff here? Look at the call sites of
msi_domain_alloc_irqs(). All of them use msi_domain_free_irqs() in case of
error. There is no reason why you can't do the same....
> /**
> @@ -396,6 +486,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;
Can you please restructure the code so it reads
if (desc->irq)
continue;
msi_handle_doorbell_mappings(irq_get_irq_data(desc->irq),
false);
irq_domain_free_irqs(desc->irq, desc->nvec_used);
desc->irq = 0;
Just blindly whacking stuff into the 80 char limit is not helping readability.
Thanks,
tglx
[toc] | [prev] | [next] | [standalone]
| From | Auger Eric <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-25 18:30 +0200 |
| Subject | Re: [PATCH v11 09/10] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs |
| Message-ID | <rYQVs-6No-51@gated-at.bofh.it> |
| In reply to | #1447117 |
Hi Thomas,
On 20/07/2016 11:04, Thomas Gleixner wrote:
> On Tue, 19 Jul 2016, Eric Auger wrote:
>> /**
>> + * 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
>> + */
>
>
> Please run that through the kernel doc generator. It does not work that way.
>
> The format is:
>
> /**
> * function_name - Short function description
> * @arg1: Description of arg1
> * @argument2: Description of argument2
> *
> * Long explanation including documentation of the return values.
> */
>
>> +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 irq_chip *chip;
>> + struct device *dev;
>> + dma_addr_t iova;
>> + 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 = irq_data_get_irq_chip(data);
>> + if (chip->msi_doorbell_info)
>> + break;
>> + }
>> + data = data->parent_data;
>> + }
>
> Please split that out into a seperate function
>
> struct irq_data *msi_get_doorbell_info(data)
> {
> .....
> if (chip->msi_doorbell_info)
> return chip->msi_get_doorbell_info(data);
> }
> return NULL;
> }
>
> info = msi_get_doorbell_info(data);
> .....
>
>> + if (!data)
>> + return 0;
>> +
>> + dbinfo = chip->msi_doorbell_info(data);
>> + if (!dbinfo)
>> + return -EINVAL;
>> +
>> + 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);
>> + }
>
> You can spare an indentation level with a helper function
>
> if (!dbinfo->doorbell_is_percpu)
> return msi_map_global_doorbell(domain, dbinfo);
>
>> +
>> + /* 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;
>> + }
>> + }
>
> Same here:
>
> for_each_possible_cpu(cpu) {
> ret = msi_map_percpu_doorbell(domain, cpu);
> if (ret)
> return ret;
> }
> return 0;
>
> Hmm?
>
>> +
>> + 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 +423,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;
>
> How is that supposed to work? You clear desc->irq and then you call
> ops->handle_error.
if I don't clear the desc->irq I enter an infinite loop in pci_enable_msix_range.
This happens because msix_capability_init and pcie_enable_msix returns 1.
In msix_capability_init, at out_avail: we enumerate the msi_desc which have a non
zero irq, hence the returned value equal to 1.
Currently the only handle_error ops I found, pci_msi_domain_handle_error does not
use irq field so works although questionable.
As for the irq_domain_free_irqs I think I can remove it since handled later.
How do you advise to handle the above situation?
Thanks
Eric
>
> Why are you adding this extra stuff here? Look at the call sites of
> msi_domain_alloc_irqs(). All of them use msi_domain_free_irqs() in case of
> error. There is no reason why you can't do the same....
>
>> /**
>> @@ -396,6 +486,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;
>
> Can you please restructure the code so it reads
>
> if (desc->irq)
> continue;
>
> msi_handle_doorbell_mappings(irq_get_irq_data(desc->irq),
> false);
> irq_domain_free_irqs(desc->irq, desc->nvec_used);
> desc->irq = 0;
>
> Just blindly whacking stuff into the 80 char limit is not helping readability.
>
> Thanks,
>
> tglx
>
[toc] | [prev] | [next] | [standalone]
| From | Eric Auger <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-19 15:10 +0200 |
| Subject | [PATCH v11 10/10] genirq/msi: use the MSI doorbell's IOVA when requested |
| Message-ID | <rWCWC-4qE-31@gated-at.bofh.it> |
| In reply to | #1446435 |
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 69b5b19..e375544 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
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-07-20 11:20 +0200 |
| Subject | Re: [PATCH v11 10/10] genirq/msi: use the MSI doorbell's IOVA when requested |
| Message-ID | <rWVPA-7V0-11@gated-at.bofh.it> |
| In reply to | #1446443 |
On Tue, 19 Jul 2016, Eric Auger wrote:
First of all - valid for all patches:
Subject: sys/subsys: Sentence starts with an uppercase letter
Now for this particular one:
genirq/msi: use the MSI doorbell's IOVA when requested
> 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).
This changelog is completely useless. At least I cannot figure out what that
patch actually does. And the implementation is not self explaining either.
> @@ -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));
What the heck is this call doing? And why is there only a WARN_ON and not a
proper error return code handling?
Thanks,
tglx
[toc] | [prev] | [next] | [standalone]
| From | Auger Eric <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-25 18:40 +0200 |
| Subject | Re: [PATCH v11 10/10] genirq/msi: use the MSI doorbell's IOVA when requested |
| Message-ID | <rYR58-6Qr-11@gated-at.bofh.it> |
| In reply to | #1447120 |
Hi Thomas,
On 20/07/2016 11:09, Thomas Gleixner wrote:
> On Tue, 19 Jul 2016, Eric Auger wrote:
>
> First of all - valid for all patches:
>
> Subject: sys/subsys: Sentence starts with an uppercase letter
OK understood.
>
> Now for this particular one:
>
> genirq/msi: use the MSI doorbell's IOVA when requested
>
>> 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).
>
> This changelog is completely useless. At least I cannot figure out what that
> patch actually does. And the implementation is not self explaining either.
>
>> @@ -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));
>
> What the heck is this call doing? And why is there only a WARN_ON and not a
> proper error return code handling?
iommu_msi_msg_pa_to_va is part of the new iommu-msi API introduced in PART I of
this series. This helper function detects the physical address found in the
MSI message has a corresponding allocated IOVA. This happens if the MSI doorbell
is accessed through an IOMMU and this IOMMU do not bypass the MSI addresses
(ARM case). Allocation of this IOVA was performed in the previous patch.
So, if this is the case, the physical address is swapped with the IOVA
address. That way the PCIe device will send the MSI with this IOVA and
the address will be translated by the IOMMU into the target MSI doorbell PA.
Hope this clarifies
Thanks
Eric
>
> Thanks,
>
> tglx
>
[toc] | [prev] | [next] | [standalone]
| From | Eric Auger <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-19 15:10 +0200 |
| Subject | [PATCH v11 08/10] irqchip/gicv3-its: register the MSI global doorbell |
| Message-ID | <rWCWD-4qE-59@gated-at.bofh.it> |
| In reply to | #1446435 |
This patch adds the registration of the MSI global doorbell in
gicv3-its driver plus the implementation for irq_chip
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>
---
v10 -> v11:
- adapt to new doorbell registration API and implement msi_doorbell_info
---
drivers/irqchip/irq-gic-v3-its.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 5eb1f9e..23102e3 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>
@@ -84,6 +86,7 @@ struct its_node {
u32 ite_size;
u32 device_ids;
int numa_node;
+ struct irq_chip_msi_doorbell_info *doorbell_info;
};
#define ITS_ITT_ALIGN SZ_256
@@ -656,6 +659,16 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
msg->data = its_get_event_id(d);
}
+static struct irq_chip_msi_doorbell_info *
+its_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,
@@ -663,6 +676,7 @@ static struct irq_chip its_irq_chip = {
.irq_eoi = irq_chip_eoi_parent,
.irq_set_affinity = its_set_affinity,
.irq_compose_msi_msg = its_irq_compose_msi_msg,
+ .msi_doorbell_info = its_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,23 @@ 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 (IS_ERR_OR_NULL(its->doorbell_info)) {
+ 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
[toc] | [prev] | [next] | [standalone]
| From | Eric Auger <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-19 15:10 +0200 |
| Subject | [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe |
| Message-ID | <rWCWD-4qE-49@gated-at.bofh.it> |
| In reply to | #1446435 |
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 | 15 +++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h
index 146bfbc..9c30141 100644
--- a/include/linux/msi-doorbell.h
+++ b/include/linux/msi-doorbell.h
@@ -43,6 +43,13 @@ void msi_doorbell_unregister_global(struct irq_chip_msi_doorbell_info *db);
*/
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 struct irq_chip_msi_doorbell_info *
@@ -61,6 +68,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 a5bde37..fa5b429 100644
--- a/kernel/irq/msi-doorbell.c
+++ b/kernel/irq/msi-doorbell.c
@@ -112,3 +112,18 @@ int msi_doorbell_pages(unsigned int order)
return ret;
}
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);
\ No newline at end of file
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-07-20 10:20 +0200 |
| Subject | Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe |
| Message-ID | <rWUTw-7mO-21@gated-at.bofh.it> |
| In reply to | #1446450 |
On Tue, 19 Jul 2016, Eric Auger wrote:
> +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;
db->info.irq_remapping is set in msi_doorbell_register(). So you can keep book
about that there. No need to iterate here.
Thanks,
tglx
[toc] | [prev] | [next] | [standalone]
| From | Auger Eric <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-21 15:40 +0200 |
| Subject | Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe |
| Message-ID | <rXmmJ-81c-25@gated-at.bofh.it> |
| In reply to | #1447076 |
Hi,
On 20/07/2016 10:12, Thomas Gleixner wrote:
> On Tue, 19 Jul 2016, Eric Auger wrote:
>> +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;
>
> db->info.irq_remapping is set in msi_doorbell_register(). So you can keep book
> about that there. No need to iterate here.
Yes makes sense to store the info at registration time. Currently this
function is not in any fast path but that's cleaner from a general
perspective. I will need to do such iteration at un-registration though.
Thanks
Eric
>
> Thanks,
>
> tglx
>
>
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-07-22 14:50 +0200 |
| Subject | Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe |
| Message-ID | <rXI3U-5Un-21@gated-at.bofh.it> |
| In reply to | #1447888 |
On Thu, 21 Jul 2016, Auger Eric wrote:
> On 20/07/2016 10:12, Thomas Gleixner wrote:
> > On Tue, 19 Jul 2016, Eric Auger wrote:
> >> +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;
> >
> > db->info.irq_remapping is set in msi_doorbell_register(). So you can keep book
> > about that there. No need to iterate here.
> Yes makes sense to store the info at registration time. Currently this
> function is not in any fast path but that's cleaner from a general
> perspective. I will need to do such iteration at un-registration though.
Two simple counter should be sufficient.
nr_registered_bells;
nr_remapped_bells;
....
Thanks,
tglx
[toc] | [prev] | [next] | [standalone]
| From | Auger Eric <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-22 16:10 +0200 |
| Subject | Re: [PATCH v11 06/10] genirq/msi-doorbell: msi_doorbell_safe |
| Message-ID | <rXJjk-6SA-13@gated-at.bofh.it> |
| In reply to | #1448566 |
Hi Thomas,
On 22/07/2016 14:44, Thomas Gleixner wrote:
> On Thu, 21 Jul 2016, Auger Eric wrote:
>> On 20/07/2016 10:12, Thomas Gleixner wrote:
>>> On Tue, 19 Jul 2016, Eric Auger wrote:
>>>> +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;
>>>
>>> db->info.irq_remapping is set in msi_doorbell_register(). So you can keep book
>>> about that there. No need to iterate here.
>> Yes makes sense to store the info at registration time. Currently this
>> function is not in any fast path but that's cleaner from a general
>> perspective. I will need to do such iteration at un-registration though.
>
> Two simple counter should be sufficient.
>
> nr_registered_bells;
> nr_remapped_bells;
Yes definitively smarter to use counters! mental viscosity.
Thanks
Eric
>
> ....
>
> Thanks,
>
> tglx
>
[toc] | [prev] | [next] | [standalone]
| From | Eric Auger <eric.auger@redhat.com> |
|---|---|
| Date | 2016-07-19 15:10 +0200 |
| Subject | [PATCH v11 01/10] genirq/msi: export msi_get_domain_info |
| Message-ID | <rWCWD-4qE-57@gated-at.bofh.it> |
| In reply to | #1446435 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web