* [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction
@ 2015-05-16 23:28 Aurelien Jarno
2015-05-18 13:18 ` Alexander Graf
2015-05-18 15:35 ` Richard Henderson
0 siblings, 2 replies; 5+ messages in thread
From: Aurelien Jarno @ 2015-05-16 23:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Graf, Aurelien Jarno, Richard Henderson
Commit 7a6c7067f optimized CC computation by only saving cc_op before
calling helpers as they either don't touch the CC or generate a new
static value. This however doesn't work for the EX instruction as the
helper changes or not the CC value depending on the actual executed
instruction (e.g. MVC vs CLC).
This patches force a CC computation before calling the helper. This
fixes random memory corruption occuring in guests.
Cc: Richard Henderson <rth@twiddle.net>
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
target-s390x/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 80e3a54..10522df 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2095,7 +2095,7 @@ static ExitStatus op_ex(DisasContext *s, DisasOps *o)
TCGv_i64 tmp;
update_psw_addr(s);
- update_cc_op(s);
+ gen_op_calc_cc(s);
tmp = tcg_const_i64(s->next_pc);
gen_helper_ex(cc_op, cpu_env, cc_op, o->in1, o->in2, tmp);
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction
2015-05-16 23:28 [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction Aurelien Jarno
@ 2015-05-18 13:18 ` Alexander Graf
2015-05-18 15:35 ` Richard Henderson
1 sibling, 0 replies; 5+ messages in thread
From: Alexander Graf @ 2015-05-18 13:18 UTC (permalink / raw)
To: Aurelien Jarno, qemu-devel; +Cc: Richard Henderson
On 05/17/2015 01:28 AM, Aurelien Jarno wrote:
> Commit 7a6c7067f optimized CC computation by only saving cc_op before
> calling helpers as they either don't touch the CC or generate a new
> static value. This however doesn't work for the EX instruction as the
> helper changes or not the CC value depending on the actual executed
> instruction (e.g. MVC vs CLC).
>
> This patches force a CC computation before calling the helper. This
> fixes random memory corruption occuring in guests.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Sounds plausible to me, though I'm surprised I didn't run into this
myself yet.
Richard?
Alex
> ---
> target-s390x/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
> index 80e3a54..10522df 100644
> --- a/target-s390x/translate.c
> +++ b/target-s390x/translate.c
> @@ -2095,7 +2095,7 @@ static ExitStatus op_ex(DisasContext *s, DisasOps *o)
> TCGv_i64 tmp;
>
> update_psw_addr(s);
> - update_cc_op(s);
> + gen_op_calc_cc(s);
>
> tmp = tcg_const_i64(s->next_pc);
> gen_helper_ex(cc_op, cpu_env, cc_op, o->in1, o->in2, tmp);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction
2015-05-16 23:28 [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction Aurelien Jarno
2015-05-18 13:18 ` Alexander Graf
@ 2015-05-18 15:35 ` Richard Henderson
2015-05-18 17:07 ` Alexander Graf
1 sibling, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2015-05-18 15:35 UTC (permalink / raw)
To: Aurelien Jarno, qemu-devel; +Cc: Alexander Graf
On 05/16/2015 04:28 PM, Aurelien Jarno wrote:
> Commit 7a6c7067f optimized CC computation by only saving cc_op before
> calling helpers as they either don't touch the CC or generate a new
> static value. This however doesn't work for the EX instruction as the
> helper changes or not the CC value depending on the actual executed
> instruction (e.g. MVC vs CLC).
>
> This patches force a CC computation before calling the helper. This
> fixes random memory corruption occuring in guests.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-s390x/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
> index 80e3a54..10522df 100644
> --- a/target-s390x/translate.c
> +++ b/target-s390x/translate.c
> @@ -2095,7 +2095,7 @@ static ExitStatus op_ex(DisasContext *s, DisasOps *o)
> TCGv_i64 tmp;
>
> update_psw_addr(s);
> - update_cc_op(s);
> + gen_op_calc_cc(s);
>
> tmp = tcg_const_i64(s->next_pc);
> gen_helper_ex(cc_op, cpu_env, cc_op, o->in1, o->in2, tmp);
I agree this is a bug, and the right fix.
You can also remove the set_cc_static at the end of op_ex, since that's done by
gen_op_calc_cc.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction
2015-05-18 15:35 ` Richard Henderson
@ 2015-05-18 17:07 ` Alexander Graf
2015-05-18 17:32 ` Aurelien Jarno
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Graf @ 2015-05-18 17:07 UTC (permalink / raw)
To: Richard Henderson, Aurelien Jarno, qemu-devel
On 05/18/2015 05:35 PM, Richard Henderson wrote:
> On 05/16/2015 04:28 PM, Aurelien Jarno wrote:
>> Commit 7a6c7067f optimized CC computation by only saving cc_op before
>> calling helpers as they either don't touch the CC or generate a new
>> static value. This however doesn't work for the EX instruction as the
>> helper changes or not the CC value depending on the actual executed
>> instruction (e.g. MVC vs CLC).
>>
>> This patches force a CC computation before calling the helper. This
>> fixes random memory corruption occuring in guests.
>>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Cc: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>> target-s390x/translate.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
>> index 80e3a54..10522df 100644
>> --- a/target-s390x/translate.c
>> +++ b/target-s390x/translate.c
>> @@ -2095,7 +2095,7 @@ static ExitStatus op_ex(DisasContext *s, DisasOps *o)
>> TCGv_i64 tmp;
>>
>> update_psw_addr(s);
>> - update_cc_op(s);
>> + gen_op_calc_cc(s);
>>
>> tmp = tcg_const_i64(s->next_pc);
>> gen_helper_ex(cc_op, cpu_env, cc_op, o->in1, o->in2, tmp);
> I agree this is a bug, and the right fix.
>
> You can also remove the set_cc_static at the end of op_ex, since that's done by
> gen_op_calc_cc.
Thanks, I applied the patch and did the change to remove set_cc_static
from op_ex locally.
Alex
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction
2015-05-18 17:07 ` Alexander Graf
@ 2015-05-18 17:32 ` Aurelien Jarno
0 siblings, 0 replies; 5+ messages in thread
From: Aurelien Jarno @ 2015-05-18 17:32 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel, Richard Henderson
On 2015-05-18 19:07, Alexander Graf wrote:
> On 05/18/2015 05:35 PM, Richard Henderson wrote:
> >On 05/16/2015 04:28 PM, Aurelien Jarno wrote:
> >>Commit 7a6c7067f optimized CC computation by only saving cc_op before
> >>calling helpers as they either don't touch the CC or generate a new
> >>static value. This however doesn't work for the EX instruction as the
> >>helper changes or not the CC value depending on the actual executed
> >>instruction (e.g. MVC vs CLC).
> >>
> >>This patches force a CC computation before calling the helper. This
> >>fixes random memory corruption occuring in guests.
> >>
> >>Cc: Richard Henderson <rth@twiddle.net>
> >>Cc: Alexander Graf <agraf@suse.de>
> >>Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>---
> >> target-s390x/translate.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/target-s390x/translate.c b/target-s390x/translate.c
> >>index 80e3a54..10522df 100644
> >>--- a/target-s390x/translate.c
> >>+++ b/target-s390x/translate.c
> >>@@ -2095,7 +2095,7 @@ static ExitStatus op_ex(DisasContext *s, DisasOps *o)
> >> TCGv_i64 tmp;
> >> update_psw_addr(s);
> >>- update_cc_op(s);
> >>+ gen_op_calc_cc(s);
> >> tmp = tcg_const_i64(s->next_pc);
> >> gen_helper_ex(cc_op, cpu_env, cc_op, o->in1, o->in2, tmp);
> >I agree this is a bug, and the right fix.
> >
> >You can also remove the set_cc_static at the end of op_ex, since that's done by
> >gen_op_calc_cc.
>
> Thanks, I applied the patch and did the change to remove set_cc_static from
> op_ex locally.
Thanks!
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-18 17:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-16 23:28 [Qemu-devel] [PATCH] target-s390x: fix CC computation for EX instruction Aurelien Jarno
2015-05-18 13:18 ` Alexander Graf
2015-05-18 15:35 ` Richard Henderson
2015-05-18 17:07 ` Alexander Graf
2015-05-18 17:32 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).