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

["Roger Pau Monné" <roger.pau@citrix.com>]

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?

> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> 
> ---
> 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.

Thanks, Roger.

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