Re: "Convert the AMD iommu driver to the dma-iommu api" is buggy

From: Robin Murphy
Date: Wed Oct 16 2019 - 12:09:25 EST

On 16/10/2019 17:03, Joerg Roedel wrote:
On Wed, Oct 16, 2019 at 11:53:33AM -0400, Qian Cai wrote:
On Wed, 2019-10-16 at 17:31 +0200, Joerg Roedel wrote:

The x86 one might just be a mistake.

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index ad05484d0c80..63c4b894751d 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2542,7 +2542,7 @@ static int amd_iommu_map(struct iommu_domain *dom,
unsigned long iova,
ÂÂÂÂÂÂÂÂif (iommu_prot & IOMMU_WRITE)
-ÂÂÂÂÂÂÂret = iommu_map_page(domain, iova, paddr, page_size, prot, GFP_KERNEL);
+ÂÂÂÂÂÂÂret = iommu_map_page(domain, iova, paddr, page_size, prot, gfp);

Yeah, that is a bug, I spotted that too.

@@ -1185,7 +1185,7 @@ static struct iommu_dma_msi_page
*iommu_dma_get_msi_page(struct device *dev,
ÂÂÂÂÂÂÂÂif (!iova)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂgoto out_free_page;
-ÂÂÂÂÂÂÂif (iommu_map(domain, iova, msi_addr, size, prot))
+ÂÂÂÂÂÂÂif (iommu_map_atomic(domain, iova, msi_addr, size, prot))
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂgoto out_free_iova;

Not so sure this is a bug, this code is only about setting up MSIs on
ARM. It probably doesn't need to be atomic.
Right, since the iommu_dma_prepare_msi() operation was broken out to happen at MSI domain setup time, I don't think the comment in there applies any more, so we can probably stop disabling irqs around the iommu_dma_get_msi_page() call.