[Qemu-devel] [PATCH v3 00/30] target/s390x: fix, improve and implement some more instructions

[Qemu-devel] [PATCH v3 00/30] target/s390x: fix, improve and implement some more instructions

[Aurelien Jarno]

This patchset tries to improve the s390x emulation by fixing and
improving some instructions. It implement some more instructions, from
the zArchitecture base or from the Extended-Translation Facility 2. The
last patch updates the maximum TCG CPU to z800, as the ETF2 and
Long-Displacement Facility are now both fully implemented.

This patch series is based on the s390x unwind and execute patches
from Richard Henderson and the qemu cpu_models patch from Thomas Huth.
It includes feedback from both.

v1 -> v2:
- add a patch to improve IPTE
- implement local-TLB-clearing in IPTE
- use tcg_gen_atomic_xchg_i32 in TEST AND SET
- use gen_op_movi_cc in TEST ADDRESSING MODE
- use helper_atomic_ldo_be_mmu in LOAD PAIR FROM QUADWORD
- use helper_atomic_sto_be_mmu in STORE PAIR TO QUADWORD
- add a patch to implement COMPARE AND SIGNAL
- rename dest and src into src1 and src3 and check that r1 & r3 are even
  in COMPARE LOGICAL LONG EXTENDED
- check that r1 & r2 are even in COMPARE LOGICAL LONG
- check that r1 & r3 are even in COMPARE LOGICAL LONG UNICODE
- move the adj_len_to_page patch before MVCL/MVCLE changes and simplify
  the expression a bit
- fix indentation in MOVE LONG UNICODE
- rebase the cpu model upgrade to z800 onto the qemu cpu_models patch
  from Thomas Huth and use s390_find_cpu_def to get the z800 model.

v2 -> v3:
- rename _VADDR_PX into VADDR_PX in IPTE patch
- check for alignment in LOAD PAIR FROM QUADWORD using MO_ALIGN16
- check for alignment in STORE PAIR TO QUADWORD using MO_ALIGN16
- remove side effect flags from COMPARE AND SIGNAL helpers
- pass the tst value as an argument ot the TRANSLATE ONE/TWO TO ONE/TWO
  helper and check for the M3 value
- add a patch to enable the ETF2-Enhancement Facility

Aurelien Jarno (30):
  target/s390x: remove dead code in translate.c
  target/s390x: remove some Linux assumptions from IPTE
  target/s390x: implement local-TLB-clearing in IPTE
  target/s390x: implement TEST AND SET
  target/s390x: implement TEST ADDRESSING MODE
  target/s390x: implement PACK
  target/s390x: implement LOAD PAIR FROM QUADWORD
  target/s390x: implement STORE PAIR TO QUADWORD
  target/s390x: implement COMPARE AND SIGNAL
  target/s390x: implement MOVE INVERSE
  target/s390x: implement MOVE NUMERICS
  target/s390x: implement MOVE WITH OFFSET
  target/s390x: implement MOVE ZONES
  target/s390x: improve 24-bit and 31-bit addresses read
  target/s390x: improve 24-bit and 31-bit addresses write
  target/s390x: improve 24-bit and 31-bit lengths read/write
  target/s390x: fix COMPARE LOGICAL LONG EXTENDED
  target/s390x: implement COMPARE LOGICAL LONG
  target/s390x: fix adj_len_to_page
  target/s390x: improve MOVE LONG and MOVE LONG EXTENDED
  target/s390x: implement COMPARE LOGICAL LONG UNICODE
  target/s390x: implement MOVE LONG UNICODE
  target/s390x: implement PACK ASCII
  target/s390x: implement PACK UNICODE
  target/s390x: implement UNPACK ASCII
  target/s390x: implement UNPACK UNICODE
  target/s390x: implement TEST DECIMAL
  target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO
  target/s390x: mark ETF2 and ETF2-ENH facilities as available
  target/s390x: update maximum TCG model to z800

 target/s390x/cpu.h         |   2 +
 target/s390x/cpu_models.c  |  13 +-
 target/s390x/fpu_helper.c  |  27 ++
 target/s390x/helper.h      |  21 +-
 target/s390x/insn-data.def |  57 ++++
 target/s390x/mem_helper.c  | 746 +++++++++++++++++++++++++++++++++++++--------
 target/s390x/misc_helper.c |   4 +-
 target/s390x/mmu_helper.c  |   2 -
 target/s390x/translate.c   | 335 ++++++++++++++++++--
 9 files changed, 1052 insertions(+), 155 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH v3 01/30] target/s390x: remove dead code in translate.c

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/translate.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 97ca639a34..f7598184a6 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -5467,10 +5467,7 @@ void gen_intermediate_code(CPUS390XState *env, struct TranslationBlock *tb)
             gen_io_start();
         }
 
-        status = NO_EXIT;
-        if (status == NO_EXIT) {
-            status = translate_one(env, &dc);
-        }
+        status = translate_one(env, &dc);
 
         /* If we reach a page boundary, are single stepping,
            or exhaust instruction count, stop generation.  */
-- 
2.11.0

[Qemu-devel] [PATCH v3 02/30] target/s390x: remove some Linux assumptions from IPTE

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/cpu.h        |  2 ++
 target/s390x/mem_helper.c | 17 ++++++++++-------
 target/s390x/mmu_helper.c |  2 --
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 79235cfa45..ba3d50b8e0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1033,6 +1033,8 @@ struct sysib_322 {
 #define _SEGMENT_ENTRY_RO       0x200     /* page protection bit              */
 #define _SEGMENT_ENTRY_INV      0x20      /* invalid segment table entry      */
 
+#define VADDR_PX                0xff000   /* page index bits                  */
+
 #define _PAGE_RO        0x200            /* HW read-only bit  */
 #define _PAGE_INVALID   0x400            /* HW invalid bit    */
 #define _PAGE_RES0      0x800            /* bit must be zero  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e35571e342..0ebd65d9ab 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1073,19 +1073,22 @@ uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 }
 
 /* invalidate pte */
-void HELPER(ipte)(CPUS390XState *env, uint64_t pte_addr, uint64_t vaddr)
+void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
     uint64_t page = vaddr & TARGET_PAGE_MASK;
-    uint64_t pte = 0;
+    uint64_t pte_addr, pte;
 
     /* XXX broadcast to other CPUs */
 
-    /* XXX Linux is nice enough to give us the exact pte address.
-       According to spec we'd have to find it out ourselves */
-    /* XXX Linux is fine with overwriting the pte, the spec requires
-       us to only set the invalid bit */
-    stq_phys(cs->as, pte_addr, pte | _PAGE_INVALID);
+    /* Compute the page table entry address */
+    pte_addr = (pto & _SEGMENT_ENTRY_ORIGIN);
+    pte_addr += (vaddr & _VADDR_PX) >> 9;
+
+    /* Mark the page table entry as invalid */
+    pte = ldq_phys(cs->as, pte_addr);
+    pte |= _PAGE_INVALID;
+    stq_phys(cs->as, pte_addr, pte);
 
     /* XXX we exploit the fact that Linux passes the exact virtual
        address here - it's not obliged to! */
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 31eb9efa9b..501e39010d 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -143,8 +143,6 @@ static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
     return 0;
 }
 
-#define VADDR_PX    0xff000         /* Page index bits */
-
 /* Decode segment table entry */
 static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
                                  uint64_t asc, uint64_t st_entry,
-- 
2.11.0

[Qemu-devel] [PATCH v3 03/30] target/s390x: implement local-TLB-clearing in IPTE

[Aurelien Jarno]

And at the same time make IPTE SMP aware.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h     |  2 +-
 target/s390x/mem_helper.c | 21 +++++++++++++--------
 target/s390x/translate.c  |  6 +++++-
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index cc451c70a6..3f5a05d43b 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -111,7 +111,7 @@ DEF_HELPER_4(mvcs, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvcp, i32, env, i64, i64, i64)
 DEF_HELPER_4(sigp, i32, env, i64, i32, i64)
 DEF_HELPER_FLAGS_2(sacf, TCG_CALL_NO_WG, void, env, i64)
-DEF_HELPER_FLAGS_3(ipte, TCG_CALL_NO_RWG, void, env, i64, i64)
+DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(lra, i64, env, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 0ebd65d9ab..ddbebcd7ae 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1073,17 +1073,16 @@ uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 }
 
 /* invalidate pte */
-void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
+void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr,
+                  uint32_t m4)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
     uint64_t page = vaddr & TARGET_PAGE_MASK;
     uint64_t pte_addr, pte;
 
-    /* XXX broadcast to other CPUs */
-
     /* Compute the page table entry address */
     pte_addr = (pto & _SEGMENT_ENTRY_ORIGIN);
-    pte_addr += (vaddr & _VADDR_PX) >> 9;
+    pte_addr += (vaddr & VADDR_PX) >> 9;
 
     /* Mark the page table entry as invalid */
     pte = ldq_phys(cs->as, pte_addr);
@@ -1092,13 +1091,19 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
 
     /* XXX we exploit the fact that Linux passes the exact virtual
        address here - it's not obliged to! */
-    tlb_flush_page(cs, page);
+    /* XXX: the LC bit should be considered as 0 if the local-TLB-clearing
+       facility is not installed.  */
+    if (m4 & 1) {
+        tlb_flush_page(cs, page);
+    } else {
+        tlb_flush_page_all_cpus_synced(cs, page);
+    }
 
     /* XXX 31-bit hack */
-    if (page & 0x80000000) {
-        tlb_flush_page(cs, page & ~0x80000000);
+    if (m4 & 1) {
+        tlb_flush_page(cs, page ^ 0x80000000);
     } else {
-        tlb_flush_page(cs, page | 0x80000000);
+        tlb_flush_page_all_cpus_synced(cs, page ^ 0x80000000);
     }
 }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f7598184a6..f160b62c19 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2352,8 +2352,12 @@ static ExitStatus op_ipm(DisasContext *s, DisasOps *o)
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_ipte(DisasContext *s, DisasOps *o)
 {
+    TCGv_i32 m4;
+
     check_privileged(s);
-    gen_helper_ipte(cpu_env, o->in1, o->in2);
+    m4 = tcg_const_i32(get_field(s->fields, m4));
+    gen_helper_ipte(cpu_env, o->in1, o->in2, m4);
+    tcg_temp_free_i32(m4);
     return NO_EXIT;
 }
 
-- 
2.11.0

[Qemu-devel] [PATCH v3 04/30] target/s390x: implement TEST AND SET

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/insn-data.def |  3 +++
 target/s390x/translate.c   | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index f818437069..0f70acea5c 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -810,6 +810,9 @@
 /* SUPERVISOR CALL */
     C(0x0a00, SVC,     I,     Z,   0, 0, 0, 0, svc, 0)
 
+/* TEST AND SET */
+    C(0x9300, TS,      S,     Z,   0, a2, 0, 0, ts, 0)
+
 /* TEST DATA CLASS */
     C(0xed10, TCEB,    RXE,   Z,   e1, a2, 0, 0, tceb, 0)
     C(0xed11, TCDB,    RXE,   Z,   f1_o, a2, 0, 0, tcdb, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f160b62c19..0cfa8cc05e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4129,6 +4129,16 @@ static ExitStatus op_trt(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_ts(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 t1 = tcg_const_i32(0xff);
+    tcg_gen_atomic_xchg_i32(t1, o->in2, t1, get_mem_index(s), MO_UB);
+    tcg_gen_extract_i32(cc_op, t1, 7, 1);
+    tcg_temp_free_i32(t1);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_unpk(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-- 
2.11.0

[Qemu-devel] [PATCH v3 05/30] target/s390x: implement TEST ADDRESSING MODE

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/insn-data.def |  3 +++
 target/s390x/translate.c   | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 0f70acea5c..170b50ef2e 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -810,6 +810,9 @@
 /* SUPERVISOR CALL */
     C(0x0a00, SVC,     I,     Z,   0, 0, 0, 0, svc, 0)
 
+/* TEST ADDRESSING MODE */
+    C(0x010b, TAM,     E,     Z,   0, 0, 0, 0, tam, 0)
+
 /* TEST AND SET */
     C(0x9300, TS,      S,     Z,   0, a2, 0, 0, ts, 0)
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 0cfa8cc05e..7f265aeb40 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4063,6 +4063,16 @@ static ExitStatus op_svc(DisasContext *s, DisasOps *o)
     return EXIT_NORETURN;
 }
 
+static ExitStatus op_tam(DisasContext *s, DisasOps *o)
+{
+    int cc = 0;
+
+    cc |= (s->tb->flags & FLAG_MASK_64) ? 2 : 0;
+    cc |= (s->tb->flags & FLAG_MASK_32) ? 1 : 0;
+    gen_op_movi_cc(s, cc);
+    return NO_EXIT;
+}
+
 static ExitStatus op_tceb(DisasContext *s, DisasOps *o)
 {
     gen_helper_tceb(cc_op, cpu_env, o->in1, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 06/30] target/s390x: implement PACK

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  5 +++++
 target/s390x/mem_helper.c  | 37 +++++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 51 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 3f5a05d43b..c6fbc3b949 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -75,6 +75,7 @@ DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
 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(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 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_4(tre, i64, env, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 170b50ef2e..f92bfde4f8 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -639,6 +639,11 @@
     C(0x9600, OI,      SI,    Z,   m1_8u, i2_8u, new, m1_8, or, nz64)
     C(0xeb56, OIY,     SIY,   LD,  m1_8u, i2_8u, new, m1_8, or, nz64)
 
+/* PACK */
+    /* Really format SS_b, but we pack both lengths into one argument
+       for the helper call, so we might as well leave one 8-bit field.  */
+    C(0xf200, PACK,    SS_a,  Z,   la1, a2, 0, 0, pack, 0)
+
 /* PREFETCH */
     /* Implemented as nops of course.  */
     C(0xe336, PFD,     RXY_b, GIE, 0, 0, 0, 0, 0, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index ddbebcd7ae..850472e9ff 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -644,6 +644,43 @@ uint64_t HELPER(cksm)(CPUS390XState *env, uint64_t r1,
     return len;
 }
 
+void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int len_dest = len >> 4;
+    int len_src = len & 0xf;
+    uint8_t b;
+
+    dest += len_dest;
+    src += len_src;
+
+    /* last byte is special, it only flips the nibbles */
+    b = cpu_ldub_data_ra(env, src, ra);
+    cpu_stb_data_ra(env, dest, (b << 4) | (b >> 4), ra);
+    src--;
+    len_src--;
+
+    /* now pack every value */
+    while (len_dest >= 0) {
+        b = 0;
+
+        if (len_src > 0) {
+            b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
+            src--;
+            len_src--;
+        }
+        if (len_src > 0) {
+            b |= cpu_ldub_data_ra(env, src, ra) << 4;
+            src--;
+            len_src--;
+        }
+
+        len_dest--;
+        dest--;
+        cpu_stb_data_ra(env, dest, b, ra);
+    }
+}
+
 void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
                   uint64_t src)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 7f265aeb40..00b91c4f3a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3139,6 +3139,14 @@ static ExitStatus op_ori(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_pack(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_pack(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_popcnt(DisasContext *s, DisasOps *o)
 {
     gen_helper_popcnt(o->out, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 07/30] target/s390x: implement LOAD PAIR FROM QUADWORD

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 13 +++++++++++++
 target/s390x/translate.c   |  7 +++++++
 4 files changed, 23 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index c6fbc3b949..ca78d1b162 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -87,6 +87,7 @@ DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(stfle, i32, env, i64)
+DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index f92bfde4f8..53c86d5832 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -507,6 +507,8 @@
 /* LOAD PAIR DISJOINT */
     D(0xc804, LPD,     SSF,   ILA, 0, 0, new_P, r3_P32, lpd, 0, MO_TEUL)
     D(0xc805, LPDG,    SSF,   ILA, 0, 0, new_P, r3_P64, lpd, 0, MO_TEQ)
+/* LOAD PAIR FROM QUADWORD */
+    C(0xe38f, LPQ,     RXY_a, Z,   0, a2, r1_P, 0, lpq, 0)
 /* LOAD POSITIVE */
     C(0x1000, LPR,     RR_a,  Z,   0, r2_32s, new, r1_32, abs, abs32)
     C(0xb900, LPGR,    RRE,   Z,   0, r2, r1, 0, abs, abs64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 850472e9ff..4f34f873c7 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1237,6 +1237,19 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 }
 #endif
 
+/* load pair from quadword */
+uint64_t HELPER(lpq)(CPUS390XState *env, uint64_t addr)
+{
+    uintptr_t ra = GETPC();
+    int mem_idx = cpu_mmu_index(env, false);
+    TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
+
+    Int128 v = helper_atomic_ldo_be_mmu(env, addr, oi, ra);
+
+    env->retxl = int128_getlo(v);
+    return int128_gethi(v);
+}
+
 /* Execute instruction.  This instruction executes an insn modified with
    the contents of r1.  It does not change the executed instruction in memory;
    it does not change the program counter.
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 00b91c4f3a..ec61590e50 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2830,6 +2830,13 @@ static ExitStatus op_lpd(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_lpq(DisasContext *s, DisasOps *o)
+{
+    gen_helper_lpq(o->out, cpu_env, o->in2);
+    return_low128(o->out2);
+    return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_lura(DisasContext *s, DisasOps *o)
 {
-- 
2.11.0

[Qemu-devel] [PATCH v3 08/30] target/s390x: implement STORE PAIR TO QUADWORD

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 12 ++++++++++++
 target/s390x/translate.c   |  6 ++++++
 4 files changed, 21 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index ca78d1b162..596fec28ca 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 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)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 53c86d5832..5314162b3d 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -770,6 +770,8 @@
 /* STORE ACCESS MULTIPLE */
     C(0x9b00, STAM,    RS_a,  Z,   0, a2, 0, 0, stam, 0)
     C(0xeb9b, STAMY,   RSY_a, LD,  0, a2, 0, 0, stam, 0)
+/* STORE PAIR TO QUADWORD */
+    C(0xe38e, STPQ,    RXY_a, Z,   0, a2, r1_P, 0, stpq, 0)
 
 /* SUBTRACT */
     C(0x1b00, SR,      RR_a,  Z,   r1, r2, new, r1_32, sub, subs32)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4f34f873c7..15b5f45b77 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1250,6 +1250,18 @@ uint64_t HELPER(lpq)(CPUS390XState *env, uint64_t addr)
     return int128_gethi(v);
 }
 
+/* store pair to quadword */
+void HELPER(stpq)(CPUS390XState *env, uint64_t addr,
+                  uint64_t low, uint64_t high)
+{
+    uintptr_t ra = GETPC();
+    int mem_idx = cpu_mmu_index(env, false);
+    TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
+
+    Int128 v = int128_make128(low, high);
+    helper_atomic_sto_be_mmu(env, addr, v, oi, ra);
+}
+
 /* Execute instruction.  This instruction executes an insn modified with
    the contents of r1.  It does not change the executed instruction in memory;
    it does not change the program counter.
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ec61590e50..6635877bbd 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4019,6 +4019,12 @@ static ExitStatus op_stmh(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_stpq(DisasContext *s, DisasOps *o)
+{
+    gen_helper_stpq(cpu_env, o->in2, o->out2, o->out);
+    return NO_EXIT;
+}
+
 static ExitStatus op_srst(DisasContext *s, DisasOps *o)
 {
     gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 09/30] target/s390x: implement COMPARE AND SIGNAL

[Aurelien Jarno]

These functions differ from COMPARE by generating an exception for a
QNaN input. Use the non quiet version of floatXX_compare.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/fpu_helper.c  | 27 +++++++++++++++++++++++++++
 target/s390x/helper.h      |  3 +++
 target/s390x/insn-data.def |  6 ++++++
 target/s390x/translate.c   | 21 +++++++++++++++++++++
 4 files changed, 57 insertions(+)

diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index e604e9f7be..26f124fe96 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -585,6 +585,33 @@ uint64_t HELPER(fixb)(CPUS390XState *env, uint64_t ah, uint64_t al, uint32_t m3)
     return RET128(ret);
 }
 
+/* 32-bit FP compare and signal */
+uint32_t HELPER(keb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
+{
+    int cmp = float32_compare(f1, f2, &env->fpu_status);
+    handle_exceptions(env, GETPC());
+    return float_comp_to_cc(env, cmp);
+}
+
+/* 64-bit FP compare and signal */
+uint32_t HELPER(kdb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
+{
+    int cmp = float64_compare(f1, f2, &env->fpu_status);
+    handle_exceptions(env, GETPC());
+    return float_comp_to_cc(env, cmp);
+}
+
+/* 128-bit FP compare and signal */
+uint32_t HELPER(kxb)(CPUS390XState *env, uint64_t ah, uint64_t al,
+                     uint64_t bh, uint64_t bl)
+{
+    int cmp = float128_compare(make_float128(ah, al),
+                               make_float128(bh, bl),
+                               &env->fpu_status);
+    handle_exceptions(env, GETPC());
+    return float_comp_to_cc(env, cmp);
+}
+
 /* 32-bit FP multiply and add */
 uint64_t HELPER(maeb)(CPUS390XState *env, uint64_t f1,
                       uint64_t f2, uint64_t f3)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 596fec28ca..4ada89488b 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -89,6 +89,9 @@ DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 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_FLAGS_3(keb, TCG_CALL_NO_WG, i32, env, i64, i64)
+DEF_HELPER_FLAGS_3(kdb, TCG_CALL_NO_WG, i32, env, i64, i64)
+DEF_HELPER_FLAGS_5(kxb, TCG_CALL_NO_WG, i32, env, i64, i64, i64, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 5314162b3d..01278949fc 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -154,6 +154,12 @@
     C(0xb349, CXBR,    RRE,   Z,   x1_o, x2_o, 0, 0, cxb, 0)
     C(0xed09, CEB,     RXE,   Z,   e1, m2_32u, 0, 0, ceb, 0)
     C(0xed19, CDB,     RXE,   Z,   f1_o, m2_64, 0, 0, cdb, 0)
+/* COMPARE AND SIGNAL */
+    C(0xb308, KEBR,    RRE,   Z,   e1, e2, 0, 0, keb, 0)
+    C(0xb318, KDBR,    RRE,   Z,   f1_o, f2_o, 0, 0, kdb, 0)
+    C(0xb348, KXBR,    RRE,   Z,   x1_o, x2_o, 0, 0, kxb, 0)
+    C(0xed08, KEB,     RXE,   Z,   e1, m2_32u, 0, 0, keb, 0)
+    C(0xed18, KDB,     RXE,   Z,   f1_o, m2_64, 0, 0, kdb, 0)
 /* COMPARE IMMEDIATE */
     C(0xc20d, CFI,     RIL_a, EI,  r1, i2, 0, 0, 0, cmps32)
     C(0xc20c, CGFI,    RIL_a, EI,  r1, i2, 0, 0, 0, cmps64)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 6635877bbd..30d0575c03 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2369,6 +2369,27 @@ static ExitStatus op_iske(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static ExitStatus op_keb(DisasContext *s, DisasOps *o)
+{
+    gen_helper_keb(cc_op, cpu_env, o->in1, o->in2);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
+static ExitStatus op_kdb(DisasContext *s, DisasOps *o)
+{
+    gen_helper_kdb(cc_op, cpu_env, o->in1, o->in2);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
+static ExitStatus op_kxb(DisasContext *s, DisasOps *o)
+{
+    gen_helper_kxb(cc_op, cpu_env, o->out, o->out2, o->in1, o->in2);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_laa(DisasContext *s, DisasOps *o)
 {
     /* The real output is indeed the original value in memory;
-- 
2.11.0

[Qemu-devel] [PATCH v3 10/30] target/s390x: implement MOVE INVERSE

[Aurelien Jarno]

Reviewed-by: 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  | 12 ++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 23 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 4ada89488b..a618fe5539 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -3,6 +3,7 @@ DEF_HELPER_FLAGS_4(nc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(oc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(xc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_3(mvcl, i32, env, i32, i32)
 DEF_HELPER_FLAGS_4(clm, TCG_CALL_NO_WG, i32, env, i32, i32, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 01278949fc..c8f77611ab 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -572,6 +572,8 @@
     C(0xe548, MVGHI,   SIL,   GIE, la1, i2, 0, m1_64, mov2, 0)
     C(0x9200, MVI,     SI,    Z,   la1, i2, 0, m1_8, mov2, 0)
     C(0xeb52, MVIY,    SIY,   LD,  la1, i2, 0, m1_8, mov2, 0)
+/* MOVE INVERSE */
+    C(0xe800, MVCIN,   SS_a,  Z,   la1, a2, 0, 0, mvcin, 0)
 /* MOVE LONG */
     C(0x0e00, MVCL,    RR_a,  Z,   0, 0, 0, 0, mvcl, 0)
 /* MOVE LONG EXTENDED */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 15b5f45b77..eef754724d 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -231,6 +231,18 @@ void HELPER(mvc)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     do_helper_mvc(env, l, dest, src, GETPC());
 }
 
+/* move inverse  */
+void HELPER(mvcin)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+
+    for (i = 0; i <= l; i++) {
+        uint8_t v = cpu_ldub_data_ra(env, src - i, ra);
+        cpu_stb_data_ra(env, dest + i, v, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 30d0575c03..61373df29e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2940,6 +2940,14 @@ static ExitStatus op_mvc(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvcin(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvcin(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mvcl(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
-- 
2.11.0

[Qemu-devel] [PATCH v3 11/30] target/s390x: implement MOVE NUMERICS

[Aurelien Jarno]

Reviewed-by: 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  | 13 +++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 24 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index a618fe5539..e62e45534d 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -13,6 +13,7 @@ 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_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(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvst, i64, env, i64, i64, i64)
 DEF_HELPER_4(ex, void, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index c8f77611ab..6af717648c 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -578,6 +578,8 @@
     C(0x0e00, MVCL,    RR_a,  Z,   0, 0, 0, 0, mvcl, 0)
 /* MOVE LONG EXTENDED */
     C(0xa800, MVCLE,   RS_a,  Z,   0, a2, 0, 0, mvcle, 0)
+/* MOVE NUMERICS */
+    C(0xd100, MVN,     SS_a,  Z,   la1, a2, 0, 0, mvn, 0)
 /* MOVE PAGE */
     C(0xb254, MVPG,    RRE,   Z,   r1_o, r2_o, 0, 0, mvpg, 0)
 /* MOVE STRING */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index eef754724d..53852784e6 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -243,6 +243,19 @@ void HELPER(mvcin)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     }
 }
 
+/* move numerics  */
+void HELPER(mvn)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+
+    for (i = 0; i <= l; i++) {
+        uint8_t v = cpu_ldub_data_ra(env, dest + i, ra) & 0xf0;
+        v |= cpu_ldub_data_ra(env, src + i, ra) & 0x0f;
+        cpu_stb_data_ra(env, dest + i, v, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 61373df29e..4e7211203a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2990,6 +2990,14 @@ static ExitStatus op_mvcs(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static ExitStatus op_mvn(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvn(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mvpg(DisasContext *s, DisasOps *o)
 {
     gen_helper_mvpg(cc_op, cpu_env, regs[0], o->in1, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 12/30] target/s390x: implement MOVE WITH OFFSET

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  4 ++++
 target/s390x/mem_helper.c  | 31 +++++++++++++++++++++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 44 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index e62e45534d..a197b8bc39 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -14,6 +14,7 @@ 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_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)
 DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvst, i64, env, i64, i64, i64)
 DEF_HELPER_4(ex, void, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 6af717648c..47542eeaf3 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -584,6 +584,10 @@
     C(0xb254, MVPG,    RRE,   Z,   r1_o, r2_o, 0, 0, mvpg, 0)
 /* MOVE STRING */
     C(0xb255, MVST,    RRE,   Z,   r1_o, r2_o, 0, 0, mvst, 0)
+/* MOVE WITH OFFSET */
+    /* Really format SS_b, but we pack both lengths into one argument
+       for the helper call, so we might as well leave one 8-bit field.  */
+    C(0xf100, MVO,     SS_a,  Z,   la1, a2, 0, 0, mvo, 0)
 
 /* MULTIPLY */
     C(0x1c00, MR,      RR_a,  Z,   r1p1_32s, r2_32s, new, r1_D32, mul, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 53852784e6..3601bb9cdd 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -256,6 +256,37 @@ void HELPER(mvn)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     }
 }
 
+/* move with offset  */
+void HELPER(mvo)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int len_dest = l >> 4;
+    int len_src = l & 0xf;
+    uint8_t byte_dest, byte_src;
+    int i;
+
+    src += len_src;
+    dest += len_dest;
+
+    /* Handle rightmost byte */
+    byte_src = cpu_ldub_data_ra(env, src, ra);
+    byte_dest = cpu_ldub_data_ra(env, dest, ra);
+    byte_dest = (byte_dest & 0x0f) | (byte_src << 4);
+    cpu_stb_data_ra(env, dest, byte_dest, ra);
+
+    /* Process remaining bytes from right to left */
+    for (i = 1; i <= len_dest; i++) {
+        byte_dest = byte_src >> 4;
+        if (len_src - i >= 0) {
+            byte_src = cpu_ldub_data_ra(env, src - i, ra);
+        } else {
+            byte_src = 0;
+        }
+        byte_dest |= byte_src << 4;
+        cpu_stb_data_ra(env, dest - i, byte_dest, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 4e7211203a..b1877cf27b 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2998,6 +2998,14 @@ static ExitStatus op_mvn(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvo(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvo(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mvpg(DisasContext *s, DisasOps *o)
 {
     gen_helper_mvpg(cc_op, cpu_env, regs[0], o->in1, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 13/30] target/s390x: implement MOVE ZONES

[Aurelien Jarno]

Reviewed-by: 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  | 13 +++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 24 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index a197b8bc39..ba21e4d870 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -16,6 +16,7 @@ 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)
 DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
+DEF_HELPER_FLAGS_4(mvz, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(mvst, i64, env, i64, i64, i64)
 DEF_HELPER_4(ex, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 47542eeaf3..b40611b75f 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -588,6 +588,8 @@
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
     C(0xf100, MVO,     SS_a,  Z,   la1, a2, 0, 0, mvo, 0)
+/* MOVE ZONES */
+    C(0xd300, MVZ,     SS_a,  Z,   la1, a2, 0, 0, mvz, 0)
 
 /* MULTIPLY */
     C(0x1c00, MR,      RR_a,  Z,   r1p1_32s, r2_32s, new, r1_D32, mul, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 3601bb9cdd..95f701dae2 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -287,6 +287,19 @@ void HELPER(mvo)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     }
 }
 
+/* move zones  */
+void HELPER(mvz)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+
+    for (i = 0; i <= l; i++) {
+        uint8_t b = cpu_ldub_data_ra(env, dest + i, ra) & 0x0f;
+        b |= cpu_ldub_data_ra(env, src + i, ra) & 0xf0;
+        cpu_stb_data_ra(env, dest + i, b, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index b1877cf27b..95ca53c1ef 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3021,6 +3021,14 @@ static ExitStatus op_mvst(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvz(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvz(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mul(DisasContext *s, DisasOps *o)
 {
     tcg_gen_mul_i64(o->out, o->in1, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 14/30] target/s390x: improve 24-bit and 31-bit addresses read

[Aurelien Jarno]

Improve fix_address to also handle the 24-bit mode. Rename fix_address
to wrap_address to better explain what is changed.

Replace the calls to get_address with x2 = 0 and b2 = 0 by
call to wrap_address, leading to the removal of this function. Rename
get_address_31fix into get_address.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 71 +++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 40 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 95f701dae2..2425bfc984 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -365,30 +365,23 @@ uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask,
     return cc;
 }
 
-static inline uint64_t fix_address(CPUS390XState *env, uint64_t a)
+static inline uint64_t wrap_address(CPUS390XState *env, uint64_t a)
 {
-    /* 31-Bit mode */
     if (!(env->psw.mask & PSW_MASK_64)) {
-        a &= 0x7fffffff;
+        if (!(env->psw.mask & PSW_MASK_32)) {
+            /* 24-Bit mode */
+            a &= 0x00ffffff;
+        } else {
+            /* 31-Bit mode */
+            a &= 0x7fffffff;
+        }
     }
     return a;
 }
 
-static inline uint64_t get_address(CPUS390XState *env, int x2, int b2, int d2)
-{
-    uint64_t r = d2;
-    if (x2) {
-        r += env->regs[x2];
-    }
-    if (b2) {
-        r += env->regs[b2];
-    }
-    return fix_address(env, r);
-}
-
-static inline uint64_t get_address_31fix(CPUS390XState *env, int reg)
+static inline uint64_t get_address(CPUS390XState *env, int reg)
 {
-    return fix_address(env, env->regs[reg]);
+    return wrap_address(env, env->regs[reg]);
 }
 
 /* search string (c is byte to search, r2 is string, r1 end of string) */
@@ -399,8 +392,8 @@ uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
     uint32_t len;
     uint8_t v, c = r0;
 
-    str = fix_address(env, str);
-    end = fix_address(env, end);
+    str = wrap_address(env, str);
+    end = wrap_address(env, end);
 
     /* Assume for now that R2 is unmodified.  */
     env->retxl = str;
@@ -434,8 +427,8 @@ uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, uint64_t s1, uint64_t s2)
     uint32_t len;
 
     c = c & 0xff;
-    s1 = fix_address(env, s1);
-    s2 = fix_address(env, s2);
+    s1 = wrap_address(env, s1);
+    s2 = wrap_address(env, s2);
 
     /* 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.  */
@@ -481,8 +474,8 @@ uint64_t HELPER(mvst)(CPUS390XState *env, uint64_t c, uint64_t d, uint64_t s)
     uint32_t len;
 
     c = c & 0xff;
-    d = fix_address(env, d);
-    s = fix_address(env, s);
+    d = wrap_address(env, d);
+    s = wrap_address(env, s);
 
     /* 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.  */
@@ -540,9 +533,9 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 {
     uintptr_t ra = GETPC();
     uint64_t destlen = env->regs[r1 + 1] & 0xffffff;
-    uint64_t dest = get_address_31fix(env, r1);
+    uint64_t dest = get_address(env, r1);
     uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
-    uint64_t src = get_address_31fix(env, r2);
+    uint64_t src = get_address(env, r2);
     uint8_t pad = env->regs[r2 + 1] >> 24;
     uint8_t v;
     uint32_t cc;
@@ -583,9 +576,9 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 {
     uintptr_t ra = GETPC();
     uint64_t destlen = env->regs[r1 + 1];
-    uint64_t dest = env->regs[r1];
+    uint64_t dest = get_address(env, r1);
     uint64_t srclen = env->regs[r3 + 1];
-    uint64_t src = env->regs[r3];
+    uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint8_t v;
     uint32_t cc;
@@ -593,8 +586,6 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     if (!(env->psw.mask & PSW_MASK_64)) {
         destlen = (uint32_t)destlen;
         srclen = (uint32_t)srclen;
-        dest &= 0x7fffffff;
-        src &= 0x7fffffff;
     }
 
     if (destlen == srclen) {
@@ -634,9 +625,9 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 {
     uintptr_t ra = GETPC();
     uint64_t destlen = env->regs[r1 + 1];
-    uint64_t dest = get_address_31fix(env, r1);
+    uint64_t dest = get_address(env, r1);
     uint64_t srclen = env->regs[r3 + 1];
-    uint64_t src = get_address_31fix(env, r3);
+    uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint32_t cc = 0;
 
@@ -1020,7 +1011,7 @@ uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr)
     uint64_t abs_addr;
     int i;
 
-    real_addr = fix_address(env, real_addr);
+    real_addr = wrap_address(env, real_addr);
     abs_addr = mmu_real2abs(env, real_addr) & TARGET_PAGE_MASK;
     if (!address_space_access_valid(&address_space_memory, abs_addr,
                                     TARGET_PAGE_SIZE, true)) {
@@ -1054,7 +1045,7 @@ uint64_t HELPER(iske)(CPUS390XState *env, uint64_t r2)
 {
     static S390SKeysState *ss;
     static S390SKeysClass *skeyclass;
-    uint64_t addr = get_address(env, 0, 0, r2);
+    uint64_t addr = wrap_address(env, r2);
     uint8_t key;
 
     if (addr > ram_size) {
@@ -1077,7 +1068,7 @@ void HELPER(sske)(CPUS390XState *env, uint64_t r1, uint64_t r2)
 {
     static S390SKeysState *ss;
     static S390SKeysClass *skeyclass;
-    uint64_t addr = get_address(env, 0, 0, r2);
+    uint64_t addr = wrap_address(env, r2);
     uint8_t key;
 
     if (addr > ram_size) {
@@ -1234,14 +1225,14 @@ uint64_t HELPER(lura)(CPUS390XState *env, uint64_t addr)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    return (uint32_t)ldl_phys(cs->as, get_address(env, 0, 0, addr));
+    return (uint32_t)ldl_phys(cs->as, wrap_address(env, addr));
 }
 
 uint64_t HELPER(lurag)(CPUS390XState *env, uint64_t addr)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    return ldq_phys(cs->as, get_address(env, 0, 0, addr));
+    return ldq_phys(cs->as, wrap_address(env, addr));
 }
 
 /* store using real address */
@@ -1249,7 +1240,7 @@ void HELPER(stura)(CPUS390XState *env, uint64_t addr, uint64_t v1)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    stl_phys(cs->as, get_address(env, 0, 0, addr), (uint32_t)v1);
+    stl_phys(cs->as, wrap_address(env, addr), (uint32_t)v1);
 
     if ((env->psw.mask & PSW_MASK_PER) &&
         (env->cregs[9] & PER_CR9_EVENT_STORE) &&
@@ -1264,7 +1255,7 @@ void HELPER(sturg)(CPUS390XState *env, uint64_t addr, uint64_t v1)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    stq_phys(cs->as, get_address(env, 0, 0, addr), v1);
+    stq_phys(cs->as, wrap_address(env, addr), v1);
 
     if ((env->psw.mask & PSW_MASK_PER) &&
         (env->cregs[9] & PER_CR9_EVENT_STORE) &&
@@ -1382,8 +1373,8 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
             uint32_t d1 = extract64(insn, 32, 12);
             uint32_t b2 = extract64(insn, 28, 4);
             uint32_t d2 = extract64(insn, 16, 12);
-            uint64_t a1 = get_address(env, 0, b1, d1);
-            uint64_t a2 = get_address(env, 0, b2, d2);
+            uint64_t a1 = wrap_address(env, env->regs[b1] + d1);
+            uint64_t a2 = wrap_address(env, env->regs[b2] + d2);
 
             env->cc_op = helper(env, l, a1, a2, 0);
             env->psw.addr += ilen;
-- 
2.11.0

[Qemu-devel] [PATCH v3 15/30] target/s390x: improve 24-bit and 31-bit addresses write

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 2425bfc984..2113494983 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -384,6 +384,29 @@ static inline uint64_t get_address(CPUS390XState *env, int reg)
     return wrap_address(env, env->regs[reg]);
 }
 
+static inline void set_address(CPUS390XState *env, int reg, uint64_t address)
+{
+    if (env->psw.mask & PSW_MASK_64) {
+        /* 64-Bit mode */
+        env->regs[reg] = address;
+    } else {
+        if (!(env->psw.mask & PSW_MASK_32)) {
+            /* 24-Bit mode. According to the PoO it is implementation
+            dependent if bits 32-39 remain unchanged or are set to
+            zeros.  Choose the former so that the function can also be
+            used for TRT.  */
+            env->regs[reg] = deposit64(env->regs[reg], 0, 24, address);
+        } else {
+            /* 31-Bit mode. According to the PoO it is implementation
+            dependent if bit 32 remains unchanged or is set to zero.
+            Choose the latter so that the function can also be used for
+            TRT.  */
+            address &= 0x7fffffff;
+            env->regs[reg] = deposit64(env->regs[reg], 0, 32, address);
+        }
+    }
+}
+
 /* 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)
@@ -564,8 +587,8 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     env->regs[r1 + 1] = destlen;
     /* can't use srclen here, we trunc'ed it */
     env->regs[r2 + 1] -= src - env->regs[r2];
-    env->regs[r1] = dest;
-    env->regs[r2] = src;
+    set_address(env, r1, dest);
+    set_address(env, r2, src);
 
     return cc;
 }
@@ -613,8 +636,8 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     /* can't use srclen here, we trunc'ed it */
     /* FIXME: 31-bit mode! */
     env->regs[r3 + 1] -= src - env->regs[r3];
-    env->regs[r1] = dest;
-    env->regs[r3] = src;
+    set_address(env, r1, dest);
+    set_address(env, r3, src);
 
     return cc;
 }
@@ -651,8 +674,8 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     env->regs[r1 + 1] = destlen;
     /* can't use srclen here, we trunc'ed it */
     env->regs[r3 + 1] -= src - env->regs[r3];
-    env->regs[r1] = dest;
-    env->regs[r3] = src;
+    set_address(env, r1, dest);
+    set_address(env, r3, src);
 
     return cc;
 }
@@ -858,7 +881,7 @@ static uint32_t do_helper_trt(CPUS390XState *env, uint32_t len, uint64_t array,
         uint8_t sbyte = cpu_ldub_data_ra(env, trans + byte, ra);
 
         if (sbyte != 0) {
-            env->regs[1] = array + i;
+            set_address(env, 1, array + i);
             env->regs[2] = deposit64(env->regs[2], 0, 8, sbyte);
             return (i == len) ? 2 : 1;
         }
-- 
2.11.0

[Qemu-devel] [PATCH v3 16/30] target/s390x: improve 24-bit and 31-bit lengths read/write

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 2113494983..98a7aa22d3 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -407,6 +407,31 @@ static inline void set_address(CPUS390XState *env, int reg, uint64_t address)
     }
 }
 
+static inline uint64_t wrap_length(CPUS390XState *env, uint64_t length)
+{
+    if (!(env->psw.mask & PSW_MASK_64)) {
+        /* 24-Bit and 31-Bit mode */
+        length &= 0x7fffffff;
+    }
+    return length;
+}
+
+static inline uint64_t get_length(CPUS390XState *env, int reg)
+{
+    return wrap_length(env, env->regs[reg]);
+}
+
+static inline void set_length(CPUS390XState *env, int reg, uint64_t length)
+{
+    if (env->psw.mask & PSW_MASK_64) {
+        /* 64-Bit mode */
+        env->regs[reg] = length;
+    } else {
+        /* 24-Bit and 31-Bit mode */
+        env->regs[reg] = deposit64(env->regs[reg], 0, 32, 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)
@@ -598,19 +623,14 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
                        uint32_t r3)
 {
     uintptr_t ra = GETPC();
-    uint64_t destlen = env->regs[r1 + 1];
+    uint64_t destlen = get_length(env, r1 + 1);
     uint64_t dest = get_address(env, r1);
-    uint64_t srclen = env->regs[r3 + 1];
+    uint64_t srclen = get_length(env, r3 + 1);
     uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint8_t v;
     uint32_t cc;
 
-    if (!(env->psw.mask & PSW_MASK_64)) {
-        destlen = (uint32_t)destlen;
-        srclen = (uint32_t)srclen;
-    }
-
     if (destlen == srclen) {
         cc = 0;
     } else if (destlen < srclen) {
@@ -632,10 +652,9 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
         cpu_stb_data_ra(env, dest, pad, ra);
     }
 
-    env->regs[r1 + 1] = destlen;
+    set_length(env, r1 + 1 , destlen);
     /* can't use srclen here, we trunc'ed it */
-    /* FIXME: 31-bit mode! */
-    env->regs[r3 + 1] -= src - env->regs[r3];
+    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
     set_address(env, r1, dest);
     set_address(env, r3, src);
 
@@ -647,9 +666,9 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
                        uint32_t r3)
 {
     uintptr_t ra = GETPC();
-    uint64_t destlen = env->regs[r1 + 1];
+    uint64_t destlen = get_length(env, r1 + 1);
     uint64_t dest = get_address(env, r1);
-    uint64_t srclen = env->regs[r3 + 1];
+    uint64_t srclen = get_length(env, r3 + 1);
     uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint32_t cc = 0;
@@ -671,9 +690,9 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
         }
     }
 
-    env->regs[r1 + 1] = destlen;
+    set_length(env, r1 + 1, destlen);
     /* can't use srclen here, we trunc'ed it */
-    env->regs[r3 + 1] -= src - env->regs[r3];
+    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
     set_address(env, r1, dest);
     set_address(env, r3, src);
 
-- 
2.11.0

[Qemu-devel] [PATCH v3 17/30] target/s390x: fix COMPARE LOGICAL LONG EXTENDED

[Aurelien Jarno]

There are multiple issues with the COMPARE LOGICAL LONG EXTENDED
instruction:
- The test between the two operands is inverted, leading to an inversion
  of the cc values 1 and 2.
- The address and length of an operand continue to be decreased after
  reaching the end of this operand. These values are then wrong write
  back to the registers.
- We should limit the amount of bytes to process, so that interrupts can
  be served correctly.

At the same time rename dest into src1 and src into src3 to match the
operand names and make the code less confusing.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 54 ++++++++++++++++++++++++++++++++---------------
 target/s390x/translate.c  | 20 +++++++++++++-----
 2 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 98a7aa22d3..e992fd993c 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -666,35 +666,55 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
                        uint32_t r3)
 {
     uintptr_t ra = GETPC();
-    uint64_t destlen = get_length(env, r1 + 1);
-    uint64_t dest = get_address(env, r1);
-    uint64_t srclen = get_length(env, r3 + 1);
-    uint64_t src = get_address(env, r3);
+    uint64_t src1len = get_length(env, r1 + 1);
+    uint64_t src1 = get_address(env, r1);
+    uint64_t src3len = get_length(env, r3 + 1);
+    uint64_t src3 = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
+    uint64_t len = MAX(src1len, src3len);
     uint32_t cc = 0;
 
-    if (!(destlen || srclen)) {
+    if (!len) {
         return cc;
     }
 
-    if (srclen > destlen) {
-        srclen = destlen;
+    /* 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;
+        cc = 3;
     }
 
-    for (; destlen || srclen; src++, dest++, destlen--, srclen--) {
-        uint8_t v1 = srclen ? cpu_ldub_data_ra(env, src, ra) : pad;
-        uint8_t v2 = destlen ? cpu_ldub_data_ra(env, dest, ra) : pad;
-        if (v1 != v2) {
-            cc = (v1 < v2) ? 1 : 2;
+    for (; len; len--) {
+        uint8_t v1 = pad;
+        uint8_t v3 = pad;
+
+        if (src1len) {
+            v1 = cpu_ldub_data_ra(env, src1, ra);
+        }
+        if (src3len) {
+            v3 = cpu_ldub_data_ra(env, src3, ra);
+        }
+
+        if (v1 != v3) {
+            cc = (v1 < v3) ? 1 : 2;
             break;
         }
+
+        if (src1len) {
+            src1++;
+            src1len--;
+        }
+        if (src3len) {
+            src3++;
+            src3len--;
+        }
     }
 
-    set_length(env, r1 + 1, destlen);
-    /* can't use srclen here, we trunc'ed it */
-    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
-    set_address(env, r1, dest);
-    set_address(env, r3, src);
+    set_length(env, r1 + 1, src1len);
+    set_length(env, r3 + 1, src3len);
+    set_address(env, r1, src1);
+    set_address(env, r3, src3);
 
     return cc;
 }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 95ca53c1ef..9309e58009 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1917,11 +1917,21 @@ static ExitStatus op_clc(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_clcle(DisasContext *s, DisasOps *o)
 {
-    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
-    TCGv_i32 r3 = tcg_const_i32(get_field(s->fields, r3));
-    gen_helper_clcle(cc_op, cpu_env, r1, o->in2, r3);
-    tcg_temp_free_i32(r1);
-    tcg_temp_free_i32(r3);
+    int r1 = get_field(s->fields, r1);
+    int r3 = get_field(s->fields, r3);
+    TCGv_i32 t1, t3;
+
+    /* r1 and r3 must be even.  */
+    if (r1 & 1 || r3 & 1) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+
+    t1 = tcg_const_i32(r1);
+    t3 = tcg_const_i32(r3);
+    gen_helper_clcle(cc_op, cpu_env, t1, o->in2, t3);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t3);
     set_cc_static(s);
     return NO_EXIT;
 }
-- 
2.11.0

[Qemu-devel] [PATCH v3 18/30] target/s390x: implement COMPARE LOGICAL LONG

[Aurelien Jarno]

As CLCL and CLCLE mostly differ by their operands, use a common do_clcl
helper. Another difference is that CLCL is not interruptible.

Reviewed-by: 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  | 84 +++++++++++++++++++++++++++++++++-------------
 target/s390x/translate.c   | 21 ++++++++++++
 4 files changed, 84 insertions(+), 24 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index ba21e4d870..546beecdf1 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -6,6 +6,7 @@ DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_3(mvcl, i32, env, i32, i32)
+DEF_HELPER_3(clcl, i32, env, i32, i32)
 DEF_HELPER_FLAGS_4(clm, TCG_CALL_NO_WG, i32, env, i32, i32, i64)
 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)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index b40611b75f..1aa2b8b657 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -216,6 +216,8 @@
     C(0xc60e, CLGFRL,  RIL_b, GIE, r1_o, mri2_32u, 0, 0, 0, cmpu64)
     C(0xc607, CLHRL,   RIL_b, GIE, r1_o, mri2_16u, 0, 0, 0, cmpu32)
     C(0xc606, CLGHRL,  RIL_b, GIE, r1_o, mri2_16u, 0, 0, 0, cmpu64)
+/* COMPARE LOGICAL LONG */
+    C(0x0f00, CLCL,    RR_a,  Z,   0, 0, 0, 0, clcl, 0)
 /* COMPARE LOGICAL LONG EXTENDED */
     C(0xa900, CLCLE,   RS_a,  Z,   0, a2, 0, 0, clcle, 0)
 /* COMPARE LOGICAL CHARACTERS UNDER MASK */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e992fd993c..aaa347c121 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -661,17 +661,14 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     return cc;
 }
 
-/* compare logical long extended memcompare insn with padding */
-uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
-                       uint32_t r3)
-{
-    uintptr_t ra = GETPC();
-    uint64_t src1len = get_length(env, r1 + 1);
-    uint64_t src1 = get_address(env, r1);
-    uint64_t src3len = get_length(env, r3 + 1);
-    uint64_t src3 = get_address(env, r3);
-    uint8_t pad = a2 & 0xff;
-    uint64_t len = MAX(src1len, src3len);
+/* compare logical long helper */
+static inline uint32_t do_clcl(CPUS390XState *env,
+                               uint64_t *src1, uint64_t *src1len,
+                               uint64_t *src3, uint64_t *src3len,
+                               uint8_t pad, uint64_t limit,
+                               uintptr_t ra)
+{
+    uint64_t len = MAX(*src1len, *src3len);
     uint32_t cc = 0;
 
     if (!len) {
@@ -679,9 +676,9 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     }
 
     /* 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;
+       amount of work we're willing to do.  */
+    if (len > limit) {
+        len = limit;
         cc = 3;
     }
 
@@ -689,11 +686,11 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
         uint8_t v1 = pad;
         uint8_t v3 = pad;
 
-        if (src1len) {
-            v1 = cpu_ldub_data_ra(env, src1, ra);
+        if (*src1len) {
+            v1 = cpu_ldub_data_ra(env, *src1, ra);
         }
-        if (src3len) {
-            v3 = cpu_ldub_data_ra(env, src3, ra);
+        if (*src3len) {
+            v3 = cpu_ldub_data_ra(env, *src3, ra);
         }
 
         if (v1 != v3) {
@@ -701,16 +698,55 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
             break;
         }
 
-        if (src1len) {
-            src1++;
-            src1len--;
+        if (*src1len) {
+            *src1 += 1;
+            *src1len -= 1;
         }
-        if (src3len) {
-            src3++;
-            src3len--;
+        if (*src3len) {
+            *src3 += 1;
+            *src3len -= 1;
         }
     }
 
+    return cc;
+}
+
+
+/* compare logical long */
+uint32_t HELPER(clcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
+{
+    uintptr_t ra = GETPC();
+    uint64_t src1len = extract64(env->regs[r1 + 1], 0, 24);
+    uint64_t src1 = get_address(env, r1);
+    uint64_t src3len = extract64(env->regs[r2 + 1], 0, 24);
+    uint64_t src3 = get_address(env, r2);
+    uint8_t pad = env->regs[r2 + 1] >> 24;
+    uint32_t cc;
+
+    cc = do_clcl(env, &src1, &src1len, &src3, &src3len, pad, -1, ra);
+
+    env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, src1len);
+    env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, src3len);
+    set_address(env, r1, src1);
+    set_address(env, r2, src3);
+
+    return cc;
+}
+
+/* compare logical long extended memcompare insn with padding */
+uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
+                       uint32_t r3)
+{
+    uintptr_t ra = GETPC();
+    uint64_t src1len = get_length(env, r1 + 1);
+    uint64_t src1 = get_address(env, r1);
+    uint64_t src3len = get_length(env, r3 + 1);
+    uint64_t src3 = get_address(env, r3);
+    uint8_t pad = a2;
+    uint32_t cc;
+
+    cc = do_clcl(env, &src1, &src1len, &src3, &src3len, pad, 0x2000, ra);
+
     set_length(env, r1 + 1, src1len);
     set_length(env, r3 + 1, src3len);
     set_address(env, r1, src1);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 9309e58009..999d716f61 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1915,6 +1915,27 @@ static ExitStatus op_clc(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_clcl(DisasContext *s, DisasOps *o)
+{
+    int r1 = get_field(s->fields, r1);
+    int r2 = get_field(s->fields, r2);
+    TCGv_i32 t1, t2;
+
+    /* r1 and r2 must be even.  */
+    if (r1 & 1 || r2 & 1) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+
+    t1 = tcg_const_i32(r1);
+    t2 = tcg_const_i32(r2);
+    gen_helper_clcl(cc_op, cpu_env, t1, t2);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t2);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_clcle(DisasContext *s, DisasOps *o)
 {
     int r1 = get_field(s->fields, r1);
-- 
2.11.0

[Qemu-devel] [PATCH v3 19/30] target/s390x: fix adj_len_to_page

[Aurelien Jarno]

adj_len_to_page doesn't return the correct result when the address
is already page aligned and the length is bigger than a page. Fix that.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index aaa347c121..6dfa087ff1 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -61,7 +61,7 @@ static inline uint32_t adj_len_to_page(uint32_t len, uint64_t addr)
 {
 #ifndef CONFIG_USER_ONLY
     if ((addr & ~TARGET_PAGE_MASK) + len - 1 >= TARGET_PAGE_SIZE) {
-        return -addr & ~TARGET_PAGE_MASK;
+        return -(addr | TARGET_PAGE_MASK);
     }
 #endif
     return len;
-- 
2.11.0

[Qemu-devel] [PATCH v3 20/30] target/s390x: improve MOVE LONG and MOVE LONG EXTENDED

[Aurelien Jarno]

As MVCL and MVCLE only differ by their operands, use a common
do_mvcl helper. Optimize it calling fast_memmove and fast_memset.
Correctly write back addresses. Check that r1 and r2/r3 registers
are even.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 90 +++++++++++++++++++++--------------------------
 target/s390x/translate.c  | 40 +++++++++++++++------
 2 files changed, 70 insertions(+), 60 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6dfa087ff1..cb0ec3eebf 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -576,49 +576,60 @@ void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     }
 }
 
-/* move long */
-uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
+/* move long helper */
+static inline uint32_t do_mvcl(CPUS390XState *env,
+                               uint64_t *dest, uint64_t *destlen,
+                               uint64_t *src, uint64_t *srclen,
+                               uint8_t pad, uintptr_t ra)
 {
-    uintptr_t ra = GETPC();
-    uint64_t destlen = env->regs[r1 + 1] & 0xffffff;
-    uint64_t dest = get_address(env, r1);
-    uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
-    uint64_t src = get_address(env, r2);
-    uint8_t pad = env->regs[r2 + 1] >> 24;
-    uint8_t v;
+    uint64_t len = MIN(*srclen, *destlen);
     uint32_t cc;
 
-    if (destlen == srclen) {
+    if (*destlen == *srclen) {
         cc = 0;
-    } else if (destlen < srclen) {
+    } else if (*destlen < *srclen) {
         cc = 1;
     } else {
         cc = 2;
     }
 
-    if (srclen > destlen) {
-        srclen = destlen;
-    }
+    /* Copy the src array */
+    fast_memmove(env, *dest, *src, len, ra);
+    *src += len;
+    *srclen -= len;
+    *dest += len;
+    *destlen -= len;
 
-    for (; destlen && srclen; src++, dest++, destlen--, srclen--) {
-        v = cpu_ldub_data_ra(env, src, ra);
-        cpu_stb_data_ra(env, dest, v, ra);
-    }
+    /* Pad the remaining area */
+    fast_memset(env, *dest, pad, *destlen, ra);
+    *dest += *destlen;
+    *destlen = 0;
 
-    for (; destlen; dest++, destlen--) {
-        cpu_stb_data_ra(env, dest, pad, ra);
-    }
+    return cc;
+}
 
-    env->regs[r1 + 1] = destlen;
-    /* can't use srclen here, we trunc'ed it */
-    env->regs[r2 + 1] -= src - env->regs[r2];
+/* move long */
+uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
+{
+    uintptr_t ra = GETPC();
+    uint64_t destlen = env->regs[r1 + 1] & 0xffffff;
+    uint64_t dest = get_address(env, r1);
+    uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
+    uint64_t src = get_address(env, r2);
+    uint8_t pad = env->regs[r2 + 1] >> 24;
+    uint32_t cc;
+
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
+
+    env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
+    env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, srclen);
     set_address(env, r1, dest);
     set_address(env, r2, src);
 
     return cc;
 }
 
-/* move long extended another memcopy insn with more bells and whistles */
+/* move long extended */
 uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
                        uint32_t r3)
 {
@@ -627,34 +638,13 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     uint64_t dest = get_address(env, r1);
     uint64_t srclen = get_length(env, r3 + 1);
     uint64_t src = get_address(env, r3);
-    uint8_t pad = a2 & 0xff;
-    uint8_t v;
+    uint8_t pad = a2;
     uint32_t cc;
 
-    if (destlen == srclen) {
-        cc = 0;
-    } else if (destlen < srclen) {
-        cc = 1;
-    } else {
-        cc = 2;
-    }
-
-    if (srclen > destlen) {
-        srclen = destlen;
-    }
-
-    for (; destlen && srclen; src++, dest++, destlen--, srclen--) {
-        v = cpu_ldub_data_ra(env, src, ra);
-        cpu_stb_data_ra(env, dest, v, ra);
-    }
-
-    for (; destlen; dest++, destlen--) {
-        cpu_stb_data_ra(env, dest, pad, ra);
-    }
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
 
-    set_length(env, r1 + 1 , destlen);
-    /* can't use srclen here, we trunc'ed it */
-    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
+    set_length(env, r1 + 1, destlen);
+    set_length(env, r3 + 1, srclen);
     set_address(env, r1, dest);
     set_address(env, r3, src);
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 999d716f61..729d25d8f8 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2981,22 +2981,42 @@ static ExitStatus op_mvcin(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_mvcl(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_mvcl(cc_op, cpu_env, r1, r2);
-    tcg_temp_free_i32(r1);
-    tcg_temp_free_i32(r2);
+    int r1 = get_field(s->fields, r1);
+    int r2 = get_field(s->fields, r2);
+    TCGv_i32 t1, t2;
+
+    /* r1 and r2 must be even.  */
+    if (r1 & 1 || r2 & 1) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+
+    t1 = tcg_const_i32(r1);
+    t2 = tcg_const_i32(r2);
+    gen_helper_mvcl(cc_op, cpu_env, t1, t2);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t2);
     set_cc_static(s);
     return NO_EXIT;
 }
 
 static ExitStatus op_mvcle(DisasContext *s, DisasOps *o)
 {
-    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
-    TCGv_i32 r3 = tcg_const_i32(get_field(s->fields, r3));
-    gen_helper_mvcle(cc_op, cpu_env, r1, o->in2, r3);
-    tcg_temp_free_i32(r1);
-    tcg_temp_free_i32(r3);
+    int r1 = get_field(s->fields, r1);
+    int r3 = get_field(s->fields, r3);
+    TCGv_i32 t1, t3;
+
+    /* r1 and r3 must be even.  */
+    if (r1 & 1 || r3 & 1) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+
+    t1 = tcg_const_i32(r1);
+    t3 = tcg_const_i32(r3);
+    gen_helper_mvcle(cc_op, cpu_env, t1, o->in2, t3);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t3);
     set_cc_static(s);
     return NO_EXIT;
 }
-- 
2.11.0

[Qemu-devel] [PATCH v3 21/30] target/s390x: implement COMPARE LOGICAL LONG UNICODE

[Aurelien Jarno]

For that we need to make program_interrupt available to qemu-user.
Fortunately there is almost nothing to change as both kvm_enabled and
CONFIG_KVM evaluate to false in that case.

Reviewed-by: 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  | 76 ++++++++++++++++++++++++++++++++++++++--------
 target/s390x/misc_helper.c |  4 +--
 target/s390x/translate.c   | 22 ++++++++++++++
 5 files changed, 90 insertions(+), 15 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 546beecdf1..509e3f381c 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -24,6 +24,7 @@ DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_FLAGS_4(lam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_4(mvcle, i32, env, i32, i64, i32)
 DEF_HELPER_4(clcle, i32, env, i32, i64, i32)
+DEF_HELPER_4(clclu, i32, env, i32, i64, i32)
 DEF_HELPER_3(cegb, i64, env, s64, i32)
 DEF_HELPER_3(cdgb, i64, env, s64, i32)
 DEF_HELPER_3(cxgb, i64, env, s64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 1aa2b8b657..c781a97a3a 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -220,6 +220,8 @@
     C(0x0f00, CLCL,    RR_a,  Z,   0, 0, 0, 0, clcl, 0)
 /* COMPARE LOGICAL LONG EXTENDED */
     C(0xa900, CLCLE,   RS_a,  Z,   0, a2, 0, 0, clcle, 0)
+/* COMPARE LOGICAL LONG UNICODE */
+    C(0xeb8f, CLCLU,   RSY_a, E2,  0, a2, 0, 0, clclu, 0)
 /* COMPARE LOGICAL CHARACTERS UNDER MASK */
     C(0xbd00, CLM,     RS_b,  Z,   r1_o, a2, 0, 0, clm, 0)
     C(0xeb21, CLMY,    RSY_b, LD,  r1_o, a2, 0, 0, clm, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index cb0ec3eebf..59992faf69 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -67,6 +67,32 @@ static inline uint32_t adj_len_to_page(uint32_t len, uint64_t addr)
     return len;
 }
 
+/* Trigger a SPECIFICATION exception if an address or a length is not
+   naturally aligned.  */
+static inline void check_alignment(CPUS390XState *env, uint64_t v,
+                                   int wordsize, uintptr_t ra)
+{
+    if (v % wordsize) {
+        CPUState *cs = CPU(s390_env_get_cpu(env));
+        cpu_restore_state(cs, ra);
+        program_interrupt(env, PGM_SPECIFICATION, 6);
+    }
+}
+
+/* Load a value from memory according to its size.  */
+static inline uint64_t cpu_ldusize_data_ra(CPUS390XState *env, uint64_t addr,
+                                           int wordsize, uintptr_t ra)
+{
+    switch (wordsize) {
+    case 1:
+        return cpu_ldub_data_ra(env, addr, ra);
+    case 2:
+        return cpu_lduw_data_ra(env, addr, ra);
+    default:
+        abort();
+    }
+}
+
 static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
                         uint32_t l, uintptr_t ra)
 {
@@ -655,12 +681,14 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 static inline uint32_t do_clcl(CPUS390XState *env,
                                uint64_t *src1, uint64_t *src1len,
                                uint64_t *src3, uint64_t *src3len,
-                               uint8_t pad, uint64_t limit,
-                               uintptr_t ra)
+                               uint16_t pad, uint64_t limit,
+                               int wordsize, uintptr_t ra)
 {
     uint64_t len = MAX(*src1len, *src3len);
     uint32_t cc = 0;
 
+    check_alignment(env, *src1len | *src3len, wordsize, ra);
+
     if (!len) {
         return cc;
     }
@@ -672,15 +700,15 @@ static inline uint32_t do_clcl(CPUS390XState *env,
         cc = 3;
     }
 
-    for (; len; len--) {
-        uint8_t v1 = pad;
-        uint8_t v3 = pad;
+    for (; len; len -= wordsize) {
+        uint16_t v1 = pad;
+        uint16_t v3 = pad;
 
         if (*src1len) {
-            v1 = cpu_ldub_data_ra(env, *src1, ra);
+            v1 = cpu_ldusize_data_ra(env, *src1, wordsize, ra);
         }
         if (*src3len) {
-            v3 = cpu_ldub_data_ra(env, *src3, ra);
+            v3 = cpu_ldusize_data_ra(env, *src3, wordsize, ra);
         }
 
         if (v1 != v3) {
@@ -689,12 +717,12 @@ static inline uint32_t do_clcl(CPUS390XState *env,
         }
 
         if (*src1len) {
-            *src1 += 1;
-            *src1len -= 1;
+            *src1 += wordsize;
+            *src1len -= wordsize;
         }
         if (*src3len) {
-            *src3 += 1;
-            *src3len -= 1;
+            *src3 += wordsize;
+            *src3len -= wordsize;
         }
     }
 
@@ -713,7 +741,7 @@ uint32_t HELPER(clcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     uint8_t pad = env->regs[r2 + 1] >> 24;
     uint32_t cc;
 
-    cc = do_clcl(env, &src1, &src1len, &src3, &src3len, pad, -1, ra);
+    cc = do_clcl(env, &src1, &src1len, &src3, &src3len, pad, -1, 1, ra);
 
     env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, src1len);
     env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, src3len);
@@ -735,7 +763,29 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     uint8_t pad = a2;
     uint32_t cc;
 
-    cc = do_clcl(env, &src1, &src1len, &src3, &src3len, pad, 0x2000, ra);
+    cc = do_clcl(env, &src1, &src1len, &src3, &src3len, pad, 0x2000, 1, ra);
+
+    set_length(env, r1 + 1, src1len);
+    set_length(env, r3 + 1, src3len);
+    set_address(env, r1, src1);
+    set_address(env, r3, src3);
+
+    return cc;
+}
+
+/* compare logical long unicode memcompare insn with padding */
+uint32_t HELPER(clclu)(CPUS390XState *env, uint32_t r1, uint64_t a2,
+                       uint32_t r3)
+{
+    uintptr_t ra = GETPC();
+    uint64_t src1len = get_length(env, r1 + 1);
+    uint64_t src1 = get_address(env, r1);
+    uint64_t src3len = get_length(env, r3 + 1);
+    uint64_t src3 = get_address(env, r3);
+    uint16_t pad = a2;
+    uint32_t cc = 0;
+
+    cc = do_clcl(env, &src1, &src1len, &src3, &src3len, pad, 0x1000, 2, ra);
 
     set_length(env, r1 + 1, src1len);
     set_length(env, r3 + 1, src3len);
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 23ec52cf35..f083c8d3cf 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -80,8 +80,6 @@ void HELPER(exception)(CPUS390XState *env, uint32_t excp)
     cpu_loop_exit(cs);
 }
 
-#ifndef CONFIG_USER_ONLY
-
 void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
@@ -108,6 +106,8 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
     }
 }
 
+#ifndef CONFIG_USER_ONLY
+
 /* SCLP service call */
 uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 729d25d8f8..892949a05f 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1200,6 +1200,7 @@ typedef enum DisasFacility {
     FAC_ILA,                /* interlocked access facility 1 */
     FAC_LPP,                /* load-program-parameter */
     FAC_DAT_ENH,            /* DAT-enhancement */
+    FAC_E2,                 /* extended-translation facility 2 */
 } DisasFacility;
 
 struct DisasInsn {
@@ -1957,6 +1958,27 @@ static ExitStatus op_clcle(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_clclu(DisasContext *s, DisasOps *o)
+{
+    int r1 = get_field(s->fields, r1);
+    int r3 = get_field(s->fields, r3);
+    TCGv_i32 t1, t3;
+
+    /* r1 and r3 must be even.  */
+    if (r1 & 1 || r3 & 1) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+
+    t1 = tcg_const_i32(r1);
+    t3 = tcg_const_i32(r3);
+    gen_helper_clclu(cc_op, cpu_env, t1, o->in2, t3);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t3);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_clm(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 m3 = tcg_const_i32(get_field(s->fields, m3));
-- 
2.11.0

[Qemu-devel] [PATCH v3 22/30] target/s390x: implement MOVE LONG UNICODE

[Aurelien Jarno]

Reviewed-by: 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  | 47 ++++++++++++++++++++++++++++++++++++++++------
 target/s390x/translate.c   | 21 +++++++++++++++++++++
 4 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 509e3f381c..5811911c34 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -23,6 +23,7 @@ DEF_HELPER_4(ex, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_FLAGS_4(lam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_4(mvcle, i32, env, i32, i64, i32)
+DEF_HELPER_4(mvclu, i32, env, i32, i64, i32)
 DEF_HELPER_4(clcle, i32, env, i32, i64, i32)
 DEF_HELPER_4(clclu, i32, env, i32, i64, i32)
 DEF_HELPER_3(cegb, i64, env, s64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index c781a97a3a..e3b7b78834 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -582,6 +582,8 @@
     C(0x0e00, MVCL,    RR_a,  Z,   0, 0, 0, 0, mvcl, 0)
 /* MOVE LONG EXTENDED */
     C(0xa800, MVCLE,   RS_a,  Z,   0, a2, 0, 0, mvcle, 0)
+/* MOVE LONG UNICODE */
+    C(0xeb8e, MVCLU,   RSY_a, E2,  0, a2, 0, 0, mvclu, 0)
 /* MOVE NUMERICS */
     C(0xd100, MVN,     SS_a,  Z,   la1, a2, 0, 0, mvn, 0)
 /* MOVE PAGE */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 59992faf69..a2d65cfbbb 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -606,7 +606,7 @@ void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 static inline uint32_t do_mvcl(CPUS390XState *env,
                                uint64_t *dest, uint64_t *destlen,
                                uint64_t *src, uint64_t *srclen,
-                               uint8_t pad, uintptr_t ra)
+                               uint16_t pad, int wordsize, uintptr_t ra)
 {
     uint64_t len = MIN(*srclen, *destlen);
     uint32_t cc;
@@ -627,9 +627,22 @@ static inline uint32_t do_mvcl(CPUS390XState *env,
     *destlen -= len;
 
     /* Pad the remaining area */
-    fast_memset(env, *dest, pad, *destlen, ra);
-    *dest += *destlen;
-    *destlen = 0;
+    if (wordsize == 1) {
+        fast_memset(env, *dest, pad, *destlen, ra);
+        *dest += *destlen;
+        *destlen = 0;
+    } else {
+        /* If remaining length is odd, pad with odd byte first.  */
+        if (*destlen & 1) {
+            cpu_stb_data_ra(env, *dest, pad & 0xff, ra);
+            *dest += 1;
+            *destlen -= 1;
+        }
+        /* The remaining length is even, pad using words.  */
+        for (; *destlen; *dest += 2, *destlen -= 2) {
+            cpu_stw_data_ra(env, *dest, pad, ra);
+        }
+    }
 
     return cc;
 }
@@ -645,7 +658,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     uint8_t pad = env->regs[r2 + 1] >> 24;
     uint32_t cc;
 
-    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, 1, ra);
 
     env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
     env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, srclen);
@@ -667,7 +680,29 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     uint8_t pad = a2;
     uint32_t cc;
 
-    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, 1, ra);
+
+    set_length(env, r1 + 1, destlen);
+    set_length(env, r3 + 1, srclen);
+    set_address(env, r1, dest);
+    set_address(env, r3, src);
+
+    return cc;
+}
+
+/* move long unicode */
+uint32_t HELPER(mvclu)(CPUS390XState *env, uint32_t r1, uint64_t a2,
+                       uint32_t r3)
+{
+    uintptr_t ra = GETPC();
+    uint64_t destlen = get_length(env, r1 + 1);
+    uint64_t dest = get_address(env, r1);
+    uint64_t srclen = get_length(env, r3 + 1);
+    uint64_t src = get_address(env, r3);
+    uint16_t pad = a2;
+    uint32_t cc;
+
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, 2, ra);
 
     set_length(env, r1 + 1, destlen);
     set_length(env, r3 + 1, srclen);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 892949a05f..b160a0cad7 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3043,6 +3043,27 @@ static ExitStatus op_mvcle(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvclu(DisasContext *s, DisasOps *o)
+{
+    int r1 = get_field(s->fields, r1);
+    int r3 = get_field(s->fields, r3);
+    TCGv_i32 t1, t3;
+
+    /* r1 and r3 must be even.  */
+    if (r1 & 1 || r3 & 1) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+
+    t1 = tcg_const_i32(r1);
+    t3 = tcg_const_i32(r3);
+    gen_helper_mvclu(cc_op, cpu_env, t1, o->in2, t3);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t3);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_mvcp(DisasContext *s, DisasOps *o)
 {
-- 
2.11.0

[Qemu-devel] [PATCH v3 23/30] target/s390x: implement PACK ASCII

[Aurelien Jarno]

Reviewed-by: 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  | 35 +++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   | 16 ++++++++++++++++
 4 files changed, 54 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 5811911c34..78980643d1 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -83,6 +83,7 @@ DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
 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(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 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_4(tre, i64, env, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index e3b7b78834..41431affe6 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -667,6 +667,8 @@
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
     C(0xf200, PACK,    SS_a,  Z,   la1, a2, 0, 0, pack, 0)
+/* PACK ASCII */
+    C(0xe900, PKA,     SS_f,  E2,  la1, a2, 0, 0, pka, 0)
 
 /* PREFETCH */
     /* Implemented as nops of course.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index a2d65cfbbb..425d4b67df 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -914,6 +914,41 @@ void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
     }
 }
 
+void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    uintptr_t ra = GETPC();
+    int i;
+    /* The destination operand is always 16 bytes long.  */
+    const int destlen = 16;
+
+    /* The operands are processed from right to left.  */
+    src += srclen - 1;
+    dest += destlen - 1;
+
+    for (i = 0; i < destlen; i++) {
+        uint8_t b = 0;
+
+        /* Start with a positive sign */
+        if (i == 0) {
+            b = 0xc;
+        } else if (srclen > 1) {
+            b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
+            src--;
+            srclen--;
+        }
+
+        if (srclen > 1) {
+            b |= cpu_ldub_data_ra(env, src, ra) << 4;
+            src--;
+            srclen--;
+        }
+
+        cpu_stb_data_ra(env, dest, b, ra);
+        dest--;
+    }
+}
+
 void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
                   uint64_t src)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index b160a0cad7..5093995f9a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3301,6 +3301,22 @@ static ExitStatus op_pack(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_pka(DisasContext *s, DisasOps *o)
+{
+    int l2 = get_field(s->fields, l2) + 1;
+    TCGv_i32 l;
+
+    /* The length must not exceed 32 bytes.  */
+    if (l2 > 32) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l2);
+    gen_helper_pka(cpu_env, o->addr1, o->in2, l);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_popcnt(DisasContext *s, DisasOps *o)
 {
     gen_helper_popcnt(o->out, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 24/30] target/s390x: implement PACK UNICODE

[Aurelien Jarno]

Use a common helper with PACK ASCII as the differences are limited to
the stride of the source operand.

Reviewed-by: 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  | 30 +++++++++++++++++++++---------
 target/s390x/translate.c   | 16 ++++++++++++++++
 4 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 78980643d1..5b61a0de3c 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -84,6 +84,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(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
+DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 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_4(tre, i64, env, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 41431affe6..16f788c86a 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -669,6 +669,8 @@
     C(0xf200, PACK,    SS_a,  Z,   la1, a2, 0, 0, pack, 0)
 /* PACK ASCII */
     C(0xe900, PKA,     SS_f,  E2,  la1, a2, 0, 0, pka, 0)
+/* PACK UNICODE */
+    C(0xe100, PKU,     SS_f,  E2,  la1, a2, 0, 0, pku, 0)
 
 /* PREFETCH */
     /* Implemented as nops of course.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 425d4b67df..d827a12808 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -914,10 +914,9 @@ void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
     }
 }
 
-void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
-                 uint32_t srclen)
+static inline void do_pkau(CPUS390XState *env, uint64_t dest, uint64_t src,
+                           uint32_t srclen, int ssize, uintptr_t ra)
 {
-    uintptr_t ra = GETPC();
     int i;
     /* The destination operand is always 16 bytes long.  */
     const int destlen = 16;
@@ -932,16 +931,16 @@ void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
         /* Start with a positive sign */
         if (i == 0) {
             b = 0xc;
-        } else if (srclen > 1) {
+        } else if (srclen > ssize) {
             b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
-            src--;
-            srclen--;
+            src -= ssize;
+            srclen -= ssize;
         }
 
-        if (srclen > 1) {
+        if (srclen > ssize) {
             b |= cpu_ldub_data_ra(env, src, ra) << 4;
-            src--;
-            srclen--;
+            src -= ssize;
+            srclen -= ssize;
         }
 
         cpu_stb_data_ra(env, dest, b, ra);
@@ -949,6 +948,19 @@ void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
     }
 }
 
+
+void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    do_pkau(env, dest, src, srclen, 1, GETPC());
+}
+
+void HELPER(pku)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    do_pkau(env, dest, src, srclen, 2, GETPC());
+}
+
 void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
                   uint64_t src)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 5093995f9a..d8b0515f17 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3317,6 +3317,22 @@ static ExitStatus op_pka(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_pku(DisasContext *s, DisasOps *o)
+{
+    int l2 = get_field(s->fields, l2) + 1;
+    TCGv_i32 l;
+
+    /* The length must be even and should not exceed 64 bytes.  */
+    if ((l2 & 1) || (l2 > 64)) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l2);
+    gen_helper_pku(cpu_env, o->addr1, o->in2, l);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_popcnt(DisasContext *s, DisasOps *o)
 {
     gen_helper_popcnt(o->out, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v3 25/30] target/s390x: implement UNPACK ASCII

[Aurelien Jarno]

Reviewed-by: 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  | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   | 17 ++++++++++++++++
 4 files changed, 71 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 5b61a0de3c..42c3de6840 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -86,6 +86,7 @@ DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(unpka, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
 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)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 16f788c86a..33434a31d0 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -875,6 +875,8 @@
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
     C(0xf300, UNPK,    SS_a,  Z,   la1, a2, 0, 0, unpk, 0)
+/* UNPACK ASCII */
+    C(0xea00, UNPKA,   SS_a,  E2,  la1, a2, 0, 0, unpka, 0)
 
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index d827a12808..1c5f29c67a 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1008,6 +1008,57 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
     }
 }
 
+uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
+                       uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+    uint32_t cc;
+    uint8_t b;
+    /* The source operand is always 16 bytes long.  */
+    const int srclen = 16;
+
+    /* The operands are processed from right to left.  */
+    src += srclen - 1;
+    dest += destlen - 1;
+
+    /* Check for the sign.  */
+    b = cpu_ldub_data_ra(env, src, ra);
+    src--;
+    switch (b & 0xf) {
+    case 0xa:
+    case 0xc:
+    case 0xe ... 0xf:
+        cc = 0;  /* plus */
+        break;
+    case 0xb:
+    case 0xd:
+        cc = 1;  /* minus */
+        break;
+    default:
+    case 0x0 ... 0x9:
+        cc = 3;  /* invalid */
+        break;
+    }
+
+    /* Now pad every nibble with 0x30, advancing one nibble at a time. */
+    for (i = 0; i < destlen; i++) {
+        if (i == 31) {
+            /* If length is 32 bytes, the leftmost byte is 0. */
+            b = 0;
+        } else if (i % 2) {
+            b = cpu_ldub_data_ra(env, src, ra);
+            src--;
+        } else {
+            b >>= 4;
+        }
+        cpu_stb_data_ra(env, dest, 0x30 + (b & 0xf), ra);
+        dest--;
+    }
+
+    return cc;
+}
+
 static uint32_t do_helper_tr(CPUS390XState *env, uint32_t len, uint64_t array,
                              uint64_t trans, uintptr_t ra)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index d8b0515f17..2ff666573e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4357,6 +4357,23 @@ static ExitStatus op_unpk(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_unpka(DisasContext *s, DisasOps *o)
+{
+    int l1 = get_field(s->fields, l1) + 1;
+    TCGv_i32 l;
+
+    /* The length must not exceed 32 bytes.  */
+    if (l1 > 32) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l1);
+    gen_helper_unpka(cc_op, cpu_env, o->addr1, l, o->in2);
+    tcg_temp_free_i32(l);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_xc(DisasContext *s, DisasOps *o)
 {
     int d1 = get_field(s->fields, d1);
-- 
2.11.0

[Qemu-devel] [PATCH v3 26/30] target/s390x: implement UNPACK UNICODE

[Aurelien Jarno]

Reviewed-by: 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  | 49 ++++++++++++++++++++++++++++++++++++----------
 target/s390x/translate.c   | 18 +++++++++++++++++
 4 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 42c3de6840..dc85a1435f 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -87,6 +87,7 @@ DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(unpka, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
+DEF_HELPER_FLAGS_4(unpku, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
 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)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 33434a31d0..683f91bf7f 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -877,6 +877,8 @@
     C(0xf300, UNPK,    SS_a,  Z,   la1, a2, 0, 0, unpk, 0)
 /* UNPACK ASCII */
     C(0xea00, UNPKA,   SS_a,  E2,  la1, a2, 0, 0, unpka, 0)
+/* UNPACK UNICODE */
+    C(0xe200, UNPKU,   SS_a,  E2,  la1, a2, 0, 0, unpku, 0)
 
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 1c5f29c67a..5cb1a8ab43 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -93,6 +93,23 @@ static inline uint64_t cpu_ldusize_data_ra(CPUS390XState *env, uint64_t addr,
     }
 }
 
+/* Store a to memory according to its size.  */
+static inline void cpu_stsize_data_ra(CPUS390XState *env, uint64_t addr,
+                                      uint64_t value, int wordsize,
+                                      uintptr_t ra)
+{
+    switch (wordsize) {
+    case 1:
+        cpu_stb_data_ra(env, addr, value, ra);
+        break;
+    case 2:
+        cpu_stw_data_ra(env, addr, value, ra);
+        break;
+    default:
+        abort();
+    }
+}
+
 static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
                         uint32_t l, uintptr_t ra)
 {
@@ -1008,10 +1025,10 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
     }
 }
 
-uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
-                       uint64_t src)
+static inline uint32_t do_unpkau(CPUS390XState *env, uint64_t dest,
+                                 uint32_t destlen, int dsize, uint64_t src,
+                                 uintptr_t ra)
 {
-    uintptr_t ra = GETPC();
     int i;
     uint32_t cc;
     uint8_t b;
@@ -1020,7 +1037,7 @@ uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
 
     /* The operands are processed from right to left.  */
     src += srclen - 1;
-    dest += destlen - 1;
+    dest += destlen - dsize;
 
     /* Check for the sign.  */
     b = cpu_ldub_data_ra(env, src, ra);
@@ -1042,23 +1059,35 @@ uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
     }
 
     /* Now pad every nibble with 0x30, advancing one nibble at a time. */
-    for (i = 0; i < destlen; i++) {
-        if (i == 31) {
-            /* If length is 32 bytes, the leftmost byte is 0. */
+    for (i = 0; i < destlen; i += dsize) {
+        if (i == (31 * dsize)) {
+            /* If length is 32/64 bytes, the leftmost byte is 0. */
             b = 0;
-        } else if (i % 2) {
+        } else if (i % (2 * dsize)) {
             b = cpu_ldub_data_ra(env, src, ra);
             src--;
         } else {
             b >>= 4;
         }
-        cpu_stb_data_ra(env, dest, 0x30 + (b & 0xf), ra);
-        dest--;
+        cpu_stsize_data_ra(env, dest, 0x30 + (b & 0xf), dsize, ra);
+        dest -= dsize;
     }
 
     return cc;
 }
 
+uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
+                       uint64_t src)
+{
+    return do_unpkau(env, dest, destlen, 1, src, GETPC());
+}
+
+uint32_t HELPER(unpku)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
+                       uint64_t src)
+{
+    return do_unpkau(env, dest, destlen, 2, src, GETPC());
+}
+
 static uint32_t do_helper_tr(CPUS390XState *env, uint32_t len, uint64_t array,
                              uint64_t trans, uintptr_t ra)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 2ff666573e..747d4ebc10 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4374,6 +4374,24 @@ static ExitStatus op_unpka(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_unpku(DisasContext *s, DisasOps *o)
+{
+    int l1 = get_field(s->fields, l1) + 1;
+    TCGv_i32 l;
+
+    /* The length must be even and should not exceed 64 bytes.  */
+    if ((l1 & 1) || (l1 > 64)) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l1);
+    gen_helper_unpku(cc_op, cpu_env, o->addr1, l, o->in2);
+    tcg_temp_free_i32(l);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
+
 static ExitStatus op_xc(DisasContext *s, DisasOps *o)
 {
     int d1 = get_field(s->fields, d1);
-- 
2.11.0

[Qemu-devel] [PATCH v3 27/30] target/s390x: implement TEST DECIMAL

[Aurelien Jarno]

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  3 +++
 target/s390x/mem_helper.c  | 23 +++++++++++++++++++++++
 target/s390x/translate.c   |  9 +++++++++
 4 files changed, 36 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index dc85a1435f..c043be6365 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(unpka, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
 DEF_HELPER_FLAGS_4(unpku, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
+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)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 683f91bf7f..7f554ab133 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -856,6 +856,9 @@
     C(0xed11, TCDB,    RXE,   Z,   f1_o, a2, 0, 0, tcdb, 0)
     C(0xed12, TCXB,    RXE,   Z,   x1_o, a2, 0, 0, tcxb, 0)
 
+/* TEST DECIMAL */
+    C(0xebc0, TP,      RSL,   E2,  la1, 0, 0, 0, tp, 0)
+
 /* TEST UNDER MASK */
     C(0x9100, TM,      SI,    Z,   m1_8u, i2_8u, 0, 0, 0, tm32)
     C(0xeb51, TMY,     SIY,   LD,  m1_8u, i2_8u, 0, 0, 0, tm32)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 5cb1a8ab43..a9297c1af1 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1088,6 +1088,29 @@ uint32_t HELPER(unpku)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
     return do_unpkau(env, dest, destlen, 2, src, GETPC());
 }
 
+uint32_t HELPER(tp)(CPUS390XState *env, uint64_t dest, uint32_t destlen)
+{
+    uintptr_t ra = GETPC();
+    uint32_t cc = 0;
+    int i;
+
+    for (i = 0; i < destlen; i++) {
+        uint8_t b = cpu_ldub_data_ra(env, dest + i, ra);
+        /* digit */
+        cc |= (b & 0xf0) > 0x90 ? 2 : 0;
+
+        if (i == (destlen - 1)) {
+            /* sign */
+            cc |= (b & 0xf) < 0xa ? 1 : 0;
+        } else {
+            /* digit */
+            cc |= (b & 0xf) > 0x9 ? 2 : 0;
+        }
+    }
+
+    return cc;
+}
+
 static uint32_t do_helper_tr(CPUS390XState *env, uint32_t len, uint64_t array,
                              uint64_t trans, uintptr_t ra)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 747d4ebc10..a7c564fa93 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4313,6 +4313,15 @@ static ExitStatus op_tprot(DisasContext *s, DisasOps *o)
 
 #endif
 
+static ExitStatus op_tp(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l1 = tcg_const_i32(get_field(s->fields, l1) + 1);
+    gen_helper_tp(cc_op, cpu_env, o->addr1, l1);
+    tcg_temp_free_i32(l1);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_tr(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-- 
2.11.0

[Qemu-devel] [PATCH v3 28/30] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  9 +++++++++
 target/s390x/mem_helper.c  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   | 30 ++++++++++++++++++++++++++++++
 4 files changed, 85 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index c043be6365..e13961137c 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -92,6 +92,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_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)
 DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 7f554ab133..73dd05daf0 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -874,6 +874,15 @@
 /* TRANSLATE EXTENDED */
     C(0xb2a5, TRE,     RRE,   Z,   0, r2, r1_P, 0, tre, 0)
 
+/* TRANSLATE ONE TO ONE */
+    C(0xb993, TROO,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+/* TRANSLATE ONE TO TWO */
+    C(0xb992, TROT,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+/* TRANSLATE TWO TO ONE */
+    C(0xb991, TRTO,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+/* TRANSLATE TWO TO TWO */
+    C(0xb990, TRTT,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+
 /* UNPACK */
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index a9297c1af1..6fb97fa8f8 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1196,6 +1196,51 @@ uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array,
     return do_helper_trt(env, len, array, trans, GETPC());
 }
 
+/* Translate one/two to one/two */
+uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
+                      uint32_t tst, uint32_t sizes)
+{
+    uintptr_t ra = GETPC();
+    int dsize = (sizes & 1) ? 1 : 2;
+    int ssize = (sizes & 2) ? 1 : 2;
+    uint64_t tbl = get_address(env, 1) & ~7;
+    uint64_t dst = get_address(env, r1);
+    uint64_t len = get_length(env, r1 + 1);
+    uint64_t src = get_address(env, r2);
+    uint32_t cc = 3;
+    int i;
+
+    check_alignment(env, len, ssize, ra);
+
+    /* Lest we fail to service interrupts in a timely manner, */
+    /* limit the amount of work we're willing to do.   */
+    for (i = 0; i < 0x2000; i++) {
+        uint16_t sval = cpu_ldusize_data_ra(env, src, ssize, ra);
+        uint64_t tble = tbl + (sval * dsize);
+        uint16_t dval = cpu_ldusize_data_ra(env, tble, dsize, ra);
+        if (dval == tst) {
+            cc = 1;
+            break;
+        }
+        cpu_stsize_data_ra(env, dst, dval, dsize, ra);
+
+        len -= ssize;
+        src += ssize;
+        dst += dsize;
+
+        if (len == 0) {
+            cc = 0;
+            break;
+        }
+    }
+
+    set_address(env, r1, dst);
+    set_length(env, r1 + 1, len);
+    set_address(env, r2, src);
+
+    return cc;
+}
+
 void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
                   uint32_t r1, uint32_t r3)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a7c564fa93..8702cc8cc7 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4348,6 +4348,36 @@ static ExitStatus op_trt(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_trXX(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));
+    TCGv_i32 sizes = tcg_const_i32(s->insn->opc & 3);
+    TCGv_i32 tst = tcg_temp_new_i32();
+    int m3 = get_field(s->fields, m3);
+
+    /* XXX: the C bit in M3 should be considered as 0 when the
+       ETF2-enhancement facility is not installed.  */
+    if (m3 & 1) {
+        tcg_gen_movi_i32(tst, -1);
+    } else {
+        tcg_gen_extrl_i64_i32(tst, regs[0]);
+        if (s->insn->opc & 3) {
+            tcg_gen_ext8u_i32(tst, tst);
+        } else {
+            tcg_gen_ext16u_i32(tst, tst);
+        }
+    }
+    gen_helper_trXX(cc_op, cpu_env, r1, r2, tst, sizes);
+
+    tcg_temp_free_i32(r1);
+    tcg_temp_free_i32(r2);
+    tcg_temp_free_i32(sizes);
+    tcg_temp_free_i32(tst);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_ts(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 t1 = tcg_const_i32(0xff);
-- 
2.11.0

[Qemu-devel] [PATCH v3 29/30] target/s390x: mark ETF2 and ETF2-ENH facilities as available

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.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 e5e005a430..fc3cb25cc3 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -668,8 +668,10 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
     static const int feats[] = {
         S390_FEAT_STFLE,
         S390_FEAT_EXTENDED_IMMEDIATE,
+        S390_FEAT_EXTENDED_TRANSLATION_2,
         S390_FEAT_LONG_DISPLACEMENT,
         S390_FEAT_LONG_DISPLACEMENT_FAST,
+        S390_FEAT_ETF2_ENH,
         S390_FEAT_STORE_CLOCK_FAST,
         S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
         S390_FEAT_EXECUTE_EXT,
-- 
2.11.0

[Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[Aurelien Jarno]

At the same time fix the TCG version of get_max_cpu_model to return the
maximum model like on KVM. Remove the ETF2 and long-displacement
facilities from the additional features as it is included in the z800.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/cpu_models.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index fc3cb25cc3..c13bbd852c 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
     static const int feats[] = {
         S390_FEAT_STFLE,
         S390_FEAT_EXTENDED_IMMEDIATE,
-        S390_FEAT_EXTENDED_TRANSLATION_2,
-        S390_FEAT_LONG_DISPLACEMENT,
         S390_FEAT_LONG_DISPLACEMENT_FAST,
         S390_FEAT_ETF2_ENH,
         S390_FEAT_STORE_CLOCK_FAST,
@@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
     if (kvm_enabled()) {
         kvm_s390_get_host_cpu_model(&max_model, errp);
     } else {
-        /* TCG emulates a z900 (with some optional additional features) */
-        max_model.def = &s390_cpu_defs[0];
-        bitmap_copy(max_model.features, max_model.def->default_feat,
+        /* TCG emulates a z800 (with some optional additional features) */
+        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
+        bitmap_copy(max_model.features, max_model.def->full_feat,
                     S390_FEAT_MAX);
         add_qemu_cpu_model_features(max_model.features);
     }
@@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
     S390CPU *cpu = S390_CPU(obj);
 
     cpu->model = g_malloc0(sizeof(*cpu->model));
-    /* TCG emulates a z900 (with some optional additional features) */
-    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
+    /* TCG emulates a z800 (with some optional additional features) */
+    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
+           sizeof(s390_qemu_cpu_defs));
     add_qemu_cpu_model_features(s390_qemu_cpu_defs.full_feat);
     cpu->model->def = &s390_qemu_cpu_defs;
     bitmap_copy(cpu->model->features, cpu->model->def->default_feat,
-- 
2.11.0

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[David Hildenbrand]

On 01.06.2017 00:01, Aurelien Jarno wrote:
> At the same time fix the TCG version of get_max_cpu_model to return the
> maximum model like on KVM. Remove the ETF2 and long-displacement

I don't understand the part
"fix the TCG version of get_max_cpu_model to return the maximum model
like on KVM".

Can you elaborate?

> facilities from the additional features as it is included in the z800.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target/s390x/cpu_models.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index fc3cb25cc3..c13bbd852c 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>      static const int feats[] = {
>          S390_FEAT_STFLE,
>          S390_FEAT_EXTENDED_IMMEDIATE,
> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> -        S390_FEAT_LONG_DISPLACEMENT,
>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>          S390_FEAT_ETF2_ENH,
>          S390_FEAT_STORE_CLOCK_FAST,
> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>      if (kvm_enabled()) {
>          kvm_s390_get_host_cpu_model(&max_model, errp);
>      } else {
> -        /* TCG emulates a z900 (with some optional additional features) */
> -        max_model.def = &s390_cpu_defs[0];
> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> +        /* TCG emulates a z800 (with some optional additional features) */
> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>                      S390_FEAT_MAX);
>          add_qemu_cpu_model_features(max_model.features);
>      }
> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>      S390CPU *cpu = S390_CPU(obj);
>  
>      cpu->model = g_malloc0(sizeof(*cpu->model));
> -    /* TCG emulates a z900 (with some optional additional features) */
> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> +    /* TCG emulates a z800 (with some optional additional features) */
> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> +           sizeof(s390_qemu_cpu_defs));

No changing the qemu model without compatibility handling.

Thanks!

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[David Hildenbrand]

On 01.06.2017 10:38, David Hildenbrand wrote:
> On 01.06.2017 00:01, Aurelien Jarno wrote:
>> At the same time fix the TCG version of get_max_cpu_model to return the
>> maximum model like on KVM. Remove the ETF2 and long-displacement
> 
> I don't understand the part
> "fix the TCG version of get_max_cpu_model to return the maximum model
> like on KVM".
> 
> Can you elaborate?
> 
>> facilities from the additional features as it is included in the z800.
>>
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>>  target/s390x/cpu_models.c | 13 ++++++-------
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index fc3cb25cc3..c13bbd852c 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>      static const int feats[] = {
>>          S390_FEAT_STFLE,
>>          S390_FEAT_EXTENDED_IMMEDIATE,
>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>> -        S390_FEAT_LONG_DISPLACEMENT,
>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>          S390_FEAT_ETF2_ENH,
>>          S390_FEAT_STORE_CLOCK_FAST,
>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>      if (kvm_enabled()) {
>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>      } else {
>> -        /* TCG emulates a z900 (with some optional additional features) */
>> -        max_model.def = &s390_cpu_defs[0];
>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>> +        /* TCG emulates a z800 (with some optional additional features) */
>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>                      S390_FEAT_MAX);

This is most likely wrong: you're indicating features here that are not
available on tcg. esp. S390_FEAT_SIE_F2 and friends.

I think should only copy the base features and add whatever else is
available via add_qemu_cpu_model_features() as already done.

>>          add_qemu_cpu_model_features(max_model.features);
>>      }
>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>      S390CPU *cpu = S390_CPU(obj);
>>  
>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>> -    /* TCG emulates a z900 (with some optional additional features) */
>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>> +    /* TCG emulates a z800 (with some optional additional features) */
>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>> +           sizeof(s390_qemu_cpu_defs));
> 
> No changing the qemu model without compatibility handling.
> 
Please have a look at the following mail for a possible solution:

https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html

This could be moved to a separate patch. So this patch really should
just care about the maximum model, not the qemu model.

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[Aurelien Jarno]

On 2017-06-01 10:38, David Hildenbrand wrote:
> On 01.06.2017 00:01, Aurelien Jarno wrote:
> > At the same time fix the TCG version of get_max_cpu_model to return the
> > maximum model like on KVM. Remove the ETF2 and long-displacement
> 
> I don't understand the part
> "fix the TCG version of get_max_cpu_model to return the maximum model
> like on KVM".
> 
> Can you elaborate?

Currently get_max_cpu_model returns the features of the base model, so
for example the one of a z900 even on a z800. This makes impossible to
enable the features that are provided by a z800 like etf2 or ldisp.

For what I understand from the KVM code (but I haven't tested), the
function return all the features that are supported by the current CPU,
not all the features that are supported by the base model of the current
CPU.


> > facilities from the additional features as it is included in the z800.
> > 
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  target/s390x/cpu_models.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> > index fc3cb25cc3..c13bbd852c 100644
> > --- a/target/s390x/cpu_models.c
> > +++ b/target/s390x/cpu_models.c
> > @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >      static const int feats[] = {
> >          S390_FEAT_STFLE,
> >          S390_FEAT_EXTENDED_IMMEDIATE,
> > -        S390_FEAT_EXTENDED_TRANSLATION_2,
> > -        S390_FEAT_LONG_DISPLACEMENT,
> >          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >          S390_FEAT_ETF2_ENH,
> >          S390_FEAT_STORE_CLOCK_FAST,
> > @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >      if (kvm_enabled()) {
> >          kvm_s390_get_host_cpu_model(&max_model, errp);
> >      } else {
> > -        /* TCG emulates a z900 (with some optional additional features) */
> > -        max_model.def = &s390_cpu_defs[0];
> > -        bitmap_copy(max_model.features, max_model.def->default_feat,
> > +        /* TCG emulates a z800 (with some optional additional features) */
> > +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> > +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >                      S390_FEAT_MAX);
> >          add_qemu_cpu_model_features(max_model.features);
> >      }
> > @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >      S390CPU *cpu = S390_CPU(obj);
> >  
> >      cpu->model = g_malloc0(sizeof(*cpu->model));
> > -    /* TCG emulates a z900 (with some optional additional features) */
> > -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> > +    /* TCG emulates a z800 (with some optional additional features) */
> > +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> > +           sizeof(s390_qemu_cpu_defs));
> 
> No changing the qemu model without compatibility handling.

This patch series is based on the patch from Thomas Huth. It means the
QEMU model is still based on a z900, but that it is possible to enable
some more features like etf2.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[Aurelien Jarno]

On 2017-06-01 11:04, David Hildenbrand wrote:
> On 01.06.2017 10:38, David Hildenbrand wrote:
> > On 01.06.2017 00:01, Aurelien Jarno wrote:
> >> At the same time fix the TCG version of get_max_cpu_model to return the
> >> maximum model like on KVM. Remove the ETF2 and long-displacement
> > 
> > I don't understand the part
> > "fix the TCG version of get_max_cpu_model to return the maximum model
> > like on KVM".
> > 
> > Can you elaborate?
> > 
> >> facilities from the additional features as it is included in the z800.
> >>
> >> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >> ---
> >>  target/s390x/cpu_models.c | 13 ++++++-------
> >>  1 file changed, 6 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> >> index fc3cb25cc3..c13bbd852c 100644
> >> --- a/target/s390x/cpu_models.c
> >> +++ b/target/s390x/cpu_models.c
> >> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >>      static const int feats[] = {
> >>          S390_FEAT_STFLE,
> >>          S390_FEAT_EXTENDED_IMMEDIATE,
> >> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> >> -        S390_FEAT_LONG_DISPLACEMENT,
> >>          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >>          S390_FEAT_ETF2_ENH,
> >>          S390_FEAT_STORE_CLOCK_FAST,
> >> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >>      if (kvm_enabled()) {
> >>          kvm_s390_get_host_cpu_model(&max_model, errp);
> >>      } else {
> >> -        /* TCG emulates a z900 (with some optional additional features) */
> >> -        max_model.def = &s390_cpu_defs[0];
> >> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> >> +        /* TCG emulates a z800 (with some optional additional features) */
> >> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> >> +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >>                      S390_FEAT_MAX);
> 
> This is most likely wrong: you're indicating features here that are not
> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
> 
> I think should only copy the base features and add whatever else is
> available via add_qemu_cpu_model_features() as already done.

The patch series added all the z800 features exposed via STFL/STFLE.
Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
at all so the lack of these features are not exposed to the guest. In that
regard QEMU already wrongly claim to emulate a z900.


> >>          add_qemu_cpu_model_features(max_model.features);
> >>      }
> >> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >>      S390CPU *cpu = S390_CPU(obj);
> >>  
> >>      cpu->model = g_malloc0(sizeof(*cpu->model));
> >> -    /* TCG emulates a z900 (with some optional additional features) */
> >> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> >> +    /* TCG emulates a z800 (with some optional additional features) */
> >> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> >> +           sizeof(s390_qemu_cpu_defs));
> > 
> > No changing the qemu model without compatibility handling.
> > 
> Please have a look at the following mail for a possible solution:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
> 
> This could be moved to a separate patch. So this patch really should
> just care about the maximum model, not the qemu model.

From what I understand from this thread, the patch from Thomas Huth was
finally considered acceptable. I am adding him in Cc: so that he can
comment.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[David Hildenbrand]

On 01.06.2017 21:17, Aurelien Jarno wrote:
> On 2017-06-01 10:38, David Hildenbrand wrote:
>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>
>> I don't understand the part
>> "fix the TCG version of get_max_cpu_model to return the maximum model
>> like on KVM".
>>
>> Can you elaborate?
> 
> Currently get_max_cpu_model returns the features of the base model, so
> for example the one of a z900 even on a z800. This makes impossible to
> enable the features that are provided by a z800 like etf2 or ldisp.
> 

Right, you can always change the max_cpu_model, e.g. bumping up the
version or adding new features, that is just fine.

> For what I understand from the KVM code (but I haven't tested), the
> function return all the features that are supported by the current CPU,
> not all the features that are supported by the base model of the current
> CPU.

Correct, for KVM it is the detected model, that means: Base features +
optional features.


> 
> 
>>> facilities from the additional features as it is included in the z800.
>>>
>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>> ---
>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>> index fc3cb25cc3..c13bbd852c 100644
>>> --- a/target/s390x/cpu_models.c
>>> +++ b/target/s390x/cpu_models.c
>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>      static const int feats[] = {
>>>          S390_FEAT_STFLE,
>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>          S390_FEAT_ETF2_ENH,
>>>          S390_FEAT_STORE_CLOCK_FAST,
>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>      if (kvm_enabled()) {
>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>      } else {
>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>> -        max_model.def = &s390_cpu_defs[0];
>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>                      S390_FEAT_MAX);
>>>          add_qemu_cpu_model_features(max_model.features);
>>>      }
>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>>      S390CPU *cpu = S390_CPU(obj);
>>>  
>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>>> -    /* TCG emulates a z900 (with some optional additional features) */
>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>>> +    /* TCG emulates a z800 (with some optional additional features) */
>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>>> +           sizeof(s390_qemu_cpu_defs));
>>
>> No changing the qemu model without compatibility handling.
> 
> This patch series is based on the patch from Thomas Huth. It means the
> QEMU model is still based on a z900, but that it is possible to enable
> some more features like etf2.

Thomas' code did neither change features nor the "model definition". It
just allows for some more feature to be set. It is a hack.

I am pretty sure that expanding the "qemu" CPU model now (QMP
query-cpu-model-expansion) will indicate a z800, not a z900.

See cpu_info_from_model(). And that is a problem, because the QEMU CPU
model is a "migration-safe" CPU model, meaning it must remain equal for
every compatibility machine.

Thanks.

> 
> Aurelien
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[Thomas Huth]

On 01.06.2017 21:17, Aurelien Jarno wrote:
> On 2017-06-01 11:04, David Hildenbrand wrote:
>> On 01.06.2017 10:38, David Hildenbrand wrote:
>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>>
>>> I don't understand the part
>>> "fix the TCG version of get_max_cpu_model to return the maximum model
>>> like on KVM".
>>>
>>> Can you elaborate?
>>>
>>>> facilities from the additional features as it is included in the z800.
>>>>
>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>> ---
>>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>>> index fc3cb25cc3..c13bbd852c 100644
>>>> --- a/target/s390x/cpu_models.c
>>>> +++ b/target/s390x/cpu_models.c
>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>>      static const int feats[] = {
>>>>          S390_FEAT_STFLE,
>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>>          S390_FEAT_ETF2_ENH,
>>>>          S390_FEAT_STORE_CLOCK_FAST,
>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>>      if (kvm_enabled()) {
>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>>      } else {
>>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>>> -        max_model.def = &s390_cpu_defs[0];
>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>>                      S390_FEAT_MAX);
>>
>> This is most likely wrong: you're indicating features here that are not
>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
>>
>> I think should only copy the base features and add whatever else is
>> available via add_qemu_cpu_model_features() as already done.
> 
> The patch series added all the z800 features exposed via STFL/STFLE.
> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
> at all so the lack of these features are not exposed to the guest. In that
> regard QEMU already wrongly claim to emulate a z900.
> 
> 
>>>>          add_qemu_cpu_model_features(max_model.features);
>>>>      }
>>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>>>      S390CPU *cpu = S390_CPU(obj);
>>>>  
>>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>>>> -    /* TCG emulates a z900 (with some optional additional features) */
>>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>>>> +    /* TCG emulates a z800 (with some optional additional features) */
>>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>>>> +           sizeof(s390_qemu_cpu_defs));
>>>
>>> No changing the qemu model without compatibility handling.
>>>
>> Please have a look at the following mail for a possible solution:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
>>
>> This could be moved to a separate patch. So this patch really should
>> just care about the maximum model, not the qemu model.
> 
> From what I understand from this thread, the patch from Thomas Huth was
> finally considered acceptable. I am adding him in Cc: so that he can
> comment.

In the 2nd version of my patch, I only changed the full_feat of the
model definitions, but not the base_feat and default_feat fields, so
that the CPU stays the same when you run QEMU without "-cpu" parameter
(or simply with "-cpu qemu").

Consider that the following scenario should work: Start a QEMU v2.10
with "-M s390-ccw-virtio-2.9" and a QEMU v2.9 with "-M
s390-ccw-virtio-2.9 -incoming ...". Then it should be possible to
migrate from the v2.10 to the v2.9 instance without problems. This won't
work anymore if you changed the default feature bits unconditionally.

 Thomas

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[David Hildenbrand]

On 02.06.2017 10:09, Thomas Huth wrote:
> On 01.06.2017 21:17, Aurelien Jarno wrote:
>> On 2017-06-01 11:04, David Hildenbrand wrote:
>>> On 01.06.2017 10:38, David Hildenbrand wrote:
>>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>>>
>>>> I don't understand the part
>>>> "fix the TCG version of get_max_cpu_model to return the maximum model
>>>> like on KVM".
>>>>
>>>> Can you elaborate?
>>>>
>>>>> facilities from the additional features as it is included in the z800.
>>>>>
>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>> ---
>>>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>>>> index fc3cb25cc3..c13bbd852c 100644
>>>>> --- a/target/s390x/cpu_models.c
>>>>> +++ b/target/s390x/cpu_models.c
>>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>>>      static const int feats[] = {
>>>>>          S390_FEAT_STFLE,
>>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>>>          S390_FEAT_ETF2_ENH,
>>>>>          S390_FEAT_STORE_CLOCK_FAST,
>>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>>>      if (kvm_enabled()) {
>>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>>>      } else {
>>>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>>>> -        max_model.def = &s390_cpu_defs[0];
>>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>>>                      S390_FEAT_MAX);
>>>
>>> This is most likely wrong: you're indicating features here that are not
>>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
>>>
>>> I think should only copy the base features and add whatever else is
>>> available via add_qemu_cpu_model_features() as already done.
>>
>> The patch series added all the z800 features exposed via STFL/STFLE.
>> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
>> at all so the lack of these features are not exposed to the guest. In that
>> regard QEMU already wrongly claim to emulate a z900.

Please note that:

a) SIE features were never part of the max model. QEMU never claimed
that. With your change one could suddenly do a -cpu z900,sie_f2=on,
which is wrong.

b) The SIE_F2 feature tells the guest that the SIE instruction is
available. E.g. Linux will look at this bit and show SIE support in
/proc/cpuinfo and unlock the KVM module.

Please, just don't add features to the MAX model that we don't implement.

>>
>>
>>>>>          add_qemu_cpu_model_features(max_model.features);
>>>>>      }
>>>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
>>>>>      S390CPU *cpu = S390_CPU(obj);
>>>>>  
>>>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
>>>>> -    /* TCG emulates a z900 (with some optional additional features) */
>>>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
>>>>> +    /* TCG emulates a z800 (with some optional additional features) */
>>>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
>>>>> +           sizeof(s390_qemu_cpu_defs));
>>>>
>>>> No changing the qemu model without compatibility handling.
>>>>
>>> Please have a look at the following mail for a possible solution:
>>>
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
>>>
>>> This could be moved to a separate patch. So this patch really should
>>> just care about the maximum model, not the qemu model.
>>
>> From what I understand from this thread, the patch from Thomas Huth was
>> finally considered acceptable. I am adding him in Cc: so that he can
>> comment.
> 
> In the 2nd version of my patch, I only changed the full_feat of the
> model definitions, but not the base_feat and default_feat fields, so
> that the CPU stays the same when you run QEMU without "-cpu" parameter
> (or simply with "-cpu qemu").
> 
> Consider that the following scenario should work: Start a QEMU v2.10
> with "-M s390-ccw-virtio-2.9" and a QEMU v2.9 with "-M
> s390-ccw-virtio-2.9 -incoming ...". Then it should be possible to
> migrate from the v2.10 to the v2.9 instance without problems. This won't
> work anymore if you changed the default feature bits unconditionally.
> 
>  Thomas
> 

The following can be used to make sure the model does not change:

s390x-softmmu/qemu-system-s390x -S -qmp stdio

-> { "execute": "qmp_capabilities" }
-> { "execute": "query-cpu-model-expansion", "arguments": { "type":
"static", "model": { "name": "qemu" } } }

With existing compat machines, this has to give
-> "{"return": {"model": {"name": "z900-base"}}}"


Also, is there any specific reason why to go for a z800 and not a z900.3
? (IBM zSeries 900 GA3).

z900 GA3 is the EC model, z800 GA1 is the BC model. Both have same
features and capabilities. EC always smells more powerful ;) In the end
it doesn't matter.

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[Aurelien Jarno]

On 2017-06-01 21:56, David Hildenbrand wrote:
> On 01.06.2017 21:17, Aurelien Jarno wrote:
> > On 2017-06-01 10:38, David Hildenbrand wrote:
> >> On 01.06.2017 00:01, Aurelien Jarno wrote:
> >>> At the same time fix the TCG version of get_max_cpu_model to return the
> >>> maximum model like on KVM. Remove the ETF2 and long-displacement
> >>
> >> I don't understand the part
> >> "fix the TCG version of get_max_cpu_model to return the maximum model
> >> like on KVM".
> >>
> >> Can you elaborate?
> > 
> > Currently get_max_cpu_model returns the features of the base model, so
> > for example the one of a z900 even on a z800. This makes impossible to
> > enable the features that are provided by a z800 like etf2 or ldisp.
> > 
> 
> Right, you can always change the max_cpu_model, e.g. bumping up the
> version or adding new features, that is just fine.
> 
> > For what I understand from the KVM code (but I haven't tested), the
> > function return all the features that are supported by the current CPU,
> > not all the features that are supported by the base model of the current
> > CPU.
> 
> Correct, for KVM it is the detected model, that means: Base features +
> optional features.
> 
> 
> > 
> > 
> >>> facilities from the additional features as it is included in the z800.
> >>>
> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>> ---
> >>>  target/s390x/cpu_models.c | 13 ++++++-------
> >>>  1 file changed, 6 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> >>> index fc3cb25cc3..c13bbd852c 100644
> >>> --- a/target/s390x/cpu_models.c
> >>> +++ b/target/s390x/cpu_models.c
> >>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >>>      static const int feats[] = {
> >>>          S390_FEAT_STFLE,
> >>>          S390_FEAT_EXTENDED_IMMEDIATE,
> >>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> >>> -        S390_FEAT_LONG_DISPLACEMENT,
> >>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >>>          S390_FEAT_ETF2_ENH,
> >>>          S390_FEAT_STORE_CLOCK_FAST,
> >>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >>>      if (kvm_enabled()) {
> >>>          kvm_s390_get_host_cpu_model(&max_model, errp);
> >>>      } else {
> >>> -        /* TCG emulates a z900 (with some optional additional features) */
> >>> -        max_model.def = &s390_cpu_defs[0];
> >>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> >>> +        /* TCG emulates a z800 (with some optional additional features) */
> >>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> >>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >>>                      S390_FEAT_MAX);
> >>>          add_qemu_cpu_model_features(max_model.features);
> >>>      }
> >>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >>>      S390CPU *cpu = S390_CPU(obj);
> >>>  
> >>>      cpu->model = g_malloc0(sizeof(*cpu->model));
> >>> -    /* TCG emulates a z900 (with some optional additional features) */
> >>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> >>> +    /* TCG emulates a z800 (with some optional additional features) */
> >>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> >>> +           sizeof(s390_qemu_cpu_defs));
> >>
> >> No changing the qemu model without compatibility handling.
> > 
> > This patch series is based on the patch from Thomas Huth. It means the
> > QEMU model is still based on a z900, but that it is possible to enable
> > some more features like etf2.
> 
> Thomas' code did neither change features nor the "model definition". It
> just allows for some more feature to be set. It is a hack.
> 
> I am pretty sure that expanding the "qemu" CPU model now (QMP
> query-cpu-model-expansion) will indicate a z800, not a z900.
> 
> See cpu_info_from_model(). And that is a problem, because the QEMU CPU
> model is a "migration-safe" CPU model, meaning it must remain equal for
> every compatibility machine.
> 

Ok, I get what you mean now.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[Aurelien Jarno]

On 2017-06-02 13:30, David Hildenbrand wrote:
> On 02.06.2017 10:09, Thomas Huth wrote:
> > On 01.06.2017 21:17, Aurelien Jarno wrote:
> >> On 2017-06-01 11:04, David Hildenbrand wrote:
> >>> On 01.06.2017 10:38, David Hildenbrand wrote:
> >>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
> >>>>> At the same time fix the TCG version of get_max_cpu_model to return the
> >>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
> >>>>
> >>>> I don't understand the part
> >>>> "fix the TCG version of get_max_cpu_model to return the maximum model
> >>>> like on KVM".
> >>>>
> >>>> Can you elaborate?
> >>>>
> >>>>> facilities from the additional features as it is included in the z800.
> >>>>>
> >>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >>>>> ---
> >>>>>  target/s390x/cpu_models.c | 13 ++++++-------
> >>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> >>>>> index fc3cb25cc3..c13bbd852c 100644
> >>>>> --- a/target/s390x/cpu_models.c
> >>>>> +++ b/target/s390x/cpu_models.c
> >>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
> >>>>>      static const int feats[] = {
> >>>>>          S390_FEAT_STFLE,
> >>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
> >>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
> >>>>> -        S390_FEAT_LONG_DISPLACEMENT,
> >>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
> >>>>>          S390_FEAT_ETF2_ENH,
> >>>>>          S390_FEAT_STORE_CLOCK_FAST,
> >>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
> >>>>>      if (kvm_enabled()) {
> >>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
> >>>>>      } else {
> >>>>> -        /* TCG emulates a z900 (with some optional additional features) */
> >>>>> -        max_model.def = &s390_cpu_defs[0];
> >>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
> >>>>> +        /* TCG emulates a z800 (with some optional additional features) */
> >>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
> >>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
> >>>>>                      S390_FEAT_MAX);
> >>>
> >>> This is most likely wrong: you're indicating features here that are not
> >>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
> >>>
> >>> I think should only copy the base features and add whatever else is
> >>> available via add_qemu_cpu_model_features() as already done.
> >>
> >> The patch series added all the z800 features exposed via STFL/STFLE.
> >> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
> >> at all so the lack of these features are not exposed to the guest. In that
> >> regard QEMU already wrongly claim to emulate a z900.
> 
> Please note that:
> 
> a) SIE features were never part of the max model. QEMU never claimed
> that. With your change one could suddenly do a -cpu z900,sie_f2=on,
> which is wrong.
> 
> b) The SIE_F2 feature tells the guest that the SIE instruction is
> available. E.g. Linux will look at this bit and show SIE support in
> /proc/cpuinfo and unlock the KVM module.

I understand your point. That said I doubt we will support the SIE
instruction soon (it looks quite complicated and I can't find any doc).
As we are going to emulate more facilities to QEMU, it will be more and
more difficult to select a modern CPU. One will have to use -cpu,z900,
etf2=on,ldisp=on,...,eimm=on. I think we have to provide users with an
easier way to do that.

One possibility would be to filter the features that are not emulated by
QEMU (like on the ppc64 target) or to allow the user to specify a higher
model provided the non emulated features are disabled. Something like
-cpu z800,sie_f2=off.

> Please, just don't add features to the MAX model that we don't implement.

Please note that QEMU does not fully implement S390_FEAT_ESAN3 (though
that is addressed in this patchset) and S390_FEAT_ZARCH, despite
claiming it.

> >>>>>          add_qemu_cpu_model_features(max_model.features);
> >>>>>      }
> >>>>> @@ -956,8 +954,9 @@ static void s390_qemu_cpu_model_initfn(Object *obj)
> >>>>>      S390CPU *cpu = S390_CPU(obj);
> >>>>>  
> >>>>>      cpu->model = g_malloc0(sizeof(*cpu->model));
> >>>>> -    /* TCG emulates a z900 (with some optional additional features) */
> >>>>> -    memcpy(&s390_qemu_cpu_defs, &s390_cpu_defs[0], sizeof(s390_qemu_cpu_defs));
> >>>>> +    /* TCG emulates a z800 (with some optional additional features) */
> >>>>> +    memcpy(&s390_qemu_cpu_defs, s390_find_cpu_def(0x2066, 7, 3, NULL),
> >>>>> +           sizeof(s390_qemu_cpu_defs));
> >>>>
> >>>> No changing the qemu model without compatibility handling.
> >>>>
> >>> Please have a look at the following mail for a possible solution:
> >>>
> >>> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06030.html
> >>>
> >>> This could be moved to a separate patch. So this patch really should
> >>> just care about the maximum model, not the qemu model.
> >>
> >> From what I understand from this thread, the patch from Thomas Huth was
> >> finally considered acceptable. I am adding him in Cc: so that he can
> >> comment.
> > 
> > In the 2nd version of my patch, I only changed the full_feat of the
> > model definitions, but not the base_feat and default_feat fields, so
> > that the CPU stays the same when you run QEMU without "-cpu" parameter
> > (or simply with "-cpu qemu").
> > 
> > Consider that the following scenario should work: Start a QEMU v2.10
> > with "-M s390-ccw-virtio-2.9" and a QEMU v2.9 with "-M
> > s390-ccw-virtio-2.9 -incoming ...". Then it should be possible to
> > migrate from the v2.10 to the v2.9 instance without problems. This won't
> > work anymore if you changed the default feature bits unconditionally.
> > 
> >  Thomas
> > 
> 
> The following can be used to make sure the model does not change:
> 
> s390x-softmmu/qemu-system-s390x -S -qmp stdio
> 
> -> { "execute": "qmp_capabilities" }
> -> { "execute": "query-cpu-model-expansion", "arguments": { "type":
> "static", "model": { "name": "qemu" } } }
> 
> With existing compat machines, this has to give
> -> "{"return": {"model": {"name": "z900-base"}}}"

Thanks for the hint.
 
> Also, is there any specific reason why to go for a z800 and not a z900.3
> ? (IBM zSeries 900 GA3).
> 
> z900 GA3 is the EC model, z800 GA1 is the BC model. Both have same
> features and capabilities. EC always smells more powerful ;) In the end
> it doesn't matter.

No real reason. It is further in the list, so it looks better to me.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[David Hildenbrand]

On 02.06.2017 16:04, Aurelien Jarno wrote:
> On 2017-06-02 13:30, David Hildenbrand wrote:
>> On 02.06.2017 10:09, Thomas Huth wrote:
>>> On 01.06.2017 21:17, Aurelien Jarno wrote:
>>>> On 2017-06-01 11:04, David Hildenbrand wrote:
>>>>> On 01.06.2017 10:38, David Hildenbrand wrote:
>>>>>> On 01.06.2017 00:01, Aurelien Jarno wrote:
>>>>>>> At the same time fix the TCG version of get_max_cpu_model to return the
>>>>>>> maximum model like on KVM. Remove the ETF2 and long-displacement
>>>>>>
>>>>>> I don't understand the part
>>>>>> "fix the TCG version of get_max_cpu_model to return the maximum model
>>>>>> like on KVM".
>>>>>>
>>>>>> Can you elaborate?
>>>>>>
>>>>>>> facilities from the additional features as it is included in the z800.
>>>>>>>
>>>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>>>> ---
>>>>>>>  target/s390x/cpu_models.c | 13 ++++++-------
>>>>>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>>>>>> index fc3cb25cc3..c13bbd852c 100644
>>>>>>> --- a/target/s390x/cpu_models.c
>>>>>>> +++ b/target/s390x/cpu_models.c
>>>>>>> @@ -668,8 +668,6 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
>>>>>>>      static const int feats[] = {
>>>>>>>          S390_FEAT_STFLE,
>>>>>>>          S390_FEAT_EXTENDED_IMMEDIATE,
>>>>>>> -        S390_FEAT_EXTENDED_TRANSLATION_2,
>>>>>>> -        S390_FEAT_LONG_DISPLACEMENT,
>>>>>>>          S390_FEAT_LONG_DISPLACEMENT_FAST,
>>>>>>>          S390_FEAT_ETF2_ENH,
>>>>>>>          S390_FEAT_STORE_CLOCK_FAST,
>>>>>>> @@ -696,9 +694,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>>>>>      if (kvm_enabled()) {
>>>>>>>          kvm_s390_get_host_cpu_model(&max_model, errp);
>>>>>>>      } else {
>>>>>>> -        /* TCG emulates a z900 (with some optional additional features) */
>>>>>>> -        max_model.def = &s390_cpu_defs[0];
>>>>>>> -        bitmap_copy(max_model.features, max_model.def->default_feat,
>>>>>>> +        /* TCG emulates a z800 (with some optional additional features) */
>>>>>>> +        max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);
>>>>>>> +        bitmap_copy(max_model.features, max_model.def->full_feat,
>>>>>>>                      S390_FEAT_MAX);
>>>>>
>>>>> This is most likely wrong: you're indicating features here that are not
>>>>> available on tcg. esp. S390_FEAT_SIE_F2 and friends.
>>>>>
>>>>> I think should only copy the base features and add whatever else is
>>>>> available via add_qemu_cpu_model_features() as already done.
>>>>
>>>> The patch series added all the z800 features exposed via STFL/STFLE.
>>>> Indeed the SIE features are missing, but anyway QEMU doesn't emulate SIE
>>>> at all so the lack of these features are not exposed to the guest. In that
>>>> regard QEMU already wrongly claim to emulate a z900.
>>
>> Please note that:
>>
>> a) SIE features were never part of the max model. QEMU never claimed
>> that. With your change one could suddenly do a -cpu z900,sie_f2=on,
>> which is wrong.
>>
>> b) The SIE_F2 feature tells the guest that the SIE instruction is
>> available. E.g. Linux will look at this bit and show SIE support in
>> /proc/cpuinfo and unlock the KVM module.
> 
> I understand your point. That said I doubt we will support the SIE
> instruction soon (it looks quite complicated and I can't find any doc).
> As we are going to emulate more facilities to QEMU, it will be more and
> more difficult to select a modern CPU. One will have to use -cpu,z900,
> etf2=on,ldisp=on,...,eimm=on. I think we have to provide users with an
> easier way to do that.
> 

Okay, I think you got something wrong here (which happens easily with
all these different model types).

CPU definitions:
- Base definition: Minimum features we expect to be around on such a CPU
- Default definition: Features we expect to be around in sane
  environments (== KVM on LPAR)
- Full definition: Maximum features that could be around on such a CPU
  (and there are ususally quite some missing, e.g. when running KVM
   under z/VM)

Only default and full definitions can ever change.

CPU models:
- Base model: maps to base definition == will never change
   -> e.g. "z900-base"
- Default model: maps to default definition == can change (but migration
  safe via compatibility machines!)
  -> e.g. "z900" or "qemu"
- Host model: only for KVM, maps to "max model"
- Max model: (not exposed yet directly for TCG) "maximum supported model
  + features"

Full models do not exist!
So in summary, what you want to do here is:

1. Get the definition of the z800 model. Copy the base features to the
   max model.
2. Add all features that are supported additionally and not in the base.
3. (Maybe add a hack like Thomas implemented to support some further
    features that are exceeding the full model)

So, especially, the CPU model "z800" does not map to the full model, but
the default model. It will never contain SIE features (at least for now)

-cpu z800 can therefore be used just fine, as it won't select SIE
features (especially because SIE features are also only available for
KVM in rare situations as nested virtualization has to be enabled
explcitily).

You can use

{ "execute": "query-cpu-model-expansion", "arguments": { "type": "full",
"model": { "name": "z800" } } }

To see all features that are currently part of the "default" model.

{"return": {"model": {"name": "z800-base", "props": {"pfmfi": false,
"exrl": false, "stfle45": false, "cmma": false, "dateh2": false,
"gen13ptff": false, "dateh": false, "iacc2": false, "parseh": false,
"csst": false, "idter": false, "idtes": false, "msa": false, "aefsi":
false, "csst2": false, "csske": false, "msa5": false, "msa4": false,
"msa3": false, "msa2": false, "msa1": false, "sthyi": false, "stckf":
false, "stfle": false, "etf3": false, "etf2": false, "edat": false,
"hfpm": false, "ri": false, "edat2": false, "hfpue": false, "dfp":
false, "mvcos": false, "sprogp": false, "sigpif": false, "ldisphp":
false, "vx": false, "ipter": false, "emon": false, "cei": false,
"cmpsceh": false, "ginste": false, "dfppc": false, "dfpzc": false,
"dfphp": false, "stfle49": false, "asnlxr": false, "gpereh": false,
"esop": false, "ectg": false, "ib": false, "siif": false, "esan3": true,
"fpe": false, "ibs": false, "zarch": true, "stfle53": false, "sief2":
false, "eimm": false, "srs": false, "cte": false, "fpseh": false,
"ltlbc": false, "ldisp": false, "64bscao": false, "ctop": false,
"etf3eh": false, "etf2eh": false, "nonqks": false, "pfpo": false, "te":
false, "cmm": false, "tods": false, "plo": true, "gsls": false, "skey":
false}}}}

Which is the same as
{ "execute": "query-cpu-model-expansion", "arguments": { "type":
"static", "model": { "name": "z800" } } }
{"return": {"model": {"name": "z800-base"}}}

So no SIE features.


> One possibility would be to filter the features that are not emulated by
> QEMU (like on the ppc64 target) or to allow the user to specify a higher
> model provided the non emulated features are disabled. Something like
> -cpu z800,sie_f2=off.
> 
>> Please, just don't add features to the MAX model that we don't implement.
> 
> Please note that QEMU does not fully implement S390_FEAT_ESAN3 (though
> that is addressed in this patchset) and S390_FEAT_ZARCH, despite
> claiming it.

Yes, there is a lot missing on the TCG side as I noticed already :) But
I think we're going into the right direction. (as long as Linux runs ...)


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[Thomas Huth]

On 02.06.2017 16:04, Aurelien Jarno wrote:
[...]
> I understand your point. That said I doubt we will support the SIE
> instruction soon (it looks quite complicated and I can't find any doc).

There is unfortunately no public documentation for the SIE instruction.
The only spec that is available is a very old one from the S/370 age:

http://bitsavers.trailing-edge.com/pdf/ibm/370/princOps/SA22-7095-0_370-XA_Interpretive_Execution_Jan84.pdf

 Thomas

Re: [Qemu-devel] [PATCH v3 30/30] target/s390x: update maximum TCG model to z800

[David Hildenbrand]

On 02.06.2017 16:34, Thomas Huth wrote:
> On 02.06.2017 16:04, Aurelien Jarno wrote:
> [...]
>> I understand your point. That said I doubt we will support the SIE
>> instruction soon (it looks quite complicated and I can't find any doc).
> 
> There is unfortunately no public documentation for the SIE instruction.
> The only spec that is available is a very old one from the S/370 age:
> 
> http://bitsavers.trailing-edge.com/pdf/ibm/370/princOps/SA22-7095-0_370-XA_Interpretive_Execution_Jan84.pdf
> 

And of course the kvm (esp. vsie) implementation is very helpful
understanding how it works and what some bits mean :)

>  Thomas
> 


-- 

Thanks,

David