| Message ID | 1415320848-13813-4-git-send-email-m.smarduch@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/11/2014 01:40, Mario Smarduch wrote: > In the next patches, we will move parts of x86's kvm_vm_ioctl_get_dirty_log > implementation to generic code; leave the arch-specific code at the end, > similar to the existing generic function kvm_get_dirty_log. > > Reviewed-by: Mario Smarduch <m.smarduch@samsung.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> You should have a "Signed-off-by: Mario Smarduch ..." here, and you should also add my authorship. You can do the latter with "git rebase -i", adding x git commit --amend -C HEAD --author 'Paolo Bonzini <pbonzini@redhat.com>' after this patch. If you're using a patch queue instead (quilt or similar) you can just edit the "From" line in the patch. I guess if you reply to the patch with just Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> the ARM maintainers can do the above for you, if there's no need for a v14. I'll send an eighth patch to actually switch x86 to the new function. Again, the maintainers can apply it in the right place, but please include it yourself if you have to do a v14. Thanks, Paolo > --- > arch/x86/kvm/x86.c | 22 ++++++++++------------ > 1 file changed, 10 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 8f1e22d..dc8e66b 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3606,13 +3606,13 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm, > * > * 1. Take a snapshot of the bit and clear it if needed. > * 2. Write protect the corresponding page. > - * 3. Flush TLB's if needed. > - * 4. Copy the snapshot to the userspace. > + * 3. Copy the snapshot to the userspace. > + * 4. Flush TLB's if needed. > * > - * Between 2 and 3, the guest may write to the page using the remaining TLB > - * entry. This is not a problem because the page will be reported dirty at > - * step 4 using the snapshot taken before and step 3 ensures that successive > - * writes will be logged for the next call. > + * Between 2 and 4, the guest may write to the page using the remaining TLB > + * entry. This is not a problem because the page is reported dirty using > + * the snapshot taken before and step 4 ensures that writes done after > + * exiting to userspace will be logged for the next call. > */ > int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) > { > @@ -3661,6 +3661,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) > > spin_unlock(&kvm->mmu_lock); > > + r = 0; > + if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n)) > + r = -EFAULT; > + > /* See the comments in kvm_mmu_slot_remove_write_access(). */ > lockdep_assert_held(&kvm->slots_lock); > > @@ -3670,12 +3674,6 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) > */ > if (is_dirty) > kvm_flush_remote_tlbs(kvm); > - > - r = -EFAULT; > - if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n)) > - goto out; > - > - r = 0; > out: > mutex_unlock(&kvm->slots_lock); > return r; >
On 11/06/2014 11:44 PM, Paolo Bonzini wrote: > > > On 07/11/2014 01:40, Mario Smarduch wrote: >> In the next patches, we will move parts of x86's kvm_vm_ioctl_get_dirty_log >> implementation to generic code; leave the arch-specific code at the end, >> similar to the existing generic function kvm_get_dirty_log. >> >> Reviewed-by: Mario Smarduch <m.smarduch@samsung.com> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > You should have a "Signed-off-by: Mario Smarduch ..." here, and you > should also add my authorship. You can do the latter with "git rebase > -i", adding > > x git commit --amend -C HEAD --author 'Paolo Bonzini <pbonzini@redhat.com>' > > after this patch. If you're using a patch queue instead (quilt or > similar) you can just edit the "From" line in the patch. > > I guess if you reply to the patch with just > > Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> > > the ARM maintainers can do the above for you, if there's no need for > a v14. > > I'll send an eighth patch to actually switch x86 to the new function. > Again, the maintainers can apply it in the right place, but please > include it yourself if you have to do a v14. > > Thanks, > > Paolo I kind of thought something was wrong :) Thanks for the explanation, until I get the hang of it I'll defer to ARM maintainers. Maybe later update SubmittingPatches documentation. - Mario > >> --- >> arch/x86/kvm/x86.c | 22 ++++++++++------------ >> 1 file changed, 10 insertions(+), 12 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 8f1e22d..dc8e66b 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -3606,13 +3606,13 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm, >> * >> * 1. Take a snapshot of the bit and clear it if needed. >> * 2. Write protect the corresponding page. >> - * 3. Flush TLB's if needed. >> - * 4. Copy the snapshot to the userspace. >> + * 3. Copy the snapshot to the userspace. >> + * 4. Flush TLB's if needed. >> * >> - * Between 2 and 3, the guest may write to the page using the remaining TLB >> - * entry. This is not a problem because the page will be reported dirty at >> - * step 4 using the snapshot taken before and step 3 ensures that successive >> - * writes will be logged for the next call. >> + * Between 2 and 4, the guest may write to the page using the remaining TLB >> + * entry. This is not a problem because the page is reported dirty using >> + * the snapshot taken before and step 4 ensures that writes done after >> + * exiting to userspace will be logged for the next call. >> */ >> int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) >> { >> @@ -3661,6 +3661,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) >> >> spin_unlock(&kvm->mmu_lock); >> >> + r = 0; >> + if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n)) >> + r = -EFAULT; >> + >> /* See the comments in kvm_mmu_slot_remove_write_access(). */ >> lockdep_assert_held(&kvm->slots_lock); >> >> @@ -3670,12 +3674,6 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) >> */ >> if (is_dirty) >> kvm_flush_remote_tlbs(kvm); >> - >> - r = -EFAULT; >> - if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n)) >> - goto out; >> - >> - r = 0; >> out: >> mutex_unlock(&kvm->slots_lock); >> return r; >>
On Fri, Nov 07, 2014 at 11:50:09AM -0800, Mario Smarduch wrote: > On 11/06/2014 11:44 PM, Paolo Bonzini wrote: > > > > > > On 07/11/2014 01:40, Mario Smarduch wrote: > >> In the next patches, we will move parts of x86's kvm_vm_ioctl_get_dirty_log > >> implementation to generic code; leave the arch-specific code at the end, > >> similar to the existing generic function kvm_get_dirty_log. > >> > >> Reviewed-by: Mario Smarduch <m.smarduch@samsung.com> > >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > > You should have a "Signed-off-by: Mario Smarduch ..." here, and you > > should also add my authorship. You can do the latter with "git rebase > > -i", adding > > > > x git commit --amend -C HEAD --author 'Paolo Bonzini <pbonzini@redhat.com>' > > > > after this patch. If you're using a patch queue instead (quilt or > > similar) you can just edit the "From" line in the patch. > > > > I guess if you reply to the patch with just > > > > Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> > > > > the ARM maintainers can do the above for you, if there's no need for > > a v14. > > > > I'll send an eighth patch to actually switch x86 to the new function. > > Again, the maintainers can apply it in the right place, but please > > include it yourself if you have to do a v14. > > > > Thanks, > > > > Paolo > > I kind of thought something was wrong :) Thanks for the explanation, > until I get the hang of it I'll defer to ARM maintainers. Maybe later > update SubmittingPatches documentation. > Since you're doing a respin (right?) then please get the authorship and signed-off-by's right for the next iteration. If you're having trouble with the git mangling to get it right, don't be afraid to reach out, here or on IRC. -Christoffer
On 11/07/2014 12:02 PM, Christoffer Dall wrote: > On Fri, Nov 07, 2014 at 11:50:09AM -0800, Mario Smarduch wrote: >> On 11/06/2014 11:44 PM, Paolo Bonzini wrote: >>> >>> >>> On 07/11/2014 01:40, Mario Smarduch wrote: >>>> In the next patches, we will move parts of x86's kvm_vm_ioctl_get_dirty_log >>>> implementation to generic code; leave the arch-specific code at the end, >>>> similar to the existing generic function kvm_get_dirty_log. >>>> >>>> Reviewed-by: Mario Smarduch <m.smarduch@samsung.com> >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> >>> You should have a "Signed-off-by: Mario Smarduch ..." here, and you >>> should also add my authorship. You can do the latter with "git rebase >>> -i", adding >>> >>> x git commit --amend -C HEAD --author 'Paolo Bonzini <pbonzini@redhat.com>' >>> >>> after this patch. If you're using a patch queue instead (quilt or >>> similar) you can just edit the "From" line in the patch. >>> >>> I guess if you reply to the patch with just >>> >>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> >>> >>> the ARM maintainers can do the above for you, if there's no need for >>> a v14. >>> >>> I'll send an eighth patch to actually switch x86 to the new function. >>> Again, the maintainers can apply it in the right place, but please >>> include it yourself if you have to do a v14. >>> >>> Thanks, >>> >>> Paolo >> >> I kind of thought something was wrong :) Thanks for the explanation, >> until I get the hang of it I'll defer to ARM maintainers. Maybe later >> update SubmittingPatches documentation. >> > Since you're doing a respin (right?) then please get the authorship > and signed-off-by's right for the next iteration. Yes > > If you're having trouble with the git mangling to get it right, don't be > afraid to reach out, here or on IRC. Yes definitely need some coaching on this, this is not git 101 anymore. Thanks. > > -Christoffer >
On Fri, Nov 07, 2014 at 12:44:13PM -0800, Mario Smarduch wrote: > On 11/07/2014 12:02 PM, Christoffer Dall wrote: > > On Fri, Nov 07, 2014 at 11:50:09AM -0800, Mario Smarduch wrote: > >> On 11/06/2014 11:44 PM, Paolo Bonzini wrote: > >>> > >>> > >>> On 07/11/2014 01:40, Mario Smarduch wrote: > >>>> In the next patches, we will move parts of x86's kvm_vm_ioctl_get_dirty_log > >>>> implementation to generic code; leave the arch-specific code at the end, > >>>> similar to the existing generic function kvm_get_dirty_log. > >>>> > >>>> Reviewed-by: Mario Smarduch <m.smarduch@samsung.com> > >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >>> > >>> You should have a "Signed-off-by: Mario Smarduch ..." here, and you > >>> should also add my authorship. You can do the latter with "git rebase > >>> -i", adding > >>> > >>> x git commit --amend -C HEAD --author 'Paolo Bonzini <pbonzini@redhat.com>' > >>> > >>> after this patch. If you're using a patch queue instead (quilt or > >>> similar) you can just edit the "From" line in the patch. > >>> > >>> I guess if you reply to the patch with just > >>> > >>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> > >>> > >>> the ARM maintainers can do the above for you, if there's no need for > >>> a v14. > >>> > >>> I'll send an eighth patch to actually switch x86 to the new function. > >>> Again, the maintainers can apply it in the right place, but please > >>> include it yourself if you have to do a v14. > >>> > >>> Thanks, > >>> > >>> Paolo > >> > >> I kind of thought something was wrong :) Thanks for the explanation, > >> until I get the hang of it I'll defer to ARM maintainers. Maybe later > >> update SubmittingPatches documentation. > >> > > Since you're doing a respin (right?) then please get the authorship > > and signed-off-by's right for the next iteration. > > Yes > > > > If you're having trouble with the git mangling to get it right, don't be > > afraid to reach out, here or on IRC. > > Yes definitely need some coaching on this, this is not git 101 anymore. > It's not that complicated; just get all the patches applied and use 'git rebase -i', mark Paolo's commit as edit, when it stops at that commit do: # git commit -s --amend -C HEAD --author 'Paolo Bonzini <pbonzini@redhat.com>' # git rebase --continue and the '-s' to the commit should add your signed-off-by automatically. -Christoffer
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 8f1e22d..dc8e66b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3606,13 +3606,13 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm, * * 1. Take a snapshot of the bit and clear it if needed. * 2. Write protect the corresponding page. - * 3. Flush TLB's if needed. - * 4. Copy the snapshot to the userspace. + * 3. Copy the snapshot to the userspace. + * 4. Flush TLB's if needed. * - * Between 2 and 3, the guest may write to the page using the remaining TLB - * entry. This is not a problem because the page will be reported dirty at - * step 4 using the snapshot taken before and step 3 ensures that successive - * writes will be logged for the next call. + * Between 2 and 4, the guest may write to the page using the remaining TLB + * entry. This is not a problem because the page is reported dirty using + * the snapshot taken before and step 4 ensures that writes done after + * exiting to userspace will be logged for the next call. */ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) { @@ -3661,6 +3661,10 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) spin_unlock(&kvm->mmu_lock); + r = 0; + if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n)) + r = -EFAULT; + /* See the comments in kvm_mmu_slot_remove_write_access(). */ lockdep_assert_held(&kvm->slots_lock); @@ -3670,12 +3674,6 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) */ if (is_dirty) kvm_flush_remote_tlbs(kvm); - - r = -EFAULT; - if (copy_to_user(log->dirty_bitmap, dirty_bitmap_buffer, n)) - goto out; - - r = 0; out: mutex_unlock(&kvm->slots_lock); return r;