>>> On 13.02.18 at 19:37, <brian.wo...@amd.com> 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 &
(suitably line wrapped if necessary). But I continue to think the
if/else variant looks better overall. It'll be the SVM maintainers to
> On February 13, 2018 03:31:40 Jan Beulich <jbeul...@suse.com> wrote:
>>>>> On 08.02.18 at 18:01, <brian.wo...@amd.com> 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) )
>> ASSERT(!(v->arch.hvm_vcpu.guest_efer & EFER_SVME));
Xen-devel mailing list