Re: [PATCH 1/5] qemu-kvm: msix: Don't fire notifier spuriously on set/unset

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 18, 2011 at 09:50:50AM +0200, Jan Kiszka wrote:
> If MSI-X is disabled or the global mask is set, don't fire the notifier
> during registration or removal, reporting a wrong state.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Hmm.  Could you please describe how does one trigger the incorrect behaviour?
Looking at the upstream code, I notice that msix_notify_if_unmasked
already calls msix_is_masked which checks the function mask.  And it
also checks msix_entry_used which, ATM, is only set when msix is
enabled.

> ---
>  hw/msix.c |   22 ++++++++++++++--------
>  1 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 60d6d1e..6493937 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -563,10 +563,13 @@ int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f)
>      int r, n;
>      assert(!dev->msix_mask_notifier);
>      dev->msix_mask_notifier = f;
> -    for (n = 0; n < dev->msix_entries_nr; ++n) {
> -        r = msix_set_mask_notifier_for_vector(dev, n);
> -        if (r < 0) {
> -            goto undo;
> +    if ((dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &
> +        (MSIX_ENABLE_MASK | MSIX_MASKALL_MASK)) == MSIX_ENABLE_MASK) {
> +        for (n = 0; n < dev->msix_entries_nr; ++n) {
> +            r = msix_set_mask_notifier_for_vector(dev, n);
> +            if (r < 0) {
> +                goto undo;
> +            }
>          }
>      }
>      return 0;
> @@ -583,10 +586,13 @@ int msix_unset_mask_notifier(PCIDevice *dev)
>  {
>      int r, n;
>      assert(dev->msix_mask_notifier);
> -    for (n = 0; n < dev->msix_entries_nr; ++n) {
> -        r = msix_unset_mask_notifier_for_vector(dev, n);
> -        if (r < 0) {
> -            goto undo;
> +    if ((dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &
> +        (MSIX_ENABLE_MASK | MSIX_MASKALL_MASK)) == MSIX_ENABLE_MASK) {
> +        for (n = 0; n < dev->msix_entries_nr; ++n) {
> +            r = msix_unset_mask_notifier_for_vector(dev, n);
> +            if (r < 0) {
> +                goto undo;
> +            }
>          }
>      }
>      dev->msix_mask_notifier = NULL;
> -- 
> 1.7.3.4