From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@kernel.org>
Subject: Re: [patch 3/4] KVM: MMU: reload request from GET_DIRTY_LOG path
Date: Fri, 10 Oct 2014 16:09:29 +0300
Message-ID: <20141010130928.GW26540@minantech.com>
References: <20140709191250.408928362@amt.cnet>
 <20140709191611.280800634@amt.cnet>
 <20140721131424.GZ18167@minantech.com>
 <20140909152811.GA4153@amt.cnet>
 <20141004072332.GS26540@minantech.com>
 <20141006171932.GA1011@amt.cnet>
 <20141008065636.GU26540@minantech.com>
 <20141008171534.GA2651@amt.cnet>
 <20141008175937.GV26540@minantech.com>
 <20141008192231.GA5866@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, ak@linux.intel.com, pbonzini@redhat.com,
	xiaoguangrong@linux.vnet.ibm.com, avi.kivity@gmail.com
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-wg0-f41.google.com ([74.125.82.41]:63401 "EHLO
	mail-wg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751684AbaJJNJd (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 10 Oct 2014 09:09:33 -0400
Received: by mail-wg0-f41.google.com with SMTP id b13so3872241wgh.12
        for <kvm@vger.kernel.org>; Fri, 10 Oct 2014 06:09:32 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <20141008192231.GA5866@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Oct 08, 2014 at 04:22:31PM -0300, Marcelo Tosatti wrote:
> > > 
> > > Argh, lets try again:
> > > 
> > > skip_pinned = true
> > > ------------------
> > > 
> > > mark page dirty, keep spte intact
> > > 
> > > called from get dirty log path.
> > > 
> > > skip_pinned = false
> > > -------------------
> > > reload remote mmu
> > > destroy pinned spte.
> > > 
> > > called from: dirty log enablement, rmap write protect (unused for pinned
> > > sptes)
> > > 
> > > 
> > > Note this behaviour is your suggestion:
> > 
> > Yes, I remember that and I thought we will not need this skip_pinned
> > at all.  For rmap write protect case there shouldn't be any pinned pages,
> > but why dirty log enablement sets skip_pinned to false? Why not mark
> > pinned pages as dirty just like you do in get dirty log path?
> 
> Because if its a large spte, it must be nuked (or marked read-only,
> which for pinned sptes, is not possible).
> 
If a large page has one small page pinned inside it its spte will
be marked as pinned, correct?  We did nuke large ptes here until very
recently: c126d94f2c90ed9d, but we cannot drop a pte here anyway without
kicking all vcpu from a guest mode, but do you need additional skip_pinned
parameter?  Why not check if spte is large instead?

So why not have per slot pinned page list (Xiao suggested the same) and do:

spte_write_protect() {
  if (is_pinned(spte) {
    if (large(spte))
       // cannot drop while vcpu are running
       mmu_reload_pinned_vcpus();
    else
       return false;
}
    

get_dirty_log() {
 for_each(pinned pages i)
    makr_dirty(i);
}

--
			Gleb.