linux-kernel - Re: [PATCH 02/10] swiotlb: Make 'setup_io_tlb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20100226085206P.fujita.tomonori@lab.ntt.co.jp>
Date:	Fri, 26 Feb 2010 08:52:18 +0900
From:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To:	konrad.wilk@...cle.com
Cc:	fujita.tomonori@....ntt.co.jp, chrisw@...s-sol.org,
	jeremy@...p.org, Ian.Campbell@...citrix.com,
	linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
	dwmw2@...radead.org, alex.williamson@...com
Subject: Re: [PATCH 02/10] swiotlb: Make 'setup_io_tlb_npages' accept new
	'swiotlb=' syntax.

On Thu, 25 Feb 2010 07:49:17 -0500
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com> wrote:

> On Thu, Feb 25, 2010 at 03:53:20PM +0900, FUJITA Tomonori wrote:
> > On Thu, 18 Feb 2010 11:26:58 -0500
> > Konrad Rzeszutek Wilk <konrad.wilk@...cle.com> wrote:
> > 
> > > The old syntax for 'swiotlb' is still in effect, and we extend it
> > > now to include the overflow buffer size. The syntax is now:
> > > 
> > > swiotlb=[force,][nslabs=<pages>,][overflow=<size>] or more
> > > commonly know as:
> > > 
> > > swiotlb=[force]
> > > swiotlb=[nslabs=<pages>]
> > > swiotlb=[overflow=<size>]
> > > 
> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> > > ---
> > >  Documentation/x86/x86_64/boot-options.txt |    6 ++++-
> > >  lib/swiotlb.c                             |   36 +++++++++++++++++++++++++---
> > >  2 files changed, 37 insertions(+), 5 deletions(-)
> > 
> > 'overflow' is a workaround for broken drivers (that ignores DMA
> > mapping errors). We have fixed such drivers and other hardware IOMMU
> > implementations don't have such workaround. So please don't extend
> > it. We should remove it instead.
> 
> Will do. Do you remember which drivers (or e-mail threads) exhibited this
> behavior so that I can do a regression test to make absolutely certain
> that removing the overflow functionality won't blow something up?

You can easily find such drivers by doing grep
{dma|pci}_map_{single|Sig} and seeing if {dma|pci}_mapping_error is
properly used or not.

There are some yet but the majority of such drivers are unlikely to be
used with IOMMU (e.g. really old drivers).

'overflow' works in only some conditions; that is, broken drivers
could lead to data corruption even with 'overflow'
functionality.

I've not reviewed that patchset carefully but my other comments are:

- exporting too generic naming such as 'do_map_single' is not a good
idea.

- please don't mix up new feature patches with 'fixing the printk
  format, checkpatch warnings' (which I'm not interested in).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/