From be39110d4cbf82bd4f9154d9958cd0a1aea57633 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Wed, 16 Oct 2019 16:54:34 +0200 Subject: [PATCH 1/9] s390x/cpumodel: Add missing visit_free Beata Michalska noticed this missing visit_free() while reviewing arm's implementation of qmp_query_cpu_model_expansion(), which is modeled off this s390x implementation. Signed-off-by: Andrew Jones Message-Id: <20191016145434.7007-1-drjones@redhat.com> Reviewed-by: David Hildenbrand Signed-off-by: Cornelia Huck --- target/s390x/cpu_models.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 009afc38b9..7e92fb2e15 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -515,6 +515,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info, visitor = qobject_input_visitor_new(info->props); visit_start_struct(visitor, NULL, NULL, 0, errp); if (*errp) { + visit_free(visitor); object_unref(obj); return; } From 8064af6b1dac60491df611e5c49199d2e1c368c4 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Thu, 17 Oct 2019 14:19:22 +0200 Subject: [PATCH 2/9] s390x/mmu: Remove duplicate check for MMU_DATA_STORE No need to double-check if we have a write. Found by Coverity (CID: 1406404). Fixes: 31b59419069e ("target/s390x: Return exception from mmu_translate_real") Cc: Peter Maydell Signed-off-by: David Hildenbrand Message-Id: <20191017121922.18840-1-david@redhat.com> Reviewed-by: Richard Henderson Signed-off-by: Cornelia Huck --- target/s390x/mmu_helper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index 90b81335f9..c9f3f34750 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -556,9 +556,7 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, *flags |= PAGE_WRITE_INV; if (is_low_address(raddr) && rw == MMU_DATA_STORE) { /* LAP sets bit 56 */ - *tec = (raddr & TARGET_PAGE_MASK) - | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) - | 0x80; + *tec = (raddr & TARGET_PAGE_MASK) | FS_WRITE | 0x80; return PGM_PROTECTION; } } From 49a7ce4e030a24c092c82076e23473ae9226fca9 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 21 Oct 2019 10:57:10 +0200 Subject: [PATCH 3/9] s390x/tcg: Fix VECTOR MULTIPLY LOGICAL ODD We have to read from odd offsets. Fixes: 2bf3ee38f1f8 ("s390x/tcg: Implement VECTOR MULTIPLY *") Reviewed-by: Richard Henderson Signed-off-by: David Hildenbrand Message-Id: <20191021085715.3797-2-david@redhat.com> Signed-off-by: Cornelia Huck --- target/s390x/vec_int_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/vec_int_helper.c b/target/s390x/vec_int_helper.c index 68eaae407b..03ae8631d9 100644 --- a/target/s390x/vec_int_helper.c +++ b/target/s390x/vec_int_helper.c @@ -488,7 +488,7 @@ void HELPER(gvec_vmlo##BITS)(void *v1, const void *v2, const void *v3, \ { \ int i, j; \ \ - for (i = 0, j = 0; i < (128 / TBITS); i++, j += 2) { \ + for (i = 0, j = 1; i < (128 / TBITS); i++, j += 2) { \ const uint##TBITS##_t a = s390_vec_read_element##BITS(v2, j); \ const uint##TBITS##_t b = s390_vec_read_element##BITS(v3, j); \ \ From 8b952519478780ac26779018d3d7e9bf930d055a Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 21 Oct 2019 10:57:11 +0200 Subject: [PATCH 4/9] s390x/tcg: Fix VECTOR MULTIPLY AND ADD * We missed that we always read a "double-wide even-odd element pair of the fourth operand". Fix it in all four variants. Fixes: 1b430aec4157 ("s390x/tcg: Implement VECTOR MULTIPLY AND ADD *") Reviewed-by: Richard Henderson Signed-off-by: David Hildenbrand Message-Id: <20191021085715.3797-3-david@redhat.com> Signed-off-by: Cornelia Huck --- target/s390x/vec_int_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/s390x/vec_int_helper.c b/target/s390x/vec_int_helper.c index 03ae8631d9..1b3aaecbdb 100644 --- a/target/s390x/vec_int_helper.c +++ b/target/s390x/vec_int_helper.c @@ -336,7 +336,7 @@ void HELPER(gvec_vmae##BITS)(void *v1, const void *v2, const void *v3, \ for (i = 0, j = 0; i < (128 / TBITS); i++, j += 2) { \ int##TBITS##_t a = (int##BITS##_t)s390_vec_read_element##BITS(v2, j); \ int##TBITS##_t b = (int##BITS##_t)s390_vec_read_element##BITS(v3, j); \ - int##TBITS##_t c = (int##BITS##_t)s390_vec_read_element##BITS(v4, j); \ + int##TBITS##_t c = s390_vec_read_element##TBITS(v4, i); \ \ s390_vec_write_element##TBITS(v1, i, a * b + c); \ } \ @@ -354,7 +354,7 @@ void HELPER(gvec_vmale##BITS)(void *v1, const void *v2, const void *v3, \ for (i = 0, j = 0; i < (128 / TBITS); i++, j += 2) { \ uint##TBITS##_t a = s390_vec_read_element##BITS(v2, j); \ uint##TBITS##_t b = s390_vec_read_element##BITS(v3, j); \ - uint##TBITS##_t c = s390_vec_read_element##BITS(v4, j); \ + uint##TBITS##_t c = s390_vec_read_element##TBITS(v4, i); \ \ s390_vec_write_element##TBITS(v1, i, a * b + c); \ } \ @@ -372,7 +372,7 @@ void HELPER(gvec_vmao##BITS)(void *v1, const void *v2, const void *v3, \ for (i = 0, j = 1; i < (128 / TBITS); i++, j += 2) { \ int##TBITS##_t a = (int##BITS##_t)s390_vec_read_element##BITS(v2, j); \ int##TBITS##_t b = (int##BITS##_t)s390_vec_read_element##BITS(v3, j); \ - int##TBITS##_t c = (int##BITS##_t)s390_vec_read_element##BITS(v4, j); \ + int##TBITS##_t c = s390_vec_read_element##TBITS(v4, i); \ \ s390_vec_write_element##TBITS(v1, i, a * b + c); \ } \ @@ -390,7 +390,7 @@ void HELPER(gvec_vmalo##BITS)(void *v1, const void *v2, const void *v3, \ for (i = 0, j = 1; i < (128 / TBITS); i++, j += 2) { \ uint##TBITS##_t a = s390_vec_read_element##BITS(v2, j); \ uint##TBITS##_t b = s390_vec_read_element##BITS(v3, j); \ - uint##TBITS##_t c = s390_vec_read_element##BITS(v4, j); \ + uint##TBITS##_t c = s390_vec_read_element##TBITS(v4, i); \ \ s390_vec_write_element##TBITS(v1, i, a * b + c); \ } \ From b57b336876d08e303d5957d05bae77508ed0e4a2 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 21 Oct 2019 10:57:12 +0200 Subject: [PATCH 5/9] s390x/tcg: Fix VECTOR SHIFT RIGHT ARITHMETIC BY BYTE We forgot to propagate the highest bit accross the high doubleword in two cases (shift >=64). Fixes: 5f724887e3dd ("s390x/tcg: Implement VECTOR SHIFT RIGHT ARITHMETIC") Reviewed-by: Richard Henderson Signed-off-by: David Hildenbrand Message-Id: <20191021085715.3797-4-david@redhat.com> Signed-off-by: Cornelia Huck --- target/s390x/vec_int_helper.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/target/s390x/vec_int_helper.c b/target/s390x/vec_int_helper.c index 1b3aaecbdb..d38405848f 100644 --- a/target/s390x/vec_int_helper.c +++ b/target/s390x/vec_int_helper.c @@ -70,15 +70,17 @@ static void s390_vec_sar(S390Vector *d, const S390Vector *a, uint64_t count) d->doubleword[0] = a->doubleword[0]; d->doubleword[1] = a->doubleword[1]; } else if (count == 64) { + tmp = (int64_t)a->doubleword[0] >> 63; d->doubleword[1] = a->doubleword[0]; - d->doubleword[0] = 0; + d->doubleword[0] = tmp; } else if (count < 64) { tmp = a->doubleword[1] >> count; d->doubleword[1] = deposit64(tmp, 64 - count, count, a->doubleword[0]); d->doubleword[0] = (int64_t)a->doubleword[0] >> count; } else { + tmp = (int64_t)a->doubleword[0] >> 63; d->doubleword[1] = (int64_t)a->doubleword[0] >> (count - 64); - d->doubleword[0] = 0; + d->doubleword[0] = tmp; } } From 23e797749fff754b8a136ee37607c6448b06cfca Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 21 Oct 2019 10:57:13 +0200 Subject: [PATCH 6/9] s390x/tcg: Fix VECTOR SUBTRACT COMPUTE BORROW INDICATION Looks like my idea of what a "borrow" is was wrong. The PoP says: "If the resulting subtraction results in a carry out of bit zero, a value of one is placed in the corresponding element of the first operand; otherwise, a value of zero is placed in the corresponding element" As clarified by Richard, all we have to do is invert the result. Fixes: 1ee2d7ba72f6 ("s390x/tcg: Implement VECTOR SUBTRACT COMPUTE BORROW INDICATION") Signed-off-by: David Hildenbrand Message-Id: <20191021085715.3797-5-david@redhat.com> Reviewed-by: Richard Henderson Signed-off-by: Cornelia Huck --- target/s390x/translate_vx.inc.c | 7 ++++--- target/s390x/vec_int_helper.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c index 5ce7bfb0af..6032021d82 100644 --- a/target/s390x/translate_vx.inc.c +++ b/target/s390x/translate_vx.inc.c @@ -2132,12 +2132,12 @@ static DisasJumpType op_vs(DisasContext *s, DisasOps *o) static void gen_scbi_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b) { - tcg_gen_setcond_i32(TCG_COND_LTU, d, a, b); + tcg_gen_setcond_i32(TCG_COND_GEU, d, a, b); } static void gen_scbi_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b) { - tcg_gen_setcond_i64(TCG_COND_LTU, d, a, b); + tcg_gen_setcond_i64(TCG_COND_GEU, d, a, b); } static void gen_scbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, @@ -2151,7 +2151,8 @@ static void gen_scbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, tcg_gen_andi_i64(th, th, 1); tcg_gen_sub2_i64(tl, th, ah, zero, th, zero); tcg_gen_sub2_i64(tl, th, tl, th, bh, zero); - tcg_gen_andi_i64(dl, th, 1); + /* "invert" the result: -1 -> 0; 0 -> 1 */ + tcg_gen_addi_i64(dl, th, 1); tcg_gen_mov_i64(dh, zero); tcg_temp_free_i64(th); diff --git a/target/s390x/vec_int_helper.c b/target/s390x/vec_int_helper.c index d38405848f..0d6bc13dd6 100644 --- a/target/s390x/vec_int_helper.c +++ b/target/s390x/vec_int_helper.c @@ -593,7 +593,7 @@ void HELPER(gvec_vscbi##BITS)(void *v1, const void *v2, const void *v3, \ const uint##BITS##_t a = s390_vec_read_element##BITS(v2, i); \ const uint##BITS##_t b = s390_vec_read_element##BITS(v3, i); \ \ - s390_vec_write_element##BITS(v1, i, a < b); \ + s390_vec_write_element##BITS(v1, i, a >= b); \ } \ } DEF_VSCBI(8) From 2cb8a68d375450ab7be372d90e2ddf5a2a832cdc Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 21 Oct 2019 10:57:14 +0200 Subject: [PATCH 7/9] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW INDICATION Testing this, there seems to be something messed up. We are dealing with unsigned numbers. "Each operand is treated as an unsigned binary integer." Let's just implement as written in the PoP: "A subtraction is performed by adding the contents of the second operand with the bitwise complement of the third operand along with a borrow indication from the rightmost bit position of the fourth operand and the result is placed in the first operand." We can reuse gen_ac2_i64(). Fixes: 48390a7c2716 ("s390x/tcg: Implement VECTOR SUBTRACT WITH BORROW INDICATION") Signed-off-by: David Hildenbrand Message-Id: <20191021085715.3797-6-david@redhat.com> Reviewed-by: Richard Henderson Signed-off-by: Cornelia Huck --- target/s390x/translate_vx.inc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c index 6032021d82..fd746ba35f 100644 --- a/target/s390x/translate_vx.inc.c +++ b/target/s390x/translate_vx.inc.c @@ -2187,13 +2187,13 @@ static void gen_sbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, TCGv_i64 ah, TCGv_i64 bl, TCGv_i64 bh, TCGv_i64 cl, TCGv_i64 ch) { TCGv_i64 tl = tcg_temp_new_i64(); - TCGv_i64 zero = tcg_const_i64(0); + TCGv_i64 th = tcg_temp_new_i64(); - tcg_gen_andi_i64(tl, cl, 1); - tcg_gen_sub2_i64(dl, dh, al, ah, bl, bh); - tcg_gen_sub2_i64(dl, dh, dl, dh, tl, zero); + tcg_gen_not_i64(tl, bl); + tcg_gen_not_i64(th, bh); + gen_ac2_i64(dl, dh, al, ah, tl, th, cl, ch); tcg_temp_free_i64(tl); - tcg_temp_free_i64(zero); + tcg_temp_free_i64(th); } static DisasJumpType op_vsbi(DisasContext *s, DisasOps *o) From 38ad4fa3de4a0e747940711f16028fc509a4a6b6 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 21 Oct 2019 10:57:15 +0200 Subject: [PATCH 8/9] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW COMPUTE BORROW INDICATION The numbers are unsigned, the computation is wrong. "Each operand is treated as an unsigned binary integer". Let's implement as given in the PoP: "A subtraction is performed by adding the contents of the second operand with the bitwise complement of the third operand along with a borrow indication from the rightmost bit of the fourth operand." Reuse gen_accc2_i64(). Fixes: bc725e65152c ("s390x/tcg: Implement VECTOR SUBTRACT WITH BORROW COMPUTE BORROW INDICATION") Signed-off-by: David Hildenbrand Message-Id: <20191021085715.3797-7-david@redhat.com> Reviewed-by: Richard Henderson Signed-off-by: Cornelia Huck --- target/s390x/translate_vx.inc.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c index fd746ba35f..71059f9ca0 100644 --- a/target/s390x/translate_vx.inc.c +++ b/target/s390x/translate_vx.inc.c @@ -2214,20 +2214,13 @@ static void gen_sbcbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, TCGv_i64 ah, { TCGv_i64 th = tcg_temp_new_i64(); TCGv_i64 tl = tcg_temp_new_i64(); - TCGv_i64 zero = tcg_const_i64(0); - tcg_gen_andi_i64(tl, cl, 1); - tcg_gen_sub2_i64(tl, th, al, zero, tl, zero); - tcg_gen_sub2_i64(tl, th, tl, th, bl, zero); - tcg_gen_andi_i64(th, th, 1); - tcg_gen_sub2_i64(tl, th, ah, zero, th, zero); - tcg_gen_sub2_i64(tl, th, tl, th, bh, zero); - tcg_gen_andi_i64(dl, th, 1); - tcg_gen_mov_i64(dh, zero); + tcg_gen_not_i64(tl, bl); + tcg_gen_not_i64(th, bh); + gen_accc2_i64(dl, dh, al, ah, tl, th, cl, ch); tcg_temp_free_i64(tl); tcg_temp_free_i64(th); - tcg_temp_free_i64(zero); } static DisasJumpType op_vsbcbi(DisasContext *s, DisasOps *o) From de60a92ea7b7977854420c58fd98f38cb6de6de6 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 21 Oct 2019 12:05:15 +0200 Subject: [PATCH 9/9] s390x/kvm: Set default cpu model for all machine classes We have to set the default model of all machine classes, not just for the active one. Otherwise, "query-machines" will indicate the wrong CPU model ("qemu-s390x-cpu" instead of "host-s390x-cpu") as "default-cpu-type". Doing a {"execute":"query-machines"} under KVM now results in {"return": [ { "hotpluggable-cpus": true, "name": "s390-ccw-virtio-4.0", "numa-mem-supported": false, "default-cpu-type": "host-s390x-cpu", "cpu-max": 248, "deprecated": false}, { "hotpluggable-cpus": true, "name": "s390-ccw-virtio-2.7", "numa-mem-supported": false, "default-cpu-type": "host-s390x-cpu", "cpu-max": 248, "deprecated": false } ... Libvirt probes all machines via "-machine none,accel=kvm:tcg" and will currently see the wrong CPU model under KVM. Reported-by: Jiri Denemark Reviewed-by: Thomas Huth Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing") Cc: Igor Mammedov Signed-off-by: David Hildenbrand Message-Id: <20191021100515.6978-1-david@redhat.com> Signed-off-by: Cornelia Huck --- target/s390x/kvm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index c24c869e77..0c9d14b4b1 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -320,11 +320,17 @@ void kvm_s390_set_max_pagesize(uint64_t pagesize, Error **errp) cap_hpage_1m = 1; } -int kvm_arch_init(MachineState *ms, KVMState *s) +static void ccw_machine_class_foreach(ObjectClass *oc, void *opaque) { - MachineClass *mc = MACHINE_GET_CLASS(ms); + MachineClass *mc = MACHINE_CLASS(oc); mc->default_cpu_type = S390_CPU_TYPE_NAME("host"); +} + +int kvm_arch_init(MachineState *ms, KVMState *s) +{ + object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE, + false, NULL); if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) { error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "