From: David Stevens <stevensd@chromium.org> To: Will Deacon <will@kernel.org> Cc: Robin Murphy <robin.murphy@arm.com>, Joerg Roedel <joro@8bytes.org>, Lu Baolu <baolu.lu@linux.intel.com>, Tom Murphy <murphyt7@tcd.ie>, iommu@lists.linux-foundation.org, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2 3/4] dma-iommu: pass SKIP_CPU_SYNC to swiotlb unmap Date: Thu, 5 Aug 2021 14:26:10 +0900 [thread overview] Message-ID: <CAD=HUj4+62dYZTWfbPjh8eLRY6FQak8nBS8OD85t0xk_+JvDpA@mail.gmail.com> (raw) In-Reply-To: <20210802135446.GE28547@willie-the-truck> On Mon, Aug 2, 2021 at 10:54 PM Will Deacon <will@kernel.org> wrote: > > On Fri, Jul 09, 2021 at 12:35:01PM +0900, David Stevens wrote: > > From: David Stevens <stevensd@chromium.org> > > > > If SKIP_CPU_SYNC isn't already set, then iommu_dma_unmap_(page|sg) has > > already called iommu_dma_sync_(single|sg)_for_cpu, so there is no need > > to copy from the bounce buffer again. > > > > Signed-off-by: David Stevens <stevensd@chromium.org> > > --- > > drivers/iommu/dma-iommu.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > index e79e274d2dc5..0a9a9a343e64 100644 > > --- a/drivers/iommu/dma-iommu.c > > +++ b/drivers/iommu/dma-iommu.c > > @@ -505,7 +505,8 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev, dma_addr_t dma_addr, > > __iommu_dma_unmap(dev, dma_addr, size); > > > > if (unlikely(is_swiotlb_buffer(phys))) > > - swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs); > > + swiotlb_tbl_unmap_single(dev, phys, size, dir, > > + attrs | DMA_ATTR_SKIP_CPU_SYNC); > > } > > I think it would be cleaner to drop DMA_ATTR_SKIP_CPU_SYNC in the callers > once they've called iommu_dma_sync_*_for_cpu(). Dropping that flag in iommu_dma_unmap_* would result in always copying from the swiotlb here, which is the opposite direction of what this patch is trying to do. This change is aiming to address the case where DMA_ATTR_SKIP_CPU_SYNC isn't passed to dma_unmap_*. In that case, there are calls to swiotlb_sync_single_for_cpu from iommu_dma_sync_*_for_cpu, and calls to swiotlb_tlb_unmap_single here. That means we copy from the swiotlb twice. Adding the DMA_ATTR_SKIP_CPU_SYNC flag here skips the second copy. -David > Will
WARNING: multiple messages have this Message-ID (diff)
From: David Stevens <stevensd@chromium.org> To: Will Deacon <will@kernel.org> Cc: open list <linux-kernel@vger.kernel.org>, Tom Murphy <murphyt7@tcd.ie>, iommu@lists.linux-foundation.org, Robin Murphy <robin.murphy@arm.com> Subject: Re: [PATCH v2 3/4] dma-iommu: pass SKIP_CPU_SYNC to swiotlb unmap Date: Thu, 5 Aug 2021 14:26:10 +0900 [thread overview] Message-ID: <CAD=HUj4+62dYZTWfbPjh8eLRY6FQak8nBS8OD85t0xk_+JvDpA@mail.gmail.com> (raw) In-Reply-To: <20210802135446.GE28547@willie-the-truck> On Mon, Aug 2, 2021 at 10:54 PM Will Deacon <will@kernel.org> wrote: > > On Fri, Jul 09, 2021 at 12:35:01PM +0900, David Stevens wrote: > > From: David Stevens <stevensd@chromium.org> > > > > If SKIP_CPU_SYNC isn't already set, then iommu_dma_unmap_(page|sg) has > > already called iommu_dma_sync_(single|sg)_for_cpu, so there is no need > > to copy from the bounce buffer again. > > > > Signed-off-by: David Stevens <stevensd@chromium.org> > > --- > > drivers/iommu/dma-iommu.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > index e79e274d2dc5..0a9a9a343e64 100644 > > --- a/drivers/iommu/dma-iommu.c > > +++ b/drivers/iommu/dma-iommu.c > > @@ -505,7 +505,8 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev, dma_addr_t dma_addr, > > __iommu_dma_unmap(dev, dma_addr, size); > > > > if (unlikely(is_swiotlb_buffer(phys))) > > - swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs); > > + swiotlb_tbl_unmap_single(dev, phys, size, dir, > > + attrs | DMA_ATTR_SKIP_CPU_SYNC); > > } > > I think it would be cleaner to drop DMA_ATTR_SKIP_CPU_SYNC in the callers > once they've called iommu_dma_sync_*_for_cpu(). Dropping that flag in iommu_dma_unmap_* would result in always copying from the swiotlb here, which is the opposite direction of what this patch is trying to do. This change is aiming to address the case where DMA_ATTR_SKIP_CPU_SYNC isn't passed to dma_unmap_*. In that case, there are calls to swiotlb_sync_single_for_cpu from iommu_dma_sync_*_for_cpu, and calls to swiotlb_tlb_unmap_single here. That means we copy from the swiotlb twice. Adding the DMA_ATTR_SKIP_CPU_SYNC flag here skips the second copy. -David > Will _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2021-08-05 5:29 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-09 3:34 [PATCH v2 0/4] Fixes for dma-iommu swiotlb bounce buffers David Stevens 2021-07-09 3:34 ` David Stevens 2021-07-09 3:34 ` [PATCH v2 1/4] dma-iommu: fix sync_sg with swiotlb David Stevens 2021-07-09 3:34 ` David Stevens 2021-08-02 13:30 ` Will Deacon 2021-08-02 13:30 ` Will Deacon 2021-08-05 3:40 ` David Stevens 2021-08-05 3:40 ` David Stevens 2021-07-09 3:35 ` [PATCH v2 2/4] dma-iommu: fix arch_sync_dma for map " David Stevens 2021-07-09 3:35 ` David Stevens 2021-08-02 13:40 ` Will Deacon 2021-08-02 13:40 ` Will Deacon 2021-08-02 13:50 ` Will Deacon 2021-08-02 13:50 ` Will Deacon 2021-07-09 3:35 ` [PATCH v2 3/4] dma-iommu: pass SKIP_CPU_SYNC to swiotlb unmap David Stevens 2021-07-09 3:35 ` David Stevens 2021-08-02 13:54 ` Will Deacon 2021-08-02 13:54 ` Will Deacon 2021-08-05 5:26 ` David Stevens [this message] 2021-08-05 5:26 ` David Stevens 2021-08-06 11:38 ` Will Deacon 2021-08-06 11:38 ` Will Deacon 2021-07-09 3:35 ` [PATCH v2 4/4] dma-iommu: Check CONFIG_SWIOTLB more broadly David Stevens 2021-07-09 3:35 ` David Stevens
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAD=HUj4+62dYZTWfbPjh8eLRY6FQak8nBS8OD85t0xk_+JvDpA@mail.gmail.com' \ --to=stevensd@chromium.org \ --cc=baolu.lu@linux.intel.com \ --cc=iommu@lists.linux-foundation.org \ --cc=joro@8bytes.org \ --cc=linux-kernel@vger.kernel.org \ --cc=murphyt7@tcd.ie \ --cc=robin.murphy@arm.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.