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

[Yu Zhang <yu.c.zhang@linux.intel.com>]

On 6/20/2016 6:10 PM, George Dunlap wrote:
> On 20/06/16 10:03, Yu Zhang wrote:
>>
>> On 6/17/2016 6:17 PM, George Dunlap wrote:
>>> On 16/06/16 10:55, Jan Beulich wrote:
>>>>> Previously in the 2nd version, I used p2m_change_entry_type_global() to
>>>>> reset the
>>>>> outstanding p2m_ioreq_server entries back to p2m_ram_rw
>>>>> asynchronously after
>>>>> the de-registration. But we realized later that this approach means we
>>>>> can not support
>>>>> live migration. And to recalculate the whole p2m table forcefully when
>>>>> de-registration
>>>>> happens means too much cost.
>>>>>
>>>>> And further discussion with Paul was that we can leave the
>>>>> responsibility to reset p2m type
>>>>> to the device model side, and even a device model fails to do so, the
>>>>> affected one will only
>>>>> be the current VM, neither other VM nor hypervisor will get hurt.
>>>>>
>>>>> I thought we have reached agreement in the review process of version 2,
>>>>> so I removed
>>>>> this part from version 3.
>>>> In which case I would appreciate the commit message to explain
>>>> this (in particular I admit I don't recall why live migration would
>>>> be affected by the p2m_change_entry_type_global() approach,
>>>> but the request is also so that later readers have at least some
>>>> source of information other than searching the mailing list).
>>> Yes, I don't see why either.  You wouldn't de-register the ioreq server
>>> until after the final sweep after the VM has been paused, right?  At
>>> which point the lazy p2m re-calculation shouldn't really matter much I
>>> don't think.
>> Oh, seems I need to give some explanation, and sorry for the late reply.
>>
>> IIUC, p2m_change_entry_type_global() only sets the e.emt field to an
>> invalid value and turn on
>> the e.recal flag; the real p2m reset is done in resolve_misconfig() when
>> ept misconfiguration
>> happens or when ept_set_entry() is called.
>>
>> In the 2nd version patch, we leveraged this approach, by adding
>> p2m_ioreq_server into the
>> P2M_CHANGEABLE_TYPES, and triggering the p2m_change_entry_type_global()
>> when an ioreq
>> server is unbounded, hoping that later accesses to these gfns will reset
>> the p2m type back to
>> p2m_ram_rw. And for the recalculation itself, it works.
>>
>> 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.

Thanks for your reply, George.  :)
I think this could also happen even when there's no first round 
p2m_change_entry_type_global(),
the resolve_misconfig() will also change normal p2m_ioreq_server entries 
back to p2m_log_dirty.

By "go 'eagerly'", do you mean traverse the ept table? Wouldn't that be 
time consuming
also?

>
>> So one solution is to disallow the log dirty feature in XenGT, i.e. just
>> return failure when enable_logdirty()
>> is called in toolstack. But I'm afraid this will restrict XenGT's future
>> live migration feature.
> I don't understand this -- you can return -EBUSY if live migration is
> attempted while there are outstanding ioreq_server entries for the time
> being, and at some point in the future when this actually works, you can
> return success.
>

Well, the problem is we cannot easily tell if there's any outstanding 
p2m_ioreq_server entries.
Besides, do you agree it is the responsibility of device model to do the 
cleaning?

Thanks
Yu


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