From f5f9c6ea11bc807664fdeb9354915c2c9cdcbd89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sat, 24 Jun 2023 22:06:44 +0200 Subject: [PATCH 01/21] hw/s390x: Move KVM specific PV from hw/ to target/s390x/kvm/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protected Virtualization (PV) is not a real hardware device: it is a feature of the firmware on s390x that is exposed to userspace via the KVM interface. Move the pv.c/pv.h files to target/s390x/kvm/ to make this clearer. Suggested-by: Thomas Huth Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20230624200644.23931-1-philmd@linaro.org> Signed-off-by: Thomas Huth --- MAINTAINERS | 2 -- hw/s390x/ipl.c | 2 +- hw/s390x/meson.build | 1 - hw/s390x/s390-pci-kvm.c | 2 +- hw/s390x/s390-virtio-ccw.c | 2 +- hw/s390x/tod-kvm.c | 2 +- target/s390x/arch_dump.c | 2 +- target/s390x/cpu-sysemu.c | 2 +- target/s390x/cpu_features.c | 2 +- target/s390x/cpu_models.c | 2 +- target/s390x/diag.c | 2 +- target/s390x/helper.c | 2 +- target/s390x/ioinst.c | 2 +- target/s390x/kvm/kvm.c | 2 +- target/s390x/kvm/meson.build | 1 + {hw/s390x => target/s390x/kvm}/pv.c | 2 +- {include/hw/s390x => target/s390x/kvm}/pv.h | 0 17 files changed, 14 insertions(+), 16 deletions(-) rename {hw/s390x => target/s390x/kvm}/pv.c (99%) rename {include/hw/s390x => target/s390x/kvm}/pv.h (100%) diff --git a/MAINTAINERS b/MAINTAINERS index 1817cfc62f..43bd9afc19 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -452,8 +452,6 @@ S: Supported F: target/s390x/kvm/ F: target/s390x/machine.c F: target/s390x/sigp.c -F: hw/s390x/pv.c -F: include/hw/s390x/pv.h F: gdb-xml/s390*.xml T: git https://github.com/borntraeger/qemu.git s390-next L: qemu-s390x@nongnu.org diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 8612684d48..515dcf51b5 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -26,7 +26,7 @@ #include "hw/s390x/vfio-ccw.h" #include "hw/s390x/css.h" #include "hw/s390x/ebcdic.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #include "hw/scsi/scsi.h" #include "hw/virtio/virtio-net.h" #include "ipl.h" diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build index f291016fee..6fd096813a 100644 --- a/hw/s390x/meson.build +++ b/hw/s390x/meson.build @@ -22,7 +22,6 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( 'tod-kvm.c', 's390-skeys-kvm.c', 's390-stattrib-kvm.c', - 'pv.c', 's390-pci-kvm.c', )) s390x_ss.add(when: 'CONFIG_TCG', if_true: files( diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c index 9134fe185f..ff41e4106d 100644 --- a/hw/s390x/s390-pci-kvm.c +++ b/hw/s390x/s390-pci-kvm.c @@ -14,7 +14,7 @@ #include #include "kvm/kvm_s390x.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #include "hw/s390x/s390-pci-bus.h" #include "hw/s390x/s390-pci-kvm.h" #include "hw/s390x/s390-pci-inst.h" diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 2dece8eab8..4516d73ff5 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -42,7 +42,7 @@ #include "hw/s390x/tod.h" #include "sysemu/sysemu.h" #include "sysemu/cpus.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #include "migration/blocker.h" #include "qapi/visitor.h" diff --git a/hw/s390x/tod-kvm.c b/hw/s390x/tod-kvm.c index e2202dae2d..9588b90f2b 100644 --- a/hw/s390x/tod-kvm.c +++ b/hw/s390x/tod-kvm.c @@ -13,7 +13,7 @@ #include "qemu/module.h" #include "sysemu/runstate.h" #include "hw/s390x/tod.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #include "kvm/kvm_s390x.h" static void kvm_s390_get_tod_raw(S390TOD *tod, Error **errp) diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c index cb98f4894d..51a2116515 100644 --- a/target/s390x/arch_dump.c +++ b/target/s390x/arch_dump.c @@ -17,8 +17,8 @@ #include "s390x-internal.h" #include "elf.h" #include "sysemu/dump.h" -#include "hw/s390x/pv.h" #include "kvm/kvm_s390x.h" +#include "target/s390x/kvm/pv.h" struct S390xUserRegsStruct { uint64_t psw[2]; diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c index 97d6c760a8..8112561e5e 100644 --- a/target/s390x/cpu-sysemu.c +++ b/target/s390x/cpu-sysemu.c @@ -33,7 +33,7 @@ #include "qapi/qapi-visit-run-state.h" #include "sysemu/hw_accel.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #include "hw/boards.h" #include "sysemu/sysemu.h" #include "sysemu/tcg.h" diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c index 2e4e11d264..ebb155ce1c 100644 --- a/target/s390x/cpu_features.c +++ b/target/s390x/cpu_features.c @@ -15,7 +15,7 @@ #include "qemu/module.h" #include "cpu_features.h" #ifndef CONFIG_USER_ONLY -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #endif #define DEF_FEAT(_FEAT, _NAME, _TYPE, _BIT, _DESC) \ diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index ae8880e81d..42b52afdb4 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -24,7 +24,7 @@ #include "qemu/qemu-print.h" #ifndef CONFIG_USER_ONLY #include "sysemu/sysemu.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #endif #define CPUDEF_INIT(_type, _gen, _ec_ga, _mha_pow, _hmfai, _name, _desc) \ diff --git a/target/s390x/diag.c b/target/s390x/diag.c index e5f0df19e7..8ce18e08f3 100644 --- a/target/s390x/diag.c +++ b/target/s390x/diag.c @@ -19,9 +19,9 @@ #include "sysemu/cpus.h" #include "hw/s390x/ipl.h" #include "hw/s390x/s390-virtio-ccw.h" -#include "hw/s390x/pv.h" #include "sysemu/kvm.h" #include "kvm/kvm_s390x.h" +#include "target/s390x/kvm/pv.h" #include "qemu/error-report.h" diff --git a/target/s390x/helper.c b/target/s390x/helper.c index 2b363aa959..d76c06381b 100644 --- a/target/s390x/helper.c +++ b/target/s390x/helper.c @@ -24,7 +24,7 @@ #include "gdbstub/helpers.h" #include "qemu/timer.h" #include "hw/s390x/ioinst.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #include "sysemu/hw_accel.h" #include "sysemu/runstate.h" diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c index 053aaabb5a..bbe45a497a 100644 --- a/target/s390x/ioinst.c +++ b/target/s390x/ioinst.c @@ -16,7 +16,7 @@ #include "hw/s390x/ioinst.h" #include "trace.h" #include "hw/s390x/s390-pci-bus.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" /* All I/O instructions but chsc use the s format */ static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb, diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c index 3ac7ec9acf..a9e5880349 100644 --- a/target/s390x/kvm/kvm.c +++ b/target/s390x/kvm/kvm.c @@ -50,7 +50,7 @@ #include "exec/memattrs.h" #include "hw/s390x/s390-virtio-ccw.h" #include "hw/s390x/s390-virtio-hcall.h" -#include "hw/s390x/pv.h" +#include "target/s390x/kvm/pv.h" #ifndef DEBUG_KVM #define DEBUG_KVM 0 diff --git a/target/s390x/kvm/meson.build b/target/s390x/kvm/meson.build index 37253f75bf..d6aca590ae 100644 --- a/target/s390x/kvm/meson.build +++ b/target/s390x/kvm/meson.build @@ -1,5 +1,6 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( + 'pv.c', 'kvm.c' ), if_false: files( 'stubs.c' diff --git a/hw/s390x/pv.c b/target/s390x/kvm/pv.c similarity index 99% rename from hw/s390x/pv.c rename to target/s390x/kvm/pv.c index b63f3784c6..6a69be7e5c 100644 --- a/hw/s390x/pv.c +++ b/target/s390x/kvm/pv.c @@ -21,9 +21,9 @@ #include "qom/object_interfaces.h" #include "exec/confidential-guest-support.h" #include "hw/s390x/ipl.h" -#include "hw/s390x/pv.h" #include "hw/s390x/sclp.h" #include "target/s390x/kvm/kvm_s390x.h" +#include "target/s390x/kvm/pv.h" static bool info_valid; static struct kvm_s390_pv_info_vm info_vm; diff --git a/include/hw/s390x/pv.h b/target/s390x/kvm/pv.h similarity index 100% rename from include/hw/s390x/pv.h rename to target/s390x/kvm/pv.h From 78a1e153f93e7bbec5d7efefa5f89fd011e9220f Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:25 +0200 Subject: [PATCH 02/21] linux-user: elfload: Add more initial s390x PSW bits Make the PSW look more similar to the real s390x userspace PSW. Except for being there, the newly added bits should not affect the userspace code execution. Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Message-Id: <20230704081506.276055-2-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- linux-user/elfload.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 9a2ec568b0..d3d1352c4e 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1635,7 +1635,9 @@ const char *elf_hwcap_str(uint32_t bit) static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop) { regs->psw.addr = infop->entry; - regs->psw.mask = PSW_MASK_64 | PSW_MASK_32; + regs->psw.mask = PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \ + PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_MASK_64 | \ + PSW_MASK_32; regs->gprs[15] = infop->start_stack; } From 110b1bac2ecd94a78a1d38003e24e37367bf074e Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:26 +0200 Subject: [PATCH 03/21] target/s390x: Fix EPSW CC reporting EPSW should explicitly calculate and insert CC, like IPM does. Fixes: e30a9d3fea58 ("target-s390: Implement EPSW") Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Cc: qemu-stable@nongnu.org Message-Id: <20230704081506.276055-3-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- target/s390x/tcg/translate.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index a6ee2d4423..0cef6efbef 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -2383,10 +2383,14 @@ static DisasJumpType op_epsw(DisasContext *s, DisasOps *o) int r1 = get_field(s, r1); int r2 = get_field(s, r2); TCGv_i64 t = tcg_temp_new_i64(); + TCGv_i64 t_cc = tcg_temp_new_i64(); /* Note the "subsequently" in the PoO, which implies a defined result if r1 == r2. Thus we cannot defer these writes to an output hook. */ + gen_op_calc_cc(s); + tcg_gen_extu_i32_i64(t_cc, cc_op); tcg_gen_shri_i64(t, psw_mask, 32); + tcg_gen_deposit_i64(t, t, t_cc, 12, 2); store_reg32_i64(r1, t); if (r2 != 0) { store_reg32_i64(r2, psw_mask); From fed9a4fe0ce0ec917a6b3a2da0a7ecd3cb9eba56 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:27 +0200 Subject: [PATCH 04/21] target/s390x: Fix MDEB and MDEBR These instructions multiply 32 bits by 32 bits, not 32 bits by 64 bits. Fixes: 83b00736f3d8 ("target-s390: Convert FP MULTIPLY") Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Cc: qemu-stable@nongnu.org Message-Id: <20230704081506.276055-4-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- target/s390x/tcg/fpu_helper.c | 3 ++- target/s390x/tcg/insn-data.h.inc | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/target/s390x/tcg/fpu_helper.c b/target/s390x/tcg/fpu_helper.c index 57e5829283..4b7fa58af3 100644 --- a/target/s390x/tcg/fpu_helper.c +++ b/target/s390x/tcg/fpu_helper.c @@ -306,8 +306,9 @@ uint64_t HELPER(mdb)(CPUS390XState *env, uint64_t f1, uint64_t f2) /* 64/32-bit FP multiplication */ uint64_t HELPER(mdeb)(CPUS390XState *env, uint64_t f1, uint64_t f2) { + float64 f1_64 = float32_to_float64(f1, &env->fpu_status); float64 ret = float32_to_float64(f2, &env->fpu_status); - ret = float64_mul(f1, ret, &env->fpu_status); + ret = float64_mul(f1_64, ret, &env->fpu_status); handle_exceptions(env, false, GETPC()); return ret; } diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc index 0a45dbbcda..457ed25d2f 100644 --- a/target/s390x/tcg/insn-data.h.inc +++ b/target/s390x/tcg/insn-data.h.inc @@ -667,11 +667,11 @@ F(0xb317, MEEBR, RRE, Z, e1, e2, new, e1, meeb, 0, IF_BFP) F(0xb31c, MDBR, RRE, Z, f1, f2, new, f1, mdb, 0, IF_BFP) F(0xb34c, MXBR, RRE, Z, x1, x2, new_x, x1, mxb, 0, IF_BFP) - F(0xb30c, MDEBR, RRE, Z, f1, e2, new, f1, mdeb, 0, IF_BFP) + F(0xb30c, MDEBR, RRE, Z, e1, e2, new, f1, mdeb, 0, IF_BFP) F(0xb307, MXDBR, RRE, Z, f1, f2, new_x, x1, mxdb, 0, IF_BFP) F(0xed17, MEEB, RXE, Z, e1, m2_32u, new, e1, meeb, 0, IF_BFP) F(0xed1c, MDB, RXE, Z, f1, m2_64, new, f1, mdb, 0, IF_BFP) - F(0xed0c, MDEB, RXE, Z, f1, m2_32u, new, f1, mdeb, 0, IF_BFP) + F(0xed0c, MDEB, RXE, Z, e1, m2_32u, new, f1, mdeb, 0, IF_BFP) F(0xed07, MXDB, RXE, Z, f1, m2_64, new_x, x1, mxdb, 0, IF_BFP) /* MULTIPLY HALFWORD */ C(0x4c00, MH, RX_a, Z, r1_o, m2_16s, new, r1_32, mul, 0) From 92a57534619a4058544ce8f9c0beae3e054f342b Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:28 +0200 Subject: [PATCH 05/21] target/s390x: Fix MVCRL with a large value in R0 Using a large R0 causes an assertion error: qemu-s390x: target/s390x/tcg/mem_helper.c:183: access_prepare_nf: Assertion `size > 0 && size <= 4096' failed. Even though PoP explicitly advises against using more than 8 bits for the size, an emulator crash is never a good thing. Fix by truncating the size to 8 bits. Fixes: ea0a1053e276 ("s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x") Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Cc: qemu-stable@nongnu.org Message-Id: <20230704081506.276055-5-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- target/s390x/tcg/mem_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c index d02ec861d8..84ad85212c 100644 --- a/target/s390x/tcg/mem_helper.c +++ b/target/s390x/tcg/mem_helper.c @@ -514,6 +514,7 @@ void HELPER(mvcrl)(CPUS390XState *env, uint64_t l, uint64_t dest, uint64_t src) int32_t i; /* MVCRL always copies one more byte than specified - maximum is 256 */ + l &= 0xff; l++; access_prepare(&srca, env, src, l, MMU_DATA_LOAD, mmu_idx, ra); From 6da311a60d58dba27f5f790217d5ebba944e34ab Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:29 +0200 Subject: [PATCH 06/21] target/s390x: Fix LRA overwriting the top 32 bits on DAT error When a DAT error occurs, LRA is supposed to write the error information to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone. Fix by passing the original value of R1 into helper and copying the top 32 bits to the return value. Fixes: d8fe4a9c284f ("target-s390: Convert LRA") Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Cc: qemu-stable@nongnu.org Message-Id: <20230704081506.276055-6-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- target/s390x/helper.h | 2 +- target/s390x/tcg/mem_helper.c | 4 ++-- target/s390x/tcg/translate.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/target/s390x/helper.h b/target/s390x/helper.h index 6bc01df73d..05102578fc 100644 --- a/target/s390x/helper.h +++ b/target/s390x/helper.h @@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void, env, i64, i64, i32) 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) +DEF_HELPER_3(lra, i64, env, i64, i64) DEF_HELPER_1(per_check_exception, void, env) DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64) DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64) diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c index 84ad85212c..f417fb1183 100644 --- a/target/s390x/tcg/mem_helper.c +++ b/target/s390x/tcg/mem_helper.c @@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env) } /* load real address */ -uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr) +uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t addr) { uint64_t asc = env->psw.mask & PSW_MASK_ASC; uint64_t ret, tec; @@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr) exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret, &flags, &tec); if (exc) { cc = 3; - ret = exc | 0x80000000; + ret = (r1 & 0xFFFFFFFF00000000ULL) | exc | 0x80000000; } else { cc = 0; ret |= addr & ~TARGET_PAGE_MASK; diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index 0cef6efbef..a6079ab7b4 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext *s, DisasOps *o) static DisasJumpType op_lra(DisasContext *s, DisasOps *o) { - gen_helper_lra(o->out, cpu_env, o->in2); + gen_helper_lra(o->out, cpu_env, o->out, o->in2); set_cc_static(s); return DISAS_NEXT; } From b0ef81062d2404ccef0289b1cc6e70244901c9be Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:30 +0200 Subject: [PATCH 07/21] target/s390x: Fix LRA when DAT is off LRA should perform DAT regardless of whether it's on or off. Disable DAT check for MMU_S390_LRA. Fixes: defb0e3157af ("s390x: Implement opcode helpers") Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Cc: qemu-stable@nongnu.org Message-Id: <20230704081506.276055-7-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- target/s390x/mmu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index b04b57c235..fbb2f1b4d4 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -417,7 +417,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, vaddr &= TARGET_PAGE_MASK; - if (!(env->psw.mask & PSW_MASK_DAT)) { + if (rw != MMU_S390_LRA && !(env->psw.mask & PSW_MASK_DAT)) { *raddr = vaddr; goto nodat; } From 349372ff9e3e7c047e258383f061a8617f66adc3 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:31 +0200 Subject: [PATCH 08/21] target/s390x: Fix relative long instructions with large offsets The expression "imm * 2" in gen_ri2() can wrap around if imm is large enough. Fix by casting imm to int64_t, like it's done in disas_jdest(). Fixes: e8ecdfeb30f0 ("Fix EXECUTE of relative branches") Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Message-Id: <20230704081506.276055-8-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- target/s390x/tcg/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index a6079ab7b4..6661b27efa 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -5794,7 +5794,7 @@ static TCGv gen_ri2(DisasContext *s) disas_jdest(s, i2, is_imm, imm, ri2); if (is_imm) { - ri2 = tcg_constant_i64(s->base.pc_next + imm * 2); + ri2 = tcg_constant_i64(s->base.pc_next + (int64_t)imm * 2); } return ri2; From f5c2ae7134e388cd543fa0383191a30ea07d272b Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:32 +0200 Subject: [PATCH 09/21] tests/tcg/s390x: Test EPSW Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Message-Id: <20230704081506.276055-9-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/epsw.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/tcg/s390x/epsw.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 85abfbb98c..2ef22c88d9 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -36,6 +36,7 @@ TESTS+=rxsbg TESTS+=ex-relative-long TESTS+=ex-branch TESTS+=mxdb +TESTS+=epsw cdsg: CFLAGS+=-pthread cdsg: LDFLAGS+=-pthread diff --git a/tests/tcg/s390x/epsw.c b/tests/tcg/s390x/epsw.c new file mode 100644 index 0000000000..affb1a5e3a --- /dev/null +++ b/tests/tcg/s390x/epsw.c @@ -0,0 +1,23 @@ +/* + * Test the EPSW instruction. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include +#include + +int main(void) +{ + unsigned long r1 = 0x1234567887654321UL, r2 = 0x8765432112345678UL; + + asm("cr %[r1],%[r2]\n" /* cc = 1 */ + "epsw %[r1],%[r2]" + : [r1] "+r" (r1), [r2] "+r" (r2) : : "cc"); + + /* Do not check the R and RI bits. */ + r1 &= ~0x40000008UL; + assert(r1 == 0x1234567807051001UL); + assert(r2 == 0x8765432180000000UL); + + return EXIT_SUCCESS; +} From ad85ac6a8f8325b6a15058e99d76203b4cde6044 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:33 +0200 Subject: [PATCH 10/21] tests/tcg/s390x: Test LARL with a large offset Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich Acked-by: David Hildenbrand Message-Id: <20230704081506.276055-10-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/larl.c | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 tests/tcg/s390x/larl.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 2ef22c88d9..dbf64c991e 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -37,6 +37,7 @@ TESTS+=ex-relative-long TESTS+=ex-branch TESTS+=mxdb TESTS+=epsw +TESTS+=larl cdsg: CFLAGS+=-pthread cdsg: LDFLAGS+=-pthread diff --git a/tests/tcg/s390x/larl.c b/tests/tcg/s390x/larl.c new file mode 100644 index 0000000000..7c95f89be7 --- /dev/null +++ b/tests/tcg/s390x/larl.c @@ -0,0 +1,21 @@ +/* + * Test the LARL instruction. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include + +int main(void) +{ + long algfi = (long)main; + long larl; + + /* + * The compiler may emit larl for the C addition, so compute the expected + * value using algfi. + */ + asm("algfi %[r],0xd0000000" : [r] "+r" (algfi) : : "cc"); + asm("larl %[r],main+0xd0000000" : [r] "=r" (larl)); + + return algfi == larl ? EXIT_SUCCESS : EXIT_FAILURE; +} From 028dc70e1826ab182a84fb4ed43241fef1320a02 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:34 +0200 Subject: [PATCH 11/21] tests/tcg/s390x: Test LRA Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich Message-Id: <20230704081506.276055-11-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/lra.S | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/tcg/s390x/lra.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index 44dfd71629..242c7b0f83 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -20,6 +20,7 @@ ASM_TESTS = \ sam \ lpsw \ lpswe-early \ + lra \ ssm-early \ stosm-early \ unaligned-lowcore diff --git a/tests/tcg/s390x/lra.S b/tests/tcg/s390x/lra.S new file mode 100644 index 0000000000..79ab86f36b --- /dev/null +++ b/tests/tcg/s390x/lra.S @@ -0,0 +1,19 @@ + .org 0x200 /* lowcore padding */ + .globl _start +_start: + lgrl %r1,initial_r1 + lra %r1,0(%r1) + cgrl %r1,expected_r1 + jne 1f + lpswe success_psw +1: + lpswe failure_psw + .align 8 +initial_r1: + .quad 0x8765432112345678 +expected_r1: + .quad 0x8765432180000038 /* ASCE type exception */ +success_psw: + .quad 0x2000000000000,0xfff /* see is_special_wait_psw() */ +failure_psw: + .quad 0x2000000000000,0 /* disabled wait */ From 85411ac9b3e1164c4deb2e9eed6c152ef64f2c51 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:35 +0200 Subject: [PATCH 12/21] tests/tcg/s390x: Test MDEB and MDEBR Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich Acked-by: David Hildenbrand Message-Id: <20230704081506.276055-12-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/mdeb.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 tests/tcg/s390x/mdeb.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index dbf64c991e..19fbbc6e53 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -38,6 +38,7 @@ TESTS+=ex-branch TESTS+=mxdb TESTS+=epsw TESTS+=larl +TESTS+=mdeb cdsg: CFLAGS+=-pthread cdsg: LDFLAGS+=-pthread diff --git a/tests/tcg/s390x/mdeb.c b/tests/tcg/s390x/mdeb.c new file mode 100644 index 0000000000..4897d28069 --- /dev/null +++ b/tests/tcg/s390x/mdeb.c @@ -0,0 +1,30 @@ +/* + * Test the MDEB and MDEBR instructions. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include +#include + +int main(void) +{ + union { + float f[2]; + double d; + } a; + float b; + + a.f[0] = 1.2345; + a.f[1] = 999; + b = 6.789; + asm("mdeb %[a],%[b]" : [a] "+f" (a.d) : [b] "R" (b)); + assert(a.d > 8.38 && a.d < 8.39); + + a.f[0] = 1.2345; + a.f[1] = 999; + b = 6.789; + asm("mdebr %[a],%[b]" : [a] "+f" (a.d) : [b] "f" (b)); + assert(a.d > 8.38 && a.d < 8.39); + + return EXIT_SUCCESS; +} From bfde1be8b3417bf9080175fefac0becb81814631 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 4 Jul 2023 10:12:36 +0200 Subject: [PATCH 13/21] tests/tcg/s390x: Test MVCRL with a large value in R0 Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Message-Id: <20230704081506.276055-13-iii@linux.ibm.com> [thuth: Apply fix for compiling with GCC 11] Signed-off-by: Thomas Huth --- tests/tcg/s390x/mie3-mvcrl.c | 46 ++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c index 93c7b0a290..6d3d049f2c 100644 --- a/tests/tcg/s390x/mie3-mvcrl.c +++ b/tests/tcg/s390x/mie3-mvcrl.c @@ -1,29 +1,55 @@ +#include #include +#include #include - -static inline void mvcrl_8(const char *dst, const char *src) +static void mvcrl(const char *dst, const char *src, size_t len) { + register long r0 asm("r0") = len; + asm volatile ( - "llill %%r0, 8\n" ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])" - : : [dst] "d" (dst), [src] "d" (src) - : "r0", "memory"); + : : [dst] "d" (dst), [src] "d" (src), "r" (r0) + : "memory"); } - -int main(int argc, char *argv[]) +static bool test(void) { const char *alpha = "abcdefghijklmnop"; /* array missing 'i' */ - char tstr[17] = "abcdefghjklmnop\0" ; + char tstr[17] = "abcdefghjklmnop\0"; /* mvcrl reference use: 'open a hole in an array' */ - mvcrl_8(tstr + 9, tstr + 8); + mvcrl(tstr + 9, tstr + 8, 8); /* place missing 'i' */ tstr[8] = 'i'; - return strncmp(alpha, tstr, 16ul); + return strncmp(alpha, tstr, 16ul) == 0; +} + +static bool test_bad_r0(void) +{ + char src[256] = { 0 }; + + /* + * PoP says: Bits 32-55 of general register 0 should contain zeros; + * otherwise, the program may not operate compatibly in the future. + * + * Try it anyway in order to check whether this would crash QEMU itself. + */ + mvcrl(src, src, (size_t)-1); + + return true; +} + +int main(void) +{ + bool ok = true; + + ok &= test(); + ok &= test_bad_r0(); + + return ok ? EXIT_SUCCESS : EXIT_FAILURE; } From 5a7d4dc9f84b1531cfd741404f44f3ad9c30faa7 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Tue, 4 Jul 2023 09:16:53 +0200 Subject: [PATCH 14/21] tests/qtest/readconfig-test: Allow testing for arbitrary memory sizes Make test_x86_memdev_resp() more flexible by allowing arbitrary memory sizes as parameter here. Message-Id: <20230704071655.75381-2-thuth@redhat.com> Signed-off-by: Thomas Huth --- tests/qtest/readconfig-test.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c index ac7242451b..74526f3af2 100644 --- a/tests/qtest/readconfig-test.c +++ b/tests/qtest/readconfig-test.c @@ -48,7 +48,7 @@ static QTestState *qtest_init_with_config(const char *cfgdata) return qts; } -static void test_x86_memdev_resp(QObject *res) +static void test_x86_memdev_resp(QObject *res, const char *mem_id, int size) { Visitor *v; g_autoptr(MemdevList) memdevs = NULL; @@ -63,8 +63,8 @@ static void test_x86_memdev_resp(QObject *res) g_assert(!memdevs->next); memdev = memdevs->value; - g_assert_cmpstr(memdev->id, ==, "ram"); - g_assert_cmpint(memdev->size, ==, 200 * MiB); + g_assert_cmpstr(memdev->id, ==, mem_id); + g_assert_cmpint(memdev->size, ==, size * MiB); visit_free(v); } @@ -80,7 +80,7 @@ static void test_x86_memdev(void) qts = qtest_init_with_config(cfgdata); /* Test valid command */ resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }"); - test_x86_memdev_resp(qdict_get(resp, "return")); + test_x86_memdev_resp(qdict_get(resp, "return"), "ram", 200); qobject_unref(resp); qtest_quit(qts); From 25919c4025313763c3c00035f439bd11579c2c38 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Tue, 4 Jul 2023 09:16:54 +0200 Subject: [PATCH 15/21] tests/qtest: Move mkimg() and have_qemu_img() from libqos to libqtest These two functions can be useful for other qtests beside the qos-test, too, so move them to libqtest instead. Message-Id: <20230704071655.75381-3-thuth@redhat.com> Signed-off-by: Thomas Huth --- tests/qtest/libqos/libqos.c | 49 +--------------------------------- tests/qtest/libqos/libqos.h | 2 -- tests/qtest/libqtest.c | 52 +++++++++++++++++++++++++++++++++++++ tests/qtest/libqtest.h | 20 ++++++++++++++ 4 files changed, 73 insertions(+), 50 deletions(-) diff --git a/tests/qtest/libqos/libqos.c b/tests/qtest/libqos/libqos.c index 5ffda080ec..5c0fa1f7c5 100644 --- a/tests/qtest/libqos/libqos.c +++ b/tests/qtest/libqos/libqos.c @@ -137,56 +137,9 @@ void migrate(QOSState *from, QOSState *to, const char *uri) migrate_allocator(&from->alloc, &to->alloc); } -bool have_qemu_img(void) -{ - char *rpath; - const char *path = getenv("QTEST_QEMU_IMG"); - if (!path) { - return false; - } - - rpath = realpath(path, NULL); - if (!rpath) { - return false; - } else { - free(rpath); - return true; - } -} - -void mkimg(const char *file, const char *fmt, unsigned size_mb) -{ - gchar *cli; - bool ret; - int rc; - GError *err = NULL; - char *qemu_img_path; - gchar *out, *out2; - char *qemu_img_abs_path; - - qemu_img_path = getenv("QTEST_QEMU_IMG"); - g_assert(qemu_img_path); - qemu_img_abs_path = realpath(qemu_img_path, NULL); - g_assert(qemu_img_abs_path); - - cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path, - fmt, file, size_mb); - ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err); - if (err || !g_spawn_check_exit_status(rc, &err)) { - fprintf(stderr, "%s\n", err->message); - g_error_free(err); - } - g_assert(ret && !err); - - g_free(out); - g_free(out2); - g_free(cli); - free(qemu_img_abs_path); -} - void mkqcow2(const char *file, unsigned size_mb) { - return mkimg(file, "qcow2", size_mb); + g_assert_true(mkimg(file, "qcow2", size_mb)); } void prepare_blkdebug_script(const char *debug_fn, const char *event) diff --git a/tests/qtest/libqos/libqos.h b/tests/qtest/libqos/libqos.h index 12d05b2365..c04950e2b1 100644 --- a/tests/qtest/libqos/libqos.h +++ b/tests/qtest/libqos/libqos.h @@ -27,8 +27,6 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...) G_GNUC_PRINTF(2, 3); void qtest_common_shutdown(QOSState *qs); void qtest_shutdown(QOSState *qs); -bool have_qemu_img(void); -void mkimg(const char *file, const char *fmt, unsigned size_mb); void mkqcow2(const char *file, unsigned size_mb); void migrate(QOSState *from, QOSState *to, const char *uri); void prepare_blkdebug_script(const char *debug_fn, const char *event); diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 79152f0ec3..c22dfc30d3 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -1742,3 +1742,55 @@ bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property) return b; } + +bool have_qemu_img(void) +{ + char *rpath; + const char *path = getenv("QTEST_QEMU_IMG"); + if (!path) { + return false; + } + + rpath = realpath(path, NULL); + if (!rpath) { + return false; + } else { + free(rpath); + return true; + } +} + +bool mkimg(const char *file, const char *fmt, unsigned size_mb) +{ + gchar *cli; + bool ret; + int rc; + GError *err = NULL; + char *qemu_img_path; + gchar *out, *out2; + char *qemu_img_abs_path; + + qemu_img_path = getenv("QTEST_QEMU_IMG"); + if (!qemu_img_path) { + return false; + } + qemu_img_abs_path = realpath(qemu_img_path, NULL); + if (!qemu_img_abs_path) { + return false; + } + + cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path, + fmt, file, size_mb); + ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err); + if (err || !g_spawn_check_exit_status(rc, &err)) { + fprintf(stderr, "%s\n", err->message); + g_error_free(err); + } + + g_free(out); + g_free(out2); + g_free(cli); + free(qemu_img_abs_path); + + return ret && !err; +} diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h index 913acc3d5c..3a71bc45fc 100644 --- a/tests/qtest/libqtest.h +++ b/tests/qtest/libqtest.h @@ -994,4 +994,24 @@ bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property); */ pid_t qtest_pid(QTestState *s); +/** + * have_qemu_img: + * + * Returns: true if "qemu-img" is available. + */ +bool have_qemu_img(void); + +/** + * mkimg: + * @file: File name of the image that should be created + * @fmt: Format, e.g. "qcow2" or "raw" + * @size_mb: Size of the image in megabytes + * + * Create a disk image with qemu-img. Note that the QTEST_QEMU_IMG + * environment variable must point to the qemu-img file. + * + * Returns: true if the image has been created successfully. + */ +bool mkimg(const char *file, const char *fmt, unsigned size_mb); + #endif From bc55e2eaa6d662f10a1f4cff0d285d77dbc9362a Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Tue, 4 Jul 2023 09:16:55 +0200 Subject: [PATCH 16/21] tests/qtest/readconfig: Test the docs/config/q35-*.cfg files Test that we can successfully parse the docs/config/q35-emulated.cfg, docs/config/q35-virtio-graphical.cfg and docs/config/q35-virtio-serial.cfg config files (the "...-serial.cfg" file is a subset of the graphical config file, so we skip that in quick mode). These config files use two hard-coded image names which we have to replace with unique temporary files to avoid race conditions in case the tests are run in parallel. So after creating the temporary image files, we also have to create a copy of the config file where we replaced the hard-coded image names. If KVM is not available, we also have to disable the "accel" lines. Once everything is in place, we can start QEMU with the modified config file and check that everything is available in QEMU. Message-Id: <20230704071655.75381-4-thuth@redhat.com> Signed-off-by: Thomas Huth --- tests/qtest/readconfig-test.c | 196 ++++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c index 74526f3af2..760f974e63 100644 --- a/tests/qtest/readconfig-test.c +++ b/tests/qtest/readconfig-test.c @@ -197,6 +197,189 @@ static void test_docs_config_ich9(void) qtest_quit(qts); } +#if defined(CONFIG_POSIX) && defined(CONFIG_SLIRP) + +static char *make_temp_img(const char *template, const char *format, int size) +{ + GError *error = NULL; + char *temp_name; + int fd; + + /* Create a temporary image names */ + fd = g_file_open_tmp(template, &temp_name, &error); + if (fd == -1) { + fprintf(stderr, "unable to create file: %s\n", error->message); + g_error_free(error); + return NULL; + } + close(fd); + + if (!mkimg(temp_name, format, size)) { + fprintf(stderr, "qemu-img failed to create %s\n", temp_name); + g_free(temp_name); + return NULL; + } + + return temp_name; +} + +struct device { + const char *name; + const char *type; +}; + +static void test_docs_q35(const char *input_file, struct device *devices) +{ + QTestState *qts; + QDict *resp; + QObject *qobj; + int ret, i; + g_autofree char *cfg_file = NULL, *sedcmd = NULL; + g_autofree char *hd_file = NULL, *cd_file = NULL; + + /* Check that all the devices are available in the QEMU binary */ + for (i = 0; devices[i].name; i++) { + if (!qtest_has_device(devices[i].type)) { + g_test_skip("one of the required devices is not available"); + return; + } + } + + hd_file = make_temp_img("qtest_disk_XXXXXX.qcow2", "qcow2", 1); + cd_file = make_temp_img("qtest_cdrom_XXXXXX.iso", "raw", 1); + if (!hd_file || !cd_file) { + g_test_skip("could not create disk images"); + goto cleanup; + } + + /* Create a temporary config file where we replace the disk image names */ + ret = g_file_open_tmp("q35-emulated-XXXXXX.cfg", &cfg_file, NULL); + if (ret == -1) { + g_test_skip("could not create temporary config file"); + goto cleanup; + } + close(ret); + + sedcmd = g_strdup_printf("sed -e 's,guest.qcow2,%s,' -e 's,install.iso,%s,'" + " %s %s > '%s'", + hd_file, cd_file, + !qtest_has_accel("kvm") ? "-e '/accel/d'" : "", + input_file, cfg_file); + ret = system(sedcmd); + if (ret) { + g_test_skip("could not modify temporary config file"); + goto cleanup; + } + + qts = qtest_initf("-machine none -nodefaults -readconfig %s", cfg_file); + + /* Check memory size */ + resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }"); + test_x86_memdev_resp(qdict_get(resp, "return"), "pc.ram", 1024); + qobject_unref(resp); + + resp = qtest_qmp(qts, "{ 'execute': 'qom-list'," + " 'arguments': {'path': '/machine/peripheral' }}"); + qobj = qdict_get(resp, "return"); + + /* Check that all the devices have been created */ + for (i = 0; devices[i].name; i++) { + test_object_available(qobj, devices[i].name, devices[i].type); + } + + qobject_unref(resp); + + qtest_quit(qts); + +cleanup: + if (hd_file) { + unlink(hd_file); + } + if (cd_file) { + unlink(cd_file); + } + if (cfg_file) { + unlink(cfg_file); + } +} + +static void test_docs_q35_emulated(void) +{ + struct device devices[] = { + { "ich9-pcie-port-1", "ioh3420" }, + { "ich9-pcie-port-2", "ioh3420" }, + { "ich9-pcie-port-3", "ioh3420" }, + { "ich9-pcie-port-4", "ioh3420" }, + { "ich9-pci-bridge", "i82801b11-bridge" }, + { "ich9-ehci-1", "ich9-usb-ehci1" }, + { "ich9-ehci-2", "ich9-usb-ehci2" }, + { "ich9-uhci-1", "ich9-usb-uhci1" }, + { "ich9-uhci-2", "ich9-usb-uhci2" }, + { "ich9-uhci-3", "ich9-usb-uhci3" }, + { "ich9-uhci-4", "ich9-usb-uhci4" }, + { "ich9-uhci-5", "ich9-usb-uhci5" }, + { "ich9-uhci-6", "ich9-usb-uhci6" }, + { "sata-disk", "ide-hd" }, + { "sata-optical-disk", "ide-cd" }, + { "net", "e1000" }, + { "video", "VGA" }, + { "ich9-hda-audio", "ich9-intel-hda" }, + { "ich9-hda-duplex", "hda-duplex" }, + { NULL, NULL } + }; + + test_docs_q35("docs/config/q35-emulated.cfg", devices); +} + +static void test_docs_q35_virtio_graphical(void) +{ + struct device devices[] = { + { "pcie.1", "pcie-root-port" }, + { "pcie.2", "pcie-root-port" }, + { "pcie.3", "pcie-root-port" }, + { "pcie.4", "pcie-root-port" }, + { "pcie.5", "pcie-root-port" }, + { "pcie.6", "pcie-root-port" }, + { "pcie.7", "pcie-root-port" }, + { "pcie.8", "pcie-root-port" }, + { "scsi", "virtio-scsi-pci" }, + { "scsi-disk", "scsi-hd" }, + { "scsi-optical-disk", "scsi-cd" }, + { "net", "virtio-net-pci" }, + { "usb", "nec-usb-xhci" }, + { "tablet", "usb-tablet" }, + { "video", "qxl-vga" }, + { "sound", "ich9-intel-hda" }, + { "duplex", "hda-duplex" }, + { NULL, NULL } + }; + + test_docs_q35("docs/config/q35-virtio-graphical.cfg", devices); +} + +static void test_docs_q35_virtio_serial(void) +{ + struct device devices[] = { + { "pcie.1", "pcie-root-port" }, + { "pcie.2", "pcie-root-port" }, + { "pcie.3", "pcie-root-port" }, + { "pcie.4", "pcie-root-port" }, + { "pcie.5", "pcie-root-port" }, + { "pcie.6", "pcie-root-port" }, + { "pcie.7", "pcie-root-port" }, + { "pcie.8", "pcie-root-port" }, + { "scsi", "virtio-scsi-pci" }, + { "scsi-disk", "scsi-hd" }, + { "scsi-optical-disk", "scsi-cd" }, + { "net", "virtio-net-pci" }, + { NULL, NULL } + }; + + test_docs_q35("docs/config/q35-virtio-serial.cfg", devices); +} + +#endif /* CONFIG_LINUX */ + int main(int argc, char *argv[]) { const char *arch; @@ -211,6 +394,19 @@ int main(int argc, char *argv[]) qtest_has_device("ich9-usb-uhci1")) { qtest_add_func("readconfig/x86/ich9-ehci-uhci", test_docs_config_ich9); } +#if defined(CONFIG_POSIX) && defined(CONFIG_SLIRP) + qtest_add_func("readconfig/x86/q35-emulated", test_docs_q35_emulated); + qtest_add_func("readconfig/x86/q35-virtio-graphical", + test_docs_q35_virtio_graphical); + if (g_test_slow()) { + /* + * q35-virtio-serial.cfg is a subset of q35-virtio-graphical.cfg, + * so we can skip the test in quick mode + */ + qtest_add_func("readconfig/x86/q35-virtio-serial", + test_docs_q35_virtio_serial); + } +#endif } #if defined(CONFIG_SPICE) && !defined(__FreeBSD__) qtest_add_func("readconfig/spice", test_spice); From 9ffcbe2a60d24fc20b98f010dbca244df1fe82c5 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 3 Jul 2023 09:44:47 +0200 Subject: [PATCH 17/21] os-posix: Allow 'chroot' via '-run-with' and deprecate the old '-chroot' option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We recently introduced "-run-with" for options that influence the runtime behavior of QEMU. This option has the big advantage that it can group related options (so that it is easier for the users to spot them) and that the options become introspectable via QMP this way. So let's start moving more switches into this option group, starting with "-chroot" now. Reviewed-by: Claudio Imbrenda Reviewed-by: Michael Tokarev Reviewed-by: Ján Tomko Message-Id: <20230703074447.17044-1-thuth@redhat.com> Signed-off-by: Thomas Huth --- docs/about/deprecated.rst | 5 +++++ os-posix.c | 35 ++++++++++++++++++++++++++++++++++- qemu-options.hx | 18 +++++++++++++----- util/async-teardown.c | 21 --------------------- 4 files changed, 52 insertions(+), 27 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index ddc1e48460..02ea5a839f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -116,6 +116,11 @@ Use "whpx" (on Windows) or "hvf" (on macOS) instead. Use ``-run-with async-teardown=on`` instead. +``-chroot`` (since 8.1) +''''''''''''''''''''''' + +Use ``-run-with chroot=dir`` instead. + ``-singlestep`` (since 8.1) ''''''''''''''''''''''''''' diff --git a/os-posix.c b/os-posix.c index 90ea71725f..cfcb96533c 100644 --- a/os-posix.c +++ b/os-posix.c @@ -38,6 +38,7 @@ #include "qemu/cutils.h" #include "qemu/config-file.h" #include "qemu/option.h" +#include "qemu/module.h" #ifdef CONFIG_LINUX #include @@ -148,6 +149,7 @@ int os_parse_cmd_args(int index, const char *optarg) } break; case QEMU_OPTION_chroot: + warn_report("option is deprecated, use '-run-with chroot=...' instead"); chroot_dir = optarg; break; case QEMU_OPTION_daemonize: @@ -158,18 +160,25 @@ int os_parse_cmd_args(int index, const char *optarg) case QEMU_OPTION_asyncteardown: init_async_teardown(); break; +#endif case QEMU_OPTION_run_with: { + const char *str; QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("run-with"), optarg, false); if (!opts) { exit(1); } +#if defined(CONFIG_LINUX) if (qemu_opt_get_bool(opts, "async-teardown", false)) { init_async_teardown(); } +#endif + str = qemu_opt_get(opts, "chroot"); + if (str) { + chroot_dir = str; + } break; } -#endif default: return -1; } @@ -348,3 +357,27 @@ int os_mlock(void) return -ENOSYS; #endif } + +static QemuOptsList qemu_run_with_opts = { + .name = "run-with", + .head = QTAILQ_HEAD_INITIALIZER(qemu_run_with_opts.head), + .desc = { +#if defined(CONFIG_LINUX) + { + .name = "async-teardown", + .type = QEMU_OPT_BOOL, + }, +#endif + { + .name = "chroot", + .type = QEMU_OPT_STRING, + }, + { /* end of list */ } + }, +}; + +static void register_runwith(void) +{ + qemu_add_opts(&qemu_run_with_opts); +} +opts_init(register_runwith); diff --git a/qemu-options.hx b/qemu-options.hx index 96087505b2..f8f384e551 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4677,11 +4677,12 @@ ERST #ifndef _WIN32 DEF("chroot", HAS_ARG, QEMU_OPTION_chroot, \ - "-chroot dir chroot to dir just before starting the VM\n", + "-chroot dir chroot to dir just before starting the VM (deprecated)\n", QEMU_ARCH_ALL) #endif SRST ``-chroot dir`` + Deprecated, use '-run-with chroot=...' instead. Immediately before starting guest execution, chroot to the specified directory. Especially useful in combination with -runas. ERST @@ -4868,13 +4869,16 @@ SRST This option is deprecated and should no longer be used. The new option ``-run-with async-teardown=on`` is a replacement. ERST +#endif +#ifdef CONFIG_POSIX DEF("run-with", HAS_ARG, QEMU_OPTION_run_with, - "-run-with async-teardown[=on|off]\n" - " misc QEMU process lifecycle options\n" - " async-teardown=on enables asynchronous teardown\n", + "-run-with [async-teardown=on|off][,chroot=dir]\n" + " Set miscellaneous QEMU process lifecycle options:\n" + " async-teardown=on enables asynchronous teardown (Linux only)\n" + " chroot=dir chroot to dir just before starting the VM\n", QEMU_ARCH_ALL) SRST -``-run-with`` +``-run-with [async-teardown=on|off][,chroot=dir]`` Set QEMU process lifecycle options. ``async-teardown=on`` enables asynchronous teardown. A new process called @@ -4887,6 +4891,10 @@ SRST performed correctly. This only works if the cleanup process is not forcefully killed with SIGKILL before the main QEMU process has terminated completely. + + ``chroot=dir`` can be used for doing a chroot to the specified directory + immediately before starting the guest execution. This is especially useful + in combination with -runas. ERST #endif diff --git a/util/async-teardown.c b/util/async-teardown.c index 3ab19c8740..62cdeb0f20 100644 --- a/util/async-teardown.c +++ b/util/async-teardown.c @@ -12,9 +12,6 @@ */ #include "qemu/osdep.h" -#include "qemu/config-file.h" -#include "qemu/option.h" -#include "qemu/module.h" #include #include #include @@ -147,21 +144,3 @@ void init_async_teardown(void) clone(async_teardown_fn, new_stack_for_clone(), CLONE_VM, NULL); sigprocmask(SIG_SETMASK, &old_signals, NULL); } - -static QemuOptsList qemu_run_with_opts = { - .name = "run-with", - .head = QTAILQ_HEAD_INITIALIZER(qemu_run_with_opts.head), - .desc = { - { - .name = "async-teardown", - .type = QEMU_OPT_BOOL, - }, - { /* end of list */ } - }, -}; - -static void register_teardown(void) -{ - qemu_add_opts(&qemu_run_with_opts); -} -opts_init(register_teardown); From 6db77bb2c1aa1e59b4a04ba3c9b4b8bb17609d48 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 5 Jul 2023 15:36:39 +0200 Subject: [PATCH 18/21] meson.build: Skip C++ detection unless we're targeting Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only C++ code that we currently still have in the repository is the code in qga/vss-win32/ - so we can skip the C++ detection unless we are compiling binaries for Windows. Message-Id: <20230705133639.146073-1-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 58413d44a5..162e664c2b 100644 --- a/meson.build +++ b/meson.build @@ -20,7 +20,7 @@ config_host = keyval.load(meson.current_build_dir() / 'config-host.mak') cc = meson.get_compiler('c') all_languages = ['c'] -if add_languages('cpp', required: false, native: false) +if targetos == 'windows' and add_languages('cpp', required: false, native: false) all_languages += ['cpp'] cxx = meson.get_compiler('cpp') endif From cb2d7e63d19794c5344d438d943a8680d6756bdc Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 7 Jul 2023 17:42:21 +0200 Subject: [PATCH 19/21] tests/tcg/s390x: Fix test-svc with clang clang does not support expressions involving symbols in instructions like lghi yet, so building hello-s390x-asm.S with it fails. Move the expression to the literal pool and load it from there. Fixes: be4a4cb42961 ("tests/tcg/s390x: Test single-stepping SVC") Reported-by: Thomas Huth Signed-off-by: Ilya Leoshkevich Message-Id: <20230707154242.457706-1-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/s390x/gdbstub/test-svc.py | 2 +- tests/tcg/s390x/hello-s390x-asm.S | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/tcg/s390x/gdbstub/test-svc.py b/tests/tcg/s390x/gdbstub/test-svc.py index 7851ca7284..18fad3f163 100644 --- a/tests/tcg/s390x/gdbstub/test-svc.py +++ b/tests/tcg/s390x/gdbstub/test-svc.py @@ -25,7 +25,7 @@ def run_test(): gdb.execute("si") report("larl\t" in gdb.execute("x/i $pc", False, True), "insn #2") gdb.execute("si") - report("lghi\t" in gdb.execute("x/i $pc", False, True), "insn #3") + report("lgrl\t" in gdb.execute("x/i $pc", False, True), "insn #3") gdb.execute("si") report("svc\t" in gdb.execute("x/i $pc", False, True), "insn #4") gdb.execute("si") diff --git a/tests/tcg/s390x/hello-s390x-asm.S b/tests/tcg/s390x/hello-s390x-asm.S index 2e9faa1604..4dbda12d35 100644 --- a/tests/tcg/s390x/hello-s390x-asm.S +++ b/tests/tcg/s390x/hello-s390x-asm.S @@ -8,7 +8,7 @@ _start: /* puts("Hello, World!"); */ lghi %r2,1 larl %r3,foo -lghi %r4,foo_end-foo +lgrl %r4,foo_len svc 4 /* exit(0); */ @@ -18,3 +18,5 @@ svc 1 .align 2 foo: .asciz "Hello, World!\n" foo_end: +.align 8 +foo_len: .quad foo_end-foo From e02f56e3dee8dffc194e74275439f1975552da78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 1 Jun 2023 17:13:47 +0100 Subject: [PATCH 20/21] tests/qtest: massively speed up migration-test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The migration test cases that actually exercise live migration want to ensure there is a minimum of two iterations of pre-copy, in order to exercise the dirty tracking code. Historically we've queried the migration status, looking for the 'dirty-sync-count' value to increment to track iterations. This was not entirely reliable because often all the data would get transferred quickly enough that the migration would finish before we wanted it to. So we massively dropped the bandwidth and max downtime to guarantee non-convergance. This had the unfortunate side effect that every migration took at least 30 seconds to run (100 MB of dirty pages / 3 MB/sec). This optimization takes a different approach to ensuring that a mimimum of two iterations. Rather than waiting for dirty-sync-count to increment, directly look for an indication that the source VM has dirtied RAM that has already been transferred. On the source VM a magic marker is written just after the 3 MB offset. The destination VM is now montiored to detect when the magic marker is transferred. This gives a guarantee that the first 3 MB of memory have been transferred. Now the source VM memory is monitored at exactly the 3MB offset until we observe a flip in its value. This gives us a guaranteed that the guest workload has dirtied a byte that has already been transferred. Since we're looking at a place that is only 3 MB from the start of memory, with the 3 MB/sec bandwidth, this test should complete in 1 second, instead of 30 seconds. Once we've proved there is some dirty memory, migration can be set back to full speed for the remainder of the 1st iteration, and the entire of the second iteration at which point migration should be complete. On a test machine this further reduces the migration test time from 8 minutes to 1 minute 40. Signed-off-by: Daniel P. Berrangé Message-Id: <20230601161347.1803440-11-berrange@redhat.com> Signed-off-by: Thomas Huth --- tests/qtest/migration-test.c | 143 ++++++++++++++++++++++++++++++----- 1 file changed, 125 insertions(+), 18 deletions(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index b9cc194100..efa8c729db 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -46,6 +46,20 @@ static bool uffd_feature_thread_id; static bool got_src_stop; static bool got_dst_resume; +/* + * An initial 3 MB offset is used as that corresponds + * to ~1 sec of data transfer with our bandwidth setting. + */ +#define MAGIC_OFFSET_BASE (3 * 1024 * 1024) +/* + * A further 1k is added to ensure we're not a multiple + * of TEST_MEM_PAGE_SIZE, thus avoid clash with writes + * from the migration guest workload. + */ +#define MAGIC_OFFSET_SHUFFLE 1024 +#define MAGIC_OFFSET (MAGIC_OFFSET_BASE + MAGIC_OFFSET_SHUFFLE) +#define MAGIC_MARKER 0xFEED12345678CAFEULL + /* * Dirtylimit stop working if dirty page rate error * value less than DIRTYLIMIT_TOLERANCE_RANGE @@ -445,6 +459,91 @@ static void migrate_ensure_converge(QTestState *who) migrate_set_parameter_int(who, "downtime-limit", 30 * 1000); } +/* + * Our goal is to ensure that we run a single full migration + * iteration, and also dirty memory, ensuring that at least + * one further iteration is required. + * + * We can't directly synchronize with the start of a migration + * so we have to apply some tricks monitoring memory that is + * transferred. + * + * Initially we set the migration bandwidth to an insanely + * low value, with tiny max downtime too. This basically + * guarantees migration will never complete. + * + * This will result in a test that is unacceptably slow though, + * so we can't let the entire migration pass run at this speed. + * Our intent is to let it run just long enough that we can + * prove data prior to the marker has been transferred *AND* + * also prove this transferred data is dirty again. + * + * Before migration starts, we write a 64-bit magic marker + * into a fixed location in the src VM RAM. + * + * Then watch dst memory until the marker appears. This is + * proof that start_address -> MAGIC_OFFSET_BASE has been + * transferred. + * + * Finally we go back to the source and read a byte just + * before the marker untill we see it flip in value. This + * is proof that start_address -> MAGIC_OFFSET_BASE + * is now dirty again. + * + * IOW, we're guaranteed at least a 2nd migration pass + * at this point. + * + * We can now let migration run at full speed to finish + * the test + */ +static void migrate_prepare_for_dirty_mem(QTestState *from) +{ + /* + * The guest workflow iterates from start_address to + * end_address, writing 1 byte every TEST_MEM_PAGE_SIZE + * bytes. + * + * IOW, if we write to mem at a point which is NOT + * a multiple of TEST_MEM_PAGE_SIZE, our write won't + * conflict with the migration workflow. + * + * We put in a marker here, that we'll use to determine + * when the data has been transferred to the dst. + */ + qtest_writeq(from, start_address + MAGIC_OFFSET, MAGIC_MARKER); +} + +static void migrate_wait_for_dirty_mem(QTestState *from, + QTestState *to) +{ + uint64_t watch_address = start_address + MAGIC_OFFSET_BASE; + uint64_t marker_address = start_address + MAGIC_OFFSET; + uint8_t watch_byte; + + /* + * Wait for the MAGIC_MARKER to get transferred, as an + * indicator that a migration pass has made some known + * amount of progress. + */ + do { + usleep(1000 * 10); + } while (qtest_readq(to, marker_address) != MAGIC_MARKER); + + /* + * Now ensure that already transferred bytes are + * dirty again from the guest workload. Note the + * guest byte value will wrap around and by chance + * match the original watch_byte. This is harmless + * as we'll eventually see a different value if we + * keep watching + */ + watch_byte = qtest_readb(from, watch_address); + do { + usleep(1000 * 10); + } while (qtest_readb(from, watch_address) == watch_byte); +} + + static void migrate_pause(QTestState *who) { qtest_qmp_assert_success(who, "{ 'execute': 'migrate-pause' }"); @@ -577,7 +676,10 @@ typedef struct { MIG_TEST_FAIL_DEST_QUIT_ERR, } result; - /* Optional: set number of migration passes to wait for, if live==true */ + /* + * Optional: set number of migration passes to wait for, if live==true. + * If zero, then merely wait for a few MB of dirty data + */ unsigned int iterations; /* @@ -1165,12 +1267,14 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, migrate_ensure_non_converge(from); + migrate_prepare_for_dirty_mem(from); + /* Wait for the first serial output from the source */ wait_for_serial("src_serial"); migrate_qmp(from, uri, "{}"); - wait_for_migration_pass(from); + migrate_wait_for_dirty_mem(from, to); *from_ptr = from; *to_ptr = to; @@ -1405,14 +1509,8 @@ static void test_precopy_common(MigrateCommon *args) } if (args->live) { - /* - * Testing live migration, we want to ensure that some - * memory is re-dirtied after being transferred, so that - * we exercise logic for dirty page handling. We achieve - * this with a ridiculosly low bandwidth that guarantees - * non-convergance. - */ migrate_ensure_non_converge(from); + migrate_prepare_for_dirty_mem(from); } else { /* * Testing non-live migration, we allow it to run at @@ -1447,13 +1545,16 @@ static void test_precopy_common(MigrateCommon *args) } } else { if (args->live) { - if (args->iterations) { - while (args->iterations--) { - wait_for_migration_pass(from); - } - } else { + /* + * For initial iteration(s) we must do a full pass, + * but for the final iteration, we need only wait + * for some dirty mem before switching to converge + */ + while (args->iterations > 1) { wait_for_migration_pass(from); + args->iterations--; } + migrate_wait_for_dirty_mem(from, to); migrate_ensure_converge(from); @@ -1586,6 +1687,9 @@ static void test_ignore_shared(void) return; } + migrate_ensure_non_converge(from); + migrate_prepare_for_dirty_mem(from); + migrate_set_capability(from, "x-ignore-shared", true); migrate_set_capability(to, "x-ignore-shared", true); @@ -1594,7 +1698,7 @@ static void test_ignore_shared(void) migrate_qmp(from, uri, "{}"); - wait_for_migration_pass(from); + migrate_wait_for_dirty_mem(from, to); if (!got_src_stop) { qtest_qmp_eventwait(from, "STOP"); @@ -2325,6 +2429,7 @@ static void test_multifd_tcp_cancel(void) } migrate_ensure_non_converge(from); + migrate_prepare_for_dirty_mem(from); migrate_set_parameter_int(from, "multifd-channels", 16); migrate_set_parameter_int(to, "multifd-channels", 16); @@ -2343,7 +2448,7 @@ static void test_multifd_tcp_cancel(void) migrate_qmp(from, uri, "{}"); - wait_for_migration_pass(from); + migrate_wait_for_dirty_mem(from, to); migrate_cancel(from); @@ -2372,11 +2477,13 @@ static void test_multifd_tcp_cancel(void) wait_for_migration_status(from, "cancelled", NULL); - migrate_ensure_converge(from); + migrate_ensure_non_converge(from); migrate_qmp(from, uri, "{}"); - wait_for_migration_pass(from); + migrate_wait_for_dirty_mem(from, to); + + migrate_ensure_converge(from); if (!got_src_stop) { qtest_qmp_eventwait(from, "STOP"); From 7233bd122370155abfd75a42c86a9087ca5a8dbf Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 10 Jul 2023 11:26:38 +0200 Subject: [PATCH 21/21] docs/devel: Fix coding style in style.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As defined earlier in this file, the opening curly brace of functions should be placed on a separate line. So we should do it in the examples here, too. Fixes: 821f296756 ("docs: document use of automatic cleanup functions in glib") Reported-by: Konstantin Kostiuk Message-Id: <20230710092638.161625-1-thuth@redhat.com> Reviewed-by: Alex Bennée Reviewed-by: Konstantin Kostiuk Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Markus Armbruster Signed-off-by: Thomas Huth --- docs/devel/style.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/devel/style.rst b/docs/devel/style.rst index aa5e083ff8..3cfcdeb9cd 100644 --- a/docs/devel/style.rst +++ b/docs/devel/style.rst @@ -567,7 +567,8 @@ For example, instead of .. code-block:: c - int somefunc(void) { + int somefunc(void) + { int ret = -1; char *foo = g_strdup_printf("foo%", "wibble"); GList *bar = ..... @@ -588,7 +589,8 @@ Using g_autofree/g_autoptr enables the code to be written as: .. code-block:: c - int somefunc(void) { + int somefunc(void) + { g_autofree char *foo = g_strdup_printf("foo%", "wibble"); g_autoptr (GList) bar = ..... @@ -613,7 +615,8 @@ are still some caveats to beware of .. code-block:: c - char *somefunc(void) { + char *somefunc(void) + { g_autofree char *foo = g_strdup_printf("foo%", "wibble"); g_autoptr (GList) bar = .....