[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property |
Date: |
Sun, 10 Jun 2012 12:52:45 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
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 <address@hidden>
>>>
>>> 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
[Qemu-devel] [PATCH 01/13] pci: Refactor pci_change_irq_level, Jan Kiszka, 2012/06/04
[Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Jan Kiszka, 2012/06/04
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Michael S. Tsirkin, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Jan Kiszka, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Michael S. Tsirkin, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property,
Jan Kiszka <=
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Michael S. Tsirkin, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Jan Kiszka, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Michael S. Tsirkin, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Jan Kiszka, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Michael S. Tsirkin, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Alex Williamson, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Michael S. Tsirkin, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Alex Williamson, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Michael S. Tsirkin, 2012/06/10
- Re: [Qemu-devel] [PATCH 13/13] qdev-properties: Add pci-devaddr property, Alex Williamson, 2012/06/10