From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60610) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eP8VO-0005BO-Ff for qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:59:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eP8VL-00024l-AV for qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:59:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46568) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eP8VL-00023X-2Y for qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:59:27 -0500 References: <1513163959-17545-1-git-send-email-peter.maydell@linaro.org> From: Paolo Bonzini Message-ID: Date: Wed, 13 Dec 2017 15:59:22 +0100 MIME-Version: 1.0 In-Reply-To: <1513163959-17545-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target/i386: Fix handling of VEX prefixes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: patches@linaro.org, Alexandro Sanchez Bach , Richard Henderson , Eduardo Habkost On 13/12/2017 12:19, Peter Maydell wrote: > In commit e3af7c788b73a6495eb9d94992ef11f6ad6f3c56 we > replaced direct calls to to cpu_ld*_code() with calls > to the x86_ld*_code() wrappers which incorporate an > advance of s->pc. Unfortunately we didn't notice that > in one place the old code was deliberately not incrementing > s->pc: > > @@ -4501,7 +4528,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) > static const int pp_prefix[4] = { > 0, PREFIX_DATA, PREFIX_REPZ, PREFIX_REPNZ > }; > - int vex3, vex2 = cpu_ldub_code(env, s->pc); > + int vex3, vex2 = x86_ldub_code(env, s); > > if (!CODE64(s) && (vex2 & 0xc0) != 0xc0) { > /* 4.1.4.6: In 32-bit mode, bits [7:6] must be 11b, > > This meant we were mishandling this set of instructions. > Remove the manual advance of s->pc for the "is VEX" case > (which is now done by x86_ldub_code()) and instead rewind > PC in the case where we decide that this isn't really VEX. > > Signed-off-by: Peter Maydell > Cc: qemu-stable@nongnu.org > Reported-by: Alexandro Sanchez Bach > --- > I checked the rest of the changes in e3af7c788b73a and > I don't think we made this mistake anywhere else. > --- > target/i386/translate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/translate.c b/target/i386/translate.c > index 088a9d9..ed5b69d 100644 > --- a/target/i386/translate.c > +++ b/target/i386/translate.c > @@ -4547,9 +4547,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) > if (!CODE64(s) && (vex2 & 0xc0) != 0xc0) { > /* 4.1.4.6: In 32-bit mode, bits [7:6] must be 11b, > otherwise the instruction is LES or LDS. */ > + s->pc--; /* rewind the advance_pc() x86_ldub_code() did */ > break; > } > - s->pc++; > > /* 4.1.1-4.1.3: No preceding lock, 66, f2, f3, or rex prefixes. */ > if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ > Queued for 2.12, thanks. Paolo