WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Is msix_flush_writes() really needed? And multi_msi_*() flaw

To: <linux-pci@xxxxxxxxxxxxxxx>
Subject: [Xen-devel] Is msix_flush_writes() really needed? And multi_msi_*() flawed?
From: "Jan Beulich" <jbeulich@xxxxxxxxxx>
Date: Fri, 07 Nov 2008 08:53:49 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 07 Nov 2008 00:53:40 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
msix_flush_writes() is being called exclusively after calling 
msi_set_mask_bit(),
and that function already does follow writel() by readl() in the MSI-X case.

Also, isn't the single use of multi_msi_capable() broken (in the event that
the Multiple Message Capable field was 5, the shift would be undefined,
on x86 in particular would yield 1 as the result, where 0 would be needed),
and the subsequent twiddling of temp needlessly complicated (subtracting
one should be sufficient here).

And isn't multi_msi_enable(), though unused (since msi_{en,dis}able() are
unused), broken altogether (shifting num right by 1 instead of taking the
binary log)?

In case so:

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

---
 drivers/pci/msi.c |   30 ++----------------------------
 drivers/pci/msi.h |    2 +-
 2 files changed, 3 insertions(+), 29 deletions(-)

--- linux-2.6.28-rc3/drivers/pci/msi.c  2008-11-03 15:53:11.000000000 +0100
+++ 2.6.28-rc3-pci-msi-misc/drivers/pci/msi.c   2008-11-07 09:11:36.000000000 
+0100
@@ -103,29 +103,6 @@ static void msix_set_enable(struct pci_d
        }
 }
 
-static void msix_flush_writes(unsigned int irq)
-{
-       struct msi_desc *entry;
-
-       entry = get_irq_msi(irq);
-       BUG_ON(!entry || !entry->dev);
-       switch (entry->msi_attrib.type) {
-       case PCI_CAP_ID_MSI:
-               /* nothing to do */
-               break;
-       case PCI_CAP_ID_MSIX:
-       {
-               int offset = entry->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
-                       PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET;
-               readl(entry->mask_base + offset);
-               break;
-       }
-       default:
-               BUG();
-               break;
-       }
-}
-
 /*
  * PCI 2.3 does not specify mask bits for each MSI interrupt.  Attempting to
  * mask all MSI interrupts by clearing the MSI enable bit does not work
@@ -255,13 +232,11 @@ void write_msi_msg(unsigned int irq, str
 void mask_msi_irq(unsigned int irq)
 {
        msi_set_mask_bits(irq, 1, 1);
-       msix_flush_writes(irq);
 }
 
 void unmask_msi_irq(unsigned int irq)
 {
        msi_set_mask_bits(irq, 1, 0);
-       msix_flush_writes(irq);
 }
 
 static int msi_free_irqs(struct pci_dev* dev);
@@ -389,9 +364,8 @@ static int msi_capability_init(struct pc
                pci_read_config_dword(dev,
                        msi_mask_bits_reg(pos, entry->msi_attrib.is_64),
                        &maskbits);
-               temp = (1 << multi_msi_capable(control));
-               temp = ((temp - 1) & ~temp);
-               maskbits |= temp;
+               temp = 1U << (multi_msi_capable(control) - 1);
+               maskbits |= (temp << 1) - 1;
                pci_write_config_dword(dev, entry->msi_attrib.is_64, maskbits);
                entry->msi_attrib.maskbits_mask = temp;
        }
--- linux-2.6.28-rc3/drivers/pci/msi.h  2007-02-04 19:44:54.000000000 +0100
+++ 2.6.28-rc3-pci-msi-misc/drivers/pci/msi.h   2008-11-07 09:13:09.000000000 
+0100
@@ -23,7 +23,7 @@
 #define multi_msi_capable(control) \
        (1 << ((control & PCI_MSI_FLAGS_QMASK) >> 1))
 #define multi_msi_enable(control, num) \
-       control |= (((num >> 1) << 4) & PCI_MSI_FLAGS_QSIZE);
+       control |= ((ilog2(num) << 4) & PCI_MSI_FLAGS_QSIZE)
 #define is_64bit_address(control)      (!!(control & PCI_MSI_FLAGS_64BIT))
 #define is_mask_bit_support(control)   (!!(control & PCI_MSI_FLAGS_MASKBIT))
 #define msi_enable(control, num) multi_msi_enable(control, num); \




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>