On Fri, 2012-10-05 at 17:22 +0000, Blue Swirl wrote:
> On Fri, Oct 5, 2012 at 5:11 PM, Alex Williamson
> <alex.williamson@xxxxxxxxxx> wrote:
> > 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@xxxxxxxxxx> 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[];
> > };
>
> Then the kernel only expects vfio_irq_set structure, not VFIOIRQSetFD,
> so you should use &irq_set_fd.irq_set instead of &irq_set_fd for the
> ioctl(). Then VFIOIRQSetFD can be rearranged to have fd field first,
> also QEMU_PACKED is not necessary.
Sorry, I was unclear. The kernel sees fd as data[0], that's the point
of the structure, so re-arranging it makes it useless. Thanks,
Alex
> > 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
> >
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
