[PATCH 0/6] KVM: x86: Make the cause of instruction emulation available to user-space

[PATCH 0/6] KVM: x86: Make the cause of instruction emulation available to user-space

[David Edmondson]

Instruction emulation happens for a variety of reasons, yet on error
we have no idea exactly what triggered it. Add a cause of emulation to
the various originators and pass it upstream when emulation fails.

Joao originally produced the patches but is busy with other things and
I wanted to use it, so picked it up.

Tested by reverting commit 51b958e5aeb1e18c00332e0b37c5d4e95a3eff84
("KVM: x86: clflushopt should be treated as a no-op by emulation")
then running the test included in
https://lore.kernel.org/r/20201118121129.6276-1-david.edmondson@oracle.com.

Joao Martins (6):
  KVM: x86: add an emulation_reason to x86_emulate_instruction()
  KVM: x86: pass emulation_reason to handle_emulation_failure()
  KVM: x86: add emulation_reason to kvm_emulate_instruction()
  KVM: x86: pass a proper reason to kvm_emulate_instruction()
  KVM: SVM: pass a proper reason in kvm_emulate_instruction()
  KVM: VMX: pass a proper reason in kvm_emulate_instruction()

 arch/x86/include/asm/kvm_host.h | 27 +++++++++++++++++++++++--
 arch/x86/kvm/mmu/mmu.c          |  4 ++--
 arch/x86/kvm/svm/avic.c         |  3 ++-
 arch/x86/kvm/svm/svm.c          | 26 +++++++++++++-----------
 arch/x86/kvm/vmx/vmx.c          | 17 ++++++++--------
 arch/x86/kvm/x86.c              | 35 ++++++++++++++++++++++-----------
 arch/x86/kvm/x86.h              |  3 ++-
 7 files changed, 78 insertions(+), 37 deletions(-)

-- 
2.30.2

[PATCH 1/6] KVM: x86: add an emulation_reason to x86_emulate_instruction()

[David Edmondson]

From: Joao Martins <joao.m.martins@oracle.com>

Emulation can happen for a variety of reasons, yet on error we have no
idea exactly what triggered it. Expand x86_emulate_instruction() to
carry an @emulation_reason argument.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 arch/x86/kvm/mmu/mmu.c | 4 ++--
 arch/x86/kvm/x86.c     | 7 ++++---
 arch/x86/kvm/x86.h     | 3 ++-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 951dae4e7175..515ff790b7c5 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5056,8 +5056,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 error_code,
 	if (!mmio_info_in_cache(vcpu, cr2_or_gpa, direct) && !is_guest_mode(vcpu))
 		emulation_type |= EMULTYPE_ALLOW_RETRY_PF;
 emulate:
-	return x86_emulate_instruction(vcpu, cr2_or_gpa, emulation_type, insn,
-				       insn_len);
+	return x86_emulate_instruction(vcpu, cr2_or_gpa, emulation_type, 0,
+				       insn, insn_len);
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_page_fault);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index eca63625aee4..87e76f3aee64 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7445,7 +7445,8 @@ int x86_decode_emulated_instruction(struct kvm_vcpu *vcpu, int emulation_type,
 EXPORT_SYMBOL_GPL(x86_decode_emulated_instruction);
 
 int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
-			    int emulation_type, void *insn, int insn_len)
+			    int emulation_type, int emulation_reason,
+			    void *insn, int insn_len)
 {
 	int r;
 	struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
@@ -7604,14 +7605,14 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 
 int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type)
 {
-	return x86_emulate_instruction(vcpu, 0, emulation_type, NULL, 0);
+	return x86_emulate_instruction(vcpu, 0, emulation_type, 0, NULL, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_instruction);
 
 int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu,
 					void *insn, int insn_len)
 {
-	return x86_emulate_instruction(vcpu, 0, 0, insn, insn_len);
+	return x86_emulate_instruction(vcpu, 0, 0, 0, insn, insn_len);
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_instruction_from_buffer);
 
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 9035e34aa156..5686436c99da 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -276,7 +276,8 @@ void kvm_fixup_and_inject_pf_error(struct kvm_vcpu *vcpu, gva_t gva, u16 error_c
 int x86_decode_emulated_instruction(struct kvm_vcpu *vcpu, int emulation_type,
 				    void *insn, int insn_len);
 int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
-			    int emulation_type, void *insn, int insn_len);
+			    int emulation_type, int emulation_reason,
+			    void *insn, int insn_len);
 fastpath_t handle_fastpath_set_msr_irqoff(struct kvm_vcpu *vcpu);
 
 extern u64 host_xcr0;
-- 
2.30.2

[PATCH 2/6] KVM: x86: pass emulation_reason to handle_emulation_failure()

[David Edmondson]

From: Joao Martins <joao.m.martins@oracle.com>

Make the emulation_reason available up stack when reporting an
emulation failure.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 arch/x86/kvm/x86.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 87e76f3aee64..b9225012ebd2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7119,7 +7119,8 @@ void kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip)
 }
 EXPORT_SYMBOL_GPL(kvm_inject_realmode_interrupt);
 
-static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type)
+static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type,
+				    int emulation_reason)
 {
 	++vcpu->stat.insn_emulation_fail;
 	trace_kvm_emulate_insn_failed(vcpu);
@@ -7132,7 +7133,8 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type)
 	if (emulation_type & EMULTYPE_SKIP) {
 		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
 		vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
-		vcpu->run->internal.ndata = 0;
+		vcpu->run->internal.data[0] = emulation_reason;
+		vcpu->run->internal.ndata = 1;
 		return 0;
 	}
 
@@ -7141,7 +7143,8 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu, int emulation_type)
 	if (!is_guest_mode(vcpu) && static_call(kvm_x86_get_cpl)(vcpu) == 0) {
 		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
 		vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
-		vcpu->run->internal.ndata = 0;
+		vcpu->run->internal.data[0] = emulation_reason;
+		vcpu->run->internal.ndata = 1;
 		return 0;
 	}
 
@@ -7490,7 +7493,8 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 				inject_emulated_exception(vcpu);
 				return 1;
 			}
-			return handle_emulation_failure(vcpu, emulation_type);
+			return handle_emulation_failure(vcpu, emulation_type,
+							emulation_reason);
 		}
 	}
 
@@ -7547,7 +7551,8 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 					emulation_type))
 			return 1;
 
-		return handle_emulation_failure(vcpu, emulation_type);
+		return handle_emulation_failure(vcpu, emulation_type,
+						emulation_reason);
 	}
 
 	if (ctxt->have_exception) {
-- 
2.30.2

[PATCH 3/6] KVM: x86: add emulation_reason to kvm_emulate_instruction()

[David Edmondson]

From: Joao Martins <joao.m.martins@oracle.com>

Allow kvm_emulate_instruction() callers to provide the cause of
instruction emulation, which is then passed along to
x86_emulation_instruction().

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 10 ++++++++--
 arch/x86/kvm/svm/avic.c         |  2 +-
 arch/x86/kvm/svm/svm.c          | 12 ++++++------
 arch/x86/kvm/vmx/vmx.c          | 16 ++++++++--------
 arch/x86/kvm/x86.c              | 16 ++++++++++------
 5 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3768819693e5..556dc51e322a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1527,9 +1527,15 @@ extern u64 kvm_mce_cap_supported;
 #define EMULTYPE_VMWARE_GP	    (1 << 5)
 #define EMULTYPE_PF		    (1 << 6)
 
-int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type);
+enum {
+	EMULREASON_UNKNOWN = 0,
+};
+
+int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type,
+			    int emulation_reason);
 int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu,
-					void *insn, int insn_len);
+					void *insn, int insn_len,
+					int emulation_reason);
 
 void kvm_enable_efer_bits(u64);
 bool kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer);
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 78bdcfac4e40..31a17fa6a37c 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -558,7 +558,7 @@ int avic_unaccelerated_access_interception(struct vcpu_svm *svm)
 		ret = avic_unaccel_trap_write(svm);
 	} else {
 		/* Handling Fault */
-		ret = kvm_emulate_instruction(&svm->vcpu, 0);
+		ret = kvm_emulate_instruction(&svm->vcpu, 0, 0);
 	}
 
 	return ret;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 58a45bb139f8..bba3b72390a8 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -344,7 +344,7 @@ static int skip_emulated_instruction(struct kvm_vcpu *vcpu)
 	}
 
 	if (!svm->next_rip) {
-		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP))
+		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP, 0))
 			return 0;
 	} else {
 		kvm_rip_write(vcpu, svm->next_rip);
@@ -2077,7 +2077,7 @@ static int io_interception(struct vcpu_svm *svm)
 		if (sev_es_guest(vcpu->kvm))
 			return sev_es_string_io(svm, size, port, in);
 		else
-			return kvm_emulate_instruction(vcpu, 0);
+			return kvm_emulate_instruction(vcpu, 0, 0);
 	}
 
 	svm->next_rip = svm->vmcb->control.exit_info_2;
@@ -2263,7 +2263,7 @@ static int gp_interception(struct vcpu_svm *svm)
 		 */
 		if (!is_guest_mode(vcpu))
 			return kvm_emulate_instruction(vcpu,
-				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
+				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE, 0);
 	} else
 		return emulate_svm_instr(vcpu, opcode);
 
@@ -2459,7 +2459,7 @@ static int invd_interception(struct vcpu_svm *svm)
 static int invlpg_interception(struct vcpu_svm *svm)
 {
 	if (!static_cpu_has(X86_FEATURE_DECODEASSISTS))
-		return kvm_emulate_instruction(&svm->vcpu, 0);
+		return kvm_emulate_instruction(&svm->vcpu, 0, 0);
 
 	kvm_mmu_invlpg(&svm->vcpu, svm->vmcb->control.exit_info_1);
 	return kvm_skip_emulated_instruction(&svm->vcpu);
@@ -2467,12 +2467,12 @@ static int invlpg_interception(struct vcpu_svm *svm)
 
 static int emulate_on_interception(struct vcpu_svm *svm)
 {
-	return kvm_emulate_instruction(&svm->vcpu, 0);
+	return kvm_emulate_instruction(&svm->vcpu, 0, 0);
 }
 
 static int rsm_interception(struct vcpu_svm *svm)
 {
-	return kvm_emulate_instruction_from_buffer(&svm->vcpu, rsm_ins_bytes, 2);
+	return kvm_emulate_instruction_from_buffer(&svm->vcpu, rsm_ins_bytes, 2, 0);
 }
 
 static int rdpmc_interception(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 32cf8287d4a7..037b01b5a54b 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1600,7 +1600,7 @@ static int skip_emulated_instruction(struct kvm_vcpu *vcpu)
 #endif
 		kvm_rip_write(vcpu, rip);
 	} else {
-		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP))
+		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP, 0))
 			return 0;
 	}
 
@@ -4738,7 +4738,7 @@ static int handle_rmode_exception(struct kvm_vcpu *vcpu,
 	 * Cause the #SS fault with 0 error code in VM86 mode.
 	 */
 	if (((vec == GP_VECTOR) || (vec == SS_VECTOR)) && err_code == 0) {
-		if (kvm_emulate_instruction(vcpu, 0)) {
+		if (kvm_emulate_instruction(vcpu, 0, 0)) {
 			if (vcpu->arch.halt_request) {
 				vcpu->arch.halt_request = 0;
 				return kvm_vcpu_halt(vcpu);
@@ -4816,7 +4816,7 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
 			kvm_queue_exception_e(vcpu, GP_VECTOR, error_code);
 			return 1;
 		}
-		return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP);
+		return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP, 0);
 	}
 
 	/*
@@ -4930,7 +4930,7 @@ static int handle_io(struct kvm_vcpu *vcpu)
 	++vcpu->stat.io_exits;
 
 	if (string)
-		return kvm_emulate_instruction(vcpu, 0);
+		return kvm_emulate_instruction(vcpu, 0, 0);
 
 	port = exit_qualification >> 16;
 	size = (exit_qualification & 7) + 1;
@@ -5004,7 +5004,7 @@ static int handle_set_cr4(struct kvm_vcpu *vcpu, unsigned long val)
 static int handle_desc(struct kvm_vcpu *vcpu)
 {
 	WARN_ON(!(vcpu->arch.cr4 & X86_CR4_UMIP));
-	return kvm_emulate_instruction(vcpu, 0);
+	return kvm_emulate_instruction(vcpu, 0, 0);
 }
 
 static int handle_cr(struct kvm_vcpu *vcpu)
@@ -5244,7 +5244,7 @@ static int handle_apic_access(struct kvm_vcpu *vcpu)
 			return kvm_skip_emulated_instruction(vcpu);
 		}
 	}
-	return kvm_emulate_instruction(vcpu, 0);
+	return kvm_emulate_instruction(vcpu, 0, 0);
 }
 
 static int handle_apic_eoi_induced(struct kvm_vcpu *vcpu)
@@ -5375,7 +5375,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
 	 * reconstruct the page fault error code.
 	 */
 	if (unlikely(allow_smaller_maxphyaddr && kvm_vcpu_is_illegal_gpa(vcpu, gpa)))
-		return kvm_emulate_instruction(vcpu, 0);
+		return kvm_emulate_instruction(vcpu, 0, 0);
 
 	return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
 }
@@ -5424,7 +5424,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
 		if (kvm_test_request(KVM_REQ_EVENT, vcpu))
 			return 1;
 
-		if (!kvm_emulate_instruction(vcpu, 0))
+		if (!kvm_emulate_instruction(vcpu, 0, 0))
 			return 0;
 
 		if (vmx->emulation_required && !vmx->rmode.vm86_active &&
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b9225012ebd2..88519bf6bd00 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6197,7 +6197,7 @@ int handle_ud(struct kvm_vcpu *vcpu)
 		emul_type = EMULTYPE_TRAP_UD_FORCED;
 	}
 
-	return kvm_emulate_instruction(vcpu, emul_type);
+	return kvm_emulate_instruction(vcpu, emul_type, 0);
 }
 EXPORT_SYMBOL_GPL(handle_ud);
 
@@ -7608,16 +7608,20 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 	return r;
 }
 
-int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type)
+int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type,
+			    int emulation_reason)
 {
-	return x86_emulate_instruction(vcpu, 0, emulation_type, 0, NULL, 0);
+	return x86_emulate_instruction(vcpu, 0, emulation_type,
+				       emulation_reason, NULL, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_instruction);
 
 int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu,
-					void *insn, int insn_len)
+					void *insn, int insn_len,
+					int emulation_reason)
 {
-	return x86_emulate_instruction(vcpu, 0, 0, 0, insn, insn_len);
+	return x86_emulate_instruction(vcpu, 0, 0,
+				       emulation_reason, insn, insn_len);
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_instruction_from_buffer);
 
@@ -9339,7 +9343,7 @@ static inline int complete_emulated_io(struct kvm_vcpu *vcpu)
 	int r;
 
 	vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
-	r = kvm_emulate_instruction(vcpu, EMULTYPE_NO_DECODE);
+	r = kvm_emulate_instruction(vcpu, EMULTYPE_NO_DECODE, 0);
 	srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
 	return r;
 }
-- 
2.30.2

[PATCH 4/6] KVM: x86: pass a proper reason to kvm_emulate_instruction()

[David Edmondson]

From: Joao Martins <joao.m.martins@oracle.com>

Declare various causes of emulation and use them as appropriate.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 arch/x86/include/asm/kvm_host.h | 6 ++++++
 arch/x86/kvm/mmu/mmu.c          | 4 ++--
 arch/x86/kvm/x86.c              | 5 +++--
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 556dc51e322a..79e9ca756742 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1529,6 +1529,12 @@ extern u64 kvm_mce_cap_supported;
 
 enum {
 	EMULREASON_UNKNOWN = 0,
+	EMULREASON_SKIP,
+	EMULREASON_GP,
+	EMULREASON_IO,
+	EMULREASON_IO_COMPLETE,
+	EMULREASON_UD,
+	EMULREASON_PF,
 };
 
 int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type,
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 515ff790b7c5..14ddf1a5ac12 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5056,8 +5056,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 error_code,
 	if (!mmio_info_in_cache(vcpu, cr2_or_gpa, direct) && !is_guest_mode(vcpu))
 		emulation_type |= EMULTYPE_ALLOW_RETRY_PF;
 emulate:
-	return x86_emulate_instruction(vcpu, cr2_or_gpa, emulation_type, 0,
-				       insn, insn_len);
+	return x86_emulate_instruction(vcpu, cr2_or_gpa, emulation_type,
+				       EMULREASON_PF, insn, insn_len);
 }
 EXPORT_SYMBOL_GPL(kvm_mmu_page_fault);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 88519bf6bd00..41020eba8e21 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6197,7 +6197,7 @@ int handle_ud(struct kvm_vcpu *vcpu)
 		emul_type = EMULTYPE_TRAP_UD_FORCED;
 	}
 
-	return kvm_emulate_instruction(vcpu, emul_type, 0);
+	return kvm_emulate_instruction(vcpu, emul_type, EMULREASON_UD);
 }
 EXPORT_SYMBOL_GPL(handle_ud);
 
@@ -9343,7 +9343,8 @@ static inline int complete_emulated_io(struct kvm_vcpu *vcpu)
 	int r;
 
 	vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
-	r = kvm_emulate_instruction(vcpu, EMULTYPE_NO_DECODE, 0);
+	r = kvm_emulate_instruction(vcpu, EMULTYPE_NO_DECODE,
+				    EMULREASON_IO_COMPLETE);
 	srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
 	return r;
 }
-- 
2.30.2

[PATCH 5/6] KVM: SVM: pass a proper reason in kvm_emulate_instruction()

[David Edmondson]

From: Joao Martins <joao.m.martins@oracle.com>

Declare various causes of emulation and use them as appropriate.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 arch/x86/include/asm/kvm_host.h |  6 ++++++
 arch/x86/kvm/svm/avic.c         |  3 ++-
 arch/x86/kvm/svm/svm.c          | 26 +++++++++++++++-----------
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 79e9ca756742..e1284680cbdc 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1535,6 +1535,12 @@ enum {
 	EMULREASON_IO_COMPLETE,
 	EMULREASON_UD,
 	EMULREASON_PF,
+	EMULREASON_SVM_NOASSIST,
+	EMULREASON_SVM_RSM,
+	EMULREASON_SVM_RDPMC,
+	EMULREASON_SVM_CR,
+	EMULREASON_SVM_DR,
+	EMULREASON_SVM_AVIC_UNACCEL,
 };
 
 int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type,
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 31a17fa6a37c..faa5d4db7ccc 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -558,7 +558,8 @@ int avic_unaccelerated_access_interception(struct vcpu_svm *svm)
 		ret = avic_unaccel_trap_write(svm);
 	} else {
 		/* Handling Fault */
-		ret = kvm_emulate_instruction(&svm->vcpu, 0, 0);
+		ret = kvm_emulate_instruction(&svm->vcpu, 0,
+					      EMULREASON_SVM_AVIC_UNACCEL);
 	}
 
 	return ret;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index bba3b72390a8..2646aa2eae22 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -344,7 +344,8 @@ static int skip_emulated_instruction(struct kvm_vcpu *vcpu)
 	}
 
 	if (!svm->next_rip) {
-		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP, 0))
+		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP,
+					     EMULREASON_SKIP))
 			return 0;
 	} else {
 		kvm_rip_write(vcpu, svm->next_rip);
@@ -2077,7 +2078,8 @@ static int io_interception(struct vcpu_svm *svm)
 		if (sev_es_guest(vcpu->kvm))
 			return sev_es_string_io(svm, size, port, in);
 		else
-			return kvm_emulate_instruction(vcpu, 0, 0);
+			return kvm_emulate_instruction(vcpu, 0,
+						       EMULREASON_IO);
 	}
 
 	svm->next_rip = svm->vmcb->control.exit_info_2;
@@ -2263,7 +2265,8 @@ static int gp_interception(struct vcpu_svm *svm)
 		 */
 		if (!is_guest_mode(vcpu))
 			return kvm_emulate_instruction(vcpu,
-				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE, 0);
+				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE,
+				EMULREASON_GP);
 	} else
 		return emulate_svm_instr(vcpu, opcode);
 
@@ -2459,20 +2462,21 @@ static int invd_interception(struct vcpu_svm *svm)
 static int invlpg_interception(struct vcpu_svm *svm)
 {
 	if (!static_cpu_has(X86_FEATURE_DECODEASSISTS))
-		return kvm_emulate_instruction(&svm->vcpu, 0, 0);
+		return kvm_emulate_instruction(&svm->vcpu, 0, EMULREASON_SVM_NOASSIST);
 
 	kvm_mmu_invlpg(&svm->vcpu, svm->vmcb->control.exit_info_1);
 	return kvm_skip_emulated_instruction(&svm->vcpu);
 }
 
-static int emulate_on_interception(struct vcpu_svm *svm)
+static int emulate_on_interception(struct vcpu_svm *svm, int emulation_reason)
 {
-	return kvm_emulate_instruction(&svm->vcpu, 0, 0);
+	return kvm_emulate_instruction(&svm->vcpu, 0, emulation_reason);
 }
 
 static int rsm_interception(struct vcpu_svm *svm)
 {
-	return kvm_emulate_instruction_from_buffer(&svm->vcpu, rsm_ins_bytes, 2, 0);
+	return kvm_emulate_instruction_from_buffer(&svm->vcpu, rsm_ins_bytes, 2,
+						   EMULREASON_SVM_RSM);
 }
 
 static int rdpmc_interception(struct vcpu_svm *svm)
@@ -2480,7 +2484,7 @@ static int rdpmc_interception(struct vcpu_svm *svm)
 	int err;
 
 	if (!nrips)
-		return emulate_on_interception(svm);
+		return emulate_on_interception(svm, EMULREASON_SVM_RDPMC);
 
 	err = kvm_rdpmc(&svm->vcpu);
 	return kvm_complete_insn_gp(&svm->vcpu, err);
@@ -2516,10 +2520,10 @@ static int cr_interception(struct vcpu_svm *svm)
 	int err;
 
 	if (!static_cpu_has(X86_FEATURE_DECODEASSISTS))
-		return emulate_on_interception(svm);
+		return emulate_on_interception(svm, EMULREASON_SVM_NOASSIST);
 
 	if (unlikely((svm->vmcb->control.exit_info_1 & CR_VALID) == 0))
-		return emulate_on_interception(svm);
+		return emulate_on_interception(svm, EMULREASON_SVM_CR);
 
 	reg = svm->vmcb->control.exit_info_1 & SVM_EXITINFO_REG_MASK;
 	if (svm->vmcb->control.exit_code == SVM_EXIT_CR0_SEL_WRITE)
@@ -2635,7 +2639,7 @@ static int dr_interception(struct vcpu_svm *svm)
 	}
 
 	if (!boot_cpu_has(X86_FEATURE_DECODEASSISTS))
-		return emulate_on_interception(svm);
+		return emulate_on_interception(svm, EMULREASON_SVM_NOASSIST);
 
 	reg = svm->vmcb->control.exit_info_1 & SVM_EXITINFO_REG_MASK;
 	dr = svm->vmcb->control.exit_code - SVM_EXIT_READ_DR0;
-- 
2.30.2

Re: [PATCH 5/6] KVM: SVM: pass a proper reason in kvm_emulate_instruction()

[Sean Christopherson]

+Aaron

On Mon, Apr 12, 2021, David Edmondson wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
> 
> Declare various causes of emulation and use them as appropriate.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  6 ++++++
>  arch/x86/kvm/svm/avic.c         |  3 ++-
>  arch/x86/kvm/svm/svm.c          | 26 +++++++++++++++-----------
>  3 files changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 79e9ca756742..e1284680cbdc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1535,6 +1535,12 @@ enum {
>  	EMULREASON_IO_COMPLETE,
>  	EMULREASON_UD,
>  	EMULREASON_PF,
> +	EMULREASON_SVM_NOASSIST,
> +	EMULREASON_SVM_RSM,
> +	EMULREASON_SVM_RDPMC,
> +	EMULREASON_SVM_CR,
> +	EMULREASON_SVM_DR,
> +	EMULREASON_SVM_AVIC_UNACCEL,

Passing these to userspace arguably makes them ABI, i.e. they need to go into
uapi/kvm.h somewhere.  That said, I don't like passing arbitrary values for what
is effectively the VM-Exit reason.  Why not simply pass the exit reason, assuming
we do indeed want to dump this info to userspace?

What is the intended end usage of this information?  Actual emulation?  Debug?
Logging?

Depending on what you're trying to do with the info, maybe there's a better
option.  E.g. Aaron is working on a series that includes passing pass the code
stream (instruction bytes) to userspace on emulation failure, though I'm not
sure if he's planning on providing the VM-Exit reason.

Re: [PATCH 5/6] KVM: SVM: pass a proper reason in kvm_emulate_instruction()

[David Edmondson]

On Monday, 2021-04-12 at 16:04:02 GMT, Sean Christopherson wrote:

> +Aaron
>
> On Mon, Apr 12, 2021, David Edmondson wrote:
>> From: Joao Martins <joao.m.martins@oracle.com>
>> 
>> Declare various causes of emulation and use them as appropriate.
>> 
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>> ---
>>  arch/x86/include/asm/kvm_host.h |  6 ++++++
>>  arch/x86/kvm/svm/avic.c         |  3 ++-
>>  arch/x86/kvm/svm/svm.c          | 26 +++++++++++++++-----------
>>  3 files changed, 23 insertions(+), 12 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 79e9ca756742..e1284680cbdc 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -1535,6 +1535,12 @@ enum {
>>  	EMULREASON_IO_COMPLETE,
>>  	EMULREASON_UD,
>>  	EMULREASON_PF,
>> +	EMULREASON_SVM_NOASSIST,
>> +	EMULREASON_SVM_RSM,
>> +	EMULREASON_SVM_RDPMC,
>> +	EMULREASON_SVM_CR,
>> +	EMULREASON_SVM_DR,
>> +	EMULREASON_SVM_AVIC_UNACCEL,
>
> Passing these to userspace arguably makes them ABI, i.e. they need to go into
> uapi/kvm.h somewhere.  That said, I don't like passing arbitrary values for what
> is effectively the VM-Exit reason.  Why not simply pass the exit reason, assuming
> we do indeed want to dump this info to userspace?

That would suffice, yes.

> What is the intended end usage of this information?  Actual emulation?  Debug?
> Logging?

Debug (which implies logging, given that I want this to happen on
systems that are in service).

> Depending on what you're trying to do with the info, maybe there's a better
> option.  E.g. Aaron is working on a series that includes passing pass the code
> stream (instruction bytes) to userspace on emulation failure, though I'm not
> sure if he's planning on providing the VM-Exit reason.

Having the instruction stream will be good.

Aaron: do you have anything to share now? In what time frame do you
think you might submit patches?

I'm happy to re-work this to make the exit reason available, if that's
the appropriate direction.

dme.
-- 
And you're standing here beside me, I love the passing of time.

Re: [PATCH 5/6] KVM: SVM: pass a proper reason in kvm_emulate_instruction()

[Aaron Lewis]

>
> > Depending on what you're trying to do with the info, maybe there's a better
> > option.  E.g. Aaron is working on a series that includes passing pass the code
> > stream (instruction bytes) to userspace on emulation failure, though I'm not
> > sure if he's planning on providing the VM-Exit reason.
>
> Having the instruction stream will be good.
>
> Aaron: do you have anything to share now? In what time frame do you
> think you might submit patches?

I should be able to have something out later this week.  There is no
exit reason as Sean indicated, so if that's important it will have to
be reworked afterwards.  For struct internal in kvm_run I use data[0]
for flags to indicate what's contained in the rest of it, I use
data[1] as the instruction size, and I use data[2,3] to store the
instruction bytes.  Hope that helps.

>
> I'm happy to re-work this to make the exit reason available, if that's
> the appropriate direction.
>
> dme.
> --
> And you're standing here beside me, I love the passing of time.

Re: [PATCH 5/6] KVM: SVM: pass a proper reason in kvm_emulate_instruction()

[David Edmondson]

On Tuesday, 2021-04-13 at 11:45:52 -07, Aaron Lewis wrote:

>>
>> > Depending on what you're trying to do with the info, maybe there's a better
>> > option.  E.g. Aaron is working on a series that includes passing pass the code
>> > stream (instruction bytes) to userspace on emulation failure, though I'm not
>> > sure if he's planning on providing the VM-Exit reason.
>>
>> Having the instruction stream will be good.
>>
>> Aaron: do you have anything to share now? In what time frame do you
>> think you might submit patches?
>
> I should be able to have something out later this week.  There is no
> exit reason as Sean indicated, so if that's important it will have to
> be reworked afterwards.  For struct internal in kvm_run I use data[0]
> for flags to indicate what's contained in the rest of it, I use
> data[1] as the instruction size, and I use data[2,3] to store the
> instruction bytes.  Hope that helps.

Thanks. I'll hang on to look at the patches before doing anything else.

dme.
-- 
Tell me sweet little lies.

[PATCH 6/6] KVM: VMX: pass a proper reason in kvm_emulate_instruction()

[David Edmondson]

From: Joao Martins <joao.m.martins@oracle.com>

Declare various causes of emulation and use them as appropriate.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
 arch/x86/include/asm/kvm_host.h |  5 +++++
 arch/x86/kvm/vmx/vmx.c          | 17 +++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e1284680cbdc..f401e7c79ded 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1541,6 +1541,11 @@ enum {
 	EMULREASON_SVM_CR,
 	EMULREASON_SVM_DR,
 	EMULREASON_SVM_AVIC_UNACCEL,
+	EMULREASON_VMX_APIC_ACCESS,
+	EMULREASON_VMX_EPT_VIOLATION,
+	EMULREASON_VMX_DESC,
+	EMULREASON_VMX_INV_GUEST,
+	EMULREASON_VMX_RMODE_EX,
 };
 
 int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 037b01b5a54b..799eb0713b76 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1600,7 +1600,7 @@ static int skip_emulated_instruction(struct kvm_vcpu *vcpu)
 #endif
 		kvm_rip_write(vcpu, rip);
 	} else {
-		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP, 0))
+		if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP, EMULREASON_SKIP))
 			return 0;
 	}
 
@@ -4738,7 +4738,7 @@ static int handle_rmode_exception(struct kvm_vcpu *vcpu,
 	 * Cause the #SS fault with 0 error code in VM86 mode.
 	 */
 	if (((vec == GP_VECTOR) || (vec == SS_VECTOR)) && err_code == 0) {
-		if (kvm_emulate_instruction(vcpu, 0, 0)) {
+		if (kvm_emulate_instruction(vcpu, 0, EMULREASON_VMX_RMODE_EX)) {
 			if (vcpu->arch.halt_request) {
 				vcpu->arch.halt_request = 0;
 				return kvm_vcpu_halt(vcpu);
@@ -4816,7 +4816,8 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
 			kvm_queue_exception_e(vcpu, GP_VECTOR, error_code);
 			return 1;
 		}
-		return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP, 0);
+		return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP,
+					       EMULREASON_GP);
 	}
 
 	/*
@@ -4930,7 +4931,7 @@ static int handle_io(struct kvm_vcpu *vcpu)
 	++vcpu->stat.io_exits;
 
 	if (string)
-		return kvm_emulate_instruction(vcpu, 0, 0);
+		return kvm_emulate_instruction(vcpu, 0, EMULREASON_IO);
 
 	port = exit_qualification >> 16;
 	size = (exit_qualification & 7) + 1;
@@ -5004,7 +5005,7 @@ static int handle_set_cr4(struct kvm_vcpu *vcpu, unsigned long val)
 static int handle_desc(struct kvm_vcpu *vcpu)
 {
 	WARN_ON(!(vcpu->arch.cr4 & X86_CR4_UMIP));
-	return kvm_emulate_instruction(vcpu, 0, 0);
+	return kvm_emulate_instruction(vcpu, 0, EMULREASON_VMX_DESC);
 }
 
 static int handle_cr(struct kvm_vcpu *vcpu)
@@ -5244,7 +5245,7 @@ static int handle_apic_access(struct kvm_vcpu *vcpu)
 			return kvm_skip_emulated_instruction(vcpu);
 		}
 	}
-	return kvm_emulate_instruction(vcpu, 0, 0);
+	return kvm_emulate_instruction(vcpu, 0, EMULREASON_VMX_APIC_ACCESS);
 }
 
 static int handle_apic_eoi_induced(struct kvm_vcpu *vcpu)
@@ -5375,7 +5376,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
 	 * reconstruct the page fault error code.
 	 */
 	if (unlikely(allow_smaller_maxphyaddr && kvm_vcpu_is_illegal_gpa(vcpu, gpa)))
-		return kvm_emulate_instruction(vcpu, 0, 0);
+		return kvm_emulate_instruction(vcpu, 0, EMULREASON_VMX_EPT_VIOLATION);
 
 	return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
 }
@@ -5424,7 +5425,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
 		if (kvm_test_request(KVM_REQ_EVENT, vcpu))
 			return 1;
 
-		if (!kvm_emulate_instruction(vcpu, 0, 0))
+		if (!kvm_emulate_instruction(vcpu, 0, EMULREASON_VMX_INV_GUEST))
 			return 0;
 
 		if (vmx->emulation_required && !vmx->rmode.vm86_active &&
-- 
2.30.2

Re: [PATCH 0/6] KVM: x86: Make the cause of instruction emulation available to user-space

[Jim Mattson]

On Mon, Apr 12, 2021 at 6:09 AM David Edmondson
<david.edmondson@oracle.com> wrote:
>
> Instruction emulation happens for a variety of reasons, yet on error
> we have no idea exactly what triggered it. Add a cause of emulation to
> the various originators and pass it upstream when emulation fails.

What is userspace going to do with this information? It's hard to say
whether or not this is the right ABI without more context.

Re: [PATCH 0/6] KVM: x86: Make the cause of instruction emulation available to user-space

[David Edmondson]

On Monday, 2021-04-12 at 11:34:33 -07, Jim Mattson wrote:

> On Mon, Apr 12, 2021 at 6:09 AM David Edmondson
> <david.edmondson@oracle.com> wrote:
>>
>> Instruction emulation happens for a variety of reasons, yet on error
>> we have no idea exactly what triggered it. Add a cause of emulation to
>> the various originators and pass it upstream when emulation fails.
>
> What is userspace going to do with this information? It's hard to say
> whether or not this is the right ABI without more context.

Logging for debug purposes, see reply to Sean.

dme.
-- 
You make me feel like a natural woman.