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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.02.18 at 19:37, <Brian.Woods@xxxxxxx> wrote:
> 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. 

In that case it would better be

ASSERT(nestedhvm_enabled(v->domain) || !(v->arch.hvm_vcpu.guest_efer & 
EFER_SVME));

(suitably line wrapped if necessary). But I continue to think the
if/else variant looks better overall. It'll be the SVM maintainers to
decide, though.

Jan

> 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