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

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 1 of 3  [1] 2 3  Next page →

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

On Mon, Oct 05, 2015 at 01:20:11PM +0300, Avi Kivity wrote:
> On 10/05/2015 12:49 PM, Greg KH wrote:
> >On Mon, Oct 05, 2015 at 11:28:03AM +0300, Avi Kivity wrote:
> >>Of course it has to be documented, but this just follows vfio.
> >>
> >>Eventfd is a natural enough representation of an interrupt; both kvm and
> >>vfio use it, and are also able to share the eventfd, allowing a vfio
> >>interrupt to generate a kvm interrupt, without userspace intervention, and
> >>one day without even kernel intervention.
> >That's nice and wonderful, but it's not how UIO works today, so this is
> >now going to be a mix and match type interface, with no justification so
> >far as to why to create this new api and exactly how this is all going
> >to be used from userspace.
> 
> The intended user is dpdk (http://dpdk.org), which is a family of userspace
> networking drivers for high performance networking applications.
> 
> The natural device driver for dpdk is vfio, which both provides memory
> protection and exposes msi/msix interrupts.  However, in many cases vfio
> cannot be used, either due to the lack of an iommu (for example, in
> virtualized environments) or out of a desire to avoid the iommus performance
> impact.
> 
> The challenge in exposing msix interrupts to user space is that there are
> many of them, so you can't simply poll the device fd.  If you do, how do you
> know which interrupt was triggered?  The solution that vfio adopted was to
> associate each interrupt with an eventfd, allowing it to be individually
> polled.  Since you can pass an eventfd with SCM_RIGHTS, and since kvm can
> trigger guest interrupts using an eventfd, the solution is very flexible.
> 
> >Example code would be even better...
> >
> >
> 
> 
> This is the vfio dpdk interface code:
> 
> http://dpdk.org/browse/dpdk/tree/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> 
> basically, the equivalent uio msix code would be very similar if uio adopts
> a similar interface:
> 
> http://dpdk.org/browse/dpdk/tree/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> 
> (current code lacks msi/msix support, of course).

So you really want a driver that behaves exactly like vfio.
Which immediately begs a question: why not extend vfio
to cover your usecase.

-- 
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] | [next] | [standalone]

#1240506

On 10/06/15 17:38, Michael S. Tsirkin wrote:
> On Mon, Oct 05, 2015 at 01:20:11PM +0300, Avi Kivity wrote:
>> On 10/05/2015 12:49 PM, Greg KH wrote:
>>> On Mon, Oct 05, 2015 at 11:28:03AM +0300, Avi Kivity wrote:
>>>> Of course it has to be documented, but this just follows vfio.
>>>>
>>>> Eventfd is a natural enough representation of an interrupt; both kvm and
>>>> vfio use it, and are also able to share the eventfd, allowing a vfio
>>>> interrupt to generate a kvm interrupt, without userspace intervention, and
>>>> one day without even kernel intervention.
>>> That's nice and wonderful, but it's not how UIO works today, so this is
>>> now going to be a mix and match type interface, with no justification so
>>> far as to why to create this new api and exactly how this is all going
>>> to be used from userspace.
>> The intended user is dpdk (http://dpdk.org), which is a family of userspace
>> networking drivers for high performance networking applications.
>>
>> The natural device driver for dpdk is vfio, which both provides memory
>> protection and exposes msi/msix interrupts.  However, in many cases vfio
>> cannot be used, either due to the lack of an iommu (for example, in
>> virtualized environments) or out of a desire to avoid the iommus performance
>> impact.
>>
>> The challenge in exposing msix interrupts to user space is that there are
>> many of them, so you can't simply poll the device fd.  If you do, how do you
>> know which interrupt was triggered?  The solution that vfio adopted was to
>> associate each interrupt with an eventfd, allowing it to be individually
>> polled.  Since you can pass an eventfd with SCM_RIGHTS, and since kvm can
>> trigger guest interrupts using an eventfd, the solution is very flexible.
>>
>>> Example code would be even better...
>>>
>>>
>>
>> This is the vfio dpdk interface code:
>>
>> http://dpdk.org/browse/dpdk/tree/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>
>> basically, the equivalent uio msix code would be very similar if uio adopts
>> a similar interface:
>>
>> http://dpdk.org/browse/dpdk/tree/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>>
>> (current code lacks msi/msix support, of course).
> So you really want a driver that behaves exactly like vfio.
> Which immediately begs a question: why not extend vfio
> to cover your usecase.

The only "like VFIO" behavior we implement here is binding the MSI-X 
interrupt notification to eventfd descriptor. This doesn't justifies the 
hassle of implementing IOMMU-less VFIO mode.



>

--
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]

#1240521

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.

VFIO does all of that.

> 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.

-- 
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]

#1240550

On 10/06/15 17:56, 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.

I've already explained that there is no need for any additional memory 
protections since it won't be able to protect anything.

>
> Besides, that's not true.
> Your patch queries MSI capability, sets # of vectors.

My patch doesn't set # of vectors.

> You even hinted you want to add BAR mapping down the road.
>
> VFIO does all of that.
>
>> 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.

Again, uio_pci_generic with my patch simply follows the UIO design and 
in addition allows mapping MSI-X interrupts to eventfd's and it does it 
much more laconically compared to VFIO. Therefore these two modules 
won't be more related than they are today.

thanks,
vlad

>

--
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]

#1240570

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).

>> 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]

#1240858

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.

> >> 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]

#1241014

On Tue, 06 Oct 2015 12:51:20 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> 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

I agree with the design, but not with the taint argument.
(Unless you want to taint any and all use of UIO drivers which can
 already do this).
--
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]

#1241019

On Tue, 2015-10-06 at 22:32 +0100, Stephen Hemminger wrote:
> On Tue, 06 Oct 2015 12:51:20 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > 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
> 
> I agree with the design, but not with the taint argument.
> (Unless you want to taint any and all use of UIO drivers which can
>  already do this).

Yes, actually, if the bus master bit gets enabled all bets are off.  I
don't see how that leaves a supportable kernel, so we might as well
taint it.  Isn't this exactly why we taint for proprietary drivers, we
have no idea what it has mucked with in kernel space.  This just moves
the proprietary driver out to userspace without an iommu to protect the
host.  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]

#1241210

On 10/07/15 00:58, Stephen Hemminger wrote:
> Go ahead and submit a seperate taint bit for UIO as a patch.

This patch already does this.

thanks,
vlad

>
>
> On Tue, Oct 6, 2015 at 10:41 PM, Alex Williamson 
> <alex.williamson@redhat.com <mailto:alex.williamson@redhat.com>> wrote:
>
>     On Tue, 2015-10-06 at 22:32 +0100, Stephen Hemminger wrote:
>     > On Tue, 06 Oct 2015 12:51:20 -0600
>     > Alex Williamson <alex.williamson@redhat.com
>     <mailto:alex.williamson@redhat.com>> wrote:
>     >
>     > > 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
>     >
>     > I agree with the design, but not with the taint argument.
>     > (Unless you want to taint any and all use of UIO drivers which can
>     >  already do this).
>
>     Yes, actually, if the bus master bit gets enabled all bets are off.  I
>     don't see how that leaves a supportable kernel, so we might as well
>     taint it.  Isn't this exactly why we taint for proprietary drivers, we
>     have no idea what it has mucked with in kernel space.  This just moves
>     the proprietary driver out to userspace without an iommu to
>     protect the
>     host.  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]

#1241218

On 10/07/15 09:53, Avi Kivity wrote:
> On 10/07/2015 12:58 AM, Stephen Hemminger wrote:
>> Go ahead and submit a seperate taint bit for UIO as a patch.
>>
>
> Taint should only be applied if bus mastering is enabled (to avoid 
> annoying the users of the original uio use case)

Pls., note that this series would enable the legacy INT#X mode if 
possible and this, of course, without enabling bus mastering and without 
tainting the kernel.
This means that the current users of uio_pci_generic won't feel/get any 
difference after/if these patches are applied since before these patches 
it could only be used with the devices that do have INT#X capability.

>
>>
>> On Tue, Oct 6, 2015 at 10:41 PM, Alex Williamson 
>> <alex.williamson@redhat.com <mailto:alex.williamson@redhat.com>> wrote:
>>
>>     On Tue, 2015-10-06 at 22:32 +0100, Stephen Hemminger wrote:
>>     > On Tue, 06 Oct 2015 12:51:20 -0600
>>     > Alex Williamson <alex.williamson@redhat.com
>>     <mailto:alex.williamson@redhat.com>> wrote:
>>     >
>>     > > 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
>>     >
>>     > I agree with the design, but not with the taint argument.
>>     > (Unless you want to taint any and all use of UIO drivers which can
>>     >  already do this).
>>
>>     Yes, actually, if the bus master bit gets enabled all bets are
>>     off.  I
>>     don't see how that leaves a supportable kernel, so we might as well
>>     taint it.  Isn't this exactly why we taint for proprietary
>>     drivers, we
>>     have no idea what it has mucked with in kernel space. This just moves
>>     the proprietary driver out to userspace without an iommu to
>>     protect the
>>     host.  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]

#1241219

On 10/07/15 11:00, Vlad Zolotarov wrote:
>
>
> On 10/07/15 09:53, Avi Kivity wrote:
>> On 10/07/2015 12:58 AM, Stephen Hemminger wrote:
>>> Go ahead and submit a seperate taint bit for UIO as a patch.
>>>
>>
>> Taint should only be applied if bus mastering is enabled (to avoid 
>> annoying the users of the original uio use case)
>
> Pls., note that this series would enable the legacy INT#X mode if 
> possible 

By default I meant.

> and this, of course, without enabling bus mastering and without 
> tainting the kernel.
> This means that the current users of uio_pci_generic won't feel/get 
> any difference after/if these patches are applied since before these 
> patches it could only be used with the devices that do have INT#X 
> capability.
>
>>
>>>
>>> On Tue, Oct 6, 2015 at 10:41 PM, Alex Williamson 
>>> <alex.williamson@redhat.com <mailto:alex.williamson@redhat.com>> wrote:
>>>
>>>     On Tue, 2015-10-06 at 22:32 +0100, Stephen Hemminger wrote:
>>>     > On Tue, 06 Oct 2015 12:51:20 -0600
>>>     > Alex Williamson <alex.williamson@redhat.com
>>>     <mailto:alex.williamson@redhat.com>> wrote:
>>>     >
>>>     > > 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
>>>     >
>>>     > I agree with the design, but not with the taint argument.
>>>     > (Unless you want to taint any and all use of UIO drivers which 
>>> can
>>>     >  already do this).
>>>
>>>     Yes, actually, if the bus master bit gets enabled all bets are
>>>     off.  I
>>>     don't see how that leaves a supportable kernel, so we might as well
>>>     taint it.  Isn't this exactly why we taint for proprietary
>>>     drivers, we
>>>     have no idea what it has mucked with in kernel space. This just 
>>> moves
>>>     the proprietary driver out to userspace without an iommu to
>>>     protect the
>>>     host.  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]

#1241174

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.

--
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]

#1241674

On 10/07/2015 07:31 PM, Alex Williamson wrote:
>>>     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.

That's what I thought as well, but apparently adding msix support to the 
already insecure uio drivers is even worse.

--
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]

#1241806

On Wed, Oct 07, 2015 at 07:39:16PM +0300, Avi Kivity wrote:
> That's what I thought as well, but apparently adding msix support to the
> already insecure uio drivers is even worse.

I'm glad you finally agree what these drivers are doing is insecure.

And basically kernel cares about security, no one wants to maintain insecure stuff.

So you guys should think harder whether this code makes any sense upstream.

Getting support from kernel is probably the biggest reason to put code
upstream, and this driver taints kernel unconditionally so you don't get
that.

Alternatively, most of the problem you are trying to solve is for
virtualization - and it is is better addressed at the hypervisor level.
There are enough opensource hypervisors out there - work on IOMMU
support there would be time well spent.

-- 
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]

#1241958

On Thu, Oct 08, 2015 at 12:05:11AM +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 07, 2015 at 07:39:16PM +0300, Avi Kivity wrote:
> > That's what I thought as well, but apparently adding msix support to the
> > already insecure uio drivers is even worse.
> 
> I'm glad you finally agree what these drivers are doing is insecure.
> 
Michael, please stop this meaningless world play. The above is said in
the contexts of a device that is meant to be accessible by regular users
and obviously for that purpose uio is insecure (in its current state btw).
If you give user access to your root block device this device will be
insecure too, so according to your logic block device is insecure?
Pushing the code from uio to vfio means that vfio will have to implement
access policy by itself - allow iommu mode to regular users, but
no-iommu to root only. Implementing policy in the kernel is bad. Well
the alternative is to add /dev/vfio/nommu like you've said, but what
would be the difference between this and uio eludes me.

--
			Gleb.
--
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]

#1242029

On Thu, Oct 08, 2015 at 07:19:13AM +0300, Gleb Natapov wrote:
> Well
> the alternative is to add /dev/vfio/nommu like you've said, but what
> would be the difference between this and uio eludes me.

Are you familiar with vfio that you ask such a question?

Here's the vfio pci code:

$ wc -l drivers/vfio/pci/*
   27 drivers/vfio/pci/Kconfig
    4 drivers/vfio/pci/Makefile
 1217 drivers/vfio/pci/vfio_pci.c
 1602 drivers/vfio/pci/vfio_pci_config.c
  675 drivers/vfio/pci/vfio_pci_intrs.c
   92 drivers/vfio/pci/vfio_pci_private.h
  238 drivers/vfio/pci/vfio_pci_rdwr.c
 3855 total

There's some code dealing with iommu groups in
drivers/vfio/pci/vfio_pci.c,
but most of it is validating input and
presenting a consistent interface to userspace.

This is exactly what's missing here.

There's also drivers/vfio/virqfd.c which deals
with sending interrupts over eventfds correctly.

-- 
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]

#1242034

On Thu, Oct 08, 2015 at 10:41:53AM +0300, Michael S. Tsirkin wrote:
> On Thu, Oct 08, 2015 at 07:19:13AM +0300, Gleb Natapov wrote:
> > Well
> > the alternative is to add /dev/vfio/nommu like you've said, but what
> > would be the difference between this and uio eludes me.
> 
> Are you familiar with vfio that you ask such a question?
> 
Yes, I do and I do not see anything of value that vfio can add to nommu
setup besides complexity, but I do see why it will have to have special
interface not applicable to regular vfio (hint: there is not HW to translate
virtual address to physical) and why it will have to be accessible to
root user only.

> Here's the vfio pci code:
> 
> $ wc -l drivers/vfio/pci/*
>    27 drivers/vfio/pci/Kconfig
>     4 drivers/vfio/pci/Makefile
>  1217 drivers/vfio/pci/vfio_pci.c
>  1602 drivers/vfio/pci/vfio_pci_config.c
>   675 drivers/vfio/pci/vfio_pci_intrs.c
>    92 drivers/vfio/pci/vfio_pci_private.h
>   238 drivers/vfio/pci/vfio_pci_rdwr.c
>  3855 total
>
> There's some code dealing with iommu groups in
> drivers/vfio/pci/vfio_pci.c,
> but most of it is validating input and
> presenting a consistent interface to userspace.
> 
What is has to do with the patch series in question? Non patched
uio_generic code does not validate input. If you think it should by all
means write the code (don't break existing use cases while doing so),
but the patch under discussion does not even access pci device from
userspace, so it will not be affected by said filtering.

> This is exactly what's missing here.
It is not missing in this patch series, it is missing from upstream
code. I do not remember this been an issue when uio_generic was accepted
into the kernel. The reason was because it meant to be accessible by root
only. VFIO was designed to be used by regular user from ground up, so
obviously unrestricted access to pci space was out of the question.
Different use cases lead to different designs, how surprising.

> 
> There's also drivers/vfio/virqfd.c which deals
> with sending interrupts over eventfds correctly.
> 
As opposite to this patch that deals with them incorrectly? In what way?

--
			Gleb.
--
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]

#1242116

On Thu, Oct 08, 2015 at 10:59:10AM +0300, Gleb Natapov wrote:
> I do not remember this been an issue when uio_generic was accepted
> into the kernel. The reason was because it meant to be accessible by root
> only.

No - because it does not need bus mastering. So it can be used safely
with some devices.

[mst@robin linux]$ git grep pci_set_master|wc -l 533
[mst@robin linux]$ git grep pci_enable|wc -l 1597

Looks like about 2/3 devices don't need to be bus masters.

It's up to admin not to bind it to devices, and that is unfortunate,
but manually binding an incorrect driver to a device is generally
a hard problem to solve.

> > There's also drivers/vfio/virqfd.c which deals
> > with sending interrupts over eventfds correctly.
> > 
> As opposite to this patch that deals with them incorrectly? In what way?

cleanup on fd close is not handled.

-- 
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]

#1242126

On Thu, Oct 08, 2015 at 12:38:28PM +0300, Michael S. Tsirkin wrote:
> On Thu, Oct 08, 2015 at 10:59:10AM +0300, Gleb Natapov wrote:
> > I do not remember this been an issue when uio_generic was accepted
> > into the kernel. The reason was because it meant to be accessible by root
> > only.
> 
> No - because it does not need bus mastering. So it can be used safely
> with some devices.
> 
It still can be used safely with same devices. Admittedly I did not look
close, but I am sure the patch does not enable bus mastering if MSI
interrupt is not requested. If not, well that can be fixed. But more
importantly it can be used unsafely in its current state. Not only can,
it is widely used so.

> [mst@robin linux]$ git grep pci_set_master|wc -l 533
> [mst@robin linux]$ git grep pci_enable|wc -l 1597
> 
> Looks like about 2/3 devices don't need to be bus masters.
> 
> It's up to admin not to bind it to devices, and that is unfortunate,
> but manually binding an incorrect driver to a device is generally
> a hard problem to solve.
> 
> > > There's also drivers/vfio/virqfd.c which deals
> > > with sending interrupts over eventfds correctly.
> > > 
> > As opposite to this patch that deals with them incorrectly? In what way?
> 
> cleanup on fd close is not handled.
> 
Have you commented about this on the patch and it was not fixed?

--
			Gleb.
--
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]

#1242292

On Thu, Oct 08, 2015 at 12:45:08PM +0300, Gleb Natapov wrote:
> On Thu, Oct 08, 2015 at 12:38:28PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Oct 08, 2015 at 10:59:10AM +0300, Gleb Natapov wrote:
> > > I do not remember this been an issue when uio_generic was accepted
> > > into the kernel. The reason was because it meant to be accessible by root
> > > only.
> > 
> > No - because it does not need bus mastering. So it can be used safely
> > with some devices.
> > 
> It still can be used safely with same devices.

This patch does not add any functionality that can be used safely.
And for no good reason except it's a hassle.

> Admittedly I did not look
> close, but I am sure the patch does not enable bus mastering if MSI
> interrupt is not requested. If not, well that can be fixed. But more
> importantly it can be used unsafely in its current state. Not only can,
> it is widely used so.
> 
> > [mst@robin linux]$ git grep pci_set_master|wc -l 533
> > [mst@robin linux]$ git grep pci_enable|wc -l 1597
> > 
> > Looks like about 2/3 devices don't need to be bus masters.
> > 
> > It's up to admin not to bind it to devices, and that is unfortunate,
> > but manually binding an incorrect driver to a device is generally
> > a hard problem to solve.
> > 
> > > > There's also drivers/vfio/virqfd.c which deals
> > > > with sending interrupts over eventfds correctly.
> > > > 
> > > As opposite to this patch that deals with them incorrectly? In what way?
> > 
> > cleanup on fd close is not handled.
> > 
> Have you commented about this on the patch and it was not fixed?

No - I only noticed this when I poked at VFIO to try and explain
why it's not a good idea to duplicate its code. I'm sure there
are more issues that we'll just have to re-discover the hard way
if we do try.

> --
> 			Gleb.
--
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]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web