* [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions
@ 2010-08-08 2:27 Mohammed Gamal
2010-08-08 2:27 ` [PATCH 2/2] x86 emulator: Fix emulate_grp3 return values Mohammed Gamal
2010-08-08 11:34 ` [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions Avi Kivity
0 siblings, 2 replies; 6+ messages in thread
From: Mohammed Gamal @ 2010-08-08 2:27 UTC (permalink / raw)
To: avi; +Cc: mtosatti, kvm, Mohammed Gamal
This adds unary mul, imul, div, and idiv instructions (group 3 r/m 4-7).
Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>
---
arch/x86/kvm/emulate.c | 41 ++++++++++++++++++++++++++++++++++++++++-
1 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 7d78832..6d1ec53 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -315,6 +315,31 @@ struct group_dual {
} \
} while (0)
+#define __emulate_1op_src(_op, _src, _ax, _dx, _eflags, _suffix) \
+ do { \
+ unsigned long _tmp; \
+ \
+ __asm__ __volatile__ ( \
+ _PRE_EFLAGS("0", "4", "1") \
+ _op _suffix " %5; " \
+ _POST_EFLAGS("0", "4", "1") \
+ : "=m" (_eflags), "=&r" (_tmp), \
+ "=a" (_ax), "=d" (_dx) \
+ : "i" (EFLAGS_MASK), "m" ((_src).val), \
+ "a" (_ax), "d" (_dx)); \
+ } while (0)
+
+/* instruction has only one source operand, destination is implicit (e.g. mul, div, imul, idiv) */
+#define emulate_1op_src(_op, _src, _ax, _dx, _eflags) \
+ do { \
+ switch((_src).bytes) { \
+ case 1: __emulate_1op_src(_op, _src, _ax, _dx, _eflags, "b"); break; \
+ case 2: __emulate_1op_src(_op, _src, _ax, _dx, _eflags, "w"); break; \
+ case 4: __emulate_1op_src(_op, _src, _ax, _dx, _eflags, "l"); break; \
+ case 8: ON64(__emulate_1op_src(_op, _src, _dx, _ax, _eflags, "q")); break; \
+ } \
+ } while (0)
+
/* Fetch next part of the instruction being emulated. */
#define insn_fetch(_type, _size, _eip) \
({ unsigned long _x; \
@@ -1354,6 +1379,8 @@ static inline int emulate_grp3(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
{
struct decode_cache *c = &ctxt->decode;
+ unsigned long *eax = &c->regs[VCPU_REGS_RAX];
+ unsigned long *edx = &c->regs[VCPU_REGS_RDX];
switch (c->modrm_reg) {
case 0 ... 1: /* test */
@@ -1365,6 +1392,18 @@ static inline int emulate_grp3(struct x86_emulate_ctxt *ctxt,
case 3: /* neg */
emulate_1op("neg", c->dst, ctxt->eflags);
break;
+ case 4: /* mul */
+ emulate_1op_src("mul", c->src, *eax, *edx, ctxt->eflags);
+ break;
+ case 5: /* imul */
+ emulate_1op_src("imul", c->src, *eax, *edx, ctxt->eflags);
+ break;
+ case 6: /* div */
+ emulate_1op_src("div", c->src, *eax, *edx, ctxt->eflags);
+ break;
+ case 7: /* idiv */
+ emulate_1op_src("idiv", c->src, *eax, *edx, ctxt->eflags);
+ break;
default:
return 0;
}
@@ -2120,7 +2159,7 @@ static struct opcode group1A[] = {
static struct opcode group3[] = {
D(DstMem | SrcImm | ModRM), D(DstMem | SrcImm | ModRM),
D(DstMem | SrcNone | ModRM | Lock), D(DstMem | SrcNone | ModRM | Lock),
- X4(D(Undefined)),
+ X4(D(SrcMem | ModRM)),
};
static struct opcode group4[] = {
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] x86 emulator: Fix emulate_grp3 return values
2010-08-08 2:27 [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions Mohammed Gamal
@ 2010-08-08 2:27 ` Mohammed Gamal
2010-08-08 11:34 ` [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions Avi Kivity
1 sibling, 0 replies; 6+ messages in thread
From: Mohammed Gamal @ 2010-08-08 2:27 UTC (permalink / raw)
To: avi; +Cc: mtosatti, kvm, Mohammed Gamal
This patch let's emulate_grp3() return X86EMUL_* return codes instead of hardcoded ones
Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>
---
arch/x86/kvm/emulate.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 6d1ec53..11489a4 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1405,9 +1405,9 @@ static inline int emulate_grp3(struct x86_emulate_ctxt *ctxt,
emulate_1op_src("idiv", c->src, *eax, *edx, ctxt->eflags);
break;
default:
- return 0;
+ return X86EMUL_UNHANDLEABLE;
}
- return 1;
+ return X86EMUL_CONTINUE;
}
static inline int emulate_grp45(struct x86_emulate_ctxt *ctxt,
@@ -3158,7 +3158,7 @@ special_insn:
ctxt->eflags ^= EFLG_CF;
break;
case 0xf6 ... 0xf7: /* Grp3 */
- if (!emulate_grp3(ctxt, ops))
+ if (emulate_grp3(ctxt, ops) != X86EMUL_CONTINUE)
goto cannot_emulate;
break;
case 0xf8: /* clc */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions
2010-08-08 2:27 [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions Mohammed Gamal
2010-08-08 2:27 ` [PATCH 2/2] x86 emulator: Fix emulate_grp3 return values Mohammed Gamal
@ 2010-08-08 11:34 ` Avi Kivity
2010-08-08 11:53 ` Mohammed Gamal
1 sibling, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-08-08 11:34 UTC (permalink / raw)
To: Mohammed Gamal; +Cc: mtosatti, kvm
On 08/08/2010 05:27 AM, Mohammed Gamal wrote:
> This adds unary mul, imul, div, and idiv instructions (group 3 r/m 4-7).
>
> Signed-off-by: Mohammed Gamal<m.gamal005@gmail.com>
> ---
> arch/x86/kvm/emulate.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 7d78832..6d1ec53 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -315,6 +315,31 @@ struct group_dual {
> } \
> } while (0)
>
> +#define __emulate_1op_src(_op, _src, _ax, _dx, _eflags, _suffix) \
Not just 1op - add rax_rdx to the name to indicate these are implicit
operands.
> + do { \
> + unsigned long _tmp; \
> + \
> + __asm__ __volatile__ ( \
> + _PRE_EFLAGS("0", "4", "1") \
> + _op _suffix " %5; " \
> + _POST_EFLAGS("0", "4", "1") \
> + : "=m" (_eflags), "=&r" (_tmp), \
> + "=a" (_ax), "=d" (_dx) \
> + : "i" (EFLAGS_MASK), "m" ((_src).val), \
> + "a" (_ax), "d" (_dx)); \
> + } while (0)
The byte form of the instruction doesn't update dx, and the word form
doesn't update edx[16:31]. So the "=a" and "=d" operands need to be
"+a" and "+d" so the compiler loads them before the operation is started.
Please add a test to the effect, for example start with eax=0x12345678,
and multiply (byte size) 0x80 by 0x40, and observe that the upper 16
bits of eax are preserved (and that rdx is not modified).
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions
2010-08-08 11:34 ` [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions Avi Kivity
@ 2010-08-08 11:53 ` Mohammed Gamal
2010-08-08 20:27 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Mohammed Gamal @ 2010-08-08 11:53 UTC (permalink / raw)
To: Avi Kivity; +Cc: mtosatti, kvm
On Sun, Aug 8, 2010 at 2:34 PM, Avi Kivity <avi@redhat.com> wrote:
> On 08/08/2010 05:27 AM, Mohammed Gamal wrote:
>>
>> This adds unary mul, imul, div, and idiv instructions (group 3 r/m 4-7).
>>
>> Signed-off-by: Mohammed Gamal<m.gamal005@gmail.com>
>> ---
>> arch/x86/kvm/emulate.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 40 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index 7d78832..6d1ec53 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -315,6 +315,31 @@ struct group_dual {
>> } \
>> } while (0)
>>
>> +#define __emulate_1op_src(_op, _src, _ax, _dx, _eflags, _suffix)
>> \
>
> Not just 1op - add rax_rdx to the name to indicate these are implicit
> operands.
>
>> + do { \
>> + unsigned long _tmp; \
>> + \
>> + __asm__ __volatile__ ( \
>> + _PRE_EFLAGS("0", "4", "1") \
>> + _op _suffix " %5; " \
>> + _POST_EFLAGS("0", "4", "1") \
>> + : "=m" (_eflags), "=&r" (_tmp), \
>> + "=a" (_ax), "=d" (_dx) \
>> + : "i" (EFLAGS_MASK), "m" ((_src).val), \
>> + "a" (_ax), "d" (_dx)); \
>> + } while (0)
>
> The byte form of the instruction doesn't update dx, and the word form
> doesn't update edx[16:31]. So the "=a" and "=d" operands need to be "+a"
> and "+d" so the compiler loads them before the operation is started.
>
> Please add a test to the effect, for example start with eax=0x12345678, and
> multiply (byte size) 0x80 by 0x40,
We already have a value store in eax, so I can only either multiply it
by 0x80 or 0x40. Or do you mean we should have eax=0x12345680 and then
perhaps multply by 0x40 and make sure the upper 16 bits are preserved?
> and observe that the upper 16 bits of eax
> are preserved (and that rdx is not modified).
>
> --
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions
2010-08-08 11:53 ` Mohammed Gamal
@ 2010-08-08 20:27 ` Avi Kivity
2010-08-08 21:19 ` Mohammed Gamal
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-08-08 20:27 UTC (permalink / raw)
To: Mohammed Gamal; +Cc: mtosatti, kvm
On 08/08/2010 07:53 AM, Mohammed Gamal wrote:
> On Sun, Aug 8, 2010 at 2:34 PM, Avi Kivity<avi@redhat.com> wrote:
>> On 08/08/2010 05:27 AM, Mohammed Gamal wrote:
>>> This adds unary mul, imul, div, and idiv instructions (group 3 r/m 4-7).
>>>
>>> Signed-off-by: Mohammed Gamal<m.gamal005@gmail.com>
>>> ---
>>> arch/x86/kvm/emulate.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>>> 1 files changed, 40 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>> index 7d78832..6d1ec53 100644
>>> --- a/arch/x86/kvm/emulate.c
>>> +++ b/arch/x86/kvm/emulate.c
>>> @@ -315,6 +315,31 @@ struct group_dual {
>>> } \
>>> } while (0)
>>>
>>> +#define __emulate_1op_src(_op, _src, _ax, _dx, _eflags, _suffix)
>>> \
>> Not just 1op - add rax_rdx to the name to indicate these are implicit
>> operands.
>>
>>> + do { \
>>> + unsigned long _tmp; \
>>> + \
>>> + __asm__ __volatile__ ( \
>>> + _PRE_EFLAGS("0", "4", "1") \
>>> + _op _suffix " %5; " \
>>> + _POST_EFLAGS("0", "4", "1") \
>>> + : "=m" (_eflags), "=&r" (_tmp), \
>>> + "=a" (_ax), "=d" (_dx) \
>>> + : "i" (EFLAGS_MASK), "m" ((_src).val), \
>>> + "a" (_ax), "d" (_dx)); \
>>> + } while (0)
>> The byte form of the instruction doesn't update dx, and the word form
>> doesn't update edx[16:31]. So the "=a" and "=d" operands need to be "+a"
>> and "+d" so the compiler loads them before the operation is started.
>>
I see you have "a" and "d" also in the inputs section. So the code is
correct.
>> Please add a test to the effect, for example start with eax=0x12345678, and
>> multiply (byte size) 0x80 by 0x40,
> We already have a value store in eax, so I can only either multiply it
> by 0x80 or 0x40. Or do you mean we should have eax=0x12345680 and then
> perhaps multply by 0x40 and make sure the upper 16 bits are preserved?
Yes.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions
2010-08-08 20:27 ` Avi Kivity
@ 2010-08-08 21:19 ` Mohammed Gamal
0 siblings, 0 replies; 6+ messages in thread
From: Mohammed Gamal @ 2010-08-08 21:19 UTC (permalink / raw)
To: Avi Kivity; +Cc: mtosatti, kvm
On 8/8/10, Avi Kivity <avi@redhat.com> wrote:
> On 08/08/2010 07:53 AM, Mohammed Gamal wrote:
>> On Sun, Aug 8, 2010 at 2:34 PM, Avi Kivity<avi@redhat.com> wrote:
>>> On 08/08/2010 05:27 AM, Mohammed Gamal wrote:
>>>> This adds unary mul, imul, div, and idiv instructions (group 3 r/m 4-7).
>>>>
>>>> Signed-off-by: Mohammed Gamal<m.gamal005@gmail.com>
>>>> ---
>>>> arch/x86/kvm/emulate.c | 41
>>>> ++++++++++++++++++++++++++++++++++++++++-
>>>> 1 files changed, 40 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>>> index 7d78832..6d1ec53 100644
>>>> --- a/arch/x86/kvm/emulate.c
>>>> +++ b/arch/x86/kvm/emulate.c
>>>> @@ -315,6 +315,31 @@ struct group_dual {
>>>> }
>>>> \
>>>> } while (0)
>>>>
>>>> +#define __emulate_1op_src(_op, _src, _ax, _dx, _eflags, _suffix)
>>>> \
>>> Not just 1op - add rax_rdx to the name to indicate these are implicit
>>> operands.
>>>
>>>> + do {
>>>> \
>>>> + unsigned long _tmp;
>>>> \
>>>> +
>>>> \
>>>> + __asm__ __volatile__ (
>>>> \
>>>> + _PRE_EFLAGS("0", "4", "1")
>>>> \
>>>> + _op _suffix " %5; "
>>>> \
>>>> + _POST_EFLAGS("0", "4", "1")
>>>> \
>>>> + : "=m" (_eflags), "=&r" (_tmp),
>>>> \
>>>> + "=a" (_ax), "=d" (_dx)
>>>> \
>>>> + : "i" (EFLAGS_MASK), "m" ((_src).val),
>>>> \
>>>> + "a" (_ax), "d" (_dx));
>>>> \
>>>> + } while (0)
>>> The byte form of the instruction doesn't update dx, and the word form
>>> doesn't update edx[16:31]. So the "=a" and "=d" operands need to be "+a"
>>> and "+d" so the compiler loads them before the operation is started.
>>>
>
> I see you have "a" and "d" also in the inputs section. So the code is
> correct.
>
>>> Please add a test to the effect, for example start with eax=0x12345678,
>>> and
>>> multiply (byte size) 0x80 by 0x40,
>> We already have a value store in eax, so I can only either multiply it
>> by 0x80 or 0x40. Or do you mean we should have eax=0x12345680 and then
>> perhaps multply by 0x40 and make sure the upper 16 bits are preserved?
>
> Yes.
>
Cool. I already sent out a new test that does this.
> --
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-08 21:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-08 2:27 [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions Mohammed Gamal
2010-08-08 2:27 ` [PATCH 2/2] x86 emulator: Fix emulate_grp3 return values Mohammed Gamal
2010-08-08 11:34 ` [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions Avi Kivity
2010-08-08 11:53 ` Mohammed Gamal
2010-08-08 20:27 ` Avi Kivity
2010-08-08 21:19 ` Mohammed Gamal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.