| Message ID | 1378237329-18630-1-git-send-email-brogers@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 03, 2013 at 01:42:09PM -0600, Bruce Rogers wrote: > Opcode CA > > This gets used by a DOS based NetWare guest. > Thanks. All emulator fixes should be accompanied by unit test[1]. Can you write one? [1] https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/tree/x86/realmode.c > Signed-off-by: Bruce Rogers <brogers@suse.com> > --- > arch/x86/kvm/emulate.c | 23 ++++++++++++++++++++++- > 1 files changed, 22 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 2bc1e81..aee238a 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -2025,6 +2025,26 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt) > return rc; > } > > +static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) > +{ > + int rc; > + unsigned long cs; > + > + rc = emulate_pop(ctxt, &ctxt->_eip, ctxt->op_bytes); > + if (rc != X86EMUL_CONTINUE) > + return rc; > + if (ctxt->op_bytes == 4) > + ctxt->_eip = (u32)ctxt->_eip; > + rc = emulate_pop(ctxt, &cs, ctxt->op_bytes); > + if (rc != X86EMUL_CONTINUE) > + return rc; > + rc = load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS); > + if (rc != X86EMUL_CONTINUE) > + return rc; > + rsp_increment(ctxt, ctxt->src.val); > + return X86EMUL_CONTINUE; > +} > + > static int em_cmpxchg(struct x86_emulate_ctxt *ctxt) > { > /* Save real source value, then compare EAX against destination. */ > @@ -3763,7 +3783,8 @@ static const struct opcode opcode_table[256] = { > G(ByteOp, group11), G(0, group11), > /* 0xC8 - 0xCF */ > I(Stack | SrcImmU16 | Src2ImmByte, em_enter), I(Stack, em_leave), > - N, I(ImplicitOps | Stack, em_ret_far), > + I(ImplicitOps | Stack | SrcImmU16, em_ret_far_imm), > + I(ImplicitOps | Stack, em_ret_far), > D(ImplicitOps), DI(SrcImmByte, intn), > D(ImplicitOps | No64), II(ImplicitOps, em_iret, iret), > /* 0xD0 - 0xD7 */ > -- > 1.7.7 -- Gleb. -- 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
On Tue, Sep 03, 2013 at 01:42:09PM -0600, Bruce Rogers wrote: > Opcode CA > > This gets used by a DOS based NetWare guest. > > Signed-off-by: Bruce Rogers <brogers@suse.com> > --- > arch/x86/kvm/emulate.c | 23 ++++++++++++++++++++++- > 1 files changed, 22 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 2bc1e81..aee238a 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -2025,6 +2025,26 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt) > return rc; > } > > +static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) > +{ > + int rc; > + unsigned long cs; > + > + rc = emulate_pop(ctxt, &ctxt->_eip, ctxt->op_bytes); > + if (rc != X86EMUL_CONTINUE) > + return rc; > + if (ctxt->op_bytes == 4) > + ctxt->_eip = (u32)ctxt->_eip; > + rc = emulate_pop(ctxt, &cs, ctxt->op_bytes); > + if (rc != X86EMUL_CONTINUE) > + return rc; > + rc = load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS); > + if (rc != X86EMUL_CONTINUE) > + return rc; > + rsp_increment(ctxt, ctxt->src.val); > + return X86EMUL_CONTINUE; > +} > + Why not: static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) { int rc; rc = em_ret_far(struct x86_emulate_ctxt *ctxt); if (rc != X86EMUL_CONTINUE) return rc; rsp_increment(ctxt, ctxt->src.val); return X86EMUL_CONTINUE; } -- Gleb. -- 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
>>> On 9/8/2013 at 07:13 AM, Gleb Natapov <gleb@redhat.com> wrote: > On Tue, Sep 03, 2013 at 01:42:09PM -0600, Bruce Rogers wrote: >> Opcode CA >> >> This gets used by a DOS based NetWare guest. >> >> Signed-off-by: Bruce Rogers <brogers@suse.com> >> --- >> arch/x86/kvm/emulate.c | 23 ++++++++++++++++++++++- >> 1 files changed, 22 insertions(+), 1 deletions(-) >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index 2bc1e81..aee238a 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -2025,6 +2025,26 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt) >> return rc; >> } >> >> +static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) >> +{ >> + int rc; >> + unsigned long cs; >> + >> + rc = emulate_pop(ctxt, &ctxt->_eip, ctxt->op_bytes); >> + if (rc != X86EMUL_CONTINUE) >> + return rc; >> + if (ctxt->op_bytes == 4) >> + ctxt->_eip = (u32)ctxt->_eip; >> + rc = emulate_pop(ctxt, &cs, ctxt->op_bytes); >> + if (rc != X86EMUL_CONTINUE) >> + return rc; >> + rc = load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS); >> + if (rc != X86EMUL_CONTINUE) >> + return rc; >> + rsp_increment(ctxt, ctxt->src.val); >> + return X86EMUL_CONTINUE; >> +} >> + > Why not: > > static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) > { > int rc; > rc = em_ret_far(struct x86_emulate_ctxt *ctxt); > if (rc != X86EMUL_CONTINUE) > return rc; > rsp_increment(ctxt, ctxt->src.val); > return X86EMUL_CONTINUE; > } > > -- > Gleb. Yes, that does seem better. Ack. Bruce -- 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
On Mon, Sep 09, 2013 at 07:09:15AM -0600, Bruce Rogers wrote: > >>> On 9/8/2013 at 07:13 AM, Gleb Natapov <gleb@redhat.com> wrote: > > On Tue, Sep 03, 2013 at 01:42:09PM -0600, Bruce Rogers wrote: > >> Opcode CA > >> > >> This gets used by a DOS based NetWare guest. > >> > >> Signed-off-by: Bruce Rogers <brogers@suse.com> > >> --- > >> arch/x86/kvm/emulate.c | 23 ++++++++++++++++++++++- > >> 1 files changed, 22 insertions(+), 1 deletions(-) > >> > >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > >> index 2bc1e81..aee238a 100644 > >> --- a/arch/x86/kvm/emulate.c > >> +++ b/arch/x86/kvm/emulate.c > >> @@ -2025,6 +2025,26 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt) > >> return rc; > >> } > >> > >> +static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) > >> +{ > >> + int rc; > >> + unsigned long cs; > >> + > >> + rc = emulate_pop(ctxt, &ctxt->_eip, ctxt->op_bytes); > >> + if (rc != X86EMUL_CONTINUE) > >> + return rc; > >> + if (ctxt->op_bytes == 4) > >> + ctxt->_eip = (u32)ctxt->_eip; > >> + rc = emulate_pop(ctxt, &cs, ctxt->op_bytes); > >> + if (rc != X86EMUL_CONTINUE) > >> + return rc; > >> + rc = load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS); > >> + if (rc != X86EMUL_CONTINUE) > >> + return rc; > >> + rsp_increment(ctxt, ctxt->src.val); > >> + return X86EMUL_CONTINUE; > >> +} > >> + > > Why not: > > > > static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) > > { > > int rc; > > rc = em_ret_far(struct x86_emulate_ctxt *ctxt); > > if (rc != X86EMUL_CONTINUE) > > return rc; > > rsp_increment(ctxt, ctxt->src.val); > > return X86EMUL_CONTINUE; > > } > > > > -- > > Gleb. > > Yes, that does seem better. Ack. > Somebody still needs to write a proper patch :) Can you do it please? -- Gleb. -- 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
>>> On 9/9/2013 at 07:10 AM, Gleb Natapov <gleb@redhat.com> wrote: > On Mon, Sep 09, 2013 at 07:09:15AM -0600, Bruce Rogers wrote: >> >>> On 9/8/2013 at 07:13 AM, Gleb Natapov <gleb@redhat.com> wrote: >> > On Tue, Sep 03, 2013 at 01:42:09PM -0600, Bruce Rogers wrote: >> >> Opcode CA >> >> >> >> This gets used by a DOS based NetWare guest. >> >> >> >> Signed-off-by: Bruce Rogers <brogers@suse.com> >> >> --- >> >> arch/x86/kvm/emulate.c | 23 ++++++++++++++++++++++- >> >> 1 files changed, 22 insertions(+), 1 deletions(-) >> >> >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> >> index 2bc1e81..aee238a 100644 >> >> --- a/arch/x86/kvm/emulate.c >> >> +++ b/arch/x86/kvm/emulate.c >> >> @@ -2025,6 +2025,26 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt) >> >> return rc; >> >> } >> >> >> >> +static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) >> >> +{ >> >> + int rc; >> >> + unsigned long cs; >> >> + >> >> + rc = emulate_pop(ctxt, &ctxt->_eip, ctxt->op_bytes); >> >> + if (rc != X86EMUL_CONTINUE) >> >> + return rc; >> >> + if (ctxt->op_bytes == 4) >> >> + ctxt->_eip = (u32)ctxt->_eip; >> >> + rc = emulate_pop(ctxt, &cs, ctxt->op_bytes); >> >> + if (rc != X86EMUL_CONTINUE) >> >> + return rc; >> >> + rc = load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS); >> >> + if (rc != X86EMUL_CONTINUE) >> >> + return rc; >> >> + rsp_increment(ctxt, ctxt->src.val); >> >> + return X86EMUL_CONTINUE; >> >> +} >> >> + >> > Why not: >> > >> > static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) >> > { >> > int rc; >> > rc = em_ret_far(struct x86_emulate_ctxt *ctxt); >> > if (rc != X86EMUL_CONTINUE) >> > return rc; >> > rsp_increment(ctxt, ctxt->src.val); >> > return X86EMUL_CONTINUE; >> > } >> > >> > -- >> > Gleb. >> >> Yes, that does seem better. Ack. >> > Somebody still needs to write a proper patch :) Can you do it please? Sure, will do. Bruce -- 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
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 2bc1e81..aee238a 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -2025,6 +2025,26 @@ static int em_ret_far(struct x86_emulate_ctxt *ctxt) return rc; } +static int em_ret_far_imm(struct x86_emulate_ctxt *ctxt) +{ + int rc; + unsigned long cs; + + rc = emulate_pop(ctxt, &ctxt->_eip, ctxt->op_bytes); + if (rc != X86EMUL_CONTINUE) + return rc; + if (ctxt->op_bytes == 4) + ctxt->_eip = (u32)ctxt->_eip; + rc = emulate_pop(ctxt, &cs, ctxt->op_bytes); + if (rc != X86EMUL_CONTINUE) + return rc; + rc = load_segment_descriptor(ctxt, (u16)cs, VCPU_SREG_CS); + if (rc != X86EMUL_CONTINUE) + return rc; + rsp_increment(ctxt, ctxt->src.val); + return X86EMUL_CONTINUE; +} + static int em_cmpxchg(struct x86_emulate_ctxt *ctxt) { /* Save real source value, then compare EAX against destination. */ @@ -3763,7 +3783,8 @@ static const struct opcode opcode_table[256] = { G(ByteOp, group11), G(0, group11), /* 0xC8 - 0xCF */ I(Stack | SrcImmU16 | Src2ImmByte, em_enter), I(Stack, em_leave), - N, I(ImplicitOps | Stack, em_ret_far), + I(ImplicitOps | Stack | SrcImmU16, em_ret_far_imm), + I(ImplicitOps | Stack, em_ret_far), D(ImplicitOps), DI(SrcImmByte, intn), D(ImplicitOps | No64), II(ImplicitOps, em_iret, iret), /* 0xD0 - 0xD7 */
Opcode CA This gets used by a DOS based NetWare guest. Signed-off-by: Bruce Rogers <brogers@suse.com> --- arch/x86/kvm/emulate.c | 23 ++++++++++++++++++++++- 1 files changed, 22 insertions(+), 1 deletions(-)