Re: [PATCH v2 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 4/21/2016 12:30 AM, Paul Durrant wrote:
>> -----Original Message-----
>> From: George Dunlap
>> Sent: 20 April 2016 16:38
>> To: Paul Durrant
>> Cc: Yu, Zhang; Kevin Tian; jun.nakajima@intel.com; Andrew Cooper; Tim
>> (Xen.org); xen-devel@lists.xen.org; Lv, Zhiyuan; Jan Beulich
>> Subject: Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to
>> map guest ram with p2m_ioreq_server to an ioreq server
>>
>> On Wed, Apr 20, 2016 at 3:57 PM, Paul Durrant <Paul.Durrant@citrix.com>
>> wrote:
>>>> -----Original Message-----
>>>> From: George Dunlap
>>>> Sent: 20 April 2016 15:50
>>>> To: Yu, Zhang
>>>> Cc: Paul Durrant; xen-devel@lists.xen.org; Kevin Tian; Jan Beulich;
>> Andrew
>>>> Cooper; Tim (Xen.org); Lv, Zhiyuan; jun.nakajima@intel.com
>>>> Subject: Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to
>>>> map guest ram with p2m_ioreq_server to an ioreq server
>>>>
>>>> On Tue, Apr 19, 2016 at 12:59 PM, Yu, Zhang
>> <yu.c.zhang@linux.intel.com>
>>>> wrote:
>>>>>>> So what about the VM suspend case you mentioned above? Will that
>>>> trigger
>>>>>>> the unmapping of ioreq server? Could the device model also take the
>>>> role
>>>>>>> to change the p2m type back in such case?
>>>>>>
>>>>>>
>>>>>> Yes. The device model has to be told by the toolstack that the VM is
>>>>>> suspending, otherwise it can't disable the ioreq server which puts the
>>>>>> shared ioreq pages back into the guest p2m. If that's not done then
>> the
>>>>>> pages will be leaked.
>>>>>>
>>>>>>>
>>>>>>> It would be much simpler if hypervisor side does not need to provide
>>>>>>> the p2m resetting logic, and we can support live migration at the
>> same
>>>>>>> time then. :)
>>>>>>>
>>>>>>
>>>>>> That really should not be hypervisor's job.
>>>>>>
>>>>>>    Paul
>>>>>>
>>>>>
>>>>> Oh. So let's just remove the p2m type recalculation code from this
>>>>> patch, no need to call p2m_change_entry_type_global, and no need to
>>>>> worry about the log dirty part.
>>>>>
>>>>> George, do you think this acceptable?
>>>>
>>>> Sorry, just to make sure I understand your intentions:
>>>>
>>>> 1. The ioreq servers will be responsible for changing all the
>>>> p2m_ioreq_server types back to p2m_ram_rw before detaching
>>>> 2. Xen will refuse to set logdirty mode if there are any attached ioreq
>> servers
>>>>
>>>> The one problem with #1 is what to do if the ioreq server is buggy /
>>>> killed, and forgets / is unable to clean up its ioreq_server entries?
>>>>
>>>
>>> If we ever want to meaningfully migrate a VM with XenGT enabled then I
>> think it has to be possible to migrate the p2m_ioreq_server page types as-is.
>> I'd expect a new instance of the xengt emulator to claim them on the far and
>> any emulator state be transferred in a similar way to which qemu state is
>> transferred such that vgpu emulation can be continued after the migration.
>>> After all, it's not like we magically reset the type of mmio-dm pages on
>> migration. It is just expected that a new instance of QEMU is spawned to
>> take care of them on the far end when the VM is unpaused.
>>
>> I don't see the implication you're trying to make regarding the
>> attach/detach interface.
>>
>> In any case, is it really not the case that the devicemodels on the
>> far side have to re-register which IO ranges they're interested in?
>> Shouldn't the devicemodel on the far side re-register the pfns it
>> wants to watch as well?  That again seems like the most robust
>> solution, rather than assuming that the pfns you expect to have been
>> marked are already marked.  To do otherwise risks bugs where 1) pages
>> you think were being watched aren't 2) pages being watched that you
>> don't care about (and you don't think to un-watch them because you
>> didn't think you'd watched them in the first place).
>>
>
> That sounds a little inefficient when there could be thousands of pages in play. If the p2m types were faithfully reproduced in the receiving domain then the new incarnation of the emulator just needs to claim the type rather than making thousands of set-mem-type hypercalls as well. I agree that the information would need to be transferred though, otherwise the new incarnation wouldn't be able reset the page types when it was done with them.
>

I agree. For XenGT to live migrate, device model may probably actively
take role like this.

>> On a slightly different topic: come to think of it, there's no need to
>> switch the p2m type for ioreq_server when we turn on logdirty mode; we
>> just have to call paging_mark_dirty() if it's a write, don't we?
>>

Yes. Unlike the log dirty pages, which will be changed back to
p2m_ram_rw after the first ept violation(to keep it simple, no
intel PML considered), pages with p2m_ioreq_server will continue
to be trapped. Since this patch series is only for the basic
functionality of XenGT, I suggest we do not include this until
we plan to support XenGT live migration in the future. And at
that time, some kind optimization may be necessary(i.e. how to
remove the redundant paging_mark_dirty for write protected pages).


>
> That sounds right to me.
>
>   Paul
>
>>  -George

B.R.
Yu

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