Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-16 Thread Avi Kivity
On 02/16/2012 06:50 AM, Xiao Guangrong wrote: I think we do not need handle all tlb-flushed request here since all of these request can be delayed to the point where mmu-lock is released , we can simply do it: void kvm_mmu_defer_remote_flush(kvm, need_flush) { if (need_flush)

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-16 Thread Xiao Guangrong
On 02/16/2012 07:57 PM, Avi Kivity wrote: Suppose at point A another thread executes defer_remote_flush(), commit_remote_flush(), and defer_remote_flush() again. This brings the balue of tlbs_dirty back to 1 again, with the tlbs dirty. The cmpxchg() then resets tlbs_dirty, leaving the

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-15 Thread Avi Kivity
On 02/14/2012 09:43 PM, Marcelo Tosatti wrote: Also it should not be necessary for these flushes to be inside mmu_lock on EPT/NPT case (since there is no write protection there). We do write protect with TDP, if nested virt is active. The question is whether we have indirect pages or not, not

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-15 Thread Avi Kivity
On 02/15/2012 11:18 AM, Avi Kivity wrote: On 02/14/2012 09:43 PM, Marcelo Tosatti wrote: Also it should not be necessary for these flushes to be inside mmu_lock on EPT/NPT case (since there is no write protection there). We do write protect with TDP, if nested virt is active. The question

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-15 Thread Xiao Guangrong
On 02/15/2012 05:47 PM, Avi Kivity wrote: On 02/15/2012 11:18 AM, Avi Kivity wrote: On 02/14/2012 09:43 PM, Marcelo Tosatti wrote: Also it should not be necessary for these flushes to be inside mmu_lock on EPT/NPT case (since there is no write protection there). We do write protect with

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-15 Thread Avi Kivity
On 02/15/2012 01:37 PM, Xiao Guangrong wrote: I would really like to move the IPI back out of the lock. How about something like a sequence lock: spin_lock(mmu_lock) need_flush = write_protect_stuff(); atomic_add(kvm-want_flush_counter, need_flush);

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-15 Thread Andrea Arcangeli
On Wed, Feb 15, 2012 at 04:07:49PM +0200, Avi Kivity wrote: Well, it still has flushes inside the lock. And it seems to be more complicated, but maybe that's because I thought of my idea and didn't fully grok yours yet. If we go more complicated I prefer Avi's suggestion to move them all

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-15 Thread Xiao Guangrong
On 02/15/2012 10:07 PM, Avi Kivity wrote: On 02/15/2012 01:37 PM, Xiao Guangrong wrote: I would really like to move the IPI back out of the lock. How about something like a sequence lock: spin_lock(mmu_lock) need_flush = write_protect_stuff();

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-14 Thread Andrea Arcangeli
On Fri, Feb 10, 2012 at 03:28:31PM +0900, Takuya Yoshikawa wrote: Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush. It's correct to flush the shadow MMU TLB without the mmu_lock only in the context of mmu notifier

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-14 Thread Andrea Arcangeli
On Fri, Feb 10, 2012 at 03:52:49PM +0800, Xiao Guangrong wrote: On 02/10/2012 02:28 PM, Takuya Yoshikawa wrote: Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush. It is possible that flush tlb in mmu lock

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-14 Thread Marcelo Tosatti
On Tue, Feb 14, 2012 at 06:10:44PM +0100, Andrea Arcangeli wrote: On Fri, Feb 10, 2012 at 03:28:31PM +0900, Takuya Yoshikawa wrote: Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush. It's correct to flush the

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-14 Thread Andrea Arcangeli
On Tue, Feb 14, 2012 at 03:29:47PM -0200, Marcelo Tosatti wrote: The problem the patch is fixing is not related to page freeing, but rmap_write_protect. From 8bf3f6f06fcdfd097b6c6ec51531d8292fa0d81d Can't find the commit on kvm.git. (replace A (get_dirty_log) with

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-14 Thread Marcelo Tosatti
On Tue, Feb 14, 2012 at 07:53:56PM +0100, Andrea Arcangeli wrote: On Tue, Feb 14, 2012 at 03:29:47PM -0200, Marcelo Tosatti wrote: The problem the patch is fixing is not related to page freeing, but rmap_write_protect. From 8bf3f6f06fcdfd097b6c6ec51531d8292fa0d81d Can't find the commit on

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-12 Thread Takuya Yoshikawa
(2012/02/10 16:52), Xiao Guangrong wrote: On 02/10/2012 02:28 PM, Takuya Yoshikawa wrote: Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush. It is possible that flush tlb in mmu lock only when writeable spte is

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-11 Thread Marcelo Tosatti
On Fri, Feb 10, 2012 at 03:28:31PM +0900, Takuya Yoshikawa wrote: Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush. Signed-off-by: Takuya Yoshikawa yoshikawa.tak...@oss.ntt.co.jp --- virt/kvm/kvm_main.c | 19

[PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-09 Thread Takuya Yoshikawa
Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush. Signed-off-by: Takuya Yoshikawa yoshikawa.tak...@oss.ntt.co.jp --- virt/kvm/kvm_main.c | 19 ++- 1 files changed, 10 insertions(+), 9 deletions(-)

Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock

2012-02-09 Thread Xiao Guangrong
On 02/10/2012 02:28 PM, Takuya Yoshikawa wrote: Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush. It is possible that flush tlb in mmu lock only when writeable spte is invalided? Sometimes, kvm_flush_remote_tlbs