Re: [Qemu-devel] [PATCH v3 17/46] ivshmem: improve debug messages

[Claudio Fontana]

On 15.09.2015 18:07, address@hidden wrote:
> From: Marc-André Lureau <address@hidden>
> 
> Some misc improvements to ivshmem debug.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  hw/misc/ivshmem.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index cda7dce..084bc89 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -208,10 +208,13 @@ static void ivshmem_io_write(void *opaque, hwaddr addr,
>              if (vector < s->peers[dest].nb_eventfds) {
>                  IVSHMEM_DPRINTF("Notifying VM %d on vector %d\n", dest, 
> vector);
>                  event_notifier_set(&s->peers[dest].eventfds[vector]);
> +            } else {
> +                IVSHMEM_DPRINTF("Invalid destination vector %d on VM %d\n",
> +                                vector, dest);
>              }
>              break;
>          default:
> -            IVSHMEM_DPRINTF("Invalid VM Doorbell VM %d\n", dest);
> +            IVSHMEM_DPRINTF("Unhandled write " TARGET_FMT_plx "\n", addr);
>      }
>  }
>  
> @@ -263,9 +266,9 @@ static void ivshmem_receive(void *opaque, const uint8_t 
> *buf, int size)
>  {
>      IVShmemState *s = opaque;
>  
> -    ivshmem_IntrStatus_write(s, *buf);
> +    IVSHMEM_DPRINTF("ivshmem_receive 0x%02x size: %d\n", *buf, size);
>  
> -    IVSHMEM_DPRINTF("ivshmem_receive 0x%02x\n", *buf);
> +    ivshmem_IntrStatus_write(s, *buf);
>  }
>  
>  static int ivshmem_can_receive(void * opaque)
> @@ -592,6 +595,7 @@ static void ivshmem_use_msix(IVShmemState * s)
>      PCIDevice *d = PCI_DEVICE(s);
>      int i;
>  
> +    IVSHMEM_DPRINTF("use msix\n");
>      if (!msix_present(d)) {
>          return;
>      }
> 

This MSI-X use vs not use is a bit confusing to me.
I see that the use of MSI is controlled mainly by IVSHMEM_MSI (Property "msi"),
but then there are if (msix_present()) checks spread around.

Could this printf be a bit more clear, possibly adding other DPRINTFs as 
necessary?

Is your IVSHMEM_DPRINTF("use msix\n"); actually intended to mean ("using 
MSIX\n")? But then why is the check for if (!msix_present(d)) only afterwards?

Ciao,

Claudio