From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 2/5] qemu-kvm: msix: Only invoke msix_handle_mask_update on changes Date: Tue, 18 Oct 2011 13:43:31 +0200 Message-ID: <20111018114331.GE28776@redhat.com> References: 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]:38809 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754949Ab1JRLm1 (ORCPT ); Tue, 18 Oct 2011 07:42:27 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Oct 18, 2011 at 09:50:51AM +0200, Jan Kiszka wrote: > Reorganize msix_mmio_writel so that msix_handle_mask_update is only > called on mask changes. Pass previous config space value to > msix_write_config so that it can check if a mask change took place. > > Signed-off-by: Jan Kiszka What we did in other cases is track the old value in device state. This makes the API easier to use correctly. I'm testing the following as a replacement - any comments? diff --git a/hw/msix.c b/hw/msix.c index b15bafc..655a600 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -79,6 +79,7 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries, /* Make flags bit writable. */ pdev->wmask[config_offset + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK | MSIX_MASKALL_MASK; + pdev->msix_function_masked = false; return 0; } @@ -117,16 +118,11 @@ static void msix_clr_pending(PCIDevice *dev, int vector) *msix_pending_byte(dev, vector) &= ~msix_pending_mask(vector); } -static int msix_function_masked(PCIDevice *dev) -{ - return dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] & MSIX_MASKALL_MASK; -} - static int msix_is_masked(PCIDevice *dev, int vector) { unsigned offset = vector * PCI_MSIX_ENTRY_SIZE + PCI_MSIX_ENTRY_VECTOR_CTRL; - return msix_function_masked(dev) || + return dev->msix_function_masked || dev->msix_table_page[offset] & PCI_MSIX_ENTRY_CTRL_MASKBIT; } @@ -144,6 +140,7 @@ void msix_write_config(PCIDevice *dev, uint32_t addr, { unsigned enable_pos = dev->msix_cap + MSIX_CONTROL_OFFSET; int vector; + bool fmsk; if (!range_covers_byte(addr, len, enable_pos)) { return; @@ -155,10 +152,12 @@ void msix_write_config(PCIDevice *dev, uint32_t addr, pci_device_deassert_intx(dev); - if (msix_function_masked(dev)) { + fmsk = dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] & MSIX_MASKALL_MASK; + if (dev->msix_function_masked == fmsk) { return; } + dev->msix_function_masked = fmsk; for (vector = 0; vector < dev->msix_entries_nr; ++vector) { msix_handle_mask_update(dev, vector); } diff --git a/hw/pci.h b/hw/pci.h index 86a81c8..5b67a6b 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -178,6 +178,8 @@ struct PCIDevice { unsigned *msix_entry_used; /* Region including the MSI-X table */ uint32_t msix_bar_size; + /* MSIX function mask */ + bool msix_function_masked; /* Version id needed for VMState */ int32_t version_id;