| Message ID | 1456771254-17511-20-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 --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) {
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(-)