Re: [PATCH] hw/sd/sdhci: Do not force sdhci_mmio_*_ops onto all SD controllers
Am 16. Juli 2023 19:53:37 UTC schrieb Bernhard Beschow : > > >Am 10. Juli 2023 16:01:46 UTC schrieb Bernhard Beschow : >> >> >>Am 10. Juli 2023 10:16:35 UTC schrieb "Philippe Mathieu-Daudé" >>: >>>On 9/7/23 10:09, Bernhard Beschow wrote: Since commit c0a55a0c9da2 "hw/sd/sdhci: Support big endian SD host controller interfaces" sdhci_common_realize() forces all SD card controllers to use either sdhci_mmio_le_ops or sdhci_mmio_be_ops, depending on the "endianness" property. However, there are device models which use different MMIO ops: TYPE_IMX_USDHC uses usdhc_mmio_ops and TYPE_S3C_SDHCI uses sdhci_s3c_mmio_ops. Forcing sdhci_mmio_le_ops breaks SD card handling on the "sabrelite" board, for example. Fix this by defaulting the io_ops to little endian and switch to big endian in sdhci_common_realize() only if there is a matchig big endian variant available. Fixes: c0a55a0c9da2 ("hw/sd/sdhci: Support big endian SD host controller interfaces") Signed-off-by: Bernhard Beschow --- hw/sd/sdhci.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 6811f0f1a8..362c2c86aa 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -1382,6 +1382,8 @@ void sdhci_initfn(SDHCIState *s) s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s); s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s); + +s->io_ops = &sdhci_mmio_le_ops; } void sdhci_uninitfn(SDHCIState *s) @@ -1399,9 +1401,13 @@ void sdhci_common_realize(SDHCIState *s, Error **errp) >>> >>>What about simply keeping the same code guarded with 'if (!s->io_ops)'? >> >>I chose below approach since it provides an error message when one attempts >>to set one of the other device models to BE rather than just silently >>ignoring it. >> >>Also, I didn't want to make the assumption that `s->io_ops == NULL` implied >>that sdhci_mmio_*_ops is needed. That's similar material the bug fixed is >>made of, so I wanted to prevent that in the first place by being more >>explicit. >> >>In combination with the new error message the limitations of the current code >>become hopefully very apparent now, and at the same time should provide >>enough hints for adding BE support to the other device models if ever needed. >> >>Best regards, >>Bernhard > >Ping Ping^2 I would like to have the bug fixed in 8.1. Best regards, Bernhard > >+ qemu--stable > >> >>> switch (s->endianness) { case DEVICE_LITTLE_ENDIAN: -s->io_ops = &sdhci_mmio_le_ops; +/* s->io_ops is little endian by default */ break; case DEVICE_BIG_ENDIAN: +if (s->io_ops != &sdhci_mmio_le_ops) { +error_setg(errp, "SD controller doesn't support big endianness"); +return; +} s->io_ops = &sdhci_mmio_be_ops; break; default: >>>
2023-07-25 QEMU developers fortnightly conference call for agenda
Hi If you have topics for this week call, please let's us now. Later, Juan. QEMU developers fortnightly conference call Tuesday 2023-07-25 ⋅ 15:00 – 16:00 Central European Time - Madrid Location https://meet.jit.si/kvmcallmeeting https://www.google.com/url?q=https%3A%2F%2Fmeet.jit.si%2Fkvmcallmeeting&sa=D&ust=169061526000&usg=AOvVaw2OTX-HHT_zO5VsRecJBnej If you need call details, please contact me: quint...@redhat.com Guests Philippe Mathieu-Daudé Joao Martins quint...@redhat.com Meirav Dean Felipe Franciosi afaer...@suse.de bazu...@redhat.com bbau...@redhat.com c...@f00f.org dustin.kirkl...@canonical.com ebl...@redhat.com edgar.igles...@gmail.com eric.au...@redhat.com i...@theiggy.com jan.kis...@web.de jidong.x...@gmail.com jjhe...@linux.vnet.ibm.com m...@linux.vnet.ibm.com Peter Maydell richard.hender...@linaro.org stefa...@gmail.com Warner Losh z@139.com zwu.ker...@gmail.com Jason Gunthorpe Neo Jia David Edmondson Elena Ufimtseva Konrad Wilk a...@rev.ng a...@rev.ng Shameerali Kolothum Thodi Wang, Wei W Chao Peng qemu-devel@nongnu.org mbur...@qti.qualcomm.com
Re: [PATCH 0/6] trivial-patches for 2023-07-16
On 16/07/2023 14.05, Michael Tokarev wrote: 16.07.2023 14:57, Michael Tokarev пишет: The following changes since commit 7d07a21ec003724475566073404c5893e36de5e5: tree-wide spelling fixes in comments and some messages: hw/9pfs (2023-07-16 13:59:17 +0300) are available in the Git repository at: https://gitlab.com/mjt0k/qemu.git/ tags/trivial-patches-pull for you to fetch changes up to 7d07a21ec003724475566073404c5893e36de5e5: tree-wide spelling fixes in comments and some messages: hw/9pfs (2023-07-16 13:59:17 +0300) trivial-patches for 2023-07-16 This contains a doc fix for riscv and reviewed spelling fixes. This meant to be a PULL request, not PATCH series.. :) I guess you have to resend with a PULL in the subject? Thomas
[PATCH] vhost: fix the fd leak
When the vhost-user reconnect to the backend, the notifer should be cleanup. Otherwise, the fd resource will be exhausted. Fixes: f9a09ca3ea ("vhost: add support for configure interrupt") Signed-off-by: Li Feng --- hw/virtio/vhost.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index abf0d03c8d..e2f6ffb446 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -2044,6 +2044,8 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings) event_notifier_test_and_clear( &hdev->vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier); event_notifier_test_and_clear(&vdev->config_notifier); +event_notifier_cleanup( +&hdev->vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier); trace_vhost_dev_stop(hdev, vdev->name, vrings); -- 2.41.0
Re: [PATCH] Allow UNIX socket for VNC websocket
Hi Daniel, in commit 275e0d616b ("ui: refactor code for populating SocketAddress from vnc_display_open"), you said "This refactoring also removes the restriction that prevents enabling websockets when the plain VNC server is listening on a UNIX socket.". But you didn't remove the condition. I suppose it was a left-over? On Mon, Jul 24, 2023 at 1:59 AM Sergii Zasenko wrote: > Signed-off-by: Sergii Zasenko > lgtm Reviewed-by: Marc-André Lureau > --- > ui/vnc.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/ui/vnc.c b/ui/vnc.c > index 92964dc..dea1414 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char > *addrstr, > addr->type = SOCKET_ADDRESS_TYPE_UNIX; > addr->u.q_unix.path = g_strdup(addrstr + 5); > > -if (websocket) { > -error_setg(errp, "UNIX sockets not supported with websock"); > -goto cleanup; > -} > - > if (to) { > error_setg(errp, "Port range not support with UNIX socket"); > goto cleanup; > -- > 2.39.2 > > > -- Marc-André Lureau
[PATCH v4 07/14] target/s390x: Fix assertion failure in VFMIN/VFMAX with type 13
Type 13 is reserved, so using it should result in specification exception. Due to an off-by-1 error the code triggers an assertion at a later point in time instead. Cc: qemu-sta...@nongnu.org Fixes: da4807527f3b ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)") Reviewed-by: David Hildenbrand Reviewed-by: Richard Henderson Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/translate_vx.c.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/tcg/translate_vx.c.inc b/target/s390x/tcg/translate_vx.c.inc index 43dfbfd03f6..f8df121d3d3 100644 --- a/target/s390x/tcg/translate_vx.c.inc +++ b/target/s390x/tcg/translate_vx.c.inc @@ -3047,7 +3047,7 @@ static DisasJumpType op_vfmax(DisasContext *s, DisasOps *o) const uint8_t m5 = get_field(s, m5); gen_helper_gvec_3_ptr *fn; -if (m6 == 5 || m6 == 6 || m6 == 7 || m6 > 13) { +if (m6 == 5 || m6 == 6 || m6 == 7 || m6 >= 13) { gen_program_exception(s, PGM_SPECIFICATION); return DISAS_NORETURN; } -- 2.41.0
[PATCH v4 06/14] tcg/{i386, s390x}: Add earlyclobber to the op_add2's first output
i386 and s390x implementations of op_add2 require an earlyclobber, which is currently missing. This breaks VCKSM in s390x guests. E.g., on x86_64 the following op: add2_i32 tmp2,tmp3,tmp2,tmp3,tmp3,tmp2 dead: 0 2 3 4 5 pref=none,0x is translated to: addl %ebx, %r12d adcl %r12d, %ebx Introduce a new C_N1_O1_I4 constraint, and make sure that earlyclobber of aliased outputs is honored. Cc: qemu-sta...@nongnu.org Fixes: 82790a870992 ("tcg: Add markup for output requires new register") Reviewed-by: Richard Henderson Signed-off-by: Ilya Leoshkevich --- tcg/i386/tcg-target-con-set.h | 5 - tcg/i386/tcg-target.c.inc | 2 +- tcg/s390x/tcg-target-con-set.h | 8 +--- tcg/s390x/tcg-target.c.inc | 4 ++-- tcg/tcg.c | 8 +++- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/tcg/i386/tcg-target-con-set.h b/tcg/i386/tcg-target-con-set.h index 91ceb0e1da2..5ea3a292f0f 100644 --- a/tcg/i386/tcg-target-con-set.h +++ b/tcg/i386/tcg-target-con-set.h @@ -11,6 +11,9 @@ * * C_N1_Im(...) defines a constraint set with 1 output and inputs, * except that the output must use a new register. + * + * C_Nn_Om_Ik(...) defines a constraint set with outputs and + * inputs, except that the first outputs must use new registers. */ C_O0_I1(r) C_O0_I2(L, L) @@ -53,4 +56,4 @@ C_O2_I1(r, r, L) C_O2_I2(a, d, a, r) C_O2_I2(r, r, L, L) C_O2_I3(a, d, 0, 1, r) -C_O2_I4(r, r, 0, 1, re, re) +C_N1_O1_I4(r, r, 0, 1, re, re) diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc index ab997b5fb39..77482da0709 100644 --- a/tcg/i386/tcg-target.c.inc +++ b/tcg/i386/tcg-target.c.inc @@ -3335,7 +3335,7 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode op) case INDEX_op_add2_i64: case INDEX_op_sub2_i32: case INDEX_op_sub2_i64: -return C_O2_I4(r, r, 0, 1, re, re); +return C_N1_O1_I4(r, r, 0, 1, re, re); case INDEX_op_ctz_i32: case INDEX_op_ctz_i64: diff --git a/tcg/s390x/tcg-target-con-set.h b/tcg/s390x/tcg-target-con-set.h index cbad91b2b56..9a420374999 100644 --- a/tcg/s390x/tcg-target-con-set.h +++ b/tcg/s390x/tcg-target-con-set.h @@ -8,6 +8,9 @@ * C_On_Im(...) defines a constraint set with outputs and inputs. * Each operand should be a sequence of constraint letters as defined by * tcg-target-con-str.h; the constraint combination is inclusive or. + * + * C_Nn_Om_Ik(...) defines a constraint set with outputs and + * inputs, except that the first outputs must use new registers. */ C_O0_I1(r) C_O0_I2(r, r) @@ -41,6 +44,5 @@ C_O2_I1(o, m, r) C_O2_I2(o, m, 0, r) C_O2_I2(o, m, r, r) C_O2_I3(o, m, 0, 1, r) -C_O2_I4(r, r, 0, 1, rA, r) -C_O2_I4(r, r, 0, 1, ri, r) -C_O2_I4(r, r, 0, 1, r, r) +C_N1_O1_I4(r, r, 0, 1, ri, r) +C_N1_O1_I4(r, r, 0, 1, rA, r) diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc index a878acd8ca6..a94f7908d64 100644 --- a/tcg/s390x/tcg-target.c.inc +++ b/tcg/s390x/tcg-target.c.inc @@ -3229,11 +3229,11 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode op) case INDEX_op_add2_i32: case INDEX_op_sub2_i32: -return C_O2_I4(r, r, 0, 1, ri, r); +return C_N1_O1_I4(r, r, 0, 1, ri, r); case INDEX_op_add2_i64: case INDEX_op_sub2_i64: -return C_O2_I4(r, r, 0, 1, rA, r); +return C_N1_O1_I4(r, r, 0, 1, rA, r); case INDEX_op_st_vec: return C_O0_I2(v, r); diff --git a/tcg/tcg.c b/tcg/tcg.c index 652e8ea6b93..ddfe9a96cb7 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -648,6 +648,7 @@ static void tcg_out_movext3(TCGContext *s, const TCGMovExtend *i1, #define C_O2_I2(O1, O2, I1, I2) C_PFX4(c_o2_i2_, O1, O2, I1, I2), #define C_O2_I3(O1, O2, I1, I2, I3) C_PFX5(c_o2_i3_, O1, O2, I1, I2, I3), #define C_O2_I4(O1, O2, I1, I2, I3, I4) C_PFX6(c_o2_i4_, O1, O2, I1, I2, I3, I4), +#define C_N1_O1_I4(O1, O2, I1, I2, I3, I4) C_PFX6(c_n1_o1_i4_, O1, O2, I1, I2, I3, I4), typedef enum { #include "tcg-target-con-set.h" @@ -668,6 +669,7 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode); #undef C_O2_I2 #undef C_O2_I3 #undef C_O2_I4 +#undef C_N1_O1_I4 /* Put all of the constraint sets into an array, indexed by the enum. */ @@ -687,6 +689,7 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode); #define C_O2_I2(O1, O2, I1, I2) { .args_ct_str = { #O1, #O2, #I1, #I2 } }, #define C_O2_I3(O1, O2, I1, I2, I3) { .args_ct_str = { #O1, #O2, #I1, #I2, #I3 } }, #define C_O2_I4(O1, O2, I1, I2, I3, I4) { .args_ct_str = { #O1, #O2, #I1, #I2, #I3, #I4 } }, +#define C_N1_O1_I4(O1, O2, I1, I2, I3, I4) { .args_ct_str = { "&" #O1, #O2, #I1, #I2, #I3, #I4 } }, static const TCGTargetOpDef constraint_sets[] = { #include "tcg-target-con-set.h" @@ -706,6 +709,7 @@ static const TCGTargetOpDef constraint_sets[] = { #undef C_O2_I2 #undef C_O2_I3 #undef C_O2_I4 +#undef C_N1_O1_I4 /* Expand the enumerator to be returned from tcg_target_op_def(). */ @@ -
[PATCH v4 14/14] tests/tcg/s390x: Test VCKSM
Add a small test to prevent regressions. Tested-by: Thomas Huth Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/vcksm.c | 31 +++ tests/tcg/s390x/vx.h| 2 ++ 3 files changed, 34 insertions(+) create mode 100644 tests/tcg/s390x/vcksm.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 71bf39b78d3..1fc98099070 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -58,6 +58,7 @@ TESTS += $(PGM_SPECIFICATION_TESTS) Z13_TESTS=vistr Z13_TESTS+=lcbb Z13_TESTS+=locfhr +Z13_TESTS+=vcksm $(Z13_TESTS): CFLAGS+=-march=z13 -O2 TESTS+=$(Z13_TESTS) diff --git a/tests/tcg/s390x/vcksm.c b/tests/tcg/s390x/vcksm.c new file mode 100644 index 000..452daaae6ce --- /dev/null +++ b/tests/tcg/s390x/vcksm.c @@ -0,0 +1,31 @@ +/* + * Test the VCKSM instruction. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include +#include +#include +#include "vx.h" + +int main(void) +{ +S390Vector v1; +S390Vector v2 = { +.d[0] = 0xb2261c8140edce49ULL, +.d[1] = 0x387bf5a433af39d1ULL, +}; +S390Vector v3 = { +.d[0] = 0x73b03d2c7f9e654eULL, +.d[1] = 0x23d74e51fb479877ULL, +}; +S390Vector exp = {.d[0] = 0xdedd7f8eULL, .d[1] = 0ULL}; + +asm volatile("vcksm %[v1],%[v2],%[v3]" + : [v1] "=v" (v1.v) + : [v2] "v" (v2.v) + , [v3] "v" (v3.v)); +assert(memcmp(&v1, &exp, sizeof(v1)) == 0); + +return EXIT_SUCCESS; +} diff --git a/tests/tcg/s390x/vx.h b/tests/tcg/s390x/vx.h index 02e7fd518a8..00701dbe35f 100644 --- a/tests/tcg/s390x/vx.h +++ b/tests/tcg/s390x/vx.h @@ -1,6 +1,8 @@ #ifndef QEMU_TESTS_S390X_VX_H #define QEMU_TESTS_S390X_VX_H +#include + typedef union S390Vector { uint64_t d[2]; /* doubleword */ uint32_t w[4]; /* word */ -- 2.41.0
[PATCH v4 09/14] tests/tcg/s390x: Test CLGEBR and CGEBRA
Add a small test to prevent regressions. Tested-by: Thomas Huth Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.target | 5 + tests/tcg/s390x/cgebra.c| 32 tests/tcg/s390x/clgebr.c| 32 3 files changed, 69 insertions(+) create mode 100644 tests/tcg/s390x/cgebra.c create mode 100644 tests/tcg/s390x/clgebr.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 19fbbc6e531..71bf39b78d3 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -39,12 +39,17 @@ TESTS+=mxdb TESTS+=epsw TESTS+=larl TESTS+=mdeb +TESTS+=cgebra +TESTS+=clgebr cdsg: CFLAGS+=-pthread cdsg: LDFLAGS+=-pthread rxsbg: CFLAGS+=-O2 +cgebra: LDFLAGS+=-lm +clgebr: LDFLAGS+=-lm + include $(S390X_SRC)/pgm-specification.mak $(PGM_SPECIFICATION_TESTS): pgm-specification-user.o $(PGM_SPECIFICATION_TESTS): LDFLAGS+=pgm-specification-user.o diff --git a/tests/tcg/s390x/cgebra.c b/tests/tcg/s390x/cgebra.c new file mode 100644 index 000..f91e10d2d3c --- /dev/null +++ b/tests/tcg/s390x/cgebra.c @@ -0,0 +1,32 @@ +/* + * Test the CGEBRA instruction. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include +#include +#include + +int main(void) +{ +float r2 = 1E+300; +long long r1; +int cc; + +feclearexcept(FE_ALL_EXCEPT); +asm("cgebra %[r1],%[m3],%[r2],%[m4]\n" +"ipm %[cc]\n" +: [r1] "=r" (r1) +, [cc] "=r" (cc) +: [m3] "i" (5) /* round toward 0 */ +, [r2] "f" (r2) +, [m4] "i" (8) /* bit 0 is set, but must be ignored; XxC is not set */ +: "cc"); +cc >>= 28; + +assert(r1 == 0x7fffLL); +assert(cc == 3); +assert(fetestexcept(FE_ALL_EXCEPT) == (FE_INVALID | FE_INEXACT)); + +return EXIT_SUCCESS; +} diff --git a/tests/tcg/s390x/clgebr.c b/tests/tcg/s390x/clgebr.c new file mode 100644 index 000..d491899b56e --- /dev/null +++ b/tests/tcg/s390x/clgebr.c @@ -0,0 +1,32 @@ +/* + * Test the CLGEBR instruction. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include +#include +#include + +int main(void) +{ +float r2 = -1; +long long r1; +int cc; + +feclearexcept(FE_ALL_EXCEPT); +asm("clgebr %[r1],%[m3],%[r2],%[m4]\n" +"ipm %[cc]\n" +: [r1] "=r" (r1) +, [cc] "=r" (cc) +: [m3] "i" (5) /* round toward 0 */ +, [r2] "f" (r2) +, [m4] "i" (8) /* bit 0 is set, but must be ignored; XxC is not set */ +: "cc"); +cc >>= 28; + +assert(r1 == 0); +assert(cc == 3); +assert(fetestexcept(FE_ALL_EXCEPT) == (FE_INVALID | FE_INEXACT)); + +return EXIT_SUCCESS; +} -- 2.41.0
Re: assert fails in s390x TCG
.. adding Alex, maybe something related to multithreaded TCG? On 7/21/23 11:08, Claudio Fontana wrote: > > Hello Cornelia, Richard, > > I had some strange behavior in an s390x TCG VM that I am debugging, > > and configured latest upstream QEMU with --enable-debug --enable-debug-tcg > > and I am running the qemu binary with -d unimp,guest_errors . > > I get: > > /usr/bin/qemu-system-s390x -nodefaults -no-reboot -nographic -vga none -cpu > qemu -d unimp,guest_errors -object rng-random,filename=/dev/random,id=rng0 > -device virtio-rng-ccw,rng=rng0 -runas qemu -net none -kernel > /var/tmp/boot/kernel -initrd /var/tmp/boot/initrd -append > root=/dev/disk/by-id/virtio-0 rootfstype=ext3 > rootflags=data=writeback,nobarrier,commit=150,noatime elevator=noop > nmi_watchdog=0 rw oops=panic panic=1 quiet elevator=noop console=hvc0 > init=build -m 2048 -drive > file=/var/tmp/img,format=raw,if=none,id=disk,cache=unsafe -device > virtio-blk-ccw,drive=disk,serial=0 -drive > file=/var/tmp/swap,format=raw,if=none,id=swap,cache=unsafe -device > virtio-blk-ccw,drive=swap,serial=1 -device virtio-serial-ccw -device > virtconsole,chardev=virtiocon0 -chardev stdio,id=virtiocon0 -chardev > socket,id=monitor,server=on,wait=off,path=/var/tmp/img.qemu/monitor -mon > chardev=monitor,mode=readline -smp 8 > > unimplemented opcode 0xb9ab > unimplemented opcode 0xb2af > > ERROR:../accel/tcg/tb-maint.c:348:page_unlock__debug: assertion failed: > (page_is_locked(pd)) > Bail out! ERROR:../accel/tcg/tb-maint.c:348:page_unlock__debug: assertion > failed: (page_is_locked(pd)) > > Thread 3 "qemu-system-s39" received signal SIGABRT, Aborted. > [Switching to Thread 0x753516c0 (LWP 215975)] > (gdb) bt > #0 0x7730dabc in __pthread_kill_implementation () at /lib64/libc.so.6 > #1 0x772bc266 in raise () at /lib64/libc.so.6 > #2 0x772a4897 in abort () at /lib64/libc.so.6 > #3 0x776f0eee in () at /lib64/libglib-2.0.so.0 > #4 0x7775649a in g_assertion_message_expr () at > /lib64/libglib-2.0.so.0 > #5 0x55b96134 in page_unlock__debug (pd=0x7ffee8680440) at > ../accel/tcg/tb-maint.c:348 > #6 0x55b962a9 in page_unlock (pd=0x7ffee8680440) at > ../accel/tcg/tb-maint.c:397 > #7 0x55b96580 in tb_unlock_pages (tb=0x7fffefffeb00) at > ../accel/tcg/tb-maint.c:483 > #8 0x55b94698 in cpu_exec_longjmp_cleanup (cpu=0x56566a30) at > ../accel/tcg/cpu-exec.c:556 > #9 0x55b954e0 in cpu_exec_setjmp (cpu=0x56566a30, > sc=0x75350540) at ../accel/tcg/cpu-exec.c:1054 > #10 0x55b9557a in cpu_exec (cpu=0x56566a30) at > ../accel/tcg/cpu-exec.c:1083 > #11 0x55bb9af6 in tcg_cpus_exec (cpu=0x56566a30) at > ../accel/tcg/tcg-accel-ops.c:75 > #12 0x55bba1ae in mttcg_cpu_thread_fn (arg=0x56566a30) at > ../accel/tcg/tcg-accel-ops-mttcg.c:95 > #13 0x55dc0af3 in qemu_thread_start (args=0x565ba150) at > ../util/qemu-thread-posix.c:541 > #14 0x7730bc64 in start_thread () at /lib64/libc.so.6 > #15 0x77393550 in clone3 () at /lib64/libc.so.6 > > (gdb) frame 5 > #5 0x55b96134 in page_unlock__debug (pd=0x7ffee8680440) at > ../accel/tcg/tb-maint.c:348 > 348 g_assert(page_is_locked(pd)); > (gdb) list 348 > 343 static void page_unlock__debug(const PageDesc *pd) > 344 { > 345 bool removed; > 346 > 347 ht_pages_locked_debug_init(); > 348 g_assert(page_is_locked(pd)); > 349 removed = g_hash_table_remove(ht_pages_locked_debug, pd); > 350 g_assert(removed); > 351 } > 352 > > (gdb) info threads > Id Target IdFrame > 1Thread 0x763bef40 (LWP 215971) "qemu-system-s39" > 0x77385596 in ppoll () from /lib64/libc.so.6 > 2Thread 0x763bb6c0 (LWP 215974) "qemu-system-s39" > 0x7738b41d in syscall () from /lib64/libc.so.6 > * 3Thread 0x753516c0 (LWP 215975) "qemu-system-s39" > 0x7730dabc in __pthread_kill_implementation () from /lib64/libc.so.6 > 4Thread 0x74b506c0 (LWP 215976) "qemu-system-s39" > 0x7730820e in __futex_abstimed_wait_common () from /lib64/libc.so.6 > 5Thread 0x7ffeefdff6c0 (LWP 215977) "qemu-system-s39" > 0x7730820e in __futex_abstimed_wait_common () from /lib64/libc.so.6 > 6Thread 0x7ffeef5fe6c0 (LWP 215978) "qemu-system-s39" > 0x7730820e in __futex_abstimed_wait_common () from /lib64/libc.so.6 > 7Thread 0x7ffeeedfd6c0 (LWP 215979) "qemu-system-s39" > 0x7730820e in __futex_abstimed_wait_common () from /lib64/libc.so.6 > 8Thread 0x7ffeee5fc6c0 (LWP 215980) "qemu-system-s39" > 0x7730820e in __futex_abstimed_wait_common () from /lib64/libc.so.6 > 9Thread 0x7ffeeddfb6c0 (LWP 215981) "qemu-system-s39" > 0x7730820e in __futex_abstimed_wait_common () from /lib64/libc.so.6 > 10 Thread 0x7ffeed5fa6c0 (LWP 215982) "qemu-system-s39" > 0x0
[PATCH v4 01/14] target/s390x: Make CKSM raise an exception if R2 is odd
R2 designates an even-odd register pair; the instruction should raise a specification exception when R2 is not even. Cc: qemu-sta...@nongnu.org Fixes: e023e832d0ac ("s390x: translate engine for s390x CPU") Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/insn-data.h.inc | 2 +- target/s390x/tcg/translate.c | 6 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc index 457ed25d2fa..86a509b0ac8 100644 --- a/target/s390x/tcg/insn-data.h.inc +++ b/target/s390x/tcg/insn-data.h.inc @@ -157,7 +157,7 @@ C(0xb2fa, NIAI,E, EH, 0, 0, 0, 0, 0, 0) /* CHECKSUM */ -C(0xb241, CKSM,RRE, Z, r1_o, ra2, new, r1_32, cksm, 0) +C(0xb241, CKSM,RRE, Z, r1_o, ra2_E, new, r1_32, cksm, 0) /* COPY SIGN */ F(0xb372, CPSDR, RRF_b, FPSSH, f3, f2, new, f1, cps, 0, IF_AFP1 | IF_AFP2 | IF_AFP3) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index 6661b27efa4..d6e8acee995 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -5779,6 +5779,12 @@ static void in2_ra2(DisasContext *s, DisasOps *o) } #define SPEC_in2_ra2 0 +static void in2_ra2_E(DisasContext *s, DisasOps *o) +{ +return in2_ra2(s, o); +} +#define SPEC_in2_ra2_E SPEC_r2_even + static void in2_a2(DisasContext *s, DisasOps *o) { int x2 = have_field(s, x2) ? get_field(s, x2) : 0; -- 2.41.0
[PATCH v4 05/14] target/s390x: Make MC raise specification exception when class >= 16
MC requires bit positions 8-11 (upper 4 bits of class) to be zeros, otherwise it must raise a specification exception. Cc: qemu-sta...@nongnu.org Fixes: 20d143e2cab8 ("s390x/tcg: Implement MONITOR CALL") Reviewed-by: David Hildenbrand Reviewed-by: Richard Henderson Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/excp_helper.c | 2 +- target/s390x/tcg/translate.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c index 228aa9f2373..3da337f7c72 100644 --- a/target/s390x/tcg/excp_helper.c +++ b/target/s390x/tcg/excp_helper.c @@ -639,7 +639,7 @@ void monitor_event(CPUS390XState *env, void HELPER(monitor_call)(CPUS390XState *env, uint64_t monitor_code, uint32_t monitor_class) { -g_assert(monitor_class <= 0xff); +g_assert(monitor_class <= 0xf); if (env->cregs[8] & (0x8000 >> monitor_class)) { monitor_event(env, monitor_code, monitor_class, GETPC()); diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index 244e61ad2eb..84d76f1cea1 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -3177,9 +3177,9 @@ static DisasJumpType op_lcbb(DisasContext *s, DisasOps *o) static DisasJumpType op_mc(DisasContext *s, DisasOps *o) { -const uint16_t monitor_class = get_field(s, i2); +const uint8_t monitor_class = get_field(s, i2); -if (monitor_class & 0xff00) { +if (monitor_class & 0xf0) { gen_program_exception(s, PGM_SPECIFICATION); return DISAS_NORETURN; } -- 2.41.0
[PATCH v4 13/14] tests/tcg/s390x: Test STPQ
Add a small test to prevent regressions. Tested-by: Thomas Huth Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/stpq.S | 20 2 files changed, 21 insertions(+) create mode 100644 tests/tcg/s390x/stpq.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index 145e0bfde16..76345b6e643 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -27,6 +27,7 @@ ASM_TESTS = \ mc \ ssm-early \ stosm-early \ +stpq \ unaligned-lowcore include $(S390X_SRC)/pgm-specification.mak diff --git a/tests/tcg/s390x/stpq.S b/tests/tcg/s390x/stpq.S new file mode 100644 index 000..687a52eafa7 --- /dev/null +++ b/tests/tcg/s390x/stpq.S @@ -0,0 +1,20 @@ +.org 0x200 /* lowcore padding */ +.globl _start +_start: +lgrl %r0,value +lgrl %r1,value+8 +stpq %r0,stored_value +clc stored_value(16),value +jne failure +lpswe success_psw +failure: +lpswe failure_psw +.align 16 +value: +.quad 0x1234567887654321, 0x8765432112345678 +stored_value: +.quad 0, 0 +success_psw: +.quad 0x2,0xfff/* see is_special_wait_psw() */ +failure_psw: +.quad 0x2,0/* disabled wait */ -- 2.41.0
[PATCH v4 10/14] tests/tcg/s390x: Test CLM
Add a small test to prevent regressions. Tested-by: Thomas Huth Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/clm.S | 29 + 2 files changed, 30 insertions(+) create mode 100644 tests/tcg/s390x/clm.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index e813e318db9..062d8e368aa 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -17,6 +17,7 @@ LDFLAGS=-nostdlib -static ASM_TESTS = \ bal \ cksm \ +clm \ exrl-ssm-early \ sam \ lpsw \ diff --git a/tests/tcg/s390x/clm.S b/tests/tcg/s390x/clm.S new file mode 100644 index 000..17156a81f2a --- /dev/null +++ b/tests/tcg/s390x/clm.S @@ -0,0 +1,29 @@ +.org 0x8e +program_interruption_code: +.org 0x1d0 /* program new PSW */ +.quad 0,pgm +.org 0x200 /* lowcore padding */ +.globl _start +_start: +lgrl %r0,op1 +clm %r0,6,op2 +jle failure +lgrl %r1,bad_addr +clm %r0,0,0(%r1) +failure: +lpswe failure_psw +pgm: +chhsi program_interruption_code,5 /* addressing exception? */ +jne failure +lpswe success_psw +.align 8 +op1: +.quad 0x1234567887654321 +op2: +.quad 0x3456789abcdef012 +bad_addr: +.quad 0x +success_psw: +.quad 0x2,0xfff/* see is_special_wait_psw() */ +failure_psw: +.quad 0x2,0/* disabled wait */ -- 2.41.0
[PATCH v4 04/14] target/s390x: Fix ICM with M3=0
When the mask is zero, access exceptions should still be recognized for 1 byte at the second-operand address. CC should be set to 0. Cc: qemu-sta...@nongnu.org Fixes: e023e832d0ac ("s390x: translate engine for s390x CPU") Reviewed-by: David Hildenbrand Reviewed-by: Richard Henderson Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/translate.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index d6e8acee995..244e61ad2eb 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -2515,6 +2515,12 @@ static DisasJumpType op_icm(DisasContext *s, DisasOps *o) ccm = ((1ull << len) - 1) << pos; break; +case 0: +/* Recognize access exceptions for the first byte. */ +tcg_gen_qemu_ld_i64(tmp, o->in2, get_mem_index(s), MO_UB); +gen_op_movi_cc(s, 0); +return DISAS_NEXT; + default: /* This is going to be a sequence of loads and inserts. */ pos = base + 32 - 8; -- 2.41.0
[PATCH v4 00/14] target/s390x: Miscellaneous TCG fixes, part 2
v3: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg03902.html v3 -> v4: Introduce in2_ra2_E (Richard). Use probe_read (Richard). Add R-bs. Patches needing review: [01/14], [08/14]-[14/14]. v2: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg03762.html v2 -> v3: Document the new constraint set (Philippe). Fix clang build (Thomas). Add T-bs. v1: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg03648.html v1 -> v2: Move the case in 04/14 (David). Simplify the reserved type checking in 07/14 (David). Add R-bs. Hi, Here is another set of fixes for issues found by randomized testing. Most of them have to do with simple insufficient error handling or corner cases, but 3/14 and 6/14 took a while to figure out, and hopefully I got the fixes right. 13/14 is a test for an issue that Richard has already fixed, but I thought it would be helpful to have it anyway. Best regards, Ilya Ilya Leoshkevich (14): target/s390x: Make CKSM raise an exception if R2 is odd target/s390x: Fix CLM with M3=0 target/s390x: Fix CONVERT TO LOGICAL/FIXED with out-of-range inputs target/s390x: Fix ICM with M3=0 target/s390x: Make MC raise specification exception when class >= 16 tcg/{i386,s390x}: Add earlyclobber to the op_add2's first output target/s390x: Fix assertion failure in VFMIN/VFMAX with type 13 tests/tcg/s390x: Test CKSM tests/tcg/s390x: Test CLGEBR and CGEBRA tests/tcg/s390x: Test CLM tests/tcg/s390x: Test ICM tests/tcg/s390x: Test MC tests/tcg/s390x: Test STPQ tests/tcg/s390x: Test VCKSM target/s390x/tcg/excp_helper.c | 2 +- target/s390x/tcg/fpu_helper.c | 3 +- target/s390x/tcg/insn-data.h.inc| 2 +- target/s390x/tcg/mem_helper.c | 5 +++ target/s390x/tcg/translate.c| 16 ++- target/s390x/tcg/translate_vx.c.inc | 2 +- tcg/i386/tcg-target-con-set.h | 5 ++- tcg/i386/tcg-target.c.inc | 2 +- tcg/s390x/tcg-target-con-set.h | 8 ++-- tcg/s390x/tcg-target.c.inc | 4 +- tcg/tcg.c | 8 +++- tests/tcg/s390x/Makefile.softmmu-target | 5 +++ tests/tcg/s390x/Makefile.target | 6 +++ tests/tcg/s390x/cgebra.c| 32 ++ tests/tcg/s390x/cksm.S | 29 + tests/tcg/s390x/clgebr.c| 32 ++ tests/tcg/s390x/clm.S | 29 + tests/tcg/s390x/icm.S | 32 ++ tests/tcg/s390x/mc.S| 56 + tests/tcg/s390x/stpq.S | 20 + tests/tcg/s390x/vcksm.c | 31 ++ tests/tcg/s390x/vx.h| 2 + 22 files changed, 317 insertions(+), 14 deletions(-) create mode 100644 tests/tcg/s390x/cgebra.c create mode 100644 tests/tcg/s390x/cksm.S create mode 100644 tests/tcg/s390x/clgebr.c create mode 100644 tests/tcg/s390x/clm.S create mode 100644 tests/tcg/s390x/icm.S create mode 100644 tests/tcg/s390x/mc.S create mode 100644 tests/tcg/s390x/stpq.S create mode 100644 tests/tcg/s390x/vcksm.c -- 2.41.0
[PATCH v4 12/14] tests/tcg/s390x: Test MC
Add a small test to prevent regressions. Tested-by: Thomas Huth Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/mc.S| 56 + 2 files changed, 57 insertions(+) create mode 100644 tests/tcg/s390x/mc.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index 58684d7da71..145e0bfde16 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -24,6 +24,7 @@ ASM_TESTS = \ lpsw \ lpswe-early \ lra \ +mc \ ssm-early \ stosm-early \ unaligned-lowcore diff --git a/tests/tcg/s390x/mc.S b/tests/tcg/s390x/mc.S new file mode 100644 index 000..e7466bb4b57 --- /dev/null +++ b/tests/tcg/s390x/mc.S @@ -0,0 +1,56 @@ +.org 0x8d +ilc: +.org 0x8e +program_interruption_code: +.org 0x94 +monitor_class: +.org 0xb0 +monitor_code: +.org 0x150 +program_old_psw: +.org 0x1d0 /* program new PSW */ +.quad 0x18000,pgm /* 64-bit mode */ +.org 0x200 /* lowcore padding */ +.globl _start +_start: +stctg %c8,%c8,c8/* enable only monitor class 1 */ +mvhhi c8+6,0x4000 +lctlg %c8,%c8,c8 +mc_nop: +mc 123,0 +mc_monitor_event: +mc 321,1 +j failure +mc_specification: +mc 333,16 +j failure +pgm: +lgrl %r0,program_old_psw+8 /* ilc adjustment */ +llgc %r1,ilc +sgr %r0,%r1 +larl %r1,mc_monitor_event /* dispatch based on old PSW */ +cgrje %r0,%r1,pgm_monitor_event +larl %r1,mc_specification +cgrje %r0,%r1,pgm_specification +j failure +pgm_monitor_event: +chhsi program_interruption_code,0x40/* monitor event? */ +jne failure +chhsi monitor_class,1 /* class from mc_monitor_event? */ +jne failure +cghsi monitor_code,321 /* code from mc_monitor_event? */ +jne failure +j mc_specification /* next test */ +pgm_specification: +chhsi program_interruption_code,6 /* specification exception? */ +jne failure +lpswe success_psw +failure: +lpswe failure_psw +.align 8 +c8: +.quad 0 +success_psw: +.quad 0x2,0xfff /* see is_special_wait_psw() */ +failure_psw: +.quad 0x2,0 /* disabled wait */ -- 2.41.0
[PATCH v4 03/14] target/s390x: Fix CONVERT TO LOGICAL/FIXED with out-of-range inputs
CONVERT TO LOGICAL/FIXED deviate from IEEE 754 in that they raise an inexact exception on out-of-range inputs. float_flag_invalid_cvti aligns nicely with that behavior, so convert it to S390_IEEE_MASK_INEXACT. Cc: qemu-sta...@nongnu.org Fixes: defb0e3157af ("s390x: Implement opcode helpers") Reviewed-by: David Hildenbrand Reviewed-by: Richard Henderson Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/fpu_helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/s390x/tcg/fpu_helper.c b/target/s390x/tcg/fpu_helper.c index 4b7fa58af3e..3d941ed2d28 100644 --- a/target/s390x/tcg/fpu_helper.c +++ b/target/s390x/tcg/fpu_helper.c @@ -52,7 +52,8 @@ uint8_t s390_softfloat_exc_to_ieee(unsigned int exc) s390_exc |= (exc & float_flag_divbyzero) ? S390_IEEE_MASK_DIVBYZERO : 0; s390_exc |= (exc & float_flag_overflow) ? S390_IEEE_MASK_OVERFLOW : 0; s390_exc |= (exc & float_flag_underflow) ? S390_IEEE_MASK_UNDERFLOW : 0; -s390_exc |= (exc & float_flag_inexact) ? S390_IEEE_MASK_INEXACT : 0; +s390_exc |= (exc & (float_flag_inexact | float_flag_invalid_cvti)) ? +S390_IEEE_MASK_INEXACT : 0; return s390_exc; } -- 2.41.0
[PATCH v4 08/14] tests/tcg/s390x: Test CKSM
Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/cksm.S | 29 + 2 files changed, 30 insertions(+) create mode 100644 tests/tcg/s390x/cksm.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index 242c7b0f83c..e813e318db9 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -16,6 +16,7 @@ LDFLAGS=-nostdlib -static ASM_TESTS = \ bal \ +cksm \ exrl-ssm-early \ sam \ lpsw \ diff --git a/tests/tcg/s390x/cksm.S b/tests/tcg/s390x/cksm.S new file mode 100644 index 000..563fd3d233e --- /dev/null +++ b/tests/tcg/s390x/cksm.S @@ -0,0 +1,29 @@ +.org 0x8e +program_interruption_code: +.org 0x1d0 /* program new PSW */ +.quad 0,pgm +.org 0x200 /* lowcore padding */ +.globl _start +_start: +lmg %r0,%r1,cksm_args +cksm %r2,%r0 +c %r2,cksm_exp +jne failure +.insn rre,0xb241,%r2,%r15 /* cksm %r2,%r15 */ +failure: +lpswe failure_psw +pgm: +chhsi program_interruption_code,6 /* specification exception? */ +jne failure +lpswe success_psw +cksm_args: +.quad cksm_buf, 16 +cksm_buf: +.quad 0x, 0x12345678 +cksm_exp: +.long 0x89ab1234 +.align 8 +success_psw: +.quad 0x2,0xfff/* see is_special_wait_psw() */ +failure_psw: +.quad 0x2,0/* disabled wait */ -- 2.41.0
[PATCH v4 11/14] tests/tcg/s390x: Test ICM
Add a small test to prevent regressions. Tested-by: Thomas Huth Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/icm.S | 32 + 2 files changed, 33 insertions(+) create mode 100644 tests/tcg/s390x/icm.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index 062d8e368aa..58684d7da71 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -19,6 +19,7 @@ ASM_TESTS = \ cksm \ clm \ exrl-ssm-early \ +icm \ sam \ lpsw \ lpswe-early \ diff --git a/tests/tcg/s390x/icm.S b/tests/tcg/s390x/icm.S new file mode 100644 index 000..d24d1f52fb8 --- /dev/null +++ b/tests/tcg/s390x/icm.S @@ -0,0 +1,32 @@ +.org 0x8e +program_interruption_code: +.org 0x1d0 /* program new PSW */ +.quad 0,pgm +.org 0x200 /* lowcore padding */ +.globl _start +_start: +lgrl %r0,op1 +icm %r0,10,op2 +cg %r0,exp +jne failure +lgrl %r1,bad_addr +icm %r0,0,0(%r1) +failure: +lpswe failure_psw +pgm: +chhsi program_interruption_code,5 /* addressing exception? */ +jne failure +lpswe success_psw +.align 8 +op1: +.quad 0x1234567887654321 +op2: +.quad 0x0011223344556677 +exp: +.quad 0x1234567800651121 +bad_addr: +.quad 0x +success_psw: +.quad 0x2,0xfff/* see is_special_wait_psw() */ +failure_psw: +.quad 0x2,0/* disabled wait */ -- 2.41.0
[PATCH v4 02/14] target/s390x: Fix CLM with M3=0
When the mask is zero, access exceptions should still be recognized for 1 byte at the second-operand address. CC should be set to 0. Cc: qemu-sta...@nongnu.org Fixes: defb0e3157af ("s390x: Implement opcode helpers") Reviewed-by: David Hildenbrand Reviewed-by: Richard Henderson Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/mem_helper.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c index f417fb1183c..84103251b97 100644 --- a/target/s390x/tcg/mem_helper.c +++ b/target/s390x/tcg/mem_helper.c @@ -667,6 +667,11 @@ uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask, HELPER_LOG("%s: r1 0x%x mask 0x%x addr 0x%" PRIx64 "\n", __func__, r1, mask, addr); +if (!mask) { +/* Recognize access exceptions for the first byte */ +probe_read(env, addr, 1, cpu_mmu_index(env, false), ra); +} + while (mask) { if (mask & 8) { uint8_t d = cpu_ldub_data_ra(env, addr, ra); -- 2.41.0
Re: [PATCH for-8.2 1/2] arm/kvm: convert to kvm_set_one_reg
On Mon, Jul 24 2023, Gavin Shan wrote: > Hi Connie, > > On 7/18/23 21:14, Cornelia Huck wrote: >> We can neaten the code by switching to the kvm_set_one_reg function. >> >> Signed-off-by: Cornelia Huck >> --- >> target/arm/kvm.c | 13 +++-- >> target/arm/kvm64.c | 66 +- >> 2 files changed, 21 insertions(+), 58 deletions(-) >> > > Some wrong replacements to be fixed in kvm_arch_put_fpsimd() as below. > Apart from that, LGTM: > > Reviewed-by: Gavin Shan > @@ -725,19 +721,17 @@ static void kvm_inject_arm_sea(CPUState *c) >> static int kvm_arch_put_fpsimd(CPUState *cs) >> { >> CPUARMState *env = &ARM_CPU(cs)->env; >> -struct kvm_one_reg reg; >> int i, ret; >> >> for (i = 0; i < 32; i++) { >> uint64_t *q = aa64_vfp_qreg(env, i); >> #if HOST_BIG_ENDIAN >> uint64_t fp_val[2] = { q[1], q[0] }; >> -reg.addr = (uintptr_t)fp_val; >> +ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), >> +&fp_val); > ^^^ > s/&fp_val/fp_val >> #else >> -reg.addr = (uintptr_t)q; >> +ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), >> &q); > > ^^^ > > s/&q/q > > >> #endif Whoops, I thought I had double-checked these...
Re: [PATCH for-8.2 2/2] arm/kvm: convert to kvm_get_one_reg
On Mon, Jul 24 2023, Gavin Shan wrote: > Hi Connie, > > On 7/18/23 21:14, Cornelia Huck wrote: >> We can neaten the code by switching the callers that work on a >> CPUstate to the kvm_get_one_reg function. >> >> Signed-off-by: Cornelia Huck >> --- >> target/arm/kvm.c | 15 +++- >> target/arm/kvm64.c | 57 -- >> 2 files changed, 18 insertions(+), 54 deletions(-) >> > > The replacements look good to me. However, I guess it's worty to apply > the same replacements for target/arm/kvm64.c since we're here? > > [gshan@gshan arm]$ pwd > /home/gshan/sandbox/q/target/arm > [gshan@gshan arm]$ git grep KVM_GET_ONE_REG > kvm64.c:err = ioctl(fd, KVM_GET_ONE_REG, &idreg); > kvm64.c:return ioctl(fd, KVM_GET_ONE_REG, &idreg); > kvm64.c:ret = ioctl(fdarray[2], KVM_GET_ONE_REG, ®); These are the callers that don't work on a CPUState (all in initial feature discovery IIRC), so they need to stay that way.
Re: [PATCH] Allow UNIX socket for VNC websocket
On Mon, Jul 24, 2023 at 12:08:26PM +0400, Marc-André Lureau wrote: > Hi > > Daniel, in commit 275e0d616b ("ui: refactor code for populating > SocketAddress from vnc_display_open"), you said "This refactoring also > removes the restriction that prevents enabling websockets when the plain > VNC server is listening on a UNIX socket.". But you didn't remove the > condition. I suppose it was a left-over? That commit was allowing the regular VNC port to be put on UNIX socket, at the same time as WS was enabled, it wasn't trying to put WS on a UNIX socket. Still I seee no reason to keep the limitation. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] Allow UNIX socket for VNC websocket
On Mon, Jul 24, 2023 at 12:03:56AM +0300, Sergii Zasenko wrote: > Signed-off-by: Sergii Zasenko > --- > ui/vnc.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/ui/vnc.c b/ui/vnc.c > index 92964dc..dea1414 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char *addrstr, > addr->type = SOCKET_ADDRESS_TYPE_UNIX; > addr->u.q_unix.path = g_strdup(addrstr + 5); > > -if (websocket) { > -error_setg(errp, "UNIX sockets not supported with websock"); > -goto cleanup; > -} > - > if (to) { > error_setg(errp, "Port range not support with UNIX socket"); > goto cleanup; Missing docs update to qemu-options.hx to describe the permitted syntax for the new feature. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] tests/avocado/machine_s390_ccw_virtio: Skip the flaky virtio-gpu test by default
The virtio-gpu test is known to be flaky - that's why we also did not enable the test_s390x_fedora in the gitlab CI. However, a flaky test can also be annoying when testing locally, so let's rather skip this subtest by default and start running the test_s390x_fedora test in the gitlab CI again (since the other things that are tested here are quite valuable). Signed-off-by: Thomas Huth --- tests/avocado/machine_s390_ccw_virtio.py | 51 +--- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/tests/avocado/machine_s390_ccw_virtio.py b/tests/avocado/machine_s390_ccw_virtio.py index 78152f2ad1..e7a2a20ba6 100644 --- a/tests/avocado/machine_s390_ccw_virtio.py +++ b/tests/avocado/machine_s390_ccw_virtio.py @@ -159,7 +159,6 @@ def test_s390x_devices(self): 'MemTotal: 115640 kB') -@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') def test_s390x_fedora(self): """ @@ -229,31 +228,35 @@ def test_s390x_fedora(self): # writing to the framebuffer. Since the PPM is uncompressed, we then # can simply read the written "magic bytes" back from the PPM file to # check whether the framebuffer is working as expected. -self.log.info("Test screendump of virtio-gpu device") -exec_command_and_wait_for_pattern(self, +# Unfortunately, this test is flaky, so we don't run it by default +if os.getenv('QEMU_TEST_FLAKY_TESTS'): +self.log.info("Test screendump of virtio-gpu device") +exec_command_and_wait_for_pattern(self, 'while ! (dmesg | grep gpudrmfb) ; do sleep 1 ; done', 'virtio_gpudrmfb frame buffer device') -exec_command_and_wait_for_pattern(self, -'echo -e "\e[?25l" > /dev/tty0', ':/#') -exec_command_and_wait_for_pattern(self, 'for ((i=0;i<250;i++)); do ' -'echo " The qu ick fo x j ump s o ver a laz y d og" >> fox.txt;' -'done', -':/#') -exec_command_and_wait_for_pattern(self, -'dd if=fox.txt of=/dev/fb0 bs=1000 oflag=sync,nocache ; rm fox.txt', -'12+0 records out') -with tempfile.NamedTemporaryFile(suffix='.ppm', - prefix='qemu-scrdump-') as ppmfile: -self.vm.command('screendump', filename=ppmfile.name) -ppmfile.seek(0) -line = ppmfile.readline() -self.assertEqual(line, b"P6\n") -line = ppmfile.readline() -self.assertEqual(line, b"1280 800\n") -line = ppmfile.readline() -self.assertEqual(line, b"255\n") -line = ppmfile.readline(256) -self.assertEqual(line, b"The quick fox jumps over a lazy dog\n") +exec_command_and_wait_for_pattern(self, +'echo -e "\e[?25l" > /dev/tty0', ':/#') +exec_command_and_wait_for_pattern(self, 'for ((i=0;i<250;i++)); do ' +'echo " The qu ick fo x j ump s o ver a laz y d og" >> fox.txt;' +'done', +':/#') +exec_command_and_wait_for_pattern(self, +'dd if=fox.txt of=/dev/fb0 bs=1000 oflag=sync,nocache ; rm fox.txt', +'12+0 records out') +with tempfile.NamedTemporaryFile(suffix='.ppm', + prefix='qemu-scrdump-') as ppmfile: +self.vm.command('screendump', filename=ppmfile.name) +ppmfile.seek(0) +line = ppmfile.readline() +self.assertEqual(line, b"P6\n") +line = ppmfile.readline() +self.assertEqual(line, b"1280 800\n") +line = ppmfile.readline() +self.assertEqual(line, b"255\n") +line = ppmfile.readline(256) +self.assertEqual(line, b"The quick fox jumps over a lazy dog\n") +else: +self.log.info("Skipped flaky screendump of virtio-gpu device test") # Hot-plug a virtio-crypto device and see whether it gets accepted self.log.info("Test hot-plug virtio-crypto device") -- 2.39.3
[PULL 6/7] accel/tcg: Zero-pad vaddr in tlb_debug output
From: Anton Johansson In replacing target_ulong with vaddr and TARGET_FMT_lx with VADDR_PRIx, the zero-padding of TARGET_FMT_lx got lost. Readd 16-wide zero-padding for logging consistency. Suggested-by: Peter Maydell Signed-off-by: Anton Johansson Message-Id: <20230713120746.26897-1-a...@rev.ng> Reviewed-by: Richard Henderson Signed-off-by: Richard Henderson --- accel/tcg/cputlb.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index e0079c9a9d..ba44501a7c 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -497,8 +497,8 @@ static void tlb_flush_page_locked(CPUArchState *env, int midx, vaddr page) /* Check if we need to flush due to large pages. */ if ((page & lp_mask) == lp_addr) { -tlb_debug("forcing full flush midx %d (%" - VADDR_PRIx "/%" VADDR_PRIx ")\n", +tlb_debug("forcing full flush midx %d (%016" + VADDR_PRIx "/%016" VADDR_PRIx ")\n", midx, lp_addr, lp_mask); tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime()); } else { @@ -527,7 +527,7 @@ static void tlb_flush_page_by_mmuidx_async_0(CPUState *cpu, assert_cpu_is_self(cpu); -tlb_debug("page addr: %" VADDR_PRIx " mmu_map:0x%x\n", addr, idxmap); +tlb_debug("page addr: %016" VADDR_PRIx " mmu_map:0x%x\n", addr, idxmap); qemu_spin_lock(&env_tlb(env)->c.lock); for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) { @@ -591,7 +591,7 @@ static void tlb_flush_page_by_mmuidx_async_2(CPUState *cpu, void tlb_flush_page_by_mmuidx(CPUState *cpu, vaddr addr, uint16_t idxmap) { -tlb_debug("addr: %" VADDR_PRIx " mmu_idx:%" PRIx16 "\n", addr, idxmap); +tlb_debug("addr: %016" VADDR_PRIx " mmu_idx:%" PRIx16 "\n", addr, idxmap); /* This should already be page aligned */ addr &= TARGET_PAGE_MASK; @@ -625,7 +625,7 @@ void tlb_flush_page(CPUState *cpu, vaddr addr) void tlb_flush_page_by_mmuidx_all_cpus(CPUState *src_cpu, vaddr addr, uint16_t idxmap) { -tlb_debug("addr: %" VADDR_PRIx " mmu_idx:%"PRIx16"\n", addr, idxmap); +tlb_debug("addr: %016" VADDR_PRIx " mmu_idx:%"PRIx16"\n", addr, idxmap); /* This should already be page aligned */ addr &= TARGET_PAGE_MASK; @@ -666,7 +666,7 @@ void tlb_flush_page_by_mmuidx_all_cpus_synced(CPUState *src_cpu, vaddr addr, uint16_t idxmap) { -tlb_debug("addr: %" VADDR_PRIx " mmu_idx:%"PRIx16"\n", addr, idxmap); +tlb_debug("addr: %016" VADDR_PRIx " mmu_idx:%"PRIx16"\n", addr, idxmap); /* This should already be page aligned */ addr &= TARGET_PAGE_MASK; @@ -728,7 +728,7 @@ static void tlb_flush_range_locked(CPUArchState *env, int midx, */ if (mask < f->mask || len > f->mask) { tlb_debug("forcing full flush midx %d (" - "%" VADDR_PRIx "/%" VADDR_PRIx "+%" VADDR_PRIx ")\n", + "%016" VADDR_PRIx "/%016" VADDR_PRIx "+%016" VADDR_PRIx ")\n", midx, addr, mask, len); tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime()); return; @@ -741,7 +741,7 @@ static void tlb_flush_range_locked(CPUArchState *env, int midx, */ if (((addr + len - 1) & d->large_page_mask) == d->large_page_addr) { tlb_debug("forcing full flush midx %d (" - "%" VADDR_PRIx "/%" VADDR_PRIx ")\n", + "%016" VADDR_PRIx "/%016" VADDR_PRIx ")\n", midx, d->large_page_addr, d->large_page_mask); tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime()); return; @@ -773,7 +773,7 @@ static void tlb_flush_range_by_mmuidx_async_0(CPUState *cpu, assert_cpu_is_self(cpu); -tlb_debug("range: %" VADDR_PRIx "/%u+%" VADDR_PRIx " mmu_map:0x%x\n", +tlb_debug("range: %016" VADDR_PRIx "/%u+%016" VADDR_PRIx " mmu_map:0x%x\n", d.addr, d.bits, d.len, d.idxmap); qemu_spin_lock(&env_tlb(env)->c.lock); @@ -1165,7 +1165,7 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx, &xlat, &sz, full->attrs, &prot); assert(sz >= TARGET_PAGE_SIZE); -tlb_debug("vaddr=%" VADDR_PRIx " paddr=0x" HWADDR_FMT_plx +tlb_debug("vaddr=%016" VADDR_PRIx " paddr=0x" HWADDR_FMT_plx " prot=%x idx=%d\n", addr, full->phys_addr, prot, mmu_idx); -- 2.34.1
[PULL 2/7] include/exec: Add WITH_MMAP_LOCK_GUARD
Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 10 ++ bsd-user/mmap.c | 1 + linux-user/mmap.c | 1 + 3 files changed, 12 insertions(+) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 5fa0687cd2..d02517e95f 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -629,6 +629,15 @@ void TSA_NO_TSA mmap_lock(void); void TSA_NO_TSA mmap_unlock(void); bool have_mmap_lock(void); +static inline void mmap_unlock_guard(void *unused) +{ +mmap_unlock(); +} + +#define WITH_MMAP_LOCK_GUARD()\ +for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard))) \ + = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1) + /** * adjust_signal_pc: * @pc: raw pc from the host signal ucontext_t. @@ -683,6 +692,7 @@ G_NORETURN void cpu_loop_exit_sigbus(CPUState *cpu, target_ulong addr, #else static inline void mmap_lock(void) {} static inline void mmap_unlock(void) {} +#define WITH_MMAP_LOCK_GUARD() void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length); void tlb_set_dirty(CPUState *cpu, vaddr addr); diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c index aca8764356..74ed00b9fe 100644 --- a/bsd-user/mmap.c +++ b/bsd-user/mmap.c @@ -32,6 +32,7 @@ void mmap_lock(void) void mmap_unlock(void) { +assert(mmap_lock_count > 0); if (--mmap_lock_count == 0) { pthread_mutex_unlock(&mmap_mutex); } diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 44b53bd446..a5dfb56545 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -36,6 +36,7 @@ void mmap_lock(void) void mmap_unlock(void) { +assert(mmap_lock_count > 0); if (--mmap_lock_count == 0) { pthread_mutex_unlock(&mmap_mutex); } -- 2.34.1
[PULL 3/7] accel/tcg: Fix sense of read-only probes in ldst_atomicity
In the initial commit, cdfac37be0d, the sense of the test is incorrect, as the -1/0 return was confusing. In bef6f008b981, we mechanically invert all callers while changing to false/true return, preserving the incorrectness of the test. Now that the return sense is sane, it's easy to see that if !write, then the page is not modifiable (i.e. most likely read-only, with PROT_NONE handled via SIGSEGV). Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- accel/tcg/ldst_atomicity.c.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/accel/tcg/ldst_atomicity.c.inc b/accel/tcg/ldst_atomicity.c.inc index 4de0a80492..de70531a7a 100644 --- a/accel/tcg/ldst_atomicity.c.inc +++ b/accel/tcg/ldst_atomicity.c.inc @@ -159,7 +159,7 @@ static uint64_t load_atomic8_or_exit(CPUArchState *env, uintptr_t ra, void *pv) * another process, because the fallback start_exclusive solution * provides no protection across processes. */ -if (page_check_range(h2g(pv), 8, PAGE_WRITE_ORG)) { +if (!page_check_range(h2g(pv), 8, PAGE_WRITE_ORG)) { uint64_t *p = __builtin_assume_aligned(pv, 8); return *p; } @@ -194,7 +194,7 @@ static Int128 load_atomic16_or_exit(CPUArchState *env, uintptr_t ra, void *pv) * another process, because the fallback start_exclusive solution * provides no protection across processes. */ -if (page_check_range(h2g(p), 16, PAGE_WRITE_ORG)) { +if (!page_check_range(h2g(p), 16, PAGE_WRITE_ORG)) { return *p; } #endif -- 2.34.1
[PULL 1/7] tcg/ppc: Fix race in goto_tb implementation
From: Jordan Niethe Commit 20b6643324 ("tcg/ppc: Reorg goto_tb implementation") modified goto_tb to ensure only a single instruction was patched to prevent incorrect behavior if a thread was in the middle of multiple instructions when they were replaced. However this introduced a race between loading the jmp target into TCG_REG_TB and patching and executing the direct branch. The relevant part of the goto_tb implementation: ld TCG_REG_TB, TARGET_ADDR_LOCATION(TCG_REG_TB) patch_location: mtctr TCG_REG_TB bctr tb_target_set_jmp_target() will replace 'patch_location' with a direct branch if the target is in range. The direct branch now relies on TCG_REG_TB being set up correctly by the ld. Prior to this commit multiple instructions were patched in for the direct branch case; these instructions would initialize TCG_REG_TB to the same value as the branch target. Imagine the following sequence: 1) Thread A is executing the goto_tb sequence and loads the jmp target into TCG_REG_TB. 2) Thread B updates the jmp target address and calls tb_target_set_jmp_target(). This patches a new direct branch into the goto_tb sequence. 3) Thread A executes the newly patched direct branch. The value in TCG_REG_TB still contains the old jmp target. TCG_REG_TB MUST contain the translation block's tc.ptr. Execution will eventually crash after performing memory accesses generated from a faulty value in TCG_REG_TB. This presents as segfaults or illegal instruction exceptions. Do not revert commit 20b6643324 as it did fix a different race condition. Instead remove the direct branch optimization and always use indirect branches. The direct branch optimization can be re-added later with a race free sequence. Fixes: 20b6643324 ("tcg/ppc: Reorg goto_tb implementation") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1726 Reported-by: Anushree Mathur Tested-by: Anushree Mathur Tested-by: Michael Tokarev Reviewed-by: Richard Henderson Co-developed-by: Benjamin Gray Signed-off-by: Jordan Niethe Signed-off-by: Benjamin Gray Message-Id: <20230717093001.13167-1-jniet...@gmail.com> --- tcg/ppc/tcg-target.c.inc | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc index c866f2c997..511e14b180 100644 --- a/tcg/ppc/tcg-target.c.inc +++ b/tcg/ppc/tcg-target.c.inc @@ -2496,11 +2496,10 @@ static void tcg_out_goto_tb(TCGContext *s, int which) ptrdiff_t offset = tcg_tbrel_diff(s, (void *)ptr); tcg_out_mem_long(s, LD, LDX, TCG_REG_TB, TCG_REG_TB, offset); -/* Direct branch will be patched by tb_target_set_jmp_target. */ +/* TODO: Use direct branches when possible. */ set_jmp_insn_offset(s, which); tcg_out32(s, MTSPR | RS(TCG_REG_TB) | CTR); -/* When branch is out of range, fall through to indirect. */ tcg_out32(s, BCCTR | BO_ALWAYS); /* For the unlinked case, need to reset TCG_REG_TB. */ @@ -2528,10 +2527,12 @@ void tb_target_set_jmp_target(const TranslationBlock *tb, int n, intptr_t diff = addr - jmp_rx; tcg_insn_unit insn; +if (USE_REG_TB) { +return; +} + if (in_range_b(diff)) { insn = B | (diff & 0x3fc); -} else if (USE_REG_TB) { -insn = MTSPR | RS(TCG_REG_TB) | CTR; } else { insn = NOP; } -- 2.34.1
[PULL 7/7] accel/tcg: Fix type of 'last' for pageflags_{find,next}
From: Luca Bonissi These should match 'start' as target_ulong, not target_long. On 32bit targets, the parameter was sign-extended to uint64_t, so only the first mmap within the upper 2GB memory can succeed. Signed-off-by: Luca Bonissi Message-Id: <327460e2-0ebd-9edb-426b-1df80d16c...@bonslack.org> Reviewed-by: Richard Henderson Signed-off-by: Richard Henderson --- accel/tcg/user-exec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index ac38c2bf96..ab48cb41e4 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -144,7 +144,7 @@ typedef struct PageFlagsNode { static IntervalTreeRoot pageflags_root; -static PageFlagsNode *pageflags_find(target_ulong start, target_long last) +static PageFlagsNode *pageflags_find(target_ulong start, target_ulong last) { IntervalTreeNode *n; @@ -153,7 +153,7 @@ static PageFlagsNode *pageflags_find(target_ulong start, target_long last) } static PageFlagsNode *pageflags_next(PageFlagsNode *p, target_ulong start, - target_long last) + target_ulong last) { IntervalTreeNode *n; -- 2.34.1
[PULL for-8.1-rc1 0/7] tcg patch queue
The following changes since commit d1181d29370a4318a9f11ea92065bea6bb159f83: Merge tag 'pull-nbd-2023-07-19' of https://repo.or.cz/qemu/ericb into staging (2023-07-20 09:54:07 +0100) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20230724 for you to fetch changes up to 32b120394c578bc824f1db4835b3bffbeca88fae: accel/tcg: Fix type of 'last' for pageflags_{find,next} (2023-07-24 09:48:49 +0100) accel/tcg: Zero-pad vaddr in tlb debug output accel/tcg: Fix type of 'last' for pageflags_{find,next} accel/tcg: Fix sense of read-only probes in ldst_atomicity accel/tcg: Take mmap_lock in load_atomic*_or_exit tcg: Add earlyclobber to op_add2 for x86 and s390x tcg/ppc: Fix race in goto_tb implementation Anton Johansson (1): accel/tcg: Zero-pad vaddr in tlb_debug output Ilya Leoshkevich (1): tcg/{i386, s390x}: Add earlyclobber to the op_add2's first output Jordan Niethe (1): tcg/ppc: Fix race in goto_tb implementation Luca Bonissi (1): accel/tcg: Fix type of 'last' for pageflags_{find,next} Richard Henderson (3): include/exec: Add WITH_MMAP_LOCK_GUARD accel/tcg: Fix sense of read-only probes in ldst_atomicity accel/tcg: Take mmap_lock in load_atomic*_or_exit include/exec/exec-all.h| 10 ++ tcg/i386/tcg-target-con-set.h | 5 - tcg/s390x/tcg-target-con-set.h | 8 +--- accel/tcg/cputlb.c | 20 ++-- accel/tcg/user-exec.c | 4 ++-- bsd-user/mmap.c| 1 + linux-user/mmap.c | 1 + tcg/tcg.c | 8 +++- accel/tcg/ldst_atomicity.c.inc | 32 ++-- tcg/i386/tcg-target.c.inc | 2 +- tcg/ppc/tcg-target.c.inc | 9 + tcg/s390x/tcg-target.c.inc | 4 ++-- 12 files changed, 66 insertions(+), 38 deletions(-)
[PULL 5/7] tcg/{i386, s390x}: Add earlyclobber to the op_add2's first output
From: Ilya Leoshkevich i386 and s390x implementations of op_add2 require an earlyclobber, which is currently missing. This breaks VCKSM in s390x guests. E.g., on x86_64 the following op: add2_i32 tmp2,tmp3,tmp2,tmp3,tmp3,tmp2 dead: 0 2 3 4 5 pref=none,0x is translated to: addl %ebx, %r12d adcl %r12d, %ebx Introduce a new C_N1_O1_I4 constraint, and make sure that earlyclobber of aliased outputs is honored. Cc: qemu-sta...@nongnu.org Fixes: 82790a870992 ("tcg: Add markup for output requires new register") Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Message-Id: <20230719221310.1968845-7-...@linux.ibm.com> Signed-off-by: Richard Henderson --- tcg/i386/tcg-target-con-set.h | 5 - tcg/s390x/tcg-target-con-set.h | 8 +--- tcg/tcg.c | 8 +++- tcg/i386/tcg-target.c.inc | 2 +- tcg/s390x/tcg-target.c.inc | 4 ++-- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/tcg/i386/tcg-target-con-set.h b/tcg/i386/tcg-target-con-set.h index 91ceb0e1da..5ea3a292f0 100644 --- a/tcg/i386/tcg-target-con-set.h +++ b/tcg/i386/tcg-target-con-set.h @@ -11,6 +11,9 @@ * * C_N1_Im(...) defines a constraint set with 1 output and inputs, * except that the output must use a new register. + * + * C_Nn_Om_Ik(...) defines a constraint set with outputs and + * inputs, except that the first outputs must use new registers. */ C_O0_I1(r) C_O0_I2(L, L) @@ -53,4 +56,4 @@ C_O2_I1(r, r, L) C_O2_I2(a, d, a, r) C_O2_I2(r, r, L, L) C_O2_I3(a, d, 0, 1, r) -C_O2_I4(r, r, 0, 1, re, re) +C_N1_O1_I4(r, r, 0, 1, re, re) diff --git a/tcg/s390x/tcg-target-con-set.h b/tcg/s390x/tcg-target-con-set.h index cbad91b2b5..9a42037499 100644 --- a/tcg/s390x/tcg-target-con-set.h +++ b/tcg/s390x/tcg-target-con-set.h @@ -8,6 +8,9 @@ * C_On_Im(...) defines a constraint set with outputs and inputs. * Each operand should be a sequence of constraint letters as defined by * tcg-target-con-str.h; the constraint combination is inclusive or. + * + * C_Nn_Om_Ik(...) defines a constraint set with outputs and + * inputs, except that the first outputs must use new registers. */ C_O0_I1(r) C_O0_I2(r, r) @@ -41,6 +44,5 @@ C_O2_I1(o, m, r) C_O2_I2(o, m, 0, r) C_O2_I2(o, m, r, r) C_O2_I3(o, m, 0, 1, r) -C_O2_I4(r, r, 0, 1, rA, r) -C_O2_I4(r, r, 0, 1, ri, r) -C_O2_I4(r, r, 0, 1, r, r) +C_N1_O1_I4(r, r, 0, 1, ri, r) +C_N1_O1_I4(r, r, 0, 1, rA, r) diff --git a/tcg/tcg.c b/tcg/tcg.c index 652e8ea6b9..ddfe9a96cb 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -648,6 +648,7 @@ static void tcg_out_movext3(TCGContext *s, const TCGMovExtend *i1, #define C_O2_I2(O1, O2, I1, I2) C_PFX4(c_o2_i2_, O1, O2, I1, I2), #define C_O2_I3(O1, O2, I1, I2, I3) C_PFX5(c_o2_i3_, O1, O2, I1, I2, I3), #define C_O2_I4(O1, O2, I1, I2, I3, I4) C_PFX6(c_o2_i4_, O1, O2, I1, I2, I3, I4), +#define C_N1_O1_I4(O1, O2, I1, I2, I3, I4) C_PFX6(c_n1_o1_i4_, O1, O2, I1, I2, I3, I4), typedef enum { #include "tcg-target-con-set.h" @@ -668,6 +669,7 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode); #undef C_O2_I2 #undef C_O2_I3 #undef C_O2_I4 +#undef C_N1_O1_I4 /* Put all of the constraint sets into an array, indexed by the enum. */ @@ -687,6 +689,7 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode); #define C_O2_I2(O1, O2, I1, I2) { .args_ct_str = { #O1, #O2, #I1, #I2 } }, #define C_O2_I3(O1, O2, I1, I2, I3) { .args_ct_str = { #O1, #O2, #I1, #I2, #I3 } }, #define C_O2_I4(O1, O2, I1, I2, I3, I4) { .args_ct_str = { #O1, #O2, #I1, #I2, #I3, #I4 } }, +#define C_N1_O1_I4(O1, O2, I1, I2, I3, I4) { .args_ct_str = { "&" #O1, #O2, #I1, #I2, #I3, #I4 } }, static const TCGTargetOpDef constraint_sets[] = { #include "tcg-target-con-set.h" @@ -706,6 +709,7 @@ static const TCGTargetOpDef constraint_sets[] = { #undef C_O2_I2 #undef C_O2_I3 #undef C_O2_I4 +#undef C_N1_O1_I4 /* Expand the enumerator to be returned from tcg_target_op_def(). */ @@ -725,6 +729,7 @@ static const TCGTargetOpDef constraint_sets[] = { #define C_O2_I2(O1, O2, I1, I2) C_PFX4(c_o2_i2_, O1, O2, I1, I2) #define C_O2_I3(O1, O2, I1, I2, I3) C_PFX5(c_o2_i3_, O1, O2, I1, I2, I3) #define C_O2_I4(O1, O2, I1, I2, I3, I4) C_PFX6(c_o2_i4_, O1, O2, I1, I2, I3, I4) +#define C_N1_O1_I4(O1, O2, I1, I2, I3, I4) C_PFX6(c_n1_o1_i4_, O1, O2, I1, I2, I3, I4) #include "tcg-target.c.inc" @@ -4703,7 +4708,8 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp *op) * dead after the instruction, we must allocate a new * register and move it. */ -if (temp_readonly(ts) || !IS_DEAD_ARG(i)) { +if (temp_readonly(ts) || !IS_DEAD_ARG(i) +|| def->args_ct[arg_ct->alias_index].newreg) { allocate_new_reg = true; } else if (ts->val_type == TEMP_VAL_REG) { /* diff --git a/tcg/i386/tcg-target.c.inc b/tcg
[PULL 4/7] accel/tcg: Take mmap_lock in load_atomic*_or_exit
For user-only, the probe for page writability may race with another thread's mprotect. Take the mmap_lock around the operation. This is still faster than the start/end_exclusive fallback. Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- accel/tcg/ldst_atomicity.c.inc | 32 ++-- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/accel/tcg/ldst_atomicity.c.inc b/accel/tcg/ldst_atomicity.c.inc index de70531a7a..e5c590a499 100644 --- a/accel/tcg/ldst_atomicity.c.inc +++ b/accel/tcg/ldst_atomicity.c.inc @@ -159,9 +159,11 @@ static uint64_t load_atomic8_or_exit(CPUArchState *env, uintptr_t ra, void *pv) * another process, because the fallback start_exclusive solution * provides no protection across processes. */ -if (!page_check_range(h2g(pv), 8, PAGE_WRITE_ORG)) { -uint64_t *p = __builtin_assume_aligned(pv, 8); -return *p; +WITH_MMAP_LOCK_GUARD() { +if (!page_check_range(h2g(pv), 8, PAGE_WRITE_ORG)) { +uint64_t *p = __builtin_assume_aligned(pv, 8); +return *p; +} } #endif @@ -186,25 +188,27 @@ static Int128 load_atomic16_or_exit(CPUArchState *env, uintptr_t ra, void *pv) return atomic16_read_ro(p); } -#ifdef CONFIG_USER_ONLY /* * We can only use cmpxchg to emulate a load if the page is writable. * If the page is not writable, then assume the value is immutable * and requires no locking. This ignores the case of MAP_SHARED with * another process, because the fallback start_exclusive solution * provides no protection across processes. + * + * In system mode all guest pages are writable. For user mode, + * we must take mmap_lock so that the query remains valid until + * the write is complete -- tests/tcg/multiarch/munmap-pthread.c + * is an example that can race. */ -if (!page_check_range(h2g(p), 16, PAGE_WRITE_ORG)) { -return *p; -} +WITH_MMAP_LOCK_GUARD() { +#ifdef CONFIG_USER_ONLY +if (!page_check_range(h2g(p), 16, PAGE_WRITE_ORG)) { +return *p; +} #endif - -/* - * In system mode all guest pages are writable, and for user-only - * we have just checked writability. Try cmpxchg. - */ -if (HAVE_ATOMIC128_RW) { -return atomic16_read_rw(p); +if (HAVE_ATOMIC128_RW) { +return atomic16_read_rw(p); +} } /* Ultimate fallback: re-execute in serial context. */ -- 2.34.1
Re: [PATCH v4 01/14] target/s390x: Make CKSM raise an exception if R2 is odd
On 7/24/23 09:15, Ilya Leoshkevich wrote: R2 designates an even-odd register pair; the instruction should raise a specification exception when R2 is not even. Cc:qemu-sta...@nongnu.org Fixes: e023e832d0ac ("s390x: translate engine for s390x CPU") Signed-off-by: Ilya Leoshkevich --- target/s390x/tcg/insn-data.h.inc | 2 +- target/s390x/tcg/translate.c | 6 ++ 2 files changed, 7 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [Qemu PATCH 0/9] Enabling DCD emulation support in Qemu
On Sat, 22 Jul 2023 21:52:06 -0700 Ira Weiny wrote: > nifan@ wrote: > > From: Fan Ni > > > > The patch series provides dynamic capacity device (DCD) emulation in Qemu. > > I don't the patches on the list. > > https://lore.kernel.org/all/sg2pr06mb33976bb3f9c47cbe08f02d09b2...@sg2pr06mb3397.apcprd06.prod.outlook.com/ > > Did they get sent? Something odd going on... They are in https://lists.nongnu.org/archive/html/qemu-devel/2023-07/msg04105.html though threading is broken. Also seems to have made it to qemu-devel on lore https://lore.kernel.org/all/sg2pr06mb3397f3e74a083607f7492fa4b2...@sg2pr06mb3397.apcprd06.prod.outlook.com/ Might be a backlog somewhere that will take a while to drain... Fan, perhaps just resend the lot and make sure the threading works so we can find them more easily. Jonathan > > Ira > > > More specifically, it provides the following functionalities: > > 1. Extended type3 memory device to support DC regions and extents. > > 2. Implemented DCD related mailbox command support in CXL r3.0: 8.2.9.8.9. > > 3. ADD QMP interfaces for adding and releasing DC extents to simulate FM > > functions for DCD described in cxl r3.0: 7.6.7.6.5 and 7.6.7.6.6. > > 4. Add new ct3d properties for DCD devices (host backend, number of dc > > regions, etc.) > > 5. Add read/write support from/to DC regions of the device. > > 6. Add mechanism to validate accessed to DC region address space. > > > > A more detailed description can be found from the previously posted RFC[1]. > > > > Compared to the previously posted RFC[1], following changes have been made: > > 1. Rebased the code on top of Jonathan's branch > > https://gitlab.com/jic23/qemu/-/tree/cxl-2023-05-25. > > 2. Extracted the rename of mem_size to a separated patch.(Jonathan) > > 3. Reordered the patch series to improve its readability.(Jonathan) > > 4. Split the validation of accesses to DC region address space as a separate > > patch. > > 5. Redesigned the QMP interfaces for adding and releasing DC extents to make > > them easier to understand and act like existing QMP interfaces (like the > > interface for cxl-inject-uncorrectable-errors). (Jonathan) > > 6. Updated dvsec range register setting to support DCD devices without > > static > > capacity. > > 7. Fixed other issues mentioned in the comments (Jonathan&Nathan Fontenot). > > 8. Fixed the format issues and checked with checkpatch.pl under qemu code > > dir. > > > > > > The code is tested with the DCD patch series at the kernel side[2]. The test > > is similar to those mentioned in the cover letter of [1]. > > > > > > [1]: > > https://lore.kernel.org/all/20230511175609.2091136-1-fan...@samsung.com/ > > [2]: > > https://lore.kernel.org/linux-cxl/649da378c28a3_968bb29420@iweiny-mobl.notmuch/T/#t > > > > Fan Ni (9): > > hw/cxl/cxl-mailbox-utils: Add dc_event_log_size field to output > > payload of identify memory device command > > hw/cxl/cxl-mailbox-utils: Add dynamic capacity region representative > > and mailbox command support > > include/hw/cxl/cxl_device: Rename mem_size as static_mem_size for > > type3 memory devices > > hw/mem/cxl_type3: Add support to create DC regions to type3 memory > > devices > > hw/mem/cxl_type3: Add host backend and address space handling for DC > > regions > > hw/mem/cxl_type3: Add DC extent list representative and get DC extent > > list mailbox support > > hw/cxl/cxl-mailbox-utils: Add mailbox commands to support add/release > > dynamic capacity response > > hw/cxl/events: Add qmp interfaces to add/release dynamic capacity > > extents > > hw/mem/cxl_type3: Add dpa range validation for accesses to dc regions > > > > hw/cxl/cxl-mailbox-utils.c | 421 +++- > > hw/mem/cxl_type3.c | 539 +--- > > hw/mem/cxl_type3_stubs.c| 6 + > > include/hw/cxl/cxl_device.h | 49 +++- > > include/hw/cxl/cxl_events.h | 16 ++ > > qapi/cxl.json | 49 > > 6 files changed, 1034 insertions(+), 46 deletions(-) > > > > -- > > 2.39.2 > > > > >
Re: [PATCH] tests/avocado/migration: Remove the malfunctioning s390x tests
Thomas Huth wrote: > The tests from tests/avocado/migration.py do not work at all > on s390x - the bios shuts down immediately when it cannot find > a boot disk, so there is nothing left to migrate here. For doing > a proper migration test, we would need a proper payload, but we > already do such tests in the migration *qtest*, so it is unnecessary > to redo such a test here, thus let's simply remove this test. > > Signed-off-by: Thomas Huth Reviewed-by: Juan Quintela > --- > I'm tempted to remove this file completely - what test coverage do > we get here that we don't get by tests/qtest/migration-test.c already? Nothing new there, just three small tests. That are included in migration-test.c I agree we can drop it if we want to. Later, Juan.
Re: [PATCH 0/6] trivial-patches for 2023-07-16
On Mon, 17 Jul 2023 at 10:50, Michael Tokarev wrote: > > 16.07.2023 18:58, Philippe Mathieu-Daudé wrote: > ... > >> Michael Tokarev (5): > >>tree-wide spelling fixes in comments and some messages: migration/ > >>tree-wide spelling fixes in comments and some messages: s390x > >>tree-wide spelling fixes in comments and some messages: arm > >>tree-wide spelling fixes in comments and some messages: other > >> architectures > >>tree-wide spelling fixes in comments and some messages: hw/9pfs > > > > FYI patch subject is usually "subsystem: Topic", see > > https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message: > > > >QEMU follows the usual standard for git commit messages: the first > >line (which becomes the email subject line) is “subsystem: single > >line summary of change”. > > Yes Philippe, I know. In this case though, it really is "tree-wide". I tried > to group them by subsystem but it doesn't work that well. Especially having > in mind how many changes there are (about 400 in total). But you have the subsystem name, you just put it on the wrong end of the subject line. You could fix this up if you're going to send these as a pull request... thanks -- PMM
Re: [PATCH QEMU v9 4/9] migration: Introduce dirty-limit capability
~hyman writes: > From: Hyman Huang(黄勇) > > Introduce migration dirty-limit capability, which can > be turned on before live migration and limit dirty > page rate durty live migration. > > Introduce migrate_dirty_limit function to help check > if dirty-limit capability enabled during live migration. > > Meanwhile, refactor vcpu_dirty_rate_stat_collect > so that period can be configured instead of hardcoded. > > dirty-limit capability is kind of like auto-converge > but using dirty limit instead of traditional cpu-throttle > to throttle guest down. To enable this feature, turn on > the dirty-limit capability before live migration using > migrate-set-capabilities, and set the parameters > "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably > to speed up convergence. > > Signed-off-by: Hyman Huang(黄勇) > Acked-by: Peter Xu > Reviewed-by: Markus Armbruster > Reviewed-by: Juan Quintela Looks good now, thanks!
Re: [PATCH QEMU v9 2/9] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
~hyman writes: > From: Hyman Huang(黄勇) > > Introduce "x-vcpu-dirty-limit-period" migration experimental > parameter, which is in the range of 1 to 1000ms and used to > make dirty page rate calculation period configurable. > > Currently with the "x-vcpu-dirty-limit-period" varies, the Currently, as the ... > total time of live migration changes, test results show the Suggest period instead of comma. > optimal value of "x-vcpu-dirty-limit-period" ranges from > 500ms to 1000 ms. "x-vcpu-dirty-limit-period" should be made > stable once it proves best value can not be determined with > developer's experiments. > > Signed-off-by: Hyman Huang(黄勇) > Reviewed-by: Markus Armbruster > Reviewed-by: Juan Quintela This commit message wordsmithing doesn't justify yet another respin by itself. But if you need to respin the series for some other reason, please consider my suggestions.
Re: [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries
On 7/21/23 17:14, Michael Tokarev wrote: 19.07.2023 18:52, Helge Deller wrote: qemu-user crashes immediately when running static binaries on the armhf architecture. The problem is the memory layout where the executable is loaded before the interpreter library, in which case the reserved brk region clashes with the interpreter code and is released before qemu tries to start the program. At load time qemu calculates a brk value for interpreter and executable each. The fix is to choose the higher one of both. Signed-off-by: Helge Deller Cc: Andreas Schwab Cc: qemu-sta...@nongnu.org Reported-by: venkata.p...@toshiba-tsip.com Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981 --- linux-user/elfload.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index a26200d9f3..94951630b1 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -3615,6 +3615,13 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) if (elf_interpreter) { load_elf_interp(elf_interpreter, &interp_info, bprm->buf); + /* + * adjust brk address if the interpreter was loaded above the main + * executable, e.g. happens with static binaries on armhf + */ + if (interp_info.brk > info->brk) { + info->brk = interp_info.brk; + } So, this is kinda amusing. This broke arm64, ppc64el and s390x: arm64$ ./qemu-aarch64 /bin/sh -c '/bin/ls -dCFl *[t]* >/dev/null' qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault (it was just a quick test from debian qemu-user package). Reverting this patch makes it work again.. It seems the whole loading of binaries on aarch64 is somewhat wrong. My patch just triggers the whole thing to blow up. This is how the memory-map looks on physical hardware: deller@amdahl:~$ uname -a Linux amdahl 5.10.0-23-arm64 #1 SMP Debian 5.10.179-2 (2023-07-14) aarch64 GNU/Linux deller@amdahl:~$ cat /proc/self/maps afb7-afb78000 r-xp fe:02 131743 /bin/cat afb88000-afb89000 r--p 8000 fe:02 131743 /bin/cat afb89000-afb8a000 rw-p 9000 fe:02 131743 /bin/cat e022b000-e024c000 rw-p 00:00 0 [heap] 9ce78000-9ce9a000 rw-p 00:00 0 9ce9a000-9cff6000 r-xp fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so 9cff6000-9d005000 ---p 0015c000 fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so 9d005000-9d009000 r--p 0015b000 fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so 9d009000-9d00b000 rw-p 0015f000 fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so 9d00b000-9d00e000 rw-p 00:00 0 9d00e000-9d02f000 r-xp fe:02 790820 /lib/aarch64-linux-gnu/ld-2.31.so 9d033000-9d035000 rw-p 00:00 0 9d03c000-9d03e000 r--p 00:00 0 [vvar] 9d03e000-9d03f000 r-xp 00:00 0 [vdso] 9d03f000-9d04 r--p 00021000 fe:02 790820 /lib/aarch64-linux-gnu/ld-2.31.so 9d04-9d042000 rw-p 00022000 fe:02 790820 /lib/aarch64-linux-gnu/ld-2.31.so e9ea3000-e9ec4000 rw-p 00:00 0 [stack] this is on qemu-linux-user-aarch64: I have no name!@paq:/# cat /proc/self/maps 55-559000 r-xp 08:01 2380521 /usr/bin/cat 559000-550001f000 ---p 00:00 0 550001f000-550002 r--p f000 08:01 2380521 /usr/bin/cat 550002-5500021000 rw-p 0001 08:01 2380521 /usr/bin/cat 5500021000-5500042000 rw-p 00:00 0 5502021000-5502022000 ---p 00:00 0 5502022000-5502822000 rw-p 00:00 0 [stack] 5502822000-5502848000 r-xp 08:01 2382563 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 5502848000-550286 ---p 00:00 0 550286-5502862000 r--p 0002e000 08:01 2382563 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 5502862000-5502864000 rw-p 0003 08:01 2382563 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 5502864000-5502865000 r-xp 00:00 0 5502865000-5502867000 rw-p 00:00 0 550287-55029f7000 r-xp 08:01 2382566 /usr/lib/aarch64-linux-gnu/libc.so.6 55029f7000-5502a0d000 ---p 00187000 08:01 2382566 /usr/lib/aarch64-linux-gnu/libc.so.6 5502a0d000-5502a1 r--p 0018d000 08:01 2382566 /usr/lib/aarch64-linux-gnu/libc.so.6 5502a1-5502a12000 rw-p 0019 08:01 2382566 /usr/lib/aarch64-linux-g
Re: [PATCH] kvm: Remove KVM_CREATE_IRQCHIP support assumption
On 22/07/2023 08.21, Andrew Jones wrote: Since Linux commit 00f918f61c56 ("RISC-V: KVM: Skeletal in-kernel AIA irqchip support") checking KVM_CAP_IRQCHIP returns non-zero when the RISC-V platform has AIA. The cap indicates KVM supports at least one of the following ioctls: KVM_CREATE_IRQCHIP KVM_IRQ_LINE KVM_GET_IRQCHIP KVM_SET_IRQCHIP KVM_GET_LAPIC KVM_SET_LAPIC but the cap doesn't imply that KVM must support any of those ioctls in particular. However, QEMU was assuming the KVM_CREATE_IRQCHIP ioctl was supported. Stop making that assumption by introducing a KVM parameter that each architecture which supports KVM_CREATE_IRQCHIP sets. Adding parameters isn't awesome, but given how the KVM_CAP_IRQCHIP isn't very helpful on its own, we don't have a lot of options. Signed-off-by: Andrew Jones --- While this fixes booting guests on riscv KVM with AIA it's unlikely to get merged before the QEMU support for KVM AIA[1] lands, which would also fix the issue. I think this patch is still worth considering though since QEMU's assumption is wrong. [1] https://lore.kernel.org/all/20230714084429.22349-1-yongxuan.w...@sifive.com/ accel/kvm/kvm-all.c| 5 - include/sysemu/kvm.h | 1 + target/arm/kvm.c | 3 +++ target/i386/kvm/kvm.c | 2 ++ target/s390x/kvm/kvm.c | 3 +++ 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 373d876c0580..0f5ff8630502 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -86,6 +86,7 @@ struct KVMParkedVcpu { }; KVMState *kvm_state; +bool kvm_has_create_irqchip; bool kvm_kernel_irqchip; bool kvm_split_irqchip; bool kvm_async_interrupts_allowed; @@ -2377,8 +2378,10 @@ static void kvm_irqchip_create(KVMState *s) if (s->kernel_irqchip_split == ON_OFF_AUTO_ON) { error_report("Split IRQ chip mode not supported."); exit(1); -} else { +} else if (kvm_has_create_irqchip) { ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP); +} else { +return; } } if (ret < 0) { I think I'd do this differntly... at the beginning of the function, there is a check for kvm_check_extension(s, KVM_CAP_IRQCHIP) etc. ... I think you could now replace that check with a simple if (!kvm_has_create_irqchip) { return; } The "kvm_vm_enable_cap(s, KVM_CAP_S390_IRQCHIP, 0)" of course has to be moved to the target/s390x/kvm/kvm.c file, too. Thomas diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 115f0cca79d1..84b1bb3dc91e 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -32,6 +32,7 @@ #ifdef CONFIG_KVM_IS_POSSIBLE extern bool kvm_allowed; +extern bool kvm_has_create_irqchip; extern bool kvm_kernel_irqchip; extern bool kvm_split_irqchip; extern bool kvm_async_interrupts_allowed; diff --git a/target/arm/kvm.c b/target/arm/kvm.c index b4c7654f4980..2fa87b495d68 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -250,6 +250,9 @@ int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa) int kvm_arch_init(MachineState *ms, KVMState *s) { int ret = 0; + +kvm_has_create_irqchip = kvm_check_extension(s, KVM_CAP_IRQCHIP); + /* For ARM interrupt delivery is always asynchronous, * whether we are using an in-kernel VGIC or not. */ diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index ebfaf3d24c79..6363e67f092d 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -2771,6 +2771,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s) } } +kvm_has_create_irqchip = kvm_check_extension(s, KVM_CAP_IRQCHIP); + return 0; } diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c index a9e5880349d9..c053304adf94 100644 --- a/target/s390x/kvm/kvm.c +++ b/target/s390x/kvm/kvm.c @@ -391,6 +391,9 @@ int kvm_arch_init(MachineState *ms, KVMState *s) } kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES); + +kvm_has_create_irqchip = kvm_check_extension(s, KVM_CAP_S390_IRQCHIP); + return 0; }
Re: [PATCH v1 0/9] gfxstream + rutabaga_gfx
Gurchetan Singh writes: > In terms of API stability/versioning/packaging, once this series is > reviewed, the plan is to cut a "gfxstream upstream release branch". We > will have the same API guarantees as any other QEMU project then, i.e no > breaking API changes for 5 years. What about Rutabaga? signature.asc Description: PGP signature
Re: [PATCH] hw: Add compat machines for 8.2
On 7/18/23 16:22, Cornelia Huck wrote: Add 8.2 machine types for arm/i440fx/m68k/q35/s390x/spapr. Signed-off-by: Cornelia Huck --- hw/arm/virt.c | 9 - hw/core/machine.c | 3 +++ hw/i386/pc.c | 3 +++ hw/i386/pc_piix.c | 16 +--- hw/i386/pc_q35.c | 14 -- hw/m68k/virt.c | 9 - hw/ppc/spapr.c | 15 +-- hw/s390x/s390-virtio-ccw.c | 14 +- include/hw/boards.h| 3 +++ include/hw/i386/pc.h | 3 +++ 10 files changed, 79 insertions(+), 10 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 7d9dbc26633a..2a560271b5fc 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -3170,10 +3170,17 @@ static void machvirt_machine_init(void) } type_init(machvirt_machine_init); +static void virt_machine_8_2_options(MachineClass *mc) +{ +} +DEFINE_VIRT_MACHINE_AS_LATEST(8, 2) + static void virt_machine_8_1_options(MachineClass *mc) { +virt_machine_8_2_options(mc); +compat_props_add(mc->compat_props, hw_compat_8_1, hw_compat_8_1_len); } -DEFINE_VIRT_MACHINE_AS_LATEST(8, 1) +DEFINE_VIRT_MACHINE(8, 1) static void virt_machine_8_0_options(MachineClass *mc) { diff --git a/hw/core/machine.c b/hw/core/machine.c index f0d35c640184..da699cf4e147 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -39,6 +39,9 @@ #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-pci.h" +GlobalProperty hw_compat_8_1[] = {}; +const size_t hw_compat_8_1_len = G_N_ELEMENTS(hw_compat_8_1); + GlobalProperty hw_compat_8_0[] = { { "migration", "multifd-flush-after-each-section", "on"}, { TYPE_PCI_DEVICE, "x-pcie-ari-nextfn-1", "on" }, diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 3109d5e0e035..54838c0c411d 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -114,6 +114,9 @@ { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\ { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, }, +GlobalProperty pc_compat_8_1[] = {}; +const size_t pc_compat_8_1_len = G_N_ELEMENTS(pc_compat_8_1); + GlobalProperty pc_compat_8_0[] = { { "virtio-mem", "unplugged-inaccessible", "auto" }, }; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ac72e8f5bee1..ce1ac9527493 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -504,13 +504,25 @@ static void pc_i440fx_machine_options(MachineClass *m) machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE); } -static void pc_i440fx_8_1_machine_options(MachineClass *m) +static void pc_i440fx_8_2_machine_options(MachineClass *m) { pc_i440fx_machine_options(m); m->alias = "pc"; m->is_default = true; } +DEFINE_I440FX_MACHINE(v8_2, "pc-i440fx-8.2", NULL, + pc_i440fx_8_2_machine_options); + +static void pc_i440fx_8_1_machine_options(MachineClass *m) +{ +pc_i440fx_8_2_machine_options(m); +m->alias = NULL; +m->is_default = false; +compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len); +compat_props_add(m->compat_props, pc_compat_8_1, pc_compat_8_1_len); +} + DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL, pc_i440fx_8_1_machine_options); @@ -519,8 +531,6 @@ static void pc_i440fx_8_0_machine_options(MachineClass *m) PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_i440fx_8_1_machine_options(m); -m->alias = NULL; -m->is_default = false; compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len); compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index dc27a9e223a2..37c4814bedf2 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -379,12 +379,23 @@ static void pc_q35_machine_options(MachineClass *m) machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE); } -static void pc_q35_8_1_machine_options(MachineClass *m) +static void pc_q35_8_2_machine_options(MachineClass *m) { pc_q35_machine_options(m); m->alias = "q35"; } +DEFINE_Q35_MACHINE(v8_2, "pc-q35-8.2", NULL, + pc_q35_8_2_machine_options); + +static void pc_q35_8_1_machine_options(MachineClass *m) +{ +pc_q35_8_2_machine_options(m); +m->alias = NULL; +compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len); +compat_props_add(m->compat_props, pc_compat_8_1, pc_compat_8_1_len); +} + DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL, pc_q35_8_1_machine_options); @@ -393,7 +404,6 @@ static void pc_q35_8_0_machine_options(MachineClass *m) PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_q35_8_1_machine_options(m); -m->alias = NULL; compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len); compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len); diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c in
[PATCH] Allow UNIX socket option for VNC websocket
- Remove unix socket option limitation for VNC websocket - Reflect websocket option changes in documentation Signed-off-by: Sergii Zasenko --- qemu-options.hx | 4 ui/vnc.c| 5 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 29b98c3..8cc910d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2451,6 +2451,10 @@ SRST host. It is possible to control the websocket listen address independently, using the syntax ``websocket``\ =host:port. +Websocket could be allowed over UNIX domain socket, using the syntax +``websocket``\ =unix:path, where path is the location of a unix socket +to listen for connections on. + If no TLS credentials are provided, the websocket connection runs in unencrypted mode. If TLS credentials are provided, the websocket connection requires encrypted client connections. diff --git a/ui/vnc.c b/ui/vnc.c index 92964dc..dea1414 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char *addrstr, addr->type = SOCKET_ADDRESS_TYPE_UNIX; addr->u.q_unix.path = g_strdup(addrstr + 5); -if (websocket) { -error_setg(errp, "UNIX sockets not supported with websock"); -goto cleanup; -} - if (to) { error_setg(errp, "Port range not support with UNIX socket"); goto cleanup; -- 2.39.2
Re: [PATCH] Allow UNIX socket option for VNC websocket
On Mon, Jul 24, 2023 at 2:04 PM Sergii Zasenko wrote: > - Remove unix socket option limitation for VNC websocket > - Reflect websocket option changes in documentation > > Signed-off-by: Sergii Zasenko > Reviewed-by: Marc-André Lureau > --- > qemu-options.hx | 4 > ui/vnc.c| 5 - > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index 29b98c3..8cc910d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2451,6 +2451,10 @@ SRST > host. It is possible to control the websocket listen address > independently, using the syntax ``websocket``\ =host:port. > > +Websocket could be allowed over UNIX domain socket, using the > syntax > +``websocket``\ =unix:path, where path is the location of a unix > socket > +to listen for connections on. > + > If no TLS credentials are provided, the websocket connection > runs in unencrypted mode. If TLS credentials are provided, the > websocket connection requires encrypted client connections. > diff --git a/ui/vnc.c b/ui/vnc.c > index 92964dc..dea1414 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char > *addrstr, > addr->type = SOCKET_ADDRESS_TYPE_UNIX; > addr->u.q_unix.path = g_strdup(addrstr + 5); > > -if (websocket) { > -error_setg(errp, "UNIX sockets not supported with websock"); > -goto cleanup; > -} > - > if (to) { > error_setg(errp, "Port range not support with UNIX socket"); > goto cleanup; > -- > 2.39.2 > > > -- Marc-André Lureau
[PATCH v4 0/9] Misc fixes for throttle
v3 -> v4: - Hanna pointed out that 'throttle type' is not clear enough, 'throttle direction' would be better in the v3. Use 'ThrottleDirection' instead, also rename 'ThrottleType throttle' to 'ThrottleDirection direction'. - For patch 'throttle: support read-only and write-only', reduce codes by: for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) { ... } - Add commit message for the removed 'FIXME' tag. - Append 'throttle: use THROTTLE_MAX/ARRAY_SIZE for hard code' - Append 'fsdev: Use ThrottleDirection instread of bool is_write' - Append 'block/throttle-groups: Use ThrottleDirection instread of bool is_write' Finally, 'bool is_write' has been fully removed from throttle related codes, 'type foo[2]' becomes 'type foo[THROTTLE_MAX]'. v2 -> v3: - patch 1 -> patch 5 are already reviewed by Alberto - append patch 6: throttle: use enum ThrottleType instead of bool is_write v1 -> v2: - rename 'ThrottleTimerType' to 'ThrottleType' - add assertion to throttle_schedule_timer v1: - introduce enum ThrottleTimerType instead of timers[0], timer[1]... - support read-only and write-only for throttle - adapt related test codes - cryptodev uses a write-only throttle timer Zhenwei Pi (9): throttle: introduce enum ThrottleDirection test-throttle: use enum ThrottleDirection throttle: support read-only and write-only test-throttle: test read only and write only cryptodev: use NULL throttle timer cb for read direction throttle: use enum ThrottleDirection instead of bool is_write throttle: use THROTTLE_MAX/ARRAY_SIZE for hard code fsdev: Use ThrottleDirection instread of bool is_write block/throttle-groups: Use ThrottleDirection instread of bool is_write backends/cryptodev.c| 12 ++-- block/throttle-groups.c | 107 block/throttle.c| 8 +-- fsdev/qemu-fsdev-throttle.c | 18 +++--- fsdev/qemu-fsdev-throttle.h | 4 +- hw/9pfs/cofile.c| 4 +- include/block/throttle-groups.h | 6 +- include/qemu/throttle.h | 16 +++-- tests/unit/test-throttle.c | 76 +-- util/throttle.c | 84 +++-- 10 files changed, 216 insertions(+), 119 deletions(-) -- 2.34.1
[PATCH v4 5/9] cryptodev: use NULL throttle timer cb for read direction
Operations on a cryptodev are considered as *write* only, the callback of read direction is never invoked. Use NULL instead of an unreachable path(cryptodev_backend_throttle_timer_cb on read direction). The dummy read timer(never invoked) is already removed here, it means that the 'FIXME' tag is no longer needed. Reviewed-by: Alberto Garcia Reviewed-by: Hanna Czenczek Cc: Gonglei Signed-off-by: zhenwei pi --- backends/cryptodev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backends/cryptodev.c b/backends/cryptodev.c index 7d29517843..5cfa25c61c 100644 --- a/backends/cryptodev.c +++ b/backends/cryptodev.c @@ -331,8 +331,7 @@ static void cryptodev_backend_set_throttle(CryptoDevBackend *backend, int field, if (!enabled) { throttle_init(&backend->ts); throttle_timers_init(&backend->tt, qemu_get_aio_context(), - QEMU_CLOCK_REALTIME, - cryptodev_backend_throttle_timer_cb, /* FIXME */ + QEMU_CLOCK_REALTIME, NULL, cryptodev_backend_throttle_timer_cb, backend); } -- 2.34.1
[PATCH v4 1/9] throttle: introduce enum ThrottleDirection
Use enum ThrottleDirection instead of number index. Reviewed-by: Alberto Garcia Reviewed-by: Hanna Czenczek Signed-off-by: zhenwei pi --- include/qemu/throttle.h | 11 --- util/throttle.c | 16 +--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 05f6346137..9ca5ab8197 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -99,13 +99,18 @@ typedef struct ThrottleState { int64_t previous_leak;/* timestamp of the last leak done */ } ThrottleState; +typedef enum { +THROTTLE_READ = 0, +THROTTLE_WRITE, +THROTTLE_MAX +} ThrottleDirection; + typedef struct ThrottleTimers { -QEMUTimer *timers[2]; /* timers used to do the throttling */ +QEMUTimer *timers[THROTTLE_MAX];/* timers used to do the throttling */ QEMUClockType clock_type; /* the clock used */ /* Callbacks */ -QEMUTimerCB *read_timer_cb; -QEMUTimerCB *write_timer_cb; +QEMUTimerCB *timer_cb[THROTTLE_MAX]; void *timer_opaque; } ThrottleTimers; diff --git a/util/throttle.c b/util/throttle.c index 81f247a8d1..5642e61763 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -199,10 +199,12 @@ static bool throttle_compute_timer(ThrottleState *ts, void throttle_timers_attach_aio_context(ThrottleTimers *tt, AioContext *new_context) { -tt->timers[0] = aio_timer_new(new_context, tt->clock_type, SCALE_NS, - tt->read_timer_cb, tt->timer_opaque); -tt->timers[1] = aio_timer_new(new_context, tt->clock_type, SCALE_NS, - tt->write_timer_cb, tt->timer_opaque); +tt->timers[THROTTLE_READ] = +aio_timer_new(new_context, tt->clock_type, SCALE_NS, + tt->timer_cb[THROTTLE_READ], tt->timer_opaque); +tt->timers[THROTTLE_WRITE] = +aio_timer_new(new_context, tt->clock_type, SCALE_NS, + tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque); } /* @@ -236,8 +238,8 @@ void throttle_timers_init(ThrottleTimers *tt, memset(tt, 0, sizeof(ThrottleTimers)); tt->clock_type = clock_type; -tt->read_timer_cb = read_timer_cb; -tt->write_timer_cb = write_timer_cb; +tt->timer_cb[THROTTLE_READ] = read_timer_cb; +tt->timer_cb[THROTTLE_WRITE] = write_timer_cb; tt->timer_opaque = timer_opaque; throttle_timers_attach_aio_context(tt, aio_context); } @@ -256,7 +258,7 @@ void throttle_timers_detach_aio_context(ThrottleTimers *tt) { int i; -for (i = 0; i < 2; i++) { +for (i = 0; i < THROTTLE_MAX; i++) { throttle_timer_destroy(&tt->timers[i]); } } -- 2.34.1
[PATCH v4 2/9] test-throttle: use enum ThrottleDirection
Use enum ThrottleDirection instead in the throttle test codes. Reviewed-by: Alberto Garcia Reviewed-by: Hanna Czenczek Signed-off-by: zhenwei pi --- tests/unit/test-throttle.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c index 7adb5e6652..a60b5fe22e 100644 --- a/tests/unit/test-throttle.c +++ b/tests/unit/test-throttle.c @@ -169,8 +169,8 @@ static void test_init(void) /* check initialized fields */ g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL); -g_assert(tt->timers[0]); -g_assert(tt->timers[1]); +g_assert(tt->timers[THROTTLE_READ]); +g_assert(tt->timers[THROTTLE_WRITE]); /* check other fields where cleared */ g_assert(!ts.previous_leak); @@ -191,7 +191,7 @@ static void test_destroy(void) throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL, read_timer_cb, write_timer_cb, &ts); throttle_timers_destroy(tt); -for (i = 0; i < 2; i++) { +for (i = 0; i < THROTTLE_MAX; i++) { g_assert(!tt->timers[i]); } } -- 2.34.1
[PATCH v4 6/9] throttle: use enum ThrottleDirection instead of bool is_write
enum ThrottleDirection is already there, use ThrottleDirection instead of 'bool is_write' for throttle API, also modify related codes from block, fsdev, cryptodev and tests. Signed-off-by: zhenwei pi --- backends/cryptodev.c| 9 + block/throttle-groups.c | 6 -- fsdev/qemu-fsdev-throttle.c | 8 +--- include/qemu/throttle.h | 5 +++-- tests/unit/test-throttle.c | 4 ++-- util/throttle.c | 31 +-- 6 files changed, 36 insertions(+), 27 deletions(-) diff --git a/backends/cryptodev.c b/backends/cryptodev.c index 5cfa25c61c..06142eae57 100644 --- a/backends/cryptodev.c +++ b/backends/cryptodev.c @@ -242,10 +242,11 @@ static void cryptodev_backend_throttle_timer_cb(void *opaque) continue; } -throttle_account(&backend->ts, true, ret); +throttle_account(&backend->ts, THROTTLE_WRITE, ret); cryptodev_backend_operation(backend, op_info); if (throttle_enabled(&backend->tc) && -throttle_schedule_timer(&backend->ts, &backend->tt, true)) { +throttle_schedule_timer(&backend->ts, &backend->tt, +THROTTLE_WRITE)) { break; } } @@ -261,7 +262,7 @@ int cryptodev_backend_crypto_operation( goto do_account; } -if (throttle_schedule_timer(&backend->ts, &backend->tt, true) || +if (throttle_schedule_timer(&backend->ts, &backend->tt, THROTTLE_WRITE) || !QTAILQ_EMPTY(&backend->opinfos)) { QTAILQ_INSERT_TAIL(&backend->opinfos, op_info, next); return 0; @@ -273,7 +274,7 @@ do_account: return ret; } -throttle_account(&backend->ts, true, ret); +throttle_account(&backend->ts, THROTTLE_WRITE, ret); return cryptodev_backend_operation(backend, op_info); } diff --git a/block/throttle-groups.c b/block/throttle-groups.c index fb203c3ced..3847d27801 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -270,6 +270,7 @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm, ThrottleState *ts = tgm->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); ThrottleTimers *tt = &tgm->throttle_timers; +ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ; bool must_wait; if (qatomic_read(&tgm->io_limits_disabled)) { @@ -281,7 +282,7 @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm, return true; } -must_wait = throttle_schedule_timer(ts, tt, is_write); +must_wait = throttle_schedule_timer(ts, tt, direction); /* If a timer just got armed, set tgm as the current token */ if (must_wait) { @@ -364,6 +365,7 @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm bool must_wait; ThrottleGroupMember *token; ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts); +ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ; assert(bytes >= 0); @@ -386,7 +388,7 @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm } /* The I/O will be executed, so do the accounting */ -throttle_account(tgm->throttle_state, is_write, bytes); +throttle_account(tgm->throttle_state, direction, bytes); /* Schedule the next request */ schedule_next_request(tgm, is_write); diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c index 5c83a1cc09..1c137d6f0f 100644 --- a/fsdev/qemu-fsdev-throttle.c +++ b/fsdev/qemu-fsdev-throttle.c @@ -97,16 +97,18 @@ void fsdev_throttle_init(FsThrottle *fst) void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write, struct iovec *iov, int iovcnt) { +ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ; + if (throttle_enabled(&fst->cfg)) { -if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write) || +if (throttle_schedule_timer(&fst->ts, &fst->tt, direction) || !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) { qemu_co_queue_wait(&fst->throttled_reqs[is_write], NULL); } -throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt)); +throttle_account(&fst->ts, direction, iov_size(iov, iovcnt)); if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) && -!throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) { +!throttle_schedule_timer(&fst->ts, &fst->tt, direction)) { qemu_co_queue_next(&fst->throttled_reqs[is_write]); } } diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 9ca5ab8197..181245d29b 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -154,9 +154,10 @@ void throttle_config_init(ThrottleConfig *cfg); /* usage */ bool throttle_schedule_timer(ThrottleState *ts,
[PATCH v4 7/9] throttle: use THROTTLE_MAX/ARRAY_SIZE for hard code
The first dimension of both to_check and bucket_types_size/bucket_types_units is used as throttle direction, use THROTTLE_MAX instead of hard coded number. Also use ARRAY_SIZE() to avoid hard coded number for the second dimension. Hanna noticed that the two array should be static. Yes, turn them into static variables. Signed-off-by: zhenwei pi --- util/throttle.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/util/throttle.c b/util/throttle.c index 9a37209bb8..9baa6b8a3a 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -142,7 +142,8 @@ int64_t throttle_compute_wait(LeakyBucket *bkt) static int64_t throttle_compute_wait_for(ThrottleState *ts, ThrottleDirection direction) { -BucketType to_check[2][4] = { {THROTTLE_BPS_TOTAL, +static const BucketType to_check[THROTTLE_MAX][4] = { + {THROTTLE_BPS_TOTAL, THROTTLE_OPS_TOTAL, THROTTLE_BPS_READ, THROTTLE_OPS_READ}, @@ -153,7 +154,7 @@ static int64_t throttle_compute_wait_for(ThrottleState *ts, int64_t wait, max_wait = 0; int i; -for (i = 0; i < 4; i++) { +for (i = 0; i < ARRAY_SIZE(to_check[THROTTLE_READ]); i++) { BucketType index = to_check[direction][i]; wait = throttle_compute_wait(&ts->cfg.buckets[index]); if (wait > max_wait) { @@ -469,11 +470,11 @@ bool throttle_schedule_timer(ThrottleState *ts, void throttle_account(ThrottleState *ts, ThrottleDirection direction, uint64_t size) { -const BucketType bucket_types_size[2][2] = { +static const BucketType bucket_types_size[THROTTLE_MAX][2] = { { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ }, { THROTTLE_BPS_TOTAL, THROTTLE_BPS_WRITE } }; -const BucketType bucket_types_units[2][2] = { +static const BucketType bucket_types_units[THROTTLE_MAX][2] = { { THROTTLE_OPS_TOTAL, THROTTLE_OPS_READ }, { THROTTLE_OPS_TOTAL, THROTTLE_OPS_WRITE } }; @@ -486,7 +487,7 @@ void throttle_account(ThrottleState *ts, ThrottleDirection direction, units = (double) size / ts->cfg.op_size; } -for (i = 0; i < 2; i++) { +for (i = 0; i < ARRAY_SIZE(bucket_types_size[THROTTLE_READ]); i++) { LeakyBucket *bkt; bkt = &ts->cfg.buckets[bucket_types_size[direction][i]]; -- 2.34.1
[PATCH v4 9/9] block/throttle-groups: Use ThrottleDirection instread of bool is_write
'bool is_write' style is obsolete from throttle framework, adapt block throttle groups to the new style. Use a simple python script to test the new style: #!/usr/bin/python3 import subprocess import random import time commands = ['virsh blkdeviotune jammy vda --write-bytes-sec ', \ 'virsh blkdeviotune jammy vda --write-iops-sec ', \ 'virsh blkdeviotune jammy vda --read-bytes-sec ', \ 'virsh blkdeviotune jammy vda --read-iops-sec '] for loop in range(1, 1000): time.sleep(random.randrange(3, 5)) command = commands[random.randrange(0, 3)] + str(random.randrange(0, 100)) subprocess.run(command, shell=True, check=True) This works fine. Signed-off-by: zhenwei pi --- block/throttle-groups.c | 105 block/throttle.c| 8 +-- include/block/throttle-groups.h | 6 +- 3 files changed, 60 insertions(+), 59 deletions(-) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 3847d27801..c7c700fdb6 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -37,7 +37,7 @@ static void throttle_group_obj_init(Object *obj); static void throttle_group_obj_complete(UserCreatable *obj, Error **errp); -static void timer_cb(ThrottleGroupMember *tgm, bool is_write); +static void timer_cb(ThrottleGroupMember *tgm, ThrottleDirection direction); /* The ThrottleGroup structure (with its ThrottleState) is shared * among different ThrottleGroupMembers and it's independent from @@ -73,8 +73,8 @@ struct ThrottleGroup { QemuMutex lock; /* This lock protects the following four fields */ ThrottleState ts; QLIST_HEAD(, ThrottleGroupMember) head; -ThrottleGroupMember *tokens[2]; -bool any_timer_armed[2]; +ThrottleGroupMember *tokens[THROTTLE_MAX]; +bool any_timer_armed[THROTTLE_MAX]; QEMUClockType clock_type; /* This field is protected by the global QEMU mutex */ @@ -197,13 +197,13 @@ static ThrottleGroupMember *throttle_group_next_tgm(ThrottleGroupMember *tgm) * This assumes that tg->lock is held. * * @tgm:the ThrottleGroupMember - * @is_write: the type of operation (read/write) + * @direction: the ThrottleDirection * @ret:whether the ThrottleGroupMember has pending requests. */ static inline bool tgm_has_pending_reqs(ThrottleGroupMember *tgm, -bool is_write) +ThrottleDirection direction) { -return tgm->pending_reqs[is_write]; +return tgm->pending_reqs[direction]; } /* Return the next ThrottleGroupMember in the round-robin sequence with pending @@ -212,12 +212,12 @@ static inline bool tgm_has_pending_reqs(ThrottleGroupMember *tgm, * This assumes that tg->lock is held. * * @tgm: the current ThrottleGroupMember - * @is_write: the type of operation (read/write) + * @direction: the ThrottleDirection * @ret: the next ThrottleGroupMember with pending requests, or tgm if * there is none. */ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm, -bool is_write) +ThrottleDirection direction) { ThrottleState *ts = tgm->throttle_state; ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); @@ -227,16 +227,16 @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm, * it's being drained. Skip the round-robin search and return tgm * immediately if it has pending requests. Otherwise we could be * forcing it to wait for other member's throttled requests. */ -if (tgm_has_pending_reqs(tgm, is_write) && +if (tgm_has_pending_reqs(tgm, direction) && qatomic_read(&tgm->io_limits_disabled)) { return tgm; } -start = token = tg->tokens[is_write]; +start = token = tg->tokens[direction]; /* get next bs round in round robin style */ token = throttle_group_next_tgm(token); -while (token != start && !tgm_has_pending_reqs(token, is_write)) { +while (token != start && !tgm_has_pending_reqs(token, direction)) { token = throttle_group_next_tgm(token); } @@ -244,12 +244,12 @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm, * then decide the token is the current tgm because chances are * the current tgm got the current request queued. */ -if (token == start && !tgm_has_pending_reqs(token, is_write)) { +if (token == start && !tgm_has_pending_reqs(token, direction)) { token = tgm; } /* Either we return the original TGM, or one with pending requests */ -assert(token == tgm || tgm_has_pending_reqs(token, is_write)); +assert(token == tgm || tgm_has_pending_reqs(token, direction)); return token; } @@ -261,16 +261,15 @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm, * This a
[PATCH v4 3/9] throttle: support read-only and write-only
Only one direction is necessary in several scenarios: - a read-only disk - operations on a device are considered as *write* only. For example, encrypt/decrypt/sign/verify operations on a cryptodev use a single *write* timer(read timer callback is defined, but never invoked). Allow a single direction in throttle, this reduces memory, and uplayer does not need a dummy callback any more. Reviewed-by: Alberto Garcia Reviewed-by: Hanna Czenczek Signed-off-by: zhenwei pi --- util/throttle.c | 42 -- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/util/throttle.c b/util/throttle.c index 5642e61763..0439028d21 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -199,12 +199,15 @@ static bool throttle_compute_timer(ThrottleState *ts, void throttle_timers_attach_aio_context(ThrottleTimers *tt, AioContext *new_context) { -tt->timers[THROTTLE_READ] = -aio_timer_new(new_context, tt->clock_type, SCALE_NS, - tt->timer_cb[THROTTLE_READ], tt->timer_opaque); -tt->timers[THROTTLE_WRITE] = -aio_timer_new(new_context, tt->clock_type, SCALE_NS, - tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque); +ThrottleDirection dir; + +for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) { +if (tt->timer_cb[dir]) { +tt->timers[dir] = +aio_timer_new(new_context, tt->clock_type, SCALE_NS, + tt->timer_cb[dir], tt->timer_opaque); +} +} } /* @@ -235,6 +238,7 @@ void throttle_timers_init(ThrottleTimers *tt, QEMUTimerCB *write_timer_cb, void *timer_opaque) { +assert(read_timer_cb || write_timer_cb); memset(tt, 0, sizeof(ThrottleTimers)); tt->clock_type = clock_type; @@ -247,7 +251,9 @@ void throttle_timers_init(ThrottleTimers *tt, /* destroy a timer */ static void throttle_timer_destroy(QEMUTimer **timer) { -assert(*timer != NULL); +if (*timer == NULL) { +return; +} timer_free(*timer); *timer = NULL; @@ -256,10 +262,10 @@ static void throttle_timer_destroy(QEMUTimer **timer) /* Remove timers from event loop */ void throttle_timers_detach_aio_context(ThrottleTimers *tt) { -int i; +ThrottleDirection dir; -for (i = 0; i < THROTTLE_MAX; i++) { -throttle_timer_destroy(&tt->timers[i]); +for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) { +throttle_timer_destroy(&tt->timers[dir]); } } @@ -272,8 +278,12 @@ void throttle_timers_destroy(ThrottleTimers *tt) /* is any throttling timer configured */ bool throttle_timers_are_initialized(ThrottleTimers *tt) { -if (tt->timers[0]) { -return true; +ThrottleDirection dir; + +for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) { +if (tt->timers[dir]) { +return true; +} } return false; @@ -424,8 +434,12 @@ bool throttle_schedule_timer(ThrottleState *ts, { int64_t now = qemu_clock_get_ns(tt->clock_type); int64_t next_timestamp; +QEMUTimer *timer; bool must_wait; +timer = is_write ? tt->timers[THROTTLE_WRITE] : tt->timers[THROTTLE_READ]; +assert(timer); + must_wait = throttle_compute_timer(ts, is_write, now, @@ -437,12 +451,12 @@ bool throttle_schedule_timer(ThrottleState *ts, } /* request throttled and timer pending -> do nothing */ -if (timer_pending(tt->timers[is_write])) { +if (timer_pending(timer)) { return true; } /* request throttled and timer not pending -> arm timer */ -timer_mod(tt->timers[is_write], next_timestamp); +timer_mod(timer, next_timestamp); return true; } -- 2.34.1
[PATCH v4 8/9] fsdev: Use ThrottleDirection instread of bool is_write
'bool is_write' style is obsolete from throttle framework, adapt fsdev to the new style. Cc: Greg Kurz Signed-off-by: zhenwei pi --- fsdev/qemu-fsdev-throttle.c | 14 +++--- fsdev/qemu-fsdev-throttle.h | 4 ++-- hw/9pfs/cofile.c| 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c index 1c137d6f0f..d912da906d 100644 --- a/fsdev/qemu-fsdev-throttle.c +++ b/fsdev/qemu-fsdev-throttle.c @@ -94,22 +94,22 @@ void fsdev_throttle_init(FsThrottle *fst) } } -void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write, +void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, +ThrottleDirection direction, struct iovec *iov, int iovcnt) { -ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ; - +assert(direction < THROTTLE_MAX); if (throttle_enabled(&fst->cfg)) { if (throttle_schedule_timer(&fst->ts, &fst->tt, direction) || -!qemu_co_queue_empty(&fst->throttled_reqs[is_write])) { -qemu_co_queue_wait(&fst->throttled_reqs[is_write], NULL); +!qemu_co_queue_empty(&fst->throttled_reqs[direction])) { +qemu_co_queue_wait(&fst->throttled_reqs[direction], NULL); } throttle_account(&fst->ts, direction, iov_size(iov, iovcnt)); -if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) && +if (!qemu_co_queue_empty(&fst->throttled_reqs[direction]) && !throttle_schedule_timer(&fst->ts, &fst->tt, direction)) { -qemu_co_queue_next(&fst->throttled_reqs[is_write]); +qemu_co_queue_next(&fst->throttled_reqs[direction]); } } } diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h index a21aecddc7..daa8ca2494 100644 --- a/fsdev/qemu-fsdev-throttle.h +++ b/fsdev/qemu-fsdev-throttle.h @@ -23,14 +23,14 @@ typedef struct FsThrottle { ThrottleState ts; ThrottleTimers tt; ThrottleConfig cfg; -CoQueue throttled_reqs[2]; +CoQueue throttled_reqs[THROTTLE_MAX]; } FsThrottle; int fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *, Error **); void fsdev_throttle_init(FsThrottle *); -void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool , +void coroutine_fn fsdev_co_throttle_request(FsThrottle *, ThrottleDirection , struct iovec *, int); void fsdev_throttle_cleanup(FsThrottle *); diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c index 9c5344039e..71174c3e4a 100644 --- a/hw/9pfs/cofile.c +++ b/hw/9pfs/cofile.c @@ -252,7 +252,7 @@ int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp, if (v9fs_request_cancelled(pdu)) { return -EINTR; } -fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt); +fsdev_co_throttle_request(s->ctx.fst, THROTTLE_WRITE, iov, iovcnt); v9fs_co_run_in_worker( { err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset); @@ -272,7 +272,7 @@ int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp, if (v9fs_request_cancelled(pdu)) { return -EINTR; } -fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt); +fsdev_co_throttle_request(s->ctx.fst, THROTTLE_READ, iov, iovcnt); v9fs_co_run_in_worker( { err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset); -- 2.34.1
[PATCH v4 4/9] test-throttle: test read only and write only
Reviewed-by: Alberto Garcia Reviewed-by: Hanna Czenczek Signed-off-by: zhenwei pi --- tests/unit/test-throttle.c | 66 ++ 1 file changed, 66 insertions(+) diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c index a60b5fe22e..5547837a58 100644 --- a/tests/unit/test-throttle.c +++ b/tests/unit/test-throttle.c @@ -184,6 +184,70 @@ static void test_init(void) throttle_timers_destroy(tt); } +static void test_init_readonly(void) +{ +int i; + +tt = &tgm.throttle_timers; + +/* fill the structures with crap */ +memset(&ts, 1, sizeof(ts)); +memset(tt, 1, sizeof(*tt)); + +/* init structures */ +throttle_init(&ts); +throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL, + read_timer_cb, NULL, &ts); + +/* check initialized fields */ +g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL); +g_assert(tt->timers[THROTTLE_READ]); +g_assert(!tt->timers[THROTTLE_WRITE]); + +/* check other fields where cleared */ +g_assert(!ts.previous_leak); +g_assert(!ts.cfg.op_size); +for (i = 0; i < BUCKETS_COUNT; i++) { +g_assert(!ts.cfg.buckets[i].avg); +g_assert(!ts.cfg.buckets[i].max); +g_assert(!ts.cfg.buckets[i].level); +} + +throttle_timers_destroy(tt); +} + +static void test_init_writeonly(void) +{ +int i; + +tt = &tgm.throttle_timers; + +/* fill the structures with crap */ +memset(&ts, 1, sizeof(ts)); +memset(tt, 1, sizeof(*tt)); + +/* init structures */ +throttle_init(&ts); +throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL, + NULL, write_timer_cb, &ts); + +/* check initialized fields */ +g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL); +g_assert(!tt->timers[THROTTLE_READ]); +g_assert(tt->timers[THROTTLE_WRITE]); + +/* check other fields where cleared */ +g_assert(!ts.previous_leak); +g_assert(!ts.cfg.op_size); +for (i = 0; i < BUCKETS_COUNT; i++) { +g_assert(!ts.cfg.buckets[i].avg); +g_assert(!ts.cfg.buckets[i].max); +g_assert(!ts.cfg.buckets[i].level); +} + +throttle_timers_destroy(tt); +} + static void test_destroy(void) { int i; @@ -752,6 +816,8 @@ int main(int argc, char **argv) g_test_add_func("/throttle/leak_bucket",test_leak_bucket); g_test_add_func("/throttle/compute_wait", test_compute_wait); g_test_add_func("/throttle/init", test_init); +g_test_add_func("/throttle/init_readonly", test_init_readonly); +g_test_add_func("/throttle/init_writeonly", test_init_writeonly); g_test_add_func("/throttle/destroy",test_destroy); g_test_add_func("/throttle/have_timer", test_have_timer); g_test_add_func("/throttle/detach_attach", test_detach_attach); -- 2.34.1
Re: [PATCH v21 01/20] s390x/cpu topology: add s390 specifics to CPU topology
On Fri, 2023-07-21 at 13:24 +0200, Pierre Morel wrote: > > On 7/18/23 18:31, Nina Schoetterl-Glausch wrote: > > Reviewed-by: Nina Schoetterl-Glausch > > > > Some notes below. > > > > The s390x/ prefix in the title might suggest that this patch > > is s390 specific, but it touches common files. > > > Right. > > What do you suggest? > > I can cut it in two or squash it with patch number 2. > > The first idea was to separate the patch to ease the review but the > functionality introduced in patch 1 do only make sense with patch 2. > > So I would be for squashing the first two patches. > > ? Oh, I'd just change the title. CPU topology: extend with s390 specifics or similar, it was just a nit not to create the impression that the patch only touches s390 stuff. > > > > > > On Fri, 2023-06-30 at 11:17 +0200, Pierre Morel wrote: > > > S390 adds two new SMP levels, drawers and books to the CPU > > > topology. > > > The S390 CPU have specific topology features like dedication > > S390 CPUs have specific topology features like dedication and > > entitlement. These indicate to the guest information on host > > vCPU scheduling and help the guest make better scheduling > > decisions. > > > > > and entitlement to give to the guest indications on the host > > > vCPUs scheduling and help the guest take the best decisions > > > on the scheduling of threads on the vCPUs. > > > > > > Let us provide the SMP properties with books and drawers levels > > > and S390 CPU with dedication and entitlement, > > > > > > Signed-off-by: Pierre Morel > > > --- > > > qapi/machine-common.json | 22 + > > > qapi/machine.json | 21 ++--- > > > include/hw/boards.h | 10 +- > > > include/hw/qdev-properties-system.h | 4 +++ > > > target/s390x/cpu.h | 6 > > > hw/core/machine-smp.c | 48 > > > --- > > > -- > > > hw/core/machine.c | 4 +++ > > > hw/core/qdev-properties-system.c | 13 > > > hw/s390x/s390-virtio-ccw.c | 2 ++ > > > softmmu/vl.c | 6 > > > target/s390x/cpu.c | 7 + > > > qapi/meson.build | 1 + > > > qemu-options.hx | 7 +++-- > > > 13 files changed, 137 insertions(+), 14 deletions(-) > > > create mode 100644 qapi/machine-common.json > > > > > > diff --git a/qapi/machine-common.json b/qapi/machine-common.json > > > new file mode 100644 > > > index 00..bc0d76829c > > > --- /dev/null > > > +++ b/qapi/machine-common.json > > > @@ -0,0 +1,22 @@ > > > +# -*- Mode: Python -*- > > > +# vim: filetype=python > > > +# > > > +# This work is licensed under the terms of the GNU GPL, version > > > 2 or > > > later. > > > +# See the COPYING file in the top-level directory. > > > + > > > +## > > > +# = Machines S390 data types > > Common definitions for machine.json and machine-target.json > > > > > > [...]
Re: [PATCH] hw: Add compat machines for 8.2
On 18/07/2023 16.22, Cornelia Huck wrote: Add 8.2 machine types for arm/i440fx/m68k/q35/s390x/spapr. Signed-off-by: Cornelia Huck --- hw/arm/virt.c | 9 - hw/core/machine.c | 3 +++ hw/i386/pc.c | 3 +++ hw/i386/pc_piix.c | 16 +--- hw/i386/pc_q35.c | 14 -- hw/m68k/virt.c | 9 - hw/ppc/spapr.c | 15 +-- hw/s390x/s390-virtio-ccw.c | 14 +- include/hw/boards.h| 3 +++ include/hw/i386/pc.h | 3 +++ 10 files changed, 79 insertions(+), 10 deletions(-) ... diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 4516d73ff5fc..c52a1fcf6700 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -828,14 +828,26 @@ bool css_migration_enabled(void) } \ type_init(ccw_machine_register_##suffix) +static void ccw_machine_8_2_instance_options(MachineState *machine) +{ +} + +static void ccw_machine_8_2_class_options(MachineClass *mc) +{ +} +DEFINE_CCW_MACHINE(8_2, "8.2", true); + static void ccw_machine_8_1_instance_options(MachineState *machine) { +ccw_machine_8_2_instance_options(machine); } static void ccw_machine_8_1_class_options(MachineClass *mc) { +ccw_machine_8_2_class_options(mc); +compat_props_add(mc->compat_props, hw_compat_8_1, hw_compat_8_1_len); } -DEFINE_CCW_MACHINE(8_1, "8.1", true); +DEFINE_CCW_MACHINE(8_1, "8.1", false); static void ccw_machine_8_0_instance_options(MachineState *machine) { s390 part: Reviewed-by: Thomas Huth
Re: [PULL 0/2] xen-virtio-1-tag
On Sat, 22 Jul 2023 at 02:10, Stefano Stabellini wrote: > > The following changes since commit d1181d29370a4318a9f11ea92065bea6bb159f83: > > Merge tag 'pull-nbd-2023-07-19' of https://repo.or.cz/qemu/ericb into > staging (2023-07-20 09:54:07 +0100) > > are available in the Git repository at: > > https://gitlab.com/sstabellini/qemu.git xen-virtio-1-tag > > for you to fetch changes up to 6bb48c66946dfbd653f06ad5f3fc957972333b56: > > xen_arm: Initialize RAM and add hi/low memory regions (2023-07-21 18:00:29 > -0700) > > > Oleksandr Tyshchenko (2): > xen_arm: Create virtio-mmio devices during initialization > xen_arm: Initialize RAM and add hi/low memory regions > > hw/arm/xen_arm.c | 80 > > 1 file changed, 80 insertions(+) Fails to build, multiple targets: https://gitlab.com/qemu-project/qemu/-/jobs/4726472678 https://gitlab.com/qemu-project/qemu/-/jobs/4726472642 etc ../hw/arm/xen_arm.c: In function 'xen_set_irq': ../hw/arm/xen_arm.c:78:5: error: implicit declaration of function 'xendevicemodel_set_irq_level'; did you mean 'xendevicemodel_set_isa_irq_level'? [-Werror=implicit-function-declaration] ../hw/arm/xen_arm.c:78:5: error: nested extern declaration of 'xendevicemodel_set_irq_level' [-Werror=nested-externs] ../hw/arm/xen_arm.c: In function 'xen_create_virtio_mmio_devices': ../hw/arm/xen_arm.c:74:5: error: 'GUEST_VIRTIO_MMIO_SPI_LAST' undeclared (first use in this function) 74 | (GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST) | ^~ and others. thanks -- PMM
Re: [PATCH QEMU v9 2/9] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
I missed something... ~hyman writes: > From: Hyman Huang(黄勇) > > Introduce "x-vcpu-dirty-limit-period" migration experimental > parameter, which is in the range of 1 to 1000ms and used to > make dirty page rate calculation period configurable. > > Currently with the "x-vcpu-dirty-limit-period" varies, the > total time of live migration changes, test results show the > optimal value of "x-vcpu-dirty-limit-period" ranges from > 500ms to 1000 ms. "x-vcpu-dirty-limit-period" should be made > stable once it proves best value can not be determined with > developer's experiments. > > Signed-off-by: Hyman Huang(黄勇) > Reviewed-by: Markus Armbruster > Reviewed-by: Juan Quintela [...] > diff --git a/qapi/migration.json b/qapi/migration.json > index 47dfef0278..363055d252 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -789,9 +789,14 @@ > # Nodes are mapped to their block device name if there is one, and > # to their node name otherwise. (Since 5.2) > # > +# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty > +# limit during live migration. Should be in the range 1 to 1000ms, > +# defaults to 1000ms. (Since 8.1) You need to adjust all the "Since" tags to 8.2. [...]
Re: [PATCH v3 1/2] kvm: Introduce kvm_arch_get_default_type hook
On 22/7/23 08:22, Akihiko Odaki wrote: kvm_arch_get_default_type() returns the default KVM type. This hook is particularly useful to derive a KVM type that is valid for "none" machine model, which is used by libvirt to probe the availability of KVM. For MIPS, the existing mips_kvm_type() is reused. This function ensures the availability of VZ which is mandatory to use KVM on the current QEMU. Pre-existing: mips_kvm_type() returns -1. Should we check for 'type' in kvm_init() before calling kvm_ioctl()? Signed-off-by: Akihiko Odaki --- include/sysemu/kvm.h | 2 ++ target/mips/kvm_mips.h | 9 - accel/kvm/kvm-all.c | 4 +++- hw/mips/loongson3_virt.c | 1 - target/arm/kvm.c | 5 + target/i386/kvm/kvm.c| 5 + target/mips/kvm.c| 2 +- target/ppc/kvm.c | 5 + target/riscv/kvm.c | 5 + target/s390x/kvm/kvm.c | 5 + 10 files changed, 31 insertions(+), 12 deletions(-)
Re: [PULL 0/2] QAPI patches patches for 2023-07-10
Markus Armbruster writes: > Did this fall through the cracks? Hmm, looks like Peter is taking care of merging right now.
Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)
Doesn't apply to master. Care to rebase?
Re: [PATCH] tests/avocado/machine_s390_ccw_virtio: Skip the flaky virtio-gpu test by default
On 24/7/23 10:48, Thomas Huth wrote: The virtio-gpu test is known to be flaky - that's why we also did not enable the test_s390x_fedora in the gitlab CI. However, a flaky test can also be annoying when testing locally, so let's rather skip this subtest by default and start running the test_s390x_fedora test in the gitlab CI again (since the other things that are tested here are quite valuable). Signed-off-by: Thomas Huth --- tests/avocado/machine_s390_ccw_virtio.py | 51 +--- 1 file changed, 27 insertions(+), 24 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)
On 24.07.2023 12:56, Markus Armbruster wrote: Doesn't apply to master. Care to rebase? The series is now based on David's virtio-mem-memslots patches (specifically, commit 6769107d1a4f from [1]) since it depends on support for exposing device memory via multiple memslots provided by that series. I'm sorry if that wasn't clear from the cover letter. Thanks, Maciej [1]: https://github.com/davidhildenbrand/qemu/tree/virtio-mem-memslots
Re: [PATCH v4 01/14] target/s390x: Make CKSM raise an exception if R2 is odd
On 24.07.23 10:15, Ilya Leoshkevich wrote: R2 designates an even-odd register pair; the instruction should raise a specification exception when R2 is not even. Cc: qemu-sta...@nongnu.org Fixes: e023e832d0ac ("s390x: translate engine for s390x CPU") Signed-off-by: Ilya Leoshkevich --- Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
Re: [PATCH v6 4/6] qapi: Add HvBalloonDeviceInfo sub-type to MemoryDeviceInfo
"Maciej S. Szmigiero" writes: > From: "Maciej S. Szmigiero" > > Used by the hv-balloon driver to report its provided memory state > information. > > Co-developed-by: David Hildenbrand > Signed-off-by: Maciej S. Szmigiero > --- > hw/core/machine-hmp-cmds.c | 15 +++ > qapi/machine.json | 39 -- > 2 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c > index c3e55ef9e9cd..7b06ed35decb 100644 > --- a/hw/core/machine-hmp-cmds.c > +++ b/hw/core/machine-hmp-cmds.c > @@ -247,6 +247,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict > *qdict) > MemoryDeviceInfo *value; > PCDIMMDeviceInfo *di; > SgxEPCDeviceInfo *se; > +HvBalloonDeviceInfo *hi; > > for (info = info_list; info; info = info->next) { > value = info->value; > @@ -304,6 +305,20 @@ void hmp_info_memory_devices(Monitor *mon, const QDict > *qdict) > monitor_printf(mon, " node: %" PRId64 "\n", se->node); > monitor_printf(mon, " memdev: %s\n", se->memdev); > break; > +case MEMORY_DEVICE_INFO_KIND_HV_BALLOON: This is the only occurence of MEMORY_DEVICE_INFO_KIND_HV_BALLOON at the end of the series. Where are MemoryDeviceInfo with this union tag created? > +hi = value->u.hv_balloon.data; > +monitor_printf(mon, "Memory device [%s]: \"%s\"\n", > + MemoryDeviceInfoKind_str(value->type), > + hi->id ? hi->id : ""); > +if (hi->has_memaddr) { > +monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n", > + hi->memaddr); > +} > +monitor_printf(mon, " max-size: %" PRIu64 "\n", > hi->max_size); > +if (hi->memdev) { > +monitor_printf(mon, " memdev: %s\n", hi->memdev); > +} > +break; > default: > g_assert_not_reached(); > } [...]
Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)
"Maciej S. Szmigiero" writes: > On 24.07.2023 12:56, Markus Armbruster wrote: >> Doesn't apply to master. Care to rebase? >> > > The series is now based on David's virtio-mem-memslots patches > (specifically, commit 6769107d1a4f from [1]) since it depends > on support for exposing device memory via multiple memslots > provided by that series. > > I'm sorry if that wasn't clear from the cover letter. Aha! I just fetched David's branch, applied your patches on top, and rebased to current master. I recommend to list dependencies like Based-on: so Patchew applies them. > Thanks, > Maciej > > [1]: https://github.com/davidhildenbrand/qemu/tree/virtio-mem-memslots
Re: [PATCH v6 4/6] qapi: Add HvBalloonDeviceInfo sub-type to MemoryDeviceInfo
On 24.07.2023 13:37, Markus Armbruster wrote: "Maciej S. Szmigiero" writes: From: "Maciej S. Szmigiero" Used by the hv-balloon driver to report its provided memory state information. Co-developed-by: David Hildenbrand Signed-off-by: Maciej S. Szmigiero --- hw/core/machine-hmp-cmds.c | 15 +++ qapi/machine.json | 39 -- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c index c3e55ef9e9cd..7b06ed35decb 100644 --- a/hw/core/machine-hmp-cmds.c +++ b/hw/core/machine-hmp-cmds.c @@ -247,6 +247,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) MemoryDeviceInfo *value; PCDIMMDeviceInfo *di; SgxEPCDeviceInfo *se; +HvBalloonDeviceInfo *hi; for (info = info_list; info; info = info->next) { value = info->value; @@ -304,6 +305,20 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict) monitor_printf(mon, " node: %" PRId64 "\n", se->node); monitor_printf(mon, " memdev: %s\n", se->memdev); break; +case MEMORY_DEVICE_INFO_KIND_HV_BALLOON: This is the only occurence of MEMORY_DEVICE_INFO_KIND_HV_BALLOON at the end of the series. Where are MemoryDeviceInfo with this union tag created? From patch 6: +static void hv_balloon_md_fill_device_info(const MemoryDeviceState *md, + MemoryDeviceInfo *info) +{ +HvBalloonDeviceInfo *hi = g_new0(HvBalloonDeviceInfo, 1); +const HvBalloon *balloon = HV_BALLOON(md); +DeviceState *dev = DEVICE(md); + +if (dev->id) { +hi->id = g_strdup(dev->id); +} + +if (balloon->hostmem) { +hi->memdev = object_get_canonical_path(OBJECT(balloon->hostmem)); +hi->memaddr = balloon->addr; +hi->has_memaddr = true; +hi->max_size = memory_region_size(balloon->mr); +/* TODO: expose current provided size or something else? */ +} else { +hi->max_size = 0; +} + +info->u.hv_balloon.data = hi; +info->type = MEMORY_DEVICE_INFO_KIND_HV_BALLOON; ^ here. Thanks, Maciej
Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)
On 24.07.2023 13:39, Markus Armbruster wrote: "Maciej S. Szmigiero" writes: On 24.07.2023 12:56, Markus Armbruster wrote: Doesn't apply to master. Care to rebase? The series is now based on David's virtio-mem-memslots patches (specifically, commit 6769107d1a4f from [1]) since it depends on support for exposing device memory via multiple memslots provided by that series. I'm sorry if that wasn't clear from the cover letter. Aha! I just fetched David's branch, applied your patches on top, and rebased to current master. I recommend to list dependencies like Based-on: so Patchew applies them. Didn't know that notation, thanks for point this out. Will use it in the future version(s). [1]: https://github.com/davidhildenbrand/qemu/tree/virtio-mem-memslots Thanks, Maciej
Re: [PATCH for-8.1 v2 1/4] util/interval-tree: Use qatomic_read for left/right while searching
On Sat, 22 Jul 2023 at 22:44, Richard Henderson wrote: > > Fixes a race condition (generally without optimization) in which > the subtree is re-read after the protecting if condition. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH for-8.1 v2 2/4] util/interval-tree: Use qatomic_set_mb in rb_link_node
On Sat, 22 Jul 2023 at 22:44, Richard Henderson wrote: > > Ensure that the stores to rb_left and rb_right are complete before > inserting the new node into the tree. Otherwise a concurrent reader > could see garbage in the new leaf. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH for-8.2? v2 3/4] util/interval-tree: Introduce pc_parent
On Sat, 22 Jul 2023 at 22:44, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > util/interval-tree.c | 13 + > 1 file changed, 9 insertions(+), 4 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH for-8.2? v2 4/4] util/interval-tree: Use qatomic_read/set for rb_parent_color
On Sat, 22 Jul 2023 at 22:44, Richard Henderson wrote: > > While less susceptible to optimization problems than left and right, > interval_tree_iter_next also reads rb_parent(), so make sure that > stores and loads are atomic. > > This goes further than technically required, changing all loads to > be atomic, rather than simply the ones in the iteration side. But > it doesn't really affect the code generation on the rebalance side > and is cleaner to handle everything the same. > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH 0/2] target/riscv: add missing riscv,isa strings
On 7/23/23 23:51, Alistair Francis wrote: On Thu, Jul 20, 2023 at 11:25 PM Daniel Henrique Barboza wrote: Hi, Found these 2 instances while working in more 8.2 material. I believe both are safe for freeze but I won't lose my sleep if we decide to postpone it. I wasn't going to squeeze them into the freeze Daniel Henrique Barboza (2): target/riscv/cpu.c: add zmmul isa string target/riscv/cpu.c: add smepmp isa string Do you mind rebasing :) https://github.com/alistair23/qemu/tree/riscv-to-apply.next :) Thanks! Daniel Alistair target/riscv/cpu.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.41.0
Re: 8.1-rc0 testfloat fails to compile
Sat, 22 Jul 2023 13:49:40 +0100 Richard Henderson : > If this is with optimization enabled, the bug should be reported to gcc > bugzilla. > The compiler should easily prove the default case is unreachable. This happens also with -O0 or without any -On, or without -Wall. https://bugzilla.suse.com/show_bug.cgi?id=1213600 https://gitlab.com/qemu-project/berkeley-testfloat-3/-/merge_requests/2 Olaf pgpetlyLwb7EN.pgp Description: Digitale Signatur von OpenPGP
Re: [PATCH] For curses display, recognize a few more control keys
On Sat, 15 Jul 2023 at 14:31, Sean Estabrooks wrote: > > The curses display handles most control-X keys, and translates > them into their corresponding keycode. Here we recognize > a few that are missing, Ctrl-@ (null), Ctrl-\ (backslash), > Ctrl-] (right bracket), Ctrl-^ (caret), Ctrl-_ (underscore). > > Signed-off-by: Sean Estabrooks > --- > ui/curses_keys.h | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/ui/curses_keys.h b/ui/curses_keys.h > index 71e04acdc7..88a2208ed1 100644 > --- a/ui/curses_keys.h > +++ b/ui/curses_keys.h > @@ -210,6 +210,12 @@ static const int _curses2keycode[CURSES_CHARS] = { > ['N' - '@'] = 49 | CNTRL, /* Control + n */ > /* Control + m collides with the keycode for Enter */ > > +['@' - '@'] = 3 | CNTRL, /* Control + @ */ > +/* Control + [ collides with the keycode for Escape */ > +['\\' - '@'] = 43 | CNTRL, /* Control + Backslash */ > +[']' - '@'] = 27 | CNTRL, /* Control + ] */ > +['^' - '@'] = 7 | CNTRL, /* Control + ^ */ > +['_' - '@'] = 12 | CNTRL, /* Control + Underscore */ > }; > > static const int _curseskey2keycode[CURSES_KEYS] = { > -- > 2.40.1 So, there's already some logic in ui/curses.c that handles control keys generically, and it was put in (or at least modified) way back in commit d03703c81a202ce in 2010 with the commit message saying it was for control-{@[\]^_} : if (chr < ' ') { keysym = chr + '@'; if (keysym >= 'A' && keysym <= 'Z') keysym += 'a' - 'A'; keysym |= KEYSYM_CNTRL; } else keysym = chr; But we only use that code if kbd_layout is set. So I'm not sure how this should work -- are these control-key combinations keyboard layout specific and that's why this is only in that branch of the code? Or are they generic and we can support them in the no-keyboard-layout case too? I did a bit of playing around with my non-standard-keyboard-layout and one of the ncurses-examples test programs, and I think we can do this regardless of keyboard layout. Unfortunately this curses code is (a) pretty old (b) has no active maintainer (c) is not used much. This patch looks like it's OK to me, but I'm guessing a bit. It looks a little odd that we programatically handle control-X in the with-keyboard-layout case but do it data-driven via the array in the without case, but that's the way the code is already written... So I guess Reviewed-by: Peter Maydell This seems like a safe enough small change, so I'm going to take it via target-arm.next, unless somebody else would like more time to review it (or to claim the vacant maintainership of the code ;-)) thanks -- PMM
[PATCH 01/26] migration/multifd: Rename threadinfo.c functions
From: Fabiano Rosas We're about to add more functions to this file so make it use the same coding style as the rest of the code. Signed-off-by: Fabiano Rosas Reviewed-by: Juan Quintela Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Xu Message-Id: <20230607161306.31425-2-faro...@suse.de> Signed-off-by: Juan Quintela --- migration/threadinfo.h | 5 ++--- migration/migration.c | 4 ++-- migration/multifd.c| 4 ++-- migration/threadinfo.c | 4 ++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/migration/threadinfo.h b/migration/threadinfo.h index 4d69423c0a..8aa6999d58 100644 --- a/migration/threadinfo.h +++ b/migration/threadinfo.h @@ -23,6 +23,5 @@ struct MigrationThread { QLIST_ENTRY(MigrationThread) node; }; -MigrationThread *MigrationThreadAdd(const char *name, int thread_id); - -void MigrationThreadDel(MigrationThread *info); +MigrationThread *migration_threads_add(const char *name, int thread_id); +void migration_threads_remove(MigrationThread *info); diff --git a/migration/migration.c b/migration/migration.c index 91bba630a8..ae49d42eab 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2953,7 +2953,7 @@ static void *migration_thread(void *opaque) MigThrError thr_error; bool urgent = false; -thread = MigrationThreadAdd("live_migration", qemu_get_thread_id()); +thread = migration_threads_add("live_migration", qemu_get_thread_id()); rcu_register_thread(); @@ -3031,7 +3031,7 @@ static void *migration_thread(void *opaque) migration_iteration_finish(s); object_unref(OBJECT(s)); rcu_unregister_thread(); -MigrationThreadDel(thread); +migration_threads_remove(thread); return NULL; } diff --git a/migration/multifd.c b/migration/multifd.c index 3387d8277f..4c6cee6547 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -651,7 +651,7 @@ static void *multifd_send_thread(void *opaque) int ret = 0; bool use_zero_copy_send = migrate_zero_copy_send(); -thread = MigrationThreadAdd(p->name, qemu_get_thread_id()); +thread = migration_threads_add(p->name, qemu_get_thread_id()); trace_multifd_send_thread_start(p->id); rcu_register_thread(); @@ -767,7 +767,7 @@ out: qemu_mutex_unlock(&p->mutex); rcu_unregister_thread(); -MigrationThreadDel(thread); +migration_threads_remove(thread); trace_multifd_send_thread_end(p->id, p->num_packets, p->total_normal_pages); return NULL; diff --git a/migration/threadinfo.c b/migration/threadinfo.c index 1de8b31855..3dd9b14ae6 100644 --- a/migration/threadinfo.c +++ b/migration/threadinfo.c @@ -14,7 +14,7 @@ static QLIST_HEAD(, MigrationThread) migration_threads; -MigrationThread *MigrationThreadAdd(const char *name, int thread_id) +MigrationThread *migration_threads_add(const char *name, int thread_id) { MigrationThread *thread = g_new0(MigrationThread, 1); thread->name = name; @@ -25,7 +25,7 @@ MigrationThread *MigrationThreadAdd(const char *name, int thread_id) return thread; } -void MigrationThreadDel(MigrationThread *thread) +void migration_threads_remove(MigrationThread *thread) { if (thread) { QLIST_REMOVE(thread, node); -- 2.40.1
[PATCH 03/26] softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit"
From: Hyman Huang(黄勇) dirty_rate paraemter of hmp command "set_vcpu_dirty_limit" is invalid if less than 0, so add parameter check for it. Note that this patch also delete the unsolicited help message and clean up the code. Signed-off-by: Hyman Huang(黄勇) Reviewed-by: Markus Armbruster Reviewed-by: Peter Xu Reviewed-by: Juan Quintela Message-Id: <168618975839.6361.1740763387474768865...@git.sr.ht> Signed-off-by: Juan Quintela --- softmmu/dirtylimit.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c index 015a9038d1..e80201097a 100644 --- a/softmmu/dirtylimit.c +++ b/softmmu/dirtylimit.c @@ -515,14 +515,15 @@ void hmp_set_vcpu_dirty_limit(Monitor *mon, const QDict *qdict) int64_t cpu_index = qdict_get_try_int(qdict, "cpu_index", -1); Error *err = NULL; +if (dirty_rate < 0) { +error_setg(&err, "invalid dirty page limit %" PRId64, dirty_rate); +goto out; +} + qmp_set_vcpu_dirty_limit(!!(cpu_index != -1), cpu_index, dirty_rate, &err); -if (err) { -hmp_handle_error(mon, err); -return; -} -monitor_printf(mon, "[Please use 'info vcpu_dirty_limit' to query " - "dirty limit for virtual CPU]\n"); +out: +hmp_handle_error(mon, err); } static struct DirtyLimitInfo *dirtylimit_query_vcpu(int cpu_index) -- 2.40.1
[PATCH 09/26] migration: Implement dirty-limit convergence algo
From: Hyman Huang(黄勇) Implement dirty-limit convergence algo for live migration, which is kind of like auto-converge algo but using dirty-limit instead of cpu throttle to make migration convergent. Enable dirty page limit if dirty_rate_high_cnt greater than 2 when dirty-limit capability enabled, Disable dirty-limit if migration be canceled. Note that "set_vcpu_dirty_limit", "cancel_vcpu_dirty_limit" commands are not allowed during dirty-limit live migration. Signed-off-by: Hyman Huang(黄勇) Reviewed-by: Markus Armbruster Message-ID: <168733225273.5845.1587182678887974167...@git.sr.ht> Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- migration/migration.c | 3 +++ migration/ram.c| 36 softmmu/dirtylimit.c | 29 + migration/trace-events | 1 + 4 files changed, 69 insertions(+) diff --git a/migration/migration.c b/migration/migration.c index ae49d42eab..49332251e8 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -166,6 +166,9 @@ void migration_cancel(const Error *error) if (error) { migrate_set_error(current_migration, error); } +if (migrate_dirty_limit()) { +qmp_cancel_vcpu_dirty_limit(false, -1, NULL); +} migrate_fd_cancel(current_migration); } diff --git a/migration/ram.c b/migration/ram.c index 1d9300f4c5..9040d66e61 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -46,6 +46,7 @@ #include "qapi/error.h" #include "qapi/qapi-types-migration.h" #include "qapi/qapi-events-migration.h" +#include "qapi/qapi-commands-migration.h" #include "qapi/qmp/qerror.h" #include "trace.h" #include "exec/ram_addr.h" @@ -59,6 +60,8 @@ #include "multifd.h" #include "sysemu/runstate.h" #include "options.h" +#include "sysemu/dirtylimit.h" +#include "sysemu/kvm.h" #include "hw/boards.h" /* for machine_dump_guest_core() */ @@ -984,6 +987,37 @@ static void migration_update_rates(RAMState *rs, int64_t end_time) } } +/* + * Enable dirty-limit to throttle down the guest + */ +static void migration_dirty_limit_guest(void) +{ +/* + * dirty page rate quota for all vCPUs fetched from + * migration parameter 'vcpu_dirty_limit' + */ +static int64_t quota_dirtyrate; +MigrationState *s = migrate_get_current(); + +/* + * If dirty limit already enabled and migration parameter + * vcpu-dirty-limit untouched. + */ +if (dirtylimit_in_service() && +quota_dirtyrate == s->parameters.vcpu_dirty_limit) { +return; +} + +quota_dirtyrate = s->parameters.vcpu_dirty_limit; + +/* + * Set all vCPU a quota dirtyrate, note that the second + * parameter will be ignored if setting all vCPU for the vm + */ +qmp_set_vcpu_dirty_limit(false, -1, quota_dirtyrate, NULL); +trace_migration_dirty_limit_guest(quota_dirtyrate); +} + static void migration_trigger_throttle(RAMState *rs) { uint64_t threshold = migrate_throttle_trigger_threshold(); @@ -1013,6 +1047,8 @@ static void migration_trigger_throttle(RAMState *rs) trace_migration_throttle(); mig_throttle_guest_down(bytes_dirty_period, bytes_dirty_threshold); +} else if (migrate_dirty_limit()) { +migration_dirty_limit_guest(); } } } diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c index 942d876523..a6d854d161 100644 --- a/softmmu/dirtylimit.c +++ b/softmmu/dirtylimit.c @@ -436,6 +436,23 @@ static void dirtylimit_cleanup(void) dirtylimit_state_finalize(); } +/* + * dirty page rate limit is not allowed to set if migration + * is running with dirty-limit capability enabled. + */ +static bool dirtylimit_is_allowed(void) +{ +MigrationState *ms = migrate_get_current(); + +if (migration_is_running(ms->state) && +(!qemu_thread_is_self(&ms->thread)) && +migrate_dirty_limit() && +dirtylimit_in_service()) { +return false; +} +return true; +} + void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index, int64_t cpu_index, Error **errp) @@ -449,6 +466,12 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index, return; } +if (!dirtylimit_is_allowed()) { +error_setg(errp, "can't cancel dirty page rate limit while" + " migration is running"); +return; +} + if (!dirtylimit_in_service()) { return; } @@ -499,6 +522,12 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index, return; } +if (!dirtylimit_is_allowed()) { +error_setg(errp, "can't set dirty page rate limit while" + " migration is running"); +return; +} + if (!dirty_rate) { qmp_cancel_vcpu_dirty_limit(has_cpu_index, cpu_index, errp); return; diff --git a/migration/trace-events b/migration/trace-events index 5259c1044b..580895e86e 1
[PATCH 06/26] migration: Introduce dirty-limit capability
From: Hyman Huang(黄勇) Introduce migration dirty-limit capability, which can be turned on before live migration and limit dirty page rate durty live migration. Introduce migrate_dirty_limit function to help check if dirty-limit capability enabled during live migration. Meanwhile, refactor vcpu_dirty_rate_stat_collect so that period can be configured instead of hardcoded. dirty-limit capability is kind of like auto-converge but using dirty limit instead of traditional cpu-throttle to throttle guest down. To enable this feature, turn on the dirty-limit capability before live migration using migrate-set-capabilities, and set the parameters "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably to speed up convergence. Signed-off-by: Hyman Huang(黄勇) Acked-by: Peter Xu Reviewed-by: Juan Quintela Message-Id: <168618975839.6361.1740763387474768865...@git.sr.ht> Signed-off-by: Juan Quintela --- qapi/migration.json | 13 - migration/options.h | 1 + migration/options.c | 23 ++- softmmu/dirtylimit.c | 18 ++ 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/qapi/migration.json b/qapi/migration.json index aa590dbf0e..cc51835cdd 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -497,6 +497,16 @@ # are present. 'return-path' capability must be enabled to use # it. (since 8.1) # +# @dirty-limit: If enabled, migration will use the dirty-limit algo to +# throttle down guest instead of auto-converge algo. +# Throttle algo only works when vCPU's dirtyrate greater +# than 'vcpu-dirty-limit', read processes in guest os +# aren't penalized any more, so this algo can improve +# performance of vCPU during live migration. This is an +# optional performance feature and should not affect the +# correctness of the existing auto-converge algo. +# (since 8.1) +# # Features: # # @unstable: Members @x-colo and @x-ignore-shared are experimental. @@ -512,7 +522,8 @@ 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate', { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] }, 'validate-uuid', 'background-snapshot', - 'zero-copy-send', 'postcopy-preempt', 'switchover-ack'] } + 'zero-copy-send', 'postcopy-preempt', 'switchover-ack', + 'dirty-limit'] } ## # @MigrationCapabilityStatus: diff --git a/migration/options.h b/migration/options.h index 9aaf363322..045e2a41a2 100644 --- a/migration/options.h +++ b/migration/options.h @@ -29,6 +29,7 @@ bool migrate_block(void); bool migrate_colo(void); bool migrate_compress(void); bool migrate_dirty_bitmaps(void); +bool migrate_dirty_limit(void); bool migrate_events(void); bool migrate_ignore_shared(void); bool migrate_late_block_activate(void); diff --git a/migration/options.c b/migration/options.c index 7d2d98830e..7d83f190d6 100644 --- a/migration/options.c +++ b/migration/options.c @@ -27,6 +27,7 @@ #include "qemu-file.h" #include "ram.h" #include "options.h" +#include "sysemu/kvm.h" /* Maximum migrate downtime set to 2000 seconds */ #define MAX_MIGRATE_DOWNTIME_SECONDS 2000 @@ -196,7 +197,7 @@ Property migration_properties[] = { #endif DEFINE_PROP_MIG_CAP("x-switchover-ack", MIGRATION_CAPABILITY_SWITCHOVER_ACK), - +DEFINE_PROP_MIG_CAP("x-dirty-limit", MIGRATION_CAPABILITY_DIRTY_LIMIT), DEFINE_PROP_END_OF_LIST(), }; @@ -242,6 +243,13 @@ bool migrate_dirty_bitmaps(void) return s->capabilities[MIGRATION_CAPABILITY_DIRTY_BITMAPS]; } +bool migrate_dirty_limit(void) +{ +MigrationState *s = migrate_get_current(); + +return s->capabilities[MIGRATION_CAPABILITY_DIRTY_LIMIT]; +} + bool migrate_events(void) { MigrationState *s = migrate_get_current(); @@ -572,6 +580,19 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp) return false; } } +if (new_caps[MIGRATION_CAPABILITY_DIRTY_LIMIT]) { +if (new_caps[MIGRATION_CAPABILITY_AUTO_CONVERGE]) { +error_setg(errp, "dirty-limit conflicts with auto-converge" + " either of then available currently"); +return false; +} + +if (!kvm_enabled() || !kvm_dirty_ring_enabled()) { +error_setg(errp, "dirty-limit requires KVM with accelerator" + " property 'dirty-ring-size' set"); +return false; +} +} return true; } diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c index e80201097a..942d876523 100644 --- a/softmmu/dirtylimit.c +++ b/softmmu/dirtylimit.c @@ -24,6 +24,9 @@ #include "hw/boards.h" #include "sysemu/kvm.h" #include "trace.h" +#include "migration/misc.h" +#include "migration/migration.h" +#include "migration/options.h" /* * Dirtylimit stop working if dirty page rate error @@ -75,14 +78,21 @@ static bool
[PATCH 08/26] migration: Put the detection logic before auto-converge checking
From: Hyman Huang(黄勇) This commit is prepared for the implementation of dirty-limit convergence algo. The detection logic of throttling condition can apply to both auto-converge and dirty-limit algo, putting it's position before the checking logic for auto-converge feature. Signed-off-by: Hyman Huang(黄勇) Reviewed-by: Juan Quintela Message-ID: <168733225273.5845.1587182678887974167...@git.sr.ht> Signed-off-by: Juan Quintela --- migration/ram.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index f31de47a47..1d9300f4c5 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -999,17 +999,18 @@ static void migration_trigger_throttle(RAMState *rs) return; } -if (migrate_auto_converge()) { -/* The following detection logic can be refined later. For now: - Check to see if the ratio between dirtied bytes and the approx. - amount of bytes that just got transferred since the last time - we were in this routine reaches the threshold. If that happens - twice, start or increase throttling. */ - -if ((bytes_dirty_period > bytes_dirty_threshold) && -(++rs->dirty_rate_high_cnt >= 2)) { +/* + * The following detection logic can be refined later. For now: + * Check to see if the ratio between dirtied bytes and the approx. + * amount of bytes that just got transferred since the last time + * we were in this routine reaches the threshold. If that happens + * twice, start or increase throttling. + */ +if ((bytes_dirty_period > bytes_dirty_threshold) && +(++rs->dirty_rate_high_cnt >= 2)) { +rs->dirty_rate_high_cnt = 0; +if (migrate_auto_converge()) { trace_migration_throttle(); -rs->dirty_rate_high_cnt = 0; mig_throttle_guest_down(bytes_dirty_period, bytes_dirty_threshold); } -- 2.40.1
[PATCH 16/26] migration: skipped field is really obsolete.
Has return zero for more than 10 years. Specifically we introduced the field in 1.5.0 commit f1c72795af573b24a7da5eb52375c9aba8a37972 Author: Peter Lieven Date: Tue Mar 26 10:58:37 2013 +0100 migration: do not sent zero pages in bulk stage during bulk stage of ram migration if a page is a zero page do not send it at all. the memory at the destination reads as zero anyway. even if there is an madvise with QEMU_MADV_DONTNEED at the target upon receipt of a zero page I have observed that the target starts swapping if the memory is overcommitted. it seems that the pages are dropped asynchronously. this patch also updates QMP to return the number of skipped pages in MigrationStats. but removed its usage in 1.5.3 commit 9ef051e5536b6368a1076046ec6c4ec4ac12b5c6 Author: Peter Lieven Date: Mon Jun 10 12:14:19 2013 +0200 Revert "migration: do not sent zero pages in bulk stage" Not sending zero pages breaks migration if a page is zero at the source but not at the destination. This can e.g. happen if different BIOS versions are used at source and destination. It has also been reported that migration on pseries is completely broken with this patch. This effectively reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972. Reviewed-by: Daniel P. Berrangé Message-ID: <20230612193344.3796-2-quint...@redhat.com> Signed-off-by: Juan Quintela --- docs/about/deprecated.rst | 10 ++ qapi/migration.json | 12 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 02ea5a839f..1c35f55666 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -451,3 +451,13 @@ both, older and future versions of QEMU. The ``blacklist`` config file option has been renamed to ``block-rpcs`` (to be in sync with the renaming of the corresponding command line option). + +Migration +- + +``skipped`` MigrationStats field (since 8.1) + + +``skipped`` field in Migration stats has been deprecated. It hasn't +been used for more than 10 years. + diff --git a/qapi/migration.json b/qapi/migration.json index 7ccb28e64f..bc9ae3fef7 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -23,7 +23,8 @@ # # @duplicate: number of duplicate (zero) pages (since 1.2) # -# @skipped: number of skipped zero pages (since 1.5) +# @skipped: number of skipped zero pages. Always zero, only provided for +# compatibility (since 1.5) # # @normal: number of normal pages (since 1.2) # @@ -62,11 +63,18 @@ # between 0 and @dirty-sync-count * @multifd-channels. (since # 7.1) # +# Features: +# +# @deprecated: Member @skipped is always zero since 1.5.3 +# # Since: 0.14 +# ## { 'struct': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , - 'duplicate': 'int', 'skipped': 'int', 'normal': 'int', + 'duplicate': 'int', + 'skipped': { 'type': 'int', 'features': ['deprecated'] }, + 'normal': 'int', 'normal-bytes': 'int', 'dirty-pages-rate': 'int', 'mbps': 'number', 'dirty-sync-count': 'int', 'postcopy-requests': 'int', 'page-size': 'int', -- 2.40.1
[PATCH 26/26] migration/rdma: Split qemu_fopen_rdma() into input/output functions
This is how everything else in QEMUFile is structured. As a bonus they are three less lines of code. Reviewed-by: Peter Xu Message-ID: <20230530183941.7223-17-quint...@redhat.com> Signed-off-by: Juan Quintela --- migration/qemu-file.h | 1 - migration/qemu-file.c | 12 migration/rdma.c | 39 +++ 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/migration/qemu-file.h b/migration/qemu-file.h index 8b8b7d27fe..47015f5201 100644 --- a/migration/qemu-file.h +++ b/migration/qemu-file.h @@ -102,7 +102,6 @@ uint64_t qemu_file_transferred_noflush(QEMUFile *f); */ void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size, bool may_free); -bool qemu_file_mode_is_not_valid(const char *mode); #include "migration/qemu-file-types.h" diff --git a/migration/qemu-file.c b/migration/qemu-file.c index d30bf3c377..19c33c9985 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -100,18 +100,6 @@ int qemu_file_shutdown(QEMUFile *f) return 0; } -bool qemu_file_mode_is_not_valid(const char *mode) -{ -if (mode == NULL || -(mode[0] != 'r' && mode[0] != 'w') || -mode[1] != 'b' || mode[2] != 0) { -fprintf(stderr, "qemu_fopen: Argument validity check failed\n"); -return true; -} - -return false; -} - static QEMUFile *qemu_file_new_impl(QIOChannel *ioc, bool is_writable) { QEMUFile *f; diff --git a/migration/rdma.c b/migration/rdma.c index dd1c039e6c..ca430d319d 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -4053,27 +4053,26 @@ static void qio_channel_rdma_register_types(void) type_init(qio_channel_rdma_register_types); -static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode) +static QEMUFile *rdma_new_input(RDMAContext *rdma) { -QIOChannelRDMA *rioc; +QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(object_new(TYPE_QIO_CHANNEL_RDMA)); -if (qemu_file_mode_is_not_valid(mode)) { -return NULL; -} +rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc)); +rioc->rdmain = rdma; +rioc->rdmaout = rdma->return_path; +qemu_file_set_hooks(rioc->file, &rdma_read_hooks); -rioc = QIO_CHANNEL_RDMA(object_new(TYPE_QIO_CHANNEL_RDMA)); +return rioc->file; +} -if (mode[0] == 'w') { -rioc->file = qemu_file_new_output(QIO_CHANNEL(rioc)); -rioc->rdmaout = rdma; -rioc->rdmain = rdma->return_path; -qemu_file_set_hooks(rioc->file, &rdma_write_hooks); -} else { -rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc)); -rioc->rdmain = rdma; -rioc->rdmaout = rdma->return_path; -qemu_file_set_hooks(rioc->file, &rdma_read_hooks); -} +static QEMUFile *rdma_new_output(RDMAContext *rdma) +{ +QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(object_new(TYPE_QIO_CHANNEL_RDMA)); + +rioc->file = qemu_file_new_output(QIO_CHANNEL(rioc)); +rioc->rdmaout = rdma; +rioc->rdmain = rdma->return_path; +qemu_file_set_hooks(rioc->file, &rdma_write_hooks); return rioc->file; } @@ -4099,9 +4098,9 @@ static void rdma_accept_incoming_migration(void *opaque) return; } -f = qemu_fopen_rdma(rdma, "rb"); +f = rdma_new_input(rdma); if (f == NULL) { -fprintf(stderr, "RDMA ERROR: could not qemu_fopen_rdma\n"); +fprintf(stderr, "RDMA ERROR: could not open RDMA for input\n"); qemu_rdma_cleanup(rdma); return; } @@ -4224,7 +4223,7 @@ void rdma_start_outgoing_migration(void *opaque, trace_rdma_start_outgoing_migration_after_rdma_connect(); -s->to_dst_file = qemu_fopen_rdma(rdma, "wb"); +s->to_dst_file = rdma_new_output(rdma); migrate_fd_connect(s, NULL); return; return_path_err: -- 2.40.1
[PATCH 02/26] migration/multifd: Protect accesses to migration_threads
From: Fabiano Rosas This doubly linked list is common for all the multifd and migration threads so we need to avoid concurrent access. Add a mutex to protect the data from concurrent access. This fixes a crash when removing two MigrationThread objects from the list at the same time during cleanup of multifd threads. Fixes: 671326201d ("migration: Introduce interface query-migrationthreads") Signed-off-by: Fabiano Rosas Reviewed-by: Peter Xu Reviewed-by: Juan Quintela Message-Id: <20230607161306.31425-3-faro...@suse.de> Signed-off-by: Juan Quintela --- migration/threadinfo.h | 2 -- migration/threadinfo.c | 15 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/migration/threadinfo.h b/migration/threadinfo.h index 8aa6999d58..2f356ff312 100644 --- a/migration/threadinfo.h +++ b/migration/threadinfo.h @@ -10,8 +10,6 @@ * See the COPYING file in the top-level directory. */ -#include "qemu/queue.h" -#include "qemu/osdep.h" #include "qapi/error.h" #include "qapi/qapi-commands-migration.h" diff --git a/migration/threadinfo.c b/migration/threadinfo.c index 3dd9b14ae6..262990dd75 100644 --- a/migration/threadinfo.c +++ b/migration/threadinfo.c @@ -10,23 +10,35 @@ * See the COPYING file in the top-level directory. */ +#include "qemu/osdep.h" +#include "qemu/queue.h" +#include "qemu/lockable.h" #include "threadinfo.h" +QemuMutex migration_threads_lock; static QLIST_HEAD(, MigrationThread) migration_threads; +static void __attribute__((constructor)) migration_threads_init(void) +{ +qemu_mutex_init(&migration_threads_lock); +} + MigrationThread *migration_threads_add(const char *name, int thread_id) { MigrationThread *thread = g_new0(MigrationThread, 1); thread->name = name; thread->thread_id = thread_id; -QLIST_INSERT_HEAD(&migration_threads, thread, node); +WITH_QEMU_LOCK_GUARD(&migration_threads_lock) { +QLIST_INSERT_HEAD(&migration_threads, thread, node); +} return thread; } void migration_threads_remove(MigrationThread *thread) { +QEMU_LOCK_GUARD(&migration_threads_lock); if (thread) { QLIST_REMOVE(thread, node); g_free(thread); @@ -39,6 +51,7 @@ MigrationThreadInfoList *qmp_query_migrationthreads(Error **errp) MigrationThreadInfoList **tail = &head; MigrationThread *thread = NULL; +QEMU_LOCK_GUARD(&migration_threads_lock); QLIST_FOREACH(thread, &migration_threads, node) { MigrationThreadInfo *info = g_new0(MigrationThreadInfo, 1); info->name = g_strdup(thread->name); -- 2.40.1
[PATCH 21/26] qemu-file: Rename qemu_file_transferred_ fast -> noflush
Fast don't say much. Noflush indicates more clearly that it is like qemu_file_transferred but without the flush. Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20230530183941.7223-2-quint...@redhat.com> Signed-off-by: Juan Quintela --- migration/qemu-file.h | 11 +-- migration/qemu-file.c | 2 +- migration/savevm.c| 4 ++-- migration/vmstate.c | 4 ++-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/migration/qemu-file.h b/migration/qemu-file.h index e649718492..aa6eee66da 100644 --- a/migration/qemu-file.h +++ b/migration/qemu-file.h @@ -86,16 +86,15 @@ int qemu_fclose(QEMUFile *f); uint64_t qemu_file_transferred(QEMUFile *f); /* - * qemu_file_transferred_fast: + * qemu_file_transferred_noflush: * - * As qemu_file_transferred except for writable - * files, where no flush is performed and the reported - * amount will include the size of any queued buffers, - * on top of the amount actually transferred. + * As qemu_file_transferred except for writable files, where no flush + * is performed and the reported amount will include the size of any + * queued buffers, on top of the amount actually transferred. * * Returns: the total bytes transferred and queued */ -uint64_t qemu_file_transferred_fast(QEMUFile *f); +uint64_t qemu_file_transferred_noflush(QEMUFile *f); /* * put_buffer without copying the buffer. diff --git a/migration/qemu-file.c b/migration/qemu-file.c index acc282654a..fdf115b5da 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -694,7 +694,7 @@ int coroutine_mixed_fn qemu_get_byte(QEMUFile *f) return result; } -uint64_t qemu_file_transferred_fast(QEMUFile *f) +uint64_t qemu_file_transferred_noflush(QEMUFile *f) { uint64_t ret = f->total_transferred; int i; diff --git a/migration/savevm.c b/migration/savevm.c index 95c2abf47c..a07070db62 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -927,9 +927,9 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se) static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc) { -uint64_t old_offset = qemu_file_transferred_fast(f); +uint64_t old_offset = qemu_file_transferred_noflush(f); se->ops->save_state(f, se->opaque); -uint64_t size = qemu_file_transferred_fast(f) - old_offset; +uint64_t size = qemu_file_transferred_noflush(f) - old_offset; if (vmdesc) { json_writer_int64(vmdesc, "size", size); diff --git a/migration/vmstate.c b/migration/vmstate.c index af01d54b6f..31842c3afb 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -361,7 +361,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, void *curr_elem = first_elem + size * i; vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems); -old_offset = qemu_file_transferred_fast(f); +old_offset = qemu_file_transferred_noflush(f); if (field->flags & VMS_ARRAY_OF_POINTER) { assert(curr_elem); curr_elem = *(void **)curr_elem; @@ -391,7 +391,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, return ret; } -written_bytes = qemu_file_transferred_fast(f) - old_offset; +written_bytes = qemu_file_transferred_noflush(f) - old_offset; vmsd_desc_field_end(vmsd, vmdesc_loop, field, written_bytes, i); /* Compressed arrays only care about the first element */ -- 2.40.1
[PATCH 04/26] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
From: Hyman Huang(黄勇) Introduce "x-vcpu-dirty-limit-period" migration experimental parameter, which is in the range of 1 to 1000ms and used to make dirtyrate calculation period configurable. Currently with the "x-vcpu-dirty-limit-period" varies, the total time of live migration changes, test results show the optimal value of "x-vcpu-dirty-limit-period" ranges from 500ms to 1000 ms. "x-vcpu-dirty-limit-period" should be made stable once it proves best value can not be determined with developer's experiments. Signed-off-by: Hyman Huang(黄勇) Reviewed-by: Markus Armbruster Reviewed-by: Juan Quintela Message-Id: <168618975839.6361.1740763387474768865...@git.sr.ht> Signed-off-by: Juan Quintela --- qapi/migration.json| 34 +++--- migration/migration-hmp-cmds.c | 8 migration/options.c| 28 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/qapi/migration.json b/qapi/migration.json index 47dfef0278..384b768e03 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -789,9 +789,14 @@ # Nodes are mapped to their block device name if there is one, and # to their node name otherwise. (Since 5.2) # +# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty limit during +# live migration. Should be in the range 1 to 1000ms, +# defaults to 1000ms. (Since 8.1) +# # Features: # -# @unstable: Member @x-checkpoint-delay is experimental. +# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period +#are experimental. # # Since: 2.4 ## @@ -809,8 +814,9 @@ 'multifd-channels', 'xbzrle-cache-size', 'max-postcopy-bandwidth', 'max-cpu-throttle', 'multifd-compression', - 'multifd-zlib-level' ,'multifd-zstd-level', - 'block-bitmap-mapping' ] } + 'multifd-zlib-level', 'multifd-zstd-level', + 'block-bitmap-mapping', + { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] } ] } ## # @MigrateSetParameters: @@ -945,9 +951,14 @@ # Nodes are mapped to their block device name if there is one, and # to their node name otherwise. (Since 5.2) # +# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty limit during +# live migration. Should be in the range 1 to 1000ms, +# defaults to 1000ms. (Since 8.1) +# # Features: # -# @unstable: Member @x-checkpoint-delay is experimental. +# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period +#are experimental. # # TODO: either fuse back into MigrationParameters, or make # MigrationParameters members mandatory @@ -982,7 +993,9 @@ '*multifd-compression': 'MultiFDCompression', '*multifd-zlib-level': 'uint8', '*multifd-zstd-level': 'uint8', -'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } } +'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ], +'*x-vcpu-dirty-limit-period': { 'type': 'uint64', +'features': [ 'unstable' ] } } } ## # @migrate-set-parameters: @@ -1137,9 +1150,14 @@ # Nodes are mapped to their block device name if there is one, and # to their node name otherwise. (Since 5.2) # +# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty limit during +# live migration. Should be in the range 1 to 1000ms, +# defaults to 1000ms. (Since 8.1) +# # Features: # -# @unstable: Member @x-checkpoint-delay is experimental. +# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period +#are experimental. # # Since: 2.4 ## @@ -1171,7 +1189,9 @@ '*multifd-compression': 'MultiFDCompression', '*multifd-zlib-level': 'uint8', '*multifd-zstd-level': 'uint8', -'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } } +'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ], +'*x-vcpu-dirty-limit-period': { 'type': 'uint64', +'features': [ 'unstable' ] } } } ## # @query-migrate-parameters: diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index 9885d7c9f7..352e9ec716 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -364,6 +364,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) } } } + +monitor_printf(mon, "%s: %" PRIu64 " ms\n", +MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD), +params->x_vcpu_dirty_limit_period); } qapi_free_MigrationParameters(params); @@ -620,6 +624,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict
[PATCH 13/26] migration-test: Create arch_opts
This will contain the options needed for both source and target. Reviewed-by: Peter Xu Message-ID: <20230608224943.3877-6-quint...@redhat.com> Signed-off-by: Juan Quintela --- tests/qtest/migration-test.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index f51a25e299..c723f083da 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -702,6 +702,8 @@ static int test_migrate_start(QTestState **from, QTestState **to, { g_autofree gchar *arch_source = NULL; g_autofree gchar *arch_target = NULL; +/* options for source and target */ +g_autofree gchar *arch_opts = NULL; g_autofree gchar *cmd_source = NULL; g_autofree gchar *cmd_target = NULL; const gchar *ignore_stderr; @@ -727,15 +729,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, assert(sizeof(x86_bootsect) == 512); init_bootfile(bootpath, x86_bootsect, sizeof(x86_bootsect)); memory_size = "150M"; -arch_source = g_strdup_printf("-drive file=%s,format=raw", bootpath); -arch_target = g_strdup(arch_source); +arch_opts = g_strdup_printf("-drive file=%s,format=raw", bootpath); start_address = X86_TEST_MEM_START; end_address = X86_TEST_MEM_END; } else if (g_str_equal(arch, "s390x")) { init_bootfile(bootpath, s390x_elf, sizeof(s390x_elf)); memory_size = "128M"; -arch_source = g_strdup_printf("-bios %s", bootpath); -arch_target = g_strdup(arch_source); +arch_opts = g_strdup_printf("-bios %s", bootpath); start_address = S390_TEST_MEM_START; end_address = S390_TEST_MEM_END; } else if (strcmp(arch, "ppc64") == 0) { @@ -743,20 +743,16 @@ static int test_migrate_start(QTestState **from, QTestState **to, memory_size = "256M"; start_address = PPC_TEST_MEM_START; end_address = PPC_TEST_MEM_END; -arch_source = g_strdup_printf("-nodefaults " - "-prom-env 'use-nvramrc?=true' -prom-env " +arch_source = g_strdup_printf("-prom-env 'use-nvramrc?=true' -prom-env " "'nvramrc=hex .\" _\" begin %x %x " "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " "until'", end_address, start_address); -arch_target = g_strdup("-nodefaults"); +arch_opts = g_strdup("-nodefaults"); } else if (strcmp(arch, "aarch64") == 0) { init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel)); machine_opts = "-machine virt,gic-version=max"; memory_size = "150M"; -arch_source = g_strdup_printf("-cpu max " - "-kernel %s", - bootpath); -arch_target = g_strdup(arch_source); +arch_opts = g_strdup_printf("-cpu max -kernel %s", bootpath); start_address = ARM_TEST_MEM_START; end_address = ARM_TEST_MEM_END; @@ -795,12 +791,14 @@ static int test_migrate_start(QTestState **from, QTestState **to, "-name source,debug-threads=on " "-m %s " "-serial file:%s/src_serial " - "%s %s %s %s", + "%s %s %s %s %s", args->use_dirty_ring ? ",dirty-ring-size=4096" : "", machine_opts ? machine_opts : "", memory_size, tmpfs, - arch_source, shmem_opts, + arch_opts ? arch_opts : "", + arch_source ? arch_source : "", + shmem_opts, args->opts_source ? args->opts_source : "", ignore_stderr); if (!args->only_target) { @@ -815,12 +813,14 @@ static int test_migrate_start(QTestState **from, QTestState **to, "-m %s " "-serial file:%s/dest_serial " "-incoming %s " - "%s %s %s %s", + "%s %s %s %s %s", args->use_dirty_ring ? ",dirty-ring-size=4096" : "", machine_opts ? machine_opts : "", memory_size, tmpfs, uri, - arch_target, shmem_opts, + arch_opts ? arch_opts : "", + arch_target ? arch_target : "", + shmem_opts,
[PATCH 05/26] qapi/migration: Introduce vcpu-dirty-limit parameters
From: Hyman Huang(黄勇) Introduce "vcpu-dirty-limit" migration parameter used to limit dirty page rate during live migration. "vcpu-dirty-limit" and "x-vcpu-dirty-limit-period" are two dirty-limit-related migration parameters, which can be set before and during live migration by qmp migrate-set-parameters. This two parameters are used to help implement the dirty page rate limit algo of migration. Signed-off-by: Hyman Huang(黄勇) Acked-by: Peter Xu Reviewed-by: Juan Quintela Message-Id: <168618975839.6361.1740763387474768865...@git.sr.ht> Signed-off-by: Juan Quintela --- qapi/migration.json| 18 +++--- migration/migration-hmp-cmds.c | 8 migration/options.c| 21 + 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/qapi/migration.json b/qapi/migration.json index 384b768e03..aa590dbf0e 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -793,6 +793,9 @@ # live migration. Should be in the range 1 to 1000ms, # defaults to 1000ms. (Since 8.1) # +# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration. +#Defaults to 1. (Since 8.1) +# # Features: # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period @@ -816,7 +819,8 @@ 'max-cpu-throttle', 'multifd-compression', 'multifd-zlib-level', 'multifd-zstd-level', 'block-bitmap-mapping', - { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] } ] } + { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] }, + 'vcpu-dirty-limit'] } ## # @MigrateSetParameters: @@ -955,6 +959,9 @@ # live migration. Should be in the range 1 to 1000ms, # defaults to 1000ms. (Since 8.1) # +# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration. +#Defaults to 1. (Since 8.1) +# # Features: # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period @@ -995,7 +1002,8 @@ '*multifd-zstd-level': 'uint8', '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ], '*x-vcpu-dirty-limit-period': { 'type': 'uint64', -'features': [ 'unstable' ] } } } +'features': [ 'unstable' ] }, +'*vcpu-dirty-limit': 'uint64'} } ## # @migrate-set-parameters: @@ -1154,6 +1162,9 @@ # live migration. Should be in the range 1 to 1000ms, # defaults to 1000ms. (Since 8.1) # +# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration. +#Defaults to 1. (Since 8.1) +# # Features: # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period @@ -1191,7 +1202,8 @@ '*multifd-zstd-level': 'uint8', '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ], '*x-vcpu-dirty-limit-period': { 'type': 'uint64', -'features': [ 'unstable' ] } } } +'features': [ 'unstable' ] }, +'*vcpu-dirty-limit': 'uint64'} } ## # @query-migrate-parameters: diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index 352e9ec716..35e8020bbf 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -368,6 +368,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, "%s: %" PRIu64 " ms\n", MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD), params->x_vcpu_dirty_limit_period); + +monitor_printf(mon, "%s: %" PRIu64 " MB/s\n", +MigrationParameter_str(MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT), +params->vcpu_dirty_limit); } qapi_free_MigrationParameters(params); @@ -628,6 +632,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) p->has_x_vcpu_dirty_limit_period = true; visit_type_size(v, param, &p->x_vcpu_dirty_limit_period, &err); break; +case MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT: +p->has_vcpu_dirty_limit = true; +visit_type_size(v, param, &p->vcpu_dirty_limit, &err); +break; default: assert(0); } diff --git a/migration/options.c b/migration/options.c index 1de63ba775..7d2d98830e 100644 --- a/migration/options.c +++ b/migration/options.c @@ -81,6 +81,7 @@ DEFINE_PROP_BOOL(name, MigrationState, capabilities[x], false) #define DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT_PERIOD 1000/* milliseconds */ +#define DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT1 /* MB/s */ Property migration_properties[] = { DEFINE_PROP_BOOL("store-global-state", MigrationState, @@ -168,6 +169,9 @@ Property migration_properties[] = { DE
[PATCH 17/26] docs/migration: Update postcopy bits
From: Peter Xu We have postcopy recovery but not reflected in the document, do an update for that. Add a very small section on postcopy preempt. Touch up the pagemap section, dropping the unsent map because it's already been dropped in the source code in commit 1e7cf8c323 ("migration/postcopy: unsentmap is not necessary for postcopy"). Touch up the postcopy section to remove "network connection" failures as downside, because now it's not fatal and can be recovered. Suggested by Laszlo. Acked-by: Laszlo Ersek Signed-off-by: Peter Xu Reviewed-by: Juan Quintela Message-ID: <20230706115611.371048-1-pet...@redhat.com> Signed-off-by: Juan Quintela --- docs/devel/migration.rst | 94 1 file changed, 67 insertions(+), 27 deletions(-) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index 6f65c23b47..c3e1400c0c 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -594,8 +594,7 @@ Postcopy 'Postcopy' migration is a way to deal with migrations that refuse to converge (or take too long to converge) its plus side is that there is an upper bound on the amount of migration traffic and time it takes, the down side is that during -the postcopy phase, a failure of *either* side or the network connection causes -the guest to be lost. +the postcopy phase, a failure of *either* side causes the guest to be lost. In postcopy the destination CPUs are started before all the memory has been transferred, and accesses to pages that are yet to be transferred cause @@ -721,6 +720,42 @@ processing. is no longer used by migration, while the listen thread carries on servicing page data until the end of migration. +Postcopy Recovery +- + +Comparing to precopy, postcopy is special on error handlings. When any +error happens (in this case, mostly network errors), QEMU cannot easily +fail a migration because VM data resides in both source and destination +QEMU instances. On the other hand, when issue happens QEMU on both sides +will go into a paused state. It'll need a recovery phase to continue a +paused postcopy migration. + +The recovery phase normally contains a few steps: + + - When network issue occurs, both QEMU will go into PAUSED state + + - When the network is recovered (or a new network is provided), the admin +can setup the new channel for migration using QMP command +'migrate-recover' on destination node, preparing for a resume. + + - On source host, the admin can continue the interrupted postcopy +migration using QMP command 'migrate' with resume=true flag set. + + - After the connection is re-established, QEMU will continue the postcopy +migration on both sides. + +During a paused postcopy migration, the VM can logically still continue +running, and it will not be impacted from any page access to pages that +were already migrated to destination VM before the interruption happens. +However, if any of the missing pages got accessed on destination VM, the VM +thread will be halted waiting for the page to be migrated, it means it can +be halted until the recovery is complete. + +The impact of accessing missing pages can be relevant to different +configurations of the guest. For example, when with async page fault +enabled, logically the guest can proactively schedule out the threads +accessing missing pages. + Postcopy states --- @@ -765,36 +800,31 @@ ADVISE->DISCARD->LISTEN->RUNNING->END (although it can't do the cleanup it would do as it finishes a normal migration). + - Paused + +Postcopy can run into a paused state (normally on both sides when +happens), where all threads will be temporarily halted mostly due to +network errors. When reaching paused state, migration will make sure +the qemu binary on both sides maintain the data without corrupting +the VM. To continue the migration, the admin needs to fix the +migration channel using the QMP command 'migrate-recover' on the +destination node, then resume the migration using QMP command 'migrate' +again on source node, with resume=true flag set. + - End The listen thread can now quit, and perform the cleanup of migration state, the migration is now complete. -Source side page maps -- - -The source side keeps two bitmaps during postcopy; 'the migration bitmap' -and 'unsent map'. The 'migration bitmap' is basically the same as in -the precopy case, and holds a bit to indicate that page is 'dirty' - -i.e. needs sending. During the precopy phase this is updated as the CPU -dirties pages, however during postcopy the CPUs are stopped and nothing -should dirty anything any more. - -The 'unsent map' is used for the transition to postcopy. It is a bitmap that -has a bit cleared whenever a page is sent to the destination, however during -the transition to postcopy mode it is combined with the migration bitmap -to form a set of pages that: - - a)
[PATCH 22/26] migration: Change qemu_file_transferred to noflush
We do a qemu_fclose() just after that, that also does a qemu_fflush(), so remove one qemu_fflush(). Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20230530183941.7223-3-quint...@redhat.com> Signed-off-by: Juan Quintela --- migration/savevm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/savevm.c b/migration/savevm.c index a07070db62..cc59ddaa87 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -3007,7 +3007,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate, goto the_end; } ret = qemu_savevm_state(f, errp); -vm_state_size = qemu_file_transferred(f); +vm_state_size = qemu_file_transferred_noflush(f); ret2 = qemu_fclose(f); if (ret < 0) { goto the_end; -- 2.40.1
[PATCH 07/26] migration: Refactor auto-converge capability logic
From: Hyman Huang(黄勇) Check if block migration is running before throttling guest down in auto-converge way. Note that this modification is kind of like code clean, because block migration does not depend on auto-converge capability, so the order of checks can be adjusted. Signed-off-by: Hyman Huang(黄勇) Acked-by: Peter Xu Reviewed-by: Juan Quintela Message-Id: <168618975839.6361.1740763387474768865...@git.sr.ht> Signed-off-by: Juan Quintela --- migration/ram.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/migration/ram.c b/migration/ram.c index 0ada6477e8..f31de47a47 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -995,7 +995,11 @@ static void migration_trigger_throttle(RAMState *rs) /* During block migration the auto-converge logic incorrectly detects * that ram migration makes no progress. Avoid this by disabling the * throttling logic during the bulk phase of block migration. */ -if (migrate_auto_converge() && !blk_mig_bulk_active()) { +if (blk_mig_bulk_active()) { +return; +} + +if (migrate_auto_converge()) { /* The following detection logic can be refined later. For now: Check to see if the ratio between dirtied bytes and the approx. amount of bytes that just got transferred since the last time -- 2.40.1
[PATCH 11/26] migration-test: Be consistent for ppc
It makes no sense that we don't have the same configuration on both sides. Reviewed-by: Laurent Vivier Message-ID: <20230608224943.3877-2-quint...@redhat.com> Signed-off-by: Juan Quintela --- tests/qtest/migration-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index e256da1216..2296ed4bf5 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -748,7 +748,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, "'nvramrc=hex .\" _\" begin %x %x " "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " "until'", end_address, start_address); -arch_target = g_strdup(""); +arch_target = g_strdup("-nodefaults"); } else if (strcmp(arch, "aarch64") == 0) { init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel)); machine_opts = "virt,gic-version=max"; -- 2.40.1
[PATCH 15/26] migration.json: Don't use space before colon
So all the file is consistent. Reviewed-by: Markus Armbruster Message-ID: <20230612191604.2219-1-quint...@redhat.com> Signed-off-by: Juan Quintela --- qapi/migration.json | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/qapi/migration.json b/qapi/migration.json index ebc15e2782..7ccb28e64f 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -67,13 +67,13 @@ { 'struct': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , 'duplicate': 'int', 'skipped': 'int', 'normal': 'int', - 'normal-bytes': 'int', 'dirty-pages-rate' : 'int', - 'mbps' : 'number', 'dirty-sync-count' : 'int', - 'postcopy-requests' : 'int', 'page-size' : 'int', - 'multifd-bytes' : 'uint64', 'pages-per-second' : 'uint64', - 'precopy-bytes' : 'uint64', 'downtime-bytes' : 'uint64', - 'postcopy-bytes' : 'uint64', - 'dirty-sync-missed-zero-copy' : 'uint64' } } + 'normal-bytes': 'int', 'dirty-pages-rate': 'int', + 'mbps': 'number', 'dirty-sync-count': 'int', + 'postcopy-requests': 'int', 'page-size': 'int', + 'multifd-bytes': 'uint64', 'pages-per-second': 'uint64', + 'precopy-bytes': 'uint64', 'downtime-bytes': 'uint64', + 'postcopy-bytes': 'uint64', + 'dirty-sync-missed-zero-copy': 'uint64' } } ## # @XBZRLECacheStats: @@ -276,7 +276,7 @@ '*cpu-throttle-percentage': 'int', '*error-desc': 'str', '*blocked-reasons': ['str'], - '*postcopy-blocktime' : 'uint32', + '*postcopy-blocktime': 'uint32', '*postcopy-vcpu-blocktime': ['uint32'], '*compression': 'CompressionStats', '*socket-address': ['SocketAddress'], @@ -551,7 +551,7 @@ # Since: 1.2 ## { 'struct': 'MigrationCapabilityStatus', - 'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } } + 'data': { 'capability': 'MigrationCapability', 'state': 'bool' } } ## # @migrate-set-capabilities: @@ -1634,7 +1634,7 @@ # Since: 2.9 ## { 'command': 'xen-set-replication', - 'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' }, + 'data': { 'enable': 'bool', 'primary': 'bool', '*failover': 'bool' }, 'if': 'CONFIG_REPLICATION' } ## -- 2.40.1
[PATCH 00/26] Migration PULL 2023-07-24
Hi This is the migration PULL request. It has: - Fabiano rosas trheadinfo cleanups - Hyman Huang dirtylimit changes - Part of my changes - Peter Xu documentation - Tejus updato to migration descriptions - Wei want improvements for postocpy and multifd setup Please apply. Now a not on CI, thas has been really bad. After too many problems with last PULLS, I decided to learn to use qemu CI. On one hand, it is not so difficult, even I can use it O:-) On the other hand, the amount of problems that I got is inmense. Some of them dissapear when I rerun the checks, but I never know if it is my PULL request, the CI system or the tests themselves. So it ends going something like: while (true); do - git pull - git rebase - git push ci blah, blah - Next day cames, and too many errors, so I rebase again The last step takes more time than expected and not always trivial to know how the failure is. This (last) patch is not part of the PULL request, but I have found that it _always_ makes gcov fail. I had to use bisect to find where the problem was. https://gitlab.com/juan.quintela/qemu/-/jobs/4571878922 I could use help to know how a change in test/qtest/migration-test.c can break block layer tests, I am all ears. Yes, I tried several times. It always fails on that patch. The passes with flying colors. Later, Juan. Fabiano Rosas (2): migration/multifd: Rename threadinfo.c functions migration/multifd: Protect accesses to migration_threads Hyman Huang(黄勇) (8): softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit" qapi/migration: Introduce x-vcpu-dirty-limit-period parameter qapi/migration: Introduce vcpu-dirty-limit parameters migration: Introduce dirty-limit capability migration: Refactor auto-converge capability logic migration: Put the detection logic before auto-converge checking migration: Implement dirty-limit convergence algo migration: Extend query-migrate to provide dirty page limit info Juan Quintela (12): migration-test: Be consistent for ppc migration-test: Make machine_opts regular with other options migration-test: Create arch_opts migration-test: machine_opts is really arch specific migration.json: Don't use space before colon migration: skipped field is really obsolete. qemu-file: Rename qemu_file_transferred_ fast -> noflush migration: Change qemu_file_transferred to noflush qemu_file: Make qemu_file_is_writable() static qemu-file: Simplify qemu_file_shutdown() qemu-file: Make qemu_file_get_error_obj() static migration/rdma: Split qemu_fopen_rdma() into input/output functions Peter Xu (1): docs/migration: Update postcopy bits Tejus GK (1): migration: Update error description whenever migration fails Wei Wang (2): migration: enforce multifd and postcopy preempt to be set before incoming qtest/migration-tests.c: use "-incoming defer" for postcopy tests docs/about/deprecated.rst | 10 +++ docs/devel/migration.rst | 94 - qapi/migration.json| 107 ++--- include/sysemu/dirtylimit.h| 2 + migration/options.h| 1 + migration/qemu-file.h | 14 ++--- migration/threadinfo.h | 7 +-- migration/migration-hmp-cmds.c | 26 migration/migration.c | 36 --- migration/multifd.c| 4 +- migration/options.c| 87 ++- migration/qemu-file.c | 24 ++-- migration/ram.c| 59 +++--- migration/rdma.c | 39 ++-- migration/savevm.c | 6 +- migration/threadinfo.c | 19 +- migration/vmstate.c| 4 +- softmmu/dirtylimit.c | 97 +++--- tests/qtest/migration-test.c | 48 +++ migration/trace-events | 1 + 20 files changed, 520 insertions(+), 165 deletions(-) -- 2.40.1
Re: [PATCH v2 0/3] qemu-img: map: implement support for compressed clusters
On 7/6/23 19:30, Andrey Drobyshev wrote: > v1 --> v2: > * Add vmdk format to the 1st commit. Tweak commit message accordingly; > * Make "compressed" field in MapEntry optional. > > v1: https://lists.nongnu.org/archive/html/qemu-block/2023-06/msg00184.html > > Andrey Drobyshev (3): > block: add BDRV_BLOCK_COMPRESSED flag for bdrv_block_status() > qemu-img: map: report compressed data blocks > qemu-iotests: update expected tests output to contain "compressed" > field > > block/qcow.c | 5 +- > block/qcow2.c | 3 + > block/vmdk.c | 2 + > include/block/block-common.h | 3 + > qapi/block-core.json | 7 +- > qemu-img.c| 16 +- > tests/qemu-iotests/122.out| 84 > tests/qemu-iotests/154.out| 194 +- > tests/qemu-iotests/179.out| 178 > tests/qemu-iotests/244.out| 24 +-- > tests/qemu-iotests/252.out| 10 +- > tests/qemu-iotests/274.out| 48 ++--- > .../tests/nbd-qemu-allocation.out | 6 +- > 13 files changed, 302 insertions(+), 278 deletions(-) > Ping
[PATCH 20/26] qtest/migration-tests.c: use "-incoming defer" for postcopy tests
From: Wei Wang The Postcopy preempt capability is expected to be set before incoming starts, so change the postcopy tests to start with deferred incoming and call migrate-incoming after the cap has been set. Why the existing tests (without this patch) didn't fail? There could be two reasons: 1) "backlog" specifies the number of pending connections. As long as the server accepts the connections faster than the clients side connecting, connection will succeed. For the preempt test, it uses only 2 channels, so very likely to not have pending connections. 2) per my tests (on kernel 6.2), the number of pending connections allowed is actually "backlog + 1", which is 2 in this case. That said, the implementation of socket_start_incoming_migration_internal expects "migrate defer" to be used, and for safety, change the test to work with the expected usage. Signed-off-by: Wei Wang Reviewed-by: Peter Xu Reviewed-by: Juan Quintela Message-ID: <20230606101910.20456-3-wei.w.w...@intel.com> Signed-off-by: Juan Quintela --- tests/qtest/migration-test.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index fd145e38d9..62d3f37021 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1239,10 +1239,9 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, QTestState **to_ptr, MigrateCommon *args) { -g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *from, *to; -if (test_migrate_start(&from, &to, uri, &args->start)) { +if (test_migrate_start(&from, &to, "defer", &args->start)) { return -1; } @@ -1262,10 +1261,13 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, migrate_ensure_non_converge(from); migrate_prepare_for_dirty_mem(from); +qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming'," + " 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}"); /* Wait for the first serial output from the source */ wait_for_serial("src_serial"); +g_autofree char *uri = migrate_get_socket_address(to, "socket-address"); migrate_qmp(from, uri, "{}"); migrate_wait_for_dirty_mem(from, to); -- 2.40.1
Re: [PATCH 0/6] qemu-img: rebase: add compression support
On 6/30/23 13:54, Denis V. Lunev wrote: > On 6/1/23 21:28, Andrey Drobyshev wrote: >> This series is adding [-c | --compress] option to "qemu-img rebase" >> command, which might prove useful for saving some disk space when, for >> instance, manipulating chains of backup images. Along the way I had to >> make a couple of minor improvements. >> >> The first 2 patches are a bug fix + corresponding test case. >> Patch 3 merely fixes wrong args used in allocation. >> Patch 4 makes write requests during rebase operation >> cluster_size-aligned, >> which seems to be beneficial for both non-compressed and compressed mode. >> The last 2 patches are the actual feature implementation + tests. >> >> Andrey Drobyshev (6): >> qemu-img: rebase: stop when reaching EOF of old backing file >> qemu-iotests: 024: add rebasing test case for overlay_size > >> backing_size >> qemu-img: rebase: use backing files' BlockBackend for buffer alignment >> qemu-img: rebase: avoid unnecessary COW operations >> qemu-img: add compression option to rebase subcommand >> iotests: add test 314 for "qemu-img rebase" with compression >> >> docs/tools/qemu-img.rst | 6 +- >> qemu-img-cmds.hx | 4 +- >> qemu-img.c | 106 ++-- >> tests/qemu-iotests/024 | 57 + >> tests/qemu-iotests/024.out | 30 +++ >> tests/qemu-iotests/314 | 165 + >> tests/qemu-iotests/314.out | 75 + >> 7 files changed, 415 insertions(+), 28 deletions(-) >> create mode 100755 tests/qemu-iotests/314 >> create mode 100644 tests/qemu-iotests/314.out >> > ping Friendly ping after 7 weeks
[PATCH 12/26] migration-test: Make machine_opts regular with other options
Reviewed-by: Peter Xu Signed-off-by: Juan Quintela Message-ID: <20230608224943.3877-5-quint...@redhat.com> --- tests/qtest/migration-test.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 2296ed4bf5..f51a25e299 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -739,7 +739,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, start_address = S390_TEST_MEM_START; end_address = S390_TEST_MEM_END; } else if (strcmp(arch, "ppc64") == 0) { -machine_opts = "vsmt=8"; +machine_opts = "-machine vsmt=8"; memory_size = "256M"; start_address = PPC_TEST_MEM_START; end_address = PPC_TEST_MEM_END; @@ -751,7 +751,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, arch_target = g_strdup("-nodefaults"); } else if (strcmp(arch, "aarch64") == 0) { init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel)); -machine_opts = "virt,gic-version=max"; +machine_opts = "-machine virt,gic-version=max"; memory_size = "150M"; arch_source = g_strdup_printf("-cpu max " "-kernel %s", @@ -791,14 +791,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, shmem_opts = g_strdup(""); } -cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s " +cmd_source = g_strdup_printf("-accel kvm%s -accel tcg %s " "-name source,debug-threads=on " "-m %s " "-serial file:%s/src_serial " "%s %s %s %s", args->use_dirty_ring ? ",dirty-ring-size=4096" : "", - machine_opts ? " -machine " : "", machine_opts ? machine_opts : "", memory_size, tmpfs, arch_source, shmem_opts, @@ -811,7 +810,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, &got_src_stop); } -cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s " +cmd_target = g_strdup_printf("-accel kvm%s -accel tcg %s " "-name target,debug-threads=on " "-m %s " "-serial file:%s/dest_serial " @@ -819,7 +818,6 @@ static int test_migrate_start(QTestState **from, QTestState **to, "%s %s %s %s", args->use_dirty_ring ? ",dirty-ring-size=4096" : "", - machine_opts ? " -machine " : "", machine_opts ? machine_opts : "", memory_size, tmpfs, uri, arch_target, shmem_opts, -- 2.40.1
[PATCH 23/26] qemu_file: Make qemu_file_is_writable() static
It is not used outside of qemu_file, and it shouldn't. Signed-off-by: Juan Quintela Message-ID: <20230530183941.7223-19-quint...@redhat.com> Signed-off-by: Juan Quintela --- migration/qemu-file.h | 1 - migration/qemu-file.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/migration/qemu-file.h b/migration/qemu-file.h index aa6eee66da..a081ef6c3f 100644 --- a/migration/qemu-file.h +++ b/migration/qemu-file.h @@ -103,7 +103,6 @@ uint64_t qemu_file_transferred_noflush(QEMUFile *f); void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size, bool may_free); bool qemu_file_mode_is_not_valid(const char *mode); -bool qemu_file_is_writable(QEMUFile *f); #include "migration/qemu-file-types.h" diff --git a/migration/qemu-file.c b/migration/qemu-file.c index fdf115b5da..9a89e17924 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -228,7 +228,7 @@ void qemu_file_set_error(QEMUFile *f, int ret) qemu_file_set_error_obj(f, ret, NULL); } -bool qemu_file_is_writable(QEMUFile *f) +static bool qemu_file_is_writable(QEMUFile *f) { return f->is_writable; } -- 2.40.1