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

Avi Kivity
2010-04-21T08:10:01Z

On 04/21/2010 07:56 AM, Fernando Luis Vázquez Cao wrote:
> 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.
>

We generally try to avoid compat ioctls; declaring that the bitmap is
little endian and uses 64-bit words works around all of the difficulties.

--
error compiling committee.c: too many arguments to function