All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: qemu-devel@nongnu.org, david@gibson.dropbear.id.au
Subject: Re: [Qemu-devel] [PATCH v2] pci: relax pci_msi_get_message()
Date: Fri, 25 Nov 2016 06:11:01 +0200	[thread overview]
Message-ID: <20161125060705-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <1479802130-22640-1-git-send-email-peterx@redhat.com>

On Tue, Nov 22, 2016 at 04:08:50PM +0800, Peter Xu wrote:
> We are very strict in the past getting MSIs from commit
> d1f6af6a1 ("kvm-irqchip: simplify kvm_irqchip_add_msi_route"), assuming
> that MSI should be configured before hand when fetching. When we have
> unrecognized configurations, we panic the system. However looks like
> this is too strict to be working on some platform, and issues occured.
> Firstly it's found on a ppc case and fixed by David in:
> 
>   6d17a01 vfio/pci: Fix regression in MSI routing configuration
> 
> However we encountered another case now with windows virtio driver and
> reported (and possibly more):
> 
>   http://bugs.debian.org/844361
> 
> To make every driver/hardware happy, let's loosen the rule and go back
> to the original behavior - instead of panic the system, when we try to
> fetch MSI without configured MSI/MSI-X system, we just provide an empty
> message to make drivers happy.
> 
> Reported-by: Maciej Kotliński <makotlinski@gmail.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/pci/pci.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 24fae16..0cd2bb0 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2606,9 +2606,11 @@ MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
>      } else if (msi_enabled(dev)) {
>          msg = msi_get_message(dev, vector);
>      } else {
> -        /* Should never happen */
> -        error_report("%s: unknown interrupt type", __func__);
> -        abort();
> +        /*
> +         * Device is not configured with MSI/MSI-X yet, let's provide
> +         * an empty message to make all device drivers happy.
> +         */
> +        memset(&msg, 0, sizeof(msg));
>      }
>      return msg;
>  }

Looks like a hack to me. Even if callers happen to treat
0 message as a nop, there's no guarantee that's true for
all platforms. Pls change pci_get_msi_message to
return an error code, and fix callers to check that.

> -- 
> 2.7.4

  parent reply	other threads:[~2016-11-25  4:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-22  8:08 [Qemu-devel] [PATCH v2] pci: relax pci_msi_get_message() Peter Xu
2016-11-24  5:29 ` Peter Xu
2016-11-25  4:11 ` Michael S. Tsirkin [this message]
2016-11-25  5:28   ` Peter Xu
2016-11-25  6:47     ` Michael S. Tsirkin
2016-11-25  7:05       ` Peter Xu
  -- strict thread matches above, loose matches on Subject: below --
2016-11-25  5:53 Changlimin
2016-11-25  6:03 Changlimin
2016-11-25  6:31 ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161125060705-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.