>>> 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 & 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 <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) ) >> svm_nested_features_on_efer_update(v); >> else >> ASSERT(!(v->arch.hvm_vcpu.guest_efer & EFER_SVME)); >> >> ? >> >> Jan >> >> >> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
