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

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 08/04/2016 02:12 PM, Julien Grall wrote:
> Hello Razvan,
> 
> On 04/08/16 10:00, Razvan Cojocaru wrote:
>> On 08/04/2016 11:51 AM, Jan Beulich wrote:
>>>>>> On 04.08.16 at 10:21, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>> 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:
>>>
>>> Not only: It would be rather bad to change register state
>>> underneath a running vCPU (such a change then would not take
>>> effect right away, and might not [fully] take effect at all). Plus,
>>> if you paused only the subject vCPU, you'd risk races with other
>>> vCPU-s interacting with the paused one.
>>>
>>> It's anyway questionable whether setting context for a vCPU
>>> after it got started is really such a good idea, even more so if
>>> you mean to do this frequently (and only then I can see that
>>> the pausing may get into the way).
>>
>> We are indeed setting (mostly GPRs) somewhat frequently, for
>> already-paused VCPUs (paused because a sync-type vm_event has been sent,
>> and the responsible VCPU is frozen until we reply). In that case, we'd
>> like to be as efficient as possible (and assumed until recently that
>> that was the case when choosing xc_vcpu_setcontext() over
>> xc_domain_hvm_setcontext()).
> 
> I am a bit confused. AFAICT, you can set the GPRs via the vm_event
> subsystem (see vm_event_set_registers). So why do you want to use
> xc_vcpusetcontext here?

XEN_DOMCTL_setvcpucontext allows manipulating a larger set of registers
than the ones that come with a vm_event, so for the "mostly" part that
would indeed be enough, but not for everything.

Extending the vm_event data, on the other hand, would increase the space
needed for a single event in the ring buffer and may not be desirable
after a certain number of registers.

In any case, this is a good parallel - with the vm_event system it is
possible to set state for a single paused VCPU while the others are
running - so whatever faults and benefits this approach has would apply
to the scenario I was presenting.


Thanks,
Razvan

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