From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39484) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sdfhe-0005Rv-RX for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:49:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sdfhd-0004T1-01 for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:49:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11576) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sdfhc-0004Sw-Nm for qemu-devel@nongnu.org; Sun, 10 Jun 2012 06:49:00 -0400 Date: Sun, 10 Jun 2012 13:49:29 +0300 From: "Michael S. Tsirkin" Message-ID: <20120610104929.GG6250@redhat.com> References: <20120610093521.GA6250@redhat.com> <4FD4738C.6020507@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FD4738C.6020507@web.de> Subject: Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Alex Williamson , qemu-devel On Sun, Jun 10, 2012 at 12:14:36PM +0200, Jan Kiszka wrote: > On 2012-06-10 11:35, Michael S. Tsirkin wrote: > > On Mon, Jun 04, 2012 at 10:52:21AM +0200, Jan Kiszka wrote: > >> Add a property to receive a fully qualified PCI device address. > >> > >> Will be used by KVM device assignment. > >> > >> Signed-off-by: Jan Kiszka > > > > I'd like to ponder this a bit more. What bothers me is that this mixes > > two things: > > - addressing of qemu devices > > Using full device addresses there is a legacy feature, > > users really should supply the parent bus and > > the bus local address. > > - addressing devices on the linux host for assignment > > It so happens that the syntax matches > > the legacy naming very closely, > > but conceptually is completely unrelated > > We can keep code duplications, of course. > > > > >> --- > >> hw/qdev-properties.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > >> hw/qdev.h | 3 +++ > >> 2 files changed, 51 insertions(+), 0 deletions(-) > >> > >> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > >> index 32e41f1..6634f22 100644 > >> --- a/hw/qdev-properties.c > >> +++ b/hw/qdev-properties.c > >> @@ -946,6 +946,54 @@ PropertyInfo qdev_prop_pci_devfn = { > >> .max = 0xFFFFFFFFULL, > >> }; > >> > >> +static void get_pci_devaddr(Object *obj, Visitor *v, void *opaque, > >> + const char *name, Error **errp) > >> +{ > >> + DeviceState *dev = DEVICE(obj); > >> + Property *prop = opaque; > >> + PCIDeviceAddress *addr = qdev_get_prop_ptr(dev, prop); > >> + char buffer[10 + 3 + 1]; > >> + char *p = buffer; > >> + > >> + snprintf(buffer, sizeof(buffer), "%04x:%02x:%02x.%02x", > >> + addr->domain, addr->bus, addr->slot, addr->function); > >> + > >> + visit_type_str(v, &p, name, errp); > >> +} > >> + > >> +static void set_pci_devaddr(Object *obj, Visitor *v, void *opaque, > >> + const char *name, Error **errp) > >> +{ > >> + DeviceState *dev = DEVICE(obj); > >> + Property *prop = opaque; > >> + PCIDeviceAddress *addr = qdev_get_prop_ptr(dev, prop); > >> + Error *local_err = NULL; > >> + char *str; > >> + > >> + if (dev->state != DEV_STATE_CREATED) { > >> + error_set(errp, QERR_PERMISSION_DENIED); > >> + return; > >> + } > >> + > >> + visit_type_str(v, &str, name, &local_err); > >> + if (local_err) { > >> + error_propagate(errp, local_err); > >> + return; > >> + } > >> + > >> + if (qemu_parse_pci_devaddr(str, addr, > >> + PCI_DEVADDR_WITH_DOM_BUS_OPT | > >> + PCI_DEVADDR_WITH_FUNC) < 0) { > >> + error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str); > >> + } > >> +} > >> + > >> +PropertyInfo qdev_prop_pci_devaddr = { > >> + .name = "pci-devaddr", > > > > This is a very confusing name. Something like host-pci-address? > > That might be an option. > > > This also should be built on linux only. > > Why, what do we gain with #ifdefs? And isn't the addressing concept generic? Not the XXX:XX.X format. And not the concept of a domain. > > Can this be part of device assignment code instead of qdev? > > How does VFIO address their host devices? You get an fd I think. I think you don't need to know the host address. > And Xen? > > Jan > >