Re: [PATCH] x86/HVM: honor p2m_ram_ro in hvm_map_guest_frame_rw()

From: "Jan Beulich" <JBeulich@suse.com>
To: Tim Deegan <tim@xen.org>
Cc: Kevin Tian <kevin.tian@intel.com>, Wei Liu <wei.liu2@citrix.com>,
	suravee.suthikulpanit@amd.com,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Eddie Dong <eddie.dong@intel.com>,
	Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Keir Fraser <keir@xen.org>
Subject: Re: [PATCH] x86/HVM: honor p2m_ram_ro in hvm_map_guest_frame_rw()
Date: Tue, 11 Aug 2015 10:01:51 -0600	[thread overview]
Message-ID: <55CA388F0200007800099C0E@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20150811154526.GC884@deinos.phlegethon.org>

>>> On 11.08.15 at 17:45, <tim@xen.org> wrote:
> At 09:37 -0600 on 11 Aug (1439285833), Jan Beulich wrote:
>> >>> On 11.08.15 at 16:34, <tim@xen.org> wrote:
>> > At 07:51 -0600 on 11 Aug (1439279513), Jan Beulich wrote:
>> >> >>> On 27.07.15 at 13:09, <tim@xen.org> wrote:
>> >> > At 13:02 +0100 on 24 Jul (1437742964), Andrew Cooper wrote:
>> >> >> On 24/07/15 10:41, Jan Beulich wrote:
>> >> >> > Beyond that log-dirty handling in _hvm_map_guest_frame() looks bogus
>> >> >> > too: What if a XEN_DOMCTL_SHADOW_OP_* gets issued and acted upon
>> >> >> > between the setting of the dirty flag and the actual write happening?
>> >> >> > I.e. shouldn't the flag instead be set in hvm_unmap_guest_frame()?
>> >> >> 
>> >> >> It does indeed.  (Ideally the dirty bit should probably be held high for 
>> >> >> the duration that a mapping exists, but that is absolutely infeasible to 
>> >> >> do).
>> >> > 
>> >> > IMO that would not be very useful -- a well-behaved toolstack will
>> >> > have to make sure that relevant mappings are torn down before
>> >> > stop-and-copy.  Forcing the dirty bit high in the meantime just makes
>> >> > every intermediate pass send a wasted copy of the page, without
>> >> > actually closing the race window if the tools are buggy.
>> >> 
>> >> Making sure such mappings got torn down in time doesn't help
>> >> when the most recent write happened _after_ the most recent
>> >> clearing of the dirty flag in a pass prior to stop-and-copy.
>> > 
>> > This is why e.g. __gnttab_unmap_common sets the dirty bit again
>> > as it unmaps.
>> 
>> And how does this help when the mapping survives until the guest
>> gets suspended?
> 
> Suspended is fine, so long as it happens before the final read of the
> bitmap.

I don't follow - nothing unmaps the page in that case, so it wouldn't
get marked dirty. But I now realize this is the wrong example: The
mapping domain wouldn't get a mapped grant saved, and the
granting domain has to rely on the mappings of the grants to be
properly torn down before the final pass anyway. Whereas here
we're talking about mappings held and dirtying done by Xen itself.

Jan