[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] XEN_DOMCTL_setvcpucontext and domain_pause()





On 04/08/16 09:21, Razvan Cojocaru wrote:
Hello,

Hello Razvan,

Looking at xen/common/domctl.c, it appears that during handling of
XEN_DOMCTL_setvcpucontext, a domain_pause() happens unconditionally:

 465         if ( ret == 0 )
 466         {
 467             domain_pause(d);
 468             ret = arch_set_info_guest(v, c);
 469             domain_unpause(d);
 470
 471             if ( ret == -ERESTART )
 472                 ret = hypercall_create_continuation(
 473                           __HYPERVISOR_domctl, "h", u_domctl);
 474         }

I assume that this is because in xen/arch/x86/domain.c,
arch_set_info_guest() uses v->domain here:

1101     if ( v->vcpu_id == 0 )
1102     {
1103         /*
1104          * In the restore case we need to deal with L4 pages which got
1105          * initialized with m2p_strict still clear (and which hence
lack the
1106          * correct initial RO_MPT_VIRT_{START,END} L4 entry).
1107          */
1108         if ( d != current->domain && !VM_ASSIST(d, m2p_strict) &&
1109              is_pv_domain(d) && !is_pv_32bit_domain(d) &&
1110              test_bit(VMASST_TYPE_m2p_strict, &c.nat->vm_assist) &&
1111              atomic_read(&d->arch.pv_domain.nr_l4_pages) )
1112         {
1113             bool_t done = 0;
1114
1115             spin_lock_recursive(&d->page_alloc_lock);
1116
1117             for ( i = 0; ; )
1118             {
1119                 struct page_info *page =
page_list_remove_head(&d->page_list);
1120
1121                 if ( page_lock(page) )
1122                 {
1123                     if ( (page->u.inuse.type_info & PGT_type_mask) ==
1124                          PGT_l4_page_table )
1125                         done = !fill_ro_mpt(page_to_mfn(page));
1126
1127                     page_unlock(page);
1128                 }
1129
1130                 page_list_add_tail(page, &d->page_list);
1131
1132                 if ( done || (!(++i & 0xff) &&
hypercall_preempt_check()) )
1133                     break;
1134             }
1135
1136             spin_unlock_recursive(&d->page_alloc_lock);
1137
1138             if ( !done )
1139                 return -ERESTART;
1140         }
1141
1142         d->vm_assist = c(vm_assist);
1143     }

However, this is not optimal for HVM guests. Since only PV guests would
seem to require a domain pause (see the is_pv_domain(d) test at line
1109, I propose to either check that the domain is PV and only then
pause / unpause it in domctl.c, or only pause it if necessary in
arch_set_info_guest(). If I haven't missed anything here, I'm happy to
post a patch.

This code is common, so you should reason if it is fine for all the architectures and not only x86.

In any case, we should really avoid to use is_pv_domain in the common code because this has no meaning on ARM (the guests are nor HVM nor PV).

I actually would like to get a rid of them in the common code in favor of feature available for a domain.

Regards,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.