Re: [PATCH 2/2] target/m68k: Perform writback before modifying SR

2022-09-21 Thread Laurent Vivier

Le 13/09/2022 à 16:28, Richard Henderson a écrit :

Writes to SR may change security state, which may involve
a swap of %ssp with %usp as reflected in %a7.  Finish the
writeback of %sp@+ before swapping stack pointers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1206
Signed-off-by: Richard Henderson 
---
  target/m68k/translate.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 87044382c3..8506da0a0b 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2285,9 +2285,9 @@ static void gen_set_sr_im(DisasContext *s, uint16_t val, 
int ccr_only)
  tcg_gen_movi_i32(QREG_CC_N, val & CCF_N ? -1 : 0);
  tcg_gen_movi_i32(QREG_CC_X, val & CCF_X ? 1 : 0);
  } else {
-TCGv sr = tcg_const_i32(val);
-gen_helper_set_sr(cpu_env, sr);
-tcg_temp_free(sr);
+/* Must writeback before changing security state. */
+do_writebacks(s);
+gen_helper_set_sr(cpu_env, tcg_constant_i32(val));
  }
  set_cc_op(s, CC_OP_FLAGS);
  }
@@ -2297,6 +2297,8 @@ static void gen_set_sr(DisasContext *s, TCGv val, int 
ccr_only)
  if (ccr_only) {
  gen_helper_set_ccr(cpu_env, val);
  } else {
+/* Must writeback before changing security state. */
+do_writebacks(s);
  gen_helper_set_sr(cpu_env, val);
  }
  set_cc_op(s, CC_OP_FLAGS);


Applied to my m68k-for-7.2 branch

Thanks,
Laurent





Re: [PATCH 2/2] target/m68k: Perform writback before modifying SR

2022-09-13 Thread Mark Cave-Ayland

On 13/09/2022 15:28, Richard Henderson wrote:


Writes to SR may change security state, which may involve
a swap of %ssp with %usp as reflected in %a7.  Finish the
writeback of %sp@+ before swapping stack pointers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1206
Signed-off-by: Richard Henderson 
---
  target/m68k/translate.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 87044382c3..8506da0a0b 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2285,9 +2285,9 @@ static void gen_set_sr_im(DisasContext *s, uint16_t val, 
int ccr_only)
  tcg_gen_movi_i32(QREG_CC_N, val & CCF_N ? -1 : 0);
  tcg_gen_movi_i32(QREG_CC_X, val & CCF_X ? 1 : 0);
  } else {
-TCGv sr = tcg_const_i32(val);
-gen_helper_set_sr(cpu_env, sr);
-tcg_temp_free(sr);
+/* Must writeback before changing security state. */
+do_writebacks(s);
+gen_helper_set_sr(cpu_env, tcg_constant_i32(val));
  }
  set_cc_op(s, CC_OP_FLAGS);
  }
@@ -2297,6 +2297,8 @@ static void gen_set_sr(DisasContext *s, TCGv val, int 
ccr_only)
  if (ccr_only) {
  gen_helper_set_ccr(cpu_env, val);
  } else {
+/* Must writeback before changing security state. */
+do_writebacks(s);
  gen_helper_set_sr(cpu_env, val);
  }
  set_cc_op(s, CC_OP_FLAGS);


Thanks Richard! Subject needs s/writback/writeback/ but anyhow:

Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.



Re: [PATCH 2/2] target/m68k: Perform writback before modifying SR

2022-09-13 Thread Laurent Vivier

Le 13/09/2022 à 16:28, Richard Henderson a écrit :

Writes to SR may change security state, which may involve
a swap of %ssp with %usp as reflected in %a7.  Finish the
writeback of %sp@+ before swapping stack pointers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1206
Signed-off-by: Richard Henderson 
---
  target/m68k/translate.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)



Reviewed-by: Laurent Vivier 




[PATCH 2/2] target/m68k: Perform writback before modifying SR

2022-09-13 Thread Richard Henderson
Writes to SR may change security state, which may involve
a swap of %ssp with %usp as reflected in %a7.  Finish the
writeback of %sp@+ before swapping stack pointers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1206
Signed-off-by: Richard Henderson 
---
 target/m68k/translate.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 87044382c3..8506da0a0b 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2285,9 +2285,9 @@ static void gen_set_sr_im(DisasContext *s, uint16_t val, 
int ccr_only)
 tcg_gen_movi_i32(QREG_CC_N, val & CCF_N ? -1 : 0);
 tcg_gen_movi_i32(QREG_CC_X, val & CCF_X ? 1 : 0);
 } else {
-TCGv sr = tcg_const_i32(val);
-gen_helper_set_sr(cpu_env, sr);
-tcg_temp_free(sr);
+/* Must writeback before changing security state. */
+do_writebacks(s);
+gen_helper_set_sr(cpu_env, tcg_constant_i32(val));
 }
 set_cc_op(s, CC_OP_FLAGS);
 }
@@ -2297,6 +2297,8 @@ static void gen_set_sr(DisasContext *s, TCGv val, int 
ccr_only)
 if (ccr_only) {
 gen_helper_set_ccr(cpu_env, val);
 } else {
+/* Must writeback before changing security state. */
+do_writebacks(s);
 gen_helper_set_sr(cpu_env, val);
 }
 set_cc_op(s, CC_OP_FLAGS);
-- 
2.34.1