From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH RFC 1/5] KVM: introduce a set_bit function for bitmaps in user space Date: Wed, 21 Apr 2010 11:09:57 +0300 Message-ID: <4BCEB2D5.6090201@redhat.com> References: <20100409182732.857de4db.yoshikawa.takuya@oss.ntt.co.jp> <20100409183021.843ca432.yoshikawa.takuya@oss.ntt.co.jp> <4BC2020B.5030402@redhat.com> <4BCE8587.7080607@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Takuya Yoshikawa , mtosatti@redhat.com, kvm@vger.kernel.org To: =?ISO-8859-1?Q?Fernando_Luis_V=E1zquez_Cao?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:52749 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751354Ab0DUIKB (ORCPT ); Wed, 21 Apr 2010 04:10:01 -0400 In-Reply-To: <4BCE8587.7080607@oss.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: On 04/21/2010 07:56 AM, Fernando Luis V=E1zquez Cao wrote: > On 04/12/2010 02:08 AM, Avi Kivity wrote: > =20 >>> +#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) \ >>> + : "=3Dr"(err) \ >>> + : "r" (nr), "m" (__m(addr)), "i" (errret), "0" (err)) >>> + >>> +#define set_bit_user(nr, addr) \ >>> +({ \ >>> + int __ret_sbu =3D 0; \ >>> + \ >>> + might_fault(); \ >>> + if (access_ok(VERIFY_WRITE, addr, nr/8 + 1)) \ >>> + __set_bit_user_asm(nr, addr, __ret_sbu, -EFAULT); \ >>> + else \ >>> + __ret_sbu =3D -EFAULT; \ >>> + \ >>> + __ret_sbu; \ >>> +}) >>> + >>> >>> =20 >> Should be called __set_bit_user() since it is non-atomic. >> >> An interesting wart is that this will use the kernel's word size ins= tead >> of userspace word size for access. So, a 32-bit process might alloc= ate >> a 4-byte bitmap, and a 64-bit kernel will use a 64-bit access to tou= ch >> 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. >> =20 > 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 additio= n to the > solution you propose we could also implement a legacy mode version th= at uses > a 32bit bts. Compat ioctls and that ilk could pontentially benefit fr= om this. > =20 We generally try to avoid compat ioctls; declaring that the bitmap is=20 little endian and uses 64-bit words works around all of the difficultie= s. --=20 error compiling committee.c: too many arguments to function