Re: [Qemu-devel] [PATCH v3 2/8] target/s390x: Implement CONVERT UNICODE insns

[Richard Henderson]

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 <r...@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~