1. Patchew
  2. QEMU
  3. [PATCH 0/4] target/m68k: MacOS supervisor/user mode switch fixes
  4. View patch

[PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR

Mark Cave-Ayland posted 4 patches 3 years, 4 months ago
[PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
Posted by Mark Cave-Ayland 3 years, 4 months ago 
Any write to SR can change the security state so always call gen_exit_tb() when
this occurs. In particular MacOS makes use of andiw/oriw in a few places to
handle the switch between user and supervisor mode.

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

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index be5561e1e9..892473d01f 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2373,6 +2373,7 @@ DISAS_INSN(arith_im)
         tcg_gen_or_i32(dest, src1, im);
         if (with_SR) {
             gen_set_sr(s, dest, opsize == OS_BYTE);
+            gen_exit_tb(s);
         } else {
             DEST_EA(env, insn, opsize, dest, &addr);
             gen_logic_cc(s, dest, opsize);
@@ -2382,6 +2383,7 @@ DISAS_INSN(arith_im)
         tcg_gen_and_i32(dest, src1, im);
         if (with_SR) {
             gen_set_sr(s, dest, opsize == OS_BYTE);
+            gen_exit_tb(s);
         } else {
             DEST_EA(env, insn, opsize, dest, &addr);
             gen_logic_cc(s, dest, opsize);
@@ -2405,6 +2407,7 @@ DISAS_INSN(arith_im)
         tcg_gen_xor_i32(dest, src1, im);
         if (with_SR) {
             gen_set_sr(s, dest, opsize == OS_BYTE);
+            gen_exit_tb(s);
         } else {
             DEST_EA(env, insn, opsize, dest, &addr);
             gen_logic_cc(s, dest, opsize);
@@ -4592,6 +4595,7 @@ DISAS_INSN(strldsr)
     }
     gen_push(s, gen_get_sr(s));
     gen_set_sr_im(s, ext, 0);
+    gen_exit_tb(s);
 }
 
 DISAS_INSN(move_from_sr)
-- 
2.30.2
Re: [PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
Posted by Laurent Vivier 3 years, 4 months ago 
Le 17/09/2022 à 13:25, Mark Cave-Ayland a écrit :
> Any write to SR can change the security state so always call gen_exit_tb() when
> this occurs. In particular MacOS makes use of andiw/oriw in a few places to
> handle the switch between user and supervisor mode.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/m68k/translate.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index be5561e1e9..892473d01f 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -2373,6 +2373,7 @@ DISAS_INSN(arith_im)
>           tcg_gen_or_i32(dest, src1, im);
>           if (with_SR) {
>               gen_set_sr(s, dest, opsize == OS_BYTE);
> +            gen_exit_tb(s);
>           } else {
>               DEST_EA(env, insn, opsize, dest, &addr);
>               gen_logic_cc(s, dest, opsize);
> @@ -2382,6 +2383,7 @@ DISAS_INSN(arith_im)
>           tcg_gen_and_i32(dest, src1, im);
>           if (with_SR) {
>               gen_set_sr(s, dest, opsize == OS_BYTE);
> +            gen_exit_tb(s);
>           } else {
>               DEST_EA(env, insn, opsize, dest, &addr);
>               gen_logic_cc(s, dest, opsize);
> @@ -2405,6 +2407,7 @@ DISAS_INSN(arith_im)
>           tcg_gen_xor_i32(dest, src1, im);
>           if (with_SR) {
>               gen_set_sr(s, dest, opsize == OS_BYTE);
> +            gen_exit_tb(s);
>           } else {
>               DEST_EA(env, insn, opsize, dest, &addr);
>               gen_logic_cc(s, dest, opsize);
> @@ -4592,6 +4595,7 @@ DISAS_INSN(strldsr)
>       }
>       gen_push(s, gen_get_sr(s));
>       gen_set_sr_im(s, ext, 0);
> +    gen_exit_tb(s);
>   }
>   
>   DISAS_INSN(move_from_sr)

Applied to my m68k-for-7.2 branch

Thanks,
Laurent
Re: [PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
Posted by Philippe Mathieu-Daudé via 3 years, 4 months ago 
On 17/9/22 13:25, Mark Cave-Ayland wrote:
> Any write to SR can change the security state so always call gen_exit_tb() when
> this occurs. In particular MacOS makes use of andiw/oriw in a few places to
> handle the switch between user and supervisor mode.

Shouldn't be safer to add the gen_exit_tb() call in gen_set_sr[_im]()?

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   target/m68k/translate.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index be5561e1e9..892473d01f 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -2373,6 +2373,7 @@ DISAS_INSN(arith_im)
>           tcg_gen_or_i32(dest, src1, im);
>           if (with_SR) {
>               gen_set_sr(s, dest, opsize == OS_BYTE);
> +            gen_exit_tb(s);
>           } else {
>               DEST_EA(env, insn, opsize, dest, &addr);
>               gen_logic_cc(s, dest, opsize);
> @@ -2382,6 +2383,7 @@ DISAS_INSN(arith_im)
>           tcg_gen_and_i32(dest, src1, im);
>           if (with_SR) {
>               gen_set_sr(s, dest, opsize == OS_BYTE);
> +            gen_exit_tb(s);
>           } else {
>               DEST_EA(env, insn, opsize, dest, &addr);
>               gen_logic_cc(s, dest, opsize);
> @@ -2405,6 +2407,7 @@ DISAS_INSN(arith_im)
>           tcg_gen_xor_i32(dest, src1, im);
>           if (with_SR) {
>               gen_set_sr(s, dest, opsize == OS_BYTE);
> +            gen_exit_tb(s);
>           } else {
>               DEST_EA(env, insn, opsize, dest, &addr);
>               gen_logic_cc(s, dest, opsize);
> @@ -4592,6 +4595,7 @@ DISAS_INSN(strldsr)
>       }
>       gen_push(s, gen_get_sr(s));
>       gen_set_sr_im(s, ext, 0);
> +    gen_exit_tb(s);
>   }
>   
>   DISAS_INSN(move_from_sr)
Re: [PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
Posted by Richard Henderson 3 years, 4 months ago 
On 9/18/22 00:29, Philippe Mathieu-Daudé wrote:
> On 17/9/22 13:25, Mark Cave-Ayland wrote:
>> Any write to SR can change the security state so always call gen_exit_tb() when
>> this occurs. In particular MacOS makes use of andiw/oriw in a few places to
>> handle the switch between user and supervisor mode.
> 
> Shouldn't be safer to add the gen_exit_tb() call in gen_set_sr[_im]()?

No.  For halt we need to raise EXCP_HLT.

r~

> 
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   target/m68k/translate.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
>> index be5561e1e9..892473d01f 100644
>> --- a/target/m68k/translate.c
>> +++ b/target/m68k/translate.c
>> @@ -2373,6 +2373,7 @@ DISAS_INSN(arith_im)
>>           tcg_gen_or_i32(dest, src1, im);
>>           if (with_SR) {
>>               gen_set_sr(s, dest, opsize == OS_BYTE);
>> +            gen_exit_tb(s);
>>           } else {
>>               DEST_EA(env, insn, opsize, dest, &addr);
>>               gen_logic_cc(s, dest, opsize);
>> @@ -2382,6 +2383,7 @@ DISAS_INSN(arith_im)
>>           tcg_gen_and_i32(dest, src1, im);
>>           if (with_SR) {
>>               gen_set_sr(s, dest, opsize == OS_BYTE);
>> +            gen_exit_tb(s);
>>           } else {
>>               DEST_EA(env, insn, opsize, dest, &addr);
>>               gen_logic_cc(s, dest, opsize);
>> @@ -2405,6 +2407,7 @@ DISAS_INSN(arith_im)
>>           tcg_gen_xor_i32(dest, src1, im);
>>           if (with_SR) {
>>               gen_set_sr(s, dest, opsize == OS_BYTE);
>> +            gen_exit_tb(s);
>>           } else {
>>               DEST_EA(env, insn, opsize, dest, &addr);
>>               gen_logic_cc(s, dest, opsize);
>> @@ -4592,6 +4595,7 @@ DISAS_INSN(strldsr)
>>       }
>>       gen_push(s, gen_get_sr(s));
>>       gen_set_sr_im(s, ext, 0);
>> +    gen_exit_tb(s);
>>   }
>>   DISAS_INSN(move_from_sr)
>
Re: [PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
Posted by Philippe Mathieu-Daudé via 3 years, 4 months ago 
On 19/9/22 10:13, Richard Henderson wrote:
> On 9/18/22 00:29, Philippe Mathieu-Daudé wrote:
>> On 17/9/22 13:25, Mark Cave-Ayland wrote:
>>> Any write to SR can change the security state so always call 
>>> gen_exit_tb() when
>>> this occurs. In particular MacOS makes use of andiw/oriw in a few 
>>> places to
>>> handle the switch between user and supervisor mode.
>>
>> Shouldn't be safer to add the gen_exit_tb() call in gen_set_sr[_im]()?
> 
> No.  For halt we need to raise EXCP_HLT.

Right, I should have looked at translate.c; I also noticed the ccr_only
flag. So:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Re: [PATCH 4/4] target/m68k: always call gen_exit_tb() after writes to SR
Posted by Richard Henderson 3 years, 4 months ago 
On 9/17/22 13:25, Mark Cave-Ayland wrote:
> Any write to SR can change the security state so always call gen_exit_tb() when
> this occurs. In particular MacOS makes use of andiw/oriw in a few places to
> handle the switch between user and supervisor mode.
> 
> Signed-off-by: Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>
> ---
>   target/m68k/translate.c | 4 ++++
>   1 file changed, 4 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.