From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Fernando_Luis_V=E1zquez_Cao?= Subject: Re: [PATCH RFC v2 6/6] KVM: introduce a new API for getting dirty bitmaps Date: Thu, 22 Apr 2010 15:09:09 +0900 Message-ID: <4BCFE805.5040500@oss.ntt.co.jp> References: <20100420195349.dab60b1d.yoshikawa.takuya@oss.ntt.co.jp> <20100420200353.2d2a6dec.yoshikawa.takuya@oss.ntt.co.jp> <4BCEB777.6040505@oss.ntt.co.jp> <264937F8-98E4-4A73-9C7A-837793E1DAC8@suse.de> <4BCFB862.7010509@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Takuya Yoshikawa , avi-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Alexander Graf Return-path: In-Reply-To: <4BCFB862.7010509-gVGce1chcLdL9jVzuh4AOg@public.gmane.org> Sender: kvm-ppc-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: kvm.vger.kernel.org On 04/22/2010 11:45 AM, Fernando Luis V=E1zquez Cao wrote: > On 04/21/2010 06:41 PM, Alexander Graf wrote: >> On 21.04.2010, at 10:29, Fernando Luis V=E1zquez Cao wrote: >> >>> On 04/20/2010 08:03 PM, Takuya Yoshikawa wrote: >>>> @@ -318,7 +318,7 @@ struct kvm_dirty_log { >>>> __u32 padding1; >>>> union { >>>> void __user *dirty_bitmap; /* one bit per page */ >>>> - __u64 padding2; >>>> + __u64 addr; >>> >>> This can break on x86_32 and x86_64-compat. addr is a long not a __= u64. >> >> So the high 32 bits are zero. Where's the problem? >=20 > If we are careful enough to cast the addr appropriately we should be = fine, > even if we keep the padding field in the union. I am not saying that = it > breaks 32 architectures but that it can potentially be problematic. >=20 >>>> + case KVM_SWITCH_DIRTY_LOG: { >>>> + struct kvm_dirty_log log; >>>> + >>>> + r =3D -EFAULT; >>>> + if (copy_from_user(&log, argp, sizeof log)) >>>> + goto out; >>>> + r =3D kvm_vm_ioctl_switch_dirty_log(kvm, &log); >>>> + if (r) >>>> + goto out; >>>> + r =3D -EFAULT; >>>> + if (copy_to_user(argp, &log, sizeof log)) >>>> + goto out; >>>> + r =3D 0; >>>> + break; >>>> + } >>> >>> In x86_64-compat mode we are handling 32bit user-space addresses >>> so we need the compat counterpart of KVM_SWITCH_DIRTY_LOG too. >> >> The compat code just forwards everything to the generic ioctls. >=20 > The compat code uses struct compat_kvm_dirty_log instead of > struct kvm_dirty_log to communicate with user space so > the necessary conversions needs to be done before invoking > the generic ioctl (see KVM_GET_DIRTY_LOG in kvm_vm_compat_ioctl). >=20 > By the way we probable should move the definition of struct > compat_kvm_dirty_log to a header file. It seems that it was you and Arnd who added the kvm_vm compat ioctl :-)= =2E Are you considering a different approach to tackle the issues that we have with a big-endian userspace? Thanks, =46ernando