Re: [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

From: George Dunlap <george.dunlap@citrix.com>
To: Jan Beulich <JBeulich@suse.com>, Yu Zhang <yu.c.zhang@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	George Dunlap <george.dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>,
	xen-devel@lists.xen.org, Paul Durrant <paul.durrant@citrix.com>,
	zhiyuan.lv@intel.com, JunNakajima <jun.nakajima@intel.com>
Subject: Re: [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
Date: Mon, 20 Jun 2016 11:32:30 +0100	[thread overview]
Message-ID: <739af5d1-7fa1-4d68-ef9c-03e9253bccd5@citrix.com> (raw)
In-Reply-To: <5767E0BB02000078000F696A@prv-mh.provo.novell.com>

On 20/06/16 11:25, Jan Beulich wrote:
>>>> On 20.06.16 at 12:10, <george.dunlap@citrix.com> wrote:
>> On 20/06/16 10:03, Yu Zhang wrote:
>>> However, there are conflicts if we take live migration  into account,
>>> i.e. if the live migration is
>>> triggered by the user(unintentionally maybe) during the gpu emulation
>>> process, resolve_misconfig()
>>> will set all the outstanding p2m_ioreq_server entries to p2m_log_dirty,
>>> which is not what we expected,
>>> because our intention is to only reset the outdated p2m_ioreq_server
>>> entries back to p2m_ram_rw.
>>
>> Well the real problem in the situation you describe is that a second
>> "lazy" p2m_change_entry_type_global() operation is starting before the
>> first one is finished.  All that's needed to resolve the situation is
>> that if you get a second p2m_change_entry_type_global() operation while
>> there are outstanding entries from the first type change, you have to
>> finish the first operation (i.e., go "eagerly" find all the
>> misconfigured entries and change them to the new type) before starting
>> the second one.
> 
> Eager resolution of outstanding entries can't be the solution here, I
> think, as that would - afaict - be as time consuming as doing the type
> change synchronously right away.

But isn't it the case that p2m_change_entry_type_global() is only
implemented for EPT?  So we've been doing the slow method for both
shadow and AMD HAP (whatever it's called these days) since the
beginning.  And in any case we'd only have to go for the "slow" case in
circumstances where the 2nd type change happened before the first one
had completed.

>  p2m_change_entry_type_global(),
> at least right now, can be invoked freely without prior type changes
> having fully propagated. The logic resolving mis-configured entries
> simply needs to be able to know the correct new type. I can't see
> why this logic shouldn't therefore be extensible to this new type
> which can be in flight - after we ought to have a way to know what
> type a particular GFN is supposed to be?

Actually, come to think of it -- since the first type change is meant to
convert all ioreq_server -> ram_rw, and the second is meant to change
all ram_rw -> logdirty,  is there any case in which we *wouldn't* want
the resulting type to be logdirty?  Isn't that exactly what we'd get if
we'd done both operations synchronously?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel