[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target/i386: Fix handling of VEX prefixes
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] target/i386: Fix handling of VEX prefixes |
Date: |
Wed, 13 Dec 2017 15:59:22 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 |
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 <address@hidden>
> Cc: address@hidden
> Reported-by: Alexandro Sanchez Bach <address@hidden>
> ---
> 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