diff mbox

x86 emulator: Add 'push/pop sreg' instructions

Message ID 1250599685-2351-1-git-send-email-m.gamal005@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mohammed Gamal Aug. 18, 2009, 12:48 p.m. UTC
Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>
---
 arch/x86/kvm/emulate.c |   60 ++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 56 insertions(+), 4 deletions(-)

Comments

Avi Kivity Aug. 18, 2009, 2:39 p.m. UTC | #1
On 08/18/2009 03:48 PM, Mohammed Gamal wrote:
> +
> +static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
> +			     struct x86_emulate_ops *ops, int seg)
> +{
> +	struct kvm_segment segment;
> +	int rc;
> +
> +	kvm_x86_ops->get_segment(ctxt->vcpu,&segment, seg);
> +	rc = emulate_pop(ctxt, ops,&segment.selector, sizeof(uint16_t));
>    

'pop seg' is still subject to the operand size (I think).

> +	kvm_x86_ops->set_segment(ctxt->vcpu,&segment, seg);
>    

You need to call kvm_load_segment_descriptor() so that the segment cache 
is also loaded correctly.

Note some of these instructions are not encodable in long mode; need to 
check for that instead of emulating the wrong instruction.

> @@ -1707,18 +1732,45 @@ special_insn:
>   	      add:		/* add */
>   		emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
>   		break;
> +	case 0x06:		/* push es */
> +		emulate_push_sreg(ctxt, VCPU_SREG_ES);
> +		break;
> +	case 0x07:		/* pop es */
> +		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
> +		if (rc != 0)
> +			goto done;
> +		break;
>   	case 0x08 ... 0x0d:
>   	      or:		/* or */
>   		emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
>   		break;
> +	case 0x0e:		/* push cs */
> +		emulate_push_sreg(ctxt, VCPU_SREG_CS);
> +		break;
>   	case 0x10 ... 0x15:
>   	      adc:		/* adc */
>   		emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
>   		break;
> +	case 0x16:		/* push ss */
> +		emulate_push_sreg(ctxt, VCPU_SREG_SS);
> +		break;
> +	case 0x17:		/* pop ss */
> +		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS);
> +		if (rc != 0)
> +			goto done;
> +		break;
>   	case 0x18 ... 0x1d:
>   	      sbb:		/* sbb */
>   		emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
>   		break;
> +	case 0x1e:		/* push ds */
> +		emulate_push_sreg(ctxt, VCPU_SREG_DS);
> +		break;
> +	case 0x1f:		/* pop ds */
> +		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS);
> +		if (rc != 0)
> +			goto done;
> +		break;
>   	case 0x20 ... 0x25:
>   	      and:		/* and */
>   		emulate_2op_SrcV("and", c->src, c->dst, ctxt->eflags);
>
Mohammed Gamal Aug. 18, 2009, 7:40 p.m. UTC | #2
On Tue, Aug 18, 2009 at 5:39 PM, Avi Kivity<avi@redhat.com> wrote:
> On 08/18/2009 03:48 PM, Mohammed Gamal wrote:
>>
>> +
>> +static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
>> +                            struct x86_emulate_ops *ops, int seg)
>> +{
>> +       struct kvm_segment segment;
>> +       int rc;
>> +
>> +       kvm_x86_ops->get_segment(ctxt->vcpu,&segment, seg);
>> +       rc = emulate_pop(ctxt, ops,&segment.selector, sizeof(uint16_t));
>>
>
> 'pop seg' is still subject to the operand size (I think).
>
>> +       kvm_x86_ops->set_segment(ctxt->vcpu,&segment, seg);
>>
But we're popping the contents of the stack top to a segment register
which is going to be of 16-bits anyway, so we know the length before
hand, no?

>
> You need to call kvm_load_segment_descriptor() so that the segment cache is
> also loaded correctly.
>
> Note some of these instructions are not encodable in long mode; need to
> check for that instead of emulating the wrong instruction.
>
>> @@ -1707,18 +1732,45 @@ special_insn:
>>              add:              /* add */
>>                emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x06:              /* push es */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_ES);
>> +               break;
>> +       case 0x07:              /* pop es */
>> +               rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
>> +               if (rc != 0)
>> +                       goto done;
>> +               break;
>>        case 0x08 ... 0x0d:
>>              or:               /* or */
>>                emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x0e:              /* push cs */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_CS);
>> +               break;
>>        case 0x10 ... 0x15:
>>              adc:              /* adc */
>>                emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x16:              /* push ss */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_SS);
>> +               break;
>> +       case 0x17:              /* pop ss */
>> +               rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS);
>> +               if (rc != 0)
>> +                       goto done;
>> +               break;
>>        case 0x18 ... 0x1d:
>>              sbb:              /* sbb */
>>                emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
>>                break;
>> +       case 0x1e:              /* push ds */
>> +               emulate_push_sreg(ctxt, VCPU_SREG_DS);
>> +               break;
>> +       case 0x1f:              /* pop ds */
>> +               rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS);
>> +               if (rc != 0)
>> +                       goto done;
>> +               break;
>>        case 0x20 ... 0x25:
>>              and:              /* and */
>>                emulate_2op_SrcV("and", c->src, c->dst, ctxt->eflags);
>>
>
>
I was under the impression that the emulator doesn't support long mode
yet, is that still the case?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity Aug. 19, 2009, 8:23 a.m. UTC | #3
On 08/18/2009 10:40 PM, Mohammed Gamal wrote:
>>
>>> +       kvm_x86_ops->set_segment(ctxt->vcpu,&segment, seg);
>>>
>>>        
> But we're popping the contents of the stack top to a segment register
> which is going to be of 16-bits anyway, so we know the length before
> hand, no?
>    

No, the operand size attribute determines the change to rsp.  If it's 
larger than 2 bytes we drop the excess bits.  See the documentation of 
the POP instruction.

>> Note some of these instructions are not encodable in long mode; need to
>> check for that instead of emulating the wrong instruction.
>>      
> I was under the impression that the emulator doesn't support long mode
> yet, is that still the case?
>    

The emulator has always supported long mode, we need it for mmio and 
pagetable updates.
diff mbox

Patch

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2eb807a..438d423 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -92,19 +92,20 @@  static u32 opcode_table[256] = {
 	/* 0x00 - 0x07 */
 	ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
 	ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-	ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 0, 0,
+	ByteOp | DstAcc | SrcImm, DstAcc | SrcImm, 
+	ImplicitOps | Stack, ImplicitOps | Stack,
 	/* 0x08 - 0x0F */
 	ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
 	ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-	0, 0, 0, 0,
+	0, 0, ImplicitOps | Stack, 0,
 	/* 0x10 - 0x17 */
 	ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
 	ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-	0, 0, 0, 0,
+	0, 0, ImplicitOps | Stack, ImplicitOps | Stack,
 	/* 0x18 - 0x1F */
 	ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
 	ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
-	0, 0, 0, 0,
+	0, 0, ImplicitOps | Stack, ImplicitOps | Stack,
 	/* 0x20 - 0x27 */
 	ByteOp | DstMem | SrcReg | ModRM, DstMem | SrcReg | ModRM,
 	ByteOp | DstReg | SrcMem | ModRM, DstReg | SrcMem | ModRM,
@@ -1186,6 +1187,30 @@  static int emulate_pop(struct x86_emulate_ctxt *ctxt,
 	return rc;
 }
 
+static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt, int seg)
+{
+	struct decode_cache *c = &ctxt->decode;
+	struct kvm_segment segment;
+
+	kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+	
+	c->src.val = segment.selector;
+	emulate_push(ctxt);
+}
+
+static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
+			     struct x86_emulate_ops *ops, int seg)
+{
+	struct kvm_segment segment;
+	int rc;
+
+	kvm_x86_ops->get_segment(ctxt->vcpu, &segment, seg);
+	rc = emulate_pop(ctxt, ops, &segment.selector, sizeof(uint16_t));
+	kvm_x86_ops->set_segment(ctxt->vcpu, &segment, seg);
+
+	return rc;
+}
+
 static inline int emulate_grp1a(struct x86_emulate_ctxt *ctxt,
 				struct x86_emulate_ops *ops)
 {
@@ -1707,18 +1732,45 @@  special_insn:
 	      add:		/* add */
 		emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
 		break;
+	case 0x06:		/* push es */
+		emulate_push_sreg(ctxt, VCPU_SREG_ES);
+		break;
+	case 0x07:		/* pop es */
+		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
+		if (rc != 0)
+			goto done;
+		break;
 	case 0x08 ... 0x0d:
 	      or:		/* or */
 		emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
 		break;
+	case 0x0e:		/* push cs */
+		emulate_push_sreg(ctxt, VCPU_SREG_CS);
+		break;
 	case 0x10 ... 0x15:
 	      adc:		/* adc */
 		emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
 		break;
+	case 0x16:		/* push ss */
+		emulate_push_sreg(ctxt, VCPU_SREG_SS);
+		break;
+	case 0x17:		/* pop ss */
+		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS);
+		if (rc != 0)
+			goto done;
+		break;
 	case 0x18 ... 0x1d:
 	      sbb:		/* sbb */
 		emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
 		break;
+	case 0x1e:		/* push ds */
+		emulate_push_sreg(ctxt, VCPU_SREG_DS);
+		break;
+	case 0x1f:		/* pop ds */
+		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS);
+		if (rc != 0)
+			goto done;
+		break;
 	case 0x20 ... 0x25:
 	      and:		/* and */
 		emulate_2op_SrcV("and", c->src, c->dst, ctxt->eflags);