marcandre.lur...@redhat.com writes: > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > Simplify the interrupt handling by having a single callback on irq&msi > cases. Remove usage of CharDriver, replace it with > qemu_set_fd_handler(). Use event_notifier_test_and_clear() to read the > eventfd. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > hw/misc/ivshmem.c | 55 > ++++++++++++++++++------------------------------------- > 1 file changed, 18 insertions(+), 37 deletions(-) > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > index 11780b1..9eb8a81 100644 > --- a/hw/misc/ivshmem.c > +++ b/hw/misc/ivshmem.c > @@ -263,15 +263,6 @@ static const MemoryRegionOps ivshmem_mmio_ops = { > }, > }; > > -static void ivshmem_receive(void *opaque, const uint8_t *buf, int size) > -{ > - IVShmemState *s = opaque; > - > - IVSHMEM_DPRINTF("ivshmem_receive 0x%02x size: %d\n", *buf, size); > - > - ivshmem_IntrStatus_write(s, *buf);
Before your patch, we write the first byte received to s->intrstatus. This is odd; ivshmem_device_spec.txt says "The status register is set to 1 when an interrupt occurs." > -} > - > static int ivshmem_can_receive(void * opaque) > { > return sizeof(int64_t); > @@ -282,15 +273,24 @@ static void ivshmem_event(void *opaque, int event) > IVSHMEM_DPRINTF("ivshmem_event %d\n", event); > } > > -static void fake_irqfd(void *opaque, const uint8_t *buf, int size) { > - > +static void ivshmem_vector_notify(void *opaque) > +{ > MSIVector *entry = opaque; > PCIDevice *pdev = entry->pdev; > IVShmemState *s = IVSHMEM(pdev); > int vector = entry - s->msi_vectors; > + EventNotifier *n = &s->peers[s->vm_id].eventfds[vector]; > + > + if (!event_notifier_test_and_clear(n)) { > + return; > + } > > IVSHMEM_DPRINTF("interrupt on vector %p %d\n", pdev, vector); > - msix_notify(pdev, vector); > + if (ivshmem_has_feature(s, IVSHMEM_MSI)) { > + msix_notify(pdev, vector); > + } else { > + ivshmem_IntrStatus_write(s, 1); After the patch, we write 1 to s->intrstatus. May well be an improvement, or even a bug fix, but it needs to be explained in the commit message. > + } > } > > static int ivshmem_vector_unmask(PCIDevice *dev, unsigned vector, > @@ -350,35 +350,16 @@ static void ivshmem_vector_poll(PCIDevice *dev, > } > } > > -static CharDriverState* create_eventfd_chr_device(IVShmemState *s, > - EventNotifier *n, > - int vector) > +static void watch_vector_notifier(IVShmemState *s, EventNotifier *n, > + int vector) > { > - /* create a event character device based on the passed eventfd */ > int eventfd = event_notifier_get_fd(n); > - CharDriverState *chr; > - > - chr = qemu_chr_open_eventfd(eventfd); > - > - if (chr == NULL) { > - error_report("creating chardriver for eventfd %d failed", eventfd); > - return NULL; > - } > - qemu_chr_fe_claim_no_fail(chr); > > /* if MSI is supported we need multiple interrupts */ > - if (ivshmem_has_feature(s, IVSHMEM_MSI)) { > - s->msi_vectors[vector].pdev = PCI_DEVICE(s); > - > - qemu_chr_add_handlers(chr, ivshmem_can_receive, fake_irqfd, > - ivshmem_event, &s->msi_vectors[vector]); > - } else { > - qemu_chr_add_handlers(chr, ivshmem_can_receive, ivshmem_receive, > - ivshmem_event, s); > - } > - > - return chr; > + s->msi_vectors[vector].pdev = PCI_DEVICE(s); > > + qemu_set_fd_handler(eventfd, ivshmem_vector_notify, > + NULL, &s->msi_vectors[vector]); > } > > static int check_shm_size(IVShmemState *s, int fd, Error **errp) > @@ -587,7 +568,7 @@ static void setup_interrupt(IVShmemState *s, int vector) > > if (!with_irqfd) { > IVSHMEM_DPRINTF("with eventfd"); > - s->eventfd_chr[vector] = create_eventfd_chr_device(s, n, vector); > + watch_vector_notifier(s, n, vector); > } else if (msix_enabled(pdev)) { > IVSHMEM_DPRINTF("with irqfd"); > if (ivshmem_add_kvm_msi_virq(s, vector) < 0) { I like the looks of it, not least because it enables removal of qemu_chr_open_eventfd() in the next patch. But I recommend to get an R-by from someone who actually understands this chardev stuff. Paolo, perhaps?