Re: [Xen-devel] [PATCH v3 2/3] x86/svm: add EFER SVME support for VGIF/VLOAD

["Woods, Brian" <Brian.Woods@xxxxxxx>]

Pardon any weird formatting, I'm replying on my phone. 

Because they are two different things.  One is an assert to make sure nothing 
wrong is happening with the EFER.SVME bit, and the other changes what features 
are enabled.  

IIRC, most asserts are on their on ifs and not in a if statement with something 
else.  I guess I should have put the assert higher in the function though but 
that's a small detail.  

Yes, you can squeeze both in one if statement but, but it being cleaner and 
easier to read (at least more logical) is better than getting rid of one if in 
my opinion.  Plus assuming asserts are disabled for release, I'd assume the 
extra if would get optimized out by gcc anyway. 

Brian


On February 13, 2018 03:31:40 Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>>> On 08.02.18 at 18:01, <brian.woods@xxxxxxx> wrote:
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -611,6 +611,12 @@ static void svm_update_guest_efer(struct vcpu *v)
>>      if ( lma )
>>          new_efer |= EFER_LME;
>>      vmcb_set_efer(vmcb, new_efer);
>> +
>> +    if ( !nestedhvm_enabled(v->domain) )
>> +        ASSERT(!(v->arch.hvm_vcpu.guest_efer & EFER_SVME));
>> +
>> +    if ( nestedhvm_enabled(v->domain) )
>> +        svm_nested_features_on_efer_update(v);
>>  }
>
> Why not
>
>     if ( nestedhvm_enabled(v->domain) )
>         svm_nested_features_on_efer_update(v);
>     else
>         ASSERT(!(v->arch.hvm_vcpu.guest_efer & EFER_SVME));
>
> ?
>
> Jan
>
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel