On 17.01.20 17:13, Philippe Mathieu-Daudé wrote:
> 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 <del...@gmx.de>
>>> Suggested-by: John David Anglin <dave.ang...@bell.net>
>>> Signed-off-by: Richard Henderson <richard.hender...@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 <del...@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);
Actually, for 64-bit the address is allowed to be 4-byte aligned, as long as the
"co" completer is given, e.g. this would be OK: "ldcw,co 0(addr),%reg".
Maybe adding something like (if TARGET_32BIT...) now would make sense, so we
don't get it
wrong when 64bit gets added?
Helge
>>> 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);
>>> }
>>>
>>
>>
>
