Re: [Qemu-devel] [PATCH] target/alpha: fix tlb_fill trap_arg2 value for instruction fetch

[Richard Henderson]

On 8/21/19 6:52 AM, Peter Maydell wrote:
> On Wed, 21 Aug 2019 at 14:42, Aurelien Jarno <address@hidden> wrote:
>>
>> Commit e41c94529740cc26 ("target/alpha: Convert to CPUClass::tlb_fill")
>> slightly changed the way the trap_arg2 value is computed in case of TLB
>> fill. The type of the variable used in the ternary operator has been
>> changed from an int to an enum. This causes the -1 value to not be
>> sign-extended to 64-bit in case of an instruction fetch. The trap_arg2
>> ends up with 0xffffffff instead of 0xffffffffffffffff. Fix that by
>> changing the -1 into -1LL.
>>
>> This fixes the execution of user space processes in qemu-system-alpha.
>>
>> Fixes: e41c94529740cc26
>> Cc: address@hidden
>> Signed-off-by: Aurelien Jarno <address@hidden>
>> ---
>>  target/alpha/helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
>> index 93b8e788b1..9e9d880c1a 100644
>> --- a/target/alpha/helper.c
>> +++ b/target/alpha/helper.c
>> @@ -283,7 +283,7 @@ bool alpha_cpu_tlb_fill(CPUState *cs, vaddr addr, int 
>> size,
>>          cs->exception_index = EXCP_MMFAULT;
>>          env->trap_arg0 = addr;
>>          env->trap_arg1 = fail;
>> -        env->trap_arg2 = (access_type == MMU_INST_FETCH ? -1 : access_type);
>> +        env->trap_arg2 = (access_type == MMU_INST_FETCH ? -1LL : 
>> access_type);
>>          cpu_loop_exit_restore(cs, retaddr);
>>      }
> 
> Oops. Thanks for the catch.
> 
> Maybe we should not rely directly on the value of the access_type
> enum to set trap_arg2 at all (ie just go for a switch on access_type and
> set env->trap_arg2 to the right h/w value in the three cases)?

Yes, I'll do that.  I'm somewhat embarrassed that I haven't tested Alpha in a
while, and moreso because we just did a release.


r~