On 2012-06-10 12:49, Michael S. Tsirkin wrote: > 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.
vfio_pci.c contains a nice function called "parse_hostaddr". You may guess what it does. ;) So we need this generic service. Let's called it pci-host-devaddr and be fine? Jan
signature.asc
Description: OpenPGP digital signature