Re: [Xen-devel] [PATCH v17 13/13] x86/domctl: Don't pause the whole domain if only getting vcpu state

[Isaila Alexandru <aisaila@xxxxxxxxxxxxxxx>]

On Mi, 2018-08-22 at 16:41 +0200, Roger Pau Monné wrote:
> On Wed, Aug 22, 2018 at 05:02:43PM +0300, Alexandru Isaila wrote:
> > 
> > This patch is focused on moving changing hvm_save_one() to save one
> > typecode from one vcpu and now that the save functions get data
> > from a
> > single vcpu we can pause the specific vcpu instead of the domain.
> With this infrastructure added allowing to save a single instance of
> a
> specific device I wonder if you would like to add a user to the code
> in the tree.
> 
> If you look at vcpu_hvm in tools/libxc/xc_dom_x86.c it saves the full
> domain context just to get the CPU and the MTRR state of VCPU#0. Do
> you think you could switch this code to use the newly introduced
> machinery to save a single instance of a specific type?

Sure, I will add a tool patch at the end of the series
> 
> > 
> > Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> > 
> > ---
> > Changes since V15:
> >     - Moved pause/unpause calls into hvm_save_one()
> >     - Re-add the loop in hvm_save_one().
> > ---
> >  xen/arch/x86/domctl.c   |  2 --
> >  xen/arch/x86/hvm/save.c | 12 ++++++++++--
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> > index 8fbbf3a..cb53980 100644
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -591,12 +591,10 @@ long arch_do_domctl(
> >               !is_hvm_domain(d) )
> >              break;
> >  
> > -        domain_pause(d);
> >          ret = hvm_save_one(d, domctl->u.hvmcontext_partial.type,
> >                             domctl->u.hvmcontext_partial.instance,
> >                             domctl->u.hvmcontext_partial.buffer,
> >                             &domctl->u.hvmcontext_partial.bufsz);
> > -        domain_unpause(d);
> >  
> >          if ( !ret )
> >              copyback = true;
> > diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
> > index 49741e0..2d35f17 100644
> > --- a/xen/arch/x86/hvm/save.c
> > +++ b/xen/arch/x86/hvm/save.c
> > @@ -149,12 +149,15 @@ int hvm_save_one(struct domain *d, unsigned
> > int typecode, unsigned int instance,
> >          instance >= d->max_vcpus )
> >          return -ENOENT;
> >      ctxt.size = hvm_sr_handlers[typecode].size;
> > -    if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_VCPU )
> > -        ctxt.size *= d->max_vcpus;
> This chunk seems to belong to a different patch?
> 
> The change just mentions pausing a vpcu instead of the whole domain,
> but the size of the save context doesn't depend on whether the whole
> domain is paused vs a single vcpu is paused.

Right, git rebase did not report any error so it tricked me. I will
have that removed from this patch. 

Thanks, 
Alex

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel