Re: [PATCH 5/6] KVM: x86: Add new dirty logging kvm_x86_ops for PML

2015-02-05 Thread Radim Krčmář
2015-02-05 14:29+0800, Kai Huang:
 +   /* Still write protect RO slot */
 +   if (new-flags  KVM_MEM_READONLY) {
 +   kvm_mmu_slot_remove_write_access(kvm, new);
 We didn't write protect RO slots before, does this patch depend on it?
 No PML doesn't depend on it to work. It's suggested by Paolo.

Thanks, it would have deserved a separate patch, IMO.
--
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 5/6] KVM: x86: Add new dirty logging kvm_x86_ops for PML

2015-02-04 Thread Kai Huang


On 02/03/2015 11:53 PM, Radim Krčmář wrote:

2015-01-28 10:54+0800, Kai Huang:

This patch adds new kvm_x86_ops dirty logging hooks to enable/disable dirty
logging for particular memory slot, and to flush potentially logged dirty GPAs
before reporting slot-dirty_bitmap to userspace.

kvm x86 common code calls these hooks when they are available so PML logic can
be hidden to VMX specific. Other ARCHs won't be impacted as these hooks are NULL
for them.

Signed-off-by: Kai Huang kai.hu...@linux.intel.com
---
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -802,6 +802,31 @@ struct kvm_x86_ops {
+
+   /*
+* Arch-specific dirty logging hooks. These hooks are only supposed to
+* be valid if the specific arch has hardware-accelerated dirty logging
+* mechanism. Currently only for PML on VMX.
+*
+*  - slot_enable_log_dirty:
+*  called when enabling log dirty mode for the slot.

(I guess that log dirty mode isn't the meaning that people will think
  after seeing 'log_dirty' ...
  I'd at least change 'log_dirty' to 'dirty_log' in these names.)


+*  - slot_disable_log_dirty:
+*  called when disabling log dirty mode for the slot.
+*  also called when slot is created with log dirty disabled.
+*  - flush_log_dirty:
+*  called before reporting dirty_bitmap to userspace.
+*  - enable_log_dirty_pt_masked:
+*  called when reenabling log dirty for the GFNs in the mask after
+*  corresponding bits are cleared in slot-dirty_bitmap.

This name is very confusing ... I think we should hint that this is
called after we learn that the page has been written to and would like
to monitor it again.

Using something like collected/refresh?  (I'd have to do horrible things
to come up with a good name, sorry.)


--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3780,6 +3780,12 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct 
kvm_dirty_log *log)
  
  	mutex_lock(kvm-slots_lock);
  
+	/*

+* Flush potentially hardware-cached dirty pages to dirty_bitmap.
+*/
+   if (kvm_x86_ops-flush_log_dirty)
+   kvm_x86_ops-flush_log_dirty(kvm);

(Flushing would make more sense in kvm_get_dirty_log_protect().)


+
r = kvm_get_dirty_log_protect(kvm, log, is_dirty);
  
  	/*

@@ -7533,6 +7539,56 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
return 0;
  }
  
+static void kvm_mmu_slot_apply_flags(struct kvm *kvm,

+struct kvm_memory_slot *new)
+{
+   /* Still write protect RO slot */
+   if (new-flags  KVM_MEM_READONLY) {
+   kvm_mmu_slot_remove_write_access(kvm, new);

We didn't write protect RO slots before, does this patch depend on it?

No PML doesn't depend on it to work. It's suggested by Paolo.

Thanks,
-Kai



@@ -7562,16 +7618,15 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
-   if ((change != KVM_MR_DELETE)  (new-flags  KVM_MEM_LOG_DIRTY_PAGES))
-   kvm_mmu_slot_remove_write_access(kvm, new);
+   if (change != KVM_MR_DELETE)
+   kvm_mmu_slot_apply_flags(kvm, new);


--
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 5/6] KVM: x86: Add new dirty logging kvm_x86_ops for PML

2015-02-03 Thread Radim Krčmář
2015-01-28 10:54+0800, Kai Huang:
 This patch adds new kvm_x86_ops dirty logging hooks to enable/disable dirty
 logging for particular memory slot, and to flush potentially logged dirty GPAs
 before reporting slot-dirty_bitmap to userspace.
 
 kvm x86 common code calls these hooks when they are available so PML logic can
 be hidden to VMX specific. Other ARCHs won't be impacted as these hooks are 
 NULL
 for them.
 
 Signed-off-by: Kai Huang kai.hu...@linux.intel.com
 ---
 --- a/arch/x86/include/asm/kvm_host.h
 +++ b/arch/x86/include/asm/kvm_host.h
 @@ -802,6 +802,31 @@ struct kvm_x86_ops {
 +
 + /*
 +  * Arch-specific dirty logging hooks. These hooks are only supposed to
 +  * be valid if the specific arch has hardware-accelerated dirty logging
 +  * mechanism. Currently only for PML on VMX.
 +  *
 +  *  - slot_enable_log_dirty:
 +  *  called when enabling log dirty mode for the slot.

(I guess that log dirty mode isn't the meaning that people will think
 after seeing 'log_dirty' ...
 I'd at least change 'log_dirty' to 'dirty_log' in these names.)

 +  *  - slot_disable_log_dirty:
 +  *  called when disabling log dirty mode for the slot.
 +  *  also called when slot is created with log dirty disabled.
 +  *  - flush_log_dirty:
 +  *  called before reporting dirty_bitmap to userspace.
 +  *  - enable_log_dirty_pt_masked:
 +  *  called when reenabling log dirty for the GFNs in the mask after
 +  *  corresponding bits are cleared in slot-dirty_bitmap.

This name is very confusing ... I think we should hint that this is
called after we learn that the page has been written to and would like
to monitor it again.

Using something like collected/refresh?  (I'd have to do horrible things
to come up with a good name, sorry.)

 --- a/arch/x86/kvm/x86.c
 +++ b/arch/x86/kvm/x86.c
 @@ -3780,6 +3780,12 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct 
 kvm_dirty_log *log)
  
   mutex_lock(kvm-slots_lock);
  
 + /*
 +  * Flush potentially hardware-cached dirty pages to dirty_bitmap.
 +  */
 + if (kvm_x86_ops-flush_log_dirty)
 + kvm_x86_ops-flush_log_dirty(kvm);

(Flushing would make more sense in kvm_get_dirty_log_protect().)

 +
   r = kvm_get_dirty_log_protect(kvm, log, is_dirty);
  
   /*
 @@ -7533,6 +7539,56 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
   return 0;
  }
  
 +static void kvm_mmu_slot_apply_flags(struct kvm *kvm,
 +  struct kvm_memory_slot *new)
 +{
 + /* Still write protect RO slot */
 + if (new-flags  KVM_MEM_READONLY) {
 + kvm_mmu_slot_remove_write_access(kvm, new);

We didn't write protect RO slots before, does this patch depend on it?

 @@ -7562,16 +7618,15 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 - if ((change != KVM_MR_DELETE)  (new-flags  KVM_MEM_LOG_DIRTY_PAGES))
 - kvm_mmu_slot_remove_write_access(kvm, new);
 + if (change != KVM_MR_DELETE)
 + kvm_mmu_slot_apply_flags(kvm, new);
--
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