On 02/15/2018 02:50 AM, David Hildenbrand wrote: > On 15.02.2018 11:38, David Hildenbrand wrote: >> Let's add proper alignment checks for a handful of instructions that >> require a SPECIFICATION exception in case alignment is violated. >> >> Introduce new wout/in functions. As we are right now only using them for >> privileged instructions, we have to add ugly ifdefs to silence >> compilers. >> >> Convert STORE CPU ID right away to make use of the wout function. >> >> Signed-off-by: David Hildenbrand <da...@redhat.com> >> --- >> target/s390x/insn-data.def | 16 ++++++++-------- >> target/s390x/mem_helper.c | 25 +++++++++++++++++++++++++ >> target/s390x/translate.c | 43 ++++++++++++++++++++++++++++++++++++++++++- >> 3 files changed, 75 insertions(+), 9 deletions(-) >> >> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def >> index 621e10d615..157619403d 100644 >> --- a/target/s390x/insn-data.def >> +++ b/target/s390x/insn-data.def >> @@ -1000,13 +1000,13 @@ >> /* ??? Not implemented - is it necessary? */ >> C(0xb204, SCK, S, Z, 0, 0, 0, 0, 0, 0) >> /* SET CLOCK COMPARATOR */ >> - C(0xb206, SCKC, S, Z, 0, m2_64, 0, 0, sckc, 0) >> + C(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0) >> /* SET CLOCK PROGRAMMABLE FIELD */ >> C(0x0107, SCKPF, E, Z, 0, 0, 0, 0, sckpf, 0) >> /* SET CPU TIMER */ >> - C(0xb208, SPT, S, Z, 0, m2_64, 0, 0, spt, 0) >> + C(0xb208, SPT, S, Z, 0, m2_64a, 0, 0, spt, 0) >> /* SET PREFIX */ >> - C(0xb210, SPX, S, Z, 0, m2_32u, 0, 0, spx, 0) >> + C(0xb210, SPX, S, Z, 0, m2_32ua, 0, 0, spx, 0) >> /* SET PSW KEY FROM ADDRESS */ >> C(0xb20a, SPKA, S, Z, 0, a2, 0, 0, spka, 0) >> /* SET STORAGE KEY EXTENDED */ >> @@ -1021,20 +1021,20 @@ >> /* STORE CLOCK EXTENDED */ >> C(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0) >> /* STORE CLOCK COMPARATOR */ >> - C(0xb207, STCKC, S, Z, la2, 0, new, m1_64, stckc, 0) >> + C(0xb207, STCKC, S, Z, la2, 0, new, m1_64a, stckc, 0) >> /* STORE CONTROL */ >> C(0xb600, STCTL, RS_a, Z, 0, a2, 0, 0, stctl, 0) >> C(0xeb25, STCTG, RSY_a, Z, 0, a2, 0, 0, stctg, 0) >> /* STORE CPU ADDRESS */ >> - C(0xb212, STAP, S, Z, la2, 0, new, m1_16, stap, 0) >> + C(0xb212, STAP, S, Z, la2, 0, new, m1_16a, stap, 0) >> /* STORE CPU ID */ >> - C(0xb202, STIDP, S, Z, la2, 0, new, 0, stidp, 0) >> + C(0xb202, STIDP, S, Z, la2, 0, new, m1_64a, stidp, 0) >> /* STORE CPU TIMER */ >> - C(0xb209, STPT, S, Z, la2, 0, new, m1_64, stpt, 0) >> + C(0xb209, STPT, S, Z, la2, 0, new, m1_64a, stpt, 0) >> /* STORE FACILITY LIST */ >> C(0xb2b1, STFL, S, Z, 0, 0, 0, 0, stfl, 0) >> /* STORE PREFIX */ >> - C(0xb211, STPX, S, Z, la2, 0, new, m1_32, stpx, 0) >> + C(0xb211, STPX, S, Z, la2, 0, new, m1_32a, stpx, 0) >> /* STORE SYSTEM INFORMATION */ >> C(0xb27d, STSI, S, Z, 0, a2, 0, 0, stsi, 0) >> /* STORE THEN AND SYSTEM MASK */ >> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c >> index 427b795a78..d5291b246e 100644 >> --- a/target/s390x/mem_helper.c >> +++ b/target/s390x/mem_helper.c >> @@ -693,6 +693,11 @@ void HELPER(lam)(CPUS390XState *env, uint32_t r1, >> uint64_t a2, uint32_t r3) >> uintptr_t ra = GETPC(); >> int i; >> >> + if (a2 & 0x3) { >> + /* we either came here by lam or lamy, which have different lengths >> */ >> + s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra); >> + } >> + >> for (i = r1;; i = (i + 1) % 16) { >> env->aregs[i] = cpu_ldl_data_ra(env, a2, ra); >> a2 += 4; >> @@ -709,6 +714,10 @@ void HELPER(stam)(CPUS390XState *env, uint32_t r1, >> uint64_t a2, uint32_t r3) >> uintptr_t ra = GETPC(); >> int i; >> >> + if (a2 & 0x3) { >> + s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra); >> + } >> + >> for (i = r1;; i = (i + 1) % 16) { >> cpu_stl_data_ra(env, a2, env->aregs[i], ra); >> a2 += 4; >> @@ -1620,6 +1629,10 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, >> uint64_t a2, uint32_t r3) >> uint64_t src = a2; >> uint32_t i; >> >> + if (src & 0x7) { >> + s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra); >> + } >> + >> for (i = r1;; i = (i + 1) % 16) { >> uint64_t val = cpu_ldq_data_ra(env, src, ra); >> if (env->cregs[i] != val && i >= 9 && i <= 11) { >> @@ -1650,6 +1663,10 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, >> uint64_t a2, uint32_t r3) >> uint64_t src = a2; >> uint32_t i; >> >> + if (src & 0x3) { >> + s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra); >> + } >> + >> for (i = r1;; i = (i + 1) % 16) { >> uint32_t val = cpu_ldl_data_ra(env, src, ra); >> if ((uint32_t)env->cregs[i] != val && i >= 9 && i <= 11) { >> @@ -1677,6 +1694,10 @@ void HELPER(stctg)(CPUS390XState *env, uint32_t r1, >> uint64_t a2, uint32_t r3) >> uint64_t dest = a2; >> uint32_t i; >> >> + if (dest & 0x7) { >> + s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra); >> + } >> + >> for (i = r1;; i = (i + 1) % 16) { >> cpu_stq_data_ra(env, dest, env->cregs[i], ra); >> dest += sizeof(uint64_t); >> @@ -1693,6 +1714,10 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, >> uint64_t a2, uint32_t r3) >> uint64_t dest = a2; >> uint32_t i; >> >> + if (dest & 0x3) { >> + s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra); >> + } >> + >> for (i = r1;; i = (i + 1) % 16) { >> cpu_stl_data_ra(env, dest, env->cregs[i], ra); >> dest += sizeof(uint32_t); >> diff --git a/target/s390x/translate.c b/target/s390x/translate.c >> index 5aea3bbca6..7d39ab350d 100644 >> --- a/target/s390x/translate.c >> +++ b/target/s390x/translate.c >> @@ -4062,7 +4062,6 @@ static ExitStatus op_stidp(DisasContext *s, DisasOps >> *o) >> { >> check_privileged(s); >> tcg_gen_ld_i64(o->out, cpu_env, offsetof(CPUS390XState, cpuid)); >> - tcg_gen_qemu_st_i64(o->out, o->addr1, get_mem_index(s), MO_TEQ | >> MO_ALIGN); >> return NO_EXIT; >> } >> >> @@ -5220,18 +5219,42 @@ static void wout_m1_16(DisasContext *s, DisasFields >> *f, DisasOps *o) >> } >> #define SPEC_wout_m1_16 0 >> >> +#ifndef CONFIG_USER_ONLY >> +static void wout_m1_16a(DisasContext *s, DisasFields *f, DisasOps *o) >> +{ >> + tcg_gen_qemu_st_tl(o->out, o->addr1, get_mem_index(s), MO_TEUW | >> MO_ALIGN); >> +} >> +#define SPEC_wout_m1_16a 0 >> +#endif >> + >> static void wout_m1_32(DisasContext *s, DisasFields *f, DisasOps *o) >> { >> tcg_gen_qemu_st32(o->out, o->addr1, get_mem_index(s)); >> } >> #define SPEC_wout_m1_32 0 >> >> +#ifndef CONFIG_USER_ONLY >> +static void wout_m1_32a(DisasContext *s, DisasFields *f, DisasOps *o) >> +{ >> + tcg_gen_qemu_st_tl(o->out, o->addr1, get_mem_index(s), MO_TEUL | >> MO_ALIGN); >> +} >> +#define SPEC_wout_m1_32a 0 >> +#endif >> + >> static void wout_m1_64(DisasContext *s, DisasFields *f, DisasOps *o) >> { >> tcg_gen_qemu_st64(o->out, o->addr1, get_mem_index(s)); >> } >> #define SPEC_wout_m1_64 0 >> >> +#ifndef CONFIG_USER_ONLY >> +static void wout_m1_64a(DisasContext *s, DisasFields *f, DisasOps *o) >> +{ >> + tcg_gen_qemu_st_i64(o->out, o->addr1, get_mem_index(s), MO_TEQ | >> MO_ALIGN); >> +} >> +#define SPEC_wout_m1_64a 0 >> +#endif >> + >> static void wout_m2_32(DisasContext *s, DisasFields *f, DisasOps *o) >> { >> tcg_gen_qemu_st32(o->out, o->in2, get_mem_index(s)); >> @@ -5657,6 +5680,15 @@ static void in2_m2_32u(DisasContext *s, DisasFields >> *f, DisasOps *o) >> } >> #define SPEC_in2_m2_32u 0 >> >> +#ifndef CONFIG_USER_ONLY >> +static void in2_m2_32ua(DisasContext *s, DisasFields *f, DisasOps *o) >> +{ >> + in2_a2(s, f, o); >> + tcg_gen_qemu_ld_tl(o->in2, o->in2, get_mem_index(s), MO_TEUL | >> MO_ALIGN); >> +} >> +#define SPEC_in2_m2_32ua 0 >> +#endif >> + >> static void in2_m2_64(DisasContext *s, DisasFields *f, DisasOps *o) >> { >> in2_a2(s, f, o); >> @@ -5664,6 +5696,15 @@ static void in2_m2_64(DisasContext *s, DisasFields >> *f, DisasOps *o) >> } >> #define SPEC_in2_m2_64 0 >> >> +#ifndef CONFIG_USER_ONLY >> +static void in2_m2_64a(DisasContext *s, DisasFields *f, DisasOps *o) >> +{ >> + in2_a2(s, f, o); >> + tcg_gen_qemu_ld_i64(o->in2, o->in2, get_mem_index(s), MO_TEQ | >> MO_ALIGN); >> +} >> +#define SPEC_in2_m2_64a 0 >> +#endif >> + >> static void in2_mri2_16u(DisasContext *s, DisasFields *f, DisasOps *o) >> { >> in2_ri2(s, f, o); >> > > Conny, if there are no new comments, can you make sure to add Richard's > r-b when applying? thanks! >
None from me. ;-) (re-)Reviewed-by: Richard Henderson <richard.hender...@linaro.org>