Re: [patch 4/4] KVM: MMU: pinned sps are not candidates for deletion.

2014-09-22 Thread Marcelo Tosatti
On Tue, Sep 09, 2014 at 12:41:27PM -0300, Marcelo Tosatti wrote:
 On Tue, Jul 22, 2014 at 05:59:42AM +0800, Xiao Guangrong wrote:
  
  On Jul 10, 2014, at 3:12 AM, mtosa...@redhat.com wrote:
  
   Skip pinned shadow pages when selecting pages to zap.
  
  It seems there is no way to prevent changing pinned spte on
  zap-all path?
 
 Xiao,
 
 The way would be to reload remote mmus, forcing the vcpu to exit,
 zapping a page, then vcpu will pagefault any necessary page via
 kvm_mmu_pin_pages.
 
 kvm_mmu_invalidate_zap_all_pages does: 
 
 - spin_lock(mmu_lock)
 - kvm_reload_remote_mmus
 ...
 - spin_unlock(mmu_lock)
 
 So its OK to change pinned spte on zap all path.
 
  I am thing if we could move pinned spte to another list (eg. 
  pinned_shadow_pages)
  instead of active list so that it can not be touched by any other free 
  paths. 
  Your idea?
 
 As mentioned it above, it is ok to zap pinned sptes as long w 
 reload remote mmus request is performed.
 
 That said, you still consider a separate list?
 

Xiao, ping?

--
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 4/4] KVM: MMU: pinned sps are not candidates for deletion.

2014-09-22 Thread Xiao Guangrong

Hi Marcelo,

Sorry for the delay.

On Sep 9, 2014, at 11:41 PM, Marcelo Tosatti mtosa...@redhat.com wrote:

 On Tue, Jul 22, 2014 at 05:59:42AM +0800, Xiao Guangrong wrote:
 
 On Jul 10, 2014, at 3:12 AM, mtosa...@redhat.com wrote:
 
 Skip pinned shadow pages when selecting pages to zap.
 
 It seems there is no way to prevent changing pinned spte on
 zap-all path?
 
 Xiao,
 
 The way would be to reload remote mmus, forcing the vcpu to exit,
 zapping a page, then vcpu will pagefault any necessary page via
 kvm_mmu_pin_pages.
 
 kvm_mmu_invalidate_zap_all_pages does: 
 
 - spin_lock(mmu_lock)
 - kvm_reload_remote_mmus
 ...
 - spin_unlock(mmu_lock)
 
 So its OK to change pinned spte on zap all path.

Got it, thanks for your explanation.

 
 I am thing if we could move pinned spte to another list (eg. 
 pinned_shadow_pages)
 instead of active list so that it can not be touched by any other free 
 paths. 
 Your idea?
 
 As mentioned it above, it is ok to zap pinned sptes as long w 
 reload remote mmus request is performed.
 
 That said, you still consider a separate list?

I need to think more about this idea… let’s address it as your patch first. :)


--
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 4/4] KVM: MMU: pinned sps are not candidates for deletion.

2014-09-09 Thread Marcelo Tosatti
On Tue, Jul 22, 2014 at 05:59:42AM +0800, Xiao Guangrong wrote:
 
 On Jul 10, 2014, at 3:12 AM, mtosa...@redhat.com wrote:
 
  Skip pinned shadow pages when selecting pages to zap.
 
 It seems there is no way to prevent changing pinned spte on
 zap-all path?

Xiao,

The way would be to reload remote mmus, forcing the vcpu to exit,
zapping a page, then vcpu will pagefault any necessary page via
kvm_mmu_pin_pages.

kvm_mmu_invalidate_zap_all_pages does: 

- spin_lock(mmu_lock)
- kvm_reload_remote_mmus
...
- spin_unlock(mmu_lock)

So its OK to change pinned spte on zap all path.

 I am thing if we could move pinned spte to another list (eg. 
 pinned_shadow_pages)
 instead of active list so that it can not be touched by any other free paths. 
 Your idea?

As mentioned it above, it is ok to zap pinned sptes as long w 
reload remote mmus request is performed.

That said, you still consider a separate list?

--
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 4/4] KVM: MMU: pinned sps are not candidates for deletion.

2014-07-21 Thread Xiao Guangrong

On Jul 10, 2014, at 3:12 AM, mtosa...@redhat.com wrote:

 Skip pinned shadow pages when selecting pages to zap.

It seems there is no way to prevent changing pinned spte on
zap-all path?

I am thing if we could move pinned spte to another list (eg. 
pinned_shadow_pages)
instead of active list so that it can not be touched by any other free paths. 
Your idea?


--
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


[patch 4/4] KVM: MMU: pinned sps are not candidates for deletion.

2014-07-09 Thread mtosatti
Skip pinned shadow pages when selecting pages to zap.

Signed-off-by: Marcelo Tosatti mtosa...@redhat.com

---
 arch/x86/kvm/mmu.c |   26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

Index: kvm.pinned-sptes/arch/x86/kvm/mmu.c
===
--- kvm.pinned-sptes.orig/arch/x86/kvm/mmu.c2014-07-09 12:09:26.433674438 
-0300
+++ kvm.pinned-sptes/arch/x86/kvm/mmu.c 2014-07-09 12:09:27.164672860 -0300
@@ -2267,16 +2267,24 @@
 static bool prepare_zap_oldest_mmu_page(struct kvm *kvm,
struct list_head *invalid_list)
 {
-   struct kvm_mmu_page *sp;
-
-   if (list_empty(kvm-arch.active_mmu_pages))
-   return false;
+   struct kvm_mmu_page *sp, *nsp;
+   LIST_HEAD(pinned_list);
 
-   sp = list_entry(kvm-arch.active_mmu_pages.prev,
-   struct kvm_mmu_page, link);
-   kvm_mmu_prepare_zap_page(kvm, sp, invalid_list);
+   list_for_each_entry_safe_reverse(sp, nsp,
+kvm-arch.active_mmu_pages, link) {
+   if (sp-pinned) {
+   list_move(sp-link, pinned_list);
+   continue;
+   }
+   if (!list_empty(pinned_list))
+   list_move(pinned_list, kvm-arch.active_mmu_pages);
+   kvm_mmu_prepare_zap_page(kvm, sp, invalid_list);
+   return true;
+   }
 
-   return true;
+   if (!list_empty(pinned_list))
+   list_move(pinned_list, kvm-arch.active_mmu_pages);
+   return false;
 }
 
 /*
@@ -4679,6 +4687,8 @@
 * Notify all vcpus to reload its shadow page table
 * and flush TLB. Then all vcpus will switch to new
 * shadow page table with the new mmu_valid_gen.
+* MMU reload request also forces fault of
+* sptes for pinned ranges.
 *
 * Note: we should do this under the protection of
 * mmu-lock, otherwise, vcpu would purge shadow page


--
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