Re: [Xen-devel] [PATCH v5 05/15] x86/altp2m: basic data structures and support routines.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.07.15 at 16:45, <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 07/14/2015 02:13 PM, Jan Beulich wrote:
>>>>> On 14.07.15 at 02:14, <edmund.h.white@xxxxxxxxx> wrote:
>>> +void
>>> +altp2m_vcpu_initialise(struct vcpu *v)
>>> +{
>>> +    if ( v != current )
>>> +        vcpu_pause(v);
>>> +
>>> +    altp2m_vcpu_reset(v);
>>> +    vcpu_altp2m(v).p2midx = 0;
>>> +    atomic_inc(&p2m_get_altp2m(v)->active_vcpus);
>>> +
>>> +    altp2m_vcpu_update_eptp(v);
>>> +
>>> +    if ( v != current )
>>> +        vcpu_unpause(v);
>>> +}
>>> +
>>> +void
>>> +altp2m_vcpu_destroy(struct vcpu *v)
>>> +{
>>> +    struct p2m_domain *p2m;
>>> +
>>> +    if ( v != current )
>>> +        vcpu_pause(v);
>>> +
>>> +    if ( (p2m = p2m_get_altp2m(v)) )
>>> +        atomic_dec(&p2m->active_vcpus);
>>> +
>>> +    altp2m_vcpu_reset(v);
>>> +
>>> +    altp2m_vcpu_update_eptp(v);
>>> +    altp2m_vcpu_update_vmfunc_ve(v);
>>> +
>>> +    if ( v != current )
>>> +        vcpu_unpause(v);
>>> +}
>> 
>> There not being any caller of altp2m_vcpu_initialise() I can't judge
>> about its pausing requirements, but for the destroy case I can't
>> see what the pausing is good for. Considering its sole user it's also
>> not really clear why the two update operations need to be done
>> while destroying.
> 
> So looking at this after all the patches have been applied, it looks
> like initialise() and destroy() are called from the
> altp2m_set_domain_state(), which the guest uses to enable or disable the
> altp2m functionality.  In that case, it seems like pausing is probably
> appropriate.

Right. Albeit I'd then question whether it wouldn't better be the caller
to do the pausing (via domain_pause_except_self()), considering that
the functions are being called in a fir_each_vcpu() loop.

Jan


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