Re: Panic during agp_intel attach

[Iain Hibbert <plunky%rya-online.net@localhost>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 21 Jan 2008, Andreas Gustafsson wrote:

> I tried booting -current (as of 20080118-1955 EET) on a Compaq DeskPro EN,
> but it paniced during boot.  Here's a partial backtrace:
>
>   #3  0xc05686dd in panic (fmt=0xc0c5ed04 "kernel %sassertion \"%s\" failed: 
> file \"%s\", line %d") at ../../../../kern/subr_prf.c:204
>   #4  0xc098486f in __kernassert (t=0xc0beaa93 "diagnostic ", f=0xc0beaa6c 
> "../../../../arch/x86/pci/pci_machdep.c", l=432, e=0xc0beaa5b "(reg & 0x3) == 
> 0") at ../../../../../../lib/libkern/__assert.c:50
>   #5  0xc07c2c88 in pci_conf_write (pc=0x0, tag={mode1 = 2147483648, mode2 = 
> {port = 0, enable = 0 '\0', forward = 128 '\200'}}, reg=145, data=112) at 
> ../../../../arch/x86/pci/pci_machdep.c:432
>   #6  0xc07c0932 in agp_intel_init (sc=0xc177e400) at 
> ../../../../dev/pci/agp_intel.c:266
>   #7  0xc07c05ea in agp_intel_attach (parent=0xc1785e00, self=0xc177e400, 
> aux=0xc0e4978c) at ../../../../dev/pci/agp_intel.c:193
>   #8  0xc07bbd31 in agpattach (parent=0xc1785e00, self=0xc177e400, 
> aux=0xc0e49788) at ../../../../dev/pci/agp.c:331
>
> Apparently this is triggered by the following change:
>
>   revision 1.31
>   date: 2008/01/14 18:44:17;  author: dyoung;  state: Exp;  lines: +4 -2
>   KASSERT() that reads/writes from/to PCI configuration space are
>   aligned on 32-bit boundaries.
>
> combined with the following code in agp_intel.c:
>
>                 pci_conf_write(sc->as_pc, sc->as_tag,
>                         AGP_INTEL_ERRSTS, 0x70);
>
> and the following #define in agpreg.h:
>
>    #define AGP_INTEL_ERRSTS        0x91    /* Not i8XX */
>
> as 0x91 is not aligned to a 32-bit boundary.  What's the correct fix?

Hi Andreas,
            Did you find the correct solution to this? I just updated my
sources and find the same trouble.. (also in PR kern/37824)

  It seems that Martin Husemann gave the most correct answer

On Mon, 21 Jan 2008 Martin Huseman wrote:
> Check what the code does, use 0x90 and shift all values/constants
> accordingly, if anythings shifts out (have yet to see that), use 0x94
> and repeat.

I guess the below patch should be correct, certainly it gets through the
KASSERT and I don't detect any misfeature by this..  can anybody with PCI
knowledge confirm if thats ok to commit?

iain

- --- /usr/src/sys/dev/pci/agp_intel.c  2008-02-13 19:35:33.000000000 +0000
+++ agp_intel.c 2008-02-15 11:37:46.000000000 +0000
@@ -264,7 +264,7 @@

        default:
                pci_conf_write(sc->as_pc, sc->as_tag,
- -                     AGP_INTEL_ERRSTS, 0x70);
+                       AGP_INTEL_ERRSTS, 0x7000);
        }

        return (0);
- --- /usr/src/sys/dev/pci/agpreg.h     2008-02-13 19:35:33.000000000 +0000
+++ agpreg.h    2008-02-15 11:53:36.000000000 +0000
@@ -59,7 +59,7 @@
 #define        AGP_INTEL_I8XX_ERRSTS   0xc8

 /* Common register */
- -#define      AGP_INTEL_ERRSTS        0x91    /* Not i8XX */
+#define        AGP_INTEL_ERRSTS        0x90    /* Not i8XX */
 #define AGP_INTEL_AGPCMD       0xa8
 # define AGPCMD_SBA            (1U << 9)
 # define AGPCMD_AGPEN          (1U << 8)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (NetBSD)

iQEVAwUBR7V//vFJxoMWDXVDAQJzIAgAuM9rSF2PQ/YkUgqIA9+oQB2g4usyybur
A8Xtv+v2SQ21KPtBE6LVOJDkTpwEM3XuhvcTTrbVqjq60TB0aBm29iQoiPr4NR5J
l9X1AiW+XXMhjUtOPuyHCgImYOAJGlD9ekECxpmf9DnumXHis1+fRv3ax07urs9/
4zHKQw70lsLWVj/tj8/SR7Je52ZuexyP4XPCj0T4eV1BdSHH6kcGZNAZKf+afZH7
M00IKJjm6NmoEPW3y07/FooEBzsOdEKH49dkLXTW80//rYCSjsFLKoZJNmVSBgmc
zt3tRnExkd6Ebhu0JSpg88XBswrRbJFl+jXqLb8KZ+XNPV9as/hrQQ==
=UF3r
-----END PGP SIGNATURE-----