On Wednesday 20 April 2011 21:46:04 MichaÅ MirosÅaw wrote: > 2011/4/20 Arnd Bergmann <arnd@xxxxxxxx>: > > No, please don't try to invent random new ways of doing this. > > Your example relies on the assumption that the task is calling > > the entry point for its native word size. Some architectures > > intentionally allow calling the 32 bit entry point from 64 bit > > tasks and vice versa, e.g. for user space emulators converting > > to a different ABI, and in that case is_compat_task() produces > > the wrong result. Don't ever rely on that. > > This doesn't make sense to me. If you call 32-bit entry point from > 64-bit process, you can't reliably pass pointers through the call > (unless you limit 64-bit process to 32-bit address space). > > Do you know a working example of something using this kind of cross-call? There are people that use 32 bit programs on x86_64 in 64 bit mode and switch on the ADDR_LIMIT_32BIT personality, IIRC. This gives you more registers and lets you do 64 bit arithmetic while not using any more memory to store long pointers. There are a few problems with this, and the new x32 ABI will make it cleaner. I believe qemu also does this to run foreign user binaries. You can use qemu-user to emulate user space with a different instruction set, but when you call into the kernel, you have to use the native data structures that the host understands. > >> I'm okay with the anon union + ``compat_ptr(*(u32 *))`` part of your > >> solution. If everyone else thinks it is reasonable, I'll submit a v7 > >> with it. > > No need for a union or a ptr_size member in the struct. Just use > > a single __u64 and let the user cast the pointer to that. This > > will work on all architectures. > > Union is just hiding this cast (it will be done in kernel) and allows > cleaner code for userspace (there's a single kernel and possibly > multiple applications that will implement this call). As I explained, it doesn't work. Please read my earlier mails. > >> However, I still think it should be implemented in compat_ioctl() > >> because compat_blkdev_ioctl() expects it. Either that, or I add to the > >> big switch in compat_blkdev_driver_ioctl(), and spreading this change > >> out to block/compat_ioctl.c does not seem like The Right Thing to me. > > Yes, you definitely need to fill the .compat_ioctl member. We don't want > > new entries in the switch statement, in particular none that are specific > > to a single driver. > > Hmm, you're right. fs/compat_ioctl.c falls back to plain .ioctl if > .compat_ioctl == NULL. No, it doesn't. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html