[PATCH] x86/emul: Correctly annotate all push/pop %sreg instructions

[PATCH] x86/emul: Correctly annotate all push/pop %sreg instructions

[Andrew Cooper]

c/s 373923ed9c2 "x86emul: fix pushing of selector registers" redirected
all push %sreg instructions into the general push path.  However, this
ends up hitting the assertion at the head of the push path.

Annotate All push and pop %sreg instructions as Mov, indicating that
they do not read the destination operand.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/x86_emulate/x86_emulate.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 38147c5..a621751 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -54,19 +54,19 @@ static const opcode_desc_t opcode_table[256] = {
     /* 0x00 - 0x07 */
     ByteOp|DstMem|SrcReg|ModRM, DstMem|SrcReg|ModRM,
     ByteOp|DstReg|SrcMem|ModRM, DstReg|SrcMem|ModRM,
-    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps, ImplicitOps,
+    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps|Mov, ImplicitOps|Mov,
     /* 0x08 - 0x0F */
     ByteOp|DstMem|SrcReg|ModRM, DstMem|SrcReg|ModRM,
     ByteOp|DstReg|SrcMem|ModRM, DstReg|SrcMem|ModRM,
-    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps, 0,
+    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps|Mov, 0,
     /* 0x10 - 0x17 */
     ByteOp|DstMem|SrcReg|ModRM, DstMem|SrcReg|ModRM,
     ByteOp|DstReg|SrcMem|ModRM, DstReg|SrcMem|ModRM,
-    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps, ImplicitOps,
+    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps|Mov, ImplicitOps|Mov,
     /* 0x18 - 0x1F */
     ByteOp|DstMem|SrcReg|ModRM, DstMem|SrcReg|ModRM,
     ByteOp|DstReg|SrcMem|ModRM, DstReg|SrcMem|ModRM,
-    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps, ImplicitOps,
+    ByteOp|DstEax|SrcImm, DstEax|SrcImm, ImplicitOps|Mov, ImplicitOps|Mov,
     /* 0x20 - 0x27 */
     ByteOp|DstMem|SrcReg|ModRM, DstMem|SrcReg|ModRM,
     ByteOp|DstReg|SrcMem|ModRM, DstReg|SrcMem|ModRM,
@@ -245,10 +245,10 @@ static const opcode_desc_t twobyte_table[256] = {
     ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov,
     ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov,
     /* 0xA0 - 0xA7 */
-    ImplicitOps, ImplicitOps, ImplicitOps, DstBitBase|SrcReg|ModRM,
+    ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps, DstBitBase|SrcReg|ModRM,
     DstMem|SrcImmByte|ModRM, DstMem|SrcReg|ModRM, ModRM, ModRM,
     /* 0xA8 - 0xAF */
-    ImplicitOps, ImplicitOps, ImplicitOps, DstBitBase|SrcReg|ModRM,
+    ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps, DstBitBase|SrcReg|ModRM,
     DstMem|SrcImmByte|ModRM, DstMem|SrcReg|ModRM,
     ImplicitOps|ModRM, DstReg|SrcMem|ModRM,
     /* 0xB0 - 0xB7 */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/emul: Correctly annotate all push/pop %sreg instructions

[Jan Beulich]

>>> On 19.10.16 at 18:30, <andrew.cooper3@citrix.com> wrote:
> c/s 373923ed9c2 "x86emul: fix pushing of selector registers" redirected
> all push %sreg instructions into the general push path.  However, this
> ends up hitting the assertion at the head of the push path.
> 
> Annotate All push and pop %sreg instructions as Mov, indicating that
> they do not read the destination operand.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

I wonder how this passed both my and osstest's testing...

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/emul: Correctly annotate all push/pop %sreg instructions

[Wei Liu]

On Mon, Oct 24, 2016 at 03:23:01AM -0600, Jan Beulich wrote:
> >>> On 19.10.16 at 18:30, <andrew.cooper3@citrix.com> wrote:
> > c/s 373923ed9c2 "x86emul: fix pushing of selector registers" redirected
> > all push %sreg instructions into the general push path.  However, this
> > ends up hitting the assertion at the head of the push path.
> > 
> > Annotate All push and pop %sreg instructions as Mov, indicating that
> > they do not read the destination operand.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> I wonder how this passed both my and osstest's testing...
> 

OSSTest Cambridge instance (with rather old hardware) caught this IIRC.

I've queued this up for committing since this is needed for 4.8.

Wei.

> Jan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/emul: Correctly annotate all push/pop %sreg instructions

[Andrew Cooper]

On 24/10/16 10:54, Wei Liu wrote:
> On Mon, Oct 24, 2016 at 03:23:01AM -0600, Jan Beulich wrote:
>>>>> On 19.10.16 at 18:30, <andrew.cooper3@citrix.com> wrote:
>>> c/s 373923ed9c2 "x86emul: fix pushing of selector registers" redirected
>>> all push %sreg instructions into the general push path.  However, this
>>> ends up hitting the assertion at the head of the push path.
>>>
>>> Annotate All push and pop %sreg instructions as Mov, indicating that
>>> they do not read the destination operand.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> I wonder how this passed both my and osstest's testing...
>>
> OSSTest Cambridge instance (with rather old hardware) caught this IIRC.

It only showed up in the VGABIOS running on Gen-1 VT-x without
unrestricted_guest.  Under those circumstances, we emulate far more.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel