diff mbox

[v7,7/8] genirq/msi: map/unmap the MSI doorbells on msi_domain_alloc/free_irqs

Message ID 1461085990-2547-8-git-send-email-eric.auger@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Auger April 19, 2016, 5:13 p.m. UTC
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@linaro.org>

---

v7: creation
---
 kernel/irq/msi.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 85 insertions(+), 9 deletions(-)

Comments

kernel test robot April 19, 2016, 6:04 p.m. UTC | #1
Hi,

[auto build test ERROR on tip/irq/core]
[also build test ERROR on v4.6-rc4 next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Eric-Auger/KVM-PCIe-MSI-passthrough-on-ARM-ARM64-kernel-part-2-3-msi-changes/20160420-011957
config: x86_64-lkp (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> kernel/irq/msi.c:17:38: fatal error: linux/dma-reserved-iommu.h: No such file or directory
    #include <linux/dma-reserved-iommu.h>
                                         ^
   compilation terminated.

vim +17 kernel/irq/msi.c

    11	 */
    12	#include <linux/types.h>
    13	#include <linux/device.h>
    14	#include <linux/irq.h>
    15	#include <linux/irqdomain.h>
    16	#include <linux/msi.h>
  > 17	#include <linux/dma-reserved-iommu.h>
    18	#include <linux/iommu.h>
    19	
    20	/* Temparory solution for building, will be removed later */

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Eric Auger April 20, 2016, 7:47 a.m. UTC | #2
Hi,

Both reported errors related to this series are due to the fact part n
has dependency on part n-1.

Does anyone know how to let the 0-day CI robot know about such
dependency between series?

If it is an inconvenience I can put all the patches back into the same
big series, specifying tentative patch split according to sub-systems in
the cover-letter?

Thank you in advance

Best Regards

Eric

04/19/2016 08:04 PM, kbuild test robot wrote:
> Hi,
> 
> [auto build test ERROR on tip/irq/core]
> [also build test ERROR on v4.6-rc4 next-20160419]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Eric-Auger/KVM-PCIe-MSI-passthrough-on-ARM-ARM64-kernel-part-2-3-msi-changes/20160420-011957
> config: x86_64-lkp (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>>> kernel/irq/msi.c:17:38: fatal error: linux/dma-reserved-iommu.h: No such file or directory
>     #include <linux/dma-reserved-iommu.h>
>                                          ^
>    compilation terminated.
> 
> vim +17 kernel/irq/msi.c
> 
>     11	 */
>     12	#include <linux/types.h>
>     13	#include <linux/device.h>
>     14	#include <linux/irq.h>
>     15	#include <linux/irqdomain.h>
>     16	#include <linux/msi.h>
>   > 17	#include <linux/dma-reserved-iommu.h>
>     18	#include <linux/iommu.h>
>     19	
>     20	/* Temparory solution for building, will be removed later */
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
Ye Xiaolong April 26, 2016, 1:24 a.m. UTC | #3
On Wed, Apr 20, 2016 at 09:47:19AM +0200, Eric Auger wrote:
>Hi,
>
Hi, Eric
>Both reported errors related to this series are due to the fact part n
>has dependency on part n-1.
>

If I understand correctly, what you meant is that you send patch series
(let's say B), and B has its dependency patch series A which is in-flight
and hasn't been merged, so the commit history is like:

        ---P---A---B

where P is the well-known commit in public linux tree. In this case,
0day just applied B based on P without A, thus caused the build error.

>Does anyone know how to let the 0-day CI robot know about such
>dependency between series?
>

Currently, we have proposed to add a new '--base' option for git-format-patch
to git community[1], developers could use this option to record the base tree
information which could help 0day to identify the state the patch series
applies to, thus 0day could avoid false report like this case.

For example, imagine that on top of the public commit P, you applied well-known
patches X, Y and Z from somebody else or yourself, and then built your
three-patch series A, B, C, the history would be like:

................................................
---P---X---Y---Z---A---B---C
................................................

With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
`--cover-letter` of using `Z..C` instead of `-3 C` to specify the
range), the base tree information block is shown at the end of the
first message the command outputs (either the first patch, or the
cover letter), like this:

------------
base-commit: P
prerequisite-patch-id: X
prerequisite-patch-id: Y
prerequisite-patch-id: Z
------------

Now this feature is still under review in git community[2], hope it could be
merged in next git release.


[1] http://thread.gmane.org/gmane.comp.version-control.git/286873
[2] http://thread.gmane.org/gmane.comp.version-control.git/292168

Thanks,
Xiaolong

>If it is an inconvenience I can put all the patches back into the same
>big series, specifying tentative patch split according to sub-systems in
>the cover-letter?
>
>Thank you in advance
>
>Best Regards
>
>Eric
>
>04/19/2016 08:04 PM, kbuild test robot wrote:
>> Hi,
>> 
>> [auto build test ERROR on tip/irq/core]
>> [also build test ERROR on v4.6-rc4 next-20160419]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>> 
>> url:    https://github.com/0day-ci/linux/commits/Eric-Auger/KVM-PCIe-MSI-passthrough-on-ARM-ARM64-kernel-part-2-3-msi-changes/20160420-011957
>> config: x86_64-lkp (attached as .config)
>> reproduce:
>>         # save the attached .config to linux build tree
>>         make ARCH=x86_64 
>> 
>> All errors (new ones prefixed by >>):
>> 
>>>> kernel/irq/msi.c:17:38: fatal error: linux/dma-reserved-iommu.h: No such file or directory
>>     #include <linux/dma-reserved-iommu.h>
>>                                          ^
>>    compilation terminated.
>> 
>> vim +17 kernel/irq/msi.c
>> 
>>     11	 */
>>     12	#include <linux/types.h>
>>     13	#include <linux/device.h>
>>     14	#include <linux/irq.h>
>>     15	#include <linux/irqdomain.h>
>>     16	#include <linux/msi.h>
>>   > 17	#include <linux/dma-reserved-iommu.h>
>>     18	#include <linux/iommu.h>
>>     19	
>>     20	/* Temparory solution for building, will be removed later */
>> 
>> ---
>> 0-DAY kernel test infrastructure                Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>> 
>
Eric Auger April 26, 2016, 4:43 p.m. UTC | #4
Hi Xiaolong
On 04/26/2016 03:24 AM, Ye Xiaolong wrote:
> On Wed, Apr 20, 2016 at 09:47:19AM +0200, Eric Auger wrote:
>> Hi,
>>
> Hi, Eric
>> Both reported errors related to this series are due to the fact part n
>> has dependency on part n-1.
>>
> 
> If I understand correctly, what you meant is that you send patch series
> (let's say B), and B has its dependency patch series A which is in-flight
> and hasn't been merged, so the commit history is like:
> 
>         ---P---A---B
> 
> where P is the well-known commit in public linux tree. In this case,
> 0day just applied B based on P without A, thus caused the build error.

yes that's it.
> 
>> Does anyone know how to let the 0-day CI robot know about such
>> dependency between series?
>>
> 
> Currently, we have proposed to add a new '--base' option for git-format-patch
> to git community[1], developers could use this option to record the base tree
> information which could help 0day to identify the state the patch series
> applies to, thus 0day could avoid false report like this case.
> 
> For example, imagine that on top of the public commit P, you applied well-known
> patches X, Y and Z from somebody else or yourself, and then built your
> three-patch series A, B, C, the history would be like:
> 
> ................................................
> ---P---X---Y---Z---A---B---C
> ................................................
> 
> With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
> `--cover-letter` of using `Z..C` instead of `-3 C` to specify the
> range), the base tree information block is shown at the end of the
> first message the command outputs (either the first patch, or the
> cover letter), like this:
> 
> ------------
> base-commit: P
> prerequisite-patch-id: X
> prerequisite-patch-id: Y
> prerequisite-patch-id: Z
> ------------
> 
> Now this feature is still under review in git community[2], hope it could be
> merged in next git release.

OK. That's good to know. Thank you very much for the info. I will follow
the progress then.

Best Regards

Eric
> 
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/286873
> [2] http://thread.gmane.org/gmane.comp.version-control.git/292168
> 
> Thanks,
> Xiaolong
> 
>> If it is an inconvenience I can put all the patches back into the same
>> big series, specifying tentative patch split according to sub-systems in
>> the cover-letter?
>>
>> Thank you in advance
>>
>> Best Regards
>>
>> Eric
>>
>> 04/19/2016 08:04 PM, kbuild test robot wrote:
>>> Hi,
>>>
>>> [auto build test ERROR on tip/irq/core]
>>> [also build test ERROR on v4.6-rc4 next-20160419]
>>> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>>>
>>> url:    https://github.com/0day-ci/linux/commits/Eric-Auger/KVM-PCIe-MSI-passthrough-on-ARM-ARM64-kernel-part-2-3-msi-changes/20160420-011957
>>> config: x86_64-lkp (attached as .config)
>>> reproduce:
>>>         # save the attached .config to linux build tree
>>>         make ARCH=x86_64 
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>>> kernel/irq/msi.c:17:38: fatal error: linux/dma-reserved-iommu.h: No such file or directory
>>>     #include <linux/dma-reserved-iommu.h>
>>>                                          ^
>>>    compilation terminated.
>>>
>>> vim +17 kernel/irq/msi.c
>>>
>>>     11	 */
>>>     12	#include <linux/types.h>
>>>     13	#include <linux/device.h>
>>>     14	#include <linux/irq.h>
>>>     15	#include <linux/irqdomain.h>
>>>     16	#include <linux/msi.h>
>>>   > 17	#include <linux/dma-reserved-iommu.h>
>>>     18	#include <linux/iommu.h>
>>>     19	
>>>     20	/* Temparory solution for building, will be removed later */
>>>
>>> ---
>>> 0-DAY kernel test infrastructure                Open Source Technology Center
>>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>>
>>
diff mbox

Patch

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 72bf4d6..e45907e 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -14,6 +14,8 @@ 
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/msi.h>
+#include <linux/dma-reserved-iommu.h>
+#include <linux/iommu.h>
 
 /* Temparory solution for building, will be removed later */
 #include <linux/pci.h>
@@ -322,6 +324,59 @@  int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
 }
 
 /**
+ * msi_handle_doorbell_mapping: 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)
+{
+
+	for (; data; data = data->parent_data) {
+		const struct irq_chip_msi_doorbell_info *dbinfo;
+		struct iommu_domain *domain;
+		struct msi_desc *desc;
+		struct irq_chip *chip;
+		dma_addr_t iova;
+		int ret = 0, i;
+
+		chip = irq_data_get_irq_chip(data);
+		desc = irq_data_get_msi_desc(data);
+
+		domain = iommu_msi_mapping_desc_to_domain(desc);
+		if (!domain)
+			continue;
+
+		if (!chip->msi_doorbell_info)
+			continue;
+
+		dbinfo = chip->msi_doorbell_info(data);
+		if (!dbinfo)
+			return -EINVAL;
+
+		for (i = 0; i < dbinfo->nb_doorbells; i++) {
+			struct irq_chip_msi_doorbell __percpu *db;
+
+			db = per_cpu_ptr(dbinfo->percpu_doorbells, i);
+			if (map) {
+				ret = iommu_get_reserved_iova(domain,
+							      db->base,
+							      db->size,
+							      db->prot,
+							      &iova);
+				if (ret)
+					return ret;
+			} else
+				iommu_put_reserved_iova(domain, db->base);
+		}
+	}
+	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 +407,25 @@  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++)
+		for (i = 0; i < desc->nvec_used; i++) {
 			irq_set_msi_desc_off(virq, i, desc);
+			ret = msi_handle_doorbell_mappings(
+				irq_get_irq_data(virq + i), true);
+			if (ret)
+				break;
+		}
+
+		if (ret) {
+			for (; i >= 0; i--) {
+				ret = msi_handle_doorbell_mappings(
+					irq_get_irq_data(virq + i), false);
+				irq_set_msi_desc_off(virq, i, NULL);
+			}
+			goto error;
+		}
 	}
 
 	if (ops->msi_finish)
@@ -377,6 +440,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 +466,12 @@  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) {
+			int ret;
+
+			ret = msi_handle_doorbell_mappings(
+				irq_get_irq_data(desc->irq),
+				false);
+			WARN_ON(ret);
 			irq_domain_free_irqs(desc->irq, desc->nvec_used);
 			desc->irq = 0;
 		}