Re: [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot

[Neo Jia <cjia@nvidia.com>]

On Mon, Oct 10, 2016 at 07:06:27PM +0200, Paolo Bonzini wrote:
> 
> 
> On 09/10/2016 10:31, Neo Jia wrote:
> > On Sun, Oct 09, 2016 at 03:41:43PM +0800, Xiaoguang Chen wrote:
> >> When a memory slot is being moved or removed users of page track
> >> can be notified. So users can drop write-protection for the pages
> >> in that memory slot.
> >>
> >> This notifier type is needed by KVMGT to sync up its shadow page
> >> table when memory slot is being moved or removed.
> > 
> > Hi Xiaoguang,
> > 
> > How is this supposed to be used by the kvmgt?
> 
> Hi Neo,
> 
> AFAIK this is needed because KVMGT doesn't paravirtualize the PPGTT,
> while nVidia does.

> "From Xiaoguang:

> This is related to KVMGT device model.
> For KVMGT some ram will be allocated and used as ppgtt page table and
> the page table is write protected.
> We use page track to implement write protect. But get problem while
> memslot is moved or removed.
> We must clear the write protect for the pages in the memslot or there
> will be errors while ppgtt do cleanup.
> So KVMGT must register the track_flush_slot to get notified while
> memslot is moved or removed."

(merging thread)

Hi Paolo and Xiaoguang,

I am just wondering how device driver can register a notifier so he can be 
notified for write-protected pages when writes are happening.

Thanks,
Neo

> 
> Paolo
> 
> > Thanks,
> > Neo
> > 
> >>
> >> Reviewed-by: Xiao Guangrong <guangrong.xiao@intel.com>
> >> Signed-off-by: Chen Xiaoguang <xiaoguang.chen@intel.com>
> >> ---
> >>  arch/x86/include/asm/kvm_page_track.h |  9 +++++++++
> >>  arch/x86/kvm/page_track.c             | 25 +++++++++++++++++++++++++
> >>  arch/x86/kvm/x86.c                    |  2 +-
> >>  3 files changed, 35 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
> >> index c2b8d24..5f66597 100644
> >> --- a/arch/x86/include/asm/kvm_page_track.h
> >> +++ b/arch/x86/include/asm/kvm_page_track.h
> >> @@ -32,6 +32,14 @@ struct kvm_page_track_notifier_node {
> >>  	 */
> >>  	void (*track_write)(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
> >>  			    int bytes);
> >> +	/*
> >> +	 * It is called when memory slot is being moved or removed
> >> +	 * users can drop write-protection for the pages in that memory slot
> >> +	 *
> >> +	 * @kvm: the kvm where memory slot being moved or removed
> >> +	 * @slot: the memory slot being moved or removed
> >> +	 */
> >> +	void (*track_flush_slot)(struct kvm *kvm, struct kvm_memory_slot *slot);
> >>  };
> >>  
> >>  void kvm_page_track_init(struct kvm *kvm);
> >> @@ -58,4 +66,5 @@ 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);
> >> +void kvm_page_track_flush_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
> >>  #endif
> >> diff --git a/arch/x86/kvm/page_track.c b/arch/x86/kvm/page_track.c
> >> index b431539..e79bb25 100644
> >> --- a/arch/x86/kvm/page_track.c
> >> +++ b/arch/x86/kvm/page_track.c
> >> @@ -225,3 +225,28 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
> >>  			n->track_write(vcpu, gpa, new, bytes);
> >>  	srcu_read_unlock(&head->track_srcu, idx);
> >>  }
> >> +
> >> +/*
> >> + * Notify the node that memory slot is being removed or moved so that it can
> >> + * drop write-protection for the pages in the memory slot.
> >> + *
> >> + * The node should figure out it has any write-protected pages in this slot
> >> + * by itself.
> >> + */
> >> +void kvm_page_track_flush_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
> >> +{
> >> +	struct kvm_page_track_notifier_head *head;
> >> +	struct kvm_page_track_notifier_node *n;
> >> +	int idx;
> >> +
> >> +	head = &kvm->arch.track_notifier_head;
> >> +
> >> +	if (hlist_empty(&head->track_notifier_list))
> >> +		return;
> >> +
> >> +	idx = srcu_read_lock(&head->track_srcu);
> >> +	hlist_for_each_entry_rcu(n, &head->track_notifier_list, node)
> >> +		if (n->track_flush_slot)
> >> +			n->track_flush_slot(kvm, slot);
> >> +	srcu_read_unlock(&head->track_srcu, idx);
> >> +}
> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> index 87f5dbb..f8ae90c 100644
> >> --- a/arch/x86/kvm/x86.c
> >> +++ b/arch/x86/kvm/x86.c
> >> @@ -8278,7 +8278,7 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
> >>  void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
> >>  				   struct kvm_memory_slot *slot)
> >>  {
> >> -	kvm_mmu_invalidate_zap_all_pages(kvm);
> >> +	kvm_page_track_flush_slot(kvm, slot);
> >>  }
> >>  
> >>  static inline bool kvm_vcpu_has_events(struct kvm_vcpu *vcpu)
> >> -- 
> >> 1.9.1
> >>
> >> --
> >> 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
> --
> 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