From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52646) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TxHCe-0003hz-Cm for qemu-devel@nongnu.org; Mon, 21 Jan 2013 08:14:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TxHCc-0001Bj-15 for qemu-devel@nongnu.org; Mon, 21 Jan 2013 08:14:20 -0500 Received: from cantor2.suse.de ([195.135.220.15]:54050 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TxHCb-0001Bf-O5 for qemu-devel@nongnu.org; Mon, 21 Jan 2013 08:14:17 -0500 Message-ID: <50FD3F25.1060504@suse.de> Date: Mon, 21 Jan 2013 14:14:13 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1358456378-29248-1-git-send-email-ehabkost@redhat.com> <1358456378-29248-5-git-send-email-ehabkost@redhat.com> <50F92DE1.7040107@suse.de> <20130118125340.GV10683@otherpad.lan.raisama.net> <50F9480D.3040400@suse.de> <20130118142013.GX10683@otherpad.lan.raisama.net> <50F9743E.2080601@redhat.com> <20130118164002.GN20133@otherpad.lan.raisama.net> <50F98A87.40307@redhat.com> In-Reply-To: <50F98A87.40307@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-1.4 04/12] kvm: Create kvm_arch_vcpu_id() function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , Eduardo Habkost Cc: Anthony Liguori , Igor Mammedov , "kvm@vger.kernel.org list" , Gleb Natapov , qemu-devel@nongnu.org -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 18.01.2013 18:46, schrieb Eric Blake: > On 01/18/2013 09:40 AM, Eduardo Habkost wrote: >> On Fri, Jan 18, 2013 at 09:11:42AM -0700, Eric Blake wrote: >>> On 01/18/2013 07:20 AM, Eduardo Habkost wrote: >>>>> Could you suggest a text for me to add please? >>>>=20 >>>> "The argument passed to KVM_CREATE_VCPU now has 'unsigned >>>> long' type instead of 'int', as expected by the Linux ioctl() >>>> syscall. Maybe an int works on most or all architectures >>>> supporting KVM, but it is safer to use an appropriate >>>> 'unsigned long' parameter." >>>=20 >>> Interestingly enough, while the Linux syscall uses 'unsigned >>> long', the POSIX definition of ioctl() uses 'int'; so the Linux >>> kernel is already constrained to never use an ioctl value that >>> doesn't fit within 'int', >>=20 >> Really? What about the ioctl()s that get a pointer as argument >> on architectures where pointers don't fit in an int? >>=20 >> Do you have a pointer to the POSIX definition you are talking >> about? >>=20 >> Note that I'm talking about the the extra ioctl() argument, not >> the ioctl() number (that is an unsigned int in the kernel code). >=20 > Okay, now you made me go back and check sources. >=20 > POSIX 2008 says: #include int ioctl(int fildes, int > request, ... /* arg */); >=20 > Gnulib says this about a bug that it works around: @item On glibc > platforms, the second parameter is of type @code{unsigned long}=20 > rather than @code{int}. >=20 > But gnulib also suggests using instead of the POSIX > header for getting ioctl(), because was > declared obsolete in POSIX 2008 and was never implemented in > glibc. >=20 > Sure enough, looking at Fedora 18 /usr/include/sys/ioctl.h, I still > see: extern int ioctl (int __fd, unsigned long int __request, ...) > __THROW; >=20 > Meanwhile, you are correct that the kernel defines request as 32 > bits: linux.git:include/uapi/asm-generic/ioctl.h /* ioctl command > encoding: 32 bits total, command in lower 16 bits, * size of the > parameter structure in the lower 14 bits of the * upper 16 bits. * > Encoding the size of the parameter structure in the ioctl request * > is useful for catching programs compiled with old versions * and to > avoid overwriting user space outside the user buffer area. * The > highest 2 bits are reserved for indicating the ``access mode''. * > NOTE: This limits the max parameter size to 16kB -1 ! */ >=20 >>=20 >>> and glibc is already responsible for ensuring that argument >>> promotion of an int doesn't change the behavior of ioctl() in >>> libc when converting it over to the unsigned long syscall >>> semantics expected by the kernel. >=20 > So a more precise wording of this is: >=20 > glibc is already responsible from converting the 'unsigned long > int' of the user declaration back into the 'unsigned int' that the > kernel expects for the second argument. The third argument (when > present), is generally treated as a pointer (of size appropriate > for the architecture). Although there _might_ be an ioctl that > uses it directly as an integer instead of dereferencing it as a > pointer, those would be the exceptions to the rule. So ... do we have a conclusion what to put into the commit message? :) It looks to me as if kvm-all.c:kvm_vm_ioctl() is using void*. I like unsigned long but maybe uintptr_t would be more correct then? Or should kvm_vm_ioctl() be fixed to use something else instead? Eric's int would be a semantic change for the 64-bit platforms, no? Andreas - --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) iQIcBAEBAgAGBQJQ/T8lAAoJEPou0S0+fgE/gl8QALesZwG5q07W21mp2j4ikL8N jrBHjG2VZ9Kda+AIGMClVsWntGZSOzHdtriJ4gjxp90D71S/LQfsYAy6bj45FIwS kPbIQblLlL5Xc6ZiTS5yTzkwyEd7gUpDVouXyv3XxeyUxqhQKwgWxpiP4RftbBRI Z8wLbVFNpIoIsHfhKoNkT4M/Ucm1iZbChV6y4zqltAfdQhcl6Gq0jzhtkAfmN41t p3tCJYldRwayiKLsO2Y0BMNrKmCJisKCEGmkCQzye/3cuFoat/WUmpjV/65hLNtm ruzfn6pCqMTEGPC4YeDdUsxAhVzVX+Sd4mBKHBGItmvhhJMFYUtwTosRwX5bOrAJ mpVLAj5/XDYTm2/jQUEOJAqpxUr5oAVMQL3sNeWJPmXkk1kNaNWTNVHHDW1iJnRj ty0YIWOnuNabkwiDEjPCz6ghjfA3wOBWy8Gk3+F21MYgRQwDTFw4JZuroOIzy3iD 6Vs4MmiBUGnoLobSqw2dUZFmjL7a1500AxZG0MwBd+EqnbLHGqD33kvLrbUYT8+F eW+cqKV+ZXo3ux343rTxD6EFgmN7GmHSkknxJN5m6ldlw5wfFQ8KhdCiKjwSq3EP X0bVGmryEdIh+6w/RbhL75Vfb/Je0mr/GzhtijtXo+FORkF8ip2mlpVSl46r0AfI KvsZ0HZqZHsfoaSBC1js =3D/cL3 -----END PGP SIGNATURE-----