* Re: [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG
2014-11-07 0:40 [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG Mario Smarduch
@ 2014-11-07 7:44 ` Paolo Bonzini
2014-11-07 19:50 ` Mario Smarduch
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2014-11-07 7:44 UTC (permalink / raw)
To: kvm-ia64
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;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG
2014-11-07 0:40 [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG Mario Smarduch
2014-11-07 7:44 ` Paolo Bonzini
@ 2014-11-07 19:50 ` Mario Smarduch
2014-11-07 20:02 ` Christoffer Dall
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Mario Smarduch @ 2014-11-07 19:50 UTC (permalink / raw)
To: kvm-ia64
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;
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG
2014-11-07 0:40 [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG Mario Smarduch
2014-11-07 7:44 ` Paolo Bonzini
2014-11-07 19:50 ` Mario Smarduch
@ 2014-11-07 20:02 ` Christoffer Dall
2014-11-07 20:44 ` Mario Smarduch
2014-11-07 21:07 ` Christoffer Dall
4 siblings, 0 replies; 6+ messages in thread
From: Christoffer Dall @ 2014-11-07 20:02 UTC (permalink / raw)
To: kvm-ia64
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG
2014-11-07 0:40 [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG Mario Smarduch
` (2 preceding siblings ...)
2014-11-07 20:02 ` Christoffer Dall
@ 2014-11-07 20:44 ` Mario Smarduch
2014-11-07 21:07 ` Christoffer Dall
4 siblings, 0 replies; 6+ messages in thread
From: Mario Smarduch @ 2014-11-07 20:44 UTC (permalink / raw)
To: kvm-ia64
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG
2014-11-07 0:40 [PATCH v13 3/7] KVM: x86: flush TLBs last before returning from KVM_GET_DIRTY_LOG Mario Smarduch
` (3 preceding siblings ...)
2014-11-07 20:44 ` Mario Smarduch
@ 2014-11-07 21:07 ` Christoffer Dall
4 siblings, 0 replies; 6+ messages in thread
From: Christoffer Dall @ 2014-11-07 21:07 UTC (permalink / raw)
To: kvm-ia64
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
^ permalink raw reply [flat|nested] 6+ messages in thread