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 07:51:53 -0600	[thread overview]
Message-ID: <55CA1A190200007800099975@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20150727110933.GA54789@deinos.phlegethon.org>

>>> 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. But
yes, holding the dirty bit high would cause overhead. Yet
setting it only in hvm_unmap_guest_frame() wouldn't, as I now
realize, address the problem either, as that may happen e.g.
only upon guest destruction (i.e. after the stop-and-copy pass).
I.e. for guest pages currently mapped this way we'd really need
a mechanism to avoid their dirty flags to get set for initial passes,
but force it set on the final one.

And other than Andrew says I think tracking these mappings
(namely permanent ones) isn't infeasible, the more that there
shouldn't be that many of them. With them being tracked the
model then would be to set the dirty flag along with removing a
page from the tracking set, and report the dirty flags set on the
final pass (to make this work without interface changes we could
use the suspended state of the domain as indicator of the final
pass being in progress) for all pages still in the tracking set.

Jan