From owner-svn-src-all@FreeBSD.ORG Tue Jul 19 03:22:51 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B48A01065670; Tue, 19 Jul 2011 03:22:51 +0000 (UTC) (envelope-from marcel@xcllnt.net) Received: from mail.xcllnt.net (mail.xcllnt.net [70.36.220.4]) by mx1.freebsd.org (Postfix) with ESMTP id 7E0278FC0C; Tue, 19 Jul 2011 03:22:51 +0000 (UTC) Received: from sa-nc-spg-102.static.jnpr.net (natint3.juniper.net [66.129.224.36]) (authenticated bits=0) by mail.xcllnt.net (8.14.5/8.14.5) with ESMTP id p6J3Mgu5001510 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Mon, 18 Jul 2011 20:22:49 -0700 (PDT) (envelope-from marcel@xcllnt.net) Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Marcel Moolenaar In-Reply-To: Date: Mon, 18 Jul 2011 20:22:37 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <451BAE6D-C142-4E24-AE43-3BEC325F43F6@xcllnt.net> References: <201107181519.p6IFJfOK028280@svn.freebsd.org> <208D139E-CDC3-428D-8D5F-F772BFAF164C@xcllnt.net> To: Attilio Rao X-Mailer: Apple Mail (2.1084) Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r224187 - in head: sys/amd64/amd64 sys/arm/arm sys/arm/sa11x0 sys/i386/i386 sys/ia64/ia64 sys/kern sys/mips/mips sys/powerpc/aim sys/powerpc/booke sys/sparc64/sparc64 sys/sys usr.bin/vm... X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jul 2011 03:22:51 -0000 On Jul 18, 2011, at 7:31 PM, Attilio Rao wrote: > 2011/7/19 Marcel Moolenaar : >>=20 >> On Jul 18, 2011, at 5:59 PM, Attilio Rao wrote: >>=20 >>> 2011/7/19 Marcel Moolenaar : >>>>=20 >>>> On Jul 18, 2011, at 8:19 AM, Attilio Rao wrote: >>>>=20 >>>>> Author: attilio >>>>> Date: Mon Jul 18 15:19:40 2011 >>>>> New Revision: 224187 >>>>> URL: http://svn.freebsd.org/changeset/base/224187 >>>>>=20 >>>>> Log: >>>>> - Remove the eintrcnt/eintrnames usage and introduce the concept = of >>>>> sintrcnt/sintrnames which are symbols containing the size of = the 2 >>>>> tables. >>>>> - For amd64/i386 remove the storage of intr* stuff from assembly = files. >>>>> This area can be widely improved by applying the same to other >>>>> architectures and likely finding an unified approach among them = and >>>>> move the whole code to be MI. More work in this area is = expected to >>>>> happen fairly soon. >>>>>=20 >>>>> No MFC is previewed for this patch. >>>>=20 >>>> You just broke ia64 and possibly other 64-bit architectures: >>>>=20 >>>> ".word" declares a 16-bit integral on ia64 and the size symbols >>>> are of type size_t (=3D64 bit). We'll be having misaligned loads >>>> (=3D kernel panics) and/or reading garbage... >>>=20 >>> I'm a bit surprised of this though. >>> .hword was supposed to be the 16-bit integral, while .word was >>> supposed to be the 32-bits one, if I read my "info as" on amd64. >>=20 >> Well... all I can say is that assembly is the least transposable >> language, besides of course machine code itself :-) >>=20 >>> Anyway, what do you think about this patch? (I still need to test = it): >>> http://www.freebsd.org/~attilio/64bits-fixup.diff >>=20 >> Looks good to me, though I don't know enough about mips to comment >> on that. I'm not going to be anal about the use of ".quad" instead >> of "data8" for ia64 -- let's get it fixed first (I think we have >> ".byte" in locore.S anyway :-) >=20 > We do. > Anyway, I've updated the patch in order to use data8 in ia64 case (you > are the maintainer, so you have the last word) even if I'm not sure > there is a real need to discourage .quad. >=20 > Thanks for pointing at this breakage, please review and approve in = case. The change for ia64 is not quite right. This is what you have in the latest patch: Index: sys/ia64/ia64/locore.S =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/ia64/ia64/locore.S (revision 224207) +++ sys/ia64/ia64/locore.S (working copy) @@ -207,13 +207,13 @@ intr_n =3D intr_n + 1 .endr EXPORT(sintrnames) - .word INTRCNT_COUNT * INTRNAME_LEN + data8 INTRCNT_COUNT * INTRNAME_LEN =20 .align 8 EXPORT(intrcnt) .fill INTRCNT_COUNT, 8, 0 EXPORT(sintrcnt) - .word INTRCNT_COUNT + data8 INTRCNT_COUNT =20 .text // in0: image base It defines sintrcnt as a 64-bit integral with value INTRCNT_COUNT, which is merely the number of counters, not the storage size of intrcnt itself. Multiply by 8 (=3D sizeof(size_t)) and you're good: Index: /sys/ia64/ia64/locore.S =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- /sys/ia64/ia64/locore.S (revision 224207) +++ /sys/ia64/ia64/locore.S (working copy) @@ -207,13 +207,13 @@ intr_n =3D intr_n + 1 .endr EXPORT(sintrnames) - .word INTRCNT_COUNT * INTRNAME_LEN + data8 INTRCNT_COUNT * INTRNAME_LEN =20 .align 8 EXPORT(intrcnt) .fill INTRCNT_COUNT, 8, 0 EXPORT(sintrcnt) - .word INTRCNT_COUNT + data8 INTRCNT_COUNT * 8 =20 .text // in0: image base Sorry for pointing this out in the rebound. I just tested the patch for ia64 and noticed interrupt counters we're getting fetched right. You did all the other architectures in the patch right, so with this last tweak it's reviewed and approved from my end. FYI, --=20 Marcel Moolenaar marcel@xcllnt.net