| Message ID | 894ec93e-05dd-2110-a0c2-e0afa2fa6fd5@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | linux-user/sparc64: Translate flushw opcode | expand |
Ping, I forgot to CC the SPARC mantainers in the previous mail. On 10/04/20 23:14, LemonBoy wrote: > From 11d0cfe58d12e0f191b435ade88622cfceb2098a Mon Sep 17 00:00:00 2001 > From: LemonBoy <thatlemon@gmail.com> > Date: Fri, 10 Apr 2020 22:55:26 +0200 > Subject: [PATCH] linux-user/sparc64: Translate flushw opcode > > The ifdef logic should unconditionally compile in the `xop == 0x2b` case > when targeting sparc64. > > Fix the handling of window spill traps by keeping cansave into account > when calculating the new CWP. > > Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com> > --- > bsd-user/main.c | 4 +++- > linux-user/sparc/cpu_loop.c | 4 +++- > target/sparc/translate.c | 2 ++ > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/bsd-user/main.c b/bsd-user/main.c > index 770c2b267a..d6b1c997e3 100644 > --- a/bsd-user/main.c > +++ b/bsd-user/main.c > @@ -413,7 +413,9 @@ static void save_window(CPUSPARCState *env) > save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > env->wim = new_wim; > #else > - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > + /* cansave is zero if the spill trap handler is triggered by `save` and */ > + /* nonzero if triggered by a `flushw` */ > + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > env->cansave++; > env->canrestore--; > #endif > diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c > index 7645cc04ca..20a7401126 100644 > --- a/linux-user/sparc/cpu_loop.c > +++ b/linux-user/sparc/cpu_loop.c > @@ -69,7 +69,9 @@ static void save_window(CPUSPARCState *env) > save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > env->wim = new_wim; > #else > - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > + /* cansave is zero if the spill trap handler is triggered by `save` and */ > + /* nonzero if triggered by a `flushw` */ > + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > env->cansave++; > env->canrestore--; > #endif > diff --git a/target/sparc/translate.c b/target/sparc/translate.c > index 9416a551cf..1a4efd4ed6 100644 > --- a/target/sparc/translate.c > +++ b/target/sparc/translate.c > @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) > #endif > gen_store_gpr(dc, rd, cpu_tmp0); > break; > +#endif > +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY) > } else if (xop == 0x2b) { /* rdtbr / V9 flushw */ > #ifdef TARGET_SPARC64 > gen_helper_flushw(cpu_env); >
Hi, could someone with SPARC knowledge review this patch? I'd like to add it to linux-user queue. Thanks, Laurent Le 17/04/2020 à 13:06, LemonBoy a écrit : > Ping, I forgot to CC the SPARC mantainers in the previous mail. > > On 10/04/20 23:14, LemonBoy wrote: >> From 11d0cfe58d12e0f191b435ade88622cfceb2098a Mon Sep 17 00:00:00 2001 >> From: LemonBoy <thatlemon@gmail.com> >> Date: Fri, 10 Apr 2020 22:55:26 +0200 >> Subject: [PATCH] linux-user/sparc64: Translate flushw opcode >> >> The ifdef logic should unconditionally compile in the `xop == 0x2b` case >> when targeting sparc64. >> >> Fix the handling of window spill traps by keeping cansave into account >> when calculating the new CWP. >> >> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com> >> --- >> bsd-user/main.c | 4 +++- >> linux-user/sparc/cpu_loop.c | 4 +++- >> target/sparc/translate.c | 2 ++ >> 3 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/bsd-user/main.c b/bsd-user/main.c >> index 770c2b267a..d6b1c997e3 100644 >> --- a/bsd-user/main.c >> +++ b/bsd-user/main.c >> @@ -413,7 +413,9 @@ static void save_window(CPUSPARCState *env) >> save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); >> env->wim = new_wim; >> #else >> - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); >> + /* cansave is zero if the spill trap handler is triggered by `save` and */ >> + /* nonzero if triggered by a `flushw` */ >> + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); >> env->cansave++; >> env->canrestore--; >> #endif >> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c >> index 7645cc04ca..20a7401126 100644 >> --- a/linux-user/sparc/cpu_loop.c >> +++ b/linux-user/sparc/cpu_loop.c >> @@ -69,7 +69,9 @@ static void save_window(CPUSPARCState *env) >> save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); >> env->wim = new_wim; >> #else >> - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); >> + /* cansave is zero if the spill trap handler is triggered by `save` and */ >> + /* nonzero if triggered by a `flushw` */ >> + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); >> env->cansave++; >> env->canrestore--; >> #endif >> diff --git a/target/sparc/translate.c b/target/sparc/translate.c >> index 9416a551cf..1a4efd4ed6 100644 >> --- a/target/sparc/translate.c >> +++ b/target/sparc/translate.c >> @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) >> #endif >> gen_store_gpr(dc, rd, cpu_tmp0); >> break; >> +#endif >> +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY) >> } else if (xop == 0x2b) { /* rdtbr / V9 flushw */ >> #ifdef TARGET_SPARC64 >> gen_helper_flushw(cpu_env); >> >
On Thu, May 7, 2020 at 4:17 PM Laurent Vivier <laurent@vivier.eu> wrote: > > Hi, > > could someone with SPARC knowledge review this patch? > I'd like to add it to linux-user queue. > Since it's TCG, I'd really preferred a review from Richard. Regards, Artyom > > Le 17/04/2020 à 13:06, LemonBoy a écrit : > > Ping, I forgot to CC the SPARC mantainers in the previous mail. > > > > On 10/04/20 23:14, LemonBoy wrote: > >> From 11d0cfe58d12e0f191b435ade88622cfceb2098a Mon Sep 17 00:00:00 2001 > >> From: LemonBoy <thatlemon@gmail.com> > >> Date: Fri, 10 Apr 2020 22:55:26 +0200 > >> Subject: [PATCH] linux-user/sparc64: Translate flushw opcode > >> > >> The ifdef logic should unconditionally compile in the `xop == 0x2b` case > >> when targeting sparc64. > >> > >> Fix the handling of window spill traps by keeping cansave into account > >> when calculating the new CWP. > >> > >> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com> > >> --- > >> bsd-user/main.c | 4 +++- > >> linux-user/sparc/cpu_loop.c | 4 +++- > >> target/sparc/translate.c | 2 ++ > >> 3 files changed, 8 insertions(+), 2 deletions(-) > >> > >> diff --git a/bsd-user/main.c b/bsd-user/main.c > >> index 770c2b267a..d6b1c997e3 100644 > >> --- a/bsd-user/main.c > >> +++ b/bsd-user/main.c > >> @@ -413,7 +413,9 @@ static void save_window(CPUSPARCState *env) > >> save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > >> env->wim = new_wim; > >> #else > >> - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > >> + /* cansave is zero if the spill trap handler is triggered by `save` and */ > >> + /* nonzero if triggered by a `flushw` */ > >> + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > >> env->cansave++; > >> env->canrestore--; > >> #endif > >> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c > >> index 7645cc04ca..20a7401126 100644 > >> --- a/linux-user/sparc/cpu_loop.c > >> +++ b/linux-user/sparc/cpu_loop.c > >> @@ -69,7 +69,9 @@ static void save_window(CPUSPARCState *env) > >> save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > >> env->wim = new_wim; > >> #else > >> - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > >> + /* cansave is zero if the spill trap handler is triggered by `save` and */ > >> + /* nonzero if triggered by a `flushw` */ > >> + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > >> env->cansave++; > >> env->canrestore--; > >> #endif > >> diff --git a/target/sparc/translate.c b/target/sparc/translate.c > >> index 9416a551cf..1a4efd4ed6 100644 > >> --- a/target/sparc/translate.c > >> +++ b/target/sparc/translate.c > >> @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) > >> #endif > >> gen_store_gpr(dc, rd, cpu_tmp0); > >> break; > >> +#endif > >> +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY) > >> } else if (xop == 0x2b) { /* rdtbr / V9 flushw */ > >> #ifdef TARGET_SPARC64 > >> gen_helper_flushw(cpu_env); > >> > > >
On 4/10/20 2:14 PM, LemonBoy wrote: > From 11d0cfe58d12e0f191b435ade88622cfceb2098a Mon Sep 17 00:00:00 2001 > From: LemonBoy <thatlemon@gmail.com> > Date: Fri, 10 Apr 2020 22:55:26 +0200 > Subject: [PATCH] linux-user/sparc64: Translate flushw opcode > > The ifdef logic should unconditionally compile in the `xop == 0x2b` case > when targeting sparc64. > > Fix the handling of window spill traps by keeping cansave into account > when calculating the new CWP. This is two separate fixes and should be two separate patches. > bsd-user/main.c | 4 +++- > linux-user/sparc/cpu_loop.c | 4 +++- First patch, > target/sparc/translate.c | 2 ++ > 3 files changed, 8 insertions(+), 2 deletions(-) Second patch. > + /* cansave is zero if the spill trap handler is triggered by `save` and */ > + /* nonzero if triggered by a `flushw` */ Format the comment like so: /* * cansave is zero... * nonzero if ... */ > + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); But this does match the implementation in sparc_cpu_do_interrupt, so the change is good. > diff --git a/target/sparc/translate.c b/target/sparc/translate.c > index 9416a551cf..1a4efd4ed6 100644 > --- a/target/sparc/translate.c > +++ b/target/sparc/translate.c > @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) > #endif > gen_store_gpr(dc, rd, cpu_tmp0); > break; > +#endif > +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY) > } else if (xop == 0x2b) { /* rdtbr / V9 flushw */ > #ifdef TARGET_SPARC64 > gen_helper_flushw(cpu_env); Look ok here too. r~
LemonBoy, could you resend a series of two patches modified as said by Richard? Changes should be only cosmetic. Thanks, Laurent Le 10/04/2020 à 23:14, LemonBoy a écrit : > From 11d0cfe58d12e0f191b435ade88622cfceb2098a Mon Sep 17 00:00:00 2001 > From: LemonBoy <thatlemon@gmail.com> > Date: Fri, 10 Apr 2020 22:55:26 +0200 > Subject: [PATCH] linux-user/sparc64: Translate flushw opcode > > The ifdef logic should unconditionally compile in the `xop == 0x2b` case > when targeting sparc64. > > Fix the handling of window spill traps by keeping cansave into account > when calculating the new CWP. > > Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com> > --- > bsd-user/main.c | 4 +++- > linux-user/sparc/cpu_loop.c | 4 +++- > target/sparc/translate.c | 2 ++ > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/bsd-user/main.c b/bsd-user/main.c > index 770c2b267a..d6b1c997e3 100644 > --- a/bsd-user/main.c > +++ b/bsd-user/main.c > @@ -413,7 +413,9 @@ static void save_window(CPUSPARCState *env) > save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > env->wim = new_wim; > #else > - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > + /* cansave is zero if the spill trap handler is triggered by `save` and */ > + /* nonzero if triggered by a `flushw` */ > + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > env->cansave++; > env->canrestore--; > #endif > diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c > index 7645cc04ca..20a7401126 100644 > --- a/linux-user/sparc/cpu_loop.c > +++ b/linux-user/sparc/cpu_loop.c > @@ -69,7 +69,9 @@ static void save_window(CPUSPARCState *env) > save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > env->wim = new_wim; > #else > - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > + /* cansave is zero if the spill trap handler is triggered by `save` and */ > + /* nonzero if triggered by a `flushw` */ > + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > env->cansave++; > env->canrestore--; > #endif > diff --git a/target/sparc/translate.c b/target/sparc/translate.c > index 9416a551cf..1a4efd4ed6 100644 > --- a/target/sparc/translate.c > +++ b/target/sparc/translate.c > @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) > #endif > gen_store_gpr(dc, rd, cpu_tmp0); > break; > +#endif > +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY) > } else if (xop == 0x2b) { /* rdtbr / V9 flushw */ > #ifdef TARGET_SPARC64 > gen_helper_flushw(cpu_env); >
Hello Giuseppe, On 4/10/20 11:14 PM, LemonBoy wrote: > From 11d0cfe58d12e0f191b435ade88622cfceb2098a Mon Sep 17 00:00:00 2001 > From: LemonBoy <thatlemon@gmail.com> > Date: Fri, 10 Apr 2020 22:55:26 +0200 > Subject: [PATCH] linux-user/sparc64: Translate flushw opcode > > The ifdef logic should unconditionally compile in the `xop == 0x2b` case > when targeting sparc64. > > Fix the handling of window spill traps by keeping cansave into account > when calculating the new CWP. Thanks for the fix. Do you mind addressing the comment Richard suggested? Thanks! Phil. > > Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com> > --- > bsd-user/main.c | 4 +++- > linux-user/sparc/cpu_loop.c | 4 +++- > target/sparc/translate.c | 2 ++ > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/bsd-user/main.c b/bsd-user/main.c > index 770c2b267a..d6b1c997e3 100644 > --- a/bsd-user/main.c > +++ b/bsd-user/main.c > @@ -413,7 +413,9 @@ static void save_window(CPUSPARCState *env) > save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > env->wim = new_wim; > #else > - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > + /* cansave is zero if the spill trap handler is triggered by `save` and */ > + /* nonzero if triggered by a `flushw` */ > + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > env->cansave++; > env->canrestore--; > #endif > diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c > index 7645cc04ca..20a7401126 100644 > --- a/linux-user/sparc/cpu_loop.c > +++ b/linux-user/sparc/cpu_loop.c > @@ -69,7 +69,9 @@ static void save_window(CPUSPARCState *env) > save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > env->wim = new_wim; > #else > - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); > + /* cansave is zero if the spill trap handler is triggered by `save` and */ > + /* nonzero if triggered by a `flushw` */ > + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); > env->cansave++; > env->canrestore--; > #endif > diff --git a/target/sparc/translate.c b/target/sparc/translate.c > index 9416a551cf..1a4efd4ed6 100644 > --- a/target/sparc/translate.c > +++ b/target/sparc/translate.c > @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) > #endif > gen_store_gpr(dc, rd, cpu_tmp0); > break; > +#endif > +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY) > } else if (xop == 0x2b) { /* rdtbr / V9 flushw */ > #ifdef TARGET_SPARC64 > gen_helper_flushw(cpu_env); >
diff --git a/bsd-user/main.c b/bsd-user/main.c index 770c2b267a..d6b1c997e3 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -413,7 +413,9 @@ static void save_window(CPUSPARCState *env) save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); env->wim = new_wim; #else - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); + /* cansave is zero if the spill trap handler is triggered by `save` and */ + /* nonzero if triggered by a `flushw` */ + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); env->cansave++; env->canrestore--; #endif diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c index 7645cc04ca..20a7401126 100644 --- a/linux-user/sparc/cpu_loop.c +++ b/linux-user/sparc/cpu_loop.c @@ -69,7 +69,9 @@ static void save_window(CPUSPARCState *env) save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); env->wim = new_wim; #else - save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2)); + /* cansave is zero if the spill trap handler is triggered by `save` and */ + /* nonzero if triggered by a `flushw` */ + save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2)); env->cansave++; env->canrestore--; #endif diff --git a/target/sparc/translate.c b/target/sparc/translate.c index 9416a551cf..1a4efd4ed6 100644 --- a/target/sparc/translate.c +++ b/target/sparc/translate.c @@ -3663,6 +3663,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) #endif gen_store_gpr(dc, rd, cpu_tmp0); break; +#endif +#if defined(TARGET_SPARC64) || !defined(CONFIG_USER_ONLY) } else if (xop == 0x2b) { /* rdtbr / V9 flushw */ #ifdef TARGET_SPARC64 gen_helper_flushw(cpu_env);