From: "T.J. Mercier" <tjmercier@google.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Christoph Hellwig <hch@infradead.org>,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Catalin Marinas <catalin.marinas@arm.com>,
isaacmanjarres@google.com, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iommu/dma: Respect SWIOTLB force_bounce
Date: Thu, 2 May 2024 10:54:31 -0700 [thread overview]
Message-ID: <CABdmKX1HdXccWp9chz-Y_-Hh5TPry-4WRcVf4fUXKV=Og3dVTg@mail.gmail.com> (raw)
In-Reply-To: <ccb525ff-d0d0-44b1-a210-14c7670b80f0@arm.com>
On Thu, May 2, 2024 at 9:54 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 02/05/2024 5:02 pm, T.J. Mercier wrote:
> > On Thu, May 2, 2024 at 5:50 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >>
> >> On 02/05/2024 6:07 am, Christoph Hellwig wrote:
> >>> On Wed, May 01, 2024 at 08:13:18PM +0000, T.J. Mercier wrote:
> >>>> iommu_dma_map_page and iommu_dma_map_sg conditionally use SWIOTLB, but
> >>>> checking if force_bounce is set for the device is not part of that
> >>>> condition. Check if devices have requested to force SWIOTLB use as part
> >>>> of deciding to take the existing SWIOTLB paths.
> >>>
> >>> This fails to explain why you'd want this somewhat surprising behavior,
> >>> and why you consider it a bug fix.
> >>
> >> Indeed, it's rather intentional that the "swiotlb=force" argument
> >> doesn't affect iommu-dma, since that's primarily for weeding out drivers
> >> making dodgy assumptions about DMA addresses, and iommu-dma is
> >> inherently even better at that already.
> >>
> >> Beyond that I think this change also seems likely to interact badly with
> >> CC_ATTR_GUEST_MEM_ENCRYPT on x86, where we invoke the SWIOTLB_FORCE flag
> >> for dma-direct, but expect that an IOMMU can provide a decrypted view
> >> in-place, thus bouncing in that path would be unnecessarily detrimental.
> >>
> >> Thanks,
> >> Robin.
> >
> > I encountered this while testing a change to DMA direct which makes
> > sure that sg_dma_mark_swiotlb is called there like it is here. (Right
> > now the SG_DMA_SWIOTLB flag is set only if dma_map_sgtable takes the
> > IOMMU path, but not if SWIOTLB is used on the direct path.) While I
> > agree IOMMU + force_bounce is an unusual config, I found it equally
> > surprising that swiotlb=force wasn't doing what is advertised, or even
> > giving a warning/error. Since the iommu-dma code is already set up for
> > conditionally bouncing through SWIOTLB, it looked straightforward to
> > give what's asked for in the case of swiotlb=force. If it's
> > intentional that SWIOTLB options don't affect IOMMU code, then should
> > we just warn about it here when it's ignored? The presence of a
> > warning like that would also be a suggestion of, "you probably don't
> > actually want what you're asking for with this configuration you've
> > specified".
>
> Traditionally, user-facing "SWIOTLB" refers to what is now dma-direct,
> in its context of bouncing to make DMA mappings accessible to devices
> with memory access limitations. The fact that the IOMMU implementations
> (originally Intel, now iommu-dma) also co-opted some of the SWIOTLB
> machinery for the very different purpose of isolation of memory
> *outside* non-page-aligned DMA mappings was always more of an internal
> implementation detail.
>
> The newest use for enforcing non-coherent cacheline alignment blurs the
> boundary a little since its purpose is again largely orthogonal to those
> cases, however it's also one to which "swiotlb=force" is semantically
> kind of meaningless once you think about it (how does one forcibly align
> a buffer which is already suitably aligned?)
>
> If there's any issue here I'd say it's only that the description in
> kernel-parameters.txt still hasn't been updated since "automatically
> used by the kernel" *did* solely imply a device DMA mask limitation.
Ok I think clarifying SWIOTLB as it applies to dma-direct (and not any
of the other uses you've mentioned above) would do it. This?
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 213d0719e2b7..84c582ac246c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6486,6 +6486,7 @@
to a power of 2.
force -- force using of bounce buffers even if they
wouldn't be automatically used by the kernel
+ where a hardware IOMMU is not involved
noforce -- Never use bounce buffers (for debugging)
switches= [HW,M68k,EARLY]
diff --git a/Documentation/arch/x86/x86_64/boot-options.rst
b/Documentation/arch/x86/x86_64/boot-options.rst
index 137432d34109..066b4bc81583 100644
--- a/Documentation/arch/x86/x86_64/boot-options.rst
+++ b/Documentation/arch/x86/x86_64/boot-options.rst
@@ -285,7 +285,7 @@ iommu options only relevant to the AMD GART hardware IOMMU:
Always panic when IOMMU overflows.
iommu options only relevant to the software bounce buffering (SWIOTLB) IOMMU
-implementation:
+implementation where a hardware IOMMU is not involved:
swiotlb=<slots>[,force,noforce]
<slots>
prev parent reply other threads:[~2024-05-02 17:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-01 20:13 [PATCH] iommu/dma: Respect SWIOTLB force_bounce T.J. Mercier
2024-05-02 5:07 ` Christoph Hellwig
2024-05-02 12:50 ` Robin Murphy
2024-05-02 16:02 ` T.J. Mercier
2024-05-02 16:54 ` Robin Murphy
2024-05-02 17:54 ` T.J. Mercier [this message]
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='CABdmKX1HdXccWp9chz-Y_-Hh5TPry-4WRcVf4fUXKV=Og3dVTg@mail.gmail.com' \
--to=tjmercier@google.com \
--cc=akpm@linux-foundation.org \
--cc=catalin.marinas@arm.com \
--cc=hch@infradead.org \
--cc=iommu@lists.linux.dev \
--cc=isaacmanjarres@google.com \
--cc=joro@8bytes.org \
--cc=linux-kernel@vger.kernel.org \
--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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).