On 04/12/2010 02:08 AM, Avi Kivity wrote: >> +#define __set_bit_user_asm(nr, addr, err, errret) \ >> + asm volatile("1: bts %1,%2\n" \ >> + "2:\n" \ >> + ".section .fixup,\"ax\"\n" \ >> + "3: mov %3,%0\n" \ >> + " jmp 2b\n" \ >> + ".previous\n" \ >> + _ASM_EXTABLE(1b, 3b) \ >> + : "=r"(err) \ >> + : "r" (nr), "m" (__m(addr)), "i" (errret), "0" (err)) >> + >> +#define set_bit_user(nr, addr) \ >> +({ \ >> + int __ret_sbu = 0; \ >> + \ >> + might_fault(); \ >> + if (access_ok(VERIFY_WRITE, addr, nr/8 + 1)) \ >> + __set_bit_user_asm(nr, addr, __ret_sbu, -EFAULT); \ >> + else \ >> + __ret_sbu = -EFAULT; \ >> + \ >> + __ret_sbu; \ >> +}) >> + >> > > Should be called __set_bit_user() since it is non-atomic. > > An interesting wart is that this will use the kernel's word size instead > of userspace word size for access. So, a 32-bit process might allocate > a 4-byte bitmap, and a 64-bit kernel will use a 64-bit access to touch > it, which might result in a fault. This might be resolved by > documenting that userspace bitmaps must be a multiple of 64-bits in size > and recommending that they be 64-bit aligned as well. Yes, the inline assembler above generates a REX prefixed bts with the W field set (48 0f ab), which means we have a 64 bit operand size. In addition to the solution you propose we could also implement a legacy mode version that uses a 32bit bts. Compat ioctls and that ilk could pontentially benefit from this. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html