Re: [PATCH v3] KVM: VMX: Enable Notify VM exit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/25/2022 7:54 PM, Paolo Bonzini wrote:
On 2/23/22 07:24, Chenyi Qiang wrote:
Nested handling
- Nested notify VM exits are not supported yet. Keep the same notify
   window control in vmcs02 as vmcs01, so that L1 can't escape the
   restriction of notify VM exits through launching L2 VM.
- When L2 VM is context invalid, synthesize a nested
   EXIT_REASON_TRIPLE_FAULT to L1 so that L1 won't be killed due to L2's
   VM_CONTEXT_INVALID happens.

Notify VM exit is defined in latest Intel Architecture Instruction Set
Extensions Programming Reference, chapter 9.2.

TODO: Allow to change the window size (to enable the feature) at runtime,
which can make it more flexible to do management.

I only have a couple questions, any changes in response to the question
I can do myself.

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 1dfe23963a9e..f306b642c3e1 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2177,6 +2177,9 @@ static void prepare_vmcs02_constant_state(struct vcpu_vmx *vmx)
      if (cpu_has_vmx_encls_vmexit())
          vmcs_write64(ENCLS_EXITING_BITMAP, INVALID_GPA);
+    if (notify_window >= 0)
+        vmcs_write32(NOTIFY_WINDOW, notify_window);

Is a value of 0 valid?

Yes, 0 is valid. That's why there is an internal value to ensure even 0 won't cause false positive

Should it be changed to the recommended value of
128000 in hardware_setup()?

+    case EXIT_REASON_NOTIFY:
+        return nested_cpu_has2(vmcs12,
+            SECONDARY_EXEC_NOTIFY_VM_EXITING);

This should be "return false" since you don't expose the secondary
control to L1 (meaning, it will never be set).

Fine with either.

+         * L0 will synthensize a nested TRIPLE_FAULT to kill L2 when
+         * notify VM exit occurred in L2 and NOTIFY_VM_CONTEXT_INVALID is
+         * set in exit qualification. In this case, if notify VM exit
+         * occurred incident to delivery of a vectored event, the IDT
+         * vectoring info are recorded in VMCS. Drop the pending event
+         * in vmcs12, otherwise L1 VMM will exit to userspace with
+         * internal error due to delivery event.
          */
-        vmcs12_save_pending_event(vcpu, vmcs12);
+        if (to_vmx(vcpu)->exit_reason.basic != EXIT_REASON_NOTIFY)
+            vmcs12_save_pending_event(vcpu, vmcs12);

I would prefer to call out the triple fault here:

                 /*
                 * Transfer the event that L0 or L1 may have wanted to inject into
                  * L2 to IDT_VECTORING_INFO_FIELD.
                  *
                 * Skip this if the exit is due to a NOTIFY_VM_CONTEXT_INVALID                  * exit; in that case, L0 will synthesize a nested TRIPLE_FAULT                  * vmexit to kill L2.  No IDT vectoring info is recorded for                  * triple faults, and __vmx_handle_exit does not expect it.
                  */
                if (!(to_vmx(vcpu)->exit_reason.basic == EXIT_REASON_NOTIFY)
                       && kvm_test_request(KVM_REQ_TRIPLE_FAULT, vcpu))
                         vmcs12_save_pending_event(vcpu, vmcs12);

looks good to me.

What do you think?

Paolo





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux