Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1333492 > unrolled thread

[PATCH v3 00/11] KVM: x86: track guest page access

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/11] KVM: x86: track guest page access Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-14 12:50 +0100
    [PATCH v3 07/11] KVM: page track: add notifier support Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-14 12:50 +0100
      Re: [PATCH v3 07/11] KVM: page track: add notifier support Paolo Bonzini <pbonzini@redhat.com> - 2016-02-19 13:00 +0100
        Re: [PATCH v3 07/11] KVM: page track: add notifier support Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-23 05:50 +0100
          Re: [PATCH v3 07/11] KVM: page track: add notifier support Paolo Bonzini <pbonzini@redhat.com> - 2016-02-23 15:20 +0100
    [PATCH v3 04/11] KVM: page track: add the framework of guest page tracking Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-14 12:50 +0100
      Re: [PATCH v3 04/11] KVM: page track: add the framework of guest page  tracking Paolo Bonzini <pbonzini@redhat.com> - 2016-02-19 12:30 +0100
        Re: [PATCH v3 04/11] KVM: page track: add the framework of guest page  tracking Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-23 05:10 +0100
    [PATCH v3 03/11] KVM: MMU: introduce kvm_mmu_slot_gfn_write_protect Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-14 12:50 +0100
    Re: [PATCH v3 00/11] KVM: x86: track guest page access Paolo Bonzini <pbonzini@redhat.com> - 2016-02-19 13:10 +0100
      Re: [PATCH v3 00/11] KVM: x86: track guest page access Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-22 11:20 +0100
        Re: [PATCH v3 00/11] KVM: x86: track guest page access Jike Song <jike.song@intel.com> - 2016-02-23 04:10 +0100
          RE: [PATCH v3 00/11] KVM: x86: track guest page access "Tian, Kevin" <kevin.tian@intel.com> - 2016-02-23 06:50 +0100
            Re: [PATCH v3 00/11] KVM: x86: track guest page access Paolo Bonzini <pbonzini@redhat.com> - 2016-02-23 13:20 +0100
          Re: [PATCH v3 00/11] KVM: x86: track guest page access Paolo Bonzini <pbonzini@redhat.com> - 2016-02-23 11:10 +0100
            Re: [PATCH v3 00/11] KVM: x86: track guest page access Jike Song <jike.song@intel.com> - 2016-02-23 13:00 +0100

#1333492 — [PATCH v3 00/11] KVM: x86: track guest page access

Changelong in v3:
- refine the code of mmu_need_write_protect() based on Huang Kai's suggestion
- rebase the patchset against current code

Changelog in v2:
- fix a issue that the track memory of memslot is freed if we only move
  the memslot or change the flags of memslot
- do not track the gfn which is not mapped in memslots
- introduce the nolock APIs at the begin of the patchset
- use 'unsigned short' as the track counter to reduce the memory and which
  should be enough for shadow page table and KVMGT

This patchset introduces the feature which allows us to track page
access in guest. Currently, only write access tracking is implemented
in this version.

Four APIs are introduces:
- kvm_page_track_add_page(kvm, gfn, mode), single guest page @gfn is
  added into the track pool of the guest instance represented by @kvm,
  @mode specifies which kind of access on the @gfn is tracked
  
- kvm_page_track_remove_page(kvm, gfn, mode), is the opposed operation
  of kvm_page_track_add_page() which removes @gfn from the tracking pool.
  gfn is no tracked after its last user is gone

- kvm_page_track_register_notifier(kvm, n), register a notifier so that
  the event triggered by page tracking will be received, at that time,
  the callback of n->track_write() will be called

- kvm_page_track_unregister_notifier(kvm, n), does the opposed operation
  of kvm_page_track_register_notifier(), which unlinks the notifier and
  stops receiving the tracked event

The first user of page track is non-leaf shadow page tables as they are
always write protected. It also gains performance improvement because
page track speeds up page fault handler for the tracked pages. The
performance result of kernel building is as followings:

   before           after
real 461.63       real 455.48
user 4529.55      user 4557.88
sys 1995.39       sys 1922.57

Furthermore, it is the infrastructure of other kind of shadow page table,
such as GPU shadow page table introduced in KVMGT (1) and native nested
IOMMU.

This patch can be divided into two parts:
- patch 1 ~ patch 7, implement page tracking
- others patches apply page tracking to non-leaf shadow page table

(1): http://lkml.iu.edu/hypermail/linux/kernel/1510.3/01562.html

Xiao Guangrong (11):
  KVM: MMU: rename has_wrprotected_page to mmu_gfn_lpage_is_disallowed
  KVM: MMU: introduce kvm_mmu_gfn_{allow,disallow}_lpage
  KVM: MMU: introduce kvm_mmu_slot_gfn_write_protect
  KVM: page track: add the framework of guest page tracking
  KVM: page track: introduce kvm_page_track_{add,remove}_page
  KVM: MMU: let page fault handler be aware tracked page
  KVM: page track: add notifier support
  KVM: MMU: use page track for non-leaf shadow pages
  KVM: MMU: simplify mmu_need_write_protect
  KVM: MMU: clear write-flooding on the fast path of tracked page
  KVM: MMU: apply page track notifier

 Documentation/virtual/kvm/mmu.txt     |   6 +-
 arch/x86/include/asm/kvm_host.h       |  12 +-
 arch/x86/include/asm/kvm_page_track.h |  67 +++++++++
 arch/x86/kvm/Makefile                 |   3 +-
 arch/x86/kvm/mmu.c                    | 209 ++++++++++++++++++---------
 arch/x86/kvm/mmu.h                    |   5 +
 arch/x86/kvm/page_track.c             | 257 ++++++++++++++++++++++++++++++++++
 arch/x86/kvm/paging_tmpl.h            |   5 +
 arch/x86/kvm/x86.c                    |  27 ++--
 9 files changed, 512 insertions(+), 79 deletions(-)
 create mode 100644 arch/x86/include/asm/kvm_page_track.h
 create mode 100644 arch/x86/kvm/page_track.c

-- 
1.8.3.1

[toc] | [next] | [standalone]

#1333493 — [PATCH v3 07/11] KVM: page track: add notifier support

Notifier list is introduced so that any node wants to receive the track
event can register to the list

Two APIs are introduced here:
- kvm_page_track_register_notifier(): register the notifier to receive
  track event

- kvm_page_track_unregister_notifier(): stop receiving track event by
  unregister the notifier

The callback, node->track_write() is called when a write access on the
write tracked page happens

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h       |  1 +
 arch/x86/include/asm/kvm_page_track.h | 39 ++++++++++++++++++++
 arch/x86/kvm/page_track.c             | 67 +++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c                    |  4 +++
 4 files changed, 111 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d8931d0..282bc2f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -696,6 +696,7 @@ struct kvm_arch {
 	 */
 	struct list_head active_mmu_pages;
 	struct list_head zapped_obsolete_pages;
+	struct kvm_page_track_notifier_head track_notifier_head;
 
 	struct list_head assigned_dev_head;
 	struct iommu_domain *iommu_domain;
diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
index 97ac9c3..1aae4ef 100644
--- a/arch/x86/include/asm/kvm_page_track.h
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -6,6 +6,36 @@ enum kvm_page_track_mode {
 	KVM_PAGE_TRACK_MAX,
 };
 
+/*
+ * The notifier represented by @kvm_page_track_notifier_node is linked into
+ * the head which will be notified when guest is triggering the track event.
+ *
+ * Write access on the head is protected by kvm->mmu_lock, read access
+ * is protected by track_srcu.
+ */
+struct kvm_page_track_notifier_head {
+	struct srcu_struct track_srcu;
+	struct hlist_head track_notifier_list;
+};
+
+struct kvm_page_track_notifier_node {
+	struct hlist_node node;
+
+	/*
+	 * It is called when guest is writing the write-tracked page
+	 * and write emulation is finished at that time.
+	 *
+	 * @vcpu: the vcpu where the write access happened.
+	 * @gpa: the physical address written by guest.
+	 * @new: the data was written to the address.
+	 * @bytes: the written length.
+	 */
+	void (*track_write)(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
+			    int bytes);
+};
+
+void kvm_page_track_init(struct kvm *kvm);
+
 void kvm_page_track_free_memslot(struct kvm_memory_slot *free,
 				 struct kvm_memory_slot *dont);
 int kvm_page_track_create_memslot(struct kvm_memory_slot *slot,
@@ -25,4 +55,13 @@ void kvm_page_track_remove_page(struct kvm *kvm, gfn_t gfn,
 				enum kvm_page_track_mode mode);
 bool kvm_page_track_check_mode(struct kvm_vcpu *vcpu, gfn_t gfn,
 			       enum kvm_page_track_mode mode);
+
+void
+kvm_page_track_register_notifier(struct kvm *kvm,
+				 struct kvm_page_track_notifier_node *n);
+void
+kvm_page_track_unregister_notifier(struct kvm *kvm,
+				   struct kvm_page_track_notifier_node *n);
+void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
+			  int bytes);
 #endif
diff --git a/arch/x86/kvm/page_track.c b/arch/x86/kvm/page_track.c
index de9b32f..0692cc6 100644
--- a/arch/x86/kvm/page_track.c
+++ b/arch/x86/kvm/page_track.c
@@ -188,3 +188,70 @@ bool kvm_page_track_check_mode(struct kvm_vcpu *vcpu, gfn_t gfn,
 
 	return !!ACCESS_ONCE(slot->arch.gfn_track[mode][index]);
 }
+
+void kvm_page_track_init(struct kvm *kvm)
+{
+	struct kvm_page_track_notifier_head *head;
+
+	head = &kvm->arch.track_notifier_head;
+	init_srcu_struct(&head->track_srcu);
+	INIT_HLIST_HEAD(&head->track_notifier_list);
+}
+
+/*
+ * register the notifier so that event interception for the tracked guest
+ * pages can be received.
+ */
+void
+kvm_page_track_register_notifier(struct kvm *kvm,
+				 struct kvm_page_track_notifier_node *n)
+{
+	struct kvm_page_track_notifier_head *head;
+
+	head = &kvm->arch.track_notifier_head;
+
+	spin_lock(&kvm->mmu_lock);
+	hlist_add_head_rcu(&n->node, &head->track_notifier_list);
+	spin_unlock(&kvm->mmu_lock);
+}
+
+/*
+ * stop receiving the event interception. It is the opposed operation of
+ * kvm_page_track_register_notifier().
+ */
+void
+kvm_page_track_unregister_notifier(struct kvm *kvm,
+				   struct kvm_page_track_notifier_node *n)
+{
+	struct kvm_page_track_notifier_head *head;
+
+	head = &kvm->arch.track_notifier_head;
+
+	spin_lock(&kvm->mmu_lock);
+	hlist_del_rcu(&n->node);
+	spin_unlock(&kvm->mmu_lock);
+	synchronize_srcu(&head->track_srcu);
+}
+
+/*
+ * Notify the node that write access is intercepted and write emulation is
+ * finished at this time.
+ *
+ * The node should figure out if the written page is the one that node is
+ * interested in by itself.
+ */
+void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
+			  int bytes)
+{
+	struct kvm_page_track_notifier_head *head;
+	struct kvm_page_track_notifier_node *n;
+	int idx;
+
+	head = &vcpu->kvm->arch.track_notifier_head;
+
+	idx = srcu_read_lock(&head->track_srcu);
+	hlist_for_each_entry_rcu(n, &head->track_notifier_list, node)
+		if (n->track_write)
+			n->track_write(vcpu, gpa, new, bytes);
+	srcu_read_unlock(&head->track_srcu, idx);
+}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e25ebb7..98019b6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4370,6 +4370,7 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 	if (ret < 0)
 		return 0;
 	kvm_mmu_pte_write(vcpu, gpa, val, bytes);
+	kvm_page_track_write(vcpu, gpa, val, bytes);
 	return 1;
 }
 
@@ -4628,6 +4629,7 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
 
 	kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
 	kvm_mmu_pte_write(vcpu, gpa, new, bytes);
+	kvm_page_track_write(vcpu, gpa, new, bytes);
 
 	return X86EMUL_CONTINUE;
 
@@ -7748,6 +7750,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	INIT_DELAYED_WORK(&kvm->arch.kvmclock_update_work, kvmclock_update_fn);
 	INIT_DELAYED_WORK(&kvm->arch.kvmclock_sync_work, kvmclock_sync_fn);
 
+	kvm_page_track_init(kvm);
+
 	return 0;
 }
 
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1338095 — Re: [PATCH v3 07/11] KVM: page track: add notifier support

On 14/02/2016 12:31, Xiao Guangrong wrote:
> Notifier list is introduced so that any node wants to receive the track
> event can register to the list
> 
> Two APIs are introduced here:
> - kvm_page_track_register_notifier(): register the notifier to receive
>   track event
> 
> - kvm_page_track_unregister_notifier(): stop receiving track event by
>   unregister the notifier
> 
> The callback, node->track_write() is called when a write access on the
> write tracked page happens
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h       |  1 +
>  arch/x86/include/asm/kvm_page_track.h | 39 ++++++++++++++++++++
>  arch/x86/kvm/page_track.c             | 67 +++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/x86.c                    |  4 +++
>  4 files changed, 111 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index d8931d0..282bc2f 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -696,6 +696,7 @@ struct kvm_arch {
>  	 */
>  	struct list_head active_mmu_pages;
>  	struct list_head zapped_obsolete_pages;
> +	struct kvm_page_track_notifier_head track_notifier_head;
>  
>  	struct list_head assigned_dev_head;
>  	struct iommu_domain *iommu_domain;
> diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
> index 97ac9c3..1aae4ef 100644
> --- a/arch/x86/include/asm/kvm_page_track.h
> +++ b/arch/x86/include/asm/kvm_page_track.h
> @@ -6,6 +6,36 @@ enum kvm_page_track_mode {
>  	KVM_PAGE_TRACK_MAX,
>  };
>  
> +/*
> + * The notifier represented by @kvm_page_track_notifier_node is linked into
> + * the head which will be notified when guest is triggering the track event.
> + *
> + * Write access on the head is protected by kvm->mmu_lock, read access
> + * is protected by track_srcu.
> + */
> +struct kvm_page_track_notifier_head {
> +	struct srcu_struct track_srcu;
> +	struct hlist_head track_notifier_list;
> +};
> +
> +struct kvm_page_track_notifier_node {
> +	struct hlist_node node;
> +
> +	/*
> +	 * It is called when guest is writing the write-tracked page
> +	 * and write emulation is finished at that time.
> +	 *
> +	 * @vcpu: the vcpu where the write access happened.
> +	 * @gpa: the physical address written by guest.
> +	 * @new: the data was written to the address.
> +	 * @bytes: the written length.
> +	 */
> +	void (*track_write)(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
> +			    int bytes);
> +};
> +
> +void kvm_page_track_init(struct kvm *kvm);
> +
>  void kvm_page_track_free_memslot(struct kvm_memory_slot *free,
>  				 struct kvm_memory_slot *dont);
>  int kvm_page_track_create_memslot(struct kvm_memory_slot *slot,
> @@ -25,4 +55,13 @@ void kvm_page_track_remove_page(struct kvm *kvm, gfn_t gfn,
>  				enum kvm_page_track_mode mode);
>  bool kvm_page_track_check_mode(struct kvm_vcpu *vcpu, gfn_t gfn,
>  			       enum kvm_page_track_mode mode);
> +
> +void
> +kvm_page_track_register_notifier(struct kvm *kvm,
> +				 struct kvm_page_track_notifier_node *n);
> +void
> +kvm_page_track_unregister_notifier(struct kvm *kvm,
> +				   struct kvm_page_track_notifier_node *n);
> +void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
> +			  int bytes);
>  #endif
> diff --git a/arch/x86/kvm/page_track.c b/arch/x86/kvm/page_track.c
> index de9b32f..0692cc6 100644
> --- a/arch/x86/kvm/page_track.c
> +++ b/arch/x86/kvm/page_track.c
> @@ -188,3 +188,70 @@ bool kvm_page_track_check_mode(struct kvm_vcpu *vcpu, gfn_t gfn,
>  
>  	return !!ACCESS_ONCE(slot->arch.gfn_track[mode][index]);
>  }
> +
> +void kvm_page_track_init(struct kvm *kvm)
> +{
> +	struct kvm_page_track_notifier_head *head;
> +
> +	head = &kvm->arch.track_notifier_head;
> +	init_srcu_struct(&head->track_srcu);
> +	INIT_HLIST_HEAD(&head->track_notifier_list);
> +}
> +
> +/*
> + * register the notifier so that event interception for the tracked guest
> + * pages can be received.
> + */
> +void
> +kvm_page_track_register_notifier(struct kvm *kvm,
> +				 struct kvm_page_track_notifier_node *n)
> +{
> +	struct kvm_page_track_notifier_head *head;
> +
> +	head = &kvm->arch.track_notifier_head;
> +
> +	spin_lock(&kvm->mmu_lock);
> +	hlist_add_head_rcu(&n->node, &head->track_notifier_list);
> +	spin_unlock(&kvm->mmu_lock);
> +}
> +
> +/*
> + * stop receiving the event interception. It is the opposed operation of
> + * kvm_page_track_register_notifier().
> + */
> +void
> +kvm_page_track_unregister_notifier(struct kvm *kvm,
> +				   struct kvm_page_track_notifier_node *n)
> +{
> +	struct kvm_page_track_notifier_head *head;
> +
> +	head = &kvm->arch.track_notifier_head;
> +
> +	spin_lock(&kvm->mmu_lock);
> +	hlist_del_rcu(&n->node);
> +	spin_unlock(&kvm->mmu_lock);
> +	synchronize_srcu(&head->track_srcu);
> +}
> +
> +/*
> + * Notify the node that write access is intercepted and write emulation is
> + * finished at this time.
> + *
> + * The node should figure out if the written page is the one that node is
> + * interested in by itself.
> + */
> +void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
> +			  int bytes)
> +{
> +	struct kvm_page_track_notifier_head *head;
> +	struct kvm_page_track_notifier_node *n;
> +	int idx;
> +
> +	head = &vcpu->kvm->arch.track_notifier_head;

Please check outside SRCU if the notifier list is empty.  If so, there
is no need to do the (relatively) expensive srcu_read_lock/unlock.

Paolo

> +	idx = srcu_read_lock(&head->track_srcu);
> +	hlist_for_each_entry_rcu(n, &head->track_notifier_list, node)
> +		if (n->track_write)
> +			n->track_write(vcpu, gpa, new, bytes);
> +	srcu_read_unlock(&head->track_srcu, idx);
> +}
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e25ebb7..98019b6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4370,6 +4370,7 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
>  	if (ret < 0)
>  		return 0;

A kvm_vcpu_mark_page_dirty is missing here, isn't it?  I can take care
of it, but it would be great if you double-checked this.  If so, that
should be fixed in stable kernels too.

Can you add a kvm_vcpu_note_page_write(vcpu, gpa, val, bytes) function
that takes care of calling kvm_vcpu_mark_page_dirty, kvm_mmu_pte_write
and kvm_page_track-write?

Thanks,

Paolo

>  	kvm_mmu_pte_write(vcpu, gpa, val, bytes);
> +	kvm_page_track_write(vcpu, gpa, val, bytes);
>  	return 1;
>  }
>  
> @@ -4628,6 +4629,7 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
>  
>  	kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
>  	kvm_mmu_pte_write(vcpu, gpa, new, bytes);
> +	kvm_page_track_write(vcpu, gpa, new, bytes);
>  
>  	return X86EMUL_CONTINUE;
>  
> @@ -7748,6 +7750,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  	INIT_DELAYED_WORK(&kvm->arch.kvmclock_update_work, kvmclock_update_fn);
>  	INIT_DELAYED_WORK(&kvm->arch.kvmclock_sync_work, kvmclock_sync_fn);
>  
> +	kvm_page_track_init(kvm);
> +
>  	return 0;
>  }
>  
> 

[toc] | [prev] | [next] | [standalone]

#1340296 — Re: [PATCH v3 07/11] KVM: page track: add notifier support

On 02/19/2016 07:51 PM, Paolo Bonzini wrote:
>
>
> On 14/02/2016 12:31, Xiao Guangrong wrote:

>> +void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
>> +			  int bytes)
>> +{
>> +	struct kvm_page_track_notifier_head *head;
>> +	struct kvm_page_track_notifier_node *n;
>> +	int idx;
>> +
>> +	head = &vcpu->kvm->arch.track_notifier_head;
>
> Please check outside SRCU if the notifier list is empty.  If so, there
> is no need to do the (relatively) expensive srcu_read_lock/unlock.
>

Good to me. I will check it by calling hlist_empty() first before holding
the srcu read lock.

> Paolo
>
>> +	idx = srcu_read_lock(&head->track_srcu);
>> +	hlist_for_each_entry_rcu(n, &head->track_notifier_list, node)
>> +		if (n->track_write)
>> +			n->track_write(vcpu, gpa, new, bytes);
>> +	srcu_read_unlock(&head->track_srcu, idx);
>> +}
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index e25ebb7..98019b6 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4370,6 +4370,7 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
>>   	if (ret < 0)
>>   		return 0;
>
> A kvm_vcpu_mark_page_dirty is missing here, isn't it?  I can take care
> of it, but it would be great if you double-checked this.  If so, that
> should be fixed in stable kernels too.

No. It's already been handled in emulator_write_phys() -> kvm_vcpu_write_guest()
-> kvm_vcpu_write_guest_page() -> __kvm_write_guest_page().

>
> Can you add a kvm_vcpu_note_page_write(vcpu, gpa, val, bytes) function
> that takes care of calling kvm_vcpu_mark_page_dirty, kvm_mmu_pte_write
> and kvm_page_track-write?
>

After this patchset, kvm_mmu_pte_write is only a static notifier callback called
by kvm_page_track_write().

And the dirty tracking in emulator_write_phys() is handled in a public API (as my
explanation above), in emulator_cmpxchg_emulated is handled by itself. So i think
it is better to leaving dirty tracking to the separate paths, no? :)

[toc] | [prev] | [next] | [standalone]

#1340670 — Re: [PATCH v3 07/11] KVM: page track: add notifier support

On 23/02/2016 05:34, Xiao Guangrong wrote:
>>
>> A kvm_vcpu_mark_page_dirty is missing here, isn't it?  I can take care
>> of it, but it would be great if you double-checked this.  If so, that
>> should be fixed in stable kernels too.
> 
> No. It's already been handled in emulator_write_phys() ->
> kvm_vcpu_write_guest()
> -> kvm_vcpu_write_guest_page() -> __kvm_write_guest_page().

You're right...

>>
>> Can you add a kvm_vcpu_note_page_write(vcpu, gpa, val, bytes) function
>> that takes care of calling kvm_vcpu_mark_page_dirty, kvm_mmu_pte_write
>> and kvm_page_track-write?
>>
> 
> After this patchset, kvm_mmu_pte_write is only a static notifier
> callback called
> by kvm_page_track_write().
> 
> And the dirty tracking in emulator_write_phys() is handled in a public
> API (as my
> explanation above), in emulator_cmpxchg_emulated is handled by itself.
> So i think
> it is better to leaving dirty tracking to the separate paths, no? :)

... and here it is indeed better to leave things as they are in v3.  Thanks,

Paolo

[toc] | [prev] | [next] | [standalone]

#1333494 — [PATCH v3 04/11] KVM: page track: add the framework of guest page tracking

The array, gfn_track[mode][gfn], is introduced in memory slot for every
guest page, this is the tracking count for the gust page on different
modes. If the page is tracked then the count is increased, the page is
not tracked after the count reaches zero

We use 'unsigned short' as the tracking count which should be enough as
shadow page table only can use 2^14 (2^3 for level, 2^1 for cr4_pae, 2^2
for quadrant, 2^3 for access, 2^1 for nxe, 2^1 for cr0_wp, 2^1 for
smep_andnot_wp, 2^1 for smap_andnot_wp, and 2^1 for smm) at most, there
is enough room for other trackers

Two callbacks, kvm_page_track_create_memslot() and
kvm_page_track_free_memslot() are implemented in this patch, they are
internally used to initialize and reclaim the memory of the array

Currently, only write track mode is supported

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h       |  2 ++
 arch/x86/include/asm/kvm_page_track.h | 13 +++++++++
 arch/x86/kvm/Makefile                 |  3 +-
 arch/x86/kvm/page_track.c             | 52 +++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c                    |  5 ++++
 5 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/include/asm/kvm_page_track.h
 create mode 100644 arch/x86/kvm/page_track.c

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e1c1f57..d8931d0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -32,6 +32,7 @@
 #include <asm/mtrr.h>
 #include <asm/msr-index.h>
 #include <asm/asm.h>
+#include <asm/kvm_page_track.h>
 
 #define KVM_MAX_VCPUS 255
 #define KVM_SOFT_MAX_VCPUS 160
@@ -650,6 +651,7 @@ struct kvm_lpage_info {
 struct kvm_arch_memory_slot {
 	struct kvm_rmap_head *rmap[KVM_NR_PAGE_SIZES];
 	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
+	unsigned short *gfn_track[KVM_PAGE_TRACK_MAX];
 };
 
 /*
diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
new file mode 100644
index 0000000..55200406
--- /dev/null
+++ b/arch/x86/include/asm/kvm_page_track.h
@@ -0,0 +1,13 @@
+#ifndef _ASM_X86_KVM_PAGE_TRACK_H
+#define _ASM_X86_KVM_PAGE_TRACK_H
+
+enum kvm_page_track_mode {
+	KVM_PAGE_TRACK_WRITE,
+	KVM_PAGE_TRACK_MAX,
+};
+
+void kvm_page_track_free_memslot(struct kvm_memory_slot *free,
+				 struct kvm_memory_slot *dont);
+int kvm_page_track_create_memslot(struct kvm_memory_slot *slot,
+				  unsigned long npages);
+#endif
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index a1ff508..464fa47 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -13,9 +13,10 @@ kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
 
 kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
 			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
-			   hyperv.o
+			   hyperv.o page_track.o
 
 kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
+
 kvm-intel-y		+= vmx.o pmu_intel.o
 kvm-amd-y		+= svm.o pmu_amd.o
 
diff --git a/arch/x86/kvm/page_track.c b/arch/x86/kvm/page_track.c
new file mode 100644
index 0000000..8c396d0
--- /dev/null
+++ b/arch/x86/kvm/page_track.c
@@ -0,0 +1,52 @@
+/*
+ * Support KVM gust page tracking
+ *
+ * This feature allows us to track page access in guest. Currently, only
+ * write access is tracked.
+ *
+ * Copyright(C) 2015 Intel Corporation.
+ *
+ * Author:
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <linux/kvm_host.h>
+#include <asm/kvm_host.h>
+#include <asm/kvm_page_track.h>
+
+#include "mmu.h"
+
+void kvm_page_track_free_memslot(struct kvm_memory_slot *free,
+				 struct kvm_memory_slot *dont)
+{
+	int i;
+
+	for (i = 0; i < KVM_PAGE_TRACK_MAX; i++)
+		if (!dont || free->arch.gfn_track[i] !=
+		      dont->arch.gfn_track[i]) {
+			kvfree(free->arch.gfn_track[i]);
+			free->arch.gfn_track[i] = NULL;
+		}
+}
+
+int kvm_page_track_create_memslot(struct kvm_memory_slot *slot,
+				  unsigned long npages)
+{
+	int  i;
+
+	for (i = 0; i < KVM_PAGE_TRACK_MAX; i++) {
+		slot->arch.gfn_track[i] = kvm_kvzalloc(npages *
+					    sizeof(*slot->arch.gfn_track[i]));
+		if (!slot->arch.gfn_track[i])
+			goto track_free;
+	}
+
+	return 0;
+
+track_free:
+	kvm_page_track_free_memslot(slot, NULL);
+	return -ENOMEM;
+}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f448e64..e25ebb7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7895,6 +7895,8 @@ void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free,
 			free->arch.lpage_info[i - 1] = NULL;
 		}
 	}
+
+	kvm_page_track_free_memslot(free, dont);
 }
 
 int kvm_arch_create_memslot(struct kvm *kvm, struct kvm_memory_slot *slot,
@@ -7943,6 +7945,9 @@ int kvm_arch_create_memslot(struct kvm *kvm, struct kvm_memory_slot *slot,
 		}
 	}
 
+	if (kvm_page_track_create_memslot(slot, npages))
+		goto out_free;
+
 	return 0;
 
 out_free:
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1338062 — Re: [PATCH v3 04/11] KVM: page track: add the framework of guest page tracking

On 14/02/2016 12:31, Xiao Guangrong wrote:
> The array, gfn_track[mode][gfn], is introduced in memory slot for every
> guest page, this is the tracking count for the gust page on different
> modes. If the page is tracked then the count is increased, the page is
> not tracked after the count reaches zero
> 
> We use 'unsigned short' as the tracking count which should be enough as
> shadow page table only can use 2^14 (2^3 for level, 2^1 for cr4_pae, 2^2
> for quadrant, 2^3 for access, 2^1 for nxe, 2^1 for cr0_wp, 2^1 for
> smep_andnot_wp, 2^1 for smap_andnot_wp, and 2^1 for smm) at most, there
> is enough room for other trackers
> 
> Two callbacks, kvm_page_track_create_memslot() and
> kvm_page_track_free_memslot() are implemented in this patch, they are
> internally used to initialize and reclaim the memory of the array
> 
> Currently, only write track mode is supported
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h       |  2 ++
>  arch/x86/include/asm/kvm_page_track.h | 13 +++++++++
>  arch/x86/kvm/Makefile                 |  3 +-
>  arch/x86/kvm/page_track.c             | 52 +++++++++++++++++++++++++++++++++++
>  arch/x86/kvm/x86.c                    |  5 ++++
>  5 files changed, 74 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/include/asm/kvm_page_track.h
>  create mode 100644 arch/x86/kvm/page_track.c
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e1c1f57..d8931d0 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -32,6 +32,7 @@
>  #include <asm/mtrr.h>
>  #include <asm/msr-index.h>
>  #include <asm/asm.h>
> +#include <asm/kvm_page_track.h>
>  
>  #define KVM_MAX_VCPUS 255
>  #define KVM_SOFT_MAX_VCPUS 160
> @@ -650,6 +651,7 @@ struct kvm_lpage_info {
>  struct kvm_arch_memory_slot {
>  	struct kvm_rmap_head *rmap[KVM_NR_PAGE_SIZES];
>  	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
> +	unsigned short *gfn_track[KVM_PAGE_TRACK_MAX];

Please add a comment at struct kvm_mmu_page_role mentioning that the
number of role bits for shadow pages (i.e. not counting direct and
invalid) must not exceed 15 (16 thoretically risks overflow already!),
and counting the 14 bits that are in use.

Paolo

>  };
>  
>  /*
> diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
> new file mode 100644
> index 0000000..55200406
> --- /dev/null
> +++ b/arch/x86/include/asm/kvm_page_track.h
> @@ -0,0 +1,13 @@
> +#ifndef _ASM_X86_KVM_PAGE_TRACK_H
> +#define _ASM_X86_KVM_PAGE_TRACK_H
> +
> +enum kvm_page_track_mode {
> +	KVM_PAGE_TRACK_WRITE,
> +	KVM_PAGE_TRACK_MAX,
> +};
> +
> +void kvm_page_track_free_memslot(struct kvm_memory_slot *free,
> +				 struct kvm_memory_slot *dont);
> +int kvm_page_track_create_memslot(struct kvm_memory_slot *slot,
> +				  unsigned long npages);
> +#endif
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index a1ff508..464fa47 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -13,9 +13,10 @@ kvm-$(CONFIG_KVM_ASYNC_PF)	+= $(KVM)/async_pf.o
>  
>  kvm-y			+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
>  			   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
> -			   hyperv.o
> +			   hyperv.o page_track.o
>  
>  kvm-$(CONFIG_KVM_DEVICE_ASSIGNMENT)	+= assigned-dev.o iommu.o
> +
>  kvm-intel-y		+= vmx.o pmu_intel.o
>  kvm-amd-y		+= svm.o pmu_amd.o
>  
> diff --git a/arch/x86/kvm/page_track.c b/arch/x86/kvm/page_track.c
> new file mode 100644
> index 0000000..8c396d0
> --- /dev/null
> +++ b/arch/x86/kvm/page_track.c
> @@ -0,0 +1,52 @@
> +/*
> + * Support KVM gust page tracking
> + *
> + * This feature allows us to track page access in guest. Currently, only
> + * write access is tracked.
> + *
> + * Copyright(C) 2015 Intel Corporation.
> + *
> + * Author:
> + *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_host.h>
> +#include <asm/kvm_page_track.h>
> +
> +#include "mmu.h"
> +
> +void kvm_page_track_free_memslot(struct kvm_memory_slot *free,
> +				 struct kvm_memory_slot *dont)
> +{
> +	int i;
> +
> +	for (i = 0; i < KVM_PAGE_TRACK_MAX; i++)
> +		if (!dont || free->arch.gfn_track[i] !=
> +		      dont->arch.gfn_track[i]) {
> +			kvfree(free->arch.gfn_track[i]);
> +			free->arch.gfn_track[i] = NULL;
> +		}
> +}
> +
> +int kvm_page_track_create_memslot(struct kvm_memory_slot *slot,
> +				  unsigned long npages)
> +{
> +	int  i;
> +
> +	for (i = 0; i < KVM_PAGE_TRACK_MAX; i++) {
> +		slot->arch.gfn_track[i] = kvm_kvzalloc(npages *
> +					    sizeof(*slot->arch.gfn_track[i]));
> +		if (!slot->arch.gfn_track[i])
> +			goto track_free;
> +	}
> +
> +	return 0;
> +
> +track_free:
> +	kvm_page_track_free_memslot(slot, NULL);
> +	return -ENOMEM;
> +}
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f448e64..e25ebb7 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7895,6 +7895,8 @@ void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free,
>  			free->arch.lpage_info[i - 1] = NULL;
>  		}
>  	}
> +
> +	kvm_page_track_free_memslot(free, dont);
>  }
>  
>  int kvm_arch_create_memslot(struct kvm *kvm, struct kvm_memory_slot *slot,
> @@ -7943,6 +7945,9 @@ int kvm_arch_create_memslot(struct kvm *kvm, struct kvm_memory_slot *slot,
>  		}
>  	}
>  
> +	if (kvm_page_track_create_memslot(slot, npages))
> +		goto out_free;
> +
>  	return 0;
>  
>  out_free:
> 

[toc] | [prev] | [next] | [standalone]

#1340247 — Re: [PATCH v3 04/11] KVM: page track: add the framework of guest page tracking

On 02/19/2016 07:24 PM, Paolo Bonzini wrote:
>
>
> On 14/02/2016 12:31, Xiao Guangrong wrote:

>>   #define KVM_MAX_VCPUS 255
>>   #define KVM_SOFT_MAX_VCPUS 160
>> @@ -650,6 +651,7 @@ struct kvm_lpage_info {
>>   struct kvm_arch_memory_slot {
>>   	struct kvm_rmap_head *rmap[KVM_NR_PAGE_SIZES];
>>   	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
>> +	unsigned short *gfn_track[KVM_PAGE_TRACK_MAX];
>
> Please add a comment at struct kvm_mmu_page_role mentioning that the
> number of role bits for shadow pages (i.e. not counting direct and
> invalid) must not exceed 15 (16 thoretically risks overflow already!),
> and counting the 14 bits that are in use.
>

Okay, good idea, it can avoid the potential issue in the future development,
will do it in the next version.

[toc] | [prev] | [next] | [standalone]

#1333495 — [PATCH v3 03/11] KVM: MMU: introduce kvm_mmu_slot_gfn_write_protect

Split rmap_write_protect() and introduce the function to abstract the write
protection based on the slot

This function will be used in the later patch

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mmu.c | 16 +++++++++++-----
 arch/x86/kvm/mmu.h |  2 ++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index e1bb66c..edad3c7 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1336,23 +1336,29 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
 		kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
 }
 
-static bool rmap_write_protect(struct kvm_vcpu *vcpu, u64 gfn)
+bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
+				    struct kvm_memory_slot *slot, u64 gfn)
 {
-	struct kvm_memory_slot *slot;
 	struct kvm_rmap_head *rmap_head;
 	int i;
 	bool write_protected = false;
 
-	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
-
 	for (i = PT_PAGE_TABLE_LEVEL; i <= PT_MAX_HUGEPAGE_LEVEL; ++i) {
 		rmap_head = __gfn_to_rmap(gfn, i, slot);
-		write_protected |= __rmap_write_protect(vcpu->kvm, rmap_head, true);
+		write_protected |= __rmap_write_protect(kvm, rmap_head, true);
 	}
 
 	return write_protected;
 }
 
+static bool rmap_write_protect(struct kvm_vcpu *vcpu, u64 gfn)
+{
+	struct kvm_memory_slot *slot;
+
+	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
+	return kvm_mmu_slot_gfn_write_protect(vcpu->kvm, slot, gfn);
+}
+
 static bool kvm_zap_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head)
 {
 	u64 *sptep;
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index de92bed..58fe98a 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -177,4 +177,6 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end);
 
 void kvm_mmu_gfn_disallow_lpage(struct kvm_memory_slot *slot, gfn_t gfn);
 void kvm_mmu_gfn_allow_lpage(struct kvm_memory_slot *slot, gfn_t gfn);
+bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
+				    struct kvm_memory_slot *slot, u64 gfn);
 #endif
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1338130

On 14/02/2016 12:31, Xiao Guangrong wrote:
> Changelong in v3:
> - refine the code of mmu_need_write_protect() based on Huang Kai's suggestion
> - rebase the patchset against current code
> 
> Changelog in v2:
> - fix a issue that the track memory of memslot is freed if we only move
>   the memslot or change the flags of memslot
> - do not track the gfn which is not mapped in memslots
> - introduce the nolock APIs at the begin of the patchset
> - use 'unsigned short' as the track counter to reduce the memory and which
>   should be enough for shadow page table and KVMGT
> 
> This patchset introduces the feature which allows us to track page
> access in guest. Currently, only write access tracking is implemented
> in this version.
> 
> Four APIs are introduces:
> - kvm_page_track_add_page(kvm, gfn, mode), single guest page @gfn is
>   added into the track pool of the guest instance represented by @kvm,
>   @mode specifies which kind of access on the @gfn is tracked
>   
> - kvm_page_track_remove_page(kvm, gfn, mode), is the opposed operation
>   of kvm_page_track_add_page() which removes @gfn from the tracking pool.
>   gfn is no tracked after its last user is gone
> 
> - kvm_page_track_register_notifier(kvm, n), register a notifier so that
>   the event triggered by page tracking will be received, at that time,
>   the callback of n->track_write() will be called
> 
> - kvm_page_track_unregister_notifier(kvm, n), does the opposed operation
>   of kvm_page_track_register_notifier(), which unlinks the notifier and
>   stops receiving the tracked event
> 
> The first user of page track is non-leaf shadow page tables as they are
> always write protected. It also gains performance improvement because
> page track speeds up page fault handler for the tracked pages. The
> performance result of kernel building is as followings:
> 
>    before           after
> real 461.63       real 455.48
> user 4529.55      user 4557.88
> sys 1995.39       sys 1922.57
> 
> Furthermore, it is the infrastructure of other kind of shadow page table,
> such as GPU shadow page table introduced in KVMGT (1) and native nested
> IOMMU.
> 
> This patch can be divided into two parts:
> - patch 1 ~ patch 7, implement page tracking
> - others patches apply page tracking to non-leaf shadow page table

Xiao,

the patches are very readable and very good.  My comments are only minor.

I still have a doubt: how are you going to handle invalidation of GPU
shadow page tables if a device (emulated in QEMU or even vhost) does DMA
to the PPGTT?  Generally, this was the reason to keep stuff out of KVM
and instead hook into the kernel mm subsystem (as with userfaultfd).

Paolo

[toc] | [prev] | [next] | [standalone]

#1339319

On 02/19/2016 08:00 PM, Paolo Bonzini wrote:
>
>
> On 14/02/2016 12:31, Xiao Guangrong wrote:
>> Changelong in v3:
>> - refine the code of mmu_need_write_protect() based on Huang Kai's suggestion
>> - rebase the patchset against current code
>>
>> Changelog in v2:
>> - fix a issue that the track memory of memslot is freed if we only move
>>    the memslot or change the flags of memslot
>> - do not track the gfn which is not mapped in memslots
>> - introduce the nolock APIs at the begin of the patchset
>> - use 'unsigned short' as the track counter to reduce the memory and which
>>    should be enough for shadow page table and KVMGT
>>
>> This patchset introduces the feature which allows us to track page
>> access in guest. Currently, only write access tracking is implemented
>> in this version.
>>
>> Four APIs are introduces:
>> - kvm_page_track_add_page(kvm, gfn, mode), single guest page @gfn is
>>    added into the track pool of the guest instance represented by @kvm,
>>    @mode specifies which kind of access on the @gfn is tracked
>>
>> - kvm_page_track_remove_page(kvm, gfn, mode), is the opposed operation
>>    of kvm_page_track_add_page() which removes @gfn from the tracking pool.
>>    gfn is no tracked after its last user is gone
>>
>> - kvm_page_track_register_notifier(kvm, n), register a notifier so that
>>    the event triggered by page tracking will be received, at that time,
>>    the callback of n->track_write() will be called
>>
>> - kvm_page_track_unregister_notifier(kvm, n), does the opposed operation
>>    of kvm_page_track_register_notifier(), which unlinks the notifier and
>>    stops receiving the tracked event
>>
>> The first user of page track is non-leaf shadow page tables as they are
>> always write protected. It also gains performance improvement because
>> page track speeds up page fault handler for the tracked pages. The
>> performance result of kernel building is as followings:
>>
>>     before           after
>> real 461.63       real 455.48
>> user 4529.55      user 4557.88
>> sys 1995.39       sys 1922.57
>>
>> Furthermore, it is the infrastructure of other kind of shadow page table,
>> such as GPU shadow page table introduced in KVMGT (1) and native nested
>> IOMMU.
>>
>> This patch can be divided into two parts:
>> - patch 1 ~ patch 7, implement page tracking
>> - others patches apply page tracking to non-leaf shadow page table
>
> Xiao,
>
> the patches are very readable and very good.  My comments are only minor.

Thank you, Paolo!

>
> I still have a doubt: how are you going to handle invalidation of GPU
> shadow page tables if a device (emulated in QEMU or even vhost) does DMA
> to the PPGTT?

I think Jike is the better one to answer this question, Jike, could you
please clarify it? :)

> Generally, this was the reason to keep stuff out of KVM
> and instead hook into the kernel mm subsystem (as with userfaultfd).

We considered it carefully but this way can not satisfy KVMGT's requirements.
The reasons i explained in the old thread (https://lkml.org/lkml/2015/12/1/516)
are:

"For the performance, shadow GPU is performance critical and requires
frequently being switched, it is not good to handle it in userspace. And
windows guest has many GPU tables and updates it frequently, that means,
we need to write protect huge number of pages which are single page based,
I am afraid userfaultfd can not handle this case efficiently.

For the functionality, userfaultfd can not fill the need of shadow page
because:
- the page is keeping readonly, userfaultfd can not fix the fault and let
    the vcpu progress (write access causes writeable gup).

- the access need to be emulated, however, userfaultfd/kernel does not have
    the ability to emulate the access as the access is trigged by guest, the
    instruction info is stored in VMCS so that only KVM can emulate it.

- shadow page needs to be notified after the emulation is finished as it
    should know the new data written to the page to update its page hierarchy.
    (some hardwares lack the 'retry' ability so the shadow page table need to
     reflect the table in guest at any time). "

Any idea?

[toc] | [prev] | [next] | [standalone]

#1340197

+Kevin

On 02/22/2016 06:05 PM, Xiao Guangrong wrote:
> 
> On 02/19/2016 08:00 PM, Paolo Bonzini wrote:
>>
>> I still have a doubt: how are you going to handle invalidation of GPU
>> shadow page tables if a device (emulated in QEMU or even vhost) does DMA
>> to the PPGTT?
> 
> I think Jike is the better one to answer this question, Jike, could you
> please clarify it? :)
> 

Sure :)

Actually in guest PPGTT is manipulated by CPU rather than GPU. The
PPGTT page table itself are plain memory, composed & modified by the
GPU driver, i.e. by CPU in Non-Root mode.

Given that, we write-protected guest PPGTT, when VM writes PPGTT, EPT
violation rather than DMA fault happens.

>> Generally, this was the reason to keep stuff out of KVM
>> and instead hook into the kernel mm subsystem (as with userfaultfd).
> 
> We considered it carefully but this way can not satisfy KVMGT's requirements.
> The reasons i explained in the old thread (https://lkml.org/lkml/2015/12/1/516)
> are:
> 
> "For the performance, shadow GPU is performance critical and requires
> frequently being switched, it is not good to handle it in userspace. And
> windows guest has many GPU tables and updates it frequently, that means,
> we need to write protect huge number of pages which are single page based,
> I am afraid userfaultfd can not handle this case efficiently.
> 
> For the functionality, userfaultfd can not fill the need of shadow page
> because:
> - the page is keeping readonly, userfaultfd can not fix the fault and let
>     the vcpu progress (write access causes writeable gup).
> 
> - the access need to be emulated, however, userfaultfd/kernel does not have
>     the ability to emulate the access as the access is trigged by guest, the
>     instruction info is stored in VMCS so that only KVM can emulate it.
> 
> - shadow page needs to be notified after the emulation is finished as it
>     should know the new data written to the page to update its page hierarchy.
>     (some hardwares lack the 'retry' ability so the shadow page table need to
>      reflect the table in guest at any time). "
> 
> Any idea?
> 

--
Thanks,
Jike

[toc] | [prev] | [next] | [standalone]

#1340316

> From: Song, Jike
> Sent: Tuesday, February 23, 2016 11:02 AM
> 
> +Kevin
> 
> On 02/22/2016 06:05 PM, Xiao Guangrong wrote:
> >
> > On 02/19/2016 08:00 PM, Paolo Bonzini wrote:
> >>
> >> I still have a doubt: how are you going to handle invalidation of GPU
> >> shadow page tables if a device (emulated in QEMU or even vhost) does DMA
> >> to the PPGTT?
> >
> > I think Jike is the better one to answer this question, Jike, could you
> > please clarify it? :)
> >
> 
> Sure :)
> 
> Actually in guest PPGTT is manipulated by CPU rather than GPU. The
> PPGTT page table itself are plain memory, composed & modified by the
> GPU driver, i.e. by CPU in Non-Root mode.
> 
> Given that, we write-protected guest PPGTT, when VM writes PPGTT, EPT
> violation rather than DMA fault happens.

'DMA to PPGTT' is NOT SUPPORTED on our vGPU device model. Today
Intel gfx driver doesn't use this method, and we explicitly list it as a
guest driver requirement to support a vGPU. If a malicious driver does 
program DMA to modify PPGTT, it can only modify guest PPGTT instead
of shadow PPGTT (being guest invisible). So there is no security issue 
either.

> 
> >> Generally, this was the reason to keep stuff out of KVM
> >> and instead hook into the kernel mm subsystem (as with userfaultfd).
> >
> > We considered it carefully but this way can not satisfy KVMGT's requirements.
> > The reasons i explained in the old thread (https://lkml.org/lkml/2015/12/1/516)
> > are:
> >
> > "For the performance, shadow GPU is performance critical and requires
> > frequently being switched, it is not good to handle it in userspace. And
> > windows guest has many GPU tables and updates it frequently, that means,
> > we need to write protect huge number of pages which are single page based,
> > I am afraid userfaultfd can not handle this case efficiently.

Yes, performance is the main concern. 

Paolo, we explained the reason for in-kernel emulation to you earlier with
your understanding:
----
> > It's definitely a fast path, e.g. command submission, shadow GPU page
> > table, etc. which are all in performance critical path. Another reason is
> > the I/O access frequency, which could be up to 100k/s for some gfx workload.
> > It's important to shorten the emulation path which can help performance
> > a lot. That's the major reason why we keep vGPU device model in the
> > kernel (will merged into i915 driver)
> 
> Ok, thanks---writing numbers down always helps.  MMIO to userspace costs
> 5000 clock cycles on the latest QEMU and processor (and does not need
> the "big QEMU lock" anymore), but still 100k/s is a ~500000 clock cycle
> difference and approximately 15% host CPU usage.
----
(I believe ~500000 should be ~500M clock cycle above)

> >
> > For the functionality, userfaultfd can not fill the need of shadow page
> > because:
> > - the page is keeping readonly, userfaultfd can not fix the fault and let
> >     the vcpu progress (write access causes writeable gup).
> >
> > - the access need to be emulated, however, userfaultfd/kernel does not have
> >     the ability to emulate the access as the access is trigged by guest, the
> >     instruction info is stored in VMCS so that only KVM can emulate it.
> >
> > - shadow page needs to be notified after the emulation is finished as it
> >     should know the new data written to the page to update its page hierarchy.
> >     (some hardwares lack the 'retry' ability so the shadow page table need to
> >      reflect the table in guest at any time). "
> >
> > Any idea?
> >
> 

Thanks Guangrong for investigating the possibility.

Based on earlier explanation, we hope KVM community can re-think the
necessity of support in-kernel emulation for KVMGT. Same framework
might be extended to other type of I/O devices using similar mediated
pass-through concept in the future, which has device model tightly
integrated with native device driver for efficiency and simplicity purpose.

Actually a related open when discussing KVMGT/VFIO integration.
There are 7 total services required to support in-kernel emulation, which 
can be categorize into two groups:

a) services to connect vGPU with VM, which are essentially what a device
driver is doing (so VFIO can fit here), including:
	1) Selectively pass-through a region to a VM
	2) Trap-and-emulate a region
	3) Inject a virtual interrupt
	4) Pin/unpin guest memory
	5) GPA->IOVA/HVA translation (as a side-effect)

b) services to support device emulation, which gonna be hypervisor
specific, including:
	6) Map/unmap guest memory
	7) Write-protect a guest memory page

We're working with VFIO community to add support of category a),
but there is still a gap in category b). This patch series can address
the requirement of 7). For 6) it's straightforward for KVM. We may
introduce a new file in KVM to wrap them together for in-kernel
emulation, but need an agreement from community first on this
direction. :-)

Thanks
Kevin

[toc] | [prev] | [next] | [standalone]

#1340574

On 23/02/2016 06:44, Tian, Kevin wrote:
>> From: Song, Jike
>> Sent: Tuesday, February 23, 2016 11:02 AM
>>
>> +Kevin
>>
>> On 02/22/2016 06:05 PM, Xiao Guangrong wrote:
>>>
>>> On 02/19/2016 08:00 PM, Paolo Bonzini wrote:
>>>>
>>>> I still have a doubt: how are you going to handle invalidation of GPU
>>>> shadow page tables if a device (emulated in QEMU or even vhost) does DMA
>>>> to the PPGTT?
>>>
>>> I think Jike is the better one to answer this question, Jike, could you
>>> please clarify it? :)
>>>
>>
>> Sure :)
>>
>> Actually in guest PPGTT is manipulated by CPU rather than GPU. The
>> PPGTT page table itself are plain memory, composed & modified by the
>> GPU driver, i.e. by CPU in Non-Root mode.
>>
>> Given that, we write-protected guest PPGTT, when VM writes PPGTT, EPT
>> violation rather than DMA fault happens.
> 
> 'DMA to PPGTT' is NOT SUPPORTED on our vGPU device model. Today
> Intel gfx driver doesn't use this method, and we explicitly list it as a
> guest driver requirement to support a vGPU. If a malicious driver does 
> program DMA to modify PPGTT, it can only modify guest PPGTT instead
> of shadow PPGTT (being guest invisible). So there is no security issue 
> either.

Ok, thanks for confirming.

Paolo

[toc] | [prev] | [next] | [standalone]

#1340491

----- Original Message -----
> From: "Jike Song" <jike.song@intel.com>
> To: "Xiao Guangrong" <guangrong.xiao@linux.intel.com>
> Cc: "Paolo Bonzini" <pbonzini@redhat.com>, gleb@kernel.org, mtosatti@redhat.com, kvm@vger.kernel.org,
> linux-kernel@vger.kernel.org, "kai huang" <kai.huang@linux.intel.com>, "Andrea Arcangeli" <aarcange@redhat.com>,
> "Kevin Tian" <kevin.tian@intel.com>
> Sent: Tuesday, February 23, 2016 4:02:25 AM
> Subject: Re: [PATCH v3 00/11] KVM: x86: track guest page access
> 
> +Kevin
> 
> On 02/22/2016 06:05 PM, Xiao Guangrong wrote:
> > 
> > On 02/19/2016 08:00 PM, Paolo Bonzini wrote:
> >>
> >> I still have a doubt: how are you going to handle invalidation of GPU
> >> shadow page tables if a device (emulated in QEMU or even vhost) does DMA
> >> to the PPGTT?
> > 
> > I think Jike is the better one to answer this question, Jike, could you
> > please clarify it? :)
> > 
> 
> Sure :)
> 
> Actually in guest PPGTT is manipulated by CPU rather than GPU. The
> PPGTT page table itself are plain memory, composed & modified by the
> GPU driver, i.e. by CPU in Non-Root mode.
> 
> Given that, we write-protected guest PPGTT, when VM writes PPGTT, EPT
> violation rather than DMA fault happens.

I am not talking of DMA faults; I am talking of a guest that reads
from disk into the PPGTT.  This is emulated DMA, and your approach of
tracking guest page access from KVM means that you are not handling
this.  Is this right?  If so, what happens if the guest does this
kind of operation (for example because it is not using the PPGTT
anymore)?  KVMGT should not be confused the next time it works on
that PPGTT page.

Paolo

[toc] | [prev] | [next] | [standalone]

#1340565

On 02/23/2016 06:01 PM, Paolo Bonzini wrote:
> ----- Original Message -----
>> From: "Jike Song" <jike.song@intel.com>
>> To: "Xiao Guangrong" <guangrong.xiao@linux.intel.com>
>> Cc: "Paolo Bonzini" <pbonzini@redhat.com>, gleb@kernel.org, mtosatti@redhat.com, kvm@vger.kernel.org,
>> linux-kernel@vger.kernel.org, "kai huang" <kai.huang@linux.intel.com>, "Andrea Arcangeli" <aarcange@redhat.com>,
>> "Kevin Tian" <kevin.tian@intel.com>
>> Sent: Tuesday, February 23, 2016 4:02:25 AM
>> Subject: Re: [PATCH v3 00/11] KVM: x86: track guest page access
>>
>> +Kevin
>>
>> On 02/22/2016 06:05 PM, Xiao Guangrong wrote:
>>>
>>> On 02/19/2016 08:00 PM, Paolo Bonzini wrote:
>>>>
>>>> I still have a doubt: how are you going to handle invalidation of GPU
>>>> shadow page tables if a device (emulated in QEMU or even vhost) does DMA
>>>> to the PPGTT?
>>>
>>> I think Jike is the better one to answer this question, Jike, could you
>>> please clarify it? :)
>>>
>>
>> Sure :)
>>
>> Actually in guest PPGTT is manipulated by CPU rather than GPU. The
>> PPGTT page table itself are plain memory, composed & modified by the
>> GPU driver, i.e. by CPU in Non-Root mode.
>>
>> Given that, we write-protected guest PPGTT, when VM writes PPGTT, EPT
>> violation rather than DMA fault happens.
> 

I may still misunderstand you, so apologize in advance ..

> I am not talking of DMA faults; I am talking of a guest that reads
> from disk into the PPGTT.

into PPGTT the page table itself? as said by Kevin in another mail,
this is NOT SUPPORTED.

> This is emulated DMA, and your approach of
> tracking guest page access from KVM means that you are not handling
> this.  Is this right?

Right, our tacking mechanism cares only CPU write, not Device write.

However, there is *NO* DMA emulation, just similar to passthrough.
The device(IGD) is only cable of r/w memory according the
shadowed PPGTT, which is managed by VGPU device-model, guaranteed
only memory that owned by this vgpu can be mapped. 

All we need is to track CPU writes from guest.

> If so, what happens if the guest does this
> kind of operation (for example because it is not using the PPGTT
> anymore)?  KVMGT should not be confused the next time it works on
> that PPGTT page.

As explained above, the device-model won't allow such things to happen.

> 
> Paolo
>

--
Thanks,
Jike

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web