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

On 4/19/2016 7:47 PM, Paul Durrant wrote:
> > -----Original Message-----
> > From: Yu, Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> > Sent: 19 April 2016 12:18
> > To: Paul Durrant; George Dunlap; xen-devel@xxxxxxxxxxxxx
> > Cc: Kevin Tian; Jan Beulich; Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan;
> > jun.nakajima@xxxxxxxxx
> > 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 4/19/2016 6:05 PM, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Yu, Zhang [mailto:yu.c.zhang@xxxxxxxxxxxxxxx]
> > > > Sent: 19 April 2016 10:44
> > > > To: Paul Durrant; George Dunlap; xen-devel@xxxxxxxxxxxxx
> > > > Cc: Kevin Tian; Jan Beulich; Andrew Cooper; Tim (Xen.org); Lv, Zhiyuan;
> > > > jun.nakajima@xxxxxxxxx
> > > > 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 4/19/2016 5:21 PM, Paul Durrant wrote:
> > > > > > -----Original Message-----
> > > > > [snip]
> > > > > > > > Does any other maintainers have any suggestions?
> > > > > > > Note that it is a requirement that an ioreq server be disabled before
> > VM
> > > > > > suspend. That means ioreq server pages essentially have to go back to
> > > > > > ram_rw semantics.
> > > > > > >       Paul
> > > > > > OK. So it should be hypervisor's responsibility to do the resetting.
> > > > > > Now we probably have 2 choices:
> > > > > > 1> we reset the p2m type synchronously when ioreq server unmapping
> > > > > > happens, instead of deferring to the misconfig handling part. This
> > > > > > means performance impact to traverse the p2m table.
> > > > > Do we need to reset at all. The p2m type does need to be transferred, it
> > > > will just be unclaimed on the far end (i.e. the pages are treated as r/w
> > ram)
> > > > until the emulator starts up there. If that cannot be done without creating
> > > > yet another p2m type to handle logdirty (which seems a suboptimal way
> > of
> > > > dealing with it) then I think migration needs to be disallowed on any
> > domain
> > > > than contains any ioreq_server type pages at this stage.
> > > > >      Paul
> > > > Yes. We need - either the device model or hypervisor should grantee
> > > > there's no p2m_ioreq_server pages left after an ioreq server is
> > unmapped
> > > > from this type (which is write protected in such senario), otherwise
> > > > its emulation might be forwarded to other unexpected device models
> > which
> > > > claims the p2m_ioreq_server later.
> > > That should be for the device model to guarantee IMO. If the 'wrong'
> > emulator claims the ioreq server type then I don't think that's Xen's problem.
> > >
> > Thanks, Paul.
> >
> > 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?

BTW, if no need to call p2m_change_entry_type_global, which is not
used for shadow mode, we can keep this p2m type in shadow code, right?

Thanks
Yu

> > B.R.
> > Yu
> >
> > > > So I guess approach 2> is your suggestion now.
> > > >
> > > > Besides, previously, Jan also questioned the necessity of resetting the
> > > > p2m type when an ioreq server is mapping to the p2m_ioreq_server. His
> > > > argument is that we should only allow such p2m transition after an
> > > > ioreq server has already mapped to this p2m_ioreq_server. I think his
> > > > point sounds also reasonable.
> > > I was kind of hoping to avoid that ordering dependency but if it makes
> > things simpler then so be it.
> > >     Paul
> > >
> > > > Thanks
> > > > Yu
> > > >
> > > > > > Or
> > > > > > 2> we just disallow live migration when p2m->ioreq.server is not NULL.
> > > > > > This is not quite accurate, because having p2m->ioreq.server mapped
> > > > > > to p2m_ioreq_server does not necessarily means there would be such
> > > > > > outstanding entries. To be more accurate, we can add some other
> > rough
> > > > > > check, e.g. both check if p2m->ioreq.server against NULL and check if
> > > > > > the hvmop_set_mem_type has ever been triggered once for the
> > > > > > p2m_ioreq_server type.
> > > > > >
> > > > > > Both choice seems suboptimal for me. And I wonder if we have any
> > > > > > better solutions?
> > > > > >
> > > > > > Thanks
> > > > > > Yu
> > > > > >
> > > > > > > > Thanks in advance! :)
> > > > > > > > > > If the answer is, "everything just works", that's perfect.
> > > > > > > > > >
> > > > > > > > > > If the answer is, "Before logdirty mode is set, the ioreq server has
> > > > the
> > > > > > > > > > opportunity to detach, removing the p2m_ioreq_server entries,
> > and
> > > > > > > > > > operating without that functionality", that's good too.
> > > > > > > > > >
> > > > > > > > > > If the answer is, "the live migration request fails and the guest
> > > > > > > > > > continues to run", that's also acceptable.  If you want this series to
> > > > > > > > > > be checked in today (the last day for 4.7), this is probably your
> > best
> > > > > > > > > > bet.
> > > > > > > > > >
> > > > > > > > > >       -George
> > > > > > > > Regards
> > > > > > > > Yu
> > > > > > > _______________________________________________
> > > > > > > Xen-devel mailing list
> > > > > > > Xen-devel@xxxxxxxxxxxxx
> > > > > > > http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel