Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1240490 > unrolled thread

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

Started by"Michael S. Tsirkin" <mst@redhat.com>
First post2015-10-06 16:40 +0200
Last post2015-10-08 10:50 +0200
Articles 5 on this page of 45 — 7 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-06 16:40 +0200
    Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-06 16:50 +0200
      Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-06 17:00 +0200
        Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-06 17:30 +0200
        Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-06 17:30 +0200
          Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Alex Williamson <alex.williamson@redhat.com> - 2015-10-06 21:00 +0200
            Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Stephen Hemminger <stephen@networkplumber.org> - 2015-10-06 23:40 +0200
              Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Alex Williamson <alex.williamson@redhat.com> - 2015-10-06 23:50 +0200
                Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-07 10:00 +0200
                Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-07 10:10 +0200
                  Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-07 10:10 +0200
            Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-07 09:00 +0200
              Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-07 18:40 +0200
                Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-07 23:10 +0200
                  Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 06:20 +0200
                    Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 09:50 +0200
                      Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 10:00 +0200
                        Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 11:40 +0200
                          Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 11:50 +0200
                            Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 14:20 +0200
                  Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-08 07:40 +0200
                    Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 09:40 +0200
                      Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-08 10:50 +0200
                        Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 11:20 +0200
                          Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-08 11:50 +0200
                            Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 14:10 +0200
                              Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 14:30 +0200
                                Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 15:30 +0200
                                  Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 15:30 +0200
                                    Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 18:50 +0200
                                      Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 19:10 +0200
                                        Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 19:40 +0200
                                          Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 20:00 +0200
                                          Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Greg KH <gregkh@linuxfoundation.org> - 2015-10-08 20:40 +0200
                    Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 10:40 +0200
                      Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Gleb Natapov <gleb@scylladb.com> - 2015-10-08 11:00 +0200
                      Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-08 11:20 +0200
                        Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 12:30 +0200
                          Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Avi Kivity <avi@scylladb.com> - 2015-10-08 15:30 +0200
                            Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 16:20 +0200
                            Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Alex Williamson <alex.williamson@redhat.com> - 2015-10-08 17:40 +0200
              Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Alex Williamson <alex.williamson@redhat.com> - 2015-10-07 18:40 +0200
                Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-07 22:10 +0200
            Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov <vladz@cloudius-systems.com> - 2015-10-07 10:00 +0200
              Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-08 10:50 +0200

Page 3 of 3 — ← Prev page 1 2 [3]


#1242534

FromAlex Williamson <alex.williamson@redhat.com>
Date2015-10-08 17:40 +0200
Message-ID<qhlsu-7EU-5@gated-at.bofh.it>
In reply to#1242343
On Thu, 2015-10-08 at 16:20 +0300, Avi Kivity wrote:
> On 10/08/2015 01:26 PM, Michael S. Tsirkin wrote:
> > On Thu, Oct 08, 2015 at 12:19:20PM +0300, Avi Kivity wrote:
> >> We are in the strange situation that the Alex is open to adding an insecure
> >> mode to vfio,
> > I don't find this strange. It seems to make sense. VFIO is
> > already used with DMA capable devices.
> 
> It's strange to me because it's charter was for iommu-protected device 
> assignment, while uio_pci_generic is for generic pci userspace.

To be clear, I'm not necessarily advocating an insecure mode of vfio,
I'm pointing out that vfio is built on the security, isolation, and
services advertised by the iommu layer.  That layer doesn't exist in a
no-iommu system, but a stub iommu driver that disregards the intended
purpose of iommu groups and implements those services could likely fool
vfio into working.  From a code re-use standpoint, there are some clear
advantages to doing that even though it's rather dastardly at the iommu
level.  There's not too much I can do to prevent such a thing, vfio has
to trust someone and in this case it's the core kernel iommu services.
So if such a task was attempted, I'd want to be involved and enlighten
vfio at least to the point where we can make it clear to users which
uses are secure and which are not.  Thanks,

Alex

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

[toc] | [prev] | [next] | [standalone]


#1241678

FromAlex Williamson <alex.williamson@redhat.com>
Date2015-10-07 18:40 +0200
Message-ID<qgZV0-1TQ-27@gated-at.bofh.it>
In reply to#1241174
On Wed, 2015-10-07 at 09:52 +0300, Avi Kivity wrote:
> 
> On 10/06/2015 09:51 PM, Alex Williamson wrote:
> > On Tue, 2015-10-06 at 18:23 +0300, Avi Kivity wrote:
> >> On 10/06/2015 05:56 PM, Michael S. Tsirkin wrote:
> >>> On Tue, Oct 06, 2015 at 05:43:50PM +0300, Vlad Zolotarov wrote:
> >>>> The only "like VFIO" behavior we implement here is binding the MSI-X
> >>>> interrupt notification to eventfd descriptor.
> >>> There will be more if you add some basic memory protections.
> >>>
> >>> Besides, that's not true.
> >>> Your patch queries MSI capability, sets # of vectors.
> >>> You even hinted you want to add BAR mapping down the road.
> >> BAR mapping is already available from sysfs; it is not mandatory.
> >>
> >>> VFIO does all of that.
> >>>
> >> Copying vfio maintainer Alex (hi!).
> >>
> >> vfio's charter is modern iommu-capable configurations. It is designed to
> >> be secure enough to be usable by an unprivileged user.
> >>
> >> For performance and hardware reasons, many dpdk deployments use
> >> uio_pci_generic.  They are willing to trade off the security provided by
> >> vfio for the performance and deployment flexibility of pci_uio_generic.
> >> Forcing these features into vfio will compromise its security and
> >> needlessly complicate its code (I guess it can be done with a "null"
> >> iommu, but then vfio will have to decide whether it is secure or not).
> > It's not just the iommu model vfio uses, it's that vfio is built around
> > iommu groups.  For instance to use a device in vfio, the user opens the
> > vfio group file and asks for the device within that group.  That's a
> > fairly fundamental part of the mechanics to sidestep.
> >
> > However, is there an opportunity at a lower level?  Systems without an
> > iommu typically have dma ops handled via a software iotlb (ie. bounce
> > buffers), but I think they simply don't have iommu ops registered.
> > Could a no-iommu, iommu subsystem provide enough dummy iommu ops to fake
> > out vfio?  It would need to iterate the devices on the bus and come up
> > with dummy iommu groups and dummy versions of iommu_map and unmap.  The
> > grouping is easy, one device per group, there's no isolation anyway.
> > The vfio type1 iommu backend will do pinning, which seems like an
> > improvement over the mlock that uio users probably try to do now.
> 
> Right now, people use hugetlbfs maps, which both locks the memory and 
> provides better performance.
> 
> >    I
> > guess the no-iommu map would error if the IOVA isn't simply the bus
> > address of the page mapped.
> >
> > Of course this is entirely unsafe and this no-iommu driver should taint
> > the kernel, but it at least standardizes on one userspace API and you're
> > already doing completely unsafe things with uio.  vfio should be
> > enlightened at least to the point that it allows only privileged users
> > access to devices under such a (lack of) iommu.
> 
> There is an additional complication.  With an iommu, userspace programs 
> the device with virtual addresses, but without it, they have to program 
> physical addresses.  So vfio would need to communicate this bit of 
> information.
> 
> We can go further and define a better translation API than the current 
> one (reading /proc/pagemap).  But it's going to be a bigger change to 
> vfio than I thought at first.

It sounds like a separate vfio iommu backend from type1, one that just
pins the page and returns the bus address.  The curse and benefit would
be that existing type1 users wouldn't "just work" in an insecure mode,
the DMA mapping code would need to be aware of the difference.  Still, I
do really prefer to keep vfio as only exposing a secure, iommu protected
device to the user because surely someone will try and users would
expect that removing iommu restrictions from vfio means they can do
device assignment to VMs w/o an iommu.

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

[toc] | [prev] | [next] | [standalone]


#1241775

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-10-07 22:10 +0200
Message-ID<qh3ce-6Lp-1@gated-at.bofh.it>
In reply to#1241678
On Wed, Oct 07, 2015 at 10:31:04AM -0600, Alex Williamson wrote:
> It sounds like a separate vfio iommu backend from type1, one that just
> pins the page and returns the bus address.  The curse and benefit would
> be that existing type1 users wouldn't "just work" in an insecure mode,
> the DMA mapping code would need to be aware of the difference.  Still, I
> do really prefer to keep vfio as only exposing a secure, iommu protected
> device to the user because surely someone will try and users would
> expect that removing iommu restrictions from vfio means they can do
> device assignment to VMs w/o an iommu.

What I had in mind is rather reusing vfio code.

What is needed is all the logic for handling device reset, protecting
BARs and config space regions, MSI/MSI-X and device-specific
work-arounds.

Also, all the interface things such as eventfd.

But I don't think it should be the same char device as vfio -
/dev/vfio/vfio is world-accessible - should be a separate non-world
accessible device, maybe /dev/vfio/noiommu.

This will ensure e.g. qemu does not attempts to use it automatically.
-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1241212

FromVlad Zolotarov <vladz@cloudius-systems.com>
Date2015-10-07 10:00 +0200
Message-ID<qgRNM-6WR-21@gated-at.bofh.it>
In reply to#1240858

On 10/06/15 21:51, Alex Williamson wrote:
> On Tue, 2015-10-06 at 18:23 +0300, Avi Kivity wrote:
>> On 10/06/2015 05:56 PM, Michael S. Tsirkin wrote:
>>> On Tue, Oct 06, 2015 at 05:43:50PM +0300, Vlad Zolotarov wrote:
>>>> The only "like VFIO" behavior we implement here is binding the MSI-X
>>>> interrupt notification to eventfd descriptor.
>>> There will be more if you add some basic memory protections.
>>>
>>> Besides, that's not true.
>>> Your patch queries MSI capability, sets # of vectors.
>>> You even hinted you want to add BAR mapping down the road.
>> BAR mapping is already available from sysfs; it is not mandatory.
>>
>>> VFIO does all of that.
>>>
>> Copying vfio maintainer Alex (hi!).
>>
>> vfio's charter is modern iommu-capable configurations. It is designed to
>> be secure enough to be usable by an unprivileged user.
>>
>> For performance and hardware reasons, many dpdk deployments use
>> uio_pci_generic.  They are willing to trade off the security provided by
>> vfio for the performance and deployment flexibility of pci_uio_generic.
>> Forcing these features into vfio will compromise its security and
>> needlessly complicate its code (I guess it can be done with a "null"
>> iommu, but then vfio will have to decide whether it is secure or not).
> It's not just the iommu model vfio uses, it's that vfio is built around
> iommu groups.  For instance to use a device in vfio, the user opens the
> vfio group file and asks for the device within that group.  That's a
> fairly fundamental part of the mechanics to sidestep.
>
> However, is there an opportunity at a lower level?  Systems without an
> iommu typically have dma ops handled via a software iotlb (ie. bounce
> buffers), but I think they simply don't have iommu ops registered.
> Could a no-iommu, iommu subsystem provide enough dummy iommu ops to fake
> out vfio?  It would need to iterate the devices on the bus and come up
> with dummy iommu groups and dummy versions of iommu_map and unmap.  The
> grouping is easy, one device per group, there's no isolation anyway.
> The vfio type1 iommu backend will do pinning, which seems like an
> improvement over the mlock that uio users probably try to do now.  I
> guess the no-iommu map would error if the IOVA isn't simply the bus
> address of the page mapped.
>
> Of course this is entirely unsafe and this no-iommu driver should taint
> the kernel, but it at least standardizes on one userspace API and you're
> already doing completely unsafe things with uio.  vfio should be
> enlightened at least to the point that it allows only privileged users
> access to devices under such a (lack of) iommu.

Thanks for clarification, Alex.
One of the important points in the above description is that vfio has 
been build around IOMMU groups - and that's a good thing!
This means that this ensures the safety for vfio users and IMHO breaking 
this by introducing the no-iommu mode won't bring any good.

What do we have on a negative side of this step:

 1. Just a description of the work that has to be done implies a
    non-trivial code that will have to be maintained later.
 2. This new mode will be absolutely unsafe while some users may
    mistakenly assume that using vfio is safe in all situations like it
    is now.
 3. The vfio user interface is "a bit" more complicated than the one of
    UIO's and if there isn't any added value (see below) users will just
    prefer continue using UIO.

Let's try to analyze the possible positive sides (as u've described them 
above):

 1. /This added feature may allow to standardize the user-space drivers
    interface. Why is it good? - This could allow us to maintain only
    one infrastructure (vfio)./ That's true but unfortunately UIO
    interface is already widely used and thus it can't be just killed.
    Therefore instead of maintaining one unsafe user-space driver
    infrastructure we'll have to maintain two. Therefore the result will
    be exactly the opposite from the expected.
 2. I'm not very familiar with all vfio features but regarding the "vfio
    type1 iommu backend going to do pinning instead of mlock" - another
    alternative to pin the pages in the memory is to use hugetlbfs,
    which UIO users like DPDK do. I may be wrong but I'm not sure that
    in this case using vfio type1 iommu backend would be beneficial it
    terms of performance and performance is usually the most important
    factor for un-safe mode users (e.g. DPDK).


So, considering the above I think that instead of complicating the 
already non-trivial vfio interface even more we'd rather have two types 
of user-space interfaces:

  * safe - VFIO
  * not safe - UIO

The thing is that this is more or less the situation right now and 
according to negatives.3 above it is likely to remain this way (at least 
the UIO part ;)) for some (long) time so all this "adding unsafe mode to 
VFIO" initiative looks completely useless.

thanks,
vlad


>
>>>> This doesn't justifies the
>>>> hassle of implementing IOMMU-less VFIO mode.
>>> This applies to both VFIO and UIO really.  I'm not sure the hassle of
>>> maintaining this functionality in tree is justified.  It remains to be
>>> seen whether there are any users that won't taint the kernel.
>>> Apparently not in the current form of the patch, but who knows.
>> It is not msix that taints the kernel, it's uio_pci_generic.  Msix is a
>> tiny feature addition that doesn't change the security situation one bit.
>>
>> btw, currently you can map BARs and dd to /dev/mem to your heart's
>> content without tainting the kernel.  I don't see how you can claim that
>> msix support makes the situation worse, when root can access every bit
>> of physical memory, either directly or via DMA.
>
>

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

[toc] | [prev] | [next] | [standalone]


#1242083

From"Michael S. Tsirkin" <mst@redhat.com>
Date2015-10-08 10:50 +0200
Message-ID<qhf3J-6MG-25@gated-at.bofh.it>
In reply to#1241212
On Wed, Oct 07, 2015 at 10:55:30AM +0300, Vlad Zolotarov wrote:
>  * not safe - UIO

That's wrong. UIO (in particular uio_pci_generic) can be used
safely in many ways, for example with any device not doing DMA.  I
wouldn't put it upstream otherwise.

Make your driver work in such a way that it can be used safely,
and it can be merged.

But when you try to do this, you will find out just why VFIO/PCI is
1000s of LOC while your patch is only 500.

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

[toc] | [prev] | [standalone]


Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel


csiph-web