Re: [Qemu-devel] [PATCH v6 3/4] vfio: vfio-pci device assignment driver

[Alex Williamson <alex.williamson@redhat.com>]

On Fri, 2012-10-05 at 16:54 +0000, Blue Swirl wrote:
> On Wed, Sep 26, 2012 at 5:19 PM, Alex Williamson
> <alex.williamson@redhat.com> wrote:
> > +
> > +typedef struct QEMU_PACKED VFIOIRQSetFD {
> > +    struct vfio_irq_set irq_set;
> > +    int32_t fd;
> > +} VFIOIRQSetFD;
> 
> I'm now getting this error from Clang:
> 
> /src/qemu/hw/vfio_pci.c:126:25: error: field 'irq_set' with variable
> sized type 'struct vfio_irq_set' not at the end of a struct or class
> is a GNU extension [-Werror,-Wgnu]
>     struct vfio_irq_set irq_set;
> 
> Does the kernel really use the fd field, isn't it implicit from the
> ioctl fd or are they different?

The kernel side is defined as:

struct vfio_irq_set {
        __u32   argsz;
        __u32   flags;
        __u32   index;
        __u32   start;
        __u32   count;
        __u8    data[];
};

Where data is the start of a variable sized array.  The data type of the
array depends on the flags.  The purpose of VFIOIRQSetFD is simply to
make a data type that I don't need to dynamically allocate.  You can
find other cases for MSI and MSIX where we don't know the array size and
do malloc the whole structure.  For this interrupt type we know there's
only one entry.  If there's a better way to do this, let me know.  VFIO
is only available on Linux hosts, so I have no particular reason to
avoid GNU extensions.

> > +
> > +static int vfio_enable_intx(VFIODevice *vdev)
> > +{
> > +    VFIOIRQSetFD irq_set_fd = {
> > +        .irq_set = {
> > +            .argsz = sizeof(irq_set_fd),
> > +            .flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_TRIGGER,
> > +            .index = VFIO_PCI_INTX_IRQ_INDEX,
> > +            .start = 0,
> > +            .count = 1,
> > +        },
> 
> Here the field is not even initialized.

It's initialized later...

> > +    };
> > +    uint8_t pin = vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1);
> > +    int ret;
> > +
> > +    if (vdev->intx.disabled || !pin) {
> > +        return 0;
> > +    }
> > +
> > +    vfio_disable_interrupts(vdev);
> > +
> > +    vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
> > +    ret = event_notifier_init(&vdev->intx.interrupt, 0);
> > +    if (ret) {
> > +        error_report("vfio: Error: event_notifier_init failed\n");
> > +        return ret;
> > +    }
> > +
> > +    irq_set_fd.fd = event_notifier_get_fd(&vdev->intx.interrupt);

Here.

Thanks,
Alex