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

[Avi Kivity <avi@redhat.com>]

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