lkml.org 
[lkml]   [2010]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH V2] VFIO driver: Non-privileged user level PCI drivers
Date
On Sunday 13 June 2010 03:23:39 am Michael S. Tsirkin wrote:
> On Fri, Jun 11, 2010 at 03:15:53PM -0700, Tom Lyon wrote:
> > [ bunch of stuff about MSI-X checking and IOMMUs and config registers...]
> >
> > OK, here's the thing. The IOMMU API today does not do squat about
> > dealing with interrupts. Interrupts are special because the APIC
> > addresses are not each in their own page. Yes, the IOMMU hardware
> > supports it (at least Intel), and there's some Intel intr remapping
> > code (not AMD), but it doesn't look like it is enough.
>
> The iommu book from AMD seems to say that interrupt remapping table
> address is taken from the device table entry. So hardware support seems
> to be there, and to me it looks like it should be enough.
> Need to look at the iommu/msi code some more to figure out
> whether what linux does is handling this correctly -
> if it doesn't we need to fix that.
>
> > Therefore, we must not allow the user level driver to diddle the MSI
> > or MSI-X areas - either in config space or in the device memory space.
>
> It won't help.
> Consider that you want to let a userspace driver control
> the device with DMA capabilities.
>
> So if there is a range of addresses that device
> can write into that can break host, these writes
> can be triggered by userspace. Limiting
> userspace access to MSI registers won't help:
> you need a way to protect host from the device.

OK, after more investigation, I realize you are right.
We definitely need the IOMMU protection for interrupts, and
if we have it, a lot of the code for config space protection is pointless.
It does seem that the Intel intr_remapping code does what we want
(accidentally) but that the AMD iommu code does not yet do any
interrupt remapping. Joerg - can you comment? On the roadmap?

I should have an AMD system w IOMMU in a couple of days, so I
can check this out.

>
> > If the device doesn't have its MSI-X registers in nice page aligned
> > areas, then it is not "well-behaved" and it is S.O.L. The SR-IOV spec
> > recommends that devices be designed the well-behaved way.
> >
> > When the code in vfio_pci_config speaks of "virtualization" it means
> > that there are fake registers which the user driver can read or write,
> > but do not affect the real registers. BARs are one case, MSI regs
> > another. The PCI vendor and device ID are virtual because SR-IOV
> > doesn't supply them but I wanted the user driver to find them in the
> > same old place.
>
> Sorry, I still don't understand why do we bother. All this is already
> implemented in userspace. Why can't we just use this existing userspace
> implementation? It seems that all kernel needs to do is prevent
> userspace from writing BARs.

I assume the userspace of which you speak is qemu? This is not what I'm
doing with vfio - I'm interested in the HPC networking model of direct
user space access to the network.

> Why can't we replace all this complexity with basically:
>
> if (addr <= PCI_BASE_ADDRESS_5 && addr + len >= PCI_BASE_ADDRESS_0)
> return -ENOPERM;
>
> And maybe another register or two. Most registers should be fine.
>
> > [ Re: Hotplug and Suspend/Resume]
> > There are *plenty* of real drivers - brand new ones - which don't
> > bother with these today. Yeah, I can see adding them to the framework
> > someday - but if there's no urgent need then it is way down the
> > priority list.
>
> Well, for kernel drivers everything mostly works out of the box, it is
> handled by the PCI subsystem. So some kind of framework will need to be
> added for userspace drivers as well. And I suspect this issue won't be
> fixable later without breaking applications.

Whatever works out of the box for the kernel drivers which don't implement
suspend/resume will work for the user level drivers which don't.
>
> > Meanwhile, the other uses beckon.
>
> Which other uses? I thought the whole point was fixing
> what's broken with current kvm implementation.
> So it seems to be we should not rush it ignoring existing issues such as
> hotplug.
Non-kvm cases. That don't care about suspend/resume.






\
 
 \ /
  Last update: 2010-06-17 23:19    [from the cache]
©2003-2011 Jasper Spaans