Received-SPF: pass (google.com: domain of linux-kernel+bounces-170383-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240503183527.1548119-1-tjmercier@google.com>
 <20240504101651.7de5106f@meshulam.tesarici.cz> <CABdmKX06v23-w8PQJab8kgfPDRYLU3bQSQ4AsC3zrzxYL955gQ@mail.gmail.com>
 <SN6PR02MB4157EE83332FF789FA6BEEE8D41C2@SN6PR02MB4157.namprd02.prod.outlook.com>
In-Reply-To: <SN6PR02MB4157EE83332FF789FA6BEEE8D41C2@SN6PR02MB4157.namprd02.prod.outlook.com>
From: "T.J. Mercier" <tjmercier@google.com>
Date: Mon, 6 May 2024 13:16:45 -0700
Message-ID: <CABdmKX2q1rELreKG4oOrKCwKQk9Xswe=gV9254rkHzMNYaD3GQ@mail.gmail.com>
Subject: Re: [PATCH] swiotlb: iommu/dma: Clarify swiotlb options apply only to dma-direct
To: Michael Kelley <mhklinux@outlook.com>
Cc: =?UTF-8?B?UGV0ciBUZXNhxZnDrWs=?= <petr@tesarici.cz>, 
	Jonathan Corbet <corbet@lwn.net>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, 
	Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, 
	"x86@kernel.org" <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>, "hch@infradead.org" <hch@infradead.org>, 
	"robin.murphy@arm.com" <robin.murphy@arm.com>, "joro@8bytes.org" <joro@8bytes.org>, 
	"will@kernel.org" <will@kernel.org>, "akpm@linux-foundation.org" <akpm@linux-foundation.org>, 
	"isaacmanjarres@google.com" <isaacmanjarres@google.com>, "iommu@lists.linux.dev" <iommu@lists.linux.dev>, 
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, May 6, 2024 at 12:58=E2=80=AFPM Michael Kelley <mhklinux@outlook.co=
m> wrote:
>
> From: T.J. Mercier <tjmercier@google.com> Sent: Monday, May 6, 2024 10:23=
 AM
> >
> > On Sat, May 4, 2024 at 1:16=E2=80=AFAM Petr Tesa=C5=99=C3=ADk <petr@tes=
arici.cz> wrote:
> > >
> > > On Fri,  3 May 2024 18:35:26 +0000
> > > "T.J. Mercier" <tjmercier@google.com> wrote:
> > >
> > > > IOMMU implementations now sometimes bounce memory through SWIOTLB t=
o
> > > > achieve cacheline alignment [1], or prevent DMA attacks by untruste=
d
> > > > devices [2]. These uses of SWIOTLB differ conceptually from histori=
cal
> > > > use which was a solution to the problem of device addressing
> > > > limitations that prevent DMA to some portion of system memory
> > > > (typically beyond 4 GiB). IOMMUs also solve the problem of device
> > > > addressing limitations and therefore eliminate the need for SWIOTLB=
 for
> > > > that purpose. However as mentioned, IOMMUs can use SWIOTLB for othe=
r
> > > > purposes.
> > > >
> > > > The swiotlb kernel command line parameter does not impact IOMMU rel=
ated
> > > > use of SWIOTLB, and that is intentional. IOMMUs cannot be forced to=
 use
> > > > SWIOTLB for all buffers. Update the documentation for the swiotlb
> > > > parameter to clarify that SWIOTLB use can only be forced in scenari=
os
> > > > where an IOMMU is not involved.
> > > >
> > > > [1] https://lore.kernel.org/all/20230612153201.554742-16-catalin.ma=
rinas@arm.com/
> > > > [2] https://lore.kernel.org/all/20190906061452.30791-1-baolu.lu@lin=
ux.intel.com/
> > > > Signed-off-by: T.J. Mercier <tjmercier@google.com>
> > > > ---
> > > >  Documentation/admin-guide/kernel-parameters.txt | 1 +
> > > >  Documentation/arch/x86/x86_64/boot-options.rst  | 2 +-
> > > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Docu=
mentation/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 i=
f they
> > > >                                wouldn't be automatically used by th=
e kernel
> > > > +                              where a hardware IOMMU is not involv=
ed
> > > >                       noforce -- Never use bounce buffers (for debu=
gging)
> > > >
> > > >       switches=3D       [HW,M68k,EARLY]
> > >
> > > Yes, this part is correct. SWIOTLB cannot be forced if there is an IO=
MMU.
> > >
> > > > diff --git a/Documentation/arch/x86/x86_64/boot-options.rst b/Docum=
entation/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 har=
dware IOMMU:
> > > >        Always panic when IOMMU overflows.
> > > >
> > > >  iommu options only relevant to the software bounce buffering (SWIO=
TLB) IOMMU
> > > > -implementation:
> > > > +implementation where a hardware IOMMU is not involved:
> > > >
> > > >      swiotlb=3D<slots>[,force,noforce]
> > > >        <slots>
> > >
> > > But this part needs some improvement. The "swiotlb" option is not
> > > entirely ignored if there is a hardware IOMMU. For example, the size =
of
> > > the SWIOTLB can be adjusted using "swiotlb=3D<slots>", and since SWIO=
TLB
> > > can be used by IOMMUs for other purposes (as you correctly note in th=
e
> > > commit message), this setting is relevant even where a hardware IOMMU
> > > is involved.
> > >
> > > Petr T
> >
> > Thanks. I think I should also update the commit message:
> > "The swiotlb=3Dforce kernel command line parameter does not impact IOMM=
U
> > related use of SWIOTLB"
> > and title:
> > "Clarify swiotlb=3Dforce option applies only to dma-direct"
> >
> > As for fixing boot-options.txt, I think it makes the most sense to
> > expand on just the force option rather than the section summary like
> > above:
> >        force
> >          Force all IO through the software TLB.
> > +        Hardware IOMMU implementations can use SWIOTLB bounce bufferin=
g in
> > +        some circumstances, but they cannot be forced to always use th=
em, so
> > +        this option only has an effect when no hardware IOMMU is invol=
ved.
> >        noforce
>
> Note also that the documentation for swiotlb=3D in boot-options.rst is so=
mewhat
> out-of-date.  It doesn't have the optional second integer parameter to sp=
ecify
> the number of "areas" that have their own lock.  Perhaps that could be fi=
xed
> at the same time?
>
> Michael

Thanks, I could add this as a second patch.

I also noticed that several of the iommu options (soft, allowed,
nofullflush, panic) are not listed in the set of possible options at
the top of the IOMMU section. Rebooting is the only other section that
does that, so I'm wondering if we're better off getting rid of it to
be like other sections.