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

[Michael Ellerman]

On Mon, 2009-05-11 at 16:40 -0700, David Miller wrote:
> From: Michael Ellerman <michael@xxxxxxxxxxxxxx>
> Date: Tue, 12 May 2009 00:30:11 +1000
> 
> > On Sun, 2009-05-10 at 22:36 -0700, David Miller wrote:
> >> From: Michael Ellerman <michael@xxxxxxxxxxxxxx>
> >> Date: Mon, 11 May 2009 11:21:51 +1000
> >> 
> >> > So I guess this device is just silently ignoring that write?
> >> 
> >> No, it signals a fault on the PCI bus, which is how we noticed
> >> this problem in the first place.
> > 
> > But the patch doesn't change that? It just removes the readl AFAICS:
> 
> I'm on slow satellite internet here out in the ocean, so I wish
> the author of this patch would help with your feedback :-/

Me too :) *cough*

> > Or has msix_mask_irq() changed since 413f81eba?
> 
> You could ask GIT, and to answer your question, indeed it has.

Sorry it was late. I don't think it has actually:

concordia powerpc $ git log 413f81eba..1d80cac0f drivers/pci/msi.c 
concordia powerpc $ 

And I don't see any changes in Jesse's git tree via gitweb.

I still see:

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().

> Please don't stall this patch very much, as the bug it is fixing konks
> out one of my primary test machines from even booting up.

Sure, chuck it in. I just don't get how it's solving the problem, and it
looks like it's writing to the mask bit without reading the reserved
ones first, which seems to violate the spec AFAICS[1]. But we can fix it
later.

cheers

[1]: I probably don't have the latest version.

Attachment: signature.asc
Description: This is a digitally signed message part