| Message ID | 1433193897-24110-10-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On 06/01/2015 02:24 PM, Aurelien Jarno wrote: > +/* TRANSLATE EXTENDED */ > + C(0xb2a5, TRE, RRE, Z, 0, 0, 0, 0, tre, 0) ... > +static ExitStatus op_tre(DisasContext *s, DisasOps *o) > +{ > + TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1)); > + TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2)); > + potential_page_fault(s); > + gen_helper_tre(cpu_env, r1, r2); > + tcg_temp_free_i32(r1); > + tcg_temp_free_i32(r2); > + set_cc_static(s); > + return NO_EXIT; > +} Missing the specification exception for odd r1. Easily fixable by using prep_r1_P. You don't necessarily have to do anything else -- merely prepping out+out2 are sufficient. But why don't we just pass and return (most) of the data to the helper? Like C(0xb2a5, TRE, RRE, Z, 0, r2, r1_P, 0, tre, 0) potential_page_fault(s); gen_helper_tre(o->out, cpu_env, o->out, o->out2, o->in2); return_low128(o->out2); set_cc_static(s); r~
On 2015-06-02 10:07, Richard Henderson wrote: > On 06/01/2015 02:24 PM, Aurelien Jarno wrote: > > +/* TRANSLATE EXTENDED */ > > + C(0xb2a5, TRE, RRE, Z, 0, 0, 0, 0, tre, 0) > ... > > +static ExitStatus op_tre(DisasContext *s, DisasOps *o) > > +{ > > + TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1)); > > + TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2)); > > + potential_page_fault(s); > > + gen_helper_tre(cpu_env, r1, r2); > > + tcg_temp_free_i32(r1); > > + tcg_temp_free_i32(r2); > > + set_cc_static(s); > > + return NO_EXIT; > > +} > > Missing the specification exception for odd r1. Good catch. > Easily fixable by using prep_r1_P. You don't necessarily have to do anything > else -- merely prepping out+out2 are sufficient. > > But why don't we just pass and return (most) of the data to the helper? Like > > C(0xb2a5, TRE, RRE, Z, 0, r2, r1_P, 0, tre, 0) > > potential_page_fault(s); > gen_helper_tre(o->out, cpu_env, o->out, o->out2, o->in2); > return_low128(o->out2); > set_cc_static(s); My point was that we need to pass 4 values (reg0, r1, r1+1 and r2) and return 3 values (r1, r1+1 and cc), so it's probably better to pass all of them the same way. It's the strategy chosen for other similar instructions (e.g mvcl), except for cc. I'll change that in the next version.
On 06/02/2015 12:05 PM, Aurelien Jarno wrote: >> But why don't we just pass and return (most) of the data to the helper? Like >> >> C(0xb2a5, TRE, RRE, Z, 0, r2, r1_P, 0, tre, 0) >> >> potential_page_fault(s); >> gen_helper_tre(o->out, cpu_env, o->out, o->out2, o->in2); >> return_low128(o->out2); >> set_cc_static(s); > > My point was that we need to pass 4 values (reg0, r1, r1+1 and r2) and > return 3 values (r1, r1+1 and cc), so it's probably better to pass all > of them the same way. It's the strategy chosen for other similar > instructions (e.g mvcl), except for cc. > > I'll change that in the next version. The reg0 and cc data is at a fixed location, and are therefore more amenable to passing implicitly. It's r1, r1+1, and r2 that are in varying locations, and therefore you either have to pass their register number or their contents. For mvcl, there are 5 return values, so we're pretty much stuck passing register numbers. r~
diff --git a/target-s390x/helper.h b/target-s390x/helper.h index 48b015e..68f0b67 100644 --- a/target-s390x/helper.h +++ b/target-s390x/helper.h @@ -77,6 +77,7 @@ DEF_HELPER_FLAGS_3(sqxb, TCG_CALL_NO_WG, i64, env, i64, i64) DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32) DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64) DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64) +DEF_HELPER_3(tre, void, env, i32, i32) DEF_HELPER_4(trt, i32, env, i32, i64, i64) DEF_HELPER_4(cksm, i64, env, i64, i64, i64) DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64) diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def index 2a7ecbd..faaedf8 100644 --- a/target-s390x/insn-data.def +++ b/target-s390x/insn-data.def @@ -761,6 +761,8 @@ C(0xdc00, TR, SS_a, Z, la1, a2, 0, 0, tr, 0) /* TRANSLATE AND TEST */ C(0xdd00, TRT, SS_a, Z, la1, a2, 0, 0, trt, 0) +/* TRANSLATE EXTENDED */ + C(0xb2a5, TRE, RRE, Z, 0, 0, 0, 0, tre, 0) /* UNPACK */ /* Really format SS_b, but we pack both lengths into one argument diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c index e19e1aa..9fe9ed7 100644 --- a/target-s390x/mem_helper.c +++ b/target-s390x/mem_helper.c @@ -804,6 +804,45 @@ void HELPER(tr)(CPUS390XState *env, uint32_t len, uint64_t array, } } +void HELPER(tre)(CPUS390XState *env, uint32_t r1, uint32_t r2) +{ + uint8_t end = env->regs[0] & 0xff; + uint64_t array = env->regs[r1]; + uint64_t len = env->regs[r1 + 1]; + uint64_t trans = env->regs[r2]; + uint64_t i; + + if (!(env->psw.mask & PSW_MASK_64)) { + array &= 0x7fffffff; + len = (uint32_t)len; + } + + /* Lest we fail to service interrupts in a timely manner, limit the + amount of work we're willing to do. For now, let's cap at 8k. */ + if (len > 0x2000) { + len = 0x2000; + env->cc_op = 3; + } else { + env->cc_op = 0; + } + + for (i = 0; i < len; i++) { + uint8_t byte, new_byte; + + byte = cpu_ldub_data(env, array + i); + + if (byte == end) { + env->cc_op = 1; + break; + } + + new_byte = cpu_ldub_data(env, trans + byte); + cpu_stb_data(env, array + i, new_byte); + } + env->regs[r1] = array + i; + env->regs[r1 + 1] -= i; +} + uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array, uint64_t trans) { diff --git a/target-s390x/translate.c b/target-s390x/translate.c index 003598d..db29993 100644 --- a/target-s390x/translate.c +++ b/target-s390x/translate.c @@ -3787,6 +3787,18 @@ static ExitStatus op_tr(DisasContext *s, DisasOps *o) return NO_EXIT; } +static ExitStatus op_tre(DisasContext *s, DisasOps *o) +{ + TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1)); + TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2)); + potential_page_fault(s); + gen_helper_tre(cpu_env, r1, r2); + tcg_temp_free_i32(r1); + tcg_temp_free_i32(r2); + set_cc_static(s); + return NO_EXIT; +} + static ExitStatus op_trt(DisasContext *s, DisasOps *o) { TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
It is part of the basic zArchitecture instructions. Cc: Alexander Graf <agraf@suse.de> Cc: Richard Henderson <rth@twiddle.net> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- target-s390x/helper.h | 1 + target-s390x/insn-data.def | 2 ++ target-s390x/mem_helper.c | 39 +++++++++++++++++++++++++++++++++++++++ target-s390x/translate.c | 12 ++++++++++++ 4 files changed, 54 insertions(+)