* [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
2017-07-14 21:01 ` Aurelien Jarno
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns Richard Henderson
` (6 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/helper.h | 1 +
target/s390x/cpu_models.c | 2 +
target/s390x/mem_helper.c | 189 +++++++++++++++++++++++++++++++++++++++++++++
target/s390x/translate.c | 13 +++-
target/s390x/insn-data.def | 2 +
5 files changed, 206 insertions(+), 1 deletion(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 964097b..23e8d1d 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -33,6 +33,7 @@ DEF_HELPER_3(celgb, i64, env, i64, i32)
DEF_HELPER_3(cdlgb, i64, env, i64, i32)
DEF_HELPER_3(cxlgb, i64, env, i64, i32)
DEF_HELPER_4(cdsg, void, env, i64, i32, i32)
+DEF_HELPER_4(csst, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_3(aeb, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(adb, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_5(axb, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i64)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7cb55dc..2c86b24 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -736,6 +736,8 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
S390_FEAT_ETF2_ENH,
S390_FEAT_STORE_CLOCK_FAST,
S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
+ S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
+ S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
S390_FEAT_EXECUTE_EXT,
S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index ede8471..513b402 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1353,6 +1353,195 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
env->regs[r1 + 1] = int128_getlo(oldv);
}
+uint32_t HELPER(csst)(CPUS390XState *env, uint32_t r3, uint64_t a1, uint64_t a2)
+{
+#if !defined(CONFIG_USER_ONLY) || defined(CONFIG_ATOMIC128)
+ uint32_t mem_idx = cpu_mmu_index(env, false);
+#endif
+ uintptr_t ra = GETPC();
+ uint32_t fc = extract32(env->regs[0], 0, 8);
+ uint32_t sc = extract32(env->regs[0], 8, 8);
+ uint64_t pl = get_address(env, 1) & -16;
+ uint64_t svh, svl;
+ uint32_t cc;
+
+ /* Sanity check the function code and storage characteristic. */
+ if (fc > 1 || sc > 3) {
+ if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
+ goto spec_exception;
+ }
+ if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
+ goto spec_exception;
+ }
+ }
+
+ /* Sanity check the alignments. */
+ if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
+ goto spec_exception;
+ }
+
+ /* Sanity check writability of the store address. */
+#ifndef CONFIG_USER_ONLY
+ probe_write(env, a2, mem_idx, ra);
+#endif
+
+ /* Note that the compare-and-swap is atomic, and the store is atomic, but
+ the complete operation is not. Therefore we do not need to assert serial
+ context in order to implement this. That said, restart early if we can't
+ support either operation that is supposed to be atomic. */
+ if (parallel_cpus) {
+ int mask = 0;
+#if !defined(CONFIG_ATOMIC64)
+ mask = -8;
+#elif !defined(CONFIG_ATOMIC128)
+ mask = -16;
+#endif
+ if (((4 << fc) | (1 << sc)) & mask) {
+ cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+ }
+ }
+
+ /* All loads happen before all stores. For simplicity, load the entire
+ store value area from the parameter list. */
+ svh = cpu_ldq_data_ra(env, pl + 16, ra);
+ svl = cpu_ldq_data_ra(env, pl + 24, ra);
+
+ switch (fc) {
+ case 0:
+ {
+ uint32_t nv = cpu_ldl_data_ra(env, pl, ra);
+ uint32_t cv = env->regs[r3];
+ uint32_t ov;
+
+ if (parallel_cpus) {
+#ifdef CONFIG_USER_ONLY
+ uint32_t *haddr = g2h(a1);
+ ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
+#else
+ TCGMemOpIdx oi = make_memop_idx(MO_TEUL | MO_ALIGN, mem_idx);
+ ov = helper_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
+#endif
+ } else {
+ ov = cpu_ldl_data_ra(env, a1, ra);
+ cpu_stl_data_ra(env, a1, (ov == cv ? nv : ov), ra);
+ }
+ cc = (ov != cv);
+ env->regs[r3] = deposit64(env->regs[r3], 32, 32, ov);
+ }
+ break;
+
+ case 1:
+ {
+ uint64_t nv = cpu_ldq_data_ra(env, pl, ra);
+ uint64_t cv = env->regs[r3];
+ uint64_t ov;
+
+ if (parallel_cpus) {
+#ifdef CONFIG_USER_ONLY
+# ifdef CONFIG_ATOMIC64
+ uint64_t *haddr = g2h(a1);
+ ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
+# else
+ /* Note that we asserted !parallel_cpus above. */
+ g_assert_not_reached();
+# endif
+#else
+ TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN, mem_idx);
+ ov = helper_atomic_cmpxchgq_be_mmu(env, a1, cv, nv, oi, ra);
+#endif
+ } else {
+ ov = cpu_ldq_data_ra(env, a1, ra);
+ cpu_stq_data_ra(env, a1, (ov == cv ? nv : ov), ra);
+ }
+ cc = (ov != cv);
+ env->regs[r3] = ov;
+ }
+ break;
+
+ case 2:
+ {
+ uint64_t nvh = cpu_ldq_data_ra(env, pl, ra);
+ uint64_t nvl = cpu_ldq_data_ra(env, pl + 8, ra);
+ Int128 nv = int128_make128(nvl, nvh);
+ Int128 cv = int128_make128(env->regs[r3 + 1], env->regs[r3]);
+ Int128 ov;
+
+ if (parallel_cpus) {
+#ifdef CONFIG_ATOMIC128
+ TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
+ ov = helper_atomic_cmpxchgo_be_mmu(env, a1, cv, nv, oi, ra);
+ cc = !int128_eq(ov, cv);
+#else
+ /* Note that we asserted !parallel_cpus above. */
+ g_assert_not_reached();
+#endif
+ } else {
+ uint64_t oh = cpu_ldq_data_ra(env, a1 + 0, ra);
+ uint64_t ol = cpu_ldq_data_ra(env, a1 + 8, ra);
+
+ ov = int128_make128(ol, oh);
+ cc = !int128_eq(ov, cv);
+ if (cc) {
+ nv = ov;
+ }
+
+ cpu_stq_data_ra(env, a1 + 0, int128_gethi(nv), ra);
+ cpu_stq_data_ra(env, a1 + 8, int128_getlo(nv), ra);
+ }
+
+ env->regs[r3 + 0] = int128_gethi(ov);
+ env->regs[r3 + 1] = int128_getlo(ov);
+ }
+ break;
+
+ default:
+ g_assert_not_reached();
+ }
+
+ /* Store only if the comparison succeeded. Note that above we use a pair
+ of 64-bit big-endian loads, so for sc < 3 we must extract the value
+ from the most-significant bits of svh. */
+ if (cc == 0) {
+ switch (sc) {
+ case 0:
+ cpu_stb_data_ra(env, a2, svh >> 56, ra);
+ break;
+ case 1:
+ cpu_stw_data_ra(env, a2, svh >> 48, ra);
+ break;
+ case 2:
+ cpu_stl_data_ra(env, a2, svh >> 32, ra);
+ break;
+ case 3:
+ cpu_stq_data_ra(env, a2, svh, ra);
+ break;
+ case 4:
+ if (parallel_cpus) {
+#ifdef CONFIG_ATOMIC128
+ TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
+ Int128 sv = int128_make128(svl, svh);
+ helper_atomic_sto_be_mmu(env, a2, sv, oi, ra);
+#else
+ /* Note that we asserted !parallel_cpus above. */
+ g_assert_not_reached();
+#endif
+ } else {
+ cpu_stq_data_ra(env, a2 + 0, svh, ra);
+ cpu_stq_data_ra(env, a2 + 8, svl, ra);
+ }
+ default:
+ g_assert_not_reached();
+ }
+ }
+
+ return cc;
+
+ spec_exception:
+ cpu_restore_state(ENV_GET_CPU(env), ra);
+ program_interrupt(env, PGM_SPECIFICATION, 6);
+ g_assert_not_reached();
+}
+
#if !defined(CONFIG_USER_ONLY)
void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
{
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 592d6b0..e739525 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2033,6 +2033,18 @@ static ExitStatus op_cdsg(DisasContext *s, DisasOps *o)
return NO_EXIT;
}
+static ExitStatus op_csst(DisasContext *s, DisasOps *o)
+{
+ int r3 = get_field(s->fields, r3);
+ TCGv_i32 t_r3 = tcg_const_i32(r3);
+
+ gen_helper_csst(cc_op, cpu_env, t_r3, o->in1, o->in2);
+ tcg_temp_free_i32(t_r3);
+
+ set_cc_static(s);
+ return NO_EXIT;
+}
+
#ifndef CONFIG_USER_ONLY
static ExitStatus op_csp(DisasContext *s, DisasOps *o)
{
@@ -5437,7 +5449,6 @@ enum DisasInsnEnum {
/* Give smaller names to the various facilities. */
#define FAC_Z S390_FEAT_ZARCH
#define FAC_CASS S390_FEAT_COMPARE_AND_SWAP_AND_STORE
-#define FAC_CASS2 S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2
#define FAC_DFP S390_FEAT_DFP
#define FAC_DFPR S390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* DFP-rounding */
#define FAC_DO S390_FEAT_STFLE_45 /* distinct-operands */
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index d3bb851..6ac12b8 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -265,6 +265,8 @@
D(0xbb00, CDS, RS_a, Z, r3_D32, r1_D32, new, r1_D32, cs, 0, MO_TEQ)
D(0xeb31, CDSY, RSY_a, LD, r3_D32, r1_D32, new, r1_D32, cs, 0, MO_TEQ)
C(0xeb3e, CDSG, RSY_a, Z, 0, 0, 0, 0, cdsg, 0)
+/* COMPARE AND SWAP AND STORE */
+ C(0xc802, CSST, SSF, CASS, la1, a2, 0, 0, csst, 0)
/* COMPARE AND TRAP */
D(0xb972, CRT, RRF_c, GIE, r1_32s, r2_32s, 0, 0, ct, 0, 0)
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST Richard Henderson
@ 2017-07-14 21:01 ` Aurelien Jarno
2017-07-15 0:22 ` Richard Henderson
0 siblings, 1 reply; 22+ messages in thread
From: Aurelien Jarno @ 2017-07-14 21:01 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, david
On 2017-07-10 10:45, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target/s390x/helper.h | 1 +
> target/s390x/cpu_models.c | 2 +
> target/s390x/mem_helper.c | 189 +++++++++++++++++++++++++++++++++++++++++++++
> target/s390x/translate.c | 13 +++-
> target/s390x/insn-data.def | 2 +
> 5 files changed, 206 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index ede8471..513b402 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1353,6 +1353,195 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
> env->regs[r1 + 1] = int128_getlo(oldv);
> }
>
> +uint32_t HELPER(csst)(CPUS390XState *env, uint32_t r3, uint64_t a1, uint64_t a2)
> +{
> +#if !defined(CONFIG_USER_ONLY) || defined(CONFIG_ATOMIC128)
> + uint32_t mem_idx = cpu_mmu_index(env, false);
> +#endif
> + uintptr_t ra = GETPC();
> + uint32_t fc = extract32(env->regs[0], 0, 8);
> + uint32_t sc = extract32(env->regs[0], 8, 8);
> + uint64_t pl = get_address(env, 1) & -16;
> + uint64_t svh, svl;
> + uint32_t cc;
> +
> + /* Sanity check the function code and storage characteristic. */
> + if (fc > 1 || sc > 3) {
> + if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
> + goto spec_exception;
> + }
> + if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
> + goto spec_exception;
> + }
> + }
> +
> + /* Sanity check the alignments. */
> + if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
> + goto spec_exception;
> + }
> +
> + /* Sanity check writability of the store address. */
> +#ifndef CONFIG_USER_ONLY
> + probe_write(env, a2, mem_idx, ra);
> +#endif
> +
> + /* Note that the compare-and-swap is atomic, and the store is atomic, but
> + the complete operation is not. Therefore we do not need to assert serial
> + context in order to implement this. That said, restart early if we can't
> + support either operation that is supposed to be atomic. */
> + if (parallel_cpus) {
> + int mask = 0;
> +#if !defined(CONFIG_ATOMIC64)
> + mask = -8;
> +#elif !defined(CONFIG_ATOMIC128)
> + mask = -16;
> +#endif
> + if (((4 << fc) | (1 << sc)) & mask) {
> + cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> + }
> + }
This doesn't look correct. For a 16-byte store, ie sc = 4, and with
ATOMIC128 support, ie mask = -16, the condition is true.
> + /* All loads happen before all stores. For simplicity, load the entire
> + store value area from the parameter list. */
> + svh = cpu_ldq_data_ra(env, pl + 16, ra);
> + svl = cpu_ldq_data_ra(env, pl + 24, ra);
> +
> + switch (fc) {
> + case 0:
> + {
> + uint32_t nv = cpu_ldl_data_ra(env, pl, ra);
> + uint32_t cv = env->regs[r3];
> + uint32_t ov;
> +
> + if (parallel_cpus) {
> +#ifdef CONFIG_USER_ONLY
> + uint32_t *haddr = g2h(a1);
> + ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
> +#else
> + TCGMemOpIdx oi = make_memop_idx(MO_TEUL | MO_ALIGN, mem_idx);
> + ov = helper_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
> +#endif
Not a problem with the patch itself, but how complicated would it be to
make helper_atomic_cmpxchgl_be_mmu (just like the o version) available
also in user mode? That would make less #ifdef in the backends.
The remaining of the patch looks all good to me.
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST
2017-07-14 21:01 ` Aurelien Jarno
@ 2017-07-15 0:22 ` Richard Henderson
2017-07-15 8:28 ` Aurelien Jarno
0 siblings, 1 reply; 22+ messages in thread
From: Richard Henderson @ 2017-07-15 0:22 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel, david
On 07/14/2017 11:01 AM, Aurelien Jarno wrote:
>> + if (parallel_cpus) {
>> + int mask = 0;
>> +#if !defined(CONFIG_ATOMIC64)
>> + mask = -8;
>> +#elif !defined(CONFIG_ATOMIC128)
>> + mask = -16;
>> +#endif
>> + if (((4 << fc) | (1 << sc)) & mask) {
>> + cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
>> + }
>> + }
>
> This doesn't look correct. For a 16-byte store, ie sc = 4, and with
> ATOMIC128 support, ie mask = -16, the condition is true.
That's WITHOUT atomic128 support that mask = -16.
If we have atomic128, then mask = 0.
>> + if (parallel_cpus) {
>> +#ifdef CONFIG_USER_ONLY
>> + uint32_t *haddr = g2h(a1);
>> + ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
>> +#else
>> + TCGMemOpIdx oi = make_memop_idx(MO_TEUL | MO_ALIGN, mem_idx);
>> + ov = helper_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
>> +#endif
>
> Not a problem with the patch itself, but how complicated would it be to
> make helper_atomic_cmpxchgl_be_mmu (just like the o version) available
> also in user mode? That would make less #ifdef in the backends.
Dunno. Probably not too bad. I have wondered about doing that, as these sorts
of functions are quite ugly to write.
r~
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST
2017-07-15 0:22 ` Richard Henderson
@ 2017-07-15 8:28 ` Aurelien Jarno
0 siblings, 0 replies; 22+ messages in thread
From: Aurelien Jarno @ 2017-07-15 8:28 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, david
On 2017-07-14 14:22, Richard Henderson wrote:
> On 07/14/2017 11:01 AM, Aurelien Jarno wrote:
> > > + if (parallel_cpus) {
> > > + int mask = 0;
> > > +#if !defined(CONFIG_ATOMIC64)
> > > + mask = -8;
> > > +#elif !defined(CONFIG_ATOMIC128)
> > > + mask = -16;
> > > +#endif
> > > + if (((4 << fc) | (1 << sc)) & mask) {
> > > + cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> > > + }
> > > + }
> >
> > This doesn't look correct. For a 16-byte store, ie sc = 4, and with
> > ATOMIC128 support, ie mask = -16, the condition is true.
>
> That's WITHOUT atomic128 support that mask = -16.
> If we have atomic128, then mask = 0.
Oh, right, it all looks correct then.
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
2017-07-11 15:18 ` Thomas Huth
2017-07-14 21:04 ` Aurelien Jarno
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 3/8] target/s390x: Tidy SRST Richard Henderson
` (5 subsequent siblings)
7 siblings, 2 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/helper.h | 6 +
target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
target/s390x/translate.c | 43 +++++++
target/s390x/insn-data.def | 13 ++
4 files changed, 372 insertions(+)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 23e8d1d..2793cf3 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -107,6 +107,12 @@ DEF_HELPER_2(stfle, i32, env, i64)
DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
+DEF_HELPER_4(cu12, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu14, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu21, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu24, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
#ifndef CONFIG_USER_ONLY
DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 513b402..0b18560 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2196,3 +2196,313 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
return cc;
}
+
+/* Decode a Unicode character. A return value < 0 indicates success, storing
+ the UTF-32 result into OCHAR and the input length into OLEN. A return
+ value >= 0 indicates failure, and the CC value to be returned. */
+typedef int (*decode_unicode_fn)(CPUS390XState *env, uint64_t addr,
+ uint64_t ilen, bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen);
+
+/* Encode a Unicode character. A return value < 0 indicates success, storing
+ the bytes into ADDR and the output length into OLEN. A return value >= 0
+ indicates failure, and the CC value to be returned. */
+typedef int (*encode_unicode_fn)(CPUS390XState *env, uint64_t addr,
+ uint64_t ilen, uintptr_t ra, uint32_t c,
+ uint32_t *olen);
+
+static int decode_utf8(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen)
+{
+ uint8_t s0, s1, s2, s3;
+ uint32_t c, l;
+
+ if (ilen < 1) {
+ return 0;
+ }
+ s0 = cpu_ldub_data_ra(env, addr, ra);
+ if (s0 <= 0x7f) {
+ /* one byte character */
+ l = 1;
+ c = s0;
+ } else if (s0 <= (enh_check ? 0xc1 : 0xbf)) {
+ /* invalid character */
+ return 2;
+ } else if (s0 <= 0xdf) {
+ /* two byte character */
+ l = 2;
+ if (ilen < 2) {
+ return 0;
+ }
+ s1 = cpu_ldub_data_ra(env, addr + 1, ra);
+ c = s0 & 0x1f;
+ c = (c << 6) | (s1 & 0x3f);
+ if (enh_check && (s1 & 0xc0) != 0x80) {
+ return 2;
+ }
+ } else if (s0 <= 0xef) {
+ /* three byte character */
+ l = 3;
+ if (ilen < 3) {
+ return 0;
+ }
+ s1 = cpu_ldub_data_ra(env, addr + 1, ra);
+ s2 = cpu_ldub_data_ra(env, addr + 2, ra);
+ c = s0 & 0x0f;
+ c = (c << 6) | (s1 & 0x3f);
+ c = (c << 6) | (s2 & 0x3f);
+ /* Fold the byte-by-byte range descriptions in the PoO into
+ tests against the complete value. It disallows encodings
+ that could be smaller, and the UTF-16 surrogates. */
+ if (enh_check
+ && ((s1 & 0xc0) != 0x80
+ || (s2 & 0xc0) != 0x80
+ || c < 0x1000
+ || (c >= 0xd800 && c <= 0xdfff))) {
+ return 2;
+ }
+ } else if (s0 <= (enh_check ? 0xf4 : 0xf7)) {
+ /* four byte character */
+ l = 4;
+ if (ilen < 4) {
+ return 0;
+ }
+ s1 = cpu_ldub_data_ra(env, addr + 1, ra);
+ s2 = cpu_ldub_data_ra(env, addr + 2, ra);
+ s3 = cpu_ldub_data_ra(env, addr + 3, ra);
+ c = s0 & 0x0f;
+ c = (c << 6) | (s1 & 0x3f);
+ c = (c << 6) | (s2 & 0x3f);
+ c = (c << 6) | (s3 & 0x3f);
+ /* See above. */
+ if (enh_check
+ && ((s1 & 0xc0) != 0x80
+ || (s2 & 0xc0) != 0x80
+ || (s3 & 0xc0) != 0x80
+ || c < 0x010000
+ || c > 0x10ffff)) {
+ return 2;
+ }
+ } else {
+ /* invalid character */
+ return 2;
+ }
+
+ *ochar = c;
+ *olen = l;
+ return -1;
+}
+
+static int decode_utf16(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen)
+{
+ uint16_t s0, s1;
+ uint32_t c, l;
+
+ if (ilen < 2) {
+ return 0;
+ }
+ s0 = cpu_lduw_data_ra(env, addr, ra);
+ if ((s0 & 0xfc00) != 0xd800) {
+ /* one word character */
+ l = 2;
+ c = s0;
+ } else {
+ /* two word character */
+ l = 4;
+ if (ilen < 4) {
+ return 0;
+ }
+ s1 = cpu_lduw_data_ra(env, addr + 2, ra);
+ c = extract32(s0, 6, 4) + 1;
+ c = (c << 6) | (s0 & 0x3f);
+ c = (c << 10) | (s1 & 0x3ff);
+ if (enh_check && (s1 & 0xfc00) != 0xdc00) {
+ /* invalid surrogate character */
+ return 2;
+ }
+ }
+
+ *ochar = c;
+ *olen = l;
+ return -1;
+}
+
+static int decode_utf32(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen)
+{
+ uint32_t c;
+
+ if (ilen < 4) {
+ return 0;
+ }
+ c = cpu_ldl_data_ra(env, addr, ra);
+ if ((c >= 0xd800 && c <= 0xdbff) || c > 0x10ffff) {
+ /* invalid unicode character */
+ return 2;
+ }
+
+ *ochar = c;
+ *olen = 4;
+ return -1;
+}
+
+static int encode_utf8(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ uintptr_t ra, uint32_t c, uint32_t *olen)
+{
+ uint8_t d[4];
+ uint32_t l, i;
+
+ if (c <= 0x7f) {
+ /* one byte character */
+ l = 1;
+ d[0] = c;
+ } else if (c <= 0x7ff) {
+ /* two byte character */
+ l = 2;
+ d[1] = 0x80 | extract32(c, 0, 6);
+ d[0] = 0xc0 | extract32(c, 6, 5);
+ } else if (c <= 0xffff) {
+ /* three byte character */
+ l = 3;
+ d[2] = 0x80 | extract32(c, 0, 6);
+ d[1] = 0x80 | extract32(c, 6, 6);
+ d[0] = 0xe0 | extract32(c, 12, 4);
+ } else {
+ /* four byte character */
+ l = 4;
+ d[3] = 0x80 | extract32(c, 0, 6);
+ d[2] = 0x80 | extract32(c, 6, 6);
+ d[1] = 0x80 | extract32(c, 12, 6);
+ d[0] = 0xf0 | extract32(c, 18, 3);
+ }
+
+ if (ilen < l) {
+ return 1;
+ }
+ for (i = 0; i < l; ++i) {
+ cpu_stb_data_ra(env, addr + i, d[i], ra);
+ }
+
+ *olen = l;
+ return -1;
+}
+
+static int encode_utf16(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ uintptr_t ra, uint32_t c, uint32_t *olen)
+{
+ uint16_t d0, d1;
+
+ if (c <= 0xffff) {
+ /* one word character */
+ if (ilen < 2) {
+ return 1;
+ }
+ cpu_stw_data_ra(env, addr, c, ra);
+ *olen = 2;
+ } else {
+ /* two word character */
+ if (ilen < 4) {
+ return 1;
+ }
+ d1 = 0xdc00 | extract32(c, 0, 10);
+ d0 = 0xd800 | extract32(c, 10, 6);
+ d0 = deposit32(d0, 6, 4, extract32(c, 16, 5) - 1);
+ cpu_stw_data_ra(env, addr + 0, d0, ra);
+ cpu_stw_data_ra(env, addr + 2, d1, ra);
+ *olen = 4;
+ }
+
+ return -1;
+}
+
+static int encode_utf32(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ uintptr_t ra, uint32_t c, uint32_t *olen)
+{
+ if (ilen < 4) {
+ return 1;
+ }
+ cpu_stl_data_ra(env, addr, c, ra);
+ *olen = 4;
+ return -1;
+}
+
+static inline uint32_t convert_unicode(CPUS390XState *env, uint32_t r1,
+ uint32_t r2, uint32_t m3, uintptr_t ra,
+ decode_unicode_fn decode,
+ encode_unicode_fn encode)
+{
+ uint64_t dst = get_address(env, r1);
+ uint64_t dlen = get_length(env, r1 + 1);
+ uint64_t src = get_address(env, r2);
+ uint64_t slen = get_length(env, r2 + 1);
+ bool enh_check = m3 & 1;
+ int cc, i;
+
+ /* 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 256. */
+ for (i = 0; i < 256; ++i) {
+ uint32_t c, ilen, olen;
+
+ cc = decode(env, src, slen, enh_check, ra, &c, &ilen);
+ if (unlikely(cc >= 0)) {
+ break;
+ }
+ cc = encode(env, dst, dlen, ra, c, &olen);
+ if (unlikely(cc >= 0)) {
+ break;
+ }
+
+ src += ilen;
+ slen -= ilen;
+ dst += olen;
+ dlen -= olen;
+ cc = 3;
+ }
+
+ set_address(env, r1, dst);
+ set_length(env, r1 + 1, dlen);
+ set_address(env, r2, src);
+ set_length(env, r2 + 1, slen);
+
+ return cc;
+}
+
+uint32_t HELPER(cu12)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf8, encode_utf16);
+}
+
+uint32_t HELPER(cu14)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf8, encode_utf32);
+}
+
+uint32_t HELPER(cu21)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf16, encode_utf8);
+}
+
+uint32_t HELPER(cu24)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf16, encode_utf32);
+}
+
+uint32_t HELPER(cu41)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf32, encode_utf8);
+}
+
+uint32_t HELPER(cu42)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf32, encode_utf16);
+}
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index e739525..339b31e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2122,6 +2122,48 @@ static ExitStatus op_ct(DisasContext *s, DisasOps *o)
return NO_EXIT;
}
+static ExitStatus op_cuXX(DisasContext *s, DisasOps *o)
+{
+ int m3 = get_field(s->fields, m3);
+ TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+ TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));
+ TCGv_i32 chk;
+
+ if (!s390_has_feat(S390_FEAT_ETF3_ENH)) {
+ m3 = 0;
+ }
+ chk = tcg_const_i32(m3);
+
+ switch (s->insn->data) {
+ case 12:
+ gen_helper_cu12(cc_op, cpu_env, r1, r2, chk);
+ break;
+ case 14:
+ gen_helper_cu14(cc_op, cpu_env, r1, r2, chk);
+ break;
+ case 21:
+ gen_helper_cu21(cc_op, cpu_env, r1, r2, chk);
+ break;
+ case 24:
+ gen_helper_cu24(cc_op, cpu_env, r1, r2, chk);
+ break;
+ case 41:
+ gen_helper_cu41(cc_op, cpu_env, r1, r2, chk);
+ break;
+ case 42:
+ gen_helper_cu42(cc_op, cpu_env, r1, r2, chk);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ tcg_temp_free_i32(r1);
+ tcg_temp_free_i32(r2);
+ tcg_temp_free_i32(chk);
+ set_cc_static(s);
+ return NO_EXIT;
+}
+
#ifndef CONFIG_USER_ONLY
static ExitStatus op_diag(DisasContext *s, DisasOps *o)
{
@@ -5477,6 +5519,7 @@ enum DisasInsnEnum {
#define FAC_EH S390_FEAT_STFLE_49 /* execution-hint */
#define FAC_PPA S390_FEAT_STFLE_49 /* processor-assist */
#define FAC_LZRB S390_FEAT_STFLE_53 /* load-and-zero-rightmost-byte */
+#define FAC_ETF3 S390_FEAT_EXTENDED_TRANSLATION_3
static const DisasInsn insn_info[] = {
#include "insn-data.def"
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 6ac12b8..323a301 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -313,6 +313,19 @@
C(0xb3a1, CDLGBR, RRF_e, FPE, 0, r2_o, f1, 0, cdlgb, 0)
C(0xb3a2, CXLGBR, RRF_e, FPE, 0, r2_o, x1, 0, cxlgb, 0)
+/* CONVERT UTF-8 TO UTF-16 */
+ D(0xb2a7, CU12, RRF_c, Z, 0, 0, 0, 0, cuXX, 0, 12)
+/* CONVERT UTF-8 TO UTF-32 */
+ D(0xb9b0, CU14, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 14)
+/* CONVERT UTF-16 to UTF-8 */
+ D(0xb2a6, CU21, RRF_c, Z, 0, 0, 0, 0, cuXX, 0, 21)
+/* CONVERT UTF-16 to UTF-32 */
+ D(0xb9b1, CU24, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 24)
+/* CONVERT UTF-32 to UTF-8 */
+ D(0xb9b2, CU41, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 41)
+/* CONVERT UTF-32 to UTF-16 */
+ D(0xb9b3, CU42, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 42)
+
/* DIVIDE */
C(0x1d00, DR, RR_a, Z, r1_D32, r2_32s, new_P, r1_P32, divs32, 0)
C(0x5d00, D, RX_a, Z, r1_D32, m2_32s, new_P, r1_P32, divs32, 0)
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns Richard Henderson
@ 2017-07-11 15:18 ` Thomas Huth
2017-07-11 18:23 ` [Qemu-devel] [PATCH v3.5 " Richard Henderson
2017-07-14 21:08 ` [Qemu-devel] [PATCH v3 " Aurelien Jarno
2017-07-14 21:04 ` Aurelien Jarno
1 sibling, 2 replies; 22+ messages in thread
From: Thomas Huth @ 2017-07-11 15:18 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: aurelien, david
On 10.07.2017 22:45, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target/s390x/helper.h | 6 +
> target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
> target/s390x/translate.c | 43 +++++++
> target/s390x/insn-data.def | 13 ++
> 4 files changed, 372 insertions(+)
>
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 23e8d1d..2793cf3 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -107,6 +107,12 @@ DEF_HELPER_2(stfle, i32, env, i64)
> DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
> DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
> DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
> +DEF_HELPER_4(cu12, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu14, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu21, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu24, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
>
> #ifndef CONFIG_USER_ONLY
> DEF_HELPER_3(servc, i32, env, i64, i64)
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 513b402..0b18560 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
[...]
> +static inline uint32_t convert_unicode(CPUS390XState *env, uint32_t r1,
> + uint32_t r2, uint32_t m3, uintptr_t ra,
> + decode_unicode_fn decode,
> + encode_unicode_fn encode)
> +{
> + uint64_t dst = get_address(env, r1);
> + uint64_t dlen = get_length(env, r1 + 1);
> + uint64_t src = get_address(env, r2);
> + uint64_t slen = get_length(env, r2 + 1);
> + bool enh_check = m3 & 1;
> + int cc, i;
According to the PoP:
"The R1 and R2 fields [...] must designate an even-
numbered register; otherwise, a specification excep-
tion is recognized."
I think you should add such a check for even-numbered registers here.
> + /* 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 256. */
> + for (i = 0; i < 256; ++i) {
> + uint32_t c, ilen, olen;
> +
> + cc = decode(env, src, slen, enh_check, ra, &c, &ilen);
> + if (unlikely(cc >= 0)) {
> + break;
> + }
> + cc = encode(env, dst, dlen, ra, c, &olen);
> + if (unlikely(cc >= 0)) {
> + break;
> + }
> +
> + src += ilen;
> + slen -= ilen;
> + dst += olen;
> + dlen -= olen;
> + cc = 3;
> + }
> +
> + set_address(env, r1, dst);
> + set_length(env, r1 + 1, dlen);
> + set_address(env, r2, src);
> + set_length(env, r2 + 1, slen);
> +
> + return cc;
> +}
Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v3.5 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-11 15:18 ` Thomas Huth
@ 2017-07-11 18:23 ` Richard Henderson
2017-07-12 6:46 ` Thomas Huth
2017-07-15 8:20 ` Aurelien Jarno
2017-07-14 21:08 ` [Qemu-devel] [PATCH v3 " Aurelien Jarno
1 sibling, 2 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-11 18:23 UTC (permalink / raw)
To: qemu-devel; +Cc: thuth
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
v3.5: Added even register checks in the translator [thuth].
---
target/s390x/helper.h | 6 +
target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
target/s390x/translate.c | 51 ++++++++
target/s390x/insn-data.def | 13 ++
4 files changed, 380 insertions(+)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 23e8d1d..2793cf3 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -107,6 +107,12 @@ DEF_HELPER_2(stfle, i32, env, i64)
DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
+DEF_HELPER_4(cu12, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu14, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu21, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu24, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
+DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
#ifndef CONFIG_USER_ONLY
DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 513b402..0b18560 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2196,3 +2196,313 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
return cc;
}
+
+/* Decode a Unicode character. A return value < 0 indicates success, storing
+ the UTF-32 result into OCHAR and the input length into OLEN. A return
+ value >= 0 indicates failure, and the CC value to be returned. */
+typedef int (*decode_unicode_fn)(CPUS390XState *env, uint64_t addr,
+ uint64_t ilen, bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen);
+
+/* Encode a Unicode character. A return value < 0 indicates success, storing
+ the bytes into ADDR and the output length into OLEN. A return value >= 0
+ indicates failure, and the CC value to be returned. */
+typedef int (*encode_unicode_fn)(CPUS390XState *env, uint64_t addr,
+ uint64_t ilen, uintptr_t ra, uint32_t c,
+ uint32_t *olen);
+
+static int decode_utf8(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen)
+{
+ uint8_t s0, s1, s2, s3;
+ uint32_t c, l;
+
+ if (ilen < 1) {
+ return 0;
+ }
+ s0 = cpu_ldub_data_ra(env, addr, ra);
+ if (s0 <= 0x7f) {
+ /* one byte character */
+ l = 1;
+ c = s0;
+ } else if (s0 <= (enh_check ? 0xc1 : 0xbf)) {
+ /* invalid character */
+ return 2;
+ } else if (s0 <= 0xdf) {
+ /* two byte character */
+ l = 2;
+ if (ilen < 2) {
+ return 0;
+ }
+ s1 = cpu_ldub_data_ra(env, addr + 1, ra);
+ c = s0 & 0x1f;
+ c = (c << 6) | (s1 & 0x3f);
+ if (enh_check && (s1 & 0xc0) != 0x80) {
+ return 2;
+ }
+ } else if (s0 <= 0xef) {
+ /* three byte character */
+ l = 3;
+ if (ilen < 3) {
+ return 0;
+ }
+ s1 = cpu_ldub_data_ra(env, addr + 1, ra);
+ s2 = cpu_ldub_data_ra(env, addr + 2, ra);
+ c = s0 & 0x0f;
+ c = (c << 6) | (s1 & 0x3f);
+ c = (c << 6) | (s2 & 0x3f);
+ /* Fold the byte-by-byte range descriptions in the PoO into
+ tests against the complete value. It disallows encodings
+ that could be smaller, and the UTF-16 surrogates. */
+ if (enh_check
+ && ((s1 & 0xc0) != 0x80
+ || (s2 & 0xc0) != 0x80
+ || c < 0x1000
+ || (c >= 0xd800 && c <= 0xdfff))) {
+ return 2;
+ }
+ } else if (s0 <= (enh_check ? 0xf4 : 0xf7)) {
+ /* four byte character */
+ l = 4;
+ if (ilen < 4) {
+ return 0;
+ }
+ s1 = cpu_ldub_data_ra(env, addr + 1, ra);
+ s2 = cpu_ldub_data_ra(env, addr + 2, ra);
+ s3 = cpu_ldub_data_ra(env, addr + 3, ra);
+ c = s0 & 0x0f;
+ c = (c << 6) | (s1 & 0x3f);
+ c = (c << 6) | (s2 & 0x3f);
+ c = (c << 6) | (s3 & 0x3f);
+ /* See above. */
+ if (enh_check
+ && ((s1 & 0xc0) != 0x80
+ || (s2 & 0xc0) != 0x80
+ || (s3 & 0xc0) != 0x80
+ || c < 0x010000
+ || c > 0x10ffff)) {
+ return 2;
+ }
+ } else {
+ /* invalid character */
+ return 2;
+ }
+
+ *ochar = c;
+ *olen = l;
+ return -1;
+}
+
+static int decode_utf16(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen)
+{
+ uint16_t s0, s1;
+ uint32_t c, l;
+
+ if (ilen < 2) {
+ return 0;
+ }
+ s0 = cpu_lduw_data_ra(env, addr, ra);
+ if ((s0 & 0xfc00) != 0xd800) {
+ /* one word character */
+ l = 2;
+ c = s0;
+ } else {
+ /* two word character */
+ l = 4;
+ if (ilen < 4) {
+ return 0;
+ }
+ s1 = cpu_lduw_data_ra(env, addr + 2, ra);
+ c = extract32(s0, 6, 4) + 1;
+ c = (c << 6) | (s0 & 0x3f);
+ c = (c << 10) | (s1 & 0x3ff);
+ if (enh_check && (s1 & 0xfc00) != 0xdc00) {
+ /* invalid surrogate character */
+ return 2;
+ }
+ }
+
+ *ochar = c;
+ *olen = l;
+ return -1;
+}
+
+static int decode_utf32(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ bool enh_check, uintptr_t ra,
+ uint32_t *ochar, uint32_t *olen)
+{
+ uint32_t c;
+
+ if (ilen < 4) {
+ return 0;
+ }
+ c = cpu_ldl_data_ra(env, addr, ra);
+ if ((c >= 0xd800 && c <= 0xdbff) || c > 0x10ffff) {
+ /* invalid unicode character */
+ return 2;
+ }
+
+ *ochar = c;
+ *olen = 4;
+ return -1;
+}
+
+static int encode_utf8(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ uintptr_t ra, uint32_t c, uint32_t *olen)
+{
+ uint8_t d[4];
+ uint32_t l, i;
+
+ if (c <= 0x7f) {
+ /* one byte character */
+ l = 1;
+ d[0] = c;
+ } else if (c <= 0x7ff) {
+ /* two byte character */
+ l = 2;
+ d[1] = 0x80 | extract32(c, 0, 6);
+ d[0] = 0xc0 | extract32(c, 6, 5);
+ } else if (c <= 0xffff) {
+ /* three byte character */
+ l = 3;
+ d[2] = 0x80 | extract32(c, 0, 6);
+ d[1] = 0x80 | extract32(c, 6, 6);
+ d[0] = 0xe0 | extract32(c, 12, 4);
+ } else {
+ /* four byte character */
+ l = 4;
+ d[3] = 0x80 | extract32(c, 0, 6);
+ d[2] = 0x80 | extract32(c, 6, 6);
+ d[1] = 0x80 | extract32(c, 12, 6);
+ d[0] = 0xf0 | extract32(c, 18, 3);
+ }
+
+ if (ilen < l) {
+ return 1;
+ }
+ for (i = 0; i < l; ++i) {
+ cpu_stb_data_ra(env, addr + i, d[i], ra);
+ }
+
+ *olen = l;
+ return -1;
+}
+
+static int encode_utf16(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ uintptr_t ra, uint32_t c, uint32_t *olen)
+{
+ uint16_t d0, d1;
+
+ if (c <= 0xffff) {
+ /* one word character */
+ if (ilen < 2) {
+ return 1;
+ }
+ cpu_stw_data_ra(env, addr, c, ra);
+ *olen = 2;
+ } else {
+ /* two word character */
+ if (ilen < 4) {
+ return 1;
+ }
+ d1 = 0xdc00 | extract32(c, 0, 10);
+ d0 = 0xd800 | extract32(c, 10, 6);
+ d0 = deposit32(d0, 6, 4, extract32(c, 16, 5) - 1);
+ cpu_stw_data_ra(env, addr + 0, d0, ra);
+ cpu_stw_data_ra(env, addr + 2, d1, ra);
+ *olen = 4;
+ }
+
+ return -1;
+}
+
+static int encode_utf32(CPUS390XState *env, uint64_t addr, uint64_t ilen,
+ uintptr_t ra, uint32_t c, uint32_t *olen)
+{
+ if (ilen < 4) {
+ return 1;
+ }
+ cpu_stl_data_ra(env, addr, c, ra);
+ *olen = 4;
+ return -1;
+}
+
+static inline uint32_t convert_unicode(CPUS390XState *env, uint32_t r1,
+ uint32_t r2, uint32_t m3, uintptr_t ra,
+ decode_unicode_fn decode,
+ encode_unicode_fn encode)
+{
+ uint64_t dst = get_address(env, r1);
+ uint64_t dlen = get_length(env, r1 + 1);
+ uint64_t src = get_address(env, r2);
+ uint64_t slen = get_length(env, r2 + 1);
+ bool enh_check = m3 & 1;
+ int cc, i;
+
+ /* 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 256. */
+ for (i = 0; i < 256; ++i) {
+ uint32_t c, ilen, olen;
+
+ cc = decode(env, src, slen, enh_check, ra, &c, &ilen);
+ if (unlikely(cc >= 0)) {
+ break;
+ }
+ cc = encode(env, dst, dlen, ra, c, &olen);
+ if (unlikely(cc >= 0)) {
+ break;
+ }
+
+ src += ilen;
+ slen -= ilen;
+ dst += olen;
+ dlen -= olen;
+ cc = 3;
+ }
+
+ set_address(env, r1, dst);
+ set_length(env, r1 + 1, dlen);
+ set_address(env, r2, src);
+ set_length(env, r2 + 1, slen);
+
+ return cc;
+}
+
+uint32_t HELPER(cu12)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf8, encode_utf16);
+}
+
+uint32_t HELPER(cu14)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf8, encode_utf32);
+}
+
+uint32_t HELPER(cu21)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf16, encode_utf8);
+}
+
+uint32_t HELPER(cu24)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf16, encode_utf32);
+}
+
+uint32_t HELPER(cu41)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf32, encode_utf8);
+}
+
+uint32_t HELPER(cu42)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
+{
+ return convert_unicode(env, r1, r2, m3, GETPC(),
+ decode_utf32, encode_utf16);
+}
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index e739525..edfdaf40 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2122,6 +2122,56 @@ static ExitStatus op_ct(DisasContext *s, DisasOps *o)
return NO_EXIT;
}
+static ExitStatus op_cuXX(DisasContext *s, DisasOps *o)
+{
+ int m3 = get_field(s->fields, m3);
+ int r1 = get_field(s->fields, r1);
+ int r2 = get_field(s->fields, r2);
+ TCGv_i32 tr1, tr2, chk;
+
+ /* R1 and R2 must both be even. */
+ if ((r1 | r2) & 1) {
+ gen_program_exception(s, PGM_SPECIFICATION);
+ return EXIT_NORETURN;
+ }
+ if (!s390_has_feat(S390_FEAT_ETF3_ENH)) {
+ m3 = 0;
+ }
+
+ tr1 = tcg_const_i32(r1);
+ tr2 = tcg_const_i32(r2);
+ chk = tcg_const_i32(m3);
+
+ switch (s->insn->data) {
+ case 12:
+ gen_helper_cu12(cc_op, cpu_env, tr1, tr2, chk);
+ break;
+ case 14:
+ gen_helper_cu14(cc_op, cpu_env, tr1, tr2, chk);
+ break;
+ case 21:
+ gen_helper_cu21(cc_op, cpu_env, tr1, tr2, chk);
+ break;
+ case 24:
+ gen_helper_cu24(cc_op, cpu_env, tr1, tr2, chk);
+ break;
+ case 41:
+ gen_helper_cu41(cc_op, cpu_env, tr1, tr2, chk);
+ break;
+ case 42:
+ gen_helper_cu42(cc_op, cpu_env, tr1, tr2, chk);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ tcg_temp_free_i32(tr1);
+ tcg_temp_free_i32(tr2);
+ tcg_temp_free_i32(chk);
+ set_cc_static(s);
+ return NO_EXIT;
+}
+
#ifndef CONFIG_USER_ONLY
static ExitStatus op_diag(DisasContext *s, DisasOps *o)
{
@@ -5477,6 +5527,7 @@ enum DisasInsnEnum {
#define FAC_EH S390_FEAT_STFLE_49 /* execution-hint */
#define FAC_PPA S390_FEAT_STFLE_49 /* processor-assist */
#define FAC_LZRB S390_FEAT_STFLE_53 /* load-and-zero-rightmost-byte */
+#define FAC_ETF3 S390_FEAT_EXTENDED_TRANSLATION_3
static const DisasInsn insn_info[] = {
#include "insn-data.def"
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 6ac12b8..323a301 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -313,6 +313,19 @@
C(0xb3a1, CDLGBR, RRF_e, FPE, 0, r2_o, f1, 0, cdlgb, 0)
C(0xb3a2, CXLGBR, RRF_e, FPE, 0, r2_o, x1, 0, cxlgb, 0)
+/* CONVERT UTF-8 TO UTF-16 */
+ D(0xb2a7, CU12, RRF_c, Z, 0, 0, 0, 0, cuXX, 0, 12)
+/* CONVERT UTF-8 TO UTF-32 */
+ D(0xb9b0, CU14, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 14)
+/* CONVERT UTF-16 to UTF-8 */
+ D(0xb2a6, CU21, RRF_c, Z, 0, 0, 0, 0, cuXX, 0, 21)
+/* CONVERT UTF-16 to UTF-32 */
+ D(0xb9b1, CU24, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 24)
+/* CONVERT UTF-32 to UTF-8 */
+ D(0xb9b2, CU41, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 41)
+/* CONVERT UTF-32 to UTF-16 */
+ D(0xb9b3, CU42, RRF_c, ETF3, 0, 0, 0, 0, cuXX, 0, 42)
+
/* DIVIDE */
C(0x1d00, DR, RR_a, Z, r1_D32, r2_32s, new_P, r1_P32, divs32, 0)
C(0x5d00, D, RX_a, Z, r1_D32, m2_32s, new_P, r1_P32, divs32, 0)
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3.5 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-11 18:23 ` [Qemu-devel] [PATCH v3.5 " Richard Henderson
@ 2017-07-12 6:46 ` Thomas Huth
2017-07-12 19:55 ` Richard Henderson
2017-07-15 8:20 ` Aurelien Jarno
1 sibling, 1 reply; 22+ messages in thread
From: Thomas Huth @ 2017-07-12 6:46 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 11.07.2017 20:23, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> v3.5: Added even register checks in the translator [thuth].
> ---
> target/s390x/helper.h | 6 +
> target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
> target/s390x/translate.c | 51 ++++++++
> target/s390x/insn-data.def | 13 ++
> 4 files changed, 380 insertions(+)
>
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 23e8d1d..2793cf3 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -107,6 +107,12 @@ DEF_HELPER_2(stfle, i32, env, i64)
> DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
> DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
> DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
> +DEF_HELPER_4(cu12, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu14, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu21, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu24, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
> +DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
>
> #ifndef CONFIG_USER_ONLY
> DEF_HELPER_3(servc, i32, env, i64, i64)
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 513b402..0b18560 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -2196,3 +2196,313 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
>
> return cc;
> }
> +
> +/* Decode a Unicode character. A return value < 0 indicates success, storing
> + the UTF-32 result into OCHAR and the input length into OLEN. A return
> + value >= 0 indicates failure, and the CC value to be returned. */
> +typedef int (*decode_unicode_fn)(CPUS390XState *env, uint64_t addr,
> + uint64_t ilen, bool enh_check, uintptr_t ra,
> + uint32_t *ochar, uint32_t *olen);
> +
> +/* Encode a Unicode character. A return value < 0 indicates success, storing
> + the bytes into ADDR and the output length into OLEN. A return value >= 0
> + indicates failure, and the CC value to be returned. */
> +typedef int (*encode_unicode_fn)(CPUS390XState *env, uint64_t addr,
> + uint64_t ilen, uintptr_t ra, uint32_t c,
> + uint32_t *olen);
> +
> +static int decode_utf8(CPUS390XState *env, uint64_t addr, uint64_t ilen,
> + bool enh_check, uintptr_t ra,
> + uint32_t *ochar, uint32_t *olen)
> +{
> + uint8_t s0, s1, s2, s3;
> + uint32_t c, l;
> +
> + if (ilen < 1) {
> + return 0;
> + }
> + s0 = cpu_ldub_data_ra(env, addr, ra);
> + if (s0 <= 0x7f) {
> + /* one byte character */
> + l = 1;
> + c = s0;
> + } else if (s0 <= (enh_check ? 0xc1 : 0xbf)) {
> + /* invalid character */
> + return 2;
> + } else if (s0 <= 0xdf) {
> + /* two byte character */
> + l = 2;
> + if (ilen < 2) {
> + return 0;
> + }
> + s1 = cpu_ldub_data_ra(env, addr + 1, ra);
> + c = s0 & 0x1f;
> + c = (c << 6) | (s1 & 0x3f);
> + if (enh_check && (s1 & 0xc0) != 0x80) {
> + return 2;
> + }
> + } else if (s0 <= 0xef) {
> + /* three byte character */
> + l = 3;
> + if (ilen < 3) {
> + return 0;
> + }
> + s1 = cpu_ldub_data_ra(env, addr + 1, ra);
> + s2 = cpu_ldub_data_ra(env, addr + 2, ra);
> + c = s0 & 0x0f;
> + c = (c << 6) | (s1 & 0x3f);
> + c = (c << 6) | (s2 & 0x3f);
> + /* Fold the byte-by-byte range descriptions in the PoO into
> + tests against the complete value. It disallows encodings
> + that could be smaller, and the UTF-16 surrogates. */
> + if (enh_check
> + && ((s1 & 0xc0) != 0x80
> + || (s2 & 0xc0) != 0x80
> + || c < 0x1000
> + || (c >= 0xd800 && c <= 0xdfff))) {
> + return 2;
> + }
> + } else if (s0 <= (enh_check ? 0xf4 : 0xf7)) {
> + /* four byte character */
> + l = 4;
> + if (ilen < 4) {
> + return 0;
> + }
> + s1 = cpu_ldub_data_ra(env, addr + 1, ra);
> + s2 = cpu_ldub_data_ra(env, addr + 2, ra);
> + s3 = cpu_ldub_data_ra(env, addr + 3, ra);
> + c = s0 & 0x0f;
I think you could also use 0x07 instead of 0x0f here. Shouldn't matter
much due to the s0 <= 0xf7 check above, though.
> + c = (c << 6) | (s1 & 0x3f);
> + c = (c << 6) | (s2 & 0x3f);
> + c = (c << 6) | (s3 & 0x3f);
> + /* See above. */
> + if (enh_check
> + && ((s1 & 0xc0) != 0x80
> + || (s2 & 0xc0) != 0x80
> + || (s3 & 0xc0) != 0x80
> + || c < 0x010000
> + || c > 0x10ffff)) {
> + return 2;
> + }
> + } else {
> + /* invalid character */
> + return 2;
> + }
> +
> + *ochar = c;
> + *olen = l;
> + return -1;
> +}
[...]
Patch looks fine to me now!
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3.5 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-12 6:46 ` Thomas Huth
@ 2017-07-12 19:55 ` Richard Henderson
0 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-12 19:55 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
On 07/11/2017 08:46 PM, Thomas Huth wrote:
>> + s1 = cpu_ldub_data_ra(env, addr + 1, ra);
>> + s2 = cpu_ldub_data_ra(env, addr + 2, ra);
>> + c = s0 & 0x0f;
>> + c = (c << 6) | (s1 & 0x3f);
>> + c = (c << 6) | (s2 & 0x3f);
>> + /* Fold the byte-by-byte range descriptions in the PoO into
>> + tests against the complete value. It disallows encodings
>> + that could be smaller, and the UTF-16 surrogates. */
>> + if (enh_check
>> + && ((s1 & 0xc0) != 0x80
>> + || (s2 & 0xc0) != 0x80
>> + || c < 0x1000
>> + || (c >= 0xd800 && c <= 0xdfff))) {
>> + return 2;
>> + }
>> + } else if (s0 <= (enh_check ? 0xf4 : 0xf7)) {
>> + /* four byte character */
>> + l = 4;
>> + if (ilen < 4) {
>> + return 0;
>> + }
>> + s1 = cpu_ldub_data_ra(env, addr + 1, ra);
>> + s2 = cpu_ldub_data_ra(env, addr + 2, ra);
>> + s3 = cpu_ldub_data_ra(env, addr + 3, ra);
>> + c = s0 & 0x0f;
> I think you could also use 0x07 instead of 0x0f here. Shouldn't matter
> much due to the s0 <= 0xf7 check above, though.
>
You're right that it doesn't matter due to the check, but fixed anyway. It
makes the pattern wrt 2, 3, and 4 byte encodings more apparent.
r~
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3.5 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-11 18:23 ` [Qemu-devel] [PATCH v3.5 " Richard Henderson
2017-07-12 6:46 ` Thomas Huth
@ 2017-07-15 8:20 ` Aurelien Jarno
1 sibling, 0 replies; 22+ messages in thread
From: Aurelien Jarno @ 2017-07-15 8:20 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, thuth
On 2017-07-11 08:23, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> v3.5: Added even register checks in the translator [thuth].
> ---
> target/s390x/helper.h | 6 +
> target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
> target/s390x/translate.c | 51 ++++++++
> target/s390x/insn-data.def | 13 ++
> 4 files changed, 380 insertions(+)
>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-11 15:18 ` Thomas Huth
2017-07-11 18:23 ` [Qemu-devel] [PATCH v3.5 " Richard Henderson
@ 2017-07-14 21:08 ` Aurelien Jarno
2017-07-15 0:23 ` Richard Henderson
1 sibling, 1 reply; 22+ messages in thread
From: Aurelien Jarno @ 2017-07-14 21:08 UTC (permalink / raw)
To: Thomas Huth; +Cc: Richard Henderson, qemu-devel, david
On 2017-07-11 17:18, Thomas Huth wrote:
> On 10.07.2017 22:45, Richard Henderson wrote:
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > ---
> > target/s390x/helper.h | 6 +
> > target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
> > target/s390x/translate.c | 43 +++++++
> > target/s390x/insn-data.def | 13 ++
> > 4 files changed, 372 insertions(+)
> >
> > diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> > index 23e8d1d..2793cf3 100644
> > --- a/target/s390x/helper.h
> > +++ b/target/s390x/helper.h
> > @@ -107,6 +107,12 @@ DEF_HELPER_2(stfle, i32, env, i64)
> > DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
> > DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
> > DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
> > +DEF_HELPER_4(cu12, i32, env, i32, i32, i32)
> > +DEF_HELPER_4(cu14, i32, env, i32, i32, i32)
> > +DEF_HELPER_4(cu21, i32, env, i32, i32, i32)
> > +DEF_HELPER_4(cu24, i32, env, i32, i32, i32)
> > +DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
> > +DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
> >
> > #ifndef CONFIG_USER_ONLY
> > DEF_HELPER_3(servc, i32, env, i64, i64)
> > diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> > index 513b402..0b18560 100644
> > --- a/target/s390x/mem_helper.c
> > +++ b/target/s390x/mem_helper.c
> [...]
> > +static inline uint32_t convert_unicode(CPUS390XState *env, uint32_t r1,
> > + uint32_t r2, uint32_t m3, uintptr_t ra,
> > + decode_unicode_fn decode,
> > + encode_unicode_fn encode)
> > +{
> > + uint64_t dst = get_address(env, r1);
> > + uint64_t dlen = get_length(env, r1 + 1);
> > + uint64_t src = get_address(env, r2);
> > + uint64_t slen = get_length(env, r2 + 1);
> > + bool enh_check = m3 & 1;
> > + int cc, i;
>
> According to the PoP:
>
> "The R1 and R2 fields [...] must designate an even-
> numbered register; otherwise, a specification excep-
> tion is recognized."
>
> I think you should add such a check for even-numbered registers here.
Actually it should not be done here, but at translation time in
translate.c.
There are a few places where the register number is checked to be even
and later loaded into a temp. I guess that can be replaced by generators
instead?
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-14 21:08 ` [Qemu-devel] [PATCH v3 " Aurelien Jarno
@ 2017-07-15 0:23 ` Richard Henderson
2017-07-15 8:25 ` Aurelien Jarno
0 siblings, 1 reply; 22+ messages in thread
From: Richard Henderson @ 2017-07-15 0:23 UTC (permalink / raw)
To: Aurelien Jarno, Thomas Huth; +Cc: qemu-devel, david
On 07/14/2017 11:08 AM, Aurelien Jarno wrote:
> On 2017-07-11 17:18, Thomas Huth wrote:
>> On 10.07.2017 22:45, Richard Henderson wrote:
>>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>>> ---
>>> target/s390x/helper.h | 6 +
>>> target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
>>> target/s390x/translate.c | 43 +++++++
>>> target/s390x/insn-data.def | 13 ++
>>> 4 files changed, 372 insertions(+)
>>>
>>> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
>>> index 23e8d1d..2793cf3 100644
>>> --- a/target/s390x/helper.h
>>> +++ b/target/s390x/helper.h
>>> @@ -107,6 +107,12 @@ DEF_HELPER_2(stfle, i32, env, i64)
>>> DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
>>> DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
>>> DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
>>> +DEF_HELPER_4(cu12, i32, env, i32, i32, i32)
>>> +DEF_HELPER_4(cu14, i32, env, i32, i32, i32)
>>> +DEF_HELPER_4(cu21, i32, env, i32, i32, i32)
>>> +DEF_HELPER_4(cu24, i32, env, i32, i32, i32)
>>> +DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
>>> +DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
>>>
>>> #ifndef CONFIG_USER_ONLY
>>> DEF_HELPER_3(servc, i32, env, i64, i64)
>>> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
>>> index 513b402..0b18560 100644
>>> --- a/target/s390x/mem_helper.c
>>> +++ b/target/s390x/mem_helper.c
>> [...]
>>> +static inline uint32_t convert_unicode(CPUS390XState *env, uint32_t r1,
>>> + uint32_t r2, uint32_t m3, uintptr_t ra,
>>> + decode_unicode_fn decode,
>>> + encode_unicode_fn encode)
>>> +{
>>> + uint64_t dst = get_address(env, r1);
>>> + uint64_t dlen = get_length(env, r1 + 1);
>>> + uint64_t src = get_address(env, r2);
>>> + uint64_t slen = get_length(env, r2 + 1);
>>> + bool enh_check = m3 & 1;
>>> + int cc, i;
>>
>> According to the PoP:
>>
>> "The R1 and R2 fields [...] must designate an even-
>> numbered register; otherwise, a specification excep-
>> tion is recognized."
>>
>> I think you should add such a check for even-numbered registers here.
>
> Actually it should not be done here, but at translation time in
> translate.c.
>
> There are a few places where the register number is checked to be even
> and later loaded into a temp. I guess that can be replaced by generators
> instead?
Yes, that's done in a v3.5 patch that's a part of this thread.
r~
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-15 0:23 ` Richard Henderson
@ 2017-07-15 8:25 ` Aurelien Jarno
2017-07-15 8:27 ` Richard Henderson
0 siblings, 1 reply; 22+ messages in thread
From: Aurelien Jarno @ 2017-07-15 8:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: Thomas Huth, qemu-devel, david
On 2017-07-14 14:23, Richard Henderson wrote:
> On 07/14/2017 11:08 AM, Aurelien Jarno wrote:
> > On 2017-07-11 17:18, Thomas Huth wrote:
> > > On 10.07.2017 22:45, Richard Henderson wrote:
> > > > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > > > ---
> > > > target/s390x/helper.h | 6 +
> > > > target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
> > > > target/s390x/translate.c | 43 +++++++
> > > > target/s390x/insn-data.def | 13 ++
> > > > 4 files changed, 372 insertions(+)
> > > >
> > > > diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> > > > index 23e8d1d..2793cf3 100644
> > > > --- a/target/s390x/helper.h
> > > > +++ b/target/s390x/helper.h
> > > > @@ -107,6 +107,12 @@ DEF_HELPER_2(stfle, i32, env, i64)
> > > > DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
> > > > DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
> > > > DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
> > > > +DEF_HELPER_4(cu12, i32, env, i32, i32, i32)
> > > > +DEF_HELPER_4(cu14, i32, env, i32, i32, i32)
> > > > +DEF_HELPER_4(cu21, i32, env, i32, i32, i32)
> > > > +DEF_HELPER_4(cu24, i32, env, i32, i32, i32)
> > > > +DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
> > > > +DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
> > > > #ifndef CONFIG_USER_ONLY
> > > > DEF_HELPER_3(servc, i32, env, i64, i64)
> > > > diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> > > > index 513b402..0b18560 100644
> > > > --- a/target/s390x/mem_helper.c
> > > > +++ b/target/s390x/mem_helper.c
> > > [...]
> > > > +static inline uint32_t convert_unicode(CPUS390XState *env, uint32_t r1,
> > > > + uint32_t r2, uint32_t m3, uintptr_t ra,
> > > > + decode_unicode_fn decode,
> > > > + encode_unicode_fn encode)
> > > > +{
> > > > + uint64_t dst = get_address(env, r1);
> > > > + uint64_t dlen = get_length(env, r1 + 1);
> > > > + uint64_t src = get_address(env, r2);
> > > > + uint64_t slen = get_length(env, r2 + 1);
> > > > + bool enh_check = m3 & 1;
> > > > + int cc, i;
> > >
> > > According to the PoP:
> > >
> > > "The R1 and R2 fields [...] must designate an even-
> > > numbered register; otherwise, a specification excep-
> > > tion is recognized."
> > >
> > > I think you should add such a check for even-numbered registers here.
> >
> > Actually it should not be done here, but at translation time in
> > translate.c.
> >
> > There are a few places where the register number is checked to be even
> > and later loaded into a temp. I guess that can be replaced by generators
> > instead?
>
> Yes, that's done in a v3.5 patch that's a part of this thread.
Sorry I have some backlogs in all those mails, and I haven't seen it.
This is correct now.
That said I still wonder if we can get generators like that:
| static void in1_r1n(DisasContext *s, DisasFields *f, DisasOps *o)
| {
| int r1 = get_field(s->fields, r1);
| o->in1 = tcg_const_i32(r1);
|
| #define SPEC_in1_r1n 0
and
| static void in1_r1n_even(DisasContext *s, DisasFields *f, DisasOps *o)
| {
| int r1 = get_field(s->fields, r1);
| o->in1 = tcg_const_i32(r1);
|
| #define SPEC_in1_r1n_even SPEC_r1_even
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-15 8:25 ` Aurelien Jarno
@ 2017-07-15 8:27 ` Richard Henderson
0 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-15 8:27 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Thomas Huth, qemu-devel, david
On 07/14/2017 10:25 PM, Aurelien Jarno wrote:
> That said I still wonder if we can get generators like that:
>
> | static void in1_r1n(DisasContext *s, DisasFields *f, DisasOps *o)
> | {
> | int r1 = get_field(s->fields, r1);
> | o->in1 = tcg_const_i32(r1);
> |
> | #define SPEC_in1_r1n 0
>
> and
>
> | static void in1_r1n_even(DisasContext *s, DisasFields *f, DisasOps *o)
> | {
> | int r1 = get_field(s->fields, r1);
> | o->in1 = tcg_const_i32(r1);
> |
> | #define SPEC_in1_r1n_even SPEC_r1_even
Well, not in to o->in1, obviously, since that's TCGv_i32 not TCGv_i64.
I suppose we could add something of the sort though.
r~
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns Richard Henderson
2017-07-11 15:18 ` Thomas Huth
@ 2017-07-14 21:04 ` Aurelien Jarno
1 sibling, 0 replies; 22+ messages in thread
From: Aurelien Jarno @ 2017-07-14 21:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, david
On 2017-07-10 10:45, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target/s390x/helper.h | 6 +
> target/s390x/mem_helper.c | 310 +++++++++++++++++++++++++++++++++++++++++++++
> target/s390x/translate.c | 43 +++++++
> target/s390x/insn-data.def | 13 ++
> 4 files changed, 372 insertions(+)
>
Besides the check for even r1 and r3, this now looks good.
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 3/8] target/s390x: Tidy SRST
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 1/8] target/s390x: Implement CSST Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 4/8] target/s390x: Implement SRSTU Richard Henderson
` (4 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
Since we require all registers saved on input, read R0 from ENV instead
of passing it manually. Recognize the specification exception when R0
contains incorrect data. Keep high bits of result registers unmodified
when in 31 or 24-bit mode.
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/helper.h | 2 +-
target/s390x/mem_helper.c | 25 ++++++++++++++-----------
target/s390x/translate.c | 9 +++++++--
target/s390x/insn-data.def | 2 +-
4 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 2793cf3..a2e5b9b 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -12,7 +12,7 @@ DEF_HELPER_FLAGS_3(divs32, TCG_CALL_NO_WG, s64, env, s64, s64)
DEF_HELPER_FLAGS_3(divu32, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(divs64, TCG_CALL_NO_WG, s64, env, s64, s64)
DEF_HELPER_FLAGS_4(divu64, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
-DEF_HELPER_4(srst, i64, env, i64, i64, i64)
+DEF_HELPER_3(srst, void, env, i32, i32)
DEF_HELPER_4(clst, i64, env, i64, i64, i64)
DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 0b18560..74b48aa 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -538,18 +538,21 @@ static inline void set_length(CPUS390XState *env, int reg, uint64_t length)
}
/* search string (c is byte to search, r2 is string, r1 end of string) */
-uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
- uint64_t str)
+void HELPER(srst)(CPUS390XState *env, uint32_t r1, uint32_t r2)
{
uintptr_t ra = GETPC();
+ uint64_t end, str;
uint32_t len;
- uint8_t v, c = r0;
+ uint8_t v, c = env->regs[0];
- str = wrap_address(env, str);
- end = wrap_address(env, end);
+ /* Bits 32-55 must contain all 0. */
+ if (env->regs[0] & 0xffffff00u) {
+ cpu_restore_state(ENV_GET_CPU(env), ra);
+ program_interrupt(env, PGM_SPECIFICATION, 6);
+ }
- /* Assume for now that R2 is unmodified. */
- env->retxl = str;
+ str = get_address(env, r2);
+ end = get_address(env, r1);
/* 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. */
@@ -557,20 +560,20 @@ uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
if (str + len == end) {
/* Character not found. R1 & R2 are unmodified. */
env->cc_op = 2;
- return end;
+ return;
}
v = cpu_ldub_data_ra(env, str + len, ra);
if (v == c) {
/* Character found. Set R1 to the location; R2 is unmodified. */
env->cc_op = 1;
- return str + len;
+ set_address(env, r1, str + len);
+ return;
}
}
/* CPU-determined bytes processed. Advance R2 to next byte to process. */
- env->retxl = str + len;
env->cc_op = 3;
- return end;
+ set_address(env, r2, str + len);
}
/* unsigned string compare (c is string terminator) */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 339b31e..ba22e3d 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4279,9 +4279,14 @@ static ExitStatus op_stpq(DisasContext *s, DisasOps *o)
static ExitStatus op_srst(DisasContext *s, DisasOps *o)
{
- gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
+ TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+ TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));
+
+ gen_helper_srst(cpu_env, r1, r2);
+
+ tcg_temp_free_i32(r1);
+ tcg_temp_free_i32(r2);
set_cc_static(s);
- return_low128(o->in2);
return NO_EXIT;
}
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 323a301..bc6ff01 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -736,7 +736,7 @@
C(0xec57, RXSBG, RIE_f, GIE, 0, r2, r1, 0, rosbg, 0)
/* SEARCH STRING */
- C(0xb25e, SRST, RRE, Z, r1_o, r2_o, 0, 0, srst, 0)
+ C(0xb25e, SRST, RRE, Z, 0, 0, 0, 0, srst, 0)
/* SET ACCESS */
C(0xb24e, SAR, RRE, Z, 0, r2_o, 0, 0, sar, 0)
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v3 4/8] target/s390x: Implement SRSTU
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
` (2 preceding siblings ...)
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 3/8] target/s390x: Tidy SRST Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 5/8] target/s390x: Implement TRTR Richard Henderson
` (3 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/helper.h | 1 +
target/s390x/mem_helper.c | 41 +++++++++++++++++++++++++++++++++++++++++
target/s390x/translate.c | 13 +++++++++++++
target/s390x/insn-data.def | 2 ++
4 files changed, 57 insertions(+)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index a2e5b9b..32314e0 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -13,6 +13,7 @@ DEF_HELPER_FLAGS_3(divu32, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(divs64, TCG_CALL_NO_WG, s64, env, s64, s64)
DEF_HELPER_FLAGS_4(divu64, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
DEF_HELPER_3(srst, void, env, i32, i32)
+DEF_HELPER_3(srstu, void, env, i32, i32)
DEF_HELPER_4(clst, i64, env, i64, i64, i64)
DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 74b48aa..e3db68d 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -576,6 +576,47 @@ void HELPER(srst)(CPUS390XState *env, uint32_t r1, uint32_t r2)
set_address(env, r2, str + len);
}
+void HELPER(srstu)(CPUS390XState *env, uint32_t r1, uint32_t r2)
+{
+ uintptr_t ra = GETPC();
+ uint32_t len;
+ uint16_t v, c = env->regs[0];
+ uint64_t end, str, adj_end;
+
+ /* Bits 32-47 of R0 must be zero. */
+ if (env->regs[0] & 0xffff0000u) {
+ cpu_restore_state(ENV_GET_CPU(env), ra);
+ program_interrupt(env, PGM_SPECIFICATION, 6);
+ }
+
+ str = get_address(env, r2);
+ end = get_address(env, r1);
+
+ /* If the LSB of the two addresses differ, use one extra byte. */
+ adj_end = end + ((str ^ end) & 1);
+
+ /* 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. */
+ for (len = 0; len < 0x2000; len += 2) {
+ if (str + len == adj_end) {
+ /* End of input found. */
+ env->cc_op = 2;
+ return;
+ }
+ v = cpu_lduw_data_ra(env, str + len, ra);
+ if (v == c) {
+ /* Character found. Set R1 to the location; R2 is unmodified. */
+ env->cc_op = 1;
+ set_address(env, r1, str + len);
+ return;
+ }
+ }
+
+ /* CPU-determined bytes processed. Advance R2 to next byte to process. */
+ env->cc_op = 3;
+ set_address(env, r2, str + len);
+}
+
/* unsigned string compare (c is string terminator) */
uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, uint64_t s1, uint64_t s2)
{
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ba22e3d..57e6909 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4290,6 +4290,19 @@ static ExitStatus op_srst(DisasContext *s, DisasOps *o)
return NO_EXIT;
}
+static ExitStatus op_srstu(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));
+
+ gen_helper_srstu(cpu_env, r1, r2);
+
+ tcg_temp_free_i32(r1);
+ tcg_temp_free_i32(r2);
+ set_cc_static(s);
+ return NO_EXIT;
+}
+
static ExitStatus op_sub(DisasContext *s, DisasOps *o)
{
tcg_gen_sub_i64(o->out, o->in1, o->in2);
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index bc6ff01..1d34df03 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -737,6 +737,8 @@
/* SEARCH STRING */
C(0xb25e, SRST, RRE, Z, 0, 0, 0, 0, srst, 0)
+/* SEARCH STRING UNICODE */
+ C(0xb9be, SRSTU, RRE, ETF3, 0, 0, 0, 0, srstu, 0)
/* SET ACCESS */
C(0xb24e, SAR, RRE, Z, 0, r2_o, 0, 0, sar, 0)
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v3 5/8] target/s390x: Implement TRTR
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
` (3 preceding siblings ...)
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 4/8] target/s390x: Implement SRSTU Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 6/8] target/s390x: Mark ETF3 and ETF3_ENH facilities as available Richard Henderson
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
Drop TRT from the set of insns handled internally by EXECUTE.
It's more important to adjust the existing helper to handle
both TRT and TRTR.
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/helper.h | 1 +
target/s390x/mem_helper.c | 20 +++++++++++++-------
target/s390x/translate.c | 9 +++++++++
target/s390x/insn-data.def | 2 ++
4 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 32314e0..4b02907 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -97,6 +97,7 @@ DEF_HELPER_FLAGS_3(tp, TCG_CALL_NO_WG, i32, env, i64, i32)
DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
DEF_HELPER_4(tre, i64, env, i64, i64, i64)
DEF_HELPER_4(trt, i32, env, i32, i64, i64)
+DEF_HELPER_4(trtr, i32, env, i32, i64, i64)
DEF_HELPER_5(trXX, i32, env, i32, i32, i32, i32)
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/mem_helper.c b/target/s390x/mem_helper.c
index e3db68d..b9d0477 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1277,17 +1277,18 @@ uint64_t HELPER(tre)(CPUS390XState *env, uint64_t array,
return array + i;
}
-static uint32_t do_helper_trt(CPUS390XState *env, uint32_t len, uint64_t array,
- uint64_t trans, uintptr_t ra)
+static inline uint32_t do_helper_trt(CPUS390XState *env, int len,
+ uint64_t array, uint64_t trans,
+ int inc, uintptr_t ra)
{
- uint32_t i;
+ int i;
for (i = 0; i <= len; i++) {
- uint8_t byte = cpu_ldub_data_ra(env, array + i, ra);
+ uint8_t byte = cpu_ldub_data_ra(env, array + i * inc, ra);
uint8_t sbyte = cpu_ldub_data_ra(env, trans + byte, ra);
if (sbyte != 0) {
- set_address(env, 1, array + i);
+ set_address(env, 1, array + i * inc);
env->regs[2] = deposit64(env->regs[2], 0, 8, sbyte);
return (i == len) ? 2 : 1;
}
@@ -1299,7 +1300,13 @@ static uint32_t do_helper_trt(CPUS390XState *env, uint32_t len, uint64_t array,
uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array,
uint64_t trans)
{
- return do_helper_trt(env, len, array, trans, GETPC());
+ return do_helper_trt(env, len, array, trans, 1, GETPC());
+}
+
+uint32_t HELPER(trtr)(CPUS390XState *env, uint32_t len, uint64_t array,
+ uint64_t trans)
+{
+ return do_helper_trt(env, len, array, trans, -1, GETPC());
}
/* Translate one/two to one/two */
@@ -2119,7 +2126,6 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
[0x6] = do_helper_oc,
[0x7] = do_helper_xc,
[0xc] = do_helper_tr,
- [0xd] = do_helper_trt,
};
dx_helper helper = dx[opc & 0xf];
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 57e6909..f94ba7c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4439,6 +4439,15 @@ static ExitStatus op_trt(DisasContext *s, DisasOps *o)
return NO_EXIT;
}
+static ExitStatus op_trtr(DisasContext *s, DisasOps *o)
+{
+ TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+ gen_helper_trtr(cc_op, cpu_env, l, o->addr1, o->in2);
+ tcg_temp_free_i32(l);
+ set_cc_static(s);
+ return NO_EXIT;
+}
+
static ExitStatus op_trXX(DisasContext *s, DisasOps *o)
{
TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 1d34df03..ad84c74 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -916,6 +916,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 AND TEST REVERSE */
+ C(0xd000, TRTR, SS_a, ETF3, la1, a2, 0, 0, trtr, 0)
/* TRANSLATE EXTENDED */
C(0xb2a5, TRE, RRE, Z, 0, r2, r1_P, 0, tre, 0)
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v3 6/8] target/s390x: Mark ETF3 and ETF3_ENH facilities as available
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
` (4 preceding siblings ...)
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 5/8] target/s390x: Implement TRTR Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 7/8] target/s390x: Allow to enable "idtes" feature for TCG Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 8/8] target/s390x: Fix risbg handling Richard Henderson
7 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/cpu_models.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 2c86b24..a4afdd9 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -731,11 +731,13 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
S390_FEAT_STFLE,
S390_FEAT_EXTENDED_IMMEDIATE,
S390_FEAT_EXTENDED_TRANSLATION_2,
+ S390_FEAT_EXTENDED_TRANSLATION_3,
S390_FEAT_LONG_DISPLACEMENT,
S390_FEAT_LONG_DISPLACEMENT_FAST,
S390_FEAT_ETF2_ENH,
S390_FEAT_STORE_CLOCK_FAST,
S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
+ S390_FEAT_ETF3_ENH,
S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v3 7/8] target/s390x: Allow to enable "idtes" feature for TCG
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
` (5 preceding siblings ...)
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 6/8] target/s390x: Mark ETF3 and ETF3_ENH facilities as available Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 8/8] target/s390x: Fix risbg handling Richard Henderson
7 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
From: David Hildenbrand <david@redhat.com>
STFL bit 4 and 5 are just indications to the guest, which TLB entries an
IDTE call will clear. These are performance indicators for the guest.
STFL bit 4:
INVALIDATE DAT TABLE ENTRY (IDTE) performs
the invalidation-and-clearing operation by
selectively clearing TLB segment-table entries
when a segment-table entry or entries are
invalidated. IDTE also performs the clearing-by-
ASCE operation. Unless bit 4 is one, IDTE simply
purges all TLBs. Bit 3 is one if bit 4 is one.
We can simply set STFL bit 4 ("idtes") and still purge the complete TLB.
Purging more than advertised is never bad. E.g. Linux doesn't even care
about this bit. We can optimized this later.
This is helpful, as the z9 base model contains this facility.
STFL bit 5 (clearing TLB region-table-entries) was never implemented on
real HW, therefore we can simply ignore it for now.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20170627161032.5014-1-david@redhat.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/cpu_models.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index a4afdd9..998bb96 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -728,6 +728,7 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
{
static const int feats[] = {
S390_FEAT_DAT_ENH,
+ S390_FEAT_IDTE_SEGMENT,
S390_FEAT_STFLE,
S390_FEAT_EXTENDED_IMMEDIATE,
S390_FEAT_EXTENDED_TRANSLATION_2,
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v3 8/8] target/s390x: Fix risbg handling
2017-07-10 20:45 [Qemu-devel] [PATCH v3 0/8] target/s390x tcg improvements Richard Henderson
` (6 preceding siblings ...)
2017-07-10 20:45 ` [Qemu-devel] [PATCH v3 7/8] target/s390x: Allow to enable "idtes" feature for TCG Richard Henderson
@ 2017-07-10 20:45 ` Richard Henderson
7 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2017-07-10 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien, david
The rotation is to the left, but extract shifts to the right.
The computation of the extract parameters needs adjusting.
For the entry condition, simplify
64 - rot + len <= 64
-rot + len <= 0
len <= rot
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Reported-by: David Hildenbrand <david@redhat.com>
Suggested-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target/s390x/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f94ba7c..0c609bd 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3471,8 +3471,8 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
}
/* In some cases we can implement this with extract. */
- if (imask == 0 && pos == 0 && len > 0 && rot + len <= 64) {
- tcg_gen_extract_i64(o->out, o->in2, rot, len);
+ if (imask == 0 && pos == 0 && len > 0 && len <= rot) {
+ tcg_gen_extract_i64(o->out, o->in2, 64 - rot, len);
return NO_EXIT;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread