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] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120716160119.GE13540@phenom.dumpdata.com>
Date:	Mon, 16 Jul 2012 12:01:19 -0400
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	Shuah Khan <shuah.khan@...com>
Cc:	fujita.tomonori@....ntt.co.jp, LKML <linux-kernel@...r.kernel.org>,
	akpm@...ux-foundation.org, paul.gortmaker@...driver.com,
	bhelgaas@...gle.com, amwang@...hat.com, shuahkhan@...il.com
Subject: Re: [PATCH RFC] swiotlb: Disable swiotlb overflow support when
 CONFIG_ISA is enabled

On Mon, Jul 16, 2012 at 09:48:20AM -0600, Shuah Khan wrote:
> On Mon, 2012-07-16 at 10:45 -0400, Konrad Rzeszutek Wilk wrote:
> > On Thu, Jul 12, 2012 at 10:17:50AM -0600, Shuah Khan wrote:
> > > Disable iotlb overflow support when CONFIG_ISA is enabled. This is the
> > > first step towards removing overflow support, to be consistent with other
> > > iommu implementations and return DMA_ERROR_CODE. This disabling step is
> > > for finding drivers that don't call dma_mapping_error to check for errors
> > > returned by the mapping interface. Once drivers are fixed overflow support
> > > can be removed.
> > 
> > I think I explained myself poorly. We don't want to break old drivers.
> > 
> > If the old drivers expect this behavior (while they should be updated
> > to check the DMA), we need to retain this behavior.
> > 
> > So I think the patch should be done the other way:
> > 
> > Disable IOTLB overflow when CONFIG_ISA is disabled.
> 
> That makes lot of sense :) I will redo the patch. It might be good to a
> tunable to control enable/disable behavior. Even though support for this
> new tunable "swiotlb_overflow" projected to be short lived, it will be
> lot less painful to disable/enable as needed. Thought and comments?

Tunnable have a habit of becoming permanant and we really don't want that.
The right solution is to drop the overflow - as you have rightly
pointed out in the first patch. But the road is blocked by these old
drivers that are making it hard.

I would say if you really really want a patch in for 3.6 before the
final mega-patch of drivers-that-suck-and-need-to-be-updated-here-is
-the-big-patch-that-does-it, then just add a WARN mentioning your
name and mine (and LKML) saying to report what you see. That way
we can at least get feedback from the field.

And for good measure include in the Documentation/*deprecate-schedule
that the overflow functionality is going away when all the drivers that
depend on it have been fixed up.

> 
> > 
> > But we also need to check whether there are other drivers
> > that don't properly check the DMA address. And if so, add them
> > to this list of must have enabled b/c you might be using this
> > driver.
> 
> Sound good. Again having a tunable would help in this case as well.
> > 
> > The first goal is to figure out which of the drivers aren't doing this
> > properly. This should be possible by just grepping for 'dma_map' and 
> > seeing which ones don't do the 'dma_check' right after.
> 
> I started some research into this and will continue and provide an
> update.

Thank you! It is pretty griddy work so I really appreciate you taking
the time to do this. If are at the LinuxCon this year the beers (or coffee
if that is your preference) are on me for tackling on this cleanup.
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ