Re: [RESEND PATCH 07/13] KVM: x86: Add explicit flag for forced emulation on #UD

From: Liran Alon <liran.alon@oracle.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	"Vitaly Kuznetsov" <vkuznets@redhat.com>,
	"Wanpeng Li" <wanpengli@tencent.com>,
	"Jim Mattson" <jmattson@google.com>,
	"Joerg Roedel" <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RESEND PATCH 07/13] KVM: x86: Add explicit flag for forced emulation on #UD
Date: Fri, 23 Aug 2019 16:47:14 +0300	[thread overview]
Message-ID: <9E01A06E-FD3E-4D43-9FFE-6FFE3BAC269A@oracle.com> (raw)
In-Reply-To: <20190823010709.24879-8-sean.j.christopherson@intel.com>

> On 23 Aug 2019, at 4:07, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> 
> Add an explicit emulation type for forced #UD emulation and use it to
> detect that KVM should unconditionally inject a #UD instead of falling
> into its standard emulation failure handling.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

The name "forced emulation on #UD" is not clear to me.

If I understand correctly, EMULTYPE_TRAP_UD is currently used to indicate
that in case the x86 emulator fails to decode instruction, the caller would like
the x86 emulator to fail early such that it can handle this condition properly.
Thus, I would rename it EMULTYPE_TRAP_DECODE_FAILURE.

But this new flag seems to do the same. So I’m left confused.
I’m probably missing something trivial here.

-Liran

> ---
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/x86.c              | 5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index d1d5b5ca1195..a38c93362945 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1318,6 +1318,7 @@ enum emulation_result {
> #define EMULTYPE_TRAP_UD	    (1 << 1)
> #define EMULTYPE_SKIP		    (1 << 2)
> #define EMULTYPE_ALLOW_RETRY	    (1 << 3)
> +#define EMULTYPE_TRAP_UD_FORCED	    (1 << 4)
> #define EMULTYPE_VMWARE_GP	    (1 << 5)
> int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type);
> int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 228ca71d5b01..a1f9e36b2d58 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5337,7 +5337,7 @@ int handle_ud(struct kvm_vcpu *vcpu)
> 				sig, sizeof(sig), &e) == 0 &&
> 	    memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
> 		kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
> -		emul_type = 0;
> +		emul_type = EMULTYPE_TRAP_UD_FORCED;
> 	}
> 
> 	er = kvm_emulate_instruction(vcpu, emul_type);
> @@ -6532,7 +6532,8 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
> 		trace_kvm_emulate_insn_start(vcpu);
> 		++vcpu->stat.insn_emulation;
> 		if (r != EMULATION_OK)  {
> -			if (emulation_type & EMULTYPE_TRAP_UD)
> +			if ((emulation_type & EMULTYPE_TRAP_UD) ||
> +			    (emulation_type & EMULTYPE_TRAP_UD_FORCED))
> 				return EMULATE_FAIL;
> 			if (reexecute_instruction(vcpu, cr2, write_fault_to_spt,
> 						emulation_type))
> -- 
> 2.22.0
> 