snd-ioctl32 is failing on x86_64 (no crashes, just errors)

snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Georgi Georgiev]

Running a 32-bit version of any application fails with:

amixer: Mixer hw:0 load error: Invalid argument

for example. I have snd-ioctl32 loaded properly (lsmod shows it).

Kernel is vanilla 2.6.10 and I just tried it with version 1.0.8rc2 of
alsa-driver. Also, alsa-lib and alsa-utils version 1.0.8rc2 are
installed in my 64-bit environment, and also in the 32-bit chroot env
where I am trying to run amixer from. The distro is Gentoo, and I also
have their alsa-headers-1.0.8_rc2 installed in the 32-bit chroot.

-- 
\/   Georgi Georgiev   \/  Professor: Doomsday device? Ah, now the     \/
/\    chutz@gg3.net    /\ ball's in Farnsworth's court. I suppose I    /\
\/  +81(90)6266-1163   \/ could part with one and still be feared.     \/


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Takashi Iwai]

At Thu, 6 Jan 2005 22:06:50 +0900,
Georgi Georgiev wrote:
> 
> Running a 32-bit version of any application fails with:
> 
> amixer: Mixer hw:0 load error: Invalid argument
> 
> for example. I have snd-ioctl32 loaded properly (lsmod shows it).

Try strace amixer and see which ioctl failed.  Maybe it's the size
mismatching of argument, or an unimplemented ioctl.

> Kernel is vanilla 2.6.10 and I just tried it with version 1.0.8rc2 of
> alsa-driver. Also, alsa-lib and alsa-utils version 1.0.8rc2 are
> installed in my 64-bit environment, and also in the 32-bit chroot env
> where I am trying to run amixer from. The distro is Gentoo, and I also
> have their alsa-headers-1.0.8_rc2 installed in the 32-bit chroot.

ALSA ioctl32 in 2.6.10 or older is buggy.  Try to avoid it.


Takashi


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Georgi Georgiev]

maillog: 07/01/2005-11:45:44(+0100): Takashi Iwai types
> At Thu, 6 Jan 2005 22:06:50 +0900,
> Georgi Georgiev wrote:
> > 
> > Running a 32-bit version of any application fails with:
> > 
> > amixer: Mixer hw:0 load error: Invalid argument
> > 
> > for example. I have snd-ioctl32 loaded properly (lsmod shows it).
> 
> Try strace amixer and see which ioctl failed.  Maybe it's the size
> mismatching of argument, or an unimplemented ioctl.

Here is the relevant part of a diff between the output of stracing 32
and 64-bit versions of amixer on 2.6.10:

--- amixer.2.6.10-y2.32.log     2005-01-08 16:07:48.000000000 +0900
+++ amixer.2.6.10-y2.64.log     2005-01-08 16:07:27.000000000 +0900
-ioctl(3, USBDEVFS_CONTROL, 0xffffcd4c)  = 0
-fcntl64(3, F_GETFL)                     = 0x2 (flags O_RDWR)
-fcntl64(3, F_SETFL, O_RDWR|O_NONBLOCK)  = 0
-ioctl(3, USBDEVFS_RELEASEINTERFACE, 0xffffcf90) = 0
-ioctl(3, USBDEVFS_RELEASEINTERFACE, 0xffffcf90) = 0
-ioctl(3, USBDEVFS_CONNECTINFO, 0xffffcc80) = 0
-ioctl(3, USBDEVFS_IOCTL, 0xffffc510)    = -1 EINVAL (Invalid argument)
-write(2, "amixer: ", 8)                 = 8
-write(2, "Mixer default load error: Invali"..., 42) = 42
-write(2, "\n", 1)                       = 1
+ioctl(3, USBDEVFS_CONTROL, 0x7fbfffeb7c) = 0
+fcntl(3, F_GETFL)                       = 0x8002 (flags O_RDWR|O_LARGEFILE|0x8000)
+fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK|0x8000) = 0
+ioctl(3, USBDEVFS_RELEASEINTERFACE, 0x7fbfffed90) = 0
+ioctl(3, USBDEVFS_RELEASEINTERFACE, 0x7fbfffed90) = 0
+open("/etc/mtab", O_RDONLY)             = 4
+fstat(4, {st_mode=S_IFREG|0644, st_size=1041, ...}) = 0
+mmap(NULL, 131072, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x2a9556c000
+read(4, "/dev/hdc3 / reiserfs rw,noatime,"..., 131072) = 1041
+close(4)                                = 0
+munmap(0x2a9556c000, 131072)            = 0
+open("/proc/meminfo", O_RDONLY)         = 4
+fstat(4, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
+mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x2a9556c000
+read(4, "MemTotal:       768156 kB\nMemFre"..., 1024) = 600
+close(4)                                = 0
+munmap(0x2a9556c000, 4096)              = 0
+ioctl(3, USBDEVFS_CONNECTINFO, 0x7fbfffeac0) = 0
+ioctl(3, USBDEVFS_IOCTL, 0x7fbfffd980)  = 0
+ioctl(3, USBDEVFS_CONNECTINFO, 0x7fbfffeac0) = 0
+ioctl(3, USBDEVFS_IOCTL, 0x7fbfffd980)  = 0
+ioctl(3, USBDEVFS_IOCTL, 0x7fbfffd980)  = 0

Except for minor differences (like 0x7fbfffda40 instead of 0x7fbfffeac0,
etc.), the log from 2.6.7 was identical to the one from 2.6.10.

> > Kernel is vanilla 2.6.10 and I just tried it with version 1.0.8rc2 of
> > alsa-driver. Also, alsa-lib and alsa-utils version 1.0.8rc2 are
> > installed in my 64-bit environment, and also in the 32-bit chroot env
> > where I am trying to run amixer from. The distro is Gentoo, and I also
> > have their alsa-headers-1.0.8_rc2 installed in the 32-bit chroot.
> 
> ALSA ioctl32 in 2.6.10 or older is buggy.  Try to avoid it.

Avoid using ioctl32? I don't have much of a choice since it doesn't work
anyway. It would be nice to get some sound on flash pages and wine,
though.

2.6.10 or older pretty much covers every kernel out there. What kernel
version is it OK to test this with?

-- 
*)   Georgi Georgiev   *) What hath Bob wrought?                       *)
(*    chutz@gg3.net    (*                                              (*
*)  +81(90)6266-1163   *)                                              *)


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Takashi Iwai]

At Sat, 8 Jan 2005 16:40:27 +0900,
Georgi Georgiev wrote:
> 
> maillog: 07/01/2005-11:45:44(+0100): Takashi Iwai types
> > At Thu, 6 Jan 2005 22:06:50 +0900,
> > Georgi Georgiev wrote:
> > > 
> > > Running a 32-bit version of any application fails with:
> > > 
> > > amixer: Mixer hw:0 load error: Invalid argument
> > > 
> > > for example. I have snd-ioctl32 loaded properly (lsmod shows it).
> > 
> > Try strace amixer and see which ioctl failed.  Maybe it's the size
> > mismatching of argument, or an unimplemented ioctl.
> 
> Here is the relevant part of a diff between the output of stracing 32
> and 64-bit versions of amixer on 2.6.10:
> 
> --- amixer.2.6.10-y2.32.log     2005-01-08 16:07:48.000000000 +0900
> +++ amixer.2.6.10-y2.64.log     2005-01-08 16:07:27.000000000 +0900
> -ioctl(3, USBDEVFS_CONTROL, 0xffffcd4c)  = 0
> -fcntl64(3, F_GETFL)                     = 0x2 (flags O_RDWR)
> -fcntl64(3, F_SETFL, O_RDWR|O_NONBLOCK)  = 0
> -ioctl(3, USBDEVFS_RELEASEINTERFACE, 0xffffcf90) = 0
> -ioctl(3, USBDEVFS_RELEASEINTERFACE, 0xffffcf90) = 0
> -ioctl(3, USBDEVFS_CONNECTINFO, 0xffffcc80) = 0
> -ioctl(3, USBDEVFS_IOCTL, 0xffffc510)    = -1 EINVAL (Invalid argument)

It's SNDRV_CTL_IOCTL_ELEM_READ.  Could you check which place returns
-EINVAL in _snd_ioctl32_ctl_elem_value in
alsa-kernel/core/ioctl32/ioctl32.c?

> > > Kernel is vanilla 2.6.10 and I just tried it with version 1.0.8rc2 of
> > > alsa-driver. Also, alsa-lib and alsa-utils version 1.0.8rc2 are
> > > installed in my 64-bit environment, and also in the 32-bit chroot env
> > > where I am trying to run amixer from. The distro is Gentoo, and I also
> > > have their alsa-headers-1.0.8_rc2 installed in the 32-bit chroot.
> > 
> > ALSA ioctl32 in 2.6.10 or older is buggy.  Try to avoid it.
> 
> Avoid using ioctl32? I don't have much of a choice since it doesn't work
> anyway. It would be nice to get some sound on flash pages and wine,
> though.

Yes, but the code in 2.6.10 may crash your system.
(On x86-64, this might work, though...)

> 2.6.10 or older pretty much covers every kernel out there. What kernel
> version is it OK to test this with?

The one in latest alsa-driver (alsa-kernel) tree.
I think Linus's bk and mm trees have included already fixes.


Takashi


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Georgi Georgiev]

maillog: 10/01/2005-12:26:28(+0100): Takashi Iwai types
> At Sat, 8 Jan 2005 16:40:27 +0900, Georgi Georgiev wrote:
> 
> It's SNDRV_CTL_IOCTL_ELEM_READ.  Could you check which place returns
> -EINVAL in _snd_ioctl32_ctl_elem_value in
> alsa-kernel/core/ioctl32/ioctl32.c?

It says:

        /* FIXME: indirect access is not supported */
        if (data->indirect) {
                err = -EINVAL;
                goto __end;
        }

There is another one shortly after this:

        switch (type) {
	...
        default:
                printk(KERN_ERR "snd_ioctl32_ctl_elem_value: unknown type %d\n", type);
                err = -EINVAL;
                goto __end;
        }

Seeing the printk I checked my logs:

Jan 10 21:08:24 lion ioctl32(amixer:243138): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc610) on /dev/snd/controlC0
Jan 10 21:08:27 lion ioctl32(amixer:243139): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc600) on /dev/snd/controlC0
Jan 10 21:21:06 lion ioctl32(amixer:243280): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0
Jan 10 21:21:09 lion ioctl32(amixer:243281): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0

though the error message is quite different than the printk above, it
could still be helpful.

I also don't see any differences between ioctl32.c from CVS and
1.0.8rc2. CVS version is at rev.1.26 and tagged as v1-0-8rc1.

> > > ALSA ioctl32 in 2.6.10 or older is buggy.  Try to avoid it.
> > 
> > Avoid using ioctl32? I don't have much of a choice since it doesn't work
> > anyway. It would be nice to get some sound on flash pages and wine,
> > though.
> 
> Yes, but the code in 2.6.10 may crash your system.
> (On x86-64, this might work, though...)
> 
> > 2.6.10 or older pretty much covers every kernel out there. What kernel
> > version is it OK to test this with?
> 
> The one in latest alsa-driver (alsa-kernel) tree.
> I think Linus's bk and mm trees have included already fixes.

The most important point that I did not get -- your advice about
avoiding ioctl32 is a different point of concern, that is completely
unrelated to the current problem, is it? In case this sounded rude --
advice is appreciated. Looked into it (I actually read a few reports
from sparc people whose machines ioctl32 broke, before submitting mine),
and if my machine ever crashes, I'll first upgrade the kernel and then
submit bugreports. But I don't have to worry about it until (if ever) my
machine hangs, right?

-- 
|    Georgi Georgiev   |  Zounds! I was never so bethumped with        |
|     chutz@gg3.net    |  words since I first called my brother's      |
|   +81(90)6266-1163   |  father dad. -- William Shakespeare, "Kind    |
|  ------------------- |  John" er                                     |


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Takashi Iwai]

[-- Attachment #1: Type: text/plain, Size: 2778 bytes --]

At Mon, 10 Jan 2005 21:33:42 +0900,
Georgi Georgiev wrote:
> 
> maillog: 10/01/2005-12:26:28(+0100): Takashi Iwai types
> > At Sat, 8 Jan 2005 16:40:27 +0900, Georgi Georgiev wrote:
> > 
> > It's SNDRV_CTL_IOCTL_ELEM_READ.  Could you check which place returns
> > -EINVAL in _snd_ioctl32_ctl_elem_value in
> > alsa-kernel/core/ioctl32/ioctl32.c?
> 
> It says:
> 
>         /* FIXME: indirect access is not supported */
>         if (data->indirect) {
>                 err = -EINVAL;
>                 goto __end;
>         }
> 
> There is another one shortly after this:
> 
>         switch (type) {
> 	...
>         default:
>                 printk(KERN_ERR "snd_ioctl32_ctl_elem_value: unknown type %d\n", type);
>                 err = -EINVAL;
>                 goto __end;
>         }

I know :)  Also, the return value from snd_ctl_elem_read() can be
-EINVAL.  My question was which of them returns indeed in your case...

> Seeing the printk I checked my logs:
> 
> Jan 10 21:08:24 lion ioctl32(amixer:243138): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc610) on /dev/snd/controlC0
> Jan 10 21:08:27 lion ioctl32(amixer:243139): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc600) on /dev/snd/controlC0
> Jan 10 21:21:06 lion ioctl32(amixer:243280): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0
> Jan 10 21:21:09 lion ioctl32(amixer:243281): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0
> 
> though the error message is quite different than the printk above, it
> could still be helpful.

... or, this can be the real problem.  It looks the struct size
doesn't match.  Could you try the attached patch?


> I also don't see any differences between ioctl32.c from CVS and
> 1.0.8rc2. CVS version is at rev.1.26 and tagged as v1-0-8rc1.

Yes.  No change since then.

> > > > ALSA ioctl32 in 2.6.10 or older is buggy.  Try to avoid it.
> > > 
> > > Avoid using ioctl32? I don't have much of a choice since it doesn't work
> > > anyway. It would be nice to get some sound on flash pages and wine,
> > > though.
> > 
> > Yes, but the code in 2.6.10 may crash your system.
> > (On x86-64, this might work, though...)
> > 
> > > 2.6.10 or older pretty much covers every kernel out there. What kernel
> > > version is it OK to test this with?
> > 
> > The one in latest alsa-driver (alsa-kernel) tree.
> > I think Linus's bk and mm trees have included already fixes.
> 
> The most important point that I did not get -- your advice about
> avoiding ioctl32 is a different point of concern, that is completely
> unrelated to the current problem, is it?

I wrote to avoid the one on 2.6.10 (or older) since it's broken.
I did NOT mean to avoid the one on the latest ALSA tree.  This should
get fixed ASAP, of course.


Takashi

[-- Attachment #2: Type: text/plain, Size: 586 bytes --]

Index: alsa-kernel/core/ioctl32/ioctl32.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/core/ioctl32/ioctl32.c,v
retrieving revision 1.30
diff -u -r1.30 ioctl32.c
--- alsa-kernel/core/ioctl32/ioctl32.c	15 Dec 2004 15:26:24 -0000	1.30
+++ alsa-kernel/core/ioctl32/ioctl32.c	10 Jan 2005 13:36:49 -0000
@@ -238,7 +238,7 @@
 		struct sndrv_aes_iec958 iec958;
         } value;
         unsigned char reserved[128];
-};
+}_ _attribute__((packed));
 
 
 /* hmm, it's so hard to retrieve the value type from the control id.. */

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Georgi Georgiev]

maillog: 10/01/2005-14:39:35(+0100): Takashi Iwai types
> At Mon, 10 Jan 2005 21:33:42 +0900, Georgi Georgiev wrote:
> > Jan 10 21:08:24 lion ioctl32(amixer:243138): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc610) on /dev/snd/controlC0
> > Jan 10 21:08:27 lion ioctl32(amixer:243139): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc600) on /dev/snd/controlC0
> > Jan 10 21:21:06 lion ioctl32(amixer:243280): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0
> > Jan 10 21:21:09 lion ioctl32(amixer:243281): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0
> > 
> > though the error message is quite different than the printk above, it
> > could still be helpful.
> 
> ... or, this can be the real problem.  It looks the struct size
> doesn't match.  Could you try the attached patch?

Except for the following small typo:

-}_ _attribute((packed));
+} __attribute((packed));

your patch worked just fine. The 32-bit amixer produces meaningful
output and RealPlayer is able to play sound on my system now.

Thanks a lot.

> Index: alsa-kernel/core/ioctl32/ioctl32.c
> ===================================================================
> RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/core/ioctl32/ioctl32.c,v
> retrieving revision 1.30
> diff -u -r1.30 ioctl32.c
> --- alsa-kernel/core/ioctl32/ioctl32.c	15 Dec 2004 15:26:24 -0000	1.30
> +++ alsa-kernel/core/ioctl32/ioctl32.c	10 Jan 2005 13:36:49 -0000
> @@ -238,7 +238,7 @@
>  		struct sndrv_aes_iec958 iec958;
>          } value;
>          unsigned char reserved[128];
> -};
> +}_ _attribute__((packed));
>  
>  
>  /* hmm, it's so hard to retrieve the value type from the control id.. */

-- 
-*   Georgi Georgiev   -* BOFH Excuse #318: Your EMAIL is now being    -*
*-    chutz@gg3.net    *- delivered by the USPS.                       *-
-*  +81(90)6266-1163   -*                                              -*


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Takashi Iwai]

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

At Tue, 11 Jan 2005 01:15:59 +0900,
Georgi Georgiev wrote:
> 
> maillog: 10/01/2005-14:39:35(+0100): Takashi Iwai types
> > At Mon, 10 Jan 2005 21:33:42 +0900, Georgi Georgiev wrote:
> > > Jan 10 21:08:24 lion ioctl32(amixer:243138): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc610) on /dev/snd/controlC0
> > > Jan 10 21:08:27 lion ioctl32(amixer:243139): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc600) on /dev/snd/controlC0
> > > Jan 10 21:21:06 lion ioctl32(amixer:243280): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0
> > > Jan 10 21:21:09 lion ioctl32(amixer:243281): Unknown cmd fd(3) cmd(c2c45512){02} arg(ffffc670) on /dev/snd/controlC0
> > > 
> > > though the error message is quite different than the printk above, it
> > > could still be helpful.
> > 
> > ... or, this can be the real problem.  It looks the struct size
> > doesn't match.  Could you try the attached patch?
> 
> Except for the following small typo:
> 
> -}_ _attribute((packed));
> +} __attribute((packed));
> 
> your patch worked just fine. The 32-bit amixer produces meaningful
> output and RealPlayer is able to play sound on my system now.

Thanks for the confirmation.

IIRC, the packed attribute was removed because it had a problem on
SPARC.

Anyway, the patch below simplifies the compat struct definition so
that it will have no alignment problem.  Could you give a try?

If this one works, I'll prefer to commit it than the previous hack.


Takashi

[-- Attachment #2: Type: text/plain, Size: 4330 bytes --]

Index: alsa-kernel/core/ioctl32/ioctl32.c
===================================================================
RCS file: /home/tiwai/cvs/alsa/alsa-kernel/core/ioctl32/ioctl32.c,v
retrieving revision 1.30
diff -u -r1.30 ioctl32.c
--- alsa-kernel/core/ioctl32/ioctl32.c	15 Dec 2004 15:26:24 -0000	1.30
+++ alsa-kernel/core/ioctl32/ioctl32.c	10 Jan 2005 16:59:33 -0000
@@ -219,25 +219,10 @@
 	struct sndrv_ctl_elem_id id;
 	unsigned int indirect;	/* bit-field causes misalignment */
         union {
-		union {
-			s32 value[128];
-			u32 value_ptr;
-		} integer;
-		union {
-			s64 value[64];
-			u32 value_ptr;
-		} integer64;
-		union {
-			u32 item[128];
-			u32 item_ptr;
-		} enumerated;
-		union {
-			unsigned char data[512];
-			u32 data_ptr;
-		} bytes;
-		struct sndrv_aes_iec958 iec958;
+		s32 integer[128];	/* integer and boolean need conversion */
+		unsigned char data[512];	/* others should be compatible */
         } value;
-        unsigned char reserved[128];
+        unsigned char reserved[128];	/* not used */
 };
 
 
@@ -269,7 +254,7 @@
 	struct sndrv_ctl_elem_value *data;
 	struct sndrv_ctl_elem_value32 __user *data32;
 	snd_ctl_file_t *ctl;
-	int err, i;
+	int err, i, indirect;
 	int type;
 
 	/* sanity check */
@@ -281,7 +266,7 @@
 		return -ENOTTY;
 
 	data32 = compat_ptr(arg);
-	data = kmalloc(sizeof(*data), GFP_KERNEL);
+	data = kcalloc(1, sizeof(*data), GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 
@@ -289,12 +274,12 @@
 		err = -EFAULT;
 		goto __end;
 	}
-	if (__get_user(data->indirect, &data32->indirect)) {
+	if (__get_user(indirect, &data32->indirect)) {
 		err = -EFAULT;
 		goto __end;
 	}
 	/* FIXME: indirect access is not supported */
-	if (data->indirect) {
+	if (indirect) {
 		err = -EINVAL;
 		goto __end;
 	}
@@ -309,7 +294,7 @@
 	case SNDRV_CTL_ELEM_TYPE_INTEGER:
 		for (i = 0; i < 128; i++) {
 			int val;
-			if (__get_user(val, &data32->value.integer.value[i])) {
+			if (__get_user(val, &data32->value.integer[i])) {
 				err = -EFAULT;
 				goto __end;
 			}
@@ -317,33 +302,12 @@
 		}
 		break;
 	case SNDRV_CTL_ELEM_TYPE_INTEGER64:
-		if (__copy_from_user(data->value.integer64.value,
-				     data32->value.integer64.value,
-				     sizeof(data->value.integer64.value))) {
-			err = -EFAULT;
-			goto __end;
-		}
-		break;
 	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
-		if (__copy_from_user(data->value.enumerated.item,
-				     data32->value.enumerated.item,
-				     sizeof(data32->value.enumerated.item))) {
-			err = -EFAULT;
-			goto __end;
-		}
-		break;
 	case SNDRV_CTL_ELEM_TYPE_BYTES:
-		if (__copy_from_user(data->value.bytes.data,
-				     data32->value.bytes.data,
-				     sizeof(data32->value.bytes.data))) {
-			err = -EFAULT;
-			goto __end;
-		}
-		break;
 	case SNDRV_CTL_ELEM_TYPE_IEC958:
-		if (__copy_from_user(&data->value.iec958,
-				     &data32->value.iec958,
-				     sizeof(data32->value.iec958))) {
+		if (__copy_from_user(data->value.data,
+				     data32->value.data,
+				     sizeof(data32->value.data))) {
 			err = -EFAULT;
 			goto __end;
 		}
@@ -367,43 +331,20 @@
 		for (i = 0; i < 128; i++) {
 			int val;
 			val = data->value.integer.value[i];
-			if (__put_user(val, &data32->value.integer.value[i])) {
+			if (__put_user(val, &data32->value.integer[i])) {
 				err = -EFAULT;
 				goto __end;
 			}
 		}
 		break;
-	case SNDRV_CTL_ELEM_TYPE_INTEGER64:
-		if (__copy_to_user(data32->value.integer64.value,
-				   data->value.integer64.value,
-				   sizeof(data32->value.integer64.value))) {
-			err = -EFAULT;
-			goto __end;
-		}
-		break;
-	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
-		if (__copy_to_user(data32->value.enumerated.item,
-				   data->value.enumerated.item,
-				   sizeof(data32->value.enumerated.item))) {
-			err = -EFAULT;
-			goto __end;
-		}
-		break;
-	case SNDRV_CTL_ELEM_TYPE_BYTES:
-		if (__copy_to_user(data32->value.bytes.data,
-				   data->value.bytes.data,
-				   sizeof(data32->value.bytes.data))) {
+	default:
+		if (__copy_to_user(data32->value.data,
+				   data->value.data,
+				   sizeof(data32->value.data))) {
 			err = -EFAULT;
 			goto __end;
 		}
 		break;
-	case SNDRV_CTL_ELEM_TYPE_IEC958:
-		if (__copy_to_user(&data32->value.iec958,
-				   &data->value.iec958,
-				   sizeof(data32->value.iec958))) {
-			err = -EFAULT;
-			goto __end;
-		}
 		break;
 	}
 	err = 0;

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Georgi Georgiev]

maillog: 11/01/2005-14:11:38(+0100): Takashi Iwai types
> 
> IIRC, the packed attribute was removed because it had a problem on
> SPARC.
> 
> Anyway, the patch below simplifies the compat struct definition so
> that it will have no alignment problem.  Could you give a try?
> 
> If this one works, I'll prefer to commit it than the previous hack.

It almost worked. There were two compilation errors on lines 308 and 342
claiming that "union has no member `data'". I reversed your changes to
the non-32-bit struct references (see below for details) and it worked.

> Index: alsa-kernel/core/ioctl32/ioctl32.c
> ===================================================================
> RCS file: /home/tiwai/cvs/alsa/alsa-kernel/core/ioctl32/ioctl32.c,v
> retrieving revision 1.30
> diff -u -r1.30 ioctl32.c
> --- alsa-kernel/core/ioctl32/ioctl32.c	15 Dec 2004 15:26:24 -0000	1.30
> +++ alsa-kernel/core/ioctl32/ioctl32.c	10 Jan 2005 16:59:33 -0000
> @@ -219,25 +219,10 @@
>  	struct sndrv_ctl_elem_id id;
>  	unsigned int indirect;	/* bit-field causes misalignment */
>          union {
> -		union {
> -			s32 value[128];
> -			u32 value_ptr;
> -		} integer;
> -		union {
> -			s64 value[64];
> -			u32 value_ptr;
> -		} integer64;
> -		union {
> -			u32 item[128];
> -			u32 item_ptr;
> -		} enumerated;
> -		union {
> -			unsigned char data[512];
> -			u32 data_ptr;
> -		} bytes;
> -		struct sndrv_aes_iec958 iec958;
> +		s32 integer[128];	/* integer and boolean need conversion */
> +		unsigned char data[512];	/* others should be compatible */
>          } value;
> -        unsigned char reserved[128];
> +        unsigned char reserved[128];	/* not used */
>  };
>  
>  
> @@ -269,7 +254,7 @@
>  	struct sndrv_ctl_elem_value *data;
>  	struct sndrv_ctl_elem_value32 __user *data32;
>  	snd_ctl_file_t *ctl;
> -	int err, i;
> +	int err, i, indirect;
>  	int type;
>  
>  	/* sanity check */
> @@ -281,7 +266,7 @@
>  		return -ENOTTY;
>  
>  	data32 = compat_ptr(arg);
> -	data = kmalloc(sizeof(*data), GFP_KERNEL);
> +	data = kcalloc(1, sizeof(*data), GFP_KERNEL);
>  	if (data == NULL)
>  		return -ENOMEM;
>  
> @@ -289,12 +274,12 @@
>  		err = -EFAULT;
>  		goto __end;
>  	}
> -	if (__get_user(data->indirect, &data32->indirect)) {
> +	if (__get_user(indirect, &data32->indirect)) {
>  		err = -EFAULT;
>  		goto __end;
>  	}
>  	/* FIXME: indirect access is not supported */
> -	if (data->indirect) {
> +	if (indirect) {
>  		err = -EINVAL;
>  		goto __end;
>  	}
> @@ -309,7 +294,7 @@
>  	case SNDRV_CTL_ELEM_TYPE_INTEGER:
>  		for (i = 0; i < 128; i++) {
>  			int val;
> -			if (__get_user(val, &data32->value.integer.value[i])) {
> +			if (__get_user(val, &data32->value.integer[i])) {
>  				err = -EFAULT;
>  				goto __end;
>  			}
> @@ -317,33 +302,12 @@
>  		}
>  		break;
>  	case SNDRV_CTL_ELEM_TYPE_INTEGER64:
> -		if (__copy_from_user(data->value.integer64.value,
> -				     data32->value.integer64.value,
> -				     sizeof(data->value.integer64.value))) {
> -			err = -EFAULT;
> -			goto __end;
> -		}
> -		break;
>  	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
> -		if (__copy_from_user(data->value.enumerated.item,
> -				     data32->value.enumerated.item,
> -				     sizeof(data32->value.enumerated.item))) {
> -			err = -EFAULT;
> -			goto __end;
> -		}
> -		break;
>  	case SNDRV_CTL_ELEM_TYPE_BYTES:
> -		if (__copy_from_user(data->value.bytes.data,
> -				     data32->value.bytes.data,
> -				     sizeof(data32->value.bytes.data))) {
> -			err = -EFAULT;
> -			goto __end;
> -		}
> -		break;
>  	case SNDRV_CTL_ELEM_TYPE_IEC958:
> -		if (__copy_from_user(&data->value.iec958,
> -				     &data32->value.iec958,
> -				     sizeof(data32->value.iec958))) {
> +		if (__copy_from_user(data->value.data,
                                     ^^^^^^^^^^^^^^^^^
   reversing this to &data->value.iec958
> +				     data32->value.data,
> +				     sizeof(data32->value.data))) {
>  			err = -EFAULT;
>  			goto __end;
>  		}
> @@ -367,43 +331,20 @@
>  		for (i = 0; i < 128; i++) {
>  			int val;
>  			val = data->value.integer.value[i];
> -			if (__put_user(val, &data32->value.integer.value[i])) {
> +			if (__put_user(val, &data32->value.integer[i])) {
>  				err = -EFAULT;
>  				goto __end;
>  			}
>  		}
>  		break;
> -	case SNDRV_CTL_ELEM_TYPE_INTEGER64:
> -		if (__copy_to_user(data32->value.integer64.value,
> -				   data->value.integer64.value,
> -				   sizeof(data32->value.integer64.value))) {
> -			err = -EFAULT;
> -			goto __end;
> -		}
> -		break;
> -	case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
> -		if (__copy_to_user(data32->value.enumerated.item,
> -				   data->value.enumerated.item,
> -				   sizeof(data32->value.enumerated.item))) {
> -			err = -EFAULT;
> -			goto __end;
> -		}
> -		break;
> -	case SNDRV_CTL_ELEM_TYPE_BYTES:
> -		if (__copy_to_user(data32->value.bytes.data,
> -				   data->value.bytes.data,
> -				   sizeof(data32->value.bytes.data))) {
> +	default:
> +		if (__copy_to_user(data32->value.data,
> +				   data->value.data,
 and this to data->value.bytes.data^^^^^^^^^^^^^^^^
> +				   sizeof(data32->value.data))) {
>  			err = -EFAULT;
>  			goto __end;
>  		}
>  		break;
> -	case SNDRV_CTL_ELEM_TYPE_IEC958:
> -		if (__copy_to_user(&data32->value.iec958,
> -				   &data->value.iec958,
> -				   sizeof(data32->value.iec958))) {
> -			err = -EFAULT;
> -			goto __end;
> -		}
>  		break;
>  	}
>  	err = 0;

made the patch compile fine. I checked with realplayer and 32-bit
alsamixer, and they both appeared to work without any problems.

-- 
*)   Georgi Georgiev   *) You can drive a horse to water, but a        *)
(*    chutz@gg3.net    (* pencil must be lead.                         (*
*)  +81(90)6266-1163   *)                                              *)


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Takashi Iwai]

At Wed, 12 Jan 2005 11:24:02 +0900,
Georgi Georgiev wrote:
> 
> maillog: 11/01/2005-14:11:38(+0100): Takashi Iwai types
> > 
> > IIRC, the packed attribute was removed because it had a problem on
> > SPARC.
> > 
> > Anyway, the patch below simplifies the compat struct definition so
> > that it will have no alignment problem.  Could you give a try?
> > 
> > If this one works, I'll prefer to commit it than the previous hack.
> 
> It almost worked. There were two compilation errors on lines 308 and 342
> claiming that "union has no member `data'". I reversed your changes to
> the non-32-bit struct references (see below for details) and it worked.

OK, now the code is on CVS (will be 1.0.8-final).


thanks,

Takashi


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Juergen Kreileder]

Georgi Georgiev <chutz@gg3.net> writes:

>> Index: alsa-kernel/core/ioctl32/ioctl32.c
>> ===================================================================
>> RCS file: /home/tiwai/cvs/alsa/alsa-kernel/core/ioctl32/ioctl32.c,v
>> retrieving revision 1.30
>> diff -u -r1.30 ioctl32.c
>> --- alsa-kernel/core/ioctl32/ioctl32.c	15 Dec 2004 15:26:24 -0000	1.30
>> +++ alsa-kernel/core/ioctl32/ioctl32.c	10 Jan 2005 16:59:33 -0000
>> @@ -219,25 +219,10 @@
>> 	struct sndrv_ctl_elem_id id;
>> 	unsigned int indirect;	/* bit-field causes misalignment */
>> union {
>> -		union {
>> -			s32 value[128];
>> -			u32 value_ptr;
>> -		} integer;
>> -		union {
>> -			s64 value[64];
>> -			u32 value_ptr;
>> -		} integer64;
>> -		union {
>> -			u32 item[128];
>> -			u32 item_ptr;
>> -		} enumerated;
>> -		union {
>> -			unsigned char data[512];
>> -			u32 data_ptr;
>> -		} bytes;
>> -		struct sndrv_aes_iec958 iec958;
>> +		s32 integer[128];	/* integer and boolean need conversion */
>> +		unsigned char data[512];	/* others should be compatible */
>> } value;
>> -        unsigned char reserved[128];
>> +        unsigned char reserved[128];	/* not used */
>> };

That breaks alignment on ppc64 again.  Fix:

--- ioctl32.c.orig	2005-01-14 06:53:41.496899917 +0100
+++ ioctl32.c	2005-01-14 06:56:26.146675819 +0100
@@ -220,6 +220,7 @@ struct sndrv_ctl_elem_value32 {
 	unsigned int indirect;	/* bit-field causes misalignment */
         union {
 		s32 integer[128];	/* integer and boolean need conversion */
+		s64 longint[64];
 		unsigned char data[512];	/* others should be compatible */
         } value;
         unsigned char reserved[128];	/* not used */
=


        Juergen

-- 
Juergen Kreileder, Blackdown Java-Linux Team
http://www.blackdown.org/java-linux/java2-status/


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Takashi Iwai]

At Fri, 14 Jan 2005 07:13:39 +0100,
Juergen Kreileder wrote:
> 
> Georgi Georgiev <chutz@gg3.net> writes:
> 
> >> Index: alsa-kernel/core/ioctl32/ioctl32.c
> >> ===================================================================
> >> RCS file: /home/tiwai/cvs/alsa/alsa-kernel/core/ioctl32/ioctl32.c,v
> >> retrieving revision 1.30
> >> diff -u -r1.30 ioctl32.c
> >> --- alsa-kernel/core/ioctl32/ioctl32.c	15 Dec 2004 15:26:24 -0000	1.30
> >> +++ alsa-kernel/core/ioctl32/ioctl32.c	10 Jan 2005 16:59:33 -0000
> >> @@ -219,25 +219,10 @@
> >> 	struct sndrv_ctl_elem_id id;
> >> 	unsigned int indirect;	/* bit-field causes misalignment */
> >> union {
> >> -		union {
> >> -			s32 value[128];
> >> -			u32 value_ptr;
> >> -		} integer;
> >> -		union {
> >> -			s64 value[64];
> >> -			u32 value_ptr;
> >> -		} integer64;
> >> -		union {
> >> -			u32 item[128];
> >> -			u32 item_ptr;
> >> -		} enumerated;
> >> -		union {
> >> -			unsigned char data[512];
> >> -			u32 data_ptr;
> >> -		} bytes;
> >> -		struct sndrv_aes_iec958 iec958;
> >> +		s32 integer[128];	/* integer and boolean need conversion */
> >> +		unsigned char data[512];	/* others should be compatible */
> >> } value;
> >> -        unsigned char reserved[128];
> >> +        unsigned char reserved[128];	/* not used */
> >> };
> 
> That breaks alignment on ppc64 again.  Fix:

Hmm.  The change was to fix alignment on x86-64 on the contrary.
When we put s64 there, it breaks.
OK, I'll add it with ifdef CONFIG_PPC64.


Takashi

> 
> --- ioctl32.c.orig	2005-01-14 06:53:41.496899917 +0100
> +++ ioctl32.c	2005-01-14 06:56:26.146675819 +0100
> @@ -220,6 +220,7 @@ struct sndrv_ctl_elem_value32 {
>  	unsigned int indirect;	/* bit-field causes misalignment */
>          union {
>  		s32 integer[128];	/* integer and boolean need conversion */
> +		s64 longint[64];
>  		unsigned char data[512];	/* others should be compatible */
>          } value;
>          unsigned char reserved[128];	/* not used */
> =
> 
> 
>         Juergen
> 
> -- 
> Juergen Kreileder, Blackdown Java-Linux Team
> http://www.blackdown.org/java-linux/java2-status/
> 


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Juergen Kreileder]

Takashi Iwai <tiwai@suse.de> writes:

> Hmm.  The change was to fix alignment on x86-64 on the contrary.
> When we put s64 there, it breaks.

Ah, I see.

> > OK, I'll add it with ifdef CONFIG_PPC64.

You might want to check how other architectures align long longs in
32-bit mode.  At least SPARC uses mod 8 too.
How about #ifndef CONFIG_X86_64?


        Juergen

-- 
Juergen Kreileder, Blackdown Java-Linux Team
http://www.blackdown.org/java-linux/java2-status/


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: snd-ioctl32 is failing on x86_64 (no crashes, just errors)

[Takashi Iwai]

At Fri, 14 Jan 2005 11:59:39 +0100,
Juergen Kreileder wrote:
> 
> Takashi Iwai <tiwai@suse.de> writes:
> 
> > Hmm.  The change was to fix alignment on x86-64 on the contrary.
> > When we put s64 there, it breaks.
> 
> Ah, I see.
> 
> > > OK, I'll add it with ifdef CONFIG_PPC64.
> 
> You might want to check how other architectures align long longs in
> 32-bit mode.  At least SPARC uses mod 8 too.
> How about #ifndef CONFIG_X86_64?

Sounds better.  I'll change it again on CVS.


thanks,

Takashi


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt