2011/7/19 Marcel Moolenaar <mar...@xcllnt.net>: > > On Jul 18, 2011, at 7:31 PM, Attilio Rao wrote: > >> 2011/7/19 Marcel Moolenaar <mar...@xcllnt.net>: >>> >>> On Jul 18, 2011, at 5:59 PM, Attilio Rao wrote: >>> >>>> 2011/7/19 Marcel Moolenaar <mar...@xcllnt.net>: >>>>> >>>>> On Jul 18, 2011, at 8:19 AM, Attilio Rao wrote: >>>>> >>>>>> Author: attilio >>>>>> Date: Mon Jul 18 15:19:40 2011 >>>>>> New Revision: 224187 >>>>>> URL: http://svn.freebsd.org/changeset/base/224187 >>>>>> >>>>>> 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. >>>>>> >>>>>> No MFC is previewed for this patch. >>>>> >>>>> You just broke ia64 and possibly other 64-bit architectures: >>>>> >>>>> ".word" declares a 16-bit integral on ia64 and the size symbols >>>>> are of type size_t (=64 bit). We'll be having misaligned loads >>>>> (= kernel panics) and/or reading garbage... >>>> >>>> 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. >>> >>> Well... all I can say is that assembly is the least transposable >>> language, besides of course machine code itself :-) >>> >>>> Anyway, what do you think about this patch? (I still need to test it): >>>> http://www.freebsd.org/~attilio/64bits-fixup.diff >>> >>> 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 :-) >> >> 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. >> >> 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 > =================================================================== > --- sys/ia64/ia64/locore.S (revision 224207) > +++ sys/ia64/ia64/locore.S (working copy) > @@ -207,13 +207,13 @@ > intr_n = intr_n + 1 > .endr > EXPORT(sintrnames) > - .word INTRCNT_COUNT * INTRNAME_LEN > + data8 INTRCNT_COUNT * INTRNAME_LEN > > .align 8 > EXPORT(intrcnt) > .fill INTRCNT_COUNT, 8, 0 > EXPORT(sintrcnt) > - .word INTRCNT_COUNT > + data8 INTRCNT_COUNT > > .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 (= sizeof(size_t)) and you're good: > > Index: /sys/ia64/ia64/locore.S > =================================================================== > --- /sys/ia64/ia64/locore.S (revision 224207) > +++ /sys/ia64/ia64/locore.S (working copy) > @@ -207,13 +207,13 @@ > intr_n = intr_n + 1 > .endr > EXPORT(sintrnames) > - .word INTRCNT_COUNT * INTRNAME_LEN > + data8 INTRCNT_COUNT * INTRNAME_LEN > > .align 8 > EXPORT(intrcnt) > .fill INTRCNT_COUNT, 8, 0 > EXPORT(sintrcnt) > - .word INTRCNT_COUNT > + data8 INTRCNT_COUNT * 8 > > .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.
Thanks Marcel for the time you spent on this. Attilio -- Peace can only be achieved by understanding - A. Einstein _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"