Re: [Xen-devel] [PATCH v2 1/4] x86/EPT: force re-evaluation of memory type as necessary

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.04.14 at 12:18, <tim@xxxxxxx> wrote:
> Hi, 
> 
> This version looks correct to me, but this tristate: 
> 
> At 13:17 +0000 on 28 Mar (1396009028), Jan Beulich wrote:
>> +    p2m_lock(p2m);
>> +
>> +    okay = -curr->arch.hvm_vmx.ept_spurious_misconfig;
> 
> [...]
> 
>> +    if ( okay > 0 )
>> +    {
>> +        struct vcpu *v;
>> +
>> +        for_each_vcpu ( curr->domain, v )
>> +            v->arch.hvm_vmx.ept_spurious_misconfig = 1;
>> +    }
>> +    curr->arch.hvm_vmx.ept_spurious_misconfig = 0;
>> +    ept_sync_domain(p2m);
>> +    p2m_unlock(p2m);
>> +
>> +    return !!okay;
> 
> seems a bit baroque.  Can we go for 'bool_t okay = 0' at the top and then
> 
>     if ( okay )
>     {
>         struct vcpu *v;
> 
>         for_each_vcpu ( curr->domain, v )
>             v->arch.hvm_vmx.ept_spurious_misconfig = 1;
>     }
>     else
>         okay = curr->arch.hvm_vmx.ept_spurious_misconfig; 
>     curr->arch.hvm_vmx.ept_spurious_misconfig = 0;
>     ept_sync_domain(p2m);
>     p2m_unlock(p2m);
> 
>     return okay;
> 
> instead?  With that change, 
> 
> Reviewed-by: Tim Deegan <tim@xxxxxxx>

Will do, even though another patch (not posted yet, removing the
recursion in ept_change_entry_type_global()) converts this to a more
conventional "rc" anyway, as I'm re-using the bulk of this function
then to flush out pending changes first thing in ept_set_entry().

Jan


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