Date: Wed, 20 Sep 2006 00:36:08 +0400 From: Ruslan Ermilov <ru@FreeBSD.org> To: Maksim Yevmenkin <maksim.yevmenkin@gmail.com> Cc: cvs-src@FreeBSD.org, Marius Strobl <marius@FreeBSD.org>, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/kbdmux kbdmux.c Message-ID: <20060919203608.GE23360@rambler-co.ru> In-Reply-To: <bb4a86c70609191300t3bea8380qa1898d1a89b6fc27@mail.gmail.com> References: <200609191303.k8JD3AHl050783@repoman.freebsd.org> <bb4a86c70609190944o2438a4a4vae7b3bb2332522ee@mail.gmail.com> <20060919190645.GA23068@rambler-co.ru> <bb4a86c70609191236j13fe1563w123cb046261ee129@mail.gmail.com> <20060919194819.GA23360@rambler-co.ru> <bb4a86c70609191300t3bea8380qa1898d1a89b6fc27@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--idY8LE8SD6/8DnRI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 19, 2006 at 01:00:55PM -0700, Maksim Yevmenkin wrote: > On 9/19/06, Ruslan Ermilov <ru@freebsd.org> wrote: > >On Tue, Sep 19, 2006 at 12:36:38PM -0700, Maksim Yevmenkin wrote: > >> nope, same crash. the only thing that seems to help is to reverting > >> back to (int *) cast just like other keyboard drivers do. then it > >> works. > >> > >> i'm in the process of getting amd64 snapshot iso to try it on a couple > >> of boxes. if it will work then i'm going to back (int *) -> (intptr_t > >> *) changes introduced in rev 1.8. > >> > >Why? amd64 and i386 are unaffected (both of my i386 and amd64 work > >with the committed revision), and on sparc64 we need to find a proper > >fix. It cannot work properly with (intptr_t *) removed as demonstrated > >by the test program I sent to you. Here it's again for reference: >=20 > ok, i'm officially confused now. from what i understood, >=20 > i386 is not affected (intptr_t *) casing, right? >=20 Yes, because intptr_t is "int" on i386. > (intptr_t *) casing causes problem with amd64, right? >=20 Caused, because you applied *(intptr_t *) to things that didn't require it, which are now reverted back to *(int *). > sparc64 crashes with (intptr_t *) casting (tested locally) >=20 We need to find *where*, and find a proper fix. > now, >=20 > what i'm suggesting is to replace (intptr_t *) casting with (int *) as > it was before rev 1.8. i claim that after this change >=20 > i386 will not affected. >=20 Yes. > sparc64 will work and will not crash (tested locally) >=20 not crash !=3D work properly. Can you print the value of argument to KDSETLED and verify it's correct? Print it both as (int)*(intptr_t *)arg and *(int *)arg, and compare. > amd64 should work (i think), but i'm in the process of getting amd64 > iso to build a test box. >=20 Yes, it should. > i tested (int *) casting on sparc64 with SETLED, SETREPEAT and it > works fine. i made sure that parameters passed via ioctl() are all > correct. i will do the same test on amd64 before committing. >=20 KDSETREPEAT _does_ use *(int *) in the committed code, as it's a normal "_IOW('K', 102, keyboard_repeat_t)" which takes a pointer as an argument. KDSETLED isn't used outside the kernel, so I assume you tested it only when it's called from within the kernel? If so, try passing it from useland to see the endianness problem; I'm pretty sure it will fire. If it does, there are two options: - Fix in-kernel callers of KDSETLED to behave like userland. - Fix the definition of KDSETLED to be: #define KDSETLED _IOW('K', 66, int) and then fix kbdmux.c to use *(int *) when it accesses KDSETLED's argument. Similarly for KDSKBSTATE. KDSETRAD should still be accessed through (int)*(intptr_t *)arg to stay binary compatible. > also, like i said before, all other keyboard drivers, including > sunkbd(4) use (int *) cast and NOT (intptr_t *), and it seems to work > just fine. >=20 I think this problem is unnoticeable because you're talking about the KDSETLED here, and it's only used inside the kernel, and inside the kernel it's passed as a "pointer to int", while in case of userland it would be passed as a 64-bit value on the stack (on sparc64), and causing an endianness problem when accessed through *(int *)arg. > while i'm interested what is going on here, i'd rather commit > something that is tested and known to work then wait. it will buy some > time to do the proper analysis and fix. >=20 kbdmux didn't work on sparc64 anyway to the best of my knowledge, or am I mistaken? Cheers, --=20 Ruslan Ermilov ru@FreeBSD.org FreeBSD committer --idY8LE8SD6/8DnRI Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (FreeBSD) iD8DBQFFEFS4qRfpzJluFF4RAibwAJ44FdosDYpXiedbrYQk8CJhTjSx1wCeLt1i sX8CqubP9NVMlUjULn4tEOI= =IwVy -----END PGP SIGNATURE----- --idY8LE8SD6/8DnRI--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060919203608.GE23360>