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

[Brian Woods <brian.woods@xxxxxxx>]

On Thu, Feb 08, 2018 at 02:45:31AM -0700, Jan Beulich wrote:
> I'm afraid I continue to be confused: A function with this name should
> imo, as said earlier, live in nestedsvm.c. However ...

I'll move it to nestedsvm.c then. 

> ... this indicates that the function does something even for the
> non-nested case. In particular ...

It makes sure that SVME isn't set when nested isn't enabled.  

If SVME is set it does certain checks to enable features if enabled.
Else it makes sure nested features are turned off.

The reason for this is that, with VGIF/VVMLOAD, they can still work even
with SVME not being set.  This sets it where they get intercepted to Xen
so that Xen can correctly emulate what should be happening.  If SVME
isn't set then nested guest shouldn't be able to do VGIF/VVMLOAD. 

> ... this entire else block. Is it necessary to do this in the non-nested
> case? IOW - do these settings ever change there (I would have
> thought that the two *_enable fields checked by the two if()s should
> never be true for nested-disabled guests)? Otherwise, as also said
> before, the caller should call here only when
> nestedhvm_enabled(v->domain), and the function would better
> move.
> 
> Jan
 
It only checks two things (two if statements) in the VMCB per EFER
update.  Suppose you add an if to check if it's a nested guest and then
run the nested_features func.  You're only saving a total of one if and
going in and out a function but you add a small divergence in the
codepath.  If this was a long computer/IO intense function, I'd
completely agree but this is two very simple checks.

I'll change it though, since it'll be easier than going back and forth
about a minor detail.

-- 
Brian Woods

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