Re: [PATCH] Enable 32bit dirty log pointers on 64bit host
Alexander Graf wrote: > On 23.10.2009, at 10:41, Jan Kiszka wrote: > >> Alexander Graf wrote: >>> From: Arnd Bergmann >>> >>> With big endian userspace, we can't quite figure out if a pointer >>> is 32 bit (shifted >> 32) or 64 bit when we read a 64 bit pointer. >>> >>> This is what happens with dirty logging. To get the pointer >>> interpreted >>> correctly, we thus need Arnd's patch to implement a compat layer for >>> the ioctl: >>> >>> A better way to do this is to add a separate compat_ioctl() method >>> that >>> converts this for you. >>> >>> From: Arnd Bergmann >>> Signed-off-by: Arnd Bergmann >>> Acked-by: Alexander Graf >>> >>> --- >>> >>> Changes from Arnd's example version: >>> >>> - s/log.log/log/ (Avi) >>> - use sizeof(compat_log) (Avi) >>> - compile fixes >>> --- >>> virt/kvm/kvm_main.c | 49 + >>> +++- >>> 1 files changed, 48 insertions(+), 1 deletions(-) >>> >>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >>> index cac69c4..54a272f 100644 >>> --- a/virt/kvm/kvm_main.c >>> +++ b/virt/kvm/kvm_main.c >>> @@ -43,6 +43,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include >>> #include >>> @@ -1542,6 +1543,52 @@ out: >>> return r; >>> } >>> >>> +#ifdef CONFIG_COMPAT >>> +struct compat_kvm_dirty_log { >>> + __u32 slot; >>> + __u32 padding1; >>> + union { >>> + compat_uptr_t dirty_bitmap; /* one bit per page */ >>> + __u64 padding2; >>> + }; >>> +}; >>> + >>> +static long kvm_vm_compat_ioctl(struct file *filp, >>> + unsigned int ioctl, unsigned long arg) >>> +{ >>> + struct kvm *kvm = filp->private_data; >>> + int r; >>> + >>> + if (kvm->mm != current->mm) >>> + return -EIO; >>> + switch (ioctl) { >>> + case KVM_GET_DIRTY_LOG: { >>> + struct compat_kvm_dirty_log compat_log; >>> + struct kvm_dirty_log log; >>> + >>> + r = -EFAULT; >>> + if (copy_from_user(&compat_log, (void __user *)arg, >>> + sizeof(compat_log))) >>> + goto out; >>> + log.slot = compat_log.slot; >>> + log.padding1 = compat_log.padding1; >>> + log.padding2 = compat_log.padding2; >>> + log.dirty_bitmap = compat_ptr(compat_log.dirty_bitmap); >>> + >>> + r = kvm_vm_ioctl_get_dirty_log(kvm, &log); >>> + if (r) >>> + goto out; >>> + break; >>> + } >>> + default: >>> + r = kvm_vm_ioctl(filp, ioctl, arg); >>> + } >>> + >>> +out: >>> + return r; >>> +} >>> +#endif >>> + >>> static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault >>> *vmf) >>> { >>> struct page *page[1]; >>> @@ -1576,7 +1623,7 @@ static int kvm_vm_mmap(struct file *file, >>> struct vm_area_struct *vma) >>> static struct file_operations kvm_vm_fops = { >>> .release= kvm_vm_release, >>> .unlocked_ioctl = kvm_vm_ioctl, >>> - .compat_ioctl = kvm_vm_ioctl, >>> + .compat_ioctl = kvm_vm_compat_ioctl, >> This fails in the absence of CONFIG_COMPAT. > > > So should it rather be > > #ifdef CONFIG_COMPAT > .compat_ioctl = kvm_vm_compat_ioctl, > #else > .compat_ioctl = kvm_vm_ioctl, > #endif > > or > > #ifdef CONFIG_COMPAT > .compat_ioctl = kvm_vm_compat_ioctl, > #endif > > ? I would say the latter as .compat_ioctl should simply be unused in case of !CONFIG_COMPAT. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Enable 32bit dirty log pointers on 64bit host
On 23.10.2009, at 10:41, Jan Kiszka wrote: Alexander Graf wrote: From: Arnd Bergmann With big endian userspace, we can't quite figure out if a pointer is 32 bit (shifted >> 32) or 64 bit when we read a 64 bit pointer. This is what happens with dirty logging. To get the pointer interpreted correctly, we thus need Arnd's patch to implement a compat layer for the ioctl: A better way to do this is to add a separate compat_ioctl() method that converts this for you. From: Arnd Bergmann Signed-off-by: Arnd Bergmann Acked-by: Alexander Graf --- Changes from Arnd's example version: - s/log.log/log/ (Avi) - use sizeof(compat_log) (Avi) - compile fixes --- virt/kvm/kvm_main.c | 49 + +++- 1 files changed, 48 insertions(+), 1 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index cac69c4..54a272f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -1542,6 +1543,52 @@ out: return r; } +#ifdef CONFIG_COMPAT +struct compat_kvm_dirty_log { + __u32 slot; + __u32 padding1; + union { + compat_uptr_t dirty_bitmap; /* one bit per page */ + __u64 padding2; + }; +}; + +static long kvm_vm_compat_ioctl(struct file *filp, + unsigned int ioctl, unsigned long arg) +{ + struct kvm *kvm = filp->private_data; + int r; + + if (kvm->mm != current->mm) + return -EIO; + switch (ioctl) { + case KVM_GET_DIRTY_LOG: { + struct compat_kvm_dirty_log compat_log; + struct kvm_dirty_log log; + + r = -EFAULT; + if (copy_from_user(&compat_log, (void __user *)arg, + sizeof(compat_log))) + goto out; + log.slot = compat_log.slot; + log.padding1 = compat_log.padding1; + log.padding2 = compat_log.padding2; + log.dirty_bitmap = compat_ptr(compat_log.dirty_bitmap); + + r = kvm_vm_ioctl_get_dirty_log(kvm, &log); + if (r) + goto out; + break; + } + default: + r = kvm_vm_ioctl(filp, ioctl, arg); + } + +out: + return r; +} +#endif + static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) { struct page *page[1]; @@ -1576,7 +1623,7 @@ static int kvm_vm_mmap(struct file *file, struct vm_area_struct *vma) static struct file_operations kvm_vm_fops = { .release= kvm_vm_release, .unlocked_ioctl = kvm_vm_ioctl, - .compat_ioctl = kvm_vm_ioctl, + .compat_ioctl = kvm_vm_compat_ioctl, This fails in the absence of CONFIG_COMPAT. So should it rather be #ifdef CONFIG_COMPAT .compat_ioctl = kvm_vm_compat_ioctl, #else .compat_ioctl = kvm_vm_ioctl, #endif or #ifdef CONFIG_COMPAT .compat_ioctl = kvm_vm_compat_ioctl, #endif ? Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Enable 32bit dirty log pointers on 64bit host
Alexander Graf wrote: > From: Arnd Bergmann > > With big endian userspace, we can't quite figure out if a pointer > is 32 bit (shifted >> 32) or 64 bit when we read a 64 bit pointer. > > This is what happens with dirty logging. To get the pointer interpreted > correctly, we thus need Arnd's patch to implement a compat layer for > the ioctl: > > A better way to do this is to add a separate compat_ioctl() method that > converts this for you. > > From: Arnd Bergmann > Signed-off-by: Arnd Bergmann > Acked-by: Alexander Graf > > --- > > Changes from Arnd's example version: > > - s/log.log/log/ (Avi) > - use sizeof(compat_log) (Avi) > - compile fixes > --- > virt/kvm/kvm_main.c | 49 - > 1 files changed, 48 insertions(+), 1 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index cac69c4..54a272f 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -43,6 +43,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -1542,6 +1543,52 @@ out: > return r; > } > > +#ifdef CONFIG_COMPAT > +struct compat_kvm_dirty_log { > + __u32 slot; > + __u32 padding1; > + union { > + compat_uptr_t dirty_bitmap; /* one bit per page */ > + __u64 padding2; > + }; > +}; > + > +static long kvm_vm_compat_ioctl(struct file *filp, > +unsigned int ioctl, unsigned long arg) > +{ > + struct kvm *kvm = filp->private_data; > + int r; > + > + if (kvm->mm != current->mm) > + return -EIO; > + switch (ioctl) { > + case KVM_GET_DIRTY_LOG: { > + struct compat_kvm_dirty_log compat_log; > + struct kvm_dirty_log log; > + > + r = -EFAULT; > + if (copy_from_user(&compat_log, (void __user *)arg, > +sizeof(compat_log))) > + goto out; > + log.slot = compat_log.slot; > + log.padding1 = compat_log.padding1; > + log.padding2 = compat_log.padding2; > + log.dirty_bitmap = compat_ptr(compat_log.dirty_bitmap); > + > + r = kvm_vm_ioctl_get_dirty_log(kvm, &log); > + if (r) > + goto out; > + break; > + } > + default: > + r = kvm_vm_ioctl(filp, ioctl, arg); > + } > + > +out: > + return r; > +} > +#endif > + > static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) > { > struct page *page[1]; > @@ -1576,7 +1623,7 @@ static int kvm_vm_mmap(struct file *file, struct > vm_area_struct *vma) > static struct file_operations kvm_vm_fops = { > .release= kvm_vm_release, > .unlocked_ioctl = kvm_vm_ioctl, > - .compat_ioctl = kvm_vm_ioctl, > + .compat_ioctl = kvm_vm_compat_ioctl, This fails in the absence of CONFIG_COMPAT. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Enable 32bit dirty log pointers on 64bit host
On Wed, Oct 21, 2009 at 04:08:29PM +0200, Alexander Graf wrote: > From: Arnd Bergmann > > With big endian userspace, we can't quite figure out if a pointer > is 32 bit (shifted >> 32) or 64 bit when we read a 64 bit pointer. > > This is what happens with dirty logging. To get the pointer interpreted > correctly, we thus need Arnd's patch to implement a compat layer for > the ioctl: > > A better way to do this is to add a separate compat_ioctl() method that > converts this for you. > > From: Arnd Bergmann > Signed-off-by: Arnd Bergmann > Acked-by: Alexander Graf > > --- > > Changes from Arnd's example version: > > - s/log.log/log/ (Avi) > - use sizeof(compat_log) (Avi) > - compile fixes Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Enable 32bit dirty log pointers on 64bit host
From: Arnd Bergmann With big endian userspace, we can't quite figure out if a pointer is 32 bit (shifted >> 32) or 64 bit when we read a 64 bit pointer. This is what happens with dirty logging. To get the pointer interpreted correctly, we thus need Arnd's patch to implement a compat layer for the ioctl: A better way to do this is to add a separate compat_ioctl() method that converts this for you. Based on initial patch from Arnd Bergmann. From: Arnd Bergmann Signed-off-by: Arnd Bergmann Signed-off-by: Alexander Graf --- Changes from Arnd's example version: - s/log.log/log/ (Avi) - use sizeof(compat_log) (Avi) - compile fixes --- virt/kvm/kvm_main.c | 49 - 1 files changed, 48 insertions(+), 1 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index cac69c4..54a272f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -1542,6 +1543,52 @@ out: return r; } +#ifdef CONFIG_COMPAT +struct compat_kvm_dirty_log { + __u32 slot; + __u32 padding1; + union { + compat_uptr_t dirty_bitmap; /* one bit per page */ + __u64 padding2; + }; +}; + +static long kvm_vm_compat_ioctl(struct file *filp, + unsigned int ioctl, unsigned long arg) +{ + struct kvm *kvm = filp->private_data; + int r; + + if (kvm->mm != current->mm) + return -EIO; + switch (ioctl) { + case KVM_GET_DIRTY_LOG: { + struct compat_kvm_dirty_log compat_log; + struct kvm_dirty_log log; + + r = -EFAULT; + if (copy_from_user(&compat_log, (void __user *)arg, + sizeof(compat_log))) + goto out; + log.slot = compat_log.slot; + log.padding1 = compat_log.padding1; + log.padding2 = compat_log.padding2; + log.dirty_bitmap = compat_ptr(compat_log.dirty_bitmap); + + r = kvm_vm_ioctl_get_dirty_log(kvm, &log); + if (r) + goto out; + break; + } + default: + r = kvm_vm_ioctl(filp, ioctl, arg); + } + +out: + return r; +} +#endif + static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) { struct page *page[1]; @@ -1576,7 +1623,7 @@ static int kvm_vm_mmap(struct file *file, struct vm_area_struct *vma) static struct file_operations kvm_vm_fops = { .release= kvm_vm_release, .unlocked_ioctl = kvm_vm_ioctl, - .compat_ioctl = kvm_vm_ioctl, + .compat_ioctl = kvm_vm_compat_ioctl, .mmap = kvm_vm_mmap, }; -- 1.6.0.2 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Enable 32bit dirty log pointers on 64bit host
On 10/22/2009 12:25 PM, Alexander Graf wrote: If you send someone's patch, you need to sign this off. That says you are legally allowed to send it along. Ack means "I have a say in this area and it looks good to me", you add it when someone else is doing the sending. Ok, so is it still a From: Arnd then? Is it still signed-off by Arnd even though I change it? Yes. Most kvm patches are From: other people, I just add my signoff (and definitely don't remote theirs). Whether to change From: depends on the changes. If you rewrite the patch, it's From: you. If you just fix a couple of bugs, you generally leave From: alone. Is there a Based-on-patch-by: tag? :-) You can always say "Based on initial patch from ...". -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Enable 32bit dirty log pointers on 64bit host
On 22.10.2009, at 12:23, Avi Kivity wrote: On 10/21/2009 04:08 PM, Alexander Graf wrote: From: Arnd Bergmann With big endian userspace, we can't quite figure out if a pointer is 32 bit (shifted>> 32) or 64 bit when we read a 64 bit pointer. This is what happens with dirty logging. To get the pointer interpreted correctly, we thus need Arnd's patch to implement a compat layer for the ioctl: A better way to do this is to add a separate compat_ioctl() method that converts this for you. From: Arnd Bergmann Signed-off-by: Arnd Bergmann Acked-by: Alexander Graf If you send someone's patch, you need to sign this off. That says you are legally allowed to send it along. Ack means "I have a say in this area and it looks good to me", you add it when someone else is doing the sending. Ok, so is it still a From: Arnd then? Is it still signed-off by Arnd even though I change it? Is there a Based-on-patch-by: tag? :-) Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Enable 32bit dirty log pointers on 64bit host
On 10/21/2009 04:08 PM, Alexander Graf wrote: From: Arnd Bergmann With big endian userspace, we can't quite figure out if a pointer is 32 bit (shifted>> 32) or 64 bit when we read a 64 bit pointer. This is what happens with dirty logging. To get the pointer interpreted correctly, we thus need Arnd's patch to implement a compat layer for the ioctl: A better way to do this is to add a separate compat_ioctl() method that converts this for you. From: Arnd Bergmann Signed-off-by: Arnd Bergmann Acked-by: Alexander Graf If you send someone's patch, you need to sign this off. That says you are legally allowed to send it along. Ack means "I have a say in this area and it looks good to me", you add it when someone else is doing the sending. Changes from Arnd's example version: This goes double when changing the patch. With all the legalese out of the way, the actual code looks good. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Enable 32bit dirty log pointers on 64bit host
From: Arnd Bergmann With big endian userspace, we can't quite figure out if a pointer is 32 bit (shifted >> 32) or 64 bit when we read a 64 bit pointer. This is what happens with dirty logging. To get the pointer interpreted correctly, we thus need Arnd's patch to implement a compat layer for the ioctl: A better way to do this is to add a separate compat_ioctl() method that converts this for you. From: Arnd Bergmann Signed-off-by: Arnd Bergmann Acked-by: Alexander Graf --- Changes from Arnd's example version: - s/log.log/log/ (Avi) - use sizeof(compat_log) (Avi) - compile fixes --- virt/kvm/kvm_main.c | 49 - 1 files changed, 48 insertions(+), 1 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index cac69c4..54a272f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -1542,6 +1543,52 @@ out: return r; } +#ifdef CONFIG_COMPAT +struct compat_kvm_dirty_log { + __u32 slot; + __u32 padding1; + union { + compat_uptr_t dirty_bitmap; /* one bit per page */ + __u64 padding2; + }; +}; + +static long kvm_vm_compat_ioctl(struct file *filp, + unsigned int ioctl, unsigned long arg) +{ + struct kvm *kvm = filp->private_data; + int r; + + if (kvm->mm != current->mm) + return -EIO; + switch (ioctl) { + case KVM_GET_DIRTY_LOG: { + struct compat_kvm_dirty_log compat_log; + struct kvm_dirty_log log; + + r = -EFAULT; + if (copy_from_user(&compat_log, (void __user *)arg, + sizeof(compat_log))) + goto out; + log.slot = compat_log.slot; + log.padding1 = compat_log.padding1; + log.padding2 = compat_log.padding2; + log.dirty_bitmap = compat_ptr(compat_log.dirty_bitmap); + + r = kvm_vm_ioctl_get_dirty_log(kvm, &log); + if (r) + goto out; + break; + } + default: + r = kvm_vm_ioctl(filp, ioctl, arg); + } + +out: + return r; +} +#endif + static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf) { struct page *page[1]; @@ -1576,7 +1623,7 @@ static int kvm_vm_mmap(struct file *file, struct vm_area_struct *vma) static struct file_operations kvm_vm_fops = { .release= kvm_vm_release, .unlocked_ioctl = kvm_vm_ioctl, - .compat_ioctl = kvm_vm_ioctl, + .compat_ioctl = kvm_vm_compat_ioctl, .mmap = kvm_vm_mmap, }; -- 1.6.0.2 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html