Re: [PATCH][RESEND] x86_emulator: Emulate cld and std instruction
On Thu, Jul 31, 2008 at 01:25:40PM +0300, Mohammed Gamal wrote: > The only check was if CS RPL equals SS RPL, so what happens is that > when a vmexit and then a vmentry occurs CS and SS RPL maybe equal, but Yes same problem here, vmentry fails because CS is set to >16 bit value (I think) but the last two bits of CS and SS are the same. That surely is the wrong check to know if we've to invoke emulation on vmentry failure. > other checks might cause a vmentry failure. I've written a function to > check that guest state is VMX valid - although not all checks are > implemented yet, and I no more get the occasional vmexits. Can you send me your latest patch so I can test. I tried to force unconditional emulation on invalid guest state exception generated by vmentry failure but even emulation fails for me, and then your patch sets SS to sane value but that's meaningless here as it's CS that is set to >16bit value. I've yet to trace where emulation fails, but I suspect it's fetching instructions from the wrong place by not emulating the segment-caches set by protected mode, maybe not. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RESEND] x86_emulator: Emulate cld and std instruction
Here is an update patch. This adds 'cld' and 'std' instructions. The group table entries are added. Signed-off-by: Mohammed Gamal <[EMAIL PROTECTED]> --- arch/x86/kvm/x86_emulate.c | 10 +- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c index 07a277c..4c7ca7f 100644 --- a/arch/x86/kvm/x86_emulate.c +++ b/arch/x86/kvm/x86_emulate.c @@ -182,7 +182,7 @@ static u16 opcode_table[256] = { ImplicitOps, ImplicitOps, Group | Group3_Byte, Group | Group3, /* 0xF8 - 0xFF */ ImplicitOps, 0, ImplicitOps, ImplicitOps, - 0, 0, Group | Group4, Group | Group5, + ImplicitOps, ImplicitOps, Group | Group4, Group | Group5, }; static u16 twobyte_table[256] = { @@ -1757,6 +1757,14 @@ special_insn: ctxt->eflags |= X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; + case 0xfc: /* cld */ + ctxt->eflags &= ~EFLG_DF; + c->dst.type = OP_NONE; /* Disable writeback. */ + break; + case 0xfd: /* std */ + ctxt->eflags |= EFLG_DF; + c->dst.type = OP_NONE; /* Disable writeback. */ + break; case 0xfe ... 0xff: /* Grp4/Grp5 */ rc = emulate_grp45(ctxt, ops); if (rc != 0) -- 1.5.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RESEND] x86_emulator: Emulate cld and std instruction
Mohammed Gamal wrote: On Thu, Jul 31, 2008 at 12:57 PM, Avi Kivity <[EMAIL PROTECTED]> wrote: Mohammed Gamal wrote: This patch adds 'cld' and 'std' instructions to the emulator I presume for big real mode? Which guest wants it? Actually I was experimenting with checking guest state and emulating on mode switches. When I was doing so, it used to happen with all guests, so I guess it maybe something related to the BIOS not a specific guest. There is indeed a cld in rombios.c (after rombios32_05), but it appears to be after all segments have been initialized. BTW, the other way we can do is still handle vmentry failures. I discovered that the problem that caused occasional exits with Guillaume's patches is very likely to be that not enough checks on the guest state. The only check was if CS RPL equals SS RPL, so what happens is that when a vmexit and then a vmentry occurs CS and SS RPL maybe equal, but other checks might cause a vmentry failure. I've written a function to check that guest state is VMX valid - although not all checks are implemented yet, and I no more get the occasional vmexits. I'm still worried about undoing the interrupt injection setup. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RESEND] x86_emulator: Emulate cld and std instruction
On Thu, Jul 31, 2008 at 12:57 PM, Avi Kivity <[EMAIL PROTECTED]> wrote: > Mohammed Gamal wrote: >> >> This patch adds 'cld' and 'std' instructions to the emulator >> >> > > I presume for big real mode? Which guest wants it? Actually I was experimenting with checking guest state and emulating on mode switches. When I was doing so, it used to happen with all guests, so I guess it maybe something related to the BIOS not a specific guest. BTW, the other way we can do is still handle vmentry failures. I discovered that the problem that caused occasional exits with Guillaume's patches is very likely to be that not enough checks on the guest state. The only check was if CS RPL equals SS RPL, so what happens is that when a vmexit and then a vmentry occurs CS and SS RPL maybe equal, but other checks might cause a vmentry failure. I've written a function to check that guest state is VMX valid - although not all checks are implemented yet, and I no more get the occasional vmexits. > >> @@ -1755,6 +1755,14 @@ special_insn: >>ctxt->eflags |= X86_EFLAGS_IF; >>c->dst.type = OP_NONE; /* Disable writeback. */ >>break; >> + case 0xfc: /* cld */ >> + ctxt->eflags &= ~EFLG_DF; >> + c->dst.type = OP_NONE; /* Disable writeback. */ >> + break; >> + case 0xfd: /* std */ >> + ctxt->eflags |= EFLG_DF; >> + c->dst.type = OP_NONE; /* Disable writeback. */ >> + break; >> > > You need to add non-zero entries in opcode_table[] for this to work. Also, > why disable writeback? If there is not DstSomething in opcode_table[], > c->dst.type will be OP_NONE anyway. > Oh! completely forgot about that, sorry! > I would like to see test cases for emulator changes. See > users/test/x86/emulator.c. Of course, right now it is impossible to cause > cld and std to execute in the emulator. But with the proposed > invalid_guest_state() change, it should be easy to to cause this state and > force emulation. > > So how about adding optional invalid_guest_state() support (controlled by a > module parameter), then start adding instructions and test cases, then > removing the module parameter when everything works? > Sounds good -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RESEND] x86_emulator: Emulate cld and std instruction
Mohammed Gamal wrote: This patch adds 'cld' and 'std' instructions to the emulator I presume for big real mode? Which guest wants it? @@ -1755,6 +1755,14 @@ special_insn: ctxt->eflags |= X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; + case 0xfc: /* cld */ + ctxt->eflags &= ~EFLG_DF; + c->dst.type = OP_NONE; /* Disable writeback. */ + break; + case 0xfd: /* std */ + ctxt->eflags |= EFLG_DF; + c->dst.type = OP_NONE; /* Disable writeback. */ + break; You need to add non-zero entries in opcode_table[] for this to work. Also, why disable writeback? If there is not DstSomething in opcode_table[], c->dst.type will be OP_NONE anyway. I would like to see test cases for emulator changes. See users/test/x86/emulator.c. Of course, right now it is impossible to cause cld and std to execute in the emulator. But with the proposed invalid_guest_state() change, it should be easy to to cause this state and force emulation. So how about adding optional invalid_guest_state() support (controlled by a module parameter), then start adding instructions and test cases, then removing the module parameter when everything works? -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][RESEND] x86_emulator: Emulate cld and std instruction
This patch adds 'cld' and 'std' instructions to the emulator Signed-off-by: Mohammed Gamal <[EMAIL PROTECTED]> --- arch/x86/kvm/x86_emulate.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c index d5da7f1..9c72a77 100644 --- a/arch/x86/kvm/x86_emulate.c +++ b/arch/x86/kvm/x86_emulate.c @@ -1755,6 +1755,14 @@ special_insn: ctxt->eflags |= X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; + case 0xfc: /* cld */ + ctxt->eflags &= ~EFLG_DF; + c->dst.type = OP_NONE; /* Disable writeback. */ + break; + case 0xfd: /* std */ + ctxt->eflags |= EFLG_DF; + c->dst.type = OP_NONE; /* Disable writeback. */ + break; case 0xfe ... 0xff: /* Grp4/Grp5 */ rc = emulate_grp45(ctxt, ops); if (rc != 0) -- 1.5.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html