[Qemu-devel] [PATCH] i386: fix read/write cr with icount option

Mihail Abakumov posted 1 patch 6 years, 10 months ago
Failed in applying to current master (apply log)
target/i386/translate.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
[Qemu-devel] [PATCH] i386: fix read/write cr with icount option
Posted by Mihail Abakumov 6 years, 10 months ago
Running Windows with icount causes a crash in instruction of write cr. 
This patch fixes it.

Reading and writing cr cause an icount read because there are called 
cpu_get_apic_tpr and cpu_set_apic_tpr functions. So, there is need 
gen_io_start()/gen_io_end() calls.

---
  target/i386/translate.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 06d8833..3b009bd 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -7907,14 +7907,26 @@ static target_ulong disas_insn(CPUX86State *env, 
DisasContext *s,
                  gen_update_cc_op(s);
                  gen_jmp_im(pc_start - s->cs_base);
                  if (b & 2) {
+                    if (s->tb->cflags & CF_USE_ICOUNT) {
+                        gen_io_start();
+                    }
                      gen_op_mov_v_reg(ot, cpu_T0, rm);
                      gen_helper_write_crN(cpu_env, tcg_const_i32(reg),
                                           cpu_T0);
+                    if (s->tb->cflags & CF_USE_ICOUNT) {
+                        gen_io_end();
+                    }
                      gen_jmp_im(s->pc - s->cs_base);
                      gen_eob(s);
                  } else {
+                    if (s->tb->cflags & CF_USE_ICOUNT) {
+                        gen_io_start();
+                    }
                      gen_helper_read_crN(cpu_T0, cpu_env, 
tcg_const_i32(reg));
                      gen_op_mov_reg_v(ot, rm, cpu_T0);
+                    if (s->tb->cflags & CF_USE_ICOUNT) {
+                        gen_io_end();
+                    }
                  }
                  break;
              default:
-- 
1.9.1

Re: [Qemu-devel] [PATCH] i386: fix read/write cr with icount option
Posted by Paolo Bonzini 6 years, 10 months ago

On 19/05/2017 11:36, Mihail Abakumov wrote:
> Running Windows with icount causes a crash in instruction of write cr.
> This patch fixes it.
> 
> Reading and writing cr cause an icount read because there are called
> cpu_get_apic_tpr and cpu_set_apic_tpr functions. So, there is need
> gen_io_start()/gen_io_end() calls.

The patch looks good, but lacks a signoff.  Please read the Developer 
Certificate of Origin[1] and reply to this email with "Signed-off-by: 
Mihail Abakumov <mikhail.abakumov@ispras.ru>".


[1]     Developer's Certificate of Origin 1.1

        By making a contribution to this project, I certify that:

        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or

        (b) The contribution is based upon previous work that, to the best
            of my knowledge, is covered under an appropriate open source
            license and I have the right under that license to submit that
            work with modifications, whether created in whole or in part
            by me, under the same open source license (unless I am
            permitted to submit under a different license), as indicated
            in the file; or

        (c) The contribution was provided directly to me by some other
            person who certified (a), (b) or (c) and I have not modified
            it.

        (d) I understand and agree that this project and the contribution
            are public and that a record of the contribution (including all
            personal information I submit with it, including my sign-off) is
            maintained indefinitely and may be redistributed consistent with
            this project or the open source license(s) involved.

Thanks,

Paolo
> ---
>  target/i386/translate.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 06d8833..3b009bd 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -7907,14 +7907,26 @@ static target_ulong disas_insn(CPUX86State *env,
> DisasContext *s,
>                  gen_update_cc_op(s);
>                  gen_jmp_im(pc_start - s->cs_base);
>                  if (b & 2) {
> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
> +                        gen_io_start();
> +                    }
>                      gen_op_mov_v_reg(ot, cpu_T0, rm);
>                      gen_helper_write_crN(cpu_env, tcg_const_i32(reg),
>                                           cpu_T0);
> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
> +                        gen_io_end();
> +                    }
>                      gen_jmp_im(s->pc - s->cs_base);
>                      gen_eob(s);
>                  } else {
> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
> +                        gen_io_start();
> +                    }
>                      gen_helper_read_crN(cpu_T0, cpu_env,
> tcg_const_i32(reg));
>                      gen_op_mov_reg_v(ot, rm, cpu_T0);
> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
> +                        gen_io_end();
> +                    }
>                  }
>                  break;
>              default:

Re: [Qemu-devel] [PATCH] i386: fix read/write cr with icount option
Posted by Mihail Abakumov 6 years, 10 months ago
Paolo Bonzini писал 2017-05-19 12:59:
> On 19/05/2017 11:36, Mihail Abakumov wrote:
>> Running Windows with icount causes a crash in instruction of write cr.
>> This patch fixes it.
>> 
>> Reading and writing cr cause an icount read because there are called
>> cpu_get_apic_tpr and cpu_set_apic_tpr functions. So, there is need
>> gen_io_start()/gen_io_end() calls.
> 
> The patch looks good, but lacks a signoff.  Please read the Developer
> Certificate of Origin[1] and reply to this email with "Signed-off-by:
> Mihail Abakumov <mikhail.abakumov@ispras.ru>".
> 
> 
> [1]     Developer's Certificate of Origin 1.1
> 
>         By making a contribution to this project, I certify that:
> 
>         (a) The contribution was created in whole or in part by me and 
> I
>             have the right to submit it under the open source license
>             indicated in the file; or
> 
>         (b) The contribution is based upon previous work that, to the 
> best
>             of my knowledge, is covered under an appropriate open 
> source
>             license and I have the right under that license to submit 
> that
>             work with modifications, whether created in whole or in 
> part
>             by me, under the same open source license (unless I am
>             permitted to submit under a different license), as 
> indicated
>             in the file; or
> 
>         (c) The contribution was provided directly to me by some other
>             person who certified (a), (b) or (c) and I have not 
> modified
>             it.
> 
>         (d) I understand and agree that this project and the 
> contribution
>             are public and that a record of the contribution (including 
> all
>             personal information I submit with it, including my 
> sign-off) is
>             maintained indefinitely and may be redistributed consistent 
> with
>             this project or the open source license(s) involved.
> 
> Thanks,
> 
> Paolo

Signed-off-by: Mihail Abakumov <mikhail.abakumov@ispras.ru>

>> ---
>>  target/i386/translate.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>> 
>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>> index 06d8833..3b009bd 100644
>> --- a/target/i386/translate.c
>> +++ b/target/i386/translate.c
>> @@ -7907,14 +7907,26 @@ static target_ulong disas_insn(CPUX86State 
>> *env,
>> DisasContext *s,
>>                  gen_update_cc_op(s);
>>                  gen_jmp_im(pc_start - s->cs_base);
>>                  if (b & 2) {
>> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
>> +                        gen_io_start();
>> +                    }
>>                      gen_op_mov_v_reg(ot, cpu_T0, rm);
>>                      gen_helper_write_crN(cpu_env, tcg_const_i32(reg),
>>                                           cpu_T0);
>> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
>> +                        gen_io_end();
>> +                    }
>>                      gen_jmp_im(s->pc - s->cs_base);
>>                      gen_eob(s);
>>                  } else {
>> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
>> +                        gen_io_start();
>> +                    }
>>                      gen_helper_read_crN(cpu_T0, cpu_env,
>> tcg_const_i32(reg));
>>                      gen_op_mov_reg_v(ot, rm, cpu_T0);
>> +                    if (s->tb->cflags & CF_USE_ICOUNT) {
>> +                        gen_io_end();
>> +                    }
>>                  }
>>                  break;
>>              default: