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

From: "Jan Beulich" <JBeulich@suse.com>
To: George Dunlap <george.dunlap@citrix.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 04:55:00 -0600	[thread overview]
Message-ID: <5767E7A402000078000F69C0@prv-mh.provo.novell.com> (raw)
In-Reply-To: <739af5d1-7fa1-4d68-ef9c-03e9253bccd5@citrix.com>

>>> On 20.06.16 at 12:32, <george.dunlap@citrix.com> wrote:
> 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?

Also for NPT, we're using a similar model in p2m-pt.c (see e.g. the
uses of RECALC_FLAGS - we're utilizing the _PAGE_USER set
unconditionally leads to NPF). And since shadow sits on top of
p2m-pt, that should be covered too.

>  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.

We can't even tell when one have fully finished.

>>  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?

I think Yu's concern is for pages which did not get converted back?
Or on the restore side? Otherwise - "yes" to both of your questions.

Jan

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