linux-kernel - Re: [PATCH] Fix MSI-X with NIU cards

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090513100725.GE15360@parisc-linux.org>
Date:	Wed, 13 May 2009 04:07:25 -0600
From:	Matthew Wilcox <matthew@....cx>
To:	Michael Ellerman <michael@...erman.id.au>
Cc:	David Miller <davem@...emloft.net>, jbarnes@...tuousgeek.org,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix MSI-X with NIU cards

On Tue, May 12, 2009 at 12:21:26PM +1000, Michael Ellerman wrote:
> 158 static void msix_mask_irq(struct msi_desc *desc, u32 flag)                  
> 159 {
> 160         u32 mask_bits = desc->masked;
> 161         unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
> 162                                         PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET;
> 163         mask_bits &= ~1;
> 164         mask_bits |= flag;
> 165         writel(mask_bits, desc->mask_base + offset);
> 166         desc->masked = mask_bits;
> 167 }
> 
> So I don't see how this patch works, all it's doing is moving the readl().

I think you're right.  My patch is clearly wrong.  I don't understand how
it worked for Dave.  Maybe his system is ignoring the errors on write
but can't ignore errors on read?  Or maybe his card silently ignores
writes and generates errors on read.

Here's an updated version:

----

The NIU device refuses to allow accesses to MSI-X registers before MSI-X
is enabled.  This patch fixes the problem by moving the read of the mask
register to after MSI-X is enabled.

Reported-by: David S. Miller <davem@...emloft.net>
Tested-by: David S. Miller <davem@...emloft.net>
Reviewed-by: David S. Miller <davem@...emloft.net>
Signed-off-by: Matthew Wilcox <willy@...ux.intel.com>
Cc: Jesse Barnes <jbarnes@...tuousgeek.org>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>


diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 6f2e629..cd66579 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -455,9 +455,6 @@ static int msix_capability_init(struct pci_dev *dev,
 		entry->msi_attrib.default_irq = dev->irq;
 		entry->msi_attrib.pos = pos;
 		entry->mask_base = base;
-		entry->masked = readl(base + j * PCI_MSIX_ENTRY_SIZE +
-					PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
-		msix_mask_irq(entry, 1);
 
 		list_add_tail(&entry->list, &dev->msi_list);
 	}
@@ -493,6 +490,13 @@ static int msix_capability_init(struct pci_dev *dev,
 	msix_set_enable(dev, 1);
 	dev->msix_enabled = 1;
 
+	list_for_each_entry(entry, &dev->msi_list, list) {
+		int vector = entry->msi_attrib.entry_nr;
+		entry->masked = readl(base + vector * PCI_MSIX_ENTRY_SIZE +
+					PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
+		msix_mask_irq(entry, 1);
+	}
+
 	return 0;
 }
 

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/