From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 4/5] msix: Don't process table changes while disabled Date: Tue, 18 Oct 2011 13:18:09 +0200 Message-ID: <20111018111809.GC28776@redhat.com> References: <16be9aa8fe4b5ebef73192c487b3cdf61c7a14a9.1318924251.git.jan.kiszka@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , Marcelo Tosatti , kvm@vger.kernel.org To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:26111 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754386Ab1JRLRF (ORCPT ); Tue, 18 Oct 2011 07:17:05 -0400 Content-Disposition: inline In-Reply-To: <16be9aa8fe4b5ebef73192c487b3cdf61c7a14a9.1318924251.git.jan.kiszka@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Oct 18, 2011 at 09:50:53AM +0200, Jan Kiszka wrote: > As long as MSI-X is disabled, it's incorrect to invoke > msix_handle_mask_update on per-vector mask changes. That may misguide > the config notifier callback or spuriously trigger an MSI event. > > Signed-off-by: Jan Kiszka Same question as on the previous patch. At the moment, the only user (virtio) seems to check msix_enabled before registering notifiers. See virtio_pci_query_guest_notifiers. Is there some way you see to trigger incorrect behaviour? > --- > hw/msix.c | 14 +++++++++----- > 1 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/hw/msix.c b/hw/msix.c > index e351c68..f5c8d08 100644 > --- a/hw/msix.c > +++ b/hw/msix.c > @@ -277,12 +277,16 @@ static void msix_mmio_write(void *opaque, target_phys_addr_t addr, > bool was_masked = msix_is_masked(dev, vector); > > pci_set_long(dev->msix_table_page + offset, val); > - if (kvm_enabled() && kvm_irqchip_in_kernel()) { > - kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev, vector)); > - } > > - if (was_masked != msix_is_masked(dev, vector)) { > - msix_handle_mask_update(dev, vector); > + if (msix_enabled(dev)) { > + if (kvm_enabled() && kvm_irqchip_in_kernel()) { > + kvm_msix_update(dev, vector, was_masked, > + msix_is_masked(dev, vector)); > + } > + > + if (was_masked != msix_is_masked(dev, vector)) { > + msix_handle_mask_update(dev, vector); > + } > } > } > > -- > 1.7.3.4