Re: [Qemu-devel] [RFC PATCH] vfio: Add sysfsdev property for pci & platform

[Daniel P. Berrange]

On Wed, Jan 20, 2016 at 11:06:55AM -0700, Alex Williamson wrote:
> vfio-pci currently requires a host= parameter, which comes in the
> form of a PCI address in [domain:]<bus:slot.function> notation.  We
> expect to find a matching entry in sysfs for that under
> /sys/bus/pci/devices/.  vfio-platform takes a similar approach, but
> defines the host= parameter to be a string, which can be matched
> directly under /sys/bus/platform/devices/.  On the PCI side, we have
> some interest in using vfio to expose vGPU devices.  These are not
> actual discrete PCI devices, so they don't have a compatible host PCI
> bus address or a device link where QEMU wants to look for it.  There's
> also really no requirement that vfio can only be used to expose
> physical devices, a new vfio bus and iommu driver could expose a
> completely emulated device.  To fit within the vfio framework, it
> would need a kernel struct device and associated IOMMU group, but
> those are easy constraints to manage.
> 
> To support such devices, which would include vGPUs, that honor the
> VFIO PCI programming API, but are not necessarily backed by a unique
> PCI address, add support for specifying any device in sysfs.  The
> vfio API already has support for probing the device type to ensure
> compatibility with either vfio-pci or vfio-platform.
> 
> With this, a vfio-pci device could either be specified as:
> 
> -device vfio-pci,host=02:00.0
> 
> or
> 
> -device vfio-pci,sysfsdev=/sys/devices/pci0000:00/0000:00:1c.0/0000:02:00.0
> 
> or even
> 
> -device vfio-pci,sysfsdev=/sys/bus/pci/devices/0000:02:00.0
> 
> When vGPU support comes along, this might look something more like:
> 
> -device 
> vfio-pci,sysfsdev=/sys/devices/virtual/intel-vgpu/address@hidden:00:02.0
> 
> NB - This is only a made up example path, but it should be noted that
> the device namespace is global for vfio, a virtual device cannot
> overlap with existing namespaces and should not create a name prone to
> conflict, such as a simple instance number.
> 
> The same changes is made for vfio-platform but is only compile tested.
> In both cases, specifying sysfsdev has precedence over the old host
> option.
> 
> Signed-off-by: Alex Williamson <address@hidden>
> ---
>  hw/vfio/pci.c                 |  130 
> +++++++++++++++++------------------------
>  hw/vfio/platform.c            |   55 ++++++++++-------
>  include/hw/vfio/vfio-common.h |    1 
>  3 files changed, 86 insertions(+), 100 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 1fb868c..bfe4215 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -880,12 +880,8 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
>      if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
>          /* Since pci handles romfile, just print a message and return */
>          if (vfio_blacklist_opt_rom(vdev) && vdev->pdev.romfile) {
> -            error_printf("Warning : Device at %04x:%02x:%02x.%x "
> -                         "is known to cause system instability issues during 
> "
> -                         "option rom execution. "
> -                         "Proceeding anyway since user specified romfile\n",
> -                         vdev->host.domain, vdev->host.bus, vdev->host.slot,
> -                         vdev->host.function);
> +            error_printf("Warning : Device at %s is known to cause system 
> instability issues during option rom execution. Proceeding anyway since user 
> specified romfile\n",
> +                         vdev->vbasedev.name);

This error message should really be split across lines like
the original code was. Likewise for other cases in this patch


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|