From owner-freebsd-hackers@FreeBSD.ORG Wed Nov 16 10:48:29 2005 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 17F1016A41F for ; Wed, 16 Nov 2005 10:48:29 +0000 (GMT) (envelope-from ntarmos@diogenis.ceid.upatras.gr) Received: from poseidon.ceid.upatras.gr (poseidon.ceid.upatras.gr [150.140.141.169]) by mx1.FreeBSD.org (Postfix) with ESMTP id D9DEE43D6E for ; Wed, 16 Nov 2005 10:48:23 +0000 (GMT) (envelope-from ntarmos@diogenis.ceid.upatras.gr) Received: from rhea.ceid.upatras.gr (rhea.ceid.upatras.gr [150.140.141.171]) by poseidon.ceid.upatras.gr (Postfix) with ESMTP id AC7175C13C1 for ; Wed, 16 Nov 2005 12:48:21 +0200 (EET) Received: from localhost (localhost [127.0.0.1]) by rhea.ceid.upatras.gr (Postfix) with ESMTP id 1F5ED80003 for ; Wed, 16 Nov 2005 12:48:20 +0200 (EET) Received: from rhea.ceid.upatras.gr ([127.0.0.1]) by localhost (rhea [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 08421-09 for ; Wed, 16 Nov 2005 12:48:20 +0200 (EET) Received: from diogenis.ceid.upatras.gr (diogenis.ceid.upatras.gr [150.140.141.181]) by rhea.ceid.upatras.gr (Postfix) with ESMTP id 4255580002 for ; Wed, 16 Nov 2005 12:48:19 +0200 (EET) Received: by diogenis.ceid.upatras.gr (Postfix, from userid 21001) id E542FB013A; Wed, 16 Nov 2005 12:48:18 +0200 (EET) Date: Wed, 16 Nov 2005 12:48:19 +0200 From: Nikos Ntarmos To: freebsd-hackers@freebsd.org Message-ID: <20051116104819.GA10128@diogenis.ceid.upatras.gr> Mail-Followup-To: Nikos Ntarmos , freebsd-hackers@freebsd.org Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="gatW/ieO32f1wygP" Content-Disposition: inline Organization: NetCInS Lab., C.E.I.D., U. of Patras, Greece WWW-Homepage: http://noth.ceid.upatras.gr/ X-PGP-Fingerprint: 9680 60A7 DE60 0298 B1F0 9B22 9BA2 7569 CF95 160A Office-Phone: +30-2610-996919 Office-Fax: +30-2610-969011 GPS-Info: 38.2594N, 21.7428E User-Agent: Mutt/1.5.9i X-Virus-Scanned: by amavisd-new at ceid.upatras.gr Subject: Bogus ioapic entry X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Nov 2005 10:48:29 -0000 --gatW/ieO32f1wygP Content-Type: multipart/mixed; boundary="LZvS9be/3tNcYl/X" Content-Disposition: inline --LZvS9be/3tNcYl/X Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi all. I spent a couple of hours last night trying to get ACPI to function correctly on my new Acer Aspire 1692WLMi. I found out that with apic enabled, the system hangs when returning from S3, while with apic disabled it suffers from interrupt storms on irq9 and the screen is not reinitialized on return from S3 (with hw.acpi.reset_video set to both "0" and "1"). While trying to hunt this thing down, I noticed that the GENERIC kernel of 6.0-STABLE (and probably of other branches as well) incorrectly discovers two ioapic devices: ioapic1: Changing APIC ID to 2 ioapic0 irqs 0-23 on motherboard ioapic1 irqs 24-23 on motherboard There is something obviously wrong with ioapic1. The cause seems to be the handling of value and numintr in ioapic_create(...). value is a uint32_t, while numintr is a u_int, both of which are 32-bits long (per and at least on my i386). The code in ioapic_create(...), after the call to ioapic_read(apic, IOAPIC_VER), uses the latter's return value to compute the number of interrupts assigned to the current ioapic. This return value is checked against 0xffffff (that's 24 1-bits) to detect an error and bail out. This code chunk is also in -current (so perhaps I'm missing something here...) The (buggy?) BIOS implementation of my laptop returns a value of 0xffffffff (32 1-bits) in ioapic_read(...). This, however, doesn't trigger the current if-clause, so the code proceeds and creates ioapic1 with -1(!) assigned IRQs (due to the and-and-shift-plus-one formula of computing numintr). The attached diff fixes this situation and seems more correct to me than the current code, given that io_numintr is 8 bits and u_int is 32 bits per C standard. The problems with resuming from S3 still haunt me though. I guess I should have a look at acpi@ later today... Cheers. \n\n --LZvS9be/3tNcYl/X Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="io_apic.c.diff" Content-Transfer-Encoding: quoted-printable --- io_apic.c.orig Fri Aug 5 22:08:25 2005 +++ io_apic.c Wed Nov 16 12:26:47 2005 @@ -478,14 +478,21 @@ value =3D ioapic_read(apic, IOAPIC_VER); mtx_unlock_spin(&icu_lock); =20 - /* If it's version register doesn't seem to work, punt. */ - if (value =3D=3D 0xffffff) { + /* Determine the number of vectors and set the APIC ID. */ + numintr =3D ((value & IOART_VER_MAXREDIR) >> MAXREDIRSHIFT) + 1; + + /* numintr is a uint32_t but io_numintr is only 8 bits long. On my + * Acer Aspire 1692WLMi, two ioapic's are discovered. However, the + * second one is bogus, since ioapic_read(apic, IOAPIC_VER) returns + * 0xffffffff, which doesn't trigger the old if-clause (value =3D=3D + * 0xffffff). Keeping around an ioapic with -1 assigned IRQs doesn't + * make much sense, so: + */ + if ((numintr & 0x000000ff) =3D=3D 0) { pmap_unmapdev((vm_offset_t)apic, IOAPIC_MEM_REGION); return (NULL); } =20 - /* Determine the number of vectors and set the APIC ID. */ - numintr =3D ((value & IOART_VER_MAXREDIR) >> MAXREDIRSHIFT) + 1; io =3D malloc(sizeof(struct ioapic) + numintr * sizeof(struct ioapic_intsrc), M_IOAPIC, M_WAITOK); io->io_pic =3D ioapic_template; --LZvS9be/3tNcYl/X-- --gatW/ieO32f1wygP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Nikos Ntarmos iD8DBQFDew5zm6J1ac+VFgoRAihHAKCGEl+nzwUPqEJsg8menyuoE4RPZwCgkGjZ U/2bmem5pLsLug6YKVKmpYU= =KfKL -----END PGP SIGNATURE----- --gatW/ieO32f1wygP--