[PATCH] target/sparc: Clear may_lookup for npc == DYNAMIC_PC

Richard Henderson posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231015232454.391788-1-richard.henderson@linaro.org
Maintainers: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>
target/sparc/translate.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
[PATCH] target/sparc: Clear may_lookup for npc == DYNAMIC_PC
Posted by Richard Henderson 1 year, 1 month ago
With pairs of jmp+rett, pc == DYNAMIC_PC_LOOKUP and
npc == DYNAMIC_PC.  Make sure that we exit for interrupts.

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

Mark, I wonder if this will cure some of your lost interrupt issues.
Spotted while looking at issues from the JMPL+RETT+RETURN patch.

r~
---
 target/sparc/translate.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index f92ff80ac8..8fabed28fd 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -5654,10 +5654,10 @@ static void sparc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
             break;
         }
 
+        may_lookup = true;
         if (dc->pc & 3) {
             switch (dc->pc) {
             case DYNAMIC_PC_LOOKUP:
-                may_lookup = true;
                 break;
             case DYNAMIC_PC:
                 may_lookup = false;
@@ -5667,10 +5667,24 @@ static void sparc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
             }
         } else {
             tcg_gen_movi_tl(cpu_pc, dc->pc);
-            may_lookup = true;
         }
 
-        save_npc(dc);
+        if (dc->npc & 3) {
+            switch (dc->npc) {
+            case JUMP_PC:
+                gen_generic_branch(dc);
+                break;
+            case DYNAMIC_PC:
+                may_lookup = false;
+                break;
+            case DYNAMIC_PC_LOOKUP:
+                break;
+            default:
+                g_assert_not_reached();
+            }
+        } else {
+            tcg_gen_movi_tl(cpu_npc, dc->npc);
+        }
         if (may_lookup) {
             tcg_gen_lookup_and_goto_ptr();
         } else {
-- 
2.34.1
Re: [PATCH] target/sparc: Clear may_lookup for npc == DYNAMIC_PC
Posted by Mark Cave-Ayland 1 year, 1 month ago
On 16/10/2023 00:24, Richard Henderson wrote:

> With pairs of jmp+rett, pc == DYNAMIC_PC_LOOKUP and
> npc == DYNAMIC_PC.  Make sure that we exit for interrupts.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Mark, I wonder if this will cure some of your lost interrupt issues.
> Spotted while looking at issues from the JMPL+RETT+RETURN patch.
> 
> r~

Yes, I believe it does! I've just tried this on my Solaris 8 image and I no longer 
see any lockups when wiggling the mouse :)  Presumably this needs a CC: qemu-stable 
and a Fixes: tag along with:

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

> ---
>   target/sparc/translate.c | 20 +++++++++++++++++---
>   1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index f92ff80ac8..8fabed28fd 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -5654,10 +5654,10 @@ static void sparc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
>               break;
>           }
>   
> +        may_lookup = true;
>           if (dc->pc & 3) {
>               switch (dc->pc) {
>               case DYNAMIC_PC_LOOKUP:
> -                may_lookup = true;
>                   break;
>               case DYNAMIC_PC:
>                   may_lookup = false;
> @@ -5667,10 +5667,24 @@ static void sparc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
>               }
>           } else {
>               tcg_gen_movi_tl(cpu_pc, dc->pc);
> -            may_lookup = true;
>           }
>   
> -        save_npc(dc);
> +        if (dc->npc & 3) {
> +            switch (dc->npc) {
> +            case JUMP_PC:
> +                gen_generic_branch(dc);
> +                break;
> +            case DYNAMIC_PC:
> +                may_lookup = false;
> +                break;
> +            case DYNAMIC_PC_LOOKUP:
> +                break;
> +            default:
> +                g_assert_not_reached();
> +            }
> +        } else {
> +            tcg_gen_movi_tl(cpu_npc, dc->npc);
> +        }
>           if (may_lookup) {
>               tcg_gen_lookup_and_goto_ptr();
>           } else {


ATB,

Mark.