| Message ID | 20200117015322.12953-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/hppa: Allow, but diagnose, LDCW aligned only mod 4 | expand |
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: " > + 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; > 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); > } >
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); >> } >> > >
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 <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); 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); >>> } >>> >> >> >
On 1/17/20 6:13 AM, Philippe Mathieu-Daudé wrote: >>> - 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? The "UL" part is also being added by a->size. This code was written this way in preparation for the 64-bit ldc, and the bug was not noticable because we don't have that yet. r~
This is not related to the patch but there is one other corner issue with the load and clear instructions. When the target register is GR0, the instruction may be implemented as a normal load and clear which clears memory, or it may be aliased to the equivalent-sized load instruction, in which case it behaves exactly like that prefetch instruction and does not clear the data in memory. See page 6-12 on PA 2.0 architecture. Don't know what was actually done. Regards, Dave
On 1/17/20 6:30 PM, Richard Henderson wrote: > On 1/17/20 6:13 AM, Philippe Mathieu-Daudé wrote: >>>> - 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? > > The "UL" part is also being added by a->size. This code was written this way > in preparation for the 64-bit ldc, and the bug was not noticable because we > don't have that yet. Ah I missed the a->size. So on 32-bit the hw trap doesn't trap on unaligned 16, but traps on unaligned 4. On 64-bit we don't know yet, but IIUC we expect to not trap on unaligned 8. Looking at target/hppa/insns.decode: &ldst t b x disp sp m scale size @ldstx ...... b:5 x:5 sp:2 scale:1 ....... m:1 t:5 &ldst disp=0 @ldim5 ...... b:5 ..... sp:2 ......... t:5 \ &ldst disp=%im5_16 x=0 scale=0 m=%ma_to_m ldc 000011 ..... ..... .. . 1 -- 0111 ...... @ldim5 size=2 ldc 000011 ..... ..... .. . 0 -- 0111 ...... @ldstx size=2 We have a->size = 2 = MO_32 = MO_UL. So do you plan to add LDCD from PA2.0 using size=3, OK. From "exec/memop.h": * MO_ALIGN supposes the alignment size is the size of a memory access. * * There are three options: * - unaligned access permitted (MO_UNALN). * - an alignment to the size of an access (MO_ALIGN); Ah so with LDCW we already access a word, so MO_ALIGN -> MO_ALIGN_4. With LDLD we'll have MO_ALIGN -> MO_ALIGN_8. Now "MemOp mop = MO_TE | MO_ALIGN | a->size;" makes sense! The power of your decodetree script amazed me again :) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On 1/17/20 7:01 AM, Helge Deller wrote: > Maybe adding something like (if TARGET_32BIT...) now would make sense, so we don't get it > wrong when 64bit gets added? I'll add a "TODO: HPPA64" comment. r~
On Fri, Jan 17, 2020 at 2:53 AM Richard Henderson <richard.henderson@linaro.org> 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. This Linux kernel commit seems relevant: https://github.com/torvalds/linux/commit/14e256c107304#diff-e85862c7227599cb24e36494f75948d5R159-R164 /* From: "Jim Hull" <jim.hull of hp.com> I've attached a summary of the change, but basically, for PA 2.0, as long as the ",CO" (coherent operation) completer is specified, then the 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead they only require "natural" alignment (4-byte for ldcw, 8-byte for ldcd). */ (I couldn't find the original post on http://lists.parisc-linux.org/pipermail/parisc-linux/) Maybe you can point to it in the description. > 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. > > > 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: " > + 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; > 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); > } > -- > 2.20.1 > >
On 1/17/20 8:50 AM, Philippe Mathieu-Daudé wrote: > On Fri, Jan 17, 2020 at 2:53 AM Richard Henderson > <richard.henderson@linaro.org> 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. > > This Linux kernel commit seems relevant: > > https://github.com/torvalds/linux/commit/14e256c107304#diff-e85862c7227599cb24e36494f75948d5R159-R164 > > /* From: "Jim Hull" <jim.hull of hp.com> > I've attached a summary of the change, but basically, for PA 2.0, as > long as the ",CO" (coherent operation) completer is specified, then the > 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead > they only require "natural" alignment (4-byte for ldcw, 8-byte for > ldcd). */ It isn't completely relevant. We don't implement hppa 2.0. I added a TODO comment for HPPA64, as I said in reply to Dave Anglin elsewhere in this thread. r~
On 17.01.20 20:23, Richard Henderson wrote: > On 1/17/20 8:50 AM, Philippe Mathieu-Daudé wrote: >> On Fri, Jan 17, 2020 at 2:53 AM Richard Henderson >> <richard.henderson@linaro.org> 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. >> >> This Linux kernel commit seems relevant: >> >> https://github.com/torvalds/linux/commit/14e256c107304#diff-e85862c7227599cb24e36494f75948d5R159-R164 >> >> /* From: "Jim Hull" <jim.hull of hp.com> >> I've attached a summary of the change, but basically, for PA 2.0, as >> long as the ",CO" (coherent operation) completer is specified, then the >> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead >> they only require "natural" alignment (4-byte for ldcw, 8-byte for >> ldcd). */ > > It isn't completely relevant. We don't implement hppa 2.0. > > I added a TODO comment for HPPA64, as I said in reply to Dave Anglin elsewhere > in this thread. I agree. A TODO comment should be sufficient for now. Thanks! Helge
On 1/17/20 8:23 PM, Richard Henderson wrote: > On 1/17/20 8:50 AM, Philippe Mathieu-Daudé wrote: >> On Fri, Jan 17, 2020 at 2:53 AM Richard Henderson >> <richard.henderson@linaro.org> 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. >> >> This Linux kernel commit seems relevant: >> >> https://github.com/torvalds/linux/commit/14e256c107304#diff-e85862c7227599cb24e36494f75948d5R159-R164 >> >> /* From: "Jim Hull" <jim.hull of hp.com> >> I've attached a summary of the change, but basically, for PA 2.0, as >> long as the ",CO" (coherent operation) completer is specified, then the >> 16-byte alignment requirement for ldcw and ldcd is relaxed, and instead >> they only require "natural" alignment (4-byte for ldcw, 8-byte for >> ldcd). */ > > It isn't completely relevant. We don't implement hppa 2.0. Oh... It was late and I misread it as: if (PA2) ... else ... "instead of PA 2.0 ... the 16-byte alignment ... is relaxed, and insted only require "natural" alignment ..." endif > > I added a TODO comment for HPPA64, as I said in reply to Dave Anglin elsewhere > in this thread.
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: " + 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; 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); }
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. r~ --- target/hppa/helper.h | 2 ++ target/hppa/op_helper.c | 9 +++++++++ target/hppa/translate.c | 6 +++++- 3 files changed, 16 insertions(+), 1 deletion(-)