diff mbox

[19/38] ivshmem: Assert interrupts are set up once

Message ID 1456771254-17511-20-git-send-email-armbru@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Markus Armbruster Feb. 29, 2016, 6:40 p.m. UTC
An interrupt is set up when the interrupt's file descriptor is
received.  Each message applies to the next interrupt vector.
Therefore, each vector cannot be set up more than once.

ivshmem_add_kvm_msi_virq() half-heartedly tries not to rely on this by
doing nothing then, but that's not going to recover from this error
should it become possible in the future.  watch_vector_notifier()
doesn't even try.

Simply assert what is the case, so we get alerted if we ever screw it
up.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/misc/ivshmem.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Marc-André Lureau March 2, 2016, 12:02 p.m. UTC | #1
Hi

On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <armbru@redhat.com> wrote:
> An interrupt is set up when the interrupt's file descriptor is
> received.  Each message applies to the next interrupt vector.
> Therefore, each vector cannot be set up more than once.
>
> ivshmem_add_kvm_msi_virq() half-heartedly tries not to rely on this by
> doing nothing then, but that's not going to recover from this error
> should it become possible in the future.  watch_vector_notifier()
> doesn't even try.
>
> Simply assert what is the case, so we get alerted if we ever screw it
> up.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/misc/ivshmem.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index fc37feb..9d2209d 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -349,7 +349,7 @@ static void watch_vector_notifier(IVShmemState *s, EventNotifier *n,
>  {
>      int eventfd = event_notifier_get_fd(n);
>
> -    /* if MSI is supported we need multiple interrupts */
> +    assert(!s->msi_vectors[vector].pdev);

ok, why not

>      s->msi_vectors[vector].pdev = PCI_DEVICE(s);
>
>      qemu_set_fd_handler(eventfd, ivshmem_vector_notify,
> @@ -535,10 +535,7 @@ static int ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector)
>      int ret;
>
>      IVSHMEM_DPRINTF("ivshmem_add_kvm_msi_virq vector:%d\n", vector);
> -
> -    if (s->msi_vectors[vector].pdev != NULL) {
> -        return 0;
> -    }
> +    assert(!s->msi_vectors[vector].pdev);

that one is more tricky, since irqfd may be enabled/disabled
dynamically from ivshmem_write_config(), and
ivshmem_add_kvm_msi_virq() may be called at different times. However,
I think an assert is correct as there shouldn't be a valid state where
add_kvm_msi_virq() is called with the same vector when irqfd is
enabled.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
diff mbox

Patch

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index fc37feb..9d2209d 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -349,7 +349,7 @@  static void watch_vector_notifier(IVShmemState *s, EventNotifier *n,
 {
     int eventfd = event_notifier_get_fd(n);
 
-    /* if MSI is supported we need multiple interrupts */
+    assert(!s->msi_vectors[vector].pdev);
     s->msi_vectors[vector].pdev = PCI_DEVICE(s);
 
     qemu_set_fd_handler(eventfd, ivshmem_vector_notify,
@@ -535,10 +535,7 @@  static int ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector)
     int ret;
 
     IVSHMEM_DPRINTF("ivshmem_add_kvm_msi_virq vector:%d\n", vector);
-
-    if (s->msi_vectors[vector].pdev != NULL) {
-        return 0;
-    }
+    assert(!s->msi_vectors[vector].pdev);
 
     ret = kvm_irqchip_add_msi_route(kvm_state, msg, pdev);
     if (ret < 0) {