[PATCH] target/i386: Fix decoding of certain BMI instructions

David Greenaway via posted 1 patch 4 years, 10 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210114063958.1508050-1-dgreenaway@google.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Richard Henderson <richard.henderson@linaro.org>
target/i386/tcg/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] target/i386: Fix decoding of certain BMI instructions
Posted by David Greenaway via 4 years, 10 months ago
This patch fixes a translation bug for a subset of x86 BMI instructions
such as the following:

   c4 e2 f9 f7 c0                shlxq   %rax, %rax, %rax

Currently, these incorrectly generate an undefined instruction exception
when SSE is disabled via CR4, while instructions like "shrxq" work fine.

The problem appears to be related to BMI instructions encoded using VEX
and with a mandatory prefix of "0x66" (data). Instructions with this
data prefix (such as shlxq) are currently rejected. Instructions with
other mandatory prefixes (such as shrxq) translate as expected.

This patch removes the incorrect check in "gen_sse" that causes the
exception to be generated. For the non-BMI cases, the check is
redundant: prefixes are already checked at line 3696.

Buglink: https://bugs.launchpad.net/qemu/+bug/1748296

Signed-off-by: David Greenaway <dgreenaway@google.com>
---

I'm a relative novice in both the QEMU source code and the x86
instruction set, so please do review carefully, and don't be afraid to
let me know if I am completely going down the wrong path.

This bug was originally reported at
https://bugs.launchpad.net/qemu/+bug/1748296 a few years ago. The
reproducer linked from the bug still fails on QEMU head, but is fixed by
this patch.

 target/i386/tcg/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 6a4c31f933..af1faf9342 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -3075,7 +3075,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
     }
     if (is_xmm
         && !(s->flags & HF_OSFXSR_MASK)
-        && ((b != 0x38 && b != 0x3a) || (s->prefix & PREFIX_DATA))) {
+        && (b != 0x38 && b != 0x3a)) {
         goto unknown_op;
     }
     if (b == 0x0e) {
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH] target/i386: Fix decoding of certain BMI instructions
Posted by David Greenaway 4 years, 9 months ago
On 14 Jan 2021, David Greenaway <dgreenaway@google.com> wrote:
> This patch fixes a translation bug for a subset of x86 BMI instructions
> such as the following: [...]

Gentle ping.

The patch is up at:

https://patchwork.kernel.org/project/qemu-devel/patch/20210114063958.1508050-1-dgreenaway@google.com/

if that helps.

Cheers,
David

Re: [PATCH] target/i386: Fix decoding of certain BMI instructions
Posted by Paolo Bonzini 4 years, 9 months ago
On 27/01/21 23:54, David Greenaway wrote:
> On 14 Jan 2021, David Greenaway <dgreenaway@google.com> wrote:
>> This patch fixes a translation bug for a subset of x86 BMI instructions
>> such as the following: [...]
> 
> Gentle ping.
> 
> The patch is up at:
> 
> https://patchwork.kernel.org/project/qemu-devel/patch/20210114063958.1508050-1-dgreenaway@google.com/
> 
> if that helps.
> 
> Cheers,
> David
> 

Hi, I didn't have time to write a test yet (for tests/tcg).  I'll take a 
look now.

Paolo