[RFC,v6,14/92] kvm: introspection: handle introspection commands before returning to guest

Commit Message

Adalbert Lazăr Aug. 9, 2019, 3:59 p.m. UTC

From: Mihai Donțu <[e-mail]>

The introspection requests (KVM_REQ_INTROSPECTION) are checked by any
introspected vCPU in two places:

  * on its way to guest - vcpu_enter_guest()
  * when halted - kvm_vcpu_block()

In kvm_vcpu_block(), we check to see if there are any introspection
requests during the swait loop, handle them outside of swait loop and
start swait again.

Signed-off-by: Mihai Donțu <[e-mail]>
Co-developed-by: Mircea Cîrjaliu <[e-mail]>
Signed-off-by: Mircea Cîrjaliu <[e-mail]>
Signed-off-by: Adalbert Lazăr <[e-mail]>
---
 arch/x86/kvm/x86.c   |  3 +++
 include/linux/kvmi.h |  2 ++
 virt/kvm/kvm_main.c  | 28 ++++++++++++++++++++++------
 3 files changed, 27 insertions(+), 6 deletions(-)

Comments

Paolo Bonzini Aug. 13, 2019, 8:26 a.m. UTC | #1

On 09/08/19 17:59, Adalbert Lazăr wrote:
> +			prepare_to_swait_exclusive(&vcpu->wq, &wait,
> +						   TASK_INTERRUPTIBLE);
> +
> +			if (kvm_vcpu_check_block(vcpu) < 0)
> +				break;
> +
> +			waited = true;
> +			schedule();
> +
> +			if (kvm_check_request(KVM_REQ_INTROSPECTION, vcpu)) {
> +				do_kvmi_work = true;
> +				break;
> +			}
> +		}
>  
> -		waited = true;
> -		schedule();
> +		finish_swait(&vcpu->wq, &wait);
> +
> +		if (do_kvmi_work)
> +			kvmi_handle_requests(vcpu);
> +		else
> +			break;
>  	}

Is this needed?  Or can it just go back to KVM_RUN and handle
KVM_REQ_INTROSPECTION there (in which case it would be basically
premature optimization)?

Paolo

Paolo Bonzini Aug. 13, 2019, 2:45 p.m. UTC | #2

On 13/08/19 15:54, Adalbert Lazăr wrote:
>     Leaving kvm_vcpu_block() in order to handle a request such as 'pause',
>     would cause the vCPU to enter the guest when resumed. Most of the
>     time this does not appear to be an issue, but during early boot it
>     can happen for a non-boot vCPU to start executing code from areas that
>     first needed to be set up by vCPU #0.
>     
>     In a particular case, vCPU #1 executed code which resided in an area
>     not covered by a memslot, which caused an EPT violation that got
>     turned in mmu_set_spte() into a MMIO request that required emulation.
>     Unfortunatelly, the emulator tripped, exited to userspace and the VM
>     was aborted.

Okay, this makes sense.  Maybe you want to handle KVM_REQ_INTROSPECTION
in vcpu_run rather than vcpu_enter_guest?

Paolo

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0163e1ad1aaa..adbdb1ceb618 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7742,6 +7742,9 @@  static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 		 */
 		if (kvm_check_request(KVM_REQ_HV_STIMER, vcpu))
 			kvm_hv_process_stimers(vcpu);
+
+		if (kvm_check_request(KVM_REQ_INTROSPECTION, vcpu))
+			kvmi_handle_requests(vcpu);
 	}
 
 	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
diff --git a/include/linux/kvmi.h b/include/linux/kvmi.h
index e8d25d7da751..ae5de1905b55 100644
--- a/include/linux/kvmi.h
+++ b/include/linux/kvmi.h
@@ -16,6 +16,7 @@  int kvmi_ioctl_event(struct kvm *kvm, void __user *argp);
 int kvmi_ioctl_unhook(struct kvm *kvm, bool force_reset);
 int kvmi_vcpu_init(struct kvm_vcpu *vcpu);
 void kvmi_vcpu_uninit(struct kvm_vcpu *vcpu);
+void kvmi_handle_requests(struct kvm_vcpu *vcpu);
 
 #else
 
@@ -25,6 +26,7 @@  static inline void kvmi_create_vm(struct kvm *kvm) { }
 static inline void kvmi_destroy_vm(struct kvm *kvm) { }
 static inline int kvmi_vcpu_init(struct kvm_vcpu *vcpu) { return 0; }
 static inline void kvmi_vcpu_uninit(struct kvm_vcpu *vcpu) { }
+static inline void kvmi_handle_requests(struct kvm_vcpu *vcpu) { }
 
 #endif /* CONFIG_KVM_INTROSPECTION */
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 94f15f393e37..2e11069b9565 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2282,16 +2282,32 @@  void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 	kvm_arch_vcpu_blocking(vcpu);
 
 	for (;;) {
-		prepare_to_swait_exclusive(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+		bool do_kvmi_work = false;
 
-		if (kvm_vcpu_check_block(vcpu) < 0)
-			break;
+		for (;;) {
+			prepare_to_swait_exclusive(&vcpu->wq, &wait,
+						   TASK_INTERRUPTIBLE);
+
+			if (kvm_vcpu_check_block(vcpu) < 0)
+				break;
+
+			waited = true;
+			schedule();
+
+			if (kvm_check_request(KVM_REQ_INTROSPECTION, vcpu)) {
+				do_kvmi_work = true;
+				break;
+			}
+		}
 
-		waited = true;
-		schedule();
+		finish_swait(&vcpu->wq, &wait);
+
+		if (do_kvmi_work)
+			kvmi_handle_requests(vcpu);
+		else
+			break;
 	}
 
-	finish_swait(&vcpu->wq, &wait);
 	cur = ktime_get();
 
 	kvm_arch_vcpu_unblocking(vcpu);