[PATCH 26/29] target/openrisc: Always exit after mtspr npc

Richard Henderson posted 29 patches 3 years, 3 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Taylor Simpson <tsimpson@quicinc.com>, Eduardo Habkost <eduardo@habkost.net>, Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Laurent Vivier <laurent@vivier.eu>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
There is a newer version of this series
[PATCH 26/29] target/openrisc: Always exit after mtspr npc
Posted by Richard Henderson 3 years, 3 months ago
We have called cpu_restore_state asserting will_exit.
Do not go back on that promise.  This affects icount.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/openrisc/sys_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 09b3c97d7c..a3508e421d 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -51,8 +51,8 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)
         if (env->pc != rb) {
             env->pc = rb;
             env->dflag = 0;
-            cpu_loop_exit(cs);
         }
+        cpu_loop_exit(cs);
         break;
 
     case TO_SPR(0, 17): /* SR */
-- 
2.34.1
Re: [PATCH 26/29] target/openrisc: Always exit after mtspr npc
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
On 24/10/22 15:24, Richard Henderson wrote:
> We have called cpu_restore_state asserting will_exit.
> Do not go back on that promise.  This affects icount.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/openrisc/sys_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
> index 09b3c97d7c..a3508e421d 100644
> --- a/target/openrisc/sys_helper.c
> +++ b/target/openrisc/sys_helper.c
> @@ -51,8 +51,8 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)

Extending the diff context from commit 24c328521b ("Tidy ppc/npc 
implementation"):

             cpu_restore_state(cs, GETPC(), true);
             /* ??? Mirror or1ksim in not trashing delayed branch state
               when "jumping" to the current instruction.  */

>           if (env->pc != rb) {
>               env->pc = rb;
>               env->dflag = 0;
> -            cpu_loop_exit(cs);
>           }
> +        cpu_loop_exit(cs);

Is the comment still relevant?

>           break;
>   
>       case TO_SPR(0, 17): /* SR */
Re: [PATCH 26/29] target/openrisc: Always exit after mtspr npc
Posted by Richard Henderson 3 years, 3 months ago
On 10/26/22 01:39, Philippe Mathieu-Daudé wrote:
> On 24/10/22 15:24, Richard Henderson wrote:
>> We have called cpu_restore_state asserting will_exit.
>> Do not go back on that promise.  This affects icount.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/openrisc/sys_helper.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
>> index 09b3c97d7c..a3508e421d 100644
>> --- a/target/openrisc/sys_helper.c
>> +++ b/target/openrisc/sys_helper.c
>> @@ -51,8 +51,8 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, 
>> target_ulong rb)
> 
> Extending the diff context from commit 24c328521b ("Tidy ppc/npc implementation"):
> 
>              cpu_restore_state(cs, GETPC(), true);
>              /* ??? Mirror or1ksim in not trashing delayed branch state
>                when "jumping" to the current instruction.  */
> 
>>           if (env->pc != rb) {
>>               env->pc = rb;
>>               env->dflag = 0;
>> -            cpu_loop_exit(cs);
>>           }
>> +        cpu_loop_exit(cs);
> 
> Is the comment still relevant?

Yes, in that when pc == rb we don't clear dflag.
The only change here is exiting to the main loop before starting the next insn.

r~