Re: [PULL 02/41] target/ppc: 603: fix restore of GPRs 0-3 on rfi

["Cédric Le Goater" <clg@kaod.org>]

On 1/31/22 13:01, Mark Cave-Ayland wrote:
> On 31/01/2022 11:07, Cédric Le Goater wrote:
> 
>> From: Christophe Leroy <christophe.leroy@csgroup.eu>
>>
>> After a TLB miss exception, GPRs 0-3 must be restored on rfi.
>>
>> This is managed by hreg_store_msr() which is called by do_rfi()
>>
>> However, hreg_store_msr() does it if MSR[TGPR] is unset in the
>> passed MSR value.
>>
>> The problem is that do_rfi() is given the content of SRR1 as
>> the value to be set in MSR, but TGPR bit is not part of SRR1
>> and that bit is used for something else and is sometimes set
>> to 1, leading to hreg_store_msr() not restoring GPRs.
>>
>> So, do the same way as for POW bit, force clearing it.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Cedric Le Goater <clg@kaod.org>
>> Cc: Fabiano Rosas <farosas@linux.ibm.com>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>> Message-Id: <20220120103824.239573-1-christophe.leroy@csgroup.eu>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   target/ppc/excp_helper.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index bc646c67a0f5..980f62fd7964 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1164,6 +1164,10 @@ static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
>>       /* MSR:POW cannot be set by any form of rfi */
>>       msr &= ~(1ULL << MSR_POW);
>> +    /* MSR:TGPR cannot be set by any form of rfi */
>> +    if (env->flags & POWERPC_FLAG_TGPR)
>> +        msr &= ~(1ULL << MSR_TGPR);
>> +
>>   #if defined(TARGET_PPC64)
>>       /* Switching to 32-bit ? Crop the nip */
>>       if (!msr_is_64bit(env, msr)) {
> 
> Have you tried a pre-PR push to Gitlab CI for your pull-ppc-20220130 tag? I'd expect this to fail the check-patch job due to the missing braces around the if() statement.

All is fine :

   https://gitlab.com/legoater/qemu/-/pipelines/458888936

I even did a checkpatch before sending and it did not complain :/

Thanks,

C.