* Re: [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect
2014-11-14 1:57 [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect Mario Smarduch
@ 2014-11-14 2:06 ` Mario Smarduch
2014-11-14 10:03 ` Paolo Bonzini
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Mario Smarduch @ 2014-11-14 2:06 UTC (permalink / raw)
To: kvm-ia64
Hi Paolo,
I changed your patch a little to use a Kconfig symbol,
hope that's fine with you.
- Mario
On 11/13/2014 05:57 PM, Mario Smarduch wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> We now have a generic function that does most of the work of
> kvm_vm_ioctl_get_dirty_log, now use it.
>
> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
> ---
> arch/x86/include/asm/kvm_host.h | 3 --
> arch/x86/kvm/Kconfig | 1 +
> arch/x86/kvm/mmu.c | 4 +--
> arch/x86/kvm/x86.c | 64 ++++++---------------------------------
> 4 files changed, 12 insertions(+), 60 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 7c492ed..934dc24 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -805,9 +805,6 @@ void kvm_mmu_set_mask_ptes(u64 user_mask, u64 accessed_mask,
>
> void kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
> void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
> - struct kvm_memory_slot *slot,
> - gfn_t gfn_offset, unsigned long mask);
> void kvm_mmu_zap_all(struct kvm *kvm);
> void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm);
> unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index f9d16ff..d073594 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -39,6 +39,7 @@ config KVM
> select PERF_EVENTS
> select HAVE_KVM_MSI
> select HAVE_KVM_CPU_RELAX_INTERCEPT
> + select KVM_GENERIC_DIRTYLOG_READ_PROTECT
> select KVM_VFIO
> ---help---
> Support hosting fully virtualized guest machines using hardware
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 9314678..bf6b82c 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1224,7 +1224,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
> }
>
> /**
> - * kvm_mmu_write_protect_pt_masked - write protect selected PT level pages
> + * kvm_arch_mmu_write_protect_pt_masked - write protect selected PT level pages
> * @kvm: kvm instance
> * @slot: slot to protect
> * @gfn_offset: start of the BITS_PER_LONG pages we care about
> @@ -1233,7 +1233,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
> * Used when we do not need to care about huge page mappings: e.g. during dirty
> * logging we do not have any such mappings.
> */
> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
> +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm,
> struct kvm_memory_slot *slot,
> gfn_t gfn_offset, unsigned long mask)
> {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 8f1e22d..9f8ae9a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3606,77 +3606,31 @@ 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)
> {
> - int r;
> - struct kvm_memory_slot *memslot;
> - unsigned long n, i;
> - unsigned long *dirty_bitmap;
> - unsigned long *dirty_bitmap_buffer;
> bool is_dirty = false;
> + int r;
>
> mutex_lock(&kvm->slots_lock);
>
> - r = -EINVAL;
> - if (log->slot >= KVM_USER_MEM_SLOTS)
> - goto out;
> -
> - memslot = id_to_memslot(kvm->memslots, log->slot);
> -
> - dirty_bitmap = memslot->dirty_bitmap;
> - r = -ENOENT;
> - if (!dirty_bitmap)
> - goto out;
> -
> - n = kvm_dirty_bitmap_bytes(memslot);
> -
> - dirty_bitmap_buffer = dirty_bitmap + n / sizeof(long);
> - memset(dirty_bitmap_buffer, 0, n);
> -
> - spin_lock(&kvm->mmu_lock);
> -
> - for (i = 0; i < n / sizeof(long); i++) {
> - unsigned long mask;
> - gfn_t offset;
> -
> - if (!dirty_bitmap[i])
> - continue;
> -
> - is_dirty = true;
> -
> - mask = xchg(&dirty_bitmap[i], 0);
> - dirty_bitmap_buffer[i] = mask;
> -
> - offset = i * BITS_PER_LONG;
> - kvm_mmu_write_protect_pt_masked(kvm, memslot, offset, mask);
> - }
> -
> - spin_unlock(&kvm->mmu_lock);
> -
> - /* See the comments in kvm_mmu_slot_remove_write_access(). */
> - lockdep_assert_held(&kvm->slots_lock);
> + r = kvm_get_dirty_log_protect(kvm, log, &is_dirty);
>
> /*
> * All the TLBs can be flushed out of mmu lock, see the comments in
> * kvm_mmu_slot_remove_write_access().
> */
> + lockdep_assert_held(&kvm->slots_lock);
> 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] 7+ messages in thread* Re: [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect
2014-11-14 1:57 [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect Mario Smarduch
2014-11-14 2:06 ` Mario Smarduch
@ 2014-11-14 10:03 ` Paolo Bonzini
2014-11-22 19:19 ` Christoffer Dall
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2014-11-14 10:03 UTC (permalink / raw)
To: kvm-ia64
On 14/11/2014 03:06, Mario Smarduch wrote:
> Hi Paolo,
>
> I changed your patch a little to use a Kconfig symbol,
> hope that's fine with you.
Of course, thanks.
Paolo
> - Mario
>
> On 11/13/2014 05:57 PM, Mario Smarduch wrote:
>> From: Paolo Bonzini <pbonzini@redhat.com>
>>
>> We now have a generic function that does most of the work of
>> kvm_vm_ioctl_get_dirty_log, now use it.
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>> ---
>> arch/x86/include/asm/kvm_host.h | 3 --
>> arch/x86/kvm/Kconfig | 1 +
>> arch/x86/kvm/mmu.c | 4 +--
>> arch/x86/kvm/x86.c | 64 ++++++---------------------------------
>> 4 files changed, 12 insertions(+), 60 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 7c492ed..934dc24 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -805,9 +805,6 @@ void kvm_mmu_set_mask_ptes(u64 user_mask, u64 accessed_mask,
>>
>> void kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
>> void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>> - struct kvm_memory_slot *slot,
>> - gfn_t gfn_offset, unsigned long mask);
>> void kvm_mmu_zap_all(struct kvm *kvm);
>> void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm);
>> unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>> index f9d16ff..d073594 100644
>> --- a/arch/x86/kvm/Kconfig
>> +++ b/arch/x86/kvm/Kconfig
>> @@ -39,6 +39,7 @@ config KVM
>> select PERF_EVENTS
>> select HAVE_KVM_MSI
>> select HAVE_KVM_CPU_RELAX_INTERCEPT
>> + select KVM_GENERIC_DIRTYLOG_READ_PROTECT
>> select KVM_VFIO
>> ---help---
>> Support hosting fully virtualized guest machines using hardware
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 9314678..bf6b82c 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -1224,7 +1224,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>> }
>>
>> /**
>> - * kvm_mmu_write_protect_pt_masked - write protect selected PT level pages
>> + * kvm_arch_mmu_write_protect_pt_masked - write protect selected PT level pages
>> * @kvm: kvm instance
>> * @slot: slot to protect
>> * @gfn_offset: start of the BITS_PER_LONG pages we care about
>> @@ -1233,7 +1233,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>> * Used when we do not need to care about huge page mappings: e.g. during dirty
>> * logging we do not have any such mappings.
>> */
>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>> +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm,
>> struct kvm_memory_slot *slot,
>> gfn_t gfn_offset, unsigned long mask)
>> {
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 8f1e22d..9f8ae9a 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -3606,77 +3606,31 @@ 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)
>> {
>> - int r;
>> - struct kvm_memory_slot *memslot;
>> - unsigned long n, i;
>> - unsigned long *dirty_bitmap;
>> - unsigned long *dirty_bitmap_buffer;
>> bool is_dirty = false;
>> + int r;
>>
>> mutex_lock(&kvm->slots_lock);
>>
>> - r = -EINVAL;
>> - if (log->slot >= KVM_USER_MEM_SLOTS)
>> - goto out;
>> -
>> - memslot = id_to_memslot(kvm->memslots, log->slot);
>> -
>> - dirty_bitmap = memslot->dirty_bitmap;
>> - r = -ENOENT;
>> - if (!dirty_bitmap)
>> - goto out;
>> -
>> - n = kvm_dirty_bitmap_bytes(memslot);
>> -
>> - dirty_bitmap_buffer = dirty_bitmap + n / sizeof(long);
>> - memset(dirty_bitmap_buffer, 0, n);
>> -
>> - spin_lock(&kvm->mmu_lock);
>> -
>> - for (i = 0; i < n / sizeof(long); i++) {
>> - unsigned long mask;
>> - gfn_t offset;
>> -
>> - if (!dirty_bitmap[i])
>> - continue;
>> -
>> - is_dirty = true;
>> -
>> - mask = xchg(&dirty_bitmap[i], 0);
>> - dirty_bitmap_buffer[i] = mask;
>> -
>> - offset = i * BITS_PER_LONG;
>> - kvm_mmu_write_protect_pt_masked(kvm, memslot, offset, mask);
>> - }
>> -
>> - spin_unlock(&kvm->mmu_lock);
>> -
>> - /* See the comments in kvm_mmu_slot_remove_write_access(). */
>> - lockdep_assert_held(&kvm->slots_lock);
>> + r = kvm_get_dirty_log_protect(kvm, log, &is_dirty);
>>
>> /*
>> * All the TLBs can be flushed out of mmu lock, see the comments in
>> * kvm_mmu_slot_remove_write_access().
>> */
>> + lockdep_assert_held(&kvm->slots_lock);
>> 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;
>> }
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect
2014-11-14 1:57 [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect Mario Smarduch
2014-11-14 2:06 ` Mario Smarduch
2014-11-14 10:03 ` Paolo Bonzini
@ 2014-11-22 19:19 ` Christoffer Dall
2014-11-24 18:35 ` Mario Smarduch
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Christoffer Dall @ 2014-11-22 19:19 UTC (permalink / raw)
To: kvm-ia64
On Thu, Nov 13, 2014 at 05:57:44PM -0800, Mario Smarduch wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> We now have a generic function that does most of the work of
> kvm_vm_ioctl_get_dirty_log, now use it.
>
> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
> ---
> arch/x86/include/asm/kvm_host.h | 3 --
> arch/x86/kvm/Kconfig | 1 +
> arch/x86/kvm/mmu.c | 4 +--
> arch/x86/kvm/x86.c | 64 ++++++---------------------------------
> 4 files changed, 12 insertions(+), 60 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 7c492ed..934dc24 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -805,9 +805,6 @@ void kvm_mmu_set_mask_ptes(u64 user_mask, u64 accessed_mask,
>
> void kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
> void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
> - struct kvm_memory_slot *slot,
> - gfn_t gfn_offset, unsigned long mask);
> void kvm_mmu_zap_all(struct kvm *kvm);
> void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm);
> unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index f9d16ff..d073594 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -39,6 +39,7 @@ config KVM
> select PERF_EVENTS
> select HAVE_KVM_MSI
> select HAVE_KVM_CPU_RELAX_INTERCEPT
> + select KVM_GENERIC_DIRTYLOG_READ_PROTECT
> select KVM_VFIO
> ---help---
> Support hosting fully virtualized guest machines using hardware
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 9314678..bf6b82c 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1224,7 +1224,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
> }
>
> /**
> - * kvm_mmu_write_protect_pt_masked - write protect selected PT level pages
> + * kvm_arch_mmu_write_protect_pt_masked - write protect selected PT level pages
> * @kvm: kvm instance
> * @slot: slot to protect
> * @gfn_offset: start of the BITS_PER_LONG pages we care about
> @@ -1233,7 +1233,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
> * Used when we do not need to care about huge page mappings: e.g. during dirty
> * logging we do not have any such mappings.
> */
> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
> +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm,
> struct kvm_memory_slot *slot,
> gfn_t gfn_offset, unsigned long mask)
> {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 8f1e22d..9f8ae9a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3606,77 +3606,31 @@ 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.
> */
this seems to duplicate the comment in virt/kvm/kvm_main.c, but
whatever.
FWIW:
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect
2014-11-14 1:57 [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect Mario Smarduch
` (2 preceding siblings ...)
2014-11-22 19:19 ` Christoffer Dall
@ 2014-11-24 18:35 ` Mario Smarduch
2014-12-08 23:12 ` Mario Smarduch
2014-12-09 19:42 ` Paolo Bonzini
5 siblings, 0 replies; 7+ messages in thread
From: Mario Smarduch @ 2014-11-24 18:35 UTC (permalink / raw)
To: kvm-ia64
On 11/22/2014 11:19 AM, Christoffer Dall wrote:
> On Thu, Nov 13, 2014 at 05:57:44PM -0800, Mario Smarduch wrote:
>> From: Paolo Bonzini <pbonzini@redhat.com>
>>
>> We now have a generic function that does most of the work of
>> kvm_vm_ioctl_get_dirty_log, now use it.
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>> ---
>> arch/x86/include/asm/kvm_host.h | 3 --
>> arch/x86/kvm/Kconfig | 1 +
>> arch/x86/kvm/mmu.c | 4 +--
>> arch/x86/kvm/x86.c | 64 ++++++---------------------------------
>> 4 files changed, 12 insertions(+), 60 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 7c492ed..934dc24 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -805,9 +805,6 @@ void kvm_mmu_set_mask_ptes(u64 user_mask, u64 accessed_mask,
>>
>> void kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
>> void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>> - struct kvm_memory_slot *slot,
>> - gfn_t gfn_offset, unsigned long mask);
>> void kvm_mmu_zap_all(struct kvm *kvm);
>> void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm);
>> unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>> index f9d16ff..d073594 100644
>> --- a/arch/x86/kvm/Kconfig
>> +++ b/arch/x86/kvm/Kconfig
>> @@ -39,6 +39,7 @@ config KVM
>> select PERF_EVENTS
>> select HAVE_KVM_MSI
>> select HAVE_KVM_CPU_RELAX_INTERCEPT
>> + select KVM_GENERIC_DIRTYLOG_READ_PROTECT
>> select KVM_VFIO
>> ---help---
>> Support hosting fully virtualized guest machines using hardware
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 9314678..bf6b82c 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -1224,7 +1224,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>> }
>>
>> /**
>> - * kvm_mmu_write_protect_pt_masked - write protect selected PT level pages
>> + * kvm_arch_mmu_write_protect_pt_masked - write protect selected PT level pages
>> * @kvm: kvm instance
>> * @slot: slot to protect
>> * @gfn_offset: start of the BITS_PER_LONG pages we care about
>> @@ -1233,7 +1233,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>> * Used when we do not need to care about huge page mappings: e.g. during dirty
>> * logging we do not have any such mappings.
>> */
>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>> +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm,
>> struct kvm_memory_slot *slot,
>> gfn_t gfn_offset, unsigned long mask)
>> {
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 8f1e22d..9f8ae9a 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -3606,77 +3606,31 @@ 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.
>> */
>
> this seems to duplicate the comment in virt/kvm/kvm_main.c, but
> whatever.
Reuses most of that text but differs slightly, the _protect version
is a subset of this one.
>
> FWIW:
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect
2014-11-14 1:57 [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect Mario Smarduch
` (3 preceding siblings ...)
2014-11-24 18:35 ` Mario Smarduch
@ 2014-12-08 23:12 ` Mario Smarduch
2014-12-09 19:42 ` Paolo Bonzini
5 siblings, 0 replies; 7+ messages in thread
From: Mario Smarduch @ 2014-12-08 23:12 UTC (permalink / raw)
To: kvm-ia64
Hi Paolo,
I took a closer look at Christoffers comment,
the _log description in x86.c is a repeat of the
_protect description in kvm_main.c.
I'm wondering if description below would be acceptable, or
perhaps you had a reason leaving it as is.
For the ARM variant I would word same. Please advise.
Thanks.
"
/**
* kvm_vm_ioctl_get_dirty_log - get and clear the log of dirty pages in
a slot
* @kvm: kvm instance
* @log: slot id and address to which we copy the log
*
* Steps 1-4 below provide general overview of dirty page logging. See
* kvm_get_dirty_log_protect() function description for additional details.
*
* We call kvm_get_dirty_log_protect() to handle steps 1-3, upon return we
* always flush the TLB (step 4) even if 'protect' failed and dirty bitmap
* may be corrupt. Regardless of previous outcome KVM logging API does not
* preclude user space subsequent dirty log read. Flushing TLB insures
writes
* will be marked dirty for next log read.
*
* 1. Take a snapshot of the bit and clear it if needed.
* 2. Write protect the corresponding page.
* 3. Copy the snapshot to the userspace.
* 4. Flush TLB's if needed.
*/
"
On 11/22/2014 11:19 AM, Christoffer Dall wrote:
> On Thu, Nov 13, 2014 at 05:57:44PM -0800, Mario Smarduch wrote:
>> From: Paolo Bonzini <pbonzini@redhat.com>
>>
>> We now have a generic function that does most of the work of
>> kvm_vm_ioctl_get_dirty_log, now use it.
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>> ---
>> arch/x86/include/asm/kvm_host.h | 3 --
>> arch/x86/kvm/Kconfig | 1 +
>> arch/x86/kvm/mmu.c | 4 +--
>> arch/x86/kvm/x86.c | 64 ++++++---------------------------------
>> 4 files changed, 12 insertions(+), 60 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 7c492ed..934dc24 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -805,9 +805,6 @@ void kvm_mmu_set_mask_ptes(u64 user_mask, u64 accessed_mask,
>>
>> void kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
>> void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>> - struct kvm_memory_slot *slot,
>> - gfn_t gfn_offset, unsigned long mask);
>> void kvm_mmu_zap_all(struct kvm *kvm);
>> void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm);
>> unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>> index f9d16ff..d073594 100644
>> --- a/arch/x86/kvm/Kconfig
>> +++ b/arch/x86/kvm/Kconfig
>> @@ -39,6 +39,7 @@ config KVM
>> select PERF_EVENTS
>> select HAVE_KVM_MSI
>> select HAVE_KVM_CPU_RELAX_INTERCEPT
>> + select KVM_GENERIC_DIRTYLOG_READ_PROTECT
>> select KVM_VFIO
>> ---help---
>> Support hosting fully virtualized guest machines using hardware
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 9314678..bf6b82c 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -1224,7 +1224,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>> }
>>
>> /**
>> - * kvm_mmu_write_protect_pt_masked - write protect selected PT level pages
>> + * kvm_arch_mmu_write_protect_pt_masked - write protect selected PT level pages
>> * @kvm: kvm instance
>> * @slot: slot to protect
>> * @gfn_offset: start of the BITS_PER_LONG pages we care about
>> @@ -1233,7 +1233,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>> * Used when we do not need to care about huge page mappings: e.g. during dirty
>> * logging we do not have any such mappings.
>> */
>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>> +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm,
>> struct kvm_memory_slot *slot,
>> gfn_t gfn_offset, unsigned long mask)
>> {
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 8f1e22d..9f8ae9a 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -3606,77 +3606,31 @@ 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.
>> */
>
> this seems to duplicate the comment in virt/kvm/kvm_main.c, but
> whatever.
>
> FWIW:
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect
2014-11-14 1:57 [PATCH v14 3/7] KVM: x86: switch to kvm_get_dirty_log_protect Mario Smarduch
` (4 preceding siblings ...)
2014-12-08 23:12 ` Mario Smarduch
@ 2014-12-09 19:42 ` Paolo Bonzini
5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2014-12-09 19:42 UTC (permalink / raw)
To: kvm-ia64
On 09/12/2014 00:12, Mario Smarduch wrote:
> Hi Paolo,
> I took a closer look at Christoffers comment,
> the _log description in x86.c is a repeat of the
> _protect description in kvm_main.c.
>
> I'm wondering if description below would be acceptable, or
> perhaps you had a reason leaving it as is.
Yes, it's okay.
> For the ARM variant I would word same. Please advise.
>
> Thanks.
>
> "
> /**
> * kvm_vm_ioctl_get_dirty_log - get and clear the log of dirty pages in
> a slot
> * @kvm: kvm instance
> * @log: slot id and address to which we copy the log
> *
> * Steps 1-4 below provide general overview of dirty page logging. See
> * kvm_get_dirty_log_protect() function description for additional details.
> *
> * We call kvm_get_dirty_log_protect() to handle steps 1-3, upon return we
> * always flush the TLB (step 4) even if 'protect' failed and dirty bitmap
even if a previous step failed and the dirty bitmap may be corrupt.
> * may be corrupt. Regardless of previous outcome KVM logging API does not
... the KVM logging API ...
> * preclude user space subsequent dirty log read. Flushing TLB insures
s/insures/ensures/
Paolo
> writes
> * will be marked dirty for next log read.
> *
> * 1. Take a snapshot of the bit and clear it if needed.
> * 2. Write protect the corresponding page.
> * 3. Copy the snapshot to the userspace.
> * 4. Flush TLB's if needed.
> */
> "
>
> On 11/22/2014 11:19 AM, Christoffer Dall wrote:
>> On Thu, Nov 13, 2014 at 05:57:44PM -0800, Mario Smarduch wrote:
>>> From: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> We now have a generic function that does most of the work of
>>> kvm_vm_ioctl_get_dirty_log, now use it.
>>>
>>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>>> ---
>>> arch/x86/include/asm/kvm_host.h | 3 --
>>> arch/x86/kvm/Kconfig | 1 +
>>> arch/x86/kvm/mmu.c | 4 +--
>>> arch/x86/kvm/x86.c | 64 ++++++---------------------------------
>>> 4 files changed, 12 insertions(+), 60 deletions(-)
>>>
>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>>> index 7c492ed..934dc24 100644
>>> --- a/arch/x86/include/asm/kvm_host.h
>>> +++ b/arch/x86/include/asm/kvm_host.h
>>> @@ -805,9 +805,6 @@ void kvm_mmu_set_mask_ptes(u64 user_mask, u64 accessed_mask,
>>>
>>> void kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
>>> void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
>>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>>> - struct kvm_memory_slot *slot,
>>> - gfn_t gfn_offset, unsigned long mask);
>>> void kvm_mmu_zap_all(struct kvm *kvm);
>>> void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm);
>>> unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm);
>>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>>> index f9d16ff..d073594 100644
>>> --- a/arch/x86/kvm/Kconfig
>>> +++ b/arch/x86/kvm/Kconfig
>>> @@ -39,6 +39,7 @@ config KVM
>>> select PERF_EVENTS
>>> select HAVE_KVM_MSI
>>> select HAVE_KVM_CPU_RELAX_INTERCEPT
>>> + select KVM_GENERIC_DIRTYLOG_READ_PROTECT
>>> select KVM_VFIO
>>> ---help---
>>> Support hosting fully virtualized guest machines using hardware
>>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>>> index 9314678..bf6b82c 100644
>>> --- a/arch/x86/kvm/mmu.c
>>> +++ b/arch/x86/kvm/mmu.c
>>> @@ -1224,7 +1224,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>>> }
>>>
>>> /**
>>> - * kvm_mmu_write_protect_pt_masked - write protect selected PT level pages
>>> + * kvm_arch_mmu_write_protect_pt_masked - write protect selected PT level pages
>>> * @kvm: kvm instance
>>> * @slot: slot to protect
>>> * @gfn_offset: start of the BITS_PER_LONG pages we care about
>>> @@ -1233,7 +1233,7 @@ static bool __rmap_write_protect(struct kvm *kvm, unsigned long *rmapp,
>>> * Used when we do not need to care about huge page mappings: e.g. during dirty
>>> * logging we do not have any such mappings.
>>> */
>>> -void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
>>> +void kvm_arch_mmu_write_protect_pt_masked(struct kvm *kvm,
>>> struct kvm_memory_slot *slot,
>>> gfn_t gfn_offset, unsigned long mask)
>>> {
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 8f1e22d..9f8ae9a 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -3606,77 +3606,31 @@ 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.
>>> */
>>
>> this seems to duplicate the comment in virt/kvm/kvm_main.c, but
>> whatever.
>>
>> FWIW:
>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread