On 04/10/2019 10:55 AM, Sean Christopherson wrote:
On Wed, Apr 10, 2019 at 07:05:30PM +0200, Paolo Bonzini wrote:
On 10/04/19 18:08, Sean Christopherson wrote:
Good point, though IMO it'd be better to go one step further and actually
consume the return value in nested_vmx_enter_non_root_mode(). For me,
having the exit reason in nested_vmx_check_vmentry_postreqs() is a nice
mental reminder that "postreqs" is referring to checks that happen once
the CPU has "committed" to VM-Enter.
It's certainly better if you don't have to return
VMX_EXIT_REASONS_FAILED_VMENTRY. However, I think it still complicates
things a bit, after all the result is always EXIT_REASON_INVALID_STATE.
The SDM says "VM-entry failure due to invalid guest state. A VM entry
failed one of the checks identified in Section 26.3.1" so the bool (or
0/-EINVAL) return code is a nice reminder that the function covers a
subset of 26.3.1.
Heh, for me, returning EXIT_REASON_INVALID_STATE is the reminder that
the function covers 26.3.1.
I think the cleaner approach would be to return -EINVAL in both
*_pre{post}reqs functions and carry the exit code via a new function
parameter. That will make the code more readable.
What if we rename the function to nested_vmx_check_vmentry_guest_state()?
My desire to return the exit reason mostly stems from the name "postreqs"
since I tend to forget what "postreqs" is referring to. And it'd be more
appropriate since the MSR load checks are handled elsewhere and really
should be considered "postreqs" as well.
May be we should also rename its counterpart
(i.e.,nested_vmx_check_vmentry_prereqs) to something like
nested_vmx_check_vmentry_pre_guest_state or something similar ?
The SDM doesn't explicitly mention whether MSR loading happens right
after guest state is loaded, though it seems that's the order. If so, we
should probably put these checks in function called
nested_vmx_check_vmentry_msr_load or something like that.
