RE: [PATCH v9 0/8] domain context infrastructure

["Lengyel, Tamas" <tamas.lengyel@intel.com>]

> -----Original Message-----
> From: Durrant, Paul <pdurrant@amazon.co.uk>
> Sent: Tuesday, September 29, 2020 8:14 AM
> To: Tamas K Lengyel <tamas.k.lengyel@gmail.com>
> Cc: Lengyel, Tamas <tamas.lengyel@intel.com>; paul@xen.org; xen-
> devel@lists.xenproject.org; Andrew Cooper <andrew.cooper3@citrix.com>;
> Daniel De Graaf <dgdegra@tycho.nsa.gov>; George Dunlap
> <george.dunlap@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Jan
> Beulich <jbeulich@suse.com>; Julien Grall <julien@xen.org>; Marek
> Marczykowski-Górecki <marmarek@invisiblethingslab.com>; Roger Pau Monné
> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Wei Liu
> <wl@xen.org>
> Subject: RE: [PATCH v9 0/8] domain context infrastructure
> 
> > -----Original Message-----
> > From: Tamas K Lengyel <tamas.k.lengyel@gmail.com>
> > Sent: 29 September 2020 13:06
> > To: Durrant, Paul <pdurrant@amazon.co.uk>
> > Cc: Lengyel, Tamas <tamas.lengyel@intel.com>; paul@xen.org;
> > xen-devel@lists.xenproject.org; Andrew Cooper
> > <andrew.cooper3@citrix.com>; Daniel De Graaf <dgdegra@tycho.nsa.gov>;
> > George Dunlap <george.dunlap@citrix.com>; Ian Jackson
> > <ian.jackson@eu.citrix.com>; Jan Beulich <jbeulich@suse.com>; Julien
> > Grall <julien@xen.org>; Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com>; Roger Pau Monné
> > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>;
> > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Wei Liu
> <wl@xen.org>
> > Subject: RE: [EXTERNAL] [PATCH v9 0/8] domain context infrastructure
> >
> > CAUTION: This email originated from outside of the organization. Do
> > not click links or open attachments unless you can confirm the sender and
> know the content is safe.
> >
> >
> >
> > On Tue, Sep 29, 2020 at 7:54 AM Durrant, Paul <pdurrant@amazon.co.uk>
> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Lengyel, Tamas <tamas.lengyel@intel.com>
> > > > Sent: 28 September 2020 15:17
> > > > To: paul@xen.org; xen-devel@lists.xenproject.org
> > > > Cc: Durrant, Paul <pdurrant@amazon.co.uk>; 'Andrew Cooper'
> > > > <andrew.cooper3@citrix.com>; 'Daniel De Graaf'
> <dgdegra@tycho.nsa.gov>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
> Jackson'
> > > > <ian.jackson@eu.citrix.com>; 'Jan Beulich' <jbeulich@suse.com>;
> > > > 'Julien Grall' <julien@xen.org>; 'Marek Marczykowski-Górecki'
> <marmarek@invisiblethingslab.com>; 'Roger Pau Monné'
> > > > <roger.pau@citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>;
> 'Volodymyr Babchuk'
> > > > <Volodymyr_Babchuk@epam.com>; 'Wei Liu' <wl@xen.org>
> > > > Subject: RE: [EXTERNAL] [PATCH v9 0/8] domain context
> > > > infrastructure
> > > >
> > > > CAUTION: This email originated from outside of the organization.
> > > > Do not click links or open attachments unless you can confirm the sender
> and know the content is safe.
> > > >
> > > >
> > > >
> > > > > > Hi Paul,
> > > > > > Could you push a git branch somewhere for this series? I would
> > > > > > like to see this being integrated with VM forking and if its
> > > > > > not too much effort just create the patch for that so that it
> > > > > > could be appended to the
> > > > > series.
> > > > > >
> > > > >
> > > > > Hi Tamas,
> > > > >
> > > > >   Done. See
> > > > > https://xenbits.xen.org/gitweb/?p=people/pauldu/xen.git;a=shortl
> > > > > og;h=refs/h
> > > > > eads/domain-save14
> > > > >
> > > > >   Cheers,
> > > > >
> > > > >     Paul
> > > >
> > > > Hi Paul,
> > > > I added a small patch that would save & load the PV context from
> > > > one domain to another that would
> > be
> > > > called during VM forking. Please take a look at
> > > >
> > https://xenbits.xen.org/gitweb/?p=people/tklengyel/xen.git;a=commitdif
> > f;h=1843ca7302e415317fdb9a63b3a4
> > > > d29a385dc766;hp=8149296fdf80c73727e61cea6fe3251aecf8b333. I called
> > > > the function
> > copy_pv_domaincontext
> > > > for now as that seemed like the most appropriate description for
> > > > it. Please let me know if this
> > looks
> > > > good to you. I'm still testing it but if everything checks out it
> > > > would be nice to just append
> > this
> > > > patch to your series.
> > >
> > > Hi Tamas,
> > >
> > >   The code structure appears to be ok... just some cosmetic tweaks:
> > >
> > > - I think you should call the function simply 'copy_domaincontext'
> > > as the idea is that all state
> > (including what is now in hvm context) will be consolidated
> >
> > Sure, I wasn't entirely clear about whether this will be limited to PV
> > context or if it will eventually add the hvm stuff too. Right now I
> > still would have to do that separately.
> >
> > > - The prevailing style in domctl.c AFAICS is that assignments are
> > > mostly not done inside if
> > statements. Personally I think this is a good thing.
> >
> > I think it cuts down on function sizes when all that is being done
> > after an assigment is a NULL-check. No need for a separate line for it
> > but I also don't care that much. So if it's more important to whoever
> > maintains this to keep the style consistent in this regard I can
> > change it.
> >
> > >
> > >   Once you have something ready to go then I'd be happy to tag it
> > > onto my series if I need to do a
> > v10... but I'm currently hoping that won't be necessary.
> >
> > I think I'll wait until HVM context is included in the framework as
> > well so that we can just switch over everything at once.
> >
> 
> It may be a while before I have everything moved over so you may still want to
> go ahead with this patch if the delay is likely to block things. Also, without this I
> assume any records I port over from HVM context (and hence remove the save
> code) are going to cause breakage for VM forking?

If you do remove the existing hvm_save/hvm_load functions then yes, that would pretty much break VM forking right away. We use those in hvm_copy_context_and_params. So if you do make significant changes in there I would ask that you at least compile test with CONFIG_MEM_SHARING enabled. That would be the point where we would want to start using this new copy_domaincontext function. Right now just for the PV stuff it's not critical as we do copy those internally ourself, it would just be nicer to use this plumbing you add that will be shared with domain save/restore/migrate.

Tamas