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

[Wen Congyang <wency@xxxxxxxxxxxxxx>]

At 07/11/2013 05:44 PM, Andrew Cooper Wrote:
> 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@xxxxxxxxx>
>> Signed-off-by: Jiang Yunhong <yunhong.jiang@xxxxxxxxx>
>> Signed-off-by: Wen Congyang <wency@xxxxxxxxxxxxxx>
>> ---
>>  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.

Hmm, these codes are copied from this function:
domain_relinquish_resources()
> 
>> +    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 ?

Hmm, I will do it

Thanks
Wen Congyang

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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel