Patchwork bug in popcnt emulation with some register operand(s)?

login
register
mail settings
Submitter malc
Date Oct. 8, 2012, 7:52 a.m.
Message ID <alpine.LNX.2.00.1210081152040.2993@linmac>
Download mbox | patch
Permalink /patch/189928/
State New
Headers show

Comments

malc - Oct. 8, 2012, 7:52 a.m.
On Mon, 8 Oct 2012, Andriy Gapon wrote:

> 
> I am running Qemu (plain, no kvm, etc) on an AMD 10h machine that
> provides popcnt instruction.  Qemu advertises availability of pocnt
> to a guest as well.  What I see in the guest that popcnt
> 0x20(%r12),%r8 instruction actually placed its result into %rax.
> With %rdi and %rax operands the instruction worked fine though.
> 
> 

Does following work?
Andriy Gapon - Oct. 8, 2012, 9:02 a.m.
on 08/10/2012 10:52 malc said the following:
> On Mon, 8 Oct 2012, Andriy Gapon wrote:
> 
>>
>> I am running Qemu (plain, no kvm, etc) on an AMD 10h machine that
>> provides popcnt instruction.  Qemu advertises availability of pocnt
>> to a guest as well.  What I see in the guest that popcnt
>> 0x20(%r12),%r8 instruction actually placed its result into %rax.
>> With %rdi and %rax operands the instruction worked fine though.
>>
>>
> 
> Does following work?

It does!  Thank you very much.

> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index e896abf..c36cc3e 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -7818,7 +7818,7 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
>              goto illegal_op;
>  
>          modrm = cpu_ldub_code(cpu_single_env, s->pc++);
> -        reg = ((modrm >> 3) & 7);
> +        reg = ((modrm >> 3) & 7) | rex_r;
>  
>          if (s->prefix & PREFIX_DATA)
>              ot = OT_WORD;
>
Andriy Gapon - Oct. 10, 2012, 9:03 p.m.
on 08/10/2012 12:02 Andriy Gapon said the following:
> on 08/10/2012 10:52 malc said the following:
>> On Mon, 8 Oct 2012, Andriy Gapon wrote:
>>
>>>
>>> I am running Qemu (plain, no kvm, etc) on an AMD 10h machine that
>>> provides popcnt instruction.  Qemu advertises availability of pocnt
>>> to a guest as well.  What I see in the guest that popcnt
>>> 0x20(%r12),%r8 instruction actually placed its result into %rax.
>>> With %rdi and %rax operands the instruction worked fine though.
>>>
>>>
>>
>> Does following work?
> 
> It does!  Thank you very much.

Do you plan to commit this fix?
Is there anything that I should do to make that happen (sooner)?


>> diff --git a/target-i386/translate.c b/target-i386/translate.c
>> index e896abf..c36cc3e 100644
>> --- a/target-i386/translate.c
>> +++ b/target-i386/translate.c
>> @@ -7818,7 +7818,7 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
>>              goto illegal_op;
>>  
>>          modrm = cpu_ldub_code(cpu_single_env, s->pc++);
>> -        reg = ((modrm >> 3) & 7);
>> +        reg = ((modrm >> 3) & 7) | rex_r;
>>  
>>          if (s->prefix & PREFIX_DATA)
>>              ot = OT_WORD;
>>
> 
>
malc - Oct. 10, 2012, 9:09 p.m.
On Thu, 11 Oct 2012, Andriy Gapon wrote:

> on 08/10/2012 12:02 Andriy Gapon said the following:
> > on 08/10/2012 10:52 malc said the following:
> >> On Mon, 8 Oct 2012, Andriy Gapon wrote:
> >>
> >>>
> >>> I am running Qemu (plain, no kvm, etc) on an AMD 10h machine that
> >>> provides popcnt instruction.  Qemu advertises availability of pocnt
> >>> to a guest as well.  What I see in the guest that popcnt
> >>> 0x20(%r12),%r8 instruction actually placed its result into %rax.
> >>> With %rdi and %rax operands the instruction worked fine though.
> >>>
> >>>
> >>
> >> Does following work?
> > 
> > It does!  Thank you very much.
> 
> Do you plan to commit this fix?
> Is there anything that I should do to make that happen (sooner)?
> 

Submit a patch with a well thought out comment and it'll be commited.

[..snip..]
Andriy Gapon - Oct. 14, 2012, 10:25 a.m.
on 11/10/2012 00:09 malc said the following:
> On Thu, 11 Oct 2012, Andriy Gapon wrote:
> 
>> on 08/10/2012 12:02 Andriy Gapon said the following:
>>> on 08/10/2012 10:52 malc said the following:
>>>> On Mon, 8 Oct 2012, Andriy Gapon wrote:
>>>>
>>>>>
>>>>> I am running Qemu (plain, no kvm, etc) on an AMD 10h machine that
>>>>> provides popcnt instruction.  Qemu advertises availability of pocnt
>>>>> to a guest as well.  What I see in the guest that popcnt
>>>>> 0x20(%r12),%r8 instruction actually placed its result into %rax.
>>>>> With %rdi and %rax operands the instruction worked fine though.
>>>>>
>>>>>
>>>>
>>>> Does following work?
>>>
>>> It does!  Thank you very much.
>>
>> Do you plan to commit this fix?
>> Is there anything that I should do to make that happen (sooner)?
>>
> 
> Submit a patch with a well thought out comment and it'll be commited.
> 
> [..snip..]
> 

Hmm... Since you are the author of the patch, wouldn't be more appropriate for
you to submit it?  Besides, I can only mostly repeat the bug report as I do not
quite understand the code and can not properly describe what the patch does.

Sorry for not taking up the work, but I am really just a qemu user.
malc - Oct. 14, 2012, 10:56 a.m.
On Sun, 14 Oct 2012, Andriy Gapon wrote:

[..snip..]

>
> Hmm... Since you are the author of the patch, wouldn't be more 
> appropriate for you to submit it?  Besides, I can only mostly repeat the 
> bug report as I do not quite understand the code and can not properly 
> describe what the patch does.

Sigh, okay, commited.

> 
> Sorry for not taking up the work, but I am really just a qemu user.
> 
>
Andriy Gapon - Oct. 14, 2012, 11:01 a.m.
on 14/10/2012 13:56 malc said the following:
> On Sun, 14 Oct 2012, Andriy Gapon wrote:
> 
> [..snip..]
> 
>>
>> Hmm... Since you are the author of the patch, wouldn't be more 
>> appropriate for you to submit it?  Besides, I can only mostly repeat the 
>> bug report as I do not quite understand the code and can not properly 
>> describe what the patch does.
> 
> Sigh, okay, commited.

Thank you again!

>>
>> Sorry for not taking up the work, but I am really just a qemu user.
>>
>>
>

Patch

diff --git a/target-i386/translate.c b/target-i386/translate.c
index e896abf..c36cc3e 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7818,7 +7818,7 @@  static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
             goto illegal_op;
 
         modrm = cpu_ldub_code(cpu_single_env, s->pc++);
-        reg = ((modrm >> 3) & 7);
+        reg = ((modrm >> 3) & 7) | rex_r;
 
         if (s->prefix & PREFIX_DATA)
             ot = OT_WORD;