[Qemu-devel] [RFCv2 PATCH 0/1] SPARC64: add icount support

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [RFCv2 PATCH 0/1] SPARC64: add icount support
@ 2018-06-14 10:53 Mark Cave-Ayland
  2018-06-14 10:53 ` [Qemu-devel] [RFCv2 PATCH 1/1] " Mark Cave-Ayland
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Cave-Ayland @ 2018-06-14 10:53 UTC (permalink / raw)
  To: qemu-devel, atar4qemu, cota, richard.henderson

This is a patch I've had in an earlier form for a while but never really developed
due to experiencing random hangs when writing to the SPARC64 CPU timers.

Fortunately it seems the recent icount timer fixes have resolved the underlying
issues and with this patch I am able to boot all my qemu-system-sparc64 OpenBIOS
test images.

I've marked this as an RFC for two reasons: firstly I'm not sure I've fully covered
all the cases where we need to exit the TB in order to allow the timer interrupt to
fire, and secondly I do occasionally see messages booting Milax in the form of
"WARNING: Time of Day clock error: reason [Jumped by 0x134]".

Hopefully more sets of eyes will be able to point me in the right direction related
to the Time of Day (m48t59) error and the TB exits in order to get this ready for
commit.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

v2:
- Rebase onto master

Mark Cave-Ayland (1):
  SPARC64: add icount support

 target/sparc/translate.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] [RFCv2 PATCH 1/1] SPARC64: add icount support
  2018-06-14 10:53 [Qemu-devel] [RFCv2 PATCH 0/1] SPARC64: add icount support Mark Cave-Ayland
@ 2018-06-14 10:53 ` Mark Cave-Ayland
  2018-06-14 21:02   ` Richard Henderson
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Cave-Ayland @ 2018-06-14 10:53 UTC (permalink / raw)
  To: qemu-devel, atar4qemu, cota, richard.henderson

This patch adds gen_io_start()/gen_io_end() to various instructions as required
in order to boot my OpenBIOS test images on qemu-system-sparc64 with icount
enabled.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/sparc/translate.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index f3d430c1b2..56101387a8 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -3400,11 +3400,17 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                         r_const = tcg_const_i32(dc->mem_idx);
                         tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                        offsetof(CPUSPARCState, tick));
+                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                            gen_io_start();
+                        }
                         gen_helper_tick_get_count(cpu_dst, cpu_env, r_tickptr,
                                                   r_const);
                         tcg_temp_free_ptr(r_tickptr);
                         tcg_temp_free_i32(r_const);
                         gen_store_gpr(dc, rd, cpu_dst);
+                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                            gen_io_end();
+                        }
                     }
                     break;
                 case 0x5: /* V9 rdpc */
@@ -3447,11 +3453,17 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                         r_const = tcg_const_i32(dc->mem_idx);
                         tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                        offsetof(CPUSPARCState, stick));
+                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                            gen_io_start();
+                        }
                         gen_helper_tick_get_count(cpu_dst, cpu_env, r_tickptr,
                                                   r_const);
                         tcg_temp_free_ptr(r_tickptr);
                         tcg_temp_free_i32(r_const);
                         gen_store_gpr(dc, rd, cpu_dst);
+                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                            gen_io_end();
+                        }
                     }
                     break;
                 case 0x19: /* System tick compare */
@@ -3576,10 +3588,16 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                         r_const = tcg_const_i32(dc->mem_idx);
                         tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                        offsetof(CPUSPARCState, tick));
+                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                            gen_io_start();
+                        }
                         gen_helper_tick_get_count(cpu_tmp0, cpu_env,
                                                   r_tickptr, r_const);
                         tcg_temp_free_ptr(r_tickptr);
                         tcg_temp_free_i32(r_const);
+                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                            gen_io_end();
+                        }
                     }
                     break;
                 case 5: // tba
@@ -4385,9 +4403,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                     r_tickptr = tcg_temp_new_ptr();
                                     tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                                    offsetof(CPUSPARCState, tick));
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_start();
+                                    }
                                     gen_helper_tick_set_limit(r_tickptr,
                                                               cpu_tick_cmpr);
                                     tcg_temp_free_ptr(r_tickptr);
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_end();
+                                        /* End TB to handle timer interrupt */
+                                        save_state(dc);
+                                        gen_op_next_insn();
+                                        tcg_gen_exit_tb(NULL, 0);
+                                        dc->base.is_jmp = DISAS_NORETURN;
+                                    }
                                 }
                                 break;
                             case 0x18: /* System tick */
@@ -4403,9 +4432,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                     r_tickptr = tcg_temp_new_ptr();
                                     tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                                    offsetof(CPUSPARCState, stick));
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_start();
+                                    }
                                     gen_helper_tick_set_count(r_tickptr,
                                                               cpu_tmp0);
                                     tcg_temp_free_ptr(r_tickptr);
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_end();
+                                        /* End TB to handle timer interrupt */
+                                        save_state(dc);
+                                        gen_op_next_insn();
+                                        tcg_gen_exit_tb(NULL, 0);
+                                        dc->base.is_jmp = DISAS_NORETURN;
+                                    }
                                 }
                                 break;
                             case 0x19: /* System tick compare */
@@ -4421,9 +4461,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                     r_tickptr = tcg_temp_new_ptr();
                                     tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                                    offsetof(CPUSPARCState, stick));
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_start();
+                                    }
                                     gen_helper_tick_set_limit(r_tickptr,
                                                               cpu_stick_cmpr);
                                     tcg_temp_free_ptr(r_tickptr);
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_end();
+                                        /* End TB to handle timer interrupt */
+                                        save_state(dc);
+                                        gen_op_next_insn();
+                                        tcg_gen_exit_tb(NULL, 0);
+                                        dc->base.is_jmp = DISAS_NORETURN;
+                                    }
                                 }
                                 break;
 
@@ -4531,9 +4582,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                     r_tickptr = tcg_temp_new_ptr();
                                     tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                                    offsetof(CPUSPARCState, tick));
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_start();
+                                    }
                                     gen_helper_tick_set_count(r_tickptr,
                                                               cpu_tmp0);
                                     tcg_temp_free_ptr(r_tickptr);
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_end();
+                                        /* End TB to handle timer interrupt */
+                                        save_state(dc);
+                                        gen_op_next_insn();
+                                        tcg_gen_exit_tb(NULL, 0);
+                                        dc->base.is_jmp = DISAS_NORETURN;
+                                    }
                                 }
                                 break;
                             case 5: // tba
@@ -4541,7 +4603,13 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                 break;
                             case 6: // pstate
                                 save_state(dc);
+                                if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                    gen_io_start();
+                                }
                                 gen_helper_wrpstate(cpu_env, cpu_tmp0);
+                                if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                    gen_io_end();
+                                }
                                 dc->npc = DYNAMIC_PC;
                                 break;
                             case 7: // tl
@@ -4551,7 +4619,13 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                 dc->npc = DYNAMIC_PC;
                                 break;
                             case 8: // pil
+                                if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                    gen_io_start();
+                                }
                                 gen_helper_wrpil(cpu_env, cpu_tmp0);
+                                if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                    gen_io_end();
+                                }
                                 break;
                             case 9: // cwp
                                 gen_helper_wrcwp(cpu_env, cpu_tmp0);
@@ -4642,9 +4716,20 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                     r_tickptr = tcg_temp_new_ptr();
                                     tcg_gen_ld_ptr(r_tickptr, cpu_env,
                                                    offsetof(CPUSPARCState, hstick));
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_start();
+                                    }
                                     gen_helper_tick_set_limit(r_tickptr,
                                                               cpu_hstick_cmpr);
                                     tcg_temp_free_ptr(r_tickptr);
+                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                        gen_io_end();
+                                        /* End TB to handle timer interrupt */
+                                        save_state(dc);
+                                        gen_op_next_insn();
+                                        tcg_gen_exit_tb(NULL, 0);
+                                        dc->base.is_jmp = DISAS_NORETURN;
+                                    }
                                 }
                                 break;
                             case 6: // hver readonly
@@ -5265,14 +5350,26 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                                 goto priv_insn;
                             dc->npc = DYNAMIC_PC;
                             dc->pc = DYNAMIC_PC;
+                            if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                gen_io_start();
+                            }
                             gen_helper_done(cpu_env);
+                            if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                gen_io_end();
+                            }
                             goto jmp_insn;
                         case 1:
                             if (!supervisor(dc))
                                 goto priv_insn;
                             dc->npc = DYNAMIC_PC;
                             dc->pc = DYNAMIC_PC;
+                            if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                gen_io_start();
+                            }
                             gen_helper_retry(cpu_env);
+                            if (dc->base.tb->cflags & CF_USE_ICOUNT) {
+                                gen_io_end();
+                            }
                             goto jmp_insn;
                         default:
                             goto illegal_insn;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [RFCv2 PATCH 1/1] SPARC64: add icount support
  2018-06-14 10:53 ` [Qemu-devel] [RFCv2 PATCH 1/1] " Mark Cave-Ayland
@ 2018-06-14 21:02   ` Richard Henderson
  2018-06-14 22:09     ` Mark Cave-Ayland
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2018-06-14 21:02 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, atar4qemu, cota

On 06/14/2018 12:53 AM, Mark Cave-Ayland wrote:
> +                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
> +                            gen_io_start();
> +                        }

Need to use tb_cflags(dc->base.tb) for the atomic_read therein.


> +                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
> +                                        gen_io_end();
> +                                        /* End TB to handle timer interrupt */
> +                                        save_state(dc);
> +                                        gen_op_next_insn();
> +                                        tcg_gen_exit_tb(NULL, 0);
> +                                        dc->base.is_jmp = DISAS_NORETURN;
> +                                    }

Exiting the TB should not be dependent on icount.

I would encourage you to introduce e.g. DISAS_EXIT, and code to handle that in
sparc_tr_tb_stop, to avoid replicating this pattern so many times.


r~

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [RFCv2 PATCH 1/1] SPARC64: add icount support
  2018-06-14 21:02   ` Richard Henderson
@ 2018-06-14 22:09     ` Mark Cave-Ayland
  2018-06-14 22:34       ` Richard Henderson
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Cave-Ayland @ 2018-06-14 22:09 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, atar4qemu, cota

On 14/06/18 22:02, Richard Henderson wrote:

> On 06/14/2018 12:53 AM, Mark Cave-Ayland wrote:
>> +                        if (dc->base.tb->cflags & CF_USE_ICOUNT) {
>> +                            gen_io_start();
>> +                        }
> 
> Need to use tb_cflags(dc->base.tb) for the atomic_read therein.
> 
> 
>> +                                    if (dc->base.tb->cflags & CF_USE_ICOUNT) {
>> +                                        gen_io_end();
>> +                                        /* End TB to handle timer interrupt */
>> +                                        save_state(dc);
>> +                                        gen_op_next_insn();
>> +                                        tcg_gen_exit_tb(NULL, 0);
>> +                                        dc->base.is_jmp = DISAS_NORETURN;
>> +                                    }
> 
> Exiting the TB should not be dependent on icount.
> 
> I would encourage you to introduce e.g. DISAS_EXIT, and code to handle that in
> sparc_tr_tb_stop, to avoid replicating this pattern so many times.

Okay thanks for the feedback. I've made the changes and will post a v3 
shortly.

I still see the "WARNING: Time of Day clock error" messages on the 
console when booting Milax with icount enabled, although I'm inclined to 
merge this if you're happy with the patch as it is a definite improvement.


ATB,

Mark.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [RFCv2 PATCH 1/1] SPARC64: add icount support
  2018-06-14 22:09     ` Mark Cave-Ayland
@ 2018-06-14 22:34       ` Richard Henderson
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2018-06-14 22:34 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel, atar4qemu, cota

On 06/14/2018 12:09 PM, Mark Cave-Ayland wrote:
> I still see the "WARNING: Time of Day clock error" messages on the console when
> booting Milax with icount enabled, although I'm inclined to merge this if
> you're happy with the patch as it is a definite improvement.

Yeah, let's fix whatever that is separately.


r~

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-06-14 22:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-14 10:53 [Qemu-devel] [RFCv2 PATCH 0/1] SPARC64: add icount support Mark Cave-Ayland
2018-06-14 10:53 ` [Qemu-devel] [RFCv2 PATCH 1/1] " Mark Cave-Ayland
2018-06-14 21:02   ` Richard Henderson
2018-06-14 22:09     ` Mark Cave-Ayland
2018-06-14 22:34       ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).