Re: [PATCH] target/hppa: Allow, but diagnose, LDCW aligned only mod 4

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/17/20 4:49 PM, Helge Deller wrote:
> On 17.01.20 02:53, Richard Henderson wrote:
>> The PA-RISC 1.1 specification says that LDCW must be aligned mod 16
>> or the operation is undefined.  However, real hardware only generates
>> an unaligned access trap for unaligned mod 4.
>>
>> Match real hardware, but diagnose with GUEST_ERROR a violation of the
>> specification.
>>
>> Reported-by: Helge Deller <deller@gmx.de>
>> Suggested-by: John David Anglin <dave.anglin@bell.net>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> Helge, can you please test this against your failing kernel?
>> You will of course want to add -D logfile -d guest_errors to
>> you qemu command-line.
> 
> Yes, works as expected.
> Thanks!
> 
> Please add:
> Tested-by: Helge Deller <deller@gmx.de>
> 
> 
> [deller]$ tail -f logfile
> Undefined ldc to address unaligned mod 16: 00000504fa6c7848
> Undefined ldc to address unaligned mod 16: 00000504fa6c7a48
> Undefined ldc to address unaligned mod 16: 00000506f9434848
> Undefined ldc to address unaligned mod 16: 00000506f9434a48
> Undefined ldc to address unaligned mod 16: 00000508fa036848
> Undefined ldc to address unaligned mod 16: 00000508fa036a48
> Undefined ldc to address unaligned mod 16: 0000050afa8c4848
> Undefined ldc to address unaligned mod 16: 0000050afa8c4a48
> Undefined ldc to address unaligned mod 16: 0000050cf94d1848
> Undefined ldc to address unaligned mod 16: 0000050cf94d1a48
> ....
> 
> 
> 
>>
>>
>> r~
>>
>> ---
>>   target/hppa/helper.h    | 2 ++
>>   target/hppa/op_helper.c | 9 +++++++++
>>   target/hppa/translate.c | 6 +++++-
>>   3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/hppa/helper.h b/target/hppa/helper.h
>> index 38d834ef6b..2d483aab58 100644
>> --- a/target/hppa/helper.h
>> +++ b/target/hppa/helper.h
>> @@ -17,6 +17,8 @@ DEF_HELPER_FLAGS_3(stby_b_parallel, TCG_CALL_NO_WG, void, env, tl, tr)
>>   DEF_HELPER_FLAGS_3(stby_e, TCG_CALL_NO_WG, void, env, tl, tr)
>>   DEF_HELPER_FLAGS_3(stby_e_parallel, TCG_CALL_NO_WG, void, env, tl, tr)
>>
>> +DEF_HELPER_FLAGS_1(ldc_check, TCG_CALL_NO_RWG, void, tl)
>> +
>>   DEF_HELPER_FLAGS_4(probe, TCG_CALL_NO_WG, tr, env, tl, i32, i32)
>>
>>   DEF_HELPER_FLAGS_1(loaded_fr0, TCG_CALL_NO_RWG, void, env)
>> diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
>> index f0516e81f1..345cef2c08 100644
>> --- a/target/hppa/op_helper.c
>> +++ b/target/hppa/op_helper.c
>> @@ -153,6 +153,15 @@ void HELPER(stby_e_parallel)(CPUHPPAState *env, target_ulong addr,
>>       do_stby_e(env, addr, val, true, GETPC());
>>   }
>>
>> +void HELPER(ldc_check)(target_ulong addr)
>> +{
>> +    if (unlikely(addr & 0xf)) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "Undefined ldc to address unaligned mod 16: "

"to unaligned address mod 16"?

>> +                      TARGET_FMT_lx "\n", addr);
>> +    }
>> +}
>> +
>>   target_ureg HELPER(probe)(CPUHPPAState *env, target_ulong addr,
>>                             uint32_t level, uint32_t want)
>>   {
>> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
>> index 2f8d407a82..669381dc1d 100644
>> --- a/target/hppa/translate.c
>> +++ b/target/hppa/translate.c
>> @@ -2942,7 +2942,7 @@ static bool trans_st(DisasContext *ctx, arg_ldst *a)
>>
>>   static bool trans_ldc(DisasContext *ctx, arg_ldst *a)
>>   {
>> -    MemOp mop = MO_TEUL | MO_ALIGN_16 | a->size;
>> +    MemOp mop = MO_TE | MO_ALIGN | a->size;


Hmmm you changed MO_TEUL -> MO_TE, so from MO_32 to MO_8.

Per your description, shouldn't this be MO_TEUL | MO_ALIGN_4?

>>       TCGv_reg zero, dest, ofs;
>>       TCGv_tl addr;
>>
>> @@ -2958,8 +2958,12 @@ static bool trans_ldc(DisasContext *ctx, arg_ldst *a)
>>
>>       form_gva(ctx, &addr, &ofs, a->b, a->x, a->scale ? a->size : 0,
>>                a->disp, a->sp, a->m, ctx->mmu_idx == MMU_PHYS_IDX);
>> +
>> +    gen_helper_ldc_check(addr);
>>       zero = tcg_const_reg(0);
>>       tcg_gen_atomic_xchg_reg(dest, addr, zero, ctx->mmu_idx, mop);
>> +    tcg_temp_free(zero);
>> +
>>       if (a->m) {
>>           save_gpr(ctx, a->b, ofs);
>>       }
>>
> 
>