Re: [RFC Patch v2 01/16] xen: introduce new hypercall to reset vcpu

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 11/07/13 09:35, Wen Congyang wrote:
> In colo mode, SVM is running, and it will create pagetable, use gdt...
> When we do a new checkpoint, we may need to rollback all this operations.
> This new hypercall will do this.
>
> Signed-off-by: Ye Wei <wei.ye1987@gmail.com>
> Signed-off-by: Jiang Yunhong <yunhong.jiang@intel.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  xen/arch/x86/domain.c       |   57 +++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/x86_64/entry.S |    4 +++
>  xen/include/public/xen.h    |    1 +
>  3 files changed, 62 insertions(+), 0 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 874742c..709f77f 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1930,6 +1930,63 @@ int domain_relinquish_resources(struct domain *d)
>      return 0;
>  }
>  
> +int do_reset_vcpu_op(unsigned long domid)
> +{
> +    struct vcpu *v;
> +    struct domain *d;
> +    int ret;
> +
> +    if ( domid == DOMID_SELF )
> +        /* We can't destroy outself pagetables */

"We can't destroy our own pagetables"

> +        return -EINVAL;
> +
> +    if ( (d = rcu_lock_domain_by_id(domid)) == NULL )
> +        return -EINVAL;
> +
> +    BUG_ON(!cpumask_empty(d->domain_dirty_cpumask));

This looks bogus.  What guarantee is there (other than the toolstack
issuing appropriate hypercalls in an appropriate order) that this is
actually true.

> +    domain_pause(d);
> +
> +    if ( d->arch.relmem == RELMEM_not_started )
> +    {
> +        for_each_vcpu ( d, v )
> +        {
> +            /* Drop the in-use references to page-table bases. */
> +            ret = vcpu_destroy_pagetables(v);
> +            if ( ret )
> +                return ret;
> +
> +            unmap_vcpu_info(v);
> +            v->is_initialised = 0;
> +        }
> +
> +        if ( !is_hvm_domain(d) )
> +        {
> +            for_each_vcpu ( d, v )
> +            {
> +                /*
> +                 * Relinquish GDT mappings. No need for explicit unmapping of the
> +                 * LDT as it automatically gets squashed with the guest mappings.
> +                 */
> +                destroy_gdt(v);
> +            }
> +
> +            if ( d->arch.pv_domain.pirq_eoi_map != NULL )
> +            {
> +                unmap_domain_page_global(d->arch.pv_domain.pirq_eoi_map);
> +                put_page_and_type(
> +                    mfn_to_page(d->arch.pv_domain.pirq_eoi_map_mfn));
> +                d->arch.pv_domain.pirq_eoi_map = NULL;
> +                d->arch.pv_domain.auto_unmask = 0;
> +            }
> +        }
> +    }
> +
> +    domain_unpause(d);
> +    rcu_unlock_domain(d);
> +
> +    return 0;
> +}
> +
>  void arch_dump_domain_info(struct domain *d)
>  {
>      paging_dump_domain_info(d);
> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
> index 5beeccb..0e4dde4 100644
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -762,6 +762,8 @@ ENTRY(hypercall_table)
>          .quad do_domctl
>          .quad do_kexec_op
>          .quad do_tmem_op
> +        .quad do_ni_hypercall       /* reserved for XenClient */
> +        .quad do_reset_vcpu_op      /* 40 */
>          .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8)
>          .quad do_ni_hypercall
>          .endr
> @@ -810,6 +812,8 @@ ENTRY(hypercall_args_table)
>          .byte 1 /* do_domctl            */
>          .byte 2 /* do_kexec             */
>          .byte 1 /* do_tmem_op           */
> +        .byte 0 /* do_ni_hypercall      */
> +        .byte 1 /* do_reset_vcpu_op     */  /* 40 */
>          .rept __HYPERVISOR_arch_0-(.-hypercall_args_table)
>          .byte 0 /* do_ni_hypercall      */
>          .endr
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 3cab74f..696f4a3 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -101,6 +101,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>  #define __HYPERVISOR_kexec_op             37
>  #define __HYPERVISOR_tmem_op              38
>  #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
> +#define __HYPERVISOR_reset_vcpu_op        40

Why can this not be a domctl subop ?

~Andrew

>  
>  /* Architecture-specific hypercall definitions. */
>  #define __HYPERVISOR_arch_0               48