| Message ID | 1242052211.7011.4.camel@concordia (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
From: Michael Ellerman <michael@ellerman.id.au> 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@ellerman.id.au> >> 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 :-/ > Or has msix_mask_irq() changed since 413f81eba? You could ask GIT, and to answer your question, indeed it has. 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. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 11 May 2009 16:40:17 -0700 (PDT) David Miller <davem@davemloft.net> wrote: > From: Michael Ellerman <michael@ellerman.id.au> > 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@ellerman.id.au> > >> 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 :-/ > > > Or has msix_mask_irq() changed since 413f81eba? > > You could ask GIT, and to answer your question, indeed it has. > > 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. Ok I'll stuff it into my for-linus queue now... Hopefully Matthew can answer the question in the meantime.
On Mon, 2009-05-11 at 16:40 -0700, David Miller wrote: > From: Michael Ellerman <michael@ellerman.id.au> > 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@ellerman.id.au> > >> 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.
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 6f2e629..3627732 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -455,8 +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);