Re: [PATCH 2/5] qemu-kvm: msix: Only invoke msix_handle_mask_update on changes

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

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 <jan.kiszka@siemens.com>

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;