* [PATCH] x86emul: fix {,i}mul and {,i}div
@ 2016-09-29 13:08 Jan Beulich
2016-09-29 18:12 ` Andrew Cooper
2016-09-29 18:21 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2016-09-29 13:08 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 4323 bytes --]
Commit a3db233ede ("x86emul: use DstEax also for {,I}{MUL,DIV}") went
a little too far: DstEax and SrcEax weren't really meant to be used
together with ModRM - they assume modrm_reg remains zero by the time
the destination / source register pointer gets calculated. Don't fully
undo that commit though, but instead just correct the register pointer,
and don't use dst.val as input for mul and imul (div and idiv did avoid
that already).
Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3845,18 +3845,19 @@ x86_emulate(
emulate_1op("neg", dst, _regs.eflags);
break;
case 4: /* mul */
+ dst.reg = (unsigned long *)&_regs.eax;
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
switch ( dst.bytes )
{
case 1:
- dst.val = (uint8_t)dst.val;
+ dst.val = (uint8_t)_regs.eax;
dst.val *= src.val;
if ( (uint8_t)dst.val != (uint16_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
dst.bytes = 2;
break;
case 2:
- dst.val = (uint16_t)dst.val;
+ dst.val = (uint16_t)_regs.eax;
dst.val *= src.val;
if ( (uint16_t)dst.val != (uint32_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
@@ -3864,7 +3865,7 @@ x86_emulate(
break;
#ifdef __x86_64__
case 4:
- dst.val = (uint32_t)dst.val;
+ dst.val = _regs._eax;
dst.val *= src.val;
if ( (uint32_t)dst.val != dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
@@ -3873,7 +3874,7 @@ x86_emulate(
#endif
default:
u[0] = src.val;
- u[1] = dst.val;
+ u[1] = _regs.eax;
if ( mul_dbl(u) )
_regs.eflags |= EFLG_OF|EFLG_CF;
_regs.edx = u[1];
@@ -3882,12 +3883,13 @@ x86_emulate(
}
break;
case 5: /* imul */
+ dst.reg = (unsigned long *)&_regs.eax;
imul:
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
switch ( dst.bytes )
{
case 1:
- dst.val = (int8_t)src.val * (int8_t)dst.val;
+ dst.val = (int8_t)src.val * (int8_t)_regs.eax;
if ( (int8_t)dst.val != (int16_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
ASSERT(b > 0x6b);
@@ -3895,7 +3897,7 @@ x86_emulate(
break;
case 2:
dst.val = ((uint32_t)(int16_t)src.val *
- (uint32_t)(int16_t)dst.val);
+ (uint32_t)(int16_t)_regs.eax);
if ( (int16_t)dst.val != (int32_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
if ( b > 0x6b )
@@ -3904,7 +3906,7 @@ x86_emulate(
#ifdef __x86_64__
case 4:
dst.val = ((uint64_t)(int32_t)src.val *
- (uint64_t)(int32_t)dst.val);
+ (uint64_t)(int32_t)_regs.eax);
if ( (int32_t)dst.val != dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
if ( b > 0x6b )
@@ -3913,7 +3915,7 @@ x86_emulate(
#endif
default:
u[0] = src.val;
- u[1] = dst.val;
+ u[1] = _regs.eax;
if ( imul_dbl(u) )
_regs.eflags |= EFLG_OF|EFLG_CF;
if ( b > 0x6b )
@@ -3923,6 +3925,7 @@ x86_emulate(
}
break;
case 6: /* div */
+ dst.reg = (unsigned long *)&_regs.eax;
switch ( src.bytes )
{
case 1:
@@ -3968,6 +3971,7 @@ x86_emulate(
}
break;
case 7: /* idiv */
+ dst.reg = (unsigned long *)&_regs.eax;
switch ( src.bytes )
{
case 1:
[-- Attachment #2: x86emul-fix-mul-div.patch --]
[-- Type: text/plain, Size: 4355 bytes --]
x86emul: fix {,i}mul and {,i}div
Commit a3db233ede ("x86emul: use DstEax also for {,I}{MUL,DIV}") went
a little too far: DstEax and SrcEax weren't really meant to be used
together with ModRM - they assume modrm_reg remains zero by the time
the destination / source register pointer gets calculated. Don't fully
undo that commit though, but instead just correct the register pointer,
and don't use dst.val as input for mul and imul (div and idiv did avoid
that already).
Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3845,18 +3845,19 @@ x86_emulate(
emulate_1op("neg", dst, _regs.eflags);
break;
case 4: /* mul */
+ dst.reg = (unsigned long *)&_regs.eax;
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
switch ( dst.bytes )
{
case 1:
- dst.val = (uint8_t)dst.val;
+ dst.val = (uint8_t)_regs.eax;
dst.val *= src.val;
if ( (uint8_t)dst.val != (uint16_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
dst.bytes = 2;
break;
case 2:
- dst.val = (uint16_t)dst.val;
+ dst.val = (uint16_t)_regs.eax;
dst.val *= src.val;
if ( (uint16_t)dst.val != (uint32_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
@@ -3864,7 +3865,7 @@ x86_emulate(
break;
#ifdef __x86_64__
case 4:
- dst.val = (uint32_t)dst.val;
+ dst.val = _regs._eax;
dst.val *= src.val;
if ( (uint32_t)dst.val != dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
@@ -3873,7 +3874,7 @@ x86_emulate(
#endif
default:
u[0] = src.val;
- u[1] = dst.val;
+ u[1] = _regs.eax;
if ( mul_dbl(u) )
_regs.eflags |= EFLG_OF|EFLG_CF;
_regs.edx = u[1];
@@ -3882,12 +3883,13 @@ x86_emulate(
}
break;
case 5: /* imul */
+ dst.reg = (unsigned long *)&_regs.eax;
imul:
_regs.eflags &= ~(EFLG_OF|EFLG_CF);
switch ( dst.bytes )
{
case 1:
- dst.val = (int8_t)src.val * (int8_t)dst.val;
+ dst.val = (int8_t)src.val * (int8_t)_regs.eax;
if ( (int8_t)dst.val != (int16_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
ASSERT(b > 0x6b);
@@ -3895,7 +3897,7 @@ x86_emulate(
break;
case 2:
dst.val = ((uint32_t)(int16_t)src.val *
- (uint32_t)(int16_t)dst.val);
+ (uint32_t)(int16_t)_regs.eax);
if ( (int16_t)dst.val != (int32_t)dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
if ( b > 0x6b )
@@ -3904,7 +3906,7 @@ x86_emulate(
#ifdef __x86_64__
case 4:
dst.val = ((uint64_t)(int32_t)src.val *
- (uint64_t)(int32_t)dst.val);
+ (uint64_t)(int32_t)_regs.eax);
if ( (int32_t)dst.val != dst.val )
_regs.eflags |= EFLG_OF|EFLG_CF;
if ( b > 0x6b )
@@ -3913,7 +3915,7 @@ x86_emulate(
#endif
default:
u[0] = src.val;
- u[1] = dst.val;
+ u[1] = _regs.eax;
if ( imul_dbl(u) )
_regs.eflags |= EFLG_OF|EFLG_CF;
if ( b > 0x6b )
@@ -3923,6 +3925,7 @@ x86_emulate(
}
break;
case 6: /* div */
+ dst.reg = (unsigned long *)&_regs.eax;
switch ( src.bytes )
{
case 1:
@@ -3968,6 +3971,7 @@ x86_emulate(
}
break;
case 7: /* idiv */
+ dst.reg = (unsigned long *)&_regs.eax;
switch ( src.bytes )
{
case 1:
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86emul: fix {,i}mul and {,i}div
2016-09-29 13:08 [PATCH] x86emul: fix {,i}mul and {,i}div Jan Beulich
@ 2016-09-29 18:12 ` Andrew Cooper
2016-09-29 18:21 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2016-09-29 18:12 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 29/09/16 14:08, Jan Beulich wrote:
> Commit a3db233ede ("x86emul: use DstEax also for {,I}{MUL,DIV}") went
> a little too far: DstEax and SrcEax weren't really meant to be used
> together with ModRM - they assume modrm_reg remains zero by the time
> the destination / source register pointer gets calculated. Don't fully
> undo that commit though, but instead just correct the register pointer,
> and don't use dst.val as input for mul and imul (div and idiv did avoid
> that already).
>
> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86emul: fix {,i}mul and {,i}div
2016-09-29 13:08 [PATCH] x86emul: fix {,i}mul and {,i}div Jan Beulich
2016-09-29 18:12 ` Andrew Cooper
@ 2016-09-29 18:21 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 3+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-29 18:21 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Andrew Cooper
On Thu, Sep 29, 2016 at 07:08:10AM -0600, Jan Beulich wrote:
> Commit a3db233ede ("x86emul: use DstEax also for {,I}{MUL,DIV}") went
> a little too far: DstEax and SrcEax weren't really meant to be used
> together with ModRM - they assume modrm_reg remains zero by the time
> the destination / source register pointer gets calculated. Don't fully
> undo that commit though, but instead just correct the register pointer,
> and don't use dst.val as input for mul and imul (div and idiv did avoid
> that already).
>
> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
and
Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-09-29 18:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-29 13:08 [PATCH] x86emul: fix {,i}mul and {,i}div Jan Beulich
2016-09-29 18:12 ` Andrew Cooper
2016-09-29 18:21 ` Konrad Rzeszutek Wilk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).