iommu/dma: Relax locking in iommu_dma_prepare_msi()
authorRobin Murphy <robin.murphy@arm.com>
Mon, 9 Dec 2019 19:47:25 +0000 (19:47 +0000)
committerJoerg Roedel <jroedel@suse.de>
Wed, 18 Dec 2019 16:41:36 +0000 (17:41 +0100)
Since commit ece6e6f0218b ("iommu/dma-iommu: Split iommu_dma_map_msi_msg()
in two parts"), iommu_dma_prepare_msi() should no longer have to worry
about preempting itself, nor being called in atomic context at all. Thus
we can downgrade the IRQ-safe locking to a simple mutex to avoid angering
the new might_sleep() check in iommu_map().

Reported-by: Qian Cai <cai@lca.pw>
Tested-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
drivers/iommu/dma-iommu.c

index 6e573d1cb8bf0df91d2f5799f76cd995f16cec2d..c363294b3bb93ff9ce1165c06cab466dcfa4e68a 100644 (file)
@@ -19,6 +19,7 @@
 #include <linux/iova.h>
 #include <linux/irq.h>
 #include <linux/mm.h>
+#include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/scatterlist.h>
 #include <linux/vmalloc.h>
@@ -44,7 +45,6 @@ struct iommu_dma_cookie {
                dma_addr_t              msi_iova;
        };
        struct list_head                msi_page_list;
-       spinlock_t                      msi_lock;
 
        /* Domain for flush queue callback; NULL if flush queue not in use */
        struct iommu_domain             *fq_domain;
@@ -63,7 +63,6 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
 
        cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
        if (cookie) {
-               spin_lock_init(&cookie->msi_lock);
                INIT_LIST_HEAD(&cookie->msi_page_list);
                cookie->type = type;
        }
@@ -1176,7 +1175,7 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
                if (msi_page->phys == msi_addr)
                        return msi_page;
 
-       msi_page = kzalloc(sizeof(*msi_page), GFP_ATOMIC);
+       msi_page = kzalloc(sizeof(*msi_page), GFP_KERNEL);
        if (!msi_page)
                return NULL;
 
@@ -1206,7 +1205,7 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)
        struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
        struct iommu_dma_cookie *cookie;
        struct iommu_dma_msi_page *msi_page;
-       unsigned long flags;
+       static DEFINE_MUTEX(msi_prepare_lock); /* see below */
 
        if (!domain || !domain->iova_cookie) {
                desc->iommu_cookie = NULL;
@@ -1216,13 +1215,13 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)
        cookie = domain->iova_cookie;
 
        /*
-        * We disable IRQs to rule out a possible inversion against
-        * irq_desc_lock if, say, someone tries to retarget the affinity
-        * of an MSI from within an IPI handler.
+        * In fact the whole prepare operation should already be serialised by
+        * irq_domain_mutex further up the callchain, but that's pretty subtle
+        * on its own, so consider this locking as failsafe documentation...
         */
-       spin_lock_irqsave(&cookie->msi_lock, flags);
+       mutex_lock(&msi_prepare_lock);
        msi_page = iommu_dma_get_msi_page(dev, msi_addr, domain);
-       spin_unlock_irqrestore(&cookie->msi_lock, flags);
+       mutex_unlock(&msi_prepare_lock);
 
        msi_desc_set_iommu_cookie(desc, msi_page);