From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55646) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLxEM-0006or-QP for qemu-devel@nongnu.org; Wed, 20 Jan 2016 13:11:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLxEJ-00073f-F9 for qemu-devel@nongnu.org; Wed, 20 Jan 2016 13:11:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42489) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLxEJ-00073N-7h for qemu-devel@nongnu.org; Wed, 20 Jan 2016 13:11:39 -0500 Date: Wed, 20 Jan 2016 18:11:34 +0000 From: "Daniel P. Berrange" Message-ID: <20160120181134.GJ13215@redhat.com> References: <20160120180552.21926.99964.stgit@gimli.home> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160120180552.21926.99964.stgit@gimli.home> Subject: Re: [Qemu-devel] [RFC PATCH] vfio: Add sysfsdev property for pci & platform Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: kevin.tian@intel.com, jike.song@intel.com, qemu-devel@nongnu.org, laine@redhat.com, eric.auger@linaro.org 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:] 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/vgpu0@0000: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 > --- > 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 :|