Re: [Qemu-devel] Guest SIGILL when different IO is implemented

[Miltiadis Hatzimihail]

​I ve seen that I ve accidentally dropped the list from the email..

So for fstps for the 32bit - how does this patch look?

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c

index a252081d..83b0fd2 100644

--- a/arch/x86/kvm/emulate.c

+++ b/arch/x86/kvm/emulate.c

@@ -1139,6 +1139,27 @@ static int em_fninit(struct x86_emulate_ctxt *ctxt)

               return X86EMUL_CONTINUE;

}



+static int em_fld(struct x86_emulate_ctxt *ctxt)

+{

+             ctxt->ops->get_fpu(ctxt);

+             asm volatile("fld %0": "+m"(ctxt->src.val));

+             ctxt->ops->put_fpu(ctxt);

+

+             return X86EMUL_CONTINUE;

+}

+

+static int em_fstp(struct x86_emulate_ctxt *ctxt)

+{

+             u32 fcw;

+

+             ctxt->ops->get_fpu(ctxt);

+             asm volatile("fstp %0": "+m"(fcw));

+             ctxt->ops->put_fpu(ctxt);

+

+             ctxt->dst.val = fcw;

+             return X86EMUL_CONTINUE;

+}

+

static int em_fnstcw(struct x86_emulate_ctxt *ctxt)

{

               u16 fcw;

@@ -4438,7 +4459,7 @@ static const struct gprefix pfx_0f_e7 = {

};



 static const struct escape escape_d9 = { {

-              N, N, N, N, N, N, N, I(DstMem16 | Mov, em_fnstcw),

+             I(SrcMem | Stack, em_fld), N, N, I(DstMem | Mov | Stack,
em_fstp), N, N, N, I(DstMem16 | Mov, em_fnstcw),

}, {

               /* 0xC0 - 0xC7 */

               N, N, N, N, N, N, N, N,​


Miltiadis Hatzimihail


On Tue, May 16, 2017 at 9:09 AM, Miltiadis Hatzimihail <
address@hidden> wrote:

> How does this patch look to you?
>
> --- a/arch/x86/kvm/emulate.c
>
> +++ b/arch/x86/kvm/emulate.c
>
> @@ -1139,6 +1139,27 @@ static int em_fninit(struct x86_emulate_ctxt *ctxt)
>
>                return X86EMUL_CONTINUE;
>
> }
>
>
>
> +static int em_fld(struct x86_emulate_ctxt *ctxt)
>
> +{
>
> +             ctxt->ops->get_fpu(ctxt);
>
> +             asm volatile("fld %0": "+m"(ctxt->src.val));
>
> +             ctxt->ops->put_fpu(ctxt);
>
> +
>
> +             return X86EMUL_CONTINUE;
>
> +}
>
> +
>
> +static int em_fstp(struct x86_emulate_ctxt *ctxt)
>
> +{
>
> +             u32 fcw;
>
> +
>
> +             ctxt->ops->get_fpu(ctxt);
>
> +             asm volatile("fstp %0": "+m"(fcw));
>
> +             ctxt->ops->put_fpu(ctxt);
>
> +
>
> +             ctxt->dst.val = fcw;
>
> +             return X86EMUL_CONTINUE;
>
> +}
>
> +
>
> static int em_fnstcw(struct x86_emulate_ctxt *ctxt)
>
> {
>
>                u16 fcw;
>
> @@ -4438,7 +4459,7 @@ static const struct gprefix pfx_0f_e7 = {
>
> };
>
>
>
>  static const struct escape escape_d9 = { {
>
> -              N, N, N, N, N, N, N, I(DstMem16 | Mov, em_fnstcw),
>
> +             I(SrcMem | Stack, em_fld), N, N, I(DstMem | Mov | Stack,
> em_fstp), N, N, N, I(DstMem16 | Mov, em_fnstcw),
>
> }, {
>
>                /* 0xC0 - 0xC7 */
>
>                N, N, N, N, N, N, N, N,
>
>
> Miltiadis Hatzimihail
>
>
> On Mon, May 15, 2017 at 8:58 AM, Paolo Bonzini <address@hidden>
> wrote:
>
>>
>>
>> On 15/05/2017 08:42, Miltiadis Hatzimihail wrote:
>> > Thanks - is it just a matter of adjusting your old patch for this
>> > command? (I looked up the opcode and it was dd or d9 I think).
>>
>> The encoding of fstps is a bit different because it's an x87
>> instruction, but the idea is the same.
>>
>> Paolo
>> >
>> > Miltiadis Hatzimihail
>> >
>> >
>> > On Fri, May 12, 2017 at 1:00 PM, Paolo Bonzini <address@hidden
>> > <mailto:address@hidden>> wrote:
>> >
>> >
>> >
>> >     On 12/05/2017 09:26, Miltiadis Hatzimihail wrote:
>> >     > I ve tried the same today using a 32-bit Guest OS and the illegal
>> >     > instruction this time is
>> >     >
>> >     > fstps %(ecx)
>> >     >
>> >     > Is it a similar case to the movss one? (the previous Guest I was
>> using
>> >     > was 64 bit).
>> >
>> >     Yes, it is.
>> >
>> >     Paolo
>> >
>> >     > Also, I had to start QEMU using the following command line
>> options:
>> >     >
>> >     > qemu -cpu host,-sse2
>> >     >
>> >     > because one my programs was giving me an illegal instruction
>> based on
>> >     > the above and it worked by disabling it.
>> >     >
>> >     > ​Regards
>> >     > Milton
>> >     >
>> >     > On Thu, May 11, 2017 at 2:12 PM, Miltiadis Hatzimihail
>> >     > <address@hidden <mailto:address@hidden>
>> >     <mailto:address@hidden <mailto:address@hidden>>>
>> wrote:
>> >     >
>> >     >     Ok many thanks for your help.
>> >     >
>> >     >     ​Milton
>> >     >
>> >     >     On Thu, May 11, 2017 at 2:11 PM, Paolo Bonzini <
>> address@hidden <mailto:address@hidden>
>> >     >     <mailto:address@hidden <mailto:address@hidden>>>
>> wrote:
>> >     >
>> >     >
>> >     >
>> >     >         On 11/05/2017 14:47, Miltiadis Hatzimihail wrote:
>> >     >         > That's great thanks for the clarification.
>> >     >         >
>> >     >         > Is this patch going to make it to the mainline at some
>> >     point?
>> >     >
>> >     >         Not exactly as is, because it has a small defect (it
>> >     always reads 16
>> >     >         bytes from memory), but something like that will.
>> >     >
>> >     >         Paolo
>> >     >
>> >     >
>> >     >
>> >
>> >
>>
>
>