diff mbox

[v1,3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

Message ID be99bef9-eeb2-6816-e0ca-979780a48a06@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini June 14, 2017, 5:08 p.m. UTC
On 14/06/2017 18:51, Richard Henderson wrote:
> On 06/14/2017 09:08 AM, Paolo Bonzini wrote:
>> I think this is a band-aid, and would rather fix the front-ends as in
>> Emilio's patch.  For Alpha my guess would be:
>>
>> diff --git a/target/alpha/translate.c b/target/alpha/translate.c
>> index 7c45ae360c..6e2ee3f958 100644
>> --- a/target/alpha/translate.c
>> +++ b/target/alpha/translate.c
>> @@ -1198,7 +1198,9 @@ static ExitStatus gen_call_pal(DisasContext
>> *ctx, int palcode)
>>               tcg_gen_andi_i64(tmp, ctx->ir[IR_A0], PS_INT_MASK);
>>                tcg_gen_st8_i64(tmp, cpu_env, offsetof(CPUAlphaState,
>> ps));
>>               tcg_temp_free(tmp);
>> -            break;
>> +
>> +            /* Reevaluate interrupts */
>> +            return EXIT_PC_STALE;
>>             case 0x36:
>>               /* RDPS */
> 
> Thanks!
> 
> You're right that adjusting SWPIPL along these lines does fix the
> problem for Alpha.  Given that Alpha would typically hang in arch_idle,
> I'd been focusing primarily on WTINT.

And MIPS:


The others seem okay.

Paolo

Comments

Richard Henderson June 14, 2017, 6:26 p.m. UTC | #1
On 06/14/2017 10:08 AM, Paolo Bonzini wrote:
> And MIPS:
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 559f8fed89..244f3cb9ab 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -13403,8 +13403,9 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
>                   save_cpu_state(ctx, 1);
>                   gen_helper_ei(t0, cpu_env);
>                   gen_store_gpr(t0, rs);
> -                /* Stop translation as we may have switched the execution mode */
> -                ctx->bstate = BS_STOP;
> +                /* BS_STOP isn't good enough here, reevaluate cpu_mips_hw_interrupts_enabled. */
> +                gen_save_pc(ctx->pc + 4);
> +                ctx->bstate = BS_EXCP;
>                   tcg_temp_free(t0);
>               }
>               break;
> 
> The others seem okay.

Thanks for this bit.  We also need to fix SSM for s390x.


r~
Alex Bennée June 14, 2017, 7:07 p.m. UTC | #2
Richard Henderson <rth@twiddle.net> writes:

> On 06/14/2017 10:08 AM, Paolo Bonzini wrote:
>> And MIPS:
>>
>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index 559f8fed89..244f3cb9ab 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -13403,8 +13403,9 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
>>                   save_cpu_state(ctx, 1);
>>                   gen_helper_ei(t0, cpu_env);
>>                   gen_store_gpr(t0, rs);
>> -                /* Stop translation as we may have switched the execution mode */
>> -                ctx->bstate = BS_STOP;
>> +                /* BS_STOP isn't good enough here, reevaluate cpu_mips_hw_interrupts_enabled. */
>> +                gen_save_pc(ctx->pc + 4);
>> +                ctx->bstate = BS_EXCP;
>>                   tcg_temp_free(t0);
>>               }
>>               break;
>>
>> The others seem okay.
>
> Thanks for this bit.  We also need to fix SSM for s390x.

If your rolling a series for all these can you also pick up Thomas
Huth's fix for --accel?

--
Alex Bennée
Richard Henderson June 14, 2017, 7:43 p.m. UTC | #3
On 06/14/2017 12:07 PM, Alex Bennée wrote:
> 
> Richard Henderson <rth@twiddle.net> writes:
> 
>> On 06/14/2017 10:08 AM, Paolo Bonzini wrote:
>>> And MIPS:
>>>
>>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>>> index 559f8fed89..244f3cb9ab 100644
>>> --- a/target/mips/translate.c
>>> +++ b/target/mips/translate.c
>>> @@ -13403,8 +13403,9 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
>>>                    save_cpu_state(ctx, 1);
>>>                    gen_helper_ei(t0, cpu_env);
>>>                    gen_store_gpr(t0, rs);
>>> -                /* Stop translation as we may have switched the execution mode */
>>> -                ctx->bstate = BS_STOP;
>>> +                /* BS_STOP isn't good enough here, reevaluate cpu_mips_hw_interrupts_enabled. */
>>> +                gen_save_pc(ctx->pc + 4);
>>> +                ctx->bstate = BS_EXCP;
>>>                    tcg_temp_free(t0);
>>>                }
>>>                break;
>>>
>>> The others seem okay.
>>
>> Thanks for this bit.  We also need to fix SSM for s390x.
> 
> If your rolling a series for all these can you also pick up Thomas
> Huth's fix for --accel?

Will do.


r~
Emilio Cota June 16, 2017, 8:01 p.m. UTC | #4
On Wed, Jun 14, 2017 at 12:43:00 -0700, Richard Henderson wrote:
> On 06/14/2017 12:07 PM, Alex Bennée wrote:
> >If your rolling a series for all these can you also pick up Thomas
> >Huth's fix for --accel?
> 
> Will do.

Just a heads up since I see the patch is in your tcg-next branch:

Paolo included this patch in a recent pull request ("Misc patches
for 2017-06-15"), although the pull has yet to happen.
  https://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg03682.html

		E.
diff mbox

Patch

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 559f8fed89..244f3cb9ab 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -13403,8 +13403,9 @@  static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
                 save_cpu_state(ctx, 1);
                 gen_helper_ei(t0, cpu_env);
                 gen_store_gpr(t0, rs);
-                /* Stop translation as we may have switched the execution mode */
-                ctx->bstate = BS_STOP;
+                /* BS_STOP isn't good enough here, reevaluate cpu_mips_hw_interrupts_enabled. */
+                gen_save_pc(ctx->pc + 4);
+                ctx->bstate = BS_EXCP;
                 tcg_temp_free(t0);
             }
             break;