[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/8] ivshmem: use a single eventfd callback, get
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 7/8] ivshmem: use a single eventfd callback, get rid of CharDriver |
Date: |
Fri, 29 Jan 2016 17:23:25 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
address@hidden writes:
> From: Marc-André Lureau <address@hidden>
>
> 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 <address@hidden>
> ---
> 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?
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 7/8] ivshmem: use a single eventfd callback, get rid of CharDriver,
Markus Armbruster <=