Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support

From: "Michael S. Tsirkin" <mst@redhat.com>
To: Gleb Natapov <gleb@scylladb.com>
Cc: Avi Kivity <avi@scylladb.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Vlad Zolotarov <vladz@cloudius-systems.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, hjk@hansjkoch.de, corbet@lwn.net,
	bruce.richardson@intel.com, avi@cloudius-systems.com,
	gleb@cloudius-systems.com, stephen@networkplumber.org,
	alexander.duyck@gmail.com
Subject: Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support
Date: Thu, 8 Oct 2015 16:20:04 +0300	[thread overview]
Message-ID: <20151008153858-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <20151008122737.GI11716@scylladb.com>

On Thu, Oct 08, 2015 at 03:27:37PM +0300, Gleb Natapov wrote:
> On Thu, Oct 08, 2015 at 03:06:07PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Oct 08, 2015 at 12:44:09PM +0300, Avi Kivity wrote:
> > > 
> > > 
> > > On 10/08/2015 12:16 PM, Michael S. Tsirkin wrote:
> > > >On Thu, Oct 08, 2015 at 11:46:30AM +0300, Avi Kivity wrote:
> > > >>
> > > >>On 10/08/2015 10:32 AM, Michael S. Tsirkin wrote:
> > > >>>On Thu, Oct 08, 2015 at 08:33:45AM +0300, Avi Kivity wrote:
> > > >>>>It is good practice to defend against root oopsing the kernel, but in some
> > > >>>>cases it cannot be achieved.
> > > >>>Absolutely. That's one of the issues with these patches. They don't even
> > > >>>try where it's absolutely possible.
> > > >>>
> > > >>Are you referring to blocking the maps of the msix BAR areas?
> > > >For example. There are more. I listed some of the issues on the mailing
> > > >list, and I might have missed some.  VFIO has code to address all this,
> > > >people should share code to avoid duplication, or at least read it
> > > >to understand the issues.
> > > 
> > > All but one of those are unrelated to the patch that adds msix support.
> > 
> > They are related because msix support enables bus mastering.  Without it
> > device is passive and can't harm anyone. With it, suddently you need to
> > be very careful with the device to avoid corrupting kernel memory.
> > 
> Most (if not all) uio_pci_generic users enable pci bus mastering. The
> fact that they do that without even tainting the kernel like the patch
> does make current situation much worse that with the patch.

It isn't worse. It's a sane interface. Whoever enables bus mastering
must be careful.  If userspace enables bus mastering then userspace
needs to be very careful with the device to avoid corrupting kernel
memory.  If kernel does it, it's kernel's responsibility.

> > > I can't comment on iommu overhead; for my use case it is likely negligible
> > > and we will use an iommu when available; but apparently it matters for
> > > others.
> > 
> > You and Vlad are the only ones who brought this up.
> > So maybe you should not bring it up anymore.
> > 
> Common, you were CCed to at least this one:
> 
>  We have a solution that makes use of IOMMU support with vfio.  The 
>  problem is there are multiple cases where that support is either not 
>  available, or using the IOMMU provides excess overhead.
> 
> 
> http://dpdk.org/ml/archives/dev/2015-October/024560.html

Thanks for the correction.  I didn't notice that one, and I
misunderstood Avi's comment to mean it's just a theoretical case (taking
"apparently" to mean "maybe").  So someone else did bring it up, it's
not just Avi and Vlad.  I'm sorry, I take my comment back.  It might
help to mention "iommu overhead on pre ivy-bridge x86 systems" - that is
what this email seems to refer to.

> --
> 			Gleb.