Re: [Qemu-devel] [PATCH2] ppc64: fix mtmsr behavior on 64-bit targets

[Nathan Whitehorn <nwhitehorn@freebsd.org>]

On 06/13/11 05:20, Alexander Graf wrote:
>
>
>
> Am 12.06.2011 um 17:49 schrieb Nathan Whitehorn<nwhitehorn@freebsd.org>:
>
>> The mtmsr instruction is required not to modify the upper 32-bits of the machine state register, but checks the current value of MSR[SF] to decide whether to do this. This has the effect of zeroing the upper 32 bits of the MSR whenever mtmsr is executed in 64-bit mode. Unconditionally preserve the upper 32-bits in mtmsr for TARGET_PPC64.
>>
>> Signed-off-by: Nathan Whitehorn<nwhitehorn@freebsd.org>
>> ---
>> target-ppc/translate.c |    5 ++---
>> 1 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 59aef85..38d2e2e 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -3884,18 +3884,17 @@ static void gen_mtmsr(DisasContext *ctx)
>>           */
>>          gen_update_nip(ctx, ctx->nip);
>> #if defined(TARGET_PPC64)
>> -        if (!ctx->sf_mode) {
>> -            TCGv t0 = tcg_temp_new();
>> -            TCGv t1 = tcg_temp_new();
>> -            tcg_gen_andi_tl(t0, cpu_msr, 0xFFFFFFFF00000000ULL);
>> -            tcg_gen_ext32u_tl(t1, cpu_gpr[rS(ctx->opcode)]);
>> -            tcg_gen_or_tl(t0, t0, t1);
>> -            tcg_temp_free(t1);
>> -            gen_helper_store_msr(t0);
>> -            tcg_temp_free(t0);
>> -        } else
>> +        TCGv t0 = tcg_temp_new();
>> +        TCGv t1 = tcg_temp_new();
> You're declaring variables in mid-scope. Please open a new scope :).

Does the gen_update_nip(ctx, ctx->nip); need to be first here? If not, 
we can just move it to the end and avoid the scoping issue.
>> +        tcg_gen_andi_tl(t0, cpu_msr, 0xFFFFFFFF00000000ULL);
>> +        tcg_gen_ext32u_tl(t1, cpu_gpr[rS(ctx->opcode)]);
>> +        tcg_gen_or_tl(t0, t0, t1);
> While at it, this is a perfect scenario for the deposit tcg op! :)
>
> If you feel like this is too cumbersome work for such a small patch, please let me know and I'll do the changes for you :)

I have no idea what that is, so I'd prefer you do it :)
-Nathan