Re: [PATCH RFC 1/5] KVM: introduce a set_bit function for bitmaps in user space

Fernando Luis Vázquez Cao
2010-04-21T04:56:41Z

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.