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

[George Dunlap <george.dunlap@eu.citrix.com>]

On 07/14/2015 02:13 PM, Jan Beulich wrote:
>>>> On 14.07.15 at 02:14, <edmund.h.white@intel.com> 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.

 -George