From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?ISO-8859-1?Q?Fernando_Luis_V=E1zquez_Cao?=
	<fernando@oss.ntt.co.jp>
Subject: Re: [PATCH RFC 1/5] KVM: introduce a set_bit function for bitmaps
 in user space
Date: Wed, 21 Apr 2010 13:56:39 +0900
Message-ID: <4BCE8587.7080607@oss.ntt.co.jp>
References: <20100409182732.857de4db.yoshikawa.takuya@oss.ntt.co.jp> <20100409183021.843ca432.yoshikawa.takuya@oss.ntt.co.jp> <4BC2020B.5030402@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>,
	mtosatti@redhat.com, kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from serv2.oss.ntt.co.jp ([222.151.198.100]:57182 "EHLO
	serv2.oss.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751185Ab0DUE4l (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 21 Apr 2010 00:56:41 -0400
In-Reply-To: <4BC2020B.5030402@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.