From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: linux-next: OOPS at bot time From: Michael Ellerman To: Ben Hutchings In-Reply-To: <1279847985.4883.391.camel@localhost> References: <20100723102202.871a3131.sfr@canb.auug.org.au> <1279847985.4883.391.camel@localhost> Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-2jwhBwbFMozNiZtbZlTX" Date: Fri, 23 Jul 2010 12:05:40 +1000 Message-ID: <1279850740.6381.19.camel@concordia> Mime-Version: 1.0 Cc: Stephen Rothwell , ppc-dev , LKML , Jesse Barnes Reply-To: michael@ellerman.id.au List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --=-2jwhBwbFMozNiZtbZlTX Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2010-07-23 at 02:19 +0100, Ben Hutchings wrote: > On Fri, 2010-07-23 at 10:22 +1000, Stephen Rothwell wrote: > > Hi all, > >=20 > > My Power7 boot test paniced like this: (next-20100722) > >=20 > > %GQLogic Fibre Channel HBA Driver: 8.03.03-k0 > > qla2xxx 0002:01:00.2: enabling device (0144 -> 0146) > > qla2xxx 0002:01:00.2: Found an ISP8001, irq 35, iobase 0xd0000800800140= 00 > > ------------[ cut here ]------------ > > kernel BUG at drivers/pci/msi.c:205! > [...] > > Call Trace: > > [c00000000278b270] [c000000000048d9c] .rtas_setup_msi_irqs+0x1d8/0x254 = (unreliable) > > [c00000000278b360] [c00000000002a9cc] .arch_setup_msi_irqs+0x34/0x4c > > [c00000000278b3e0] [c0000000002fd3fc] .pci_enable_msix+0x49c/0x4ac > [...] > > That line number is this: > >=20 > > BUG_ON(!(entry->msg.address_hi | entry->msg.address_lo | > > entry->msg.data)); > >=20 > > in read_msi_msg_desc(). That BUG_ON was added by commit > > 2ca1af9aa3285c6a5f103ed31ad09f7399fc65d7 ("PCI: MSI: Remove unsafe and > > unnecessary hardware access") from the pci tree. >=20 > I wanted to assert that read_msi_msg_desc() is only used to update > MSI/MSI-X descriptors that have already been generated by Linux. It > looks like you found an exception. > > We could make read_msi_msg() fall back to reading from the hardware, but > I think that what the pSeries code is trying to do - save an MSI message > generated by firmware - is different from what the other callers want. > Instead we could add: >=20 > void save_msi_msg(unsigned int irq) > { > struct irq_desc *desc =3D irq_to_desc(irq); > struct msi_desc *entry =3D get_irq_desc_msi(desc); > struct msi_msg *msg =3D &entry->msg; >=20 > /* ...followed by the old implementation of read_msi_msg_desc() */ > } >=20 > Possibly conditional on something like CONFIG_ARCH_NEEDS_SAVE_MSI_MSG. Maybe. But then you end up with read_msi_msg(), which doesn't actually read anything, which I think is confusing. I'd rather read_msi_msg() read the message, from the device, and we have another routine which returns the previously saved msg from the msi_desc. cheers --=-2jwhBwbFMozNiZtbZlTX Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iEYEABECAAYFAkxI+PEACgkQdSjSd0sB4dJZBwCfYn60t5zXzoyyHNimeOZ36zx7 uqIAn0MgXgdUi7R1W2kux1Ur+hoBWn8Y =qTcF -----END PGP SIGNATURE----- --=-2jwhBwbFMozNiZtbZlTX--