From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58593) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLxV6-0006xb-QE for qemu-devel@nongnu.org; Wed, 20 Jan 2016 13:29:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLxV4-0001l5-Fi for qemu-devel@nongnu.org; Wed, 20 Jan 2016 13:29:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54961) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLxV4-0001ky-8S for qemu-devel@nongnu.org; Wed, 20 Jan 2016 13:28:58 -0500 Message-ID: <1453314537.32741.303.camel@redhat.com> From: Alex Williamson Date: Wed, 20 Jan 2016 11:28:57 -0700 In-Reply-To: <20160120181134.GJ13215@redhat.com> References: <20160120180552.21926.99964.stgit@gimli.home> <20160120181134.GJ13215@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH] vfio: Add sysfsdev property for pci & platform List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: kevin.tian@intel.com, jike.song@intel.com, qemu-devel@nongnu.org, laine@redhat.com, eric.auger@linaro.org On Wed, 2016-01-20 at 18:11 +0000, Daniel P. Berrange wrote: > On Wed, Jan 20, 2016 at 11:06:55AM -0700, Alex Williamson wrote: > > vfio-pci currently requires a host=3D parameter, which comes in the > > form of a PCI address in [domain:] notation.=C2=A0= =C2=A0We > > expect to find a matching entry in sysfs for that under > > /sys/bus/pci/devices/.=C2=A0=C2=A0vfio-platform takes a similar appro= ach, but > > defines the host=3D parameter to be a string, which can be matched > > directly under /sys/bus/platform/devices/.=C2=A0=C2=A0On the PCI side= , we > > have > > some interest in using vfio to expose vGPU devices.=C2=A0=C2=A0These = 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.=C2=A0=C2=A0There'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.=C2=A0=C2=A0To fit within the vfio framewo= rk, it > > would need a kernel struct device and associated IOMMU group, but > > those are easy constraints to manage. > >=20 > > 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.=C2=A0=C2= =A0The > > vfio API already has support for probing the device type to ensure > > compatibility with either vfio-pci or vfio-platform. > >=20 > > With this, a vfio-pci device could either be specified as: > >=20 > > -device vfio-pci,host=3D02:00.0 > >=20 > > or > >=20 > > -device vfio- > > pci,sysfsdev=3D/sys/devices/pci0000:00/0000:00:1c.0/0000:02:00.0 > >=20 > > or even > >=20 > > -device vfio-pci,sysfsdev=3D/sys/bus/pci/devices/0000:02:00.0 > >=20 > > When vGPU support comes along, this might look something more like: > >=20 > > -device vfio-pci,sysfsdev=3D/sys/devices/virtual/intel-vgpu/vgpu0@000 > > 0:00:02.0 > >=20 > > 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. > >=20 > > 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. > >=20 > > Signed-off-by: Alex Williamson > > --- > > =C2=A0hw/vfio/pci.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A0130 +++++= ++++++++++++---------- > > -------------- > > =C2=A0hw/vfio/platform.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A0=C2=A055 ++++++++++------- > > =C2=A0include/hw/vfio/vfio-common.h |=C2=A0=C2=A0=C2=A0=C2=A01=C2=A0 > > =C2=A03 files changed, 86 insertions(+), 100 deletions(-) > >=20 > > 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) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (vdev->pdev.romfile || !vdev->pdev.r= om_bar) { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* Since pci ha= ndles romfile, just print a message and > > return */ > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (vfio_blackl= ist_opt_rom(vdev) && vdev->pdev.romfile) { > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0error_printf("Warning : Device at %04x:%02x:%02x.%x " > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0"is known to cause system instability > > issues during " > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0"option rom execution. " > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0"Proceeding anyway since user specified > > romfile\n", > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0vdev->host.domain, vdev->host.bus, vdev- > > >host.slot, > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0vdev->host.function); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0error_printf("Warning : Device at %s is known to cause > > system instability issues during option rom execution. Proceeding > > anyway since user specified romfile\n", > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0vdev->vbasedev.name); >=20 > This error message should really be split across lines like > the original code was. Likewise for other cases in this patch kernel and QEMU development disagree here and I tend to favor the kernel's argument that if a user is inclined enough to go grep through the source to find an error message, it should work. =C2=A0Breaking print= ed error messages at arbitrary lengths to fit an 80 column window makes that more difficult. =C2=A0Is there sufficient passion for a hard 80 colu= mn limit on printed strings in QEMU to ignore that useful property? Thanks, Alex