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 <jan.kis...@siemens.com> > > > > 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 > >