Re: [Xen-devel] [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@xxxxxxxxxxxxxxx>]

Thank you, George.

On 4/20/2016 10:50 PM, George Dunlap wrote:
> On Tue, Apr 19, 2016 at 12:59 PM, Yu, Zhang <yu.c.zhang@xxxxxxxxxxxxxxx> 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
In this version, this is done by calling p2m_change_entry_type_global()
*after* the ioreq server detaching.

But Paul & I now agreed not to reset p2m type in hypervisor, it should
be the device model's responsibility to do so. Even if buggy/malicious
device model fails to do so, the only affected one is the HVM being
serviced, neither hypervisor nor other VMs will be hurt.

> 2. Xen will refuse to set logdirty mode if there are any attached ioreq servers
In this version, I included p2m_ioreq_server to P2M_CHANGEABLE_TYPES,
so that we can use p2m_change_entry_global() for the auto resetting of
p2m_ioreq_server, yet later realized that if live migration happens
during a normal emulation process(before the detachment of the ioreq
server), entries with p2m_ioreq_server will be changed to p2m_log_dirty
(later to p2m_ram_rw in ept violation), therefore write operations will
not be able to forward to the device model.

I also realized it impossible to both keep the p2m auto resetting for
ioreq server detachment, and also support the live migration. So I once
suggested this option #2.

But it is not necessary now that we do not need the p2m resetting code.

> 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?
>
> There's a certain symmetry with the idea of having the ioreq server
> responsible both for mapping and unmapping; and it would be nice to
> have this stuff work for shadow mode as well.  But the automatic type
About shadow mode, it is not supported in this version, because routine
p2m_change_entry_global() is only supported on HAP mode. But I still
would like to keep shadow mode out in next version, because we have no
such usage model and if there's any bug it would be hard for us to find.

> change seems like a more robust option.  (Still open to persuasion on
> this.)
>
> On another note: the hard freeze is long past the already-extended
> deadline, so I was assuming this was 4.8 material at this point.
It is a pity. And I'm the one to be blamed for. Thank you very much
for all the advice and help. I'll continue to work on this feature
till no matter whichever version it is accepted to. :)

B.R.
Yu

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