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]

Date:	Tue, 12 May 2009 12:21:26 +1000
From:	Michael Ellerman <michael@...erman.id.au>
To:	David Miller <davem@...emloft.net>
Cc:	matthew@....cx, jbarnes@...tuousgeek.org,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix MSI-X with NIU cards

On Mon, 2009-05-11 at 16:40 -0700, David Miller wrote:
> From: Michael Ellerman <michael@...erman.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@...erman.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.

Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)