Re: [PATCH] KVM: nVMX: reorganize initial steps of vmx_set_nested_state

[Aaron Lewis]

On Thu, Jun 20, 2019 at 6:18 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> On 20/06/19 14:18, Vitaly Kuznetsov wrote:
> > There's also something wrong with the patch as it fails to apply because
> > of (not only?) whitespace issues or maybe I'm just applying it to the
> > wrong tree...
>
> Yes, there's a change to KVM_GET/SET_NESTED_STATE structs from Liran.
>
> Paolo

Below is a revised patch for vmx_set_nested_state_test based on your
changes.  If I applied your patch correctly I think they should look
something like this.  I don't have your changes to kvm_nested_state,
so those still have to be applied, but I think they are good
otherwise.

---
 .../kvm/x86_64/vmx_set_nested_state_test.c    | 59 ++++++++++---------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
index 9d62e2c7e024..17cf72749ca8 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
@@ -113,25 +113,6 @@ void test_vmx_nested_state(struct kvm_vm *vm)
  state->format = 1;
  test_nested_state_expect_einval(vm, state);

- /*
- * We cannot virtualize anything if the guest does not have VMX
- * enabled.
- */
- set_default_vmx_state(state, state_sz);
- test_nested_state_expect_einval(vm, state);
-
- /*
- * We cannot virtualize anything if the guest does not have VMX
- * enabled.  We expect KVM_SET_NESTED_STATE to return 0 if vmxon_pa
- * is set to -1ull.
- */
- set_default_vmx_state(state, state_sz);
- state->vmx.vmxon_pa = -1ull;
- test_nested_state(vm, state);
-
- /* Enable VMX in the guest CPUID. */
- vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
-
  /* It is invalid to have vmxon_pa == -1ull and SMM flags non-zero. */
  set_default_vmx_state(state, state_sz);
  state->vmx.vmxon_pa = -1ull;
@@ -139,19 +120,28 @@ void test_vmx_nested_state(struct kvm_vm *vm)
  test_nested_state_expect_einval(vm, state);

  /* It is invalid to have vmxon_pa == -1ull and vmcs_pa != -1ull. */
- set_default_vmx_state(state, state_sz);
- state->vmx.vmxon_pa = -1ull;
- state->vmx.vmcs_pa = 0;
+ state->vmx.smm.flags = 0;
  test_nested_state_expect_einval(vm, state);

  /*
- * Setting vmxon_pa == -1ull and vmcs_pa == -1ull exits early without
- * setting the nested state.
+ * It is invalid to have vmxon_pa == -1ull and have one or both of the
+ * flags KVM_STATE_NESTED_RUN_PENDING or KVM_STATE_NESTED_GUEST_MODE
+ * set.
  */
- set_default_vmx_state(state, state_sz);
- state->vmx.vmxon_pa = -1ull;
+ state->flags = KVM_STATE_NESTED_RUN_PENDING |
+        KVM_STATE_NESTED_GUEST_MODE;
  state->vmx.vmcs_pa = -1ull;
- test_nested_state(vm, state);
+ test_nested_state_expect_einval(vm, state);
+
+ /*
+ * We cannot virtualize anything if the guest does not have VMX
+ * enabled.
+ */
+ set_default_vmx_state(state, state_sz);
+ test_nested_state_expect_einval(vm, state);
+
+ /* Enable VMX in the guest CPUID. */
+ vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());

  /* It is invalid to have vmxon_pa set to a non-page aligned address. */
  set_default_vmx_state(state, state_sz);
@@ -195,11 +185,26 @@ void test_vmx_nested_state(struct kvm_vm *vm)
  state->vmx.vmcs_pa = 0;
  test_nested_state_expect_einval(vm, state);

+ /*
+ * It is invalid to not have the vmcs_pa set when the flag
+ * KVM_STATE_NESTED_EVMCS is not set.
+ */
+ set_default_vmx_state(state, state_sz);
+ state->vmx.vmcs_pa = -1ull;
+ state->flags = KVM_STATE_NESTED_GUEST_MODE  |
+ KVM_STATE_NESTED_RUN_PENDING;
+ test_nested_state_expect_einval(vm, state);
+
  /* The revision id for vmcs12 must be VMCS12_REVISION. */
  set_default_vmx_state(state, state_sz);
  set_revision_id_for_vmcs12(state, 0);
  test_nested_state_expect_einval(vm, state);

+ /* The KVM_STATE_NESTED_GUEST_MODE flag must be set */
+ set_default_vmx_state(state, state_sz);
+ state->flags = KVM_STATE_NESTED_EVMCS;
+ test_nested_state(vm, state);
+
  /*
  * Test that if we leave nesting the state reflects that when we get
  * it again.
--