Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property

[Alex Williamson]

On Sun, 2012-06-10 at 17:03 +0300, Michael S. Tsirkin wrote:
> On Sun, Jun 10, 2012 at 07:41:51AM -0600, Alex Williamson wrote:
> > > > >>>> vfio_pci.c contains a nice function called "parse_hostaddr". You 
> > > > >>>> may
> > > > >>>> guess what it does. ;)
> > > > >>>
> > > > >>> Interesting. Why? This looks strange to me:
> > > > >>> I would expect the admin to bind a device to vfio
> > > > >>> the way it's now bound to a stub.
> > > > >>> The pass /dev/vfioXXX to qemu.
> > > > >>
> > > > >> That's the "libvirt way". We surely also want the "qemu command line
> > > > >> way" for which this kind of service is needed.
> > > > >>
> > > > >> Jan
> > > > >>
> > > > > 
> > > > > Yes, I imagine the qemu command line passing in /dev/vfioXXX,
> > > > > the libvirt way will pass in an fd for above. No?
> > > > 
> > > > As far as I understand the API, there is no device file per assigned
> > > > device.
> > > 
> > > Does it do pci_get_domain_bus_and_slot like kvm then?
> > > With all the warts like you have to remember to bind pci stub
> > > or you get two drivers for one device?
> > > If true that's unfortunate IMHO.
> 
> I hope the answer to the above is no?

No, it does a probe for devices.  We need the devaddr to compare against
dev_name of the device to figure out which device the user is attempting
to identify.

> > > > Also, this [domain:]bus:dev.fn format is more handy for the
> > > > command line.
> > > > 
> > > > Jan
> > > > 
> > > 
> > > Then users could add udev rules that will name vfio devices
> > > like this.  Another interesting option: /dev/vfio/eth0/vf1.
> > > That's better I think: no one really likes running lspci
> > > and guessing the address from there.
> > 
> > That's not at all how VFIO works.  /dev/vfio/# represents a group, which
> > may contain one or more devices.  Even if libvirt passes a file
> > descriptor for the group, qemu needs to know which device in the group
> > to add to the guest, so parsing a device address is still necessary.
> > Thanks,
> > 
> > Alex
> 
> That's very unusual, and unfortunate.  For example this means that I
> must update applications just because I move a card to another slot.
> UIO does not have this problem.
> The fact that it's broken in kvm ATM seems to have made people
> think it's okay, but it really is a bug. We didn't fix it
> because vfio was supposed to be the solution.

I don't know what you're talking about here.  Are you suggesting that
needing to specify -device pci-assign,host=3.0 changing to host=4.0 when
you move a card is broken?  How does UIO avoid such a problem.  UIO-pci
requires the user to use pci-sysfs for resource access, so it surely
cares about the device address.

> I do realize you want to represent a group of devices somehow but can't
> this be solved without breaking naming devices with udev? For example, the
> device could be a file as well. You would then use the fd to identify the
> device within the group. And in a somewhat common case of a single device
> within the group, you can even make opening the group optional.
> Don't know if this fix I suggest makes sense at all but it's a real
> problem all the same.

Unfortunately, exposing individual devices just confuses the ownership
model we require for groups.  It would provide the illusion of being
able to assign an individual device, without the reality of the
grouping.  Groups are owned either by _a_ user or by the kernel, they
can't be split across multiple users (at least not with any guarantees
of isolation).  The current interface makes this clear.  Thanks,

Alex