From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] KVM: x86: Avoid emulating instructions on #UD mistakenly Date: Mon, 18 Aug 2014 10:31:32 +0200 Message-ID: <53F1B9E4.2090008@redhat.com> References: <1407937813-1461-1-git-send-email-namit@cs.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: gleb@kernel.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , the arch/x86 maintainers , KVM , Linux Kernel Mailing List , Nadav Amit To: Nadav Amit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:63759 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751529AbaHRIby (ORCPT ); Mon, 18 Aug 2014 04:31:54 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Il 13/08/2014 16:21, Nadav Amit ha scritto: > Correction: the word =93never=94 in the message is too harsh. > Nonetheless, there is a regression bug. I encountered it with =93wrfs= base=94 instruction. So KVM is emulating wrfsbase even if the host doesn't support it? I'm swapping the order of the two operands of &&, since the first one w= ill almost always be true and the second one will almost always be false. Also, there's now no need to test EmulateOnUD in the condition below. = Does the below look good to you? diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 37a83b24e040..ef117b842334 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4394,11 +4394,11 @@ done_prefixes: =20 ctxt->execute =3D opcode.u.execute; =20 - if (!(ctxt->d & EmulateOnUD) && ctxt->ud) + if (unlikely(ctxt->ud) && likely(!(ctxt->d & EmulateOnUD))) return EMULATION_FAILED; =20 if (unlikely(ctxt->d & - (NotImpl|EmulateOnUD|Stack|Op3264|Sse|Mmx|Intercept|CheckPerm))= ) { + (NotImpl|Stack|Op3264|Sse|Mmx|Intercept|CheckPerm))) { /* * These are copied unconditionally here, and checked unconditionall= y * in x86_emulate_insn. =20 Paolo > Nadav >=20 > On Aug 13, 2014, at 4:50 PM, Nadav Amit wro= te: >=20 >> Commit d40a6898e5 mistakenly caused instructions which are not marke= d as >> EmulateOnUD to be emulated upon #UD exception. The commit caused the= check of >> whether the instruction flags include EmulateOnUD to never be evalua= ted. As a >> result instructions whose emulation is broken may be emulated. This= fix moves >> the evaluation of EmulateOnUD so it would be evaluated. >> >> Signed-off-by: Nadav Amit >> --- >> arch/x86/kvm/emulate.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index 56657b0..37a83b2 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -4394,6 +4394,9 @@ done_prefixes: >> >> ctxt->execute =3D opcode.u.execute; >> >> + if (!(ctxt->d & EmulateOnUD) && ctxt->ud) >> + return EMULATION_FAILED; >> + >> if (unlikely(ctxt->d & >> (NotImpl|EmulateOnUD|Stack|Op3264|Sse|Mmx|Intercept|CheckPer= m))) { >> /* >> @@ -4406,9 +4409,6 @@ done_prefixes: >> if (ctxt->d & NotImpl) >> return EMULATION_FAILED; >> >> - if (!(ctxt->d & EmulateOnUD) && ctxt->ud) >> - return EMULATION_FAILED; >> - >> if (mode =3D=3D X86EMUL_MODE_PROT64 && (ctxt->d & Stack)) >> ctxt->op_bytes =3D 8; >> >> --=20 >> 1.9.1 >> >=20