Re: [PATCH] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job
Am 06.01.23 um 08:49 schrieb Thomas Huth: On 05/01/2023 22.42, Philippe Mathieu-Daudé wrote: > That said, maybe it is time to deprecate the 32-bit > hosts? Certainly fine for me, but that's up to the Windows folks to decide. Maybe you could just suggest a patch to start the discussion? Thomas Download numbers from yesterday for my latest Windows installers: qemu-w32-setup-20221230.exe - 243 qemu-w64-setup-20221230.exe - 6540 On Wednesday the ratio was 288 : 3516. As expected the 64-bit variant is used much more often, but it looks like there is still a certain desire for the 32-bit variant. Stefan
Re: [PATCH] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job
On 06/01/2023 09.15, Stefan Weil wrote: Am 06.01.23 um 08:49 schrieb Thomas Huth: On 05/01/2023 22.42, Philippe Mathieu-Daudé wrote: > That said, maybe it is time to deprecate the 32-bit > hosts? Certainly fine for me, but that's up to the Windows folks to decide. Maybe you could just suggest a patch to start the discussion? Thomas Download numbers from yesterday for my latest Windows installers: qemu-w32-setup-20221230.exe - 243 qemu-w64-setup-20221230.exe - 6540 On Wednesday the ratio was 288 : 3516. As expected the 64-bit variant is used much more often, but it looks like there is still a certain desire for the 32-bit variant. OK, thanks. Could you maybe also check the browser types in the logs? ... I'm wondering whether a big part of those w32 downloads were just automatic web crawlers? Thomas
[PULL 01/15] qemu-iotests/stream-under-throttle: do not shutdown QEMU
From: Christian Borntraeger Without a kernel or boot disk a QEMU on s390 will exit (usually with a disabled wait state). This breaks the stream-under-throttle test case. Do not exit qemu if on s390. Signed-off-by: Christian Borntraeger Message-Id: <20221207131452.8455-1-borntrae...@linux.ibm.com> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- tests/qemu-iotests/tests/stream-under-throttle | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qemu-iotests/tests/stream-under-throttle b/tests/qemu-iotests/tests/stream-under-throttle index 8d2d9e1684..c24dfbcaa2 100755 --- a/tests/qemu-iotests/tests/stream-under-throttle +++ b/tests/qemu-iotests/tests/stream-under-throttle @@ -88,6 +88,8 @@ class TestStreamWithThrottle(iotests.QMPTestCase): 'x-iops-total=1,x-bps-total=104857600') self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev)) self.vm.add_device('virtio-blk,iothread=iothr0,drive=throttled-node') +if iotests.qemu_default_machine == 's390-ccw-virtio': +self.vm.add_args('-no-shutdown') self.vm.launch() def tearDown(self) -> None: -- 2.31.1
[PULL 05/15] hw/s390x/pv: Restrict Protected Virtualization to sysemu
From: Philippe Mathieu-Daudé Protected Virtualization is irrelevant in user emulation. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20221217152454.96388-4-phi...@linaro.org> Reviewed-by: Thomas Huth Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- target/s390x/cpu_features.c | 4 target/s390x/cpu_models.c | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c index 5528acd082..2e4e11d264 100644 --- a/target/s390x/cpu_features.c +++ b/target/s390x/cpu_features.c @@ -14,7 +14,9 @@ #include "qemu/osdep.h" #include "qemu/module.h" #include "cpu_features.h" +#ifndef CONFIG_USER_ONLY #include "hw/s390x/pv.h" +#endif #define DEF_FEAT(_FEAT, _NAME, _TYPE, _BIT, _DESC) \ [S390_FEAT_##_FEAT] = {\ @@ -107,6 +109,7 @@ void s390_fill_feat_block(const S390FeatBitmap features, S390FeatType type, feat = find_next_bit(features, S390_FEAT_MAX, feat + 1); } +#ifndef CONFIG_USER_ONLY if (!s390_is_pv()) { return; } @@ -147,6 +150,7 @@ void s390_fill_feat_block(const S390FeatBitmap features, S390FeatType type, default: return; } +#endif } void s390_add_from_feat_block(S390FeatBitmap features, S390FeatType type, diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index c3a4f80633..065ec6d66c 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -23,8 +23,8 @@ #include "qemu/qemu-print.h" #ifndef CONFIG_USER_ONLY #include "sysemu/sysemu.h" -#endif #include "hw/s390x/pv.h" +#endif #define CPUDEF_INIT(_type, _gen, _ec_ga, _mha_pow, _hmfai, _name, _desc) \ {\ @@ -236,6 +236,7 @@ bool s390_has_feat(S390Feat feat) return 0; } +#ifndef CONFIG_USER_ONLY if (s390_is_pv()) { switch (feat) { case S390_FEAT_DIAG_318: @@ -259,6 +260,7 @@ bool s390_has_feat(S390Feat feat) break; } } +#endif return test_bit(feat, cpu->model->features); } -- 2.31.1
[PULL 04/15] exec/memory: Expose memory_region_access_valid()
From: Philippe Mathieu-Daudé Instead of having hardware device poking into memory internal API, expose memory_region_access_valid(). Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20221217152454.96388-2-phi...@linaro.org> Reviewed-by: Eric Farman Reviewed-by: Thomas Huth Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- include/exec/memory-internal.h | 4 include/exec/memory.h | 4 hw/s390x/s390-pci-inst.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h index 9fcc2af25c..100c1237ac 100644 --- a/include/exec/memory-internal.h +++ b/include/exec/memory-internal.h @@ -38,10 +38,6 @@ void flatview_unref(FlatView *view); extern const MemoryRegionOps unassigned_mem_ops; -bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr, -unsigned size, bool is_write, -MemTxAttrs attrs); - void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section); AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv); void address_space_dispatch_compact(AddressSpaceDispatch *d); diff --git a/include/exec/memory.h b/include/exec/memory.h index 91f8a2395a..c37ffdbcd1 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -2442,6 +2442,10 @@ void memory_global_dirty_log_stop(unsigned int flags); void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled); +bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr, +unsigned size, bool is_write, +MemTxAttrs attrs); + /** * memory_region_dispatch_read: perform a read directly to the specified * MemoryRegion. diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index 9abe95130c..2eee5db7e1 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -13,7 +13,7 @@ #include "qemu/osdep.h" #include "exec/memop.h" -#include "exec/memory-internal.h" +#include "exec/memory.h" #include "qemu/error-report.h" #include "sysemu/hw_accel.h" #include "hw/s390x/s390-pci-inst.h" -- 2.31.1
[PULL 07/15] target/s390x/tcg/excp_helper: Restrict system headers to sysemu
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20221217152454.96388-6-phi...@linaro.org> Reviewed-by: Thomas Huth Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- target/s390x/tcg/excp_helper.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c index fe02d82201..bc767f0443 100644 --- a/target/s390x/tcg/excp_helper.c +++ b/target/s390x/tcg/excp_helper.c @@ -21,15 +21,15 @@ #include "qemu/osdep.h" #include "qemu/log.h" #include "cpu.h" -#include "s390x-internal.h" #include "exec/helper-proto.h" -#include "qemu/timer.h" #include "exec/exec-all.h" #include "exec/cpu_ldst.h" -#include "hw/s390x/ioinst.h" -#include "exec/address-spaces.h" +#include "s390x-internal.h" #include "tcg_s390x.h" #ifndef CONFIG_USER_ONLY +#include "qemu/timer.h" +#include "exec/address-spaces.h" +#include "hw/s390x/ioinst.h" #include "hw/s390x/s390_flic.h" #include "hw/boards.h" #endif -- 2.31.1
[PULL 14/15] error handling: Use RETRY_ON_EINTR() macro where applicable
From: Nikita Ivanov There is a defined RETRY_ON_EINTR() macro in qemu/osdep.h which handles the same while loop. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/415 Signed-off-by: Nikita Ivanov Message-Id: <20221023090422.242617-3-niva...@cloudlinux.com> Reviewed-by: Marc-André Lureau [thuth: Dropped the hunk that changed socket_accept() in libqtest.c] Signed-off-by: Thomas Huth --- block/file-posix.c| 37 - chardev/char-pty.c| 4 +--- hw/9pfs/9p-local.c| 8 ++-- net/l2tpv3.c | 17 + net/socket.c | 16 +++- net/tap.c | 8 ++-- qga/commands-posix.c | 4 +--- semihosting/syscalls.c| 4 +--- tests/qtest/libqtest.c| 4 +--- tests/vhost-user-bridge.c | 4 +--- util/main-loop.c | 4 +--- util/osdep.c | 4 +--- util/vfio-helpers.c | 12 ++-- 13 files changed, 45 insertions(+), 81 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index b9647c5ffc..b9955db205 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1229,9 +1229,7 @@ static int hdev_get_max_segments(int fd, struct stat *st) ret = -errno; goto out; } -do { -ret = read(sysfd, buf, sizeof(buf) - 1); -} while (ret == -1 && errno == EINTR); +ret = RETRY_ON_EINTR(read(sysfd, buf, sizeof(buf) - 1)); if (ret < 0) { ret = -errno; goto out; @@ -1379,9 +1377,9 @@ static int handle_aiocb_ioctl(void *opaque) RawPosixAIOData *aiocb = opaque; int ret; -do { -ret = ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf); -} while (ret == -1 && errno == EINTR); +ret = RETRY_ON_EINTR( +ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf) +); if (ret == -1) { return -errno; } @@ -1463,18 +1461,17 @@ static ssize_t handle_aiocb_rw_vector(RawPosixAIOData *aiocb) { ssize_t len; -do { -if (aiocb->aio_type & QEMU_AIO_WRITE) -len = qemu_pwritev(aiocb->aio_fildes, - aiocb->io.iov, - aiocb->io.niov, - aiocb->aio_offset); - else -len = qemu_preadv(aiocb->aio_fildes, - aiocb->io.iov, - aiocb->io.niov, - aiocb->aio_offset); -} while (len == -1 && errno == EINTR); +len = RETRY_ON_EINTR( +(aiocb->aio_type & QEMU_AIO_WRITE) ? +qemu_pwritev(aiocb->aio_fildes, + aiocb->io.iov, + aiocb->io.niov, + aiocb->aio_offset) : +qemu_preadv(aiocb->aio_fildes, + aiocb->io.iov, + aiocb->io.niov, + aiocb->aio_offset) +); if (len == -1) { return -errno; @@ -1899,9 +1896,7 @@ static int allocate_first_block(int fd, size_t max_size) buf = qemu_memalign(max_align, write_size); memset(buf, 0, write_size); -do { -n = pwrite(fd, buf, write_size, 0); -} while (n == -1 && errno == EINTR); +n = RETRY_ON_EINTR(pwrite(fd, buf, write_size, 0)); ret = (n == -1) ? -errno : 0; diff --git a/chardev/char-pty.c b/chardev/char-pty.c index 53f25c6bbd..92fd33c854 100644 --- a/chardev/char-pty.c +++ b/chardev/char-pty.c @@ -93,9 +93,7 @@ static void pty_chr_update_read_handler(Chardev *chr) pfd.fd = fioc->fd; pfd.events = G_IO_OUT; pfd.revents = 0; -do { -rc = g_poll(&pfd, 1, 0); -} while (rc == -1 && errno == EINTR); +rc = RETRY_ON_EINTR(g_poll(&pfd, 1, 0)); assert(rc >= 0); if (pfd.revents & G_IO_HUP) { diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index d2246a3d7e..9d07620235 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -470,9 +470,7 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path, if (fd == -1) { return -1; } -do { -tsize = read(fd, (void *)buf, bufsz); -} while (tsize == -1 && errno == EINTR); +tsize = RETRY_ON_EINTR(read(fd, (void *)buf, bufsz)); close_preserve_errno(fd); } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) || (fs_ctx->export_flags & V9FS_SM_NONE)) { @@ -908,9 +906,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, } /* Write the oldpath (target) to the file. */ oldpath_size = strlen(oldpath); -do { -write_size = write(fd, (void *)oldpath, oldpath_size); -} while (write_size == -1 && errno == EINTR); +write_size = RETRY_ON_EINTR(write(fd, (void *)oldpath, oldpath_size)); close_preserve_errno(fd); if (write_size != oldpath_size) { diff --git a/net/l2tpv3.c b/net/l2tpv
[PULL 08/15] target/s390x: Restrict sysemu/reset.h to system emulation
From: Philippe Mathieu-Daudé In user emulation, threads -- implemented as CPU -- are created/destroyed, but never reset. There is no point in allowing the user emulation access the sysemu/reset API. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20221220145625.26392-5-phi...@linaro.org> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- target/s390x/cpu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index 96562c516d..b10a8541ff 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -26,7 +26,6 @@ #include "s390x-internal.h" #include "kvm/kvm_s390x.h" #include "sysemu/kvm.h" -#include "sysemu/reset.h" #include "qemu/module.h" #include "trace.h" #include "qapi/qapi-types-machine.h" @@ -35,6 +34,9 @@ #include "fpu/softfloat-helpers.h" #include "disas/capstone.h" #include "sysemu/tcg.h" +#ifndef CONFIG_USER_ONLY +#include "sysemu/reset.h" +#endif #define CR0_RESET 0xE0UL #define CR14_RESET 0xC200UL; -- 2.31.1
[PULL 10/15] i386: Deprecate the -no-hpet QEMU command line option
The HPET setting has been turned into a machine property a while ago already, so we should finally do the next step and deprecate the legacy CLI option, too. Message-Id: <20221229114913.260400-1-th...@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Ján Tomko Signed-off-by: Thomas Huth --- docs/about/deprecated.rst | 6 ++ softmmu/vl.c | 1 + qemu-options.hx | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 93affe3669..2ae6a79b21 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -114,6 +114,12 @@ form is preferred. Using ``-drive if=none`` to configure the OTP device of the sifive_u RISC-V machine is deprecated. Use ``-drive if=pflash`` instead. +``-no-hpet`` (since 8.0) + + +The HPET setting has been turned into a machine property. +Use ``-machine hpet=off`` instead. + QEMU Machine Protocol (QMP) commands diff --git a/softmmu/vl.c b/softmmu/vl.c index 798e1dc933..9bd0e52d01 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -3259,6 +3259,7 @@ void qemu_init(int argc, char **argv) qdict_put_str(machine_opts_dict, "acpi", "off"); break; case QEMU_OPTION_no_hpet: +warn_report("-no-hpet is deprecated, use '-machine hpet=off' instead"); qdict_put_str(machine_opts_dict, "hpet", "off"); break; case QEMU_OPTION_no_reboot: diff --git a/qemu-options.hx b/qemu-options.hx index 7f99d15b23..a3adb4163e 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2542,7 +2542,7 @@ DEF("no-hpet", 0, QEMU_OPTION_no_hpet, "-no-hpetdisable HPET\n", QEMU_ARCH_I386) SRST ``-no-hpet`` -Disable HPET support. +Disable HPET support. Deprecated, use '-machine hpet=off' instead. ERST DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable, -- 2.31.1
[PULL 13/15] Refactoring: refactor TFR() macro to RETRY_ON_EINTR()
From: Nikita Ivanov Rename macro name to more transparent one and refactor it to expression. Signed-off-by: Nikita Ivanov Message-Id: <20221023090422.242617-2-niva...@cloudlinux.com> Reviewed-by: Marc-André Lureau Reviewed-by: Bin Meng Reviewed-by: Christian Schoenebeck Signed-off-by: Thomas Huth --- include/qemu/osdep.h | 8 +++- chardev/char-fd.c | 2 +- chardev/char-pipe.c| 8 +--- net/tap-bsd.c | 6 +++--- net/tap-linux.c| 2 +- net/tap-solaris.c | 8 net/tap.c | 2 +- os-posix.c | 2 +- tests/qtest/libqtest.c | 2 +- 9 files changed, 24 insertions(+), 16 deletions(-) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index b9c4307779..7d059ad526 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -251,7 +251,13 @@ void QEMU_ERROR("code path is reachable") #define ESHUTDOWN 4099 #endif -#define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR) +#define RETRY_ON_EINTR(expr) \ +(__extension__ \ +({ typeof(expr) __result; \ + do { \ +__result = (expr); \ + } while (__result == -1 && errno == EINTR); \ + __result; })) /* time_t may be either 32 or 64 bits depending on the host OS, and * can be either signed or unsigned, so we can't just hardcode a diff --git a/chardev/char-fd.c b/chardev/char-fd.c index cf78454841..d2c4923359 100644 --- a/chardev/char-fd.c +++ b/chardev/char-fd.c @@ -198,7 +198,7 @@ int qmp_chardev_open_file_source(char *src, int flags, Error **errp) { int fd = -1; -TFR(fd = qemu_open_old(src, flags, 0666)); +fd = RETRY_ON_EINTR(qemu_open_old(src, flags, 0666)); if (fd == -1) { error_setg_file_open(errp, errno, src); } diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c index 66d3b85091..5ad30bcc59 100644 --- a/chardev/char-pipe.c +++ b/chardev/char-pipe.c @@ -131,8 +131,8 @@ static void qemu_chr_open_pipe(Chardev *chr, filename_in = g_strdup_printf("%s.in", filename); filename_out = g_strdup_printf("%s.out", filename); -TFR(fd_in = qemu_open_old(filename_in, O_RDWR | O_BINARY)); -TFR(fd_out = qemu_open_old(filename_out, O_RDWR | O_BINARY)); +fd_in = RETRY_ON_EINTR(qemu_open_old(filename_in, O_RDWR | O_BINARY)); +fd_out = RETRY_ON_EINTR(qemu_open_old(filename_out, O_RDWR | O_BINARY)); g_free(filename_in); g_free(filename_out); if (fd_in < 0 || fd_out < 0) { @@ -142,7 +142,9 @@ static void qemu_chr_open_pipe(Chardev *chr, if (fd_out >= 0) { close(fd_out); } -TFR(fd_in = fd_out = qemu_open_old(filename, O_RDWR | O_BINARY)); +fd_in = fd_out = RETRY_ON_EINTR( +qemu_open_old(filename, O_RDWR | O_BINARY) +); if (fd_in < 0) { error_setg_file_open(errp, errno, filename); return; diff --git a/net/tap-bsd.c b/net/tap-bsd.c index 005ce05c6e..4c98fdd337 100644 --- a/net/tap-bsd.c +++ b/net/tap-bsd.c @@ -56,7 +56,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, } else { snprintf(dname, sizeof dname, "/dev/tap%d", i); } -TFR(fd = open(dname, O_RDWR)); +fd = RETRY_ON_EINTR(open(dname, O_RDWR)); if (fd >= 0) { break; } @@ -111,7 +111,7 @@ static int tap_open_clone(char *ifname, int ifname_size, Error **errp) int fd, s, ret; struct ifreq ifr; -TFR(fd = open(PATH_NET_TAP, O_RDWR)); +fd = RETRY_ON_EINTR(open(PATH_NET_TAP, O_RDWR)); if (fd < 0) { error_setg_errno(errp, errno, "could not open %s", PATH_NET_TAP); return -1; @@ -159,7 +159,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, if (ifname[0] != '\0') { char dname[100]; snprintf(dname, sizeof dname, "/dev/%s", ifname); -TFR(fd = open(dname, O_RDWR)); +fd = RETRY_ON_EINTR(open(dname, O_RDWR)); if (fd < 0 && errno != ENOENT) { error_setg_errno(errp, errno, "could not open %s", dname); return -1; diff --git a/net/tap-linux.c b/net/tap-linux.c index 304ff45071..f54f308d35 100644 --- a/net/tap-linux.c +++ b/net/tap-linux.c @@ -45,7 +45,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int len = sizeof(struct virtio_net_hdr); unsigned int features; -TFR(fd = open(PATH_NET_TUN, O_RDWR)); +fd = RETRY_ON_EINTR(open(PATH_NET_TUN, O_RDWR)); if (fd < 0) { error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN); return -1; diff --git a/net/tap-solaris.c b/net/tap-solaris.c index a44f8805c2..38e15028bf 100644 --- a/net/tap-solaris.c +++ b/net/tap-solaris.c @@ -84,13 +84,13 @@ static int tap_alloc(char *dev, size_t dev_size, Error **errp) if( ip_fd ) close(ip_fd); -TFR(ip_fd
[PULL 09/15] tests/readconfig: spice doesn't support unix socket on windows yet
From: Marc-André Lureau Signed-off-by: Marc-André Lureau Message-Id: <20230103110814.3726795-6-marcandre.lur...@redhat.com> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- tests/qtest/readconfig-test.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c index c7a9b0c7dd..9ef870643d 100644 --- a/tests/qtest/readconfig-test.c +++ b/tests/qtest/readconfig-test.c @@ -109,8 +109,10 @@ static void test_spice(void) QTestState *qts; const char *cfgdata = "[spice]\n" -"disable-ticketing = \"on\"\n" -"unix = \"on\"\n"; +#ifndef WIN32 +"unix = \"on\"\n" +#endif +"disable-ticketing = \"on\"\n"; qts = qtest_init_with_config(cfgdata); /* Test valid command */ -- 2.31.1
[PULL 15/15] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job
The qtests are not stable in the msys2-32bit job yet - especially the test-hmp and the qom-test are failing randomly. Until this is fixed, let's better disable the qtests here again to avoid failing CI tests. Message-Id: <20230105204819.26992-1-th...@redhat.com> Signed-off-by: Thomas Huth --- .gitlab-ci.d/windows.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml index 22f794e537..a1d5790580 100644 --- a/.gitlab-ci.d/windows.yml +++ b/.gitlab-ci.d/windows.yml @@ -116,4 +116,5 @@ msys2-32bit: - ..\msys64\usr\bin\bash -lc '../configure --target-list=ppc64-softmmu --disable-opengl' - ..\msys64\usr\bin\bash -lc 'make' - - ..\msys64\usr\bin\bash -lc 'make check || { cat meson-logs/testlog.txt; exit 1; } ;' + - ..\msys64\usr\bin\bash -lc 'make check MTESTARGS=\"--no-suite qtest\" || +{ cat meson-logs/testlog.txt; exit 1; }' -- 2.31.1
[PULL 00/15] First batch of s390x, qtests and misc fixes in 2023
Hi Peter! The following changes since commit cb9c6a8e5ad6a1f0ce164d352e3102df46986e22: .gitlab-ci.d/windows: Work-around timeout and OpenGL problems of the MSYS2 jobs (2023-01-04 18:58:33 +) are available in the Git repository at: https://gitlab.com/thuth/qemu.git tags/pull-request-2023-01-06 for you to fetch changes up to 975f619662a46cb5dc7a3b17b84a1b540fb7df5c: .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job (2023-01-05 21:50:21 +0100) * s390x header clean-ups from Philippe * Rework and improvements of the EINTR handling by Nikita * Deprecate the -no-hpet command line option * Disable the qtests in the 32-bit Windows CI job again * Some other misc fixes here and there Alessandro Di Federico (1): Update scripts/meson-buildoptions.sh Christian Borntraeger (1): qemu-iotests/stream-under-throttle: do not shutdown QEMU Marc-André Lureau (1): tests/readconfig: spice doesn't support unix socket on windows yet Nikita Ivanov (2): Refactoring: refactor TFR() macro to RETRY_ON_EINTR() error handling: Use RETRY_ON_EINTR() macro where applicable Philippe Mathieu-Daudé (6): tests/vm: Update get_default_jobs() to work on non-x86_64 non-KVM hosts exec/memory: Expose memory_region_access_valid() hw/s390x/pv: Restrict Protected Virtualization to sysemu target/s390x/tcg/misc_helper: Remove unused "memory.h" include target/s390x/tcg/excp_helper: Restrict system headers to sysemu target/s390x: Restrict sysemu/reset.h to system emulation Thomas Huth (4): MAINTAINERS: Add MIPS-related docs and configs to the MIPS architecture section i386: Deprecate the -no-hpet QEMU command line option docs/interop: Change the vnc-ledstate-Pseudo-encoding doc into .rst .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job docs/about/deprecated.rst | 6 docs/interop/index.rst | 1 + ...coding.txt => vnc-ledstate-pseudo-encoding.rst} | 0 include/exec/memory-internal.h | 4 --- include/exec/memory.h | 4 +++ include/qemu/osdep.h | 8 - block/file-posix.c | 37 ++ chardev/char-fd.c | 2 +- chardev/char-pipe.c| 8 +++-- chardev/char-pty.c | 4 +-- hw/9pfs/9p-local.c | 8 ++--- hw/s390x/s390-pci-inst.c | 2 +- net/l2tpv3.c | 17 +++--- net/socket.c | 16 -- net/tap-bsd.c | 6 ++-- net/tap-linux.c| 2 +- net/tap-solaris.c | 8 ++--- net/tap.c | 10 ++ os-posix.c | 2 +- qga/commands-posix.c | 4 +-- semihosting/syscalls.c | 4 +-- softmmu/vl.c | 1 + target/s390x/cpu.c | 4 ++- target/s390x/cpu_features.c| 4 +++ target/s390x/cpu_models.c | 4 ++- target/s390x/tcg/excp_helper.c | 8 ++--- target/s390x/tcg/misc_helper.c | 1 - tests/qtest/libqtest.c | 6 ++-- tests/qtest/readconfig-test.c | 6 ++-- tests/vhost-user-bridge.c | 4 +-- util/main-loop.c | 4 +-- util/osdep.c | 4 +-- util/vfio-helpers.c| 12 +++ .gitlab-ci.d/windows.yml | 3 +- MAINTAINERS| 2 ++ qemu-options.hx| 2 +- scripts/meson-buildoptions.sh | 18 +++ tests/qemu-iotests/tests/stream-under-throttle | 2 ++ tests/vm/basevm.py | 3 +- 39 files changed, 120 insertions(+), 121 deletions(-) rename docs/interop/{vnc-ledstate-Pseudo-encoding.txt => vnc-ledstate-pseudo-encoding.rst} (100%)
[PULL 06/15] target/s390x/tcg/misc_helper: Remove unused "memory.h" include
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20221217152454.96388-5-phi...@linaro.org> Reviewed-by: Thomas Huth Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- target/s390x/tcg/misc_helper.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c index 71388a7119..576157b1f3 100644 --- a/target/s390x/tcg/misc_helper.c +++ b/target/s390x/tcg/misc_helper.c @@ -23,7 +23,6 @@ #include "qemu/main-loop.h" #include "cpu.h" #include "s390x-internal.h" -#include "exec/memory.h" #include "qemu/host-utils.h" #include "exec/helper-proto.h" #include "qemu/timer.h" -- 2.31.1
[PATCH v3 6/6] i386: Add new CPU model SapphireRapids
The new CPU model mostly inherits features from Icelake-Server, while adding new features: - AMX (Advance Matrix eXtensions) - Bus Lock Debug Exception and new instructions: - AVX VNNI (Vector Neural Network Instruction): - VPDPBUS: Multiply and Add Unsigned and Signed Bytes - VPDPBUSDS: Multiply and Add Unsigned and Signed Bytes with Saturation - VPDPWSSD: Multiply and Add Signed Word Integers - VPDPWSSDS: Multiply and Add Signed Integers with Saturation - FP16: Replicates existing AVX512 computational SP (FP32) instructions using FP16 instead of FP32 for ~2X performance gain - SERIALIZE: Provide software with a simple way to force the processor to complete all modifications, faster, allowed in all privilege levels and not causing an unconditional VM exit - TSX Suspend Load Address Tracking: Allows programmers to choose which memory accesses do not need to be tracked in the TSX read set - AVX512_BF16: Vector Neural Network Instructions supporting BFLOAT16 inputs and conversion instructions from IEEE single precision Features may be added in future versions: - CET (virtualization support hasn't been merged) Instructions may be added in future versions: - fast zero-length MOVSB (KVM doesn't support yet) - fast short STOSB (KVM doesn't support yet) - fast short CMPSB, SCASB (KVM doesn't support yet) Signed-off-by: Lei Wang Reviewed-by: Robert Hoo --- target/i386/cpu.c | 135 ++ target/i386/cpu.h | 4 ++ 2 files changed, 139 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 946df29a3d..5e1ecd50b3 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3576,6 +3576,141 @@ static const X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ } } }, +{ +.name = "SapphireRapids", +.level = 0x20, +.vendor = CPUID_VENDOR_INTEL, +.family = 6, +.model = 143, +.stepping = 4, +/* + * please keep the ascending order so that we can have a clear view of + * bit position of each feature. + */ +.features[FEAT_1_EDX] = +CPUID_FP87 | CPUID_VME | CPUID_DE | CPUID_PSE | CPUID_TSC | +CPUID_MSR | CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | +CPUID_SEP | CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | +CPUID_PAT | CPUID_PSE36 | CPUID_CLFLUSH | CPUID_MMX | CPUID_FXSR | +CPUID_SSE | CPUID_SSE2, +.features[FEAT_1_ECX] = +CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSSE3 | +CPUID_EXT_FMA | CPUID_EXT_CX16 | CPUID_EXT_PCID | CPUID_EXT_SSE41 | +CPUID_EXT_SSE42 | CPUID_EXT_X2APIC | CPUID_EXT_MOVBE | +CPUID_EXT_POPCNT | CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_AES | +CPUID_EXT_XSAVE | CPUID_EXT_AVX | CPUID_EXT_F16C | CPUID_EXT_RDRAND, +.features[FEAT_8000_0001_EDX] = +CPUID_EXT2_SYSCALL | CPUID_EXT2_NX | CPUID_EXT2_PDPE1GB | +CPUID_EXT2_RDTSCP | CPUID_EXT2_LM, +.features[FEAT_8000_0001_ECX] = +CPUID_EXT3_LAHF_LM | CPUID_EXT3_ABM | CPUID_EXT3_3DNOWPREFETCH, +.features[FEAT_8000_0008_EBX] = +CPUID_8000_0008_EBX_WBNOINVD, +.features[FEAT_7_0_EBX] = +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_HLE | +CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | +CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID | CPUID_7_0_EBX_RTM | +CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ | +CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_SMAP | +CPUID_7_0_EBX_AVX512IFMA | CPUID_7_0_EBX_CLFLUSHOPT | +CPUID_7_0_EBX_CLWB | CPUID_7_0_EBX_AVX512CD | CPUID_7_0_EBX_SHA_NI | +CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512VL, +.features[FEAT_7_0_ECX] = +CPUID_7_0_ECX_AVX512_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU | +CPUID_7_0_ECX_AVX512_VBMI2 | CPUID_7_0_ECX_GFNI | +CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ | +CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG | +CPUID_7_0_ECX_AVX512_VPOPCNTDQ | CPUID_7_0_ECX_LA57 | +CPUID_7_0_ECX_RDPID | CPUID_7_0_ECX_BUS_LOCK_DETECT, +.features[FEAT_7_0_EDX] = +CPUID_7_0_EDX_FSRM | CPUID_7_0_EDX_SERIALIZE | +CPUID_7_0_EDX_TSX_LDTRK | CPUID_7_0_EDX_AMX_BF16 | +CPUID_7_0_EDX_AVX512_FP16 | CPUID_7_0_EDX_AMX_TILE | +CPUID_7_0_EDX_AMX_INT8 | CPUID_7_0_EDX_SPEC_CTRL | +CPUID_7_0_EDX_ARCH_CAPABILITIES | CPUID_7_0_EDX_SPEC_CTRL_SSBD, +.features[FEAT_ARCH_CAPABILITIES] = +MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL | +MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO | +MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO, +.features[FEAT_XSAVE] = +
[PULL 03/15] MAINTAINERS: Add MIPS-related docs and configs to the MIPS architecture section
docs/system/target-mips.rst and configs/targets/mips* are not covered in our MAINTAINERS file yet, so let's add them now. Message-Id: <20221212171252.194864-1-th...@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7a40d4d865..5606e5dbd2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -113,6 +113,8 @@ M: Philippe Mathieu-Daudé R: Jiaxun Yang S: Odd Fixes K: ^Subject:.*(?i)mips +F: docs/system/target-mips.rst +F: configs/targets/mips* Guest CPU cores (TCG) - -- 2.31.1
[PATCH v3 3/6] i386: Introduce new struct "MultiBitFeatureInfo" for multi-bit features
Some features use multiple CPUID bits to form a value to be used, e.g., CPUID(0x1E,0):EBX[23:08] is regarded as the tmul_maxn value for AMX. Introduce a new struct "MultiBitFeatureInfo" to hold the information for those features and create a corresponding member in struct FeatureWordInfo, so that the infomation can be assigned for each item in feature_word_info array and used in the future. Signed-off-by: Lei Wang --- target/i386/cpu-internal.h | 9 +++ target/i386/cpu.c | 54 ++ 2 files changed, 63 insertions(+) diff --git a/target/i386/cpu-internal.h b/target/i386/cpu-internal.h index 9baac5c0b4..66b3d66cb4 100644 --- a/target/i386/cpu-internal.h +++ b/target/i386/cpu-internal.h @@ -25,6 +25,13 @@ typedef enum FeatureWordType { MSR_FEATURE_WORD, } FeatureWordType; +typedef struct MultiBitFeatureInfo { +const char *feat_name; +uint64_t mask; +unsigned high_bit_position; +unsigned low_bit_position; +} MultiBitFeatureInfo; + typedef struct FeatureWordInfo { FeatureWordType type; /* feature flags names are taken from "Intel Processor Identification and @@ -51,6 +58,8 @@ typedef struct FeatureWordInfo { uint64_t migratable_flags; /* Feature flags known to be migratable */ /* Features that shouldn't be auto-enabled by "-cpu host" */ uint64_t no_autoenable_flags; +unsigned num_multi_bit_features; +MultiBitFeatureInfo *multi_bit_features; } FeatureWordInfo; extern FeatureWordInfo feature_word_info[]; diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 883098bc5a..88aa780566 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1011,6 +1011,21 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, .migratable_flags = CPUID_AMX_PALETTE_1_TOTAL_TILE_BYTES_MASK | CPUID_AMX_PALETTE_1_BYTES_PER_TILE_MASK, +.num_multi_bit_features = 2, +.multi_bit_features = (MultiBitFeatureInfo[]){ +{ +.feat_name = "total_tile_bytes", +.mask = CPUID_AMX_PALETTE_1_TOTAL_TILE_BYTES_MASK, +.high_bit_position = 15, +.low_bit_position = 0, +}, +{ +.feat_name = "bytes_per_tile", +.mask = CPUID_AMX_PALETTE_1_BYTES_PER_TILE_MASK, +.high_bit_position = 31, +.low_bit_position = 16, +}, +}, }, [FEAT_1D_1_EBX] = { .type = CPUID_FEATURE_WORD, @@ -1021,6 +1036,21 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, .migratable_flags = CPUID_AMX_PALETTE_1_BYTES_PER_ROW_MASK | CPUID_AMX_PALETTE_1_MAX_NAMES_MASK, +.num_multi_bit_features = 2, +.multi_bit_features = (MultiBitFeatureInfo[]){ +{ +.feat_name = "bytes_per_row", +.mask = CPUID_AMX_PALETTE_1_BYTES_PER_ROW_MASK, +.high_bit_position = 15, +.low_bit_position = 0, +}, +{ +.feat_name = "max_names", +.mask = CPUID_AMX_PALETTE_1_MAX_NAMES_MASK, +.high_bit_position = 31, +.low_bit_position = 16, +}, +}, }, [FEAT_1D_1_ECX] = { .type = CPUID_FEATURE_WORD, @@ -1030,6 +1060,15 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .reg = R_ECX, }, .migratable_flags = CPUID_AMX_PALETTE_1_MAX_ROWS_MASK, +.num_multi_bit_features = 1, +.multi_bit_features = (MultiBitFeatureInfo[]){ +{ +.feat_name = "max_rows", +.mask = CPUID_AMX_PALETTE_1_MAX_ROWS_MASK, +.high_bit_position = 15, +.low_bit_position = 0, +}, +}, }, [FEAT_1E_0_EBX] = { .type = CPUID_FEATURE_WORD, @@ -1040,6 +1079,21 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, .migratable_flags = CPUID_AMX_TMUL_MAX_K_MASK | CPUID_AMX_TMUL_MAX_N_MASK, +.num_multi_bit_features = 2, +.multi_bit_features = (MultiBitFeatureInfo[]){ +{ +.feat_name = "tmul_maxk", +.mask = CPUID_AMX_TMUL_MAX_K_MASK, +.high_bit_position = 7, +.low_bit_position = 0, +}, +{ +.feat_name = "tmul_maxn", +.mask = CPUID_AMX_TMUL_MAX_N_MASK, +.high_bit_position = 23, +.low_bit_position = 8, +}, +}, }, /*Below are MSR exposed features*/ [FEAT_ARCH_CAPABILITIES] = { -- 2.34.1
[PATCH v3 2/6] i386: Remove unused parameter "uint32_t bit" in feature_word_description()
Parameter "uint32_t bit" is not used in function feature_word_description(), so remove it. Signed-off-by: Lei Wang --- target/i386/cpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index b6d1247e5e..883098bc5a 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4290,7 +4290,7 @@ static const TypeInfo max_x86_cpu_type_info = { .class_init = max_x86_cpu_class_init, }; -static char *feature_word_description(FeatureWordInfo *f, uint32_t bit) +static char *feature_word_description(FeatureWordInfo *f) { assert(f->type == CPUID_FEATURE_WORD || f->type == MSR_FEATURE_WORD); @@ -4329,6 +4329,7 @@ static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint64_t mask, CPUX86State *env = &cpu->env; FeatureWordInfo *f = &feature_word_info[w]; int i; +g_autofree char *feat_word_str = feature_word_description(f); if (!cpu->force_features) { env->features[w] &= ~mask; @@ -4341,7 +4342,6 @@ static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint64_t mask, for (i = 0; i < 64; ++i) { if ((1ULL << i) & mask) { -g_autofree char *feat_word_str = feature_word_description(f, i); warn_report("%s: %s%s%s [bit %d]", verbose_prefix, feat_word_str, -- 2.34.1
[PATCH v3 0/6] Support for new CPU model SapphireRapids
This series aims to add a new CPU model SapphireRapids, and tries to address the problem stated in https://lore.kernel.org/all/20220812055751.14553-1-lei4.w...@intel.com/T/#mcf67dbd1ad37c65d7988c36a2b267be9afd2fb30, so that named CPU model can define its own AMX values, and QEMU won't pass the wrong AMX values to KVM in future platforms if they have different values supported. The original patch is https://lore.kernel.org/all/20220812055751.14553-1-lei4.w...@intel.com/T/#u. --- Changelog: v3: - Rebase on the latest QEMU (d1852caab131ea898134fdcea8c14bc2ee75fbe9). - v2: https://lore.kernel.org/qemu-devel/20221102085256.81139-1-lei4.w...@intel.com/ v2: - Fix when passing all zeros of AMX-related CPUID, QEMU will warn unsupported. - Remove unnecessary function definition and make code cleaner. - Fix some typos. - v1: https://lore.kernel.org/qemu-devel/20221027020036.373140-1-lei4.w...@intel.com/T/#t Lei Wang (6): i386: Introduce FeatureWordInfo for AMX CPUID leaf 0x1D and 0x1E i386: Remove unused parameter "uint32_t bit" in feature_word_description() i386: Introduce new struct "MultiBitFeatureInfo" for multi-bit features i386: Mask and report unavailable multi-bit feature values i386: Initialize AMX CPUID leaves with corresponding env->features[] leaves i386: Add new CPU model SapphireRapids target/i386/cpu-internal.h | 11 ++ target/i386/cpu.c | 311 +++-- target/i386/cpu.h | 16 ++ 3 files changed, 322 insertions(+), 16 deletions(-) base-commit: d1852caab131ea898134fdcea8c14bc2ee75fbe9 -- 2.34.1
[PULL 02/15] tests/vm: Update get_default_jobs() to work on non-x86_64 non-KVM hosts
From: Philippe Mathieu-Daudé On non-x86_64 host, if KVM is not available we get: Traceback (most recent call last): File "tests/vm/basevm.py", line 634, in main vm = vmcls(args, config=config) File "tests/vm/basevm.py", line 104, in __init__ mem = max(4, args.jobs) TypeError: '>' not supported between instances of 'NoneType' and 'int' Fix by always returning a -- not ideal but safe -- '1' value. Fixes: b09539444a ("tests/vm: allow us to take advantage of MTTCG") Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20221209164743.70836-1-phi...@linaro.org> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- tests/vm/basevm.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 2276364c42..23229e23d1 100644 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -569,8 +569,7 @@ def get_default_jobs(): # more cores. but only up to a reasonable limit. User # can always override these limits with --jobs. return min(multiprocessing.cpu_count() // 2, 8) -else: -return 1 +return 1 parser = argparse.ArgumentParser( formatter_class=argparse.ArgumentDefaultsHelpFormatter, -- 2.31.1
[PATCH v3 1/6] i386: Introduce FeatureWordInfo for AMX CPUID leaf 0x1D and 0x1E
CPUID leaf 0x1D and 0x1E enumerate tile and TMUL information for AMX. Introduce FeatureWord FEAT_1D_1_EAX, FEAT_1D_1_EBX, FEAT_1D_1_ECX and FEAT_1E_0_EBX. Thus these features of AMX can be expanded when "-cpu host/max" and can be configured in named CPU model. Signed-off-by: Lei Wang --- target/i386/cpu.c | 55 +++ target/i386/cpu.h | 12 +++ 2 files changed, 67 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 3410e5e470..b6d1247e5e 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1002,6 +1002,45 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, .tcg_features = ~0U, }, +[FEAT_1D_1_EAX] = { +.type = CPUID_FEATURE_WORD, +.cpuid = { +.eax = 0x1D, +.needs_ecx = true, .ecx = 1, +.reg = R_EAX, +}, +.migratable_flags = CPUID_AMX_PALETTE_1_TOTAL_TILE_BYTES_MASK | +CPUID_AMX_PALETTE_1_BYTES_PER_TILE_MASK, +}, +[FEAT_1D_1_EBX] = { +.type = CPUID_FEATURE_WORD, +.cpuid = { +.eax = 0x1D, +.needs_ecx = true, .ecx = 1, +.reg = R_EBX, +}, +.migratable_flags = CPUID_AMX_PALETTE_1_BYTES_PER_ROW_MASK | +CPUID_AMX_PALETTE_1_MAX_NAMES_MASK, +}, +[FEAT_1D_1_ECX] = { +.type = CPUID_FEATURE_WORD, +.cpuid = { +.eax = 0x1D, +.needs_ecx = true, .ecx = 1, +.reg = R_ECX, +}, +.migratable_flags = CPUID_AMX_PALETTE_1_MAX_ROWS_MASK, +}, +[FEAT_1E_0_EBX] = { +.type = CPUID_FEATURE_WORD, +.cpuid = { +.eax = 0x1E, +.needs_ecx = true, .ecx = 0, +.reg = R_EBX, +}, +.migratable_flags = CPUID_AMX_TMUL_MAX_K_MASK | +CPUID_AMX_TMUL_MAX_N_MASK, +}, /*Below are MSR exposed features*/ [FEAT_ARCH_CAPABILITIES] = { .type = MSR_FEATURE_WORD, @@ -1371,6 +1410,22 @@ static FeatureDep feature_dependencies[] = { .from = { FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT }, .to = { FEAT_14_0_ECX, ~0ull }, }, +{ +.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE }, +.to = { FEAT_1D_1_EAX, ~0ull }, +}, +{ +.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE }, +.to = { FEAT_1D_1_EBX, ~0ull }, +}, +{ +.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE }, +.to = { FEAT_1D_1_ECX, ~0ull }, +}, +{ +.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE }, +.to = { FEAT_1E_0_EBX, ~0ull }, +}, { .from = { FEAT_8000_0001_EDX, CPUID_EXT2_RDTSCP }, .to = { FEAT_VMX_SECONDARY_CTLS,VMX_SECONDARY_EXEC_RDTSCP }, diff --git a/target/i386/cpu.h b/target/i386/cpu.h index d4bc19577a..3e3e0cfe59 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -585,6 +585,14 @@ typedef enum X86Seg { XSTATE_Hi16_ZMM_MASK | XSTATE_PKRU_MASK | \ XSTATE_XTILE_CFG_MASK | XSTATE_XTILE_DATA_MASK) +#define CPUID_AMX_PALETTE_1_TOTAL_TILE_BYTES_MASK 0xU +#define CPUID_AMX_PALETTE_1_BYTES_PER_TILE_MASK (0xU << 16) +#define CPUID_AMX_PALETTE_1_BYTES_PER_ROW_MASK0xU +#define CPUID_AMX_PALETTE_1_MAX_NAMES_MASK(0xU << 16) +#define CPUID_AMX_PALETTE_1_MAX_ROWS_MASK 0xU +#define CPUID_AMX_TMUL_MAX_K_MASK 0xffU +#define CPUID_AMX_TMUL_MAX_N_MASK (0xU << 8) + /* CPUID feature words */ typedef enum FeatureWord { FEAT_1_EDX, /* CPUID[1].EDX */ @@ -605,6 +613,10 @@ typedef enum FeatureWord { FEAT_6_EAX, /* CPUID[6].EAX */ FEAT_XSAVE_XCR0_LO, /* CPUID[EAX=0xd,ECX=0].EAX */ FEAT_XSAVE_XCR0_HI, /* CPUID[EAX=0xd,ECX=0].EDX */ +FEAT_1D_1_EAX, /* CPUID[EAX=0x1d,ECX=1].EAX */ +FEAT_1D_1_EBX, /* CPUID[EAX=0x1d,ECX=1].EBX */ +FEAT_1D_1_ECX, /* CPUID[EAX=0x1d,ECX=1].ECX */ +FEAT_1E_0_EBX, /* CPUID[EAX=0x1e,ECX=0].EBX */ FEAT_ARCH_CAPABILITIES, FEAT_CORE_CAPABILITY, FEAT_PERF_CAPABILITIES, -- 2.34.1
[PULL 12/15] Update scripts/meson-buildoptions.sh
From: Alessandro Di Federico Note: `Makefile` relies on modification dates in the source tree to detect changes to `meson_options.txt`. However, git does not track those. Therefore, the following was necessary to regenerate `meson-buildoptions.sh`: touch meson_options.txt cd "$BUILD_DIR" make update-buildoptions Signed-off-by: Alessandro Di Federico Message-Id: <20230102104113.3438895-1-...@rev.ng> Reviewed-by: Thomas Huth Reviewed-by: Stefan Hajnoczi Signed-off-by: Thomas Huth --- scripts/meson-buildoptions.sh | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index aa6e30ea91..0f71e92dcb 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -10,6 +10,9 @@ meson_options_help() { printf "%s\n" ' affects only QEMU, not tools like qemu-img)' printf "%s\n" ' --datadir=VALUE Data file directory [share]' printf "%s\n" ' --disable-coroutine-pool coroutine freelist (better performance)' + printf "%s\n" ' --disable-hexagon-idef-parser' + printf "%s\n" ' use idef-parser to automatically generate TCG' + printf "%s\n" ' code for the Hexagon frontend' printf "%s\n" ' --disable-install-blobs install provided firmware blobs' printf "%s\n" ' --docdir=VALUE Base directory for documentation installation' printf "%s\n" ' (can be empty) [share/doc]' @@ -40,7 +43,8 @@ meson_options_help() { printf "%s\n" ' --enable-trace-backends=CHOICES' printf "%s\n" ' Set available tracing backends [log] (choices:' printf "%s\n" ' dtrace/ftrace/log/nop/simple/syslog/ust)' - printf "%s\n" ' --firmwarepath=VALUESsearch PATH for firmware files [share/qemu-firmware]' + printf "%s\n" ' --firmwarepath=VALUESsearch PATH for firmware files [share/qemu-' + printf "%s\n" ' firmware]' printf "%s\n" ' --iasl=VALUE Path to ACPI disassembler' printf "%s\n" ' --includedir=VALUE Header file directory [include]' printf "%s\n" ' --interp-prefix=VALUEwhere to find shared libraries etc., use %M for' @@ -93,7 +97,7 @@ meson_options_help() { printf "%s\n" ' glusterfs Glusterfs block device driver' printf "%s\n" ' gnutls GNUTLS cryptography support' printf "%s\n" ' gtk GTK+ user interface' - printf "%s\n" ' gtk-clipboard clipboard support for GTK (EXPERIMENTAL, MAY HANG)' + printf "%s\n" ' gtk-clipboard clipboard support for the gtk UI (EXPERIMENTAL, MAY HANG)' printf "%s\n" ' guest-agent Build QEMU Guest Agent' printf "%s\n" ' guest-agent-msi Build MSI package for the QEMU Guest Agent' printf "%s\n" ' hax HAX acceleration support' @@ -156,6 +160,8 @@ meson_options_help() { printf "%s\n" ' usb-redir libusbredir support' printf "%s\n" ' vde vde network backend support' printf "%s\n" ' vdi vdi image format support' + printf "%s\n" ' vduse-blk-export' + printf "%s\n" ' VDUSE block export support' printf "%s\n" ' vfio-user-server' printf "%s\n" ' vfio-user server support' printf "%s\n" ' vhost-cryptovhost-user crypto backend support' @@ -164,8 +170,6 @@ meson_options_help() { printf "%s\n" ' vhost-user vhost-user backend support' printf "%s\n" ' vhost-user-blk-server' printf "%s\n" ' build vhost-user-blk server' - printf "%s\n" ' vduse-blk-export' - printf "%s\n" ' VDUSE block export support' printf "%s\n" ' vhost-vdpa vhost-vdpa kernel backend support' printf "%s\n" ' virglrenderer virgl rendering support' printf "%s\n" ' virtfs virtio-9p support' @@ -283,6 +287,8 @@ _meson_option_parse() { --disable-guest-agent-msi) printf "%s" -Dguest_agent_msi=disabled ;; --enable-hax) printf "%s" -Dhax=enabled ;; --disable-hax) printf "%s" -Dhax=disabled ;; +--enable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=true ;; +--disable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=false ;; --enable-hvf) printf "%s" -Dhvf=enabled ;; --disable-hvf) printf "%s" -Dhvf=disabled ;; --iasl=*) quote_sh "-Diasl=$2" ;; @@ -429,6 +435,8 @@ _meson_option_parse() { --disable-vde) printf "%s" -Dvde=disabled ;; --enable-vdi) printf "%s" -Dvdi=enabled ;; --disable-vdi) printf "%s" -Dvdi=disabled ;; +--enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;; +--disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;; --enable-vfio-user-server) printf "%s" -Dvfio_user_server=enabled ;; --disable-vfio-user-server) printf "%s" -Dvfio_user_server=disabled ;; --enable-vhost-crypto) printf "%s" -Dvhost_crypto=e
Re: [PATCH v4] riscv: Allow user to set the satp mode
On Fri, Jan 06, 2023 at 08:56:33AM +0100, Alexandre Ghiti wrote: > On Fri, Dec 16, 2022 at 2:03 PM Alexandre Ghiti > wrote: ... > @Andrew: Please let me know when you have some cycles to review this, I'll try to get to this yet today. Thanks for the ping. drew
[PATCH v3 4/6] i386: Mask and report unavailable multi-bit feature values
Some feature words, e.g., feature words in AMX-related CPUID leaf 0x1D and 0x1E are not bit-wise but multiple bits represents one value. Handle this situation when the values specified are not the same as which are reported by KVM. The handling includes: - The responsibility of masking bits and giving warnings are delegated to the feature enabler. A framework is also provided to enable this. - To simplify the initialization, a default function is provided if the the function is not specified. The reason why delegating this responsibility rather than just marking them as zeros when they are not same is because different multi-bit features may have different logic, which is case by case, for example: 1. CPUID.0x14_0x1:EBX[15:0]. Even though it's multi-bits field, it's a bitmap and each bit represents a separate capability. 2. CPUID.0x14_0x1:EAX[2:0] represents the number of configurable Address Ranges. 3 bits as a whole to represent a integer value. It means the maximum capability of HW. If KVM reports M, then M to 0 is legal value to configure (because KVM can emulate each value correctly). 3. CPUID.0x1D_0x1:EAX[31:16] represents palette 1 bytes_per_tile. 16 bits as a whole represent an integer value. It's not like case 2 and SW needs to configure the same value as reported. Because it's not possible for SW to configure to a different value and KVM cannot emulate it. So marking them blindly as zeros is incorrect, and delegating this responsibility can let each multi-bit feature have its own way to mask bits. Signed-off-by: Lei Wang --- target/i386/cpu-internal.h | 2 ++ target/i386/cpu.c | 36 2 files changed, 38 insertions(+) diff --git a/target/i386/cpu-internal.h b/target/i386/cpu-internal.h index 66b3d66cb4..83c7b53926 100644 --- a/target/i386/cpu-internal.h +++ b/target/i386/cpu-internal.h @@ -30,6 +30,8 @@ typedef struct MultiBitFeatureInfo { uint64_t mask; unsigned high_bit_position; unsigned low_bit_position; +void (*mark_unavailable_multi_bit)(X86CPU *cpu, FeatureWord w, int index, + const char *verbose_prefix); } MultiBitFeatureInfo; typedef struct FeatureWordInfo { diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 88aa780566..e638a31d34 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4377,6 +4377,28 @@ static bool x86_cpu_have_filtered_features(X86CPU *cpu) return false; } +static void mark_unavailable_multi_bit_default(X86CPU *cpu, FeatureWord w, + int index, + const char *verbose_prefix) +{ +FeatureWordInfo *f = &feature_word_info[w]; +g_autofree char *feat_word_str = feature_word_description(f); +uint64_t host_feat = x86_cpu_get_supported_feature_word(w, false); +MultiBitFeatureInfo mf = f->multi_bit_features[index]; + +if ((cpu->env.features[w] & mf.mask) && +((cpu->env.features[w] ^ host_feat) & mf.mask)) { +if (!cpu->force_features) { +cpu->env.features[w] &= ~mf.mask; +} +cpu->filtered_features[w] |= mf.mask; +if (verbose_prefix) +warn_report("%s: %s.%s [%u:%u]", verbose_prefix, feat_word_str, +mf.feat_name, mf.high_bit_position, +mf.low_bit_position); +} +} + static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint64_t mask, const char *verbose_prefix) { @@ -6442,6 +6464,20 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) x86_cpu_get_supported_feature_word(w, false); uint64_t requested_features = env->features[w]; uint64_t unavailable_features = requested_features & ~host_feat; +FeatureWordInfo f = feature_word_info[w]; +int i; + +for (i = 0; i < f.num_multi_bit_features; i++) { +MultiBitFeatureInfo mf = f.multi_bit_features[i]; +if (mf.mark_unavailable_multi_bit) { +mf.mark_unavailable_multi_bit(cpu, w, i, prefix); +} else { +mark_unavailable_multi_bit_default(cpu, w, i, prefix); +} + +unavailable_features &= ~mf.mask; +} + mark_unavailable_features(cpu, w, unavailable_features, prefix); } -- 2.34.1
[PULL 11/15] docs/interop: Change the vnc-ledstate-Pseudo-encoding doc into .rst
The file seems to contain perfectly valid rst syntax already, so rename it to .rst and wire it up in the index. Message-Id: <20221213101806.46640-1-th...@redhat.com> Signed-off-by: Thomas Huth --- docs/interop/index.rst | 1 + ...tate-Pseudo-encoding.txt => vnc-ledstate-pseudo-encoding.rst} | 0 2 files changed, 1 insertion(+) rename docs/interop/{vnc-ledstate-Pseudo-encoding.txt => vnc-ledstate-pseudo-encoding.rst} (100%) diff --git a/docs/interop/index.rst b/docs/interop/index.rst index b7632acb7b..6351ff9ba6 100644 --- a/docs/interop/index.rst +++ b/docs/interop/index.rst @@ -23,3 +23,4 @@ are useful for making QEMU interoperate with other software. vhost-user-gpu vhost-vdpa virtio-balloon-stats + vnc-ledstate-pseudo-encoding diff --git a/docs/interop/vnc-ledstate-Pseudo-encoding.txt b/docs/interop/vnc-ledstate-pseudo-encoding.rst similarity index 100% rename from docs/interop/vnc-ledstate-Pseudo-encoding.txt rename to docs/interop/vnc-ledstate-pseudo-encoding.rst -- 2.31.1
[PATCH v3 5/6] i386: Initialize AMX CPUID leaves with corresponding env->features[] leaves
The AMX-related CPUID value, i.e., CPUID(0x1D,1):EAX, CPUID(0x1D,1):EBX, CPUID(0x1D,1):ECX and CPUID(0x1E,0):EBX are hard-coded to Sapphire Rapids without considering future platforms. Replace these hard-coded values with env->features[], so QEMU can pass the right value to KVM. Signed-off-by: Lei Wang --- target/i386/cpu.c | 27 +-- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e638a31d34..946df29a3d 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -576,16 +576,16 @@ static CPUCacheInfo legacy_l3_cache = { #define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */ /* CPUID Leaf 0x1D constants: */ -#define INTEL_AMX_TILE_MAX_SUBLEAF 0x1 -#define INTEL_AMX_TOTAL_TILE_BYTES 0x2000 -#define INTEL_AMX_BYTES_PER_TILE 0x400 -#define INTEL_AMX_BYTES_PER_ROW0x40 -#define INTEL_AMX_TILE_MAX_NAMES 0x8 -#define INTEL_AMX_TILE_MAX_ROWS0x10 +#define INTEL_SPR_AMX_TILE_MAX_SUBLEAF 0x1 +#define INTEL_SPR_AMX_TOTAL_TILE_BYTES 0x2000 +#define INTEL_SPR_AMX_BYTES_PER_TILE 0x400 +#define INTEL_SPR_AMX_BYTES_PER_ROW0x40 +#define INTEL_SPR_AMX_TILE_MAX_NAMES 0x8 +#define INTEL_SPR_AMX_TILE_MAX_ROWS0x10 /* CPUID Leaf 0x1E constants: */ -#define INTEL_AMX_TMUL_MAX_K 0x10 -#define INTEL_AMX_TMUL_MAX_N 0x40 +#define INTEL_SPR_AMX_TMUL_MAX_K 0x10 +#define INTEL_SPR_AMX_TMUL_MAX_N 0x40 void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, uint32_t vendor2, uint32_t vendor3) @@ -5764,12 +5764,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (count == 0) { /* Highest numbered palette subleaf */ -*eax = INTEL_AMX_TILE_MAX_SUBLEAF; +*eax = INTEL_SPR_AMX_TILE_MAX_SUBLEAF; } else if (count == 1) { -*eax = INTEL_AMX_TOTAL_TILE_BYTES | - (INTEL_AMX_BYTES_PER_TILE << 16); -*ebx = INTEL_AMX_BYTES_PER_ROW | (INTEL_AMX_TILE_MAX_NAMES << 16); -*ecx = INTEL_AMX_TILE_MAX_ROWS; +*eax = env->features[FEAT_1D_1_EAX]; +*ebx = env->features[FEAT_1D_1_EBX]; +*ecx = env->features[FEAT_1D_1_ECX]; } break; } @@ -5785,7 +5784,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (count == 0) { /* Highest numbered palette subleaf */ -*ebx = INTEL_AMX_TMUL_MAX_K | (INTEL_AMX_TMUL_MAX_N << 8); +*ebx = env->features[FEAT_1E_0_EBX]; } break; } -- 2.34.1
Re: [PATCH] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job
On 6/1/23 09:19, Thomas Huth wrote: On 06/01/2023 09.15, Stefan Weil wrote: Am 06.01.23 um 08:49 schrieb Thomas Huth: On 05/01/2023 22.42, Philippe Mathieu-Daudé wrote: > That said, maybe it is time to deprecate the 32-bit > hosts? Certainly fine for me, but that's up to the Windows folks to decide. Maybe you could just suggest a patch to start the discussion? Thomas Download numbers from yesterday for my latest Windows installers: qemu-w32-setup-20221230.exe - 243 qemu-w64-setup-20221230.exe - 6540 On Wednesday the ratio was 288 : 3516. As expected the 64-bit variant is used much more often, but it looks like there is still a certain desire for the 32-bit variant. OK, thanks. Could you maybe also check the browser types in the logs? ... I'm wondering whether a big part of those w32 downloads were just automatic web crawlers? Or people downloading both variants "just in case" but only install the 64-bit one ;)
Re: [PATCH] .gitlab-ci.d/windows: Work-around timeout and OpenGL problems of the MSYS2 jobs
On Fri, Jan 6, 2023 at 3:35 AM Thomas Huth wrote: > > On 05/01/2023 09.34, Thomas Huth wrote: > > On 04/01/2023 23.01, Peter Maydell wrote: > >> On Wed, 4 Jan 2023 at 12:36, Thomas Huth wrote: > >>> > >>> The windows jobs (especially the 32-bit job) recently started to > >>> hit the timeout limit. Bump it a little bit to ease the situation > >>> (80 minutes is quite long already - OTOH, these jobs do not have to > >>> wait for a job from the container stage to finish, so this should > >>> still be OK). > >>> > >>> Additionally, some update on the container side recently enabled > >>> OpenGL in these jobs - but the corresponding code fails to compile. > >>> Thus disable OpenGL here for the time being until someone figured > >>> out the proper fix in the shader code for this. > >>> > >>> Signed-off-by: Thomas Huth > >>> --- > >>> Now that the timeout and OpenGL problems are gone, the 64-bit is > >>> working fine for me again. However, I'm still seeing random issues > >>> with the 32-bit job ... not sure whether it's a problem on the > >>> QEMU side or whether the builders are currently instable, since > >>> the issues do not reproduce reliably... > >>> > >>> .gitlab-ci.d/windows.yml | 7 --- > >>> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> Thanks; applied to master on the assumption it will improve the > >> CI situation. I found that the msys2-32bit job still timed out > >> at 1h20, though: > >> > >> https://gitlab.com/qemu-project/qemu/-/jobs/3555245586 > > > > I just gave it a try again, too, and for me, it finished within 65 minutes: > > > > https://gitlab.com/thuth/qemu/-/jobs/3557600268 > > > > ... let's keep looking for a while, maybe it's ok in most cases now, but if > > not, we have to consider something else. > > Ok, so after I've been struggling with a failing msys2-32bit job for my new > upcoming pull request the last two days (I thought I had a bad patch in > there), where I had some problems with the test-hmp and qom-test qtests, > I've come up with a new theory after looking at this CI run from the > qemu-project staging branch and seeing that these tests are also failing > there: > > https://gitlab.com/qemu-project/qemu/-/jobs/3558798544 > https://gitlab.com/qemu-project/qemu/-/jobs/3560870904 > > That might also explain the timed-out job that you have seen earlier, Peter, > it was likely a hanging qom-test since that seems to be the first test to be > executed during the "make check" there. > > So the qtests for Windows are definitely not ready for the CI yet (after > we've enabled them just in December). I think it's best to disable them > there again completely until the issues are understood and fixed. > I cannot reproduce the test failures of both tests (test-hmp and qom-test) with w32 executables. Neither did the w64 executables. My testing repo is at commit d1852caab131ea898134fdcea8c14bc2ee75fbe9. Regards, Bin
Re: [PATCH v10 3/9] KVM: Extend the memslot to support fd-based private memory
On Thu, Jan 05, 2023 at 11:23:01AM +, Jarkko Sakkinen wrote: > On Fri, Dec 02, 2022 at 02:13:41PM +0800, Chao Peng wrote: > > In memory encryption usage, guest memory may be encrypted with special > > key and can be accessed only by the guest itself. We call such memory > > private memory. It's valueless and sometimes can cause problem to allow > > userspace to access guest private memory. This new KVM memslot extension > > allows guest private memory being provided through a restrictedmem > > backed file descriptor(fd) and userspace is restricted to access the > > bookmarked memory in the fd. > > > > This new extension, indicated by the new flag KVM_MEM_PRIVATE, adds two > > additional KVM memslot fields restricted_fd/restricted_offset to allow > > userspace to instruct KVM to provide guest memory through restricted_fd. > > 'guest_phys_addr' is mapped at the restricted_offset of restricted_fd > > and the size is 'memory_size'. > > > > The extended memslot can still have the userspace_addr(hva). When use, a > > single memslot can maintain both private memory through restricted_fd > > and shared memory through userspace_addr. Whether the private or shared > > part is visible to guest is maintained by other KVM code. > > > > A restrictedmem_notifier field is also added to the memslot structure to > > allow the restricted_fd's backing store to notify KVM the memory change, > > KVM then can invalidate its page table entries or handle memory errors. > > > > Together with the change, a new config HAVE_KVM_RESTRICTED_MEM is added > > and right now it is selected on X86_64 only. > > > > To make future maintenance easy, internally use a binary compatible > > alias struct kvm_user_mem_region to handle both the normal and the > > '_ext' variants. > > Feels bit hacky IMHO, and more like a completely new feature than > an extension. > > Why not just add a new ioctl? The commit message does not address > the most essential design here. Yes, people can always choose to add a new ioctl for this kind of change and the balance point here is we want to also avoid 'too many ioctls' if the functionalities are similar. The '_ext' variant reuses all the existing fields in the 'normal' variant and most importantly KVM internally can reuse most of the code. I certainly can add some words in the commit message to explain this design choice. Thanks, Chao > > BR, Jarkko
Re: [PATCH] semihosting: add O_BINARY flag in host_open for NT compatibility
On Fri, Jan 6, 2023 at 3:39 PM Philippe Mathieu-Daudé wrote: > > On 5/1/23 22:19, Evgeny Iakovlev wrote: > > Windows open(2) implementations opens files in text mode by default and > > needs a Windows-only O_BINARY flag to open files as binary. Qemu already > > s/Qemu/QEMU/ > > > knows about that flag in osdep.h, so we can just add it to the > > host_flags for better compatibility when running qemu on Windows. > > s/qemu/QEMU/ > > > Signed-off-by: Evgeny Iakovlev > > --- > > semihosting/syscalls.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c > > index 508a0ad88c..00f77507e5 100644 > > --- a/semihosting/syscalls.c > > +++ b/semihosting/syscalls.c > > @@ -278,6 +278,8 @@ static void host_open(CPUState *cs, > > gdb_syscall_complete_cb complete, > > host_flags |= O_EXCL; > > } > > > > +host_flags |= O_BINARY; > > + > > ret = open(p, host_flags, mode); > > if (ret < 0) { > > complete(cs, -1, errno); > > Alternatively with more churn: > > -- >8 -- > diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c > index 508a0ad88c..b621d78c2d 100644 > --- a/semihosting/syscalls.c > +++ b/semihosting/syscalls.c > @@ -253,7 +253,7 @@ static void host_open(CPUState *cs, > gdb_syscall_complete_cb complete, > { > CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; > char *p; > -int ret, host_flags; > +int ret, host_flags = O_BINARY; > > ret = validate_lock_user_string(&p, cs, fname, fname_len); > if (ret < 0) { > @@ -262,11 +262,11 @@ static void host_open(CPUState *cs, > gdb_syscall_complete_cb complete, > } > > if (gdb_flags & GDB_O_WRONLY) { > -host_flags = O_WRONLY; > +host_flags |= O_WRONLY; > } else if (gdb_flags & GDB_O_RDWR) { > -host_flags = O_RDWR; > +host_flags |= O_RDWR; > } else { > -host_flags = O_RDONLY; > +host_flags |= O_RDONLY; > } > if (gdb_flags & GDB_O_CREAT) { > host_flags |= O_CREAT; > --- > > Reviewed-by: Philippe Mathieu-Daudé > With Philippe's comments addressed, Reviewed-by: Bin Meng
Re: can we reduce the time taken for the msys2 jobs?
On Tue, Jan 3, 2023 at 6:40 PM Thomas Huth wrote: > > On 16/12/2022 17.23, Philippe Mathieu-Daudé wrote: > > On 16/12/22 15:55, Peter Maydell wrote: > >> The msys2-32bit job currently seems to run into the 70 minute CI timeout > >> quite frequently. This successful pass took 61 minutes: > >> https://gitlab.com/qemu-project/qemu/-/jobs/3479763757 > >> so I don't think it's a "time limit is fine but job has intermittent > >> hang" situation. > >> > >> msys2-64bit also is quite close to the timeout, eg this job > >> https://gitlab.com/qemu-project/qemu/-/jobs/3482192864 > >> took 61 minutes. > > > > The 64-bit variant is building WHPX. > > > >> Can we cut down or split up these CI jobs? > > The Windows jobs are terribly slow, yes, and we've discussed that a couple > of times before with no satisfying solution, e.g.: > > > https://lore.kernel.org/qemu-devel/e80726cc-633d-435c-7a2a-5cae43624...@redhat.com/ > > ... so we currently settled on not running the qtests in the 64-bit job > since that would take too long. > > I also don't have a real good idea how to improve the situation ... we could > switch to even simpler targets (e.g. s390x-softmmu should compile faster and > run less tests than ppc64-softmmu, I think), or would it be still OK to bump > the timeout to 80 minutes here? (90 minutes, like it has been discussed last > year is already very borderline, but I think 80 minutes might still be OK, > especially since those jobs don't wait for another job from the container > stage?) > > > We can add --disable-tools to the slower. > > For the 32-bit job, this would further decrease the test scope ... should be > OK as a last resort, I guess. > > > What I don't understand is why the docker image is rebuilt, it should > > be pulled from the registry. > > I think those Windows jobs are still quite a bit special ... e.g. until some > months ago, the "timeout" setting was also not working at all IIRC. > I am not sure if: 1. Create a Windows docker container build job to include all MSYS2 build prerequisites 2. Change MSYS2 job to depend on the Windows docker job, and remove all the build prerequisites preparation would save us some time? Regards, Bin
[PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
Windows open(2) implementation opens files in text mode by default and needs a Windows-only O_BINARY flag to open files as binary. QEMU already knows about that flag in osdep and it is defined to 0 on non-Windows, so we can just add it to the host_flags for better compatibility. Signed-off-by: Evgeny Iakovlev Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Bin Meng --- semihosting/syscalls.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c index 508a0ad88c..b621d78c2d 100644 --- a/semihosting/syscalls.c +++ b/semihosting/syscalls.c @@ -253,7 +253,7 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, { CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; char *p; -int ret, host_flags; +int ret, host_flags = O_BINARY; ret = validate_lock_user_string(&p, cs, fname, fname_len); if (ret < 0) { @@ -262,11 +262,11 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, } if (gdb_flags & GDB_O_WRONLY) { -host_flags = O_WRONLY; +host_flags |= O_WRONLY; } else if (gdb_flags & GDB_O_RDWR) { -host_flags = O_RDWR; +host_flags |= O_RDWR; } else { -host_flags = O_RDONLY; +host_flags |= O_RDONLY; } if (gdb_flags & GDB_O_CREAT) { host_flags |= O_CREAT; -- 2.34.1
Re: [PATCH] semihosting: add O_BINARY flag in host_open for NT compatibility
On 1/6/2023 10:48, Bin Meng wrote: On Fri, Jan 6, 2023 at 3:39 PM Philippe Mathieu-Daudé wrote: On 5/1/23 22:19, Evgeny Iakovlev wrote: Windows open(2) implementations opens files in text mode by default and needs a Windows-only O_BINARY flag to open files as binary. Qemu already s/Qemu/QEMU/ knows about that flag in osdep.h, so we can just add it to the host_flags for better compatibility when running qemu on Windows. s/qemu/QEMU/ Signed-off-by: Evgeny Iakovlev --- semihosting/syscalls.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c index 508a0ad88c..00f77507e5 100644 --- a/semihosting/syscalls.c +++ b/semihosting/syscalls.c @@ -278,6 +278,8 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, host_flags |= O_EXCL; } +host_flags |= O_BINARY; + ret = open(p, host_flags, mode); if (ret < 0) { complete(cs, -1, errno); Alternatively with more churn: -- >8 -- diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c index 508a0ad88c..b621d78c2d 100644 --- a/semihosting/syscalls.c +++ b/semihosting/syscalls.c @@ -253,7 +253,7 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, { CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; char *p; -int ret, host_flags; +int ret, host_flags = O_BINARY; ret = validate_lock_user_string(&p, cs, fname, fname_len); if (ret < 0) { @@ -262,11 +262,11 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, } if (gdb_flags & GDB_O_WRONLY) { -host_flags = O_WRONLY; +host_flags |= O_WRONLY; } else if (gdb_flags & GDB_O_RDWR) { -host_flags = O_RDWR; +host_flags |= O_RDWR; } else { -host_flags = O_RDONLY; +host_flags |= O_RDONLY; } if (gdb_flags & GDB_O_CREAT) { host_flags |= O_CREAT; --- Reviewed-by: Philippe Mathieu-Daudé With Philippe's comments addressed, Reviewed-by: Bin Meng Done, sent out a v2, thanks!
Re: [PATCH] .gitlab/issue_templates: Move suggestions into comments
John Snow writes: > On Mon, Dec 19, 2022 at 2:45 PM Alex Bennée wrote: >> >> >> Thomas Huth writes: >> >> > Many users forget to remove the suggestions from the bug template >> > when creating a new issue. So when searching for strings like "s390x" >> > or "Windows", you get a lot of unrelated issues in the results. >> > Thus let's move the suggestions into HTML comments - so they will >> > still show up in the markdown when editing the bug, while being >> > hidden/ignored in the final text or in the search queries. >> > >> > Signed-off-by: Thomas Huth >> >> Queued to testing/next, thanks. >> >> -- >> Alex Bennée >> Virtualisation Tech Lead @ Linaro >> > > Remember to update the default template in the gitlab dashboard. I > don't believe I have the permission, or I'd just take care of it > myself. Should be done now. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] semihosting: add O_BINARY flag in host_open for NT compatibility
On 1/6/2023 10:48, Bin Meng wrote: On Fri, Jan 6, 2023 at 3:39 PM Philippe Mathieu-Daudé wrote: On 5/1/23 22:19, Evgeny Iakovlev wrote: Windows open(2) implementations opens files in text mode by default and needs a Windows-only O_BINARY flag to open files as binary. Qemu already s/Qemu/QEMU/ knows about that flag in osdep.h, so we can just add it to the host_flags for better compatibility when running qemu on Windows. s/qemu/QEMU/ Signed-off-by: Evgeny Iakovlev --- semihosting/syscalls.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c index 508a0ad88c..00f77507e5 100644 --- a/semihosting/syscalls.c +++ b/semihosting/syscalls.c @@ -278,6 +278,8 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, host_flags |= O_EXCL; } +host_flags |= O_BINARY; + ret = open(p, host_flags, mode); if (ret < 0) { complete(cs, -1, errno); Alternatively with more churn: -- >8 -- diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c index 508a0ad88c..b621d78c2d 100644 --- a/semihosting/syscalls.c +++ b/semihosting/syscalls.c @@ -253,7 +253,7 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, { CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; char *p; -int ret, host_flags; +int ret, host_flags = O_BINARY; ret = validate_lock_user_string(&p, cs, fname, fname_len); if (ret < 0) { @@ -262,11 +262,11 @@ static void host_open(CPUState *cs, gdb_syscall_complete_cb complete, } if (gdb_flags & GDB_O_WRONLY) { -host_flags = O_WRONLY; +host_flags |= O_WRONLY; } else if (gdb_flags & GDB_O_RDWR) { -host_flags = O_RDWR; +host_flags |= O_RDWR; } else { -host_flags = O_RDONLY; +host_flags |= O_RDONLY; } if (gdb_flags & GDB_O_CREAT) { host_flags |= O_CREAT; --- Reviewed-by: Philippe Mathieu-Daudé With Philippe's comments addressed, Reviewed-by: Bin Meng Done, sent out a v2, thanks!
Re: [PATCH v4 0/4] hw/nvme: fix broken shadow doorbells on some platforms
On Dec 12 12:44, Klaus Jensen wrote: > From: Klaus Jensen > > Guenter reports that hw/nvme is broken on riscv64[1] and big endian > platforms[2]. > > This is a regression since 7.1, so this does not warrent an rc5 for 7.2. > I'm sure Guenter can carry this patch in his tree, and maybe we can get > this out in a stable release. > > On riscv, the issue is a missing cq eventidx update. I really wonder why > this issue only shows up on riscv64. We have not observed this on other > platforms (yet). > > Further, Guenter also reported problems on big-endian platforms. The > issue here is missing endian conversions which patch 3 addresses. This > also requires a fix for the Linux kernel that I am posting separately > (can't link to it, chicken and egg problem). > > [1]: > https://lore.kernel.org/qemu-devel/20221207174918.ga1151...@roeck-us.net/ > [2]: > https://lore.kernel.org/qemu-devel/20221209110022.ga3396...@roeck-us.net/ > > v4: > - screwed up the rebase (Philippe) > > v3: > - add patch to fix big-endian platforms > > v2: > - use QOM accessor (Philippe) > - added some cleanup patches in front > > Klaus Jensen (4): > hw/nvme: use QOM accessors > hw/nvme: rename shadow doorbell related trace events > hw/nvme: fix missing endian conversions for doorbell buffers > hw/nvme: fix missing cq eventidx update > > hw/nvme/ctrl.c | 121 ++- > hw/nvme/trace-events | 8 +-- > 2 files changed, 78 insertions(+), 51 deletions(-) > > -- > 2.38.1 > Applied to nvme-next. Thansk for the reviews! signature.asc Description: PGP signature
Re: Plugin Memory Callback Debugging
Aaron Lindsay writes: > Emilio, > > On Dec 18 00:24, Emilio Cota wrote: >> On Tue, Nov 29, 2022 at 15:37:51 -0500, Aaron Lindsay wrote: >> (snip) >> > > Does this hint that there are cases where reset cpu->plugin_mem_cbs to >> > > NULL is >> > > getting optimized away, but not the code to set it in the first place? >> > >> > Is there anyone who could help take a look at this from the code gen >> > perspective? >> >> Thanks for the report. Just adding assertions was enough to uncover >> several bugs. I did not reproduce the use-after-free, but by calling >> reset from a callback it's easy to see how it can occur. >> >> I have fixes in https://github.com/cota/qemu/tree/plugins >> >> Can you please give those a try? >> >> BTW I created an issue on gitlab to track this >> https://gitlab.com/qemu-project/qemu/-/issues/1381 > > Thanks so much for digging into this! > > I rebased your plugins branch on top of v7.2.0 and tested with several > scenarios which reliably triggered the bug for me. None of them > reproduced the original problem (or hit other bugs!) with your fixes. Emilio, Are you going to be able to post the patches soon? I'd like to get the fixes in as early in the cycle as possible. Thanks, -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On Tue, Jan 03, 2023 at 05:58:01PM -0500, Chuck Zmudzinski wrote: > Hello Anthony and Paul, Hi Chuck, > I am requesting your feedback to Alex Williamson's suggestion that this > problem with assigning the correct slot address to the igd on xen should > be fixed in libxl instead of in qemu. > > It seems to me that the xen folks and the kvm folks have two different > philosophies regarding how a tool stack should be designed. kvm/libvirt > provides much greater flexibility in configuring the guest which puts > the burden on the administrator to set all the options correctly for > a given feature set, while xen/xenlight does not provide so much > flexibility and tries to automatically configure the guest based on > a high-level feature option such as the igd-passthrough=on option that > is available for xen guests using qemu but not for kvm guests using > qemu. > > What do you think? Should libxl be patched instead of fixing the problem > with this patch to qemu, which is contrary to Alex's suggestion? I do think that libxl should be able to deal with having to put a graphic card on slot 2. QEMU already provides every API necessary for a toolstack to be able to start a Xen guest with all the PCI card in the right slot. But it would just be a bit more complicated to implement in libxl. At the moment, libxl makes use of the QEMU machine 'xenfv', libxl should instead start to use the 'pc' machine and add the "xen-platform" pci device. (libxl already uses 'pc' when the "xen-platform" pci card isn't needed.) Also probably add the other pci devices to specific slot to be able to add the passthrough graphic card at the right slot. Next is to deal with migration when using the 'pc' machine, as it's just an alias to a specific version of the machine. We need to use the same machine on the receiving end, that is start with e.g. "pc-i440fx-7.1" if 'pc' was an alias for it at guest creation. I wonder if we can already avoid to patch the 'xenfv' machine with some xl config: # avoid 'xenfv' machine and use 'pc' instead xen_platform_pci=0 # add xen-platform pci device back device_model_args_hvm = [ "-device", "xen-platform,addr=3", ] But there's probably another device which is going to be auto-assigned to slot 2. If you feel like dealing with the technical dept in libxl, that is to stop using 'xenfv' and use 'pc' instead, then go for it, I can help with that. Otherwise, if the patch to QEMU only changes the behavior of the 'xenfv' machine then I think I would be ok with it. I'll do a review of that QEMU patch in another email. Cheers, -- Anthony PERARD
Re: [PATCH] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job
Am 06.01.23 um 09:19 schrieb Thomas Huth: On 06/01/2023 09.15, Stefan Weil wrote: Download numbers from yesterday for my latest Windows installers: qemu-w32-setup-20221230.exe - 243 qemu-w64-setup-20221230.exe - 6540 On Wednesday the ratio was 288 : 3516. As expected the 64-bit variant is used much more often, but it looks like there is still a certain desire for the 32-bit variant. OK, thanks. Could you maybe also check the browser types in the logs? ... I'm wondering whether a big part of those w32 downloads were just automatic web crawlers? Thomas I now checked all downloads of the latests installers since 2022-12-30. qemu-w32-setup-20221230.exe – 509 different IP addresses qemu-w64-setup-20221230.exe - 5471 different IP addresses 339 unique IP addresses are common for 32- and 64-bit, either crawlers or people who simply got both variants. So there remain 170 IP addresses which only downloaded the 32-bit variant in the last week. I see 437 different strings for the browser type, but surprisingly none of them looks like a crawler. Stefan
[PATCH] hw/pci-host/mv64361: Reuse pci_swizzle_map_irq_fn
mv64361_pcihost_map_irq() is a reimplementation of pci_swizzle_map_irq_fn(). Resolve this redundancy. Signed-off-by: Bernhard Beschow --- Testing done: * `qemu-system-ppc -machine pegasos2 \ -rtc base=localtime \ -device ati-vga,guest_hwcursor=true,romfile="" \ -cdrom morphos-3.17.iso \ -kernel morphos-3.17/boot.img` --- hw/pci-host/mv64361.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c index cc9c4d6d3b..70db142ec3 100644 --- a/hw/pci-host/mv64361.c +++ b/hw/pci-host/mv64361.c @@ -72,11 +72,6 @@ struct MV64361PCIState { uint64_t remap[5]; }; -static int mv64361_pcihost_map_irq(PCIDevice *pci_dev, int n) -{ -return (n + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS; -} - static void mv64361_pcihost_set_irq(void *opaque, int n, int level) { MV64361PCIState *s = opaque; @@ -97,7 +92,7 @@ static void mv64361_pcihost_realize(DeviceState *dev, Error **errp) g_free(name); name = g_strdup_printf("pci.%d", s->index); h->bus = pci_register_root_bus(dev, name, mv64361_pcihost_set_irq, - mv64361_pcihost_map_irq, dev, + pci_swizzle_map_irq_fn, dev, &s->mem, &s->io, 0, 4, TYPE_PCI_BUS); g_free(name); pci_create_simple(h->bus, 0, TYPE_MV64361_PCI_BRIDGE); -- 2.39.0
Re: [PATCH] hw/pci-host/mv64361: Reuse pci_swizzle_map_irq_fn
On 6/1/23 12:39, Bernhard Beschow wrote: mv64361_pcihost_map_irq() is a reimplementation of pci_swizzle_map_irq_fn(). Resolve this redundancy. Signed-off-by: Bernhard Beschow --- Testing done: * `qemu-system-ppc -machine pegasos2 \ -rtc base=localtime \ -device ati-vga,guest_hwcursor=true,romfile="" \ -cdrom morphos-3.17.iso \ -kernel morphos-3.17/boot.img` --- hw/pci-host/mv64361.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
Am 4. Januar 2023 15:35:33 UTC schrieb "Philippe Mathieu-Daudé" : >+Markus/Thomas > >On 4/1/23 15:44, Bernhard Beschow wrote: >> During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of >> TYPE_PIIX3_DEVICE. Remove this redundancy. >> >> Signed-off-by: Bernhard Beschow >> --- >> hw/i386/pc_piix.c | 4 +--- >> hw/isa/piix.c | 20 >> include/hw/southbridge/piix.h | 1 - >> 3 files changed, 1 insertion(+), 24 deletions(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 5738d9cdca..6b8de3d59d 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -235,8 +235,6 @@ static void pc_init1(MachineState *machine, >> if (pcmc->pci_enabled) { >> DeviceState *dev; >> PCIDevice *pci_dev; >> -const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE >> - : TYPE_PIIX3_DEVICE; >> int i; >> pci_bus = i440fx_init(pci_type, >> @@ -250,7 +248,7 @@ static void pc_init1(MachineState *machine, >> : pci_slot_get_pirq); >> pcms->bus = pci_bus; >> -pci_dev = pci_new_multifunction(-1, true, type); >> +pci_dev = pci_new_multifunction(-1, true, TYPE_PIIX3_DEVICE); >> object_property_set_bool(OBJECT(pci_dev), "has-usb", >>machine_usb(machine), &error_abort); >> object_property_set_bool(OBJECT(pci_dev), "has-acpi", >> diff --git a/hw/isa/piix.c b/hw/isa/piix.c >> index 98e9b12661..e4587352c9 100644 >> --- a/hw/isa/piix.c >> +++ b/hw/isa/piix.c >> @@ -33,7 +33,6 @@ >> #include "hw/qdev-properties.h" >> #include "hw/ide/piix.h" >> #include "hw/isa/isa.h" >> -#include "hw/xen/xen.h" >> #include "sysemu/runstate.h" >> #include "migration/vmstate.h" >> #include "hw/acpi/acpi_aml_interface.h" >> @@ -465,24 +464,6 @@ static const TypeInfo piix3_info = { >> .class_init= piix3_class_init, >> }; >> -static void piix3_xen_class_init(ObjectClass *klass, void *data) >> -{ >> -DeviceClass *dc = DEVICE_CLASS(klass); >> -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> - >> -k->realize = piix3_realize; >> -/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ >> -k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0; >> -dc->vmsd = &vmstate_piix3; > >IIUC, since this device is user-creatable, we can't simply remove it >without going thru the deprecation process. AFAICS this device is actually not user-creatable since dc->user_creatable is set to false once in the base class. I think it is safe to remove the Xen class unless there are ABI issues. Best regards, Bernhard >Alternatively we could >add a type alias: > >-- >8 -- >diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c >index 4b0ef65780..d94f7ea369 100644 >--- a/softmmu/qdev-monitor.c >+++ b/softmmu/qdev-monitor.c >@@ -64,6 +64,7 @@ typedef struct QDevAlias > QEMU_ARCH_LOONGARCH) > #define QEMU_ARCH_VIRTIO_CCW (QEMU_ARCH_S390X) > #define QEMU_ARCH_VIRTIO_MMIO (QEMU_ARCH_M68K) >+#define QEMU_ARCH_XEN (QEMU_ARCH_ARM | QEMU_ARCH_I386) > > /* Please keep this table sorted by typename. */ > static const QDevAlias qdev_alias_table[] = { >@@ -111,6 +112,7 @@ static const QDevAlias qdev_alias_table[] = { > { "virtio-tablet-device", "virtio-tablet", QEMU_ARCH_VIRTIO_MMIO }, > { "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW }, > { "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI }, >+{ "PIIX3", "PIIX3-xen", QEMU_ARCH_XEN }, > { } > }; >--- > >But I'm not sure due to this comment from commit ee46d8a503 >(2011-12-22 15:24:20 -0600): > >47) /* >48) * Aliases were a bad idea from the start. Let's keep them >49) * from spreading further. >50) */ > >Maybe using qdev_alias_table[] during device deprecation is >acceptable? > >> -} >> - >> -static const TypeInfo piix3_xen_info = { >> -.name = TYPE_PIIX3_XEN_DEVICE, >> -.parent= TYPE_PIIX_PCI_DEVICE, >> -.instance_init = piix3_init, >> -.class_init= piix3_xen_class_init, >> -}; >> - >> static void piix4_realize(PCIDevice *dev, Error **errp) >> { >> ERRP_GUARD(); >> @@ -534,7 +515,6 @@ static void piix3_register_types(void) >> { >> type_register_static(&piix_pci_type_info); >> type_register_static(&piix3_info); >> -type_register_static(&piix3_xen_info); >> type_register_static(&piix4_info); >> } >> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h >> index 65ad8569da..b1fc94a742 100644 >> --- a/include/hw/southbridge/piix.h >> +++ b/include/hw/southbridge/piix.h >> @@ -77,7 +77,6 @@ struct PIIXState { >> OBJECT_DECLARE_SIMPLE_TYPE(PIIXState, PIIX_PCI_DEVICE) >> #define TYPE_PIIX3_DEVICE "PIIX3" >> -#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen" >> #define TYPE_PIIX4_PCI_DEVICE "piix4-isa" >> #endif >
intermittent hang, s390x host, bios-tables-test test, TPM
I'm seeing an intermittent hang on the s390 CI runner in the bios-tables-test test. It looks like we've deadlocked because: * the TPM device is waiting for data on its socket that never arrives, and it's holding the iothread lock * QEMU is therefore not making forward progress; in particular it is unable to handle qtest queries/responses * the test binary thread 1 is waiting to get a response to its qtest command, which is not going to arrive * test binary thread 3 (tpm_emu_ctrl_thread) is has hit an assertion and is trying to kill QEMU via qtest_kill_qemu() * qtest_kill_qemu() is only a "SIGTERM and wait", so will wait forever, because QEMU won't respond to the SIGTERM while it's blocked waiting for the TPM device to release the iothread lock * because the ctrl-thread is waiting for QEMU to exit, it's never going to send the data that would unblock the TPM device emulation A secondary problem revealed by this is that gitlab CI runner jobs can be cancelled or time out in the gitlab CI, whilst leaving processes lying around (and consuming CPU in the case of livelocks) on the CI host. This means that future CI jobs then time out because the host's CPU is all being wasted by these stuck old jobs. Why does this happen and how can we configure the CI runner so timed out jobs reliably clean themselves up ? Process tree: gitlab-+ 2659701 0.0 0.0 233272 5952 ?Ssl Jan05 0:02 /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/tests/qtest/bios-tables-test --tap -k gitlab-+ 2661317 0.0 0.3 1783272 61500 ? Sl Jan05 0:00 \_ ./qemu-system-x86_64 -qtest unix:/tmp/qtest-2659701.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-2659701.qmp,id=char0 -mon chardev=char0,mode=control -display none -machine q35 -accel kvm -accel tcg -net none -chardev socket,id=chr,path=/tmp/qemu-test_acpi_q35_tcg_tis.Y88AY1/sock -tpmdev emulator,id=dev,chardev=chr -device tpm-tis,tpmdev=dev -drive id=hd0,if=none,file=tests/acpi-test-disk-bjuUil,format=raw -device ide-hd,drive=hd0 -accel qtest Here's the backtrace of the qemu-system-x86_64 process: Thread 4 (Thread 0x3ff2bfff900 (LWP 2661322)): #0 __libc_recvmsg (fd=, msg=msg@entry=0x3ff2bff9e48, flags=flags@entry=1073741824) at ../sysdeps/unix/sysv/linux/recvmsg.c:30 #1 0x02aa124aab4e in qio_channel_socket_readv (ioc=, iov=, niov=, fds=0x3ff2bffa030, nfds=0x3ff2bffa038, errp=0x0) at ../io/channel-socket.c:521 #2 0x02aa124afca4 in qio_channel_readv_full (ioc=0x2aa142f5280, iov=iov@entry=0x3ff2bffa040, niov=niov@entry=1, fds=fds@entry=0x3ff2bffa030, nfds=nfds@entry=0x3ff2bffa038, errp=0x0) at ../io/channel.c:66 #3 0x02aa12579098 in tcp_chr_recv (chr=chr@entry=0x2aa141aaa60, buf=buf@entry=0x3ff2bffa308 "", len=len@entry=4) at ../chardev/char-socket.c:284 #4 0x02aa1257b75a in tcp_chr_sync_read (chr=0x2aa141aaa60, buf=0x3ff2bffa308 "", len=) at ../chardev/char-socket.c:535 #5 0x02aa12575aa0 in qemu_chr_fe_read_all (be=be@entry=0x2aa1423df00, buf=buf@entry=0x3ff2bffa308 "", len=len@entry=4) at /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/include/chardev/char.h:231 #6 0x02aa12249872 in tpm_emulator_ctrlcmd (tpm=0x2aa1423dea0, cmd=cmd@entry=9, msg=msg@entry=0x3ff2bffa308, msg_len_in=msg_len_in@entry=0, msg_len_out=msg_len_out@entry=4) at ../backends/tpm/tpm_emulator.c:145 #7 0x02aa1224a2ca in tpm_emulator_cancel_cmd (tb=) at ../backends/tpm/tpm_emulator.c:500 #8 0x02aa121e68c4 in tpm_tis_mmio_write (opaque=0x2aa1529ec20, addr=24, val=64, size=) at ../hw/tpm/tpm_tis_common.c:663 #9 0x02aa12402348 in memory_region_write_accessor (mr=0x2aa1529ec20, addr=24, value=, size=, shift=, mask=255, attrs=...) at ../softmmu/memory.c:493 #10 0x02aa123fde26 in access_with_adjusted_size (addr=addr@entry=24, value=value@entry=0x3ff2bffa678, size=size@entry=1, access_size_min=, access_size_max=, access_size_max@entry=0, access_fn=0x2aa12402298 , mr=, attrs=...) at ../softmmu/memory.c:555 Python Exception PC not saved: Thread 3 (Thread 0x3ff70aa0900 (LWP 2661321)): #0 0x03ff801f1b32 in __GI___poll (fds=0x3ff24003280, nfds=3, timeout=) at ../sysdeps/unix/sysv/linux/poll.c:29 #1 0x03ff829d4386 in () at /lib/s390x-linux-gnu/libglib-2.0.so.0 #2 0x03ff829d4790 in g_main_loop_run () at /lib/s390x-linux-gnu/libglib-2.0.so.0 #3 0x02aa124d29d6 in iothread_run (opaque=opaque@entry=0x2aa1434d300) at ../iothread.c:74 #4 0x02aa1261c36a in qemu_thread_start (args=) at ../util/qemu-thread-posix.c:505 #5 0x03ff80307e66 in start_thread (arg=0x3ff70aa0900) at pthread_create.c:477 #6 0x03ff801fcbe6 in thread_start () at ../sysdeps/unix/sysv/linux/s390/s390-64/clone.S:65 Thread 2 (Thread 0x3ff71423900 (LWP 2661319)): #0 syscall () at ../sysdeps/unix/sysv/linux/s390/s390-64/syscall.S:37 #1 0x02aa1261d30c in qemu_futex_wait (val=, f=) at /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/include/qemu/futex.h:29 #2 qemu_event_wait (ev=ev@entry=0x2aa12e872
Re: [PULL 00/34] target-arm queue
On Thu, 5 Jan 2023 at 16:44, Peter Maydell wrote: > > Some arm patches; my to-review queue is by no means empty, but > this is a big enough set of patches to be getting on with... > > -- PMM > > The following changes since commit cb9c6a8e5ad6a1f0ce164d352e3102df46986e22: > > .gitlab-ci.d/windows: Work-around timeout and OpenGL problems of the MSYS2 > jobs (2023-01-04 18:58:33 +) > > are available in the Git repository at: > > https://git.linaro.org/people/pmaydell/qemu-arm.git > tags/pull-target-arm-20230105 > > for you to fetch changes up to 93c9678de9dc7d2e68f9e8477da072bac30ef132: > > hw/net: Fix read of uninitialized memory in imx_fec. (2023-01-05 15:33:00 > +) > > > target-arm queue: > * Implement AArch32 ARMv8-R support > * Add Cortex-R52 CPU > * fix handling of HLT semihosting in system mode > * hw/timer/ixm_epit: cleanup and fix bug in compare handling > * target/arm: Coding style fixes > * target/arm: Clean up includes > * nseries: minor code cleanups > * target/arm: align exposed ID registers with Linux > * hw/arm/smmu-common: remove unnecessary inlines > * i.MX7D: Handle GPT timers > * i.MX7D: Connect IRQs to GPIO devices > * i.MX6UL: Add a specific GPT timer instance > * hw/net: Fix read of uninitialized memory in imx_fec Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0 for any user-visible changes. -- PMM
Re: [PATCH v2 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
On 6/1/23 12:57, Bernhard Beschow wrote: Am 4. Januar 2023 15:35:33 UTC schrieb "Philippe Mathieu-Daudé" : +Markus/Thomas On 4/1/23 15:44, Bernhard Beschow wrote: During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of TYPE_PIIX3_DEVICE. Remove this redundancy. Signed-off-by: Bernhard Beschow --- hw/i386/pc_piix.c | 4 +--- hw/isa/piix.c | 20 include/hw/southbridge/piix.h | 1 - 3 files changed, 1 insertion(+), 24 deletions(-) -static void piix3_xen_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - -k->realize = piix3_realize; -/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ -k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0; -dc->vmsd = &vmstate_piix3; IIUC, since this device is user-creatable, we can't simply remove it without going thru the deprecation process. AFAICS this device is actually not user-creatable since dc->user_creatable is set to false once in the base class. I think it is safe to remove the Xen class unless there are ABI issues. Great news!
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote: > Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus, > as noted in docs/igd-assign.txt in the Qemu source code. > > Currently, when the xl toolstack is used to configure a Xen HVM guest with > Intel IGD passthrough to the guest with the Qemu upstream device model, > a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy > a different slot. This problem often prevents the guest from booting. > > The only available workaround is not good: Configure Xen HVM guests to use > the old and no longer maintained Qemu traditional device model available > from xenbits.xen.org which does reserve slot 2 for the Intel IGD. > > To implement this feature in the Qemu upstream device model for Xen HVM > guests, introduce the following new functions, types, and macros: > > * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE > * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS > * typedef XenPTQdevRealize function pointer > * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2 > * xen_igd_reserve_slot and xen_igd_clear_slot functions > > The new xen_igd_reserve_slot function uses the existing slot_reserved_mask > member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using > the xl toolstack with the gfx_passthru option enabled, which sets the > igd-passthru=on option to Qemu for the Xen HVM machine type. > > The new xen_igd_reserve_slot function also needs to be implemented in > hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case > when Qemu is configured with --enable-xen and --disable-xen-pci-passthrough, > in which case it does nothing. > > The new xen_igd_clear_slot function overrides qdev->realize of the parent > PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus > since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was > created in hw/i386/pc_piix.c for the case when igd-passthru=on. > > Move the call to xen_host_pci_device_get, and the associated error > handling, from xen_pt_realize to the new xen_igd_clear_slot function to > initialize the device class and vendor values which enables the checks for > the Intel IGD to succeed. The verification that the host device is an > Intel IGD to be passed through is done by checking the domain, bus, slot, > and function values as well as by checking that gfx_passthru is enabled, > the device class is VGA, and the device vendor in Intel. > > Signed-off-by: Chuck Zmudzinski If we are to make changes, something that might be generally useful is a new mask along the lines of slot_reserved_mask, however - only affecting auto-allocated addresses - controllable from a command line property this way one could say "don't allocate any devices to slot 2" and later "put igd device in slot 2". And, xenpv machine could set defaults for these using the compat machinery. > --- > Notes that might be helpful to reviewers of patched code in hw/xen: > > The new functions and types are based on recommendations from Qemu docs: > https://qemu.readthedocs.io/en/latest/devel/qom.html > > Notes that might be helpful to reviewers of patched code in hw/i386: > > The small patch to hw/i386/pc_piix.c is protected by CONFIG_XEN so it does > not affect builds that do not have CONFIG_XEN defined. > > xen_igd_gfx_pt_enabled() in the patched hw/i386/pc_piix.c file is an > existing function that is only true when Qemu is built with > xen-pci-passthrough enabled and the administrator has configured the Xen > HVM guest with Qemu's igd-passthru=on option. > > v2: Remove From: tag at top of commit message > > v3: Changed the test for the Intel IGD in xen_igd_clear_slot: > > if (is_igd_vga_passthrough(&s->real_device) && > (s->real_device.vendor_id == PCI_VENDOR_ID_INTEL)) { > > is changed to > > if (xen_igd_gfx_pt_enabled() && (s->hostaddr.slot == 2) > && (s->hostaddr.function == 0)) { > > I hoped that I could use the test in v2, since it matches the > other tests for the Intel IGD in Qemu and Xen, but those tests > do not work because the necessary data structures are not set with > their values yet. So instead use the test that the administrator > has enabled gfx_passthru and the device address on the host is > 02.0. This test does detect the Intel IGD correctly. > > v4: Use brchu...@aol.com instead of brchu...@netscape.net for the author's > email address to match the address used by the same author in commits > be9c61da and c0e86b76 > > Change variable for XEN_PT_DEVICE_CLASS: xptc changed to xpdc > > v5: The patch of xen_pt.c was re-worked to allow a more consistent test > for the Intel IGD that uses the same criteria as in other places. > This involved moving the call to xen_host_pci_device_get from > xen_pt_realize to xen_igd_clear_slot and updating
Re: [PATCH] hw/arm/smmuv3: Add GBPA register
Hi Mostafan jean, On 1/4/23 13:29, Jean-Philippe Brucker wrote: > Hi Mostafa, > > On Mon, Dec 19, 2022 at 12:57:20PM +, Mostafa Saleh wrote: >> GBPA register can be used to globally abort all >> transactions. >> >> Only UPDATE and ABORT bits are considered in this patch. > That's fair, although it effectively implements all bits since > smmuv3_translate() ignores memory attributes anyway I see SHCFG 0b00 value means non shareable whereas other reset values correspond to "Use Incoming". Isn't it a bit weird? Is it better to have "non shareable" or "Use Incoming" as de fault value (which is IMPLEMENTATION DEFINED) > >> It is described in the SMMU manual in "6.3.14 SMMU_GBPA". >> ABORT reset value is IMPLEMENTATION DEFINED, it is chosen to >> be zero(Do not abort incoming transactions). >> >> Signed-off-by: Mostafa Saleh >> --- >> hw/arm/smmuv3-internal.h | 4 >> hw/arm/smmuv3.c | 14 ++ >> include/hw/arm/smmuv3.h | 1 + >> 3 files changed, 19 insertions(+) >> >> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h >> index bce161870f..71f70141e8 100644 >> --- a/hw/arm/smmuv3-internal.h >> +++ b/hw/arm/smmuv3-internal.h >> @@ -79,6 +79,10 @@ REG32(CR0ACK, 0x24) >> REG32(CR1, 0x28) >> REG32(CR2, 0x2c) >> REG32(STATUSR, 0x40) >> +REG32(GBPA,0x44) >> +FIELD(GBPA, ABORT,20, 1) >> +FIELD(GBPA, UPDATE, 31, 1) >> + >> REG32(IRQ_CTRL,0x50) >> FIELD(IRQ_CTRL, GERROR_IRQEN,0, 1) >> FIELD(IRQ_CTRL, PRI_IRQEN, 1, 1) >> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c >> index 955b89c8d5..2843bc3da9 100644 >> --- a/hw/arm/smmuv3.c >> +++ b/hw/arm/smmuv3.c >> @@ -285,6 +285,7 @@ static void smmuv3_init_regs(SMMUv3State *s) >> s->gerror = 0; >> s->gerrorn = 0; >> s->statusr = 0; >> +s->gbpa = 0; >> } >> >> static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf, >> @@ -663,6 +664,11 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion >> *mr, hwaddr addr, >> goto epilogue; >> } >> >> +if (FIELD_EX32(s->gbpa, GBPA, ABORT)) { >> +status = SMMU_TRANS_ABORT; >> +goto epilogue; >> +} >> + > GBPA is only taken into account when SMMU_CR0.SMMUEN is 0 (6.3.9.6 SMMUEN) indeed "This register controls the global bypass attributes used for transactions from Non-secure StreamIDs (as determined by SSD, if supported) when SMMU_CR0.SMMUEN == 0" > >> cfg = smmuv3_get_config(sdev, &event); >> if (!cfg) { >> status = SMMU_TRANS_ERROR; >> @@ -1170,6 +1176,10 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr >> offset, >> case A_GERROR_IRQ_CFG2: >> s->gerror_irq_cfg2 = data; >> return MEMTX_OK; >> +case A_GBPA: >> +/* Ignore update bit as write is synchronous. */ actually you immediataly reset the update bit and not really "ignore" it. > We could also ignore a write that has Update=0, since that's required for > SMMUv3.2+ implementations (6.3.14.1 Update procedure) agreed: "If this register is written without simultaneously setting Update to 1, the effect is CONSTRAINED UNPREDICTABLE and has one of the following behaviors: • The write is IGNORED. This is the only permitted behavior in SMMUv3.2 and later." > >> +s->gbpa = data & ~R_GBPA_UPDATE_MASK; > Do we need to synchronize with concurrent transactions here? > I couldn't find if QEMU already serializes MMIO writes and IOMMU > translation. my understanding is qemu_global_mutex also is enough. BQL usage was discussed in https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg02403.html > > "Transactions arriving at the SMMU after completion of a GPBA update are > guaranteed to take the new attributes written." The guest tests completion > by reading the Update bit: > > vCPU (host CPU 0) Device thread (host CPU 1) > > (a) read GBPA.abort = 1 > (b) write GBPA.{update,abort} = {1,0} > (c) read GBPA.update = 0 > (d) launch DMA (e) execute DMA > (f) translation must read GBPA.abort = 0 > > I guess memory barriers after (b) and before (f) would ensure that. But I > wonder if SMMUEN also needs additional synchronization, and in that case a > rwlock would probably be simpler. > > Thanks, > Jean > >> +return MEMTX_OK; >> case A_STRTAB_BASE: /* 64b */ >> s->strtab_base = deposit64(s->strtab_base, 0, 32, data); >> return MEMTX_OK; >> @@ -1318,6 +1328,9 @@ static MemTxResult smmu_readl(SMMUv3State *s, hwaddr >> offset, >> case A_STATUSR: >> *data = s->statusr; >> return MEMTX_OK; >> +case A_GBPA: >> +*data = s->gbpa; >> +return MEMTX_OK; >> case A_IRQ_CTRL: >> case A_IRQ_CTRL_ACK: >> *data = s->irq_ctrl; >> @@ -1495,6 +1508,7 @@ static const VMStateDescription vmstate_smmuv3 = { >>
[RFC PATCH] scripts/ci: update gitlab-runner playbook to use latest runner
We were using quite and old runner on our machines and running into issues with stalling jobs. Gitlab in the meantime now reliably provide the latest packaged versions of the runner under a stable URL. This update: - creates a per-arch subdir for builds - switches from binary tarballs to deb packages - re-uses the same binary for the secondary runner - updates distro check for second to 22.04 So far I've tested on aarch64.ci.qemu.org but I shall do s390x next as its having issues with stale runners as well. Signed-off-by: Alex Bennée --- scripts/ci/setup/gitlab-runner.yml | 42 -- scripts/ci/setup/vars.yml.template | 2 -- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/scripts/ci/setup/gitlab-runner.yml b/scripts/ci/setup/gitlab-runner.yml index 33128be85d..05cdb2ae4b 100644 --- a/scripts/ci/setup/gitlab-runner.yml +++ b/scripts/ci/setup/gitlab-runner.yml @@ -30,6 +30,11 @@ home: /home/gitlab-runner shell: /bin/bash +- name: Create working directories for build + file: +path: "/home/gitlab-runner/{{ ansible_facts[\"architecture\"] }}" +state: directory + - name: Remove the .bash_logout file when on Ubuntu systems file: path: /home/gitlab-runner/.bash_logout @@ -50,17 +55,17 @@ - name: Download the matching gitlab-runner get_url: -dest: /usr/local/bin/gitlab-runner -url: "https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version }}/binaries/gitlab-runner-{{ gitlab_runner_os }}-{{ gitlab_runner_arch }}" -owner: gitlab-runner -group: gitlab-runner -mode: u=rwx,g=rwx,o=rx +dest: "/root/" +url: "https://gitlab-runner-downloads.s3.amazonaws.com/latest/deb/gitlab-runner_{{ gitlab_runner_arch }}.deb" + +- name: Install gitlab-runner via package manager + apt: deb="/root/gitlab-runner_{{ gitlab_runner_arch }}.deb" - name: Register the gitlab-runner - command: "/usr/local/bin/gitlab-runner register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell --tag-list {{ ansible_facts[\"architecture\"] }},{{ ansible_facts[\"distribution\"]|lower }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] }})'" + command: "/usr/bin/gitlab-runner register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell --tag-list {{ ansible_facts[\"architecture\"] }},{{ ansible_facts[\"distribution\"]|lower }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] }})'" - name: Install the gitlab-runner service using its own functionality - command: /usr/local/bin/gitlab-runner install --user gitlab-runner --working-directory /home/gitlab-runner + command: "/usr/bin/gitlab-runner install --user gitlab-runner --working-directory /home/gitlab-runner/{{ ansible_facts[\"architecture\"] }}" register: gitlab_runner_install_service_result failed_when: "gitlab_runner_install_service_result.rc != 0 and \"already exists\" not in gitlab_runner_install_service_result.stderr" @@ -70,33 +75,30 @@ state: started enabled: yes -- name: Download secondary gitlab-runner - get_url: -dest: /usr/local/bin/gitlab-runner-arm -url: "https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version }}/binaries/gitlab-runner-{{ gitlab_runner_os }}-arm" -owner: gitlab-runner -group: gitlab-runner -mode: u=rwx,g=rwx,o=rx +- name: Create working directories secondary runner + file: +path: "/home/gitlab-runner/arm" +state: directory when: - ansible_facts['distribution'] == 'Ubuntu' - ansible_facts['architecture'] == 'aarch64' -- ansible_facts['distribution_version'] == '20.04' +- ansible_facts['distribution_version'] == '22.04' - name: Register secondary gitlab-runner - command: "/usr/local/bin/gitlab-runner-arm register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell --tag-list aarch32,{{ ansible_facts[\"distribution\"]|lower }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] }})'" + command: "/usr/bin/gitlab-runner register --non-interactive --url {{ gitlab_runner_server_url }} --registration-to
Re: [PULL 0/7] Hexagon update
On Thu, 5 Jan 2023 at 17:38, Taylor Simpson wrote: > > The following changes since commit cb9c6a8e5ad6a1f0ce164d352e3102df46986e22: > > .gitlab-ci.d/windows: Work-around timeout and OpenGL problems of the MSYS2 > jobs (2023-01-04 18:58:33 +) > > are available in the Git repository at: > > https://github.com/quic/qemu tags/pull-hex-20230105 > > for you to fetch changes up to dc63b1492c2d8140d3b47093700bb9bb52c0d97b: > > Update scripts/meson-buildoptions.sh (2023-01-05 09:19:02 -0800) > > > Hexagon update: patches from several folks > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0 for any user-visible changes. -- PMM
Re: [PATCH v2 0/2] hw/arm: Add support for STM32 H405 and fix STM32F405 memory layout
On Fri, 30 Dec 2022 at 14:57, Felipe Balbi wrote: > > Hi, > > The following patches pass checkpatch.pl and have been tested against > 55745005e90a. > > Felipe Balbi (2): > hw/arm/stm32f405: correctly describe the memory layout > hw/arm: Add Olimex H405 Applied to target-arm.next, thanks. -- PMM
Re: [RFC PATCH] scripts/ci: update gitlab-runner playbook to use latest runner
Alex Bennée writes: > We were using quite and old runner on our machines and running into > issues with stalling jobs. Gitlab in the meantime now reliably provide > the latest packaged versions of the runner under a stable URL. This > update: > > - creates a per-arch subdir for builds > - switches from binary tarballs to deb packages > - re-uses the same binary for the secondary runner > - updates distro check for second to 22.04 > > So far I've tested on aarch64.ci.qemu.org but I shall do s390x next as > its having issues with stale runners as well. > > Signed-off-by: Alex Bennée > --- > scripts/ci/setup/gitlab-runner.yml | 42 -- > scripts/ci/setup/vars.yml.template | 2 -- > 2 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/scripts/ci/setup/gitlab-runner.yml > b/scripts/ci/setup/gitlab-runner.yml > index 33128be85d..05cdb2ae4b 100644 > --- a/scripts/ci/setup/gitlab-runner.yml > +++ b/scripts/ci/setup/gitlab-runner.yml > @@ -30,6 +30,11 @@ > home: /home/gitlab-runner > shell: /bin/bash > > +- name: Create working directories for build > + file: > +path: "/home/gitlab-runner/{{ ansible_facts[\"architecture\"] }}" > +state: directory > + > - name: Remove the .bash_logout file when on Ubuntu systems >file: > path: /home/gitlab-runner/.bash_logout > @@ -50,17 +55,17 @@ > > - name: Download the matching gitlab-runner >get_url: > -dest: /usr/local/bin/gitlab-runner > -url: "https://s3.amazonaws.com/gitlab-runner-downloads/v{{ > gitlab_runner_version }}/binaries/gitlab-runner-{{ gitlab_runner_os }}-{{ > gitlab_runner_arch }}" > -owner: gitlab-runner > -group: gitlab-runner > -mode: u=rwx,g=rwx,o=rx > +dest: "/root/" > +url: > "https://gitlab-runner-downloads.s3.amazonaws.com/latest/deb/gitlab-runner_{{ > gitlab_runner_arch }}.deb" > + > +- name: Install gitlab-runner via package manager > + apt: deb="/root/gitlab-runner_{{ gitlab_runner_arch }}.deb" > > - name: Register the gitlab-runner > - command: "/usr/local/bin/gitlab-runner register --non-interactive > --url {{ gitlab_runner_server_url }} --registration-token {{ > gitlab_runner_registration_token }} --executor shell --tag-list {{ > ansible_facts[\"architecture\"] }},{{ ansible_facts[\"distribution\"]|lower > }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ > ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] > }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] > }})'" > + command: "/usr/bin/gitlab-runner register --non-interactive --url {{ > gitlab_runner_server_url }} --registration-token {{ > gitlab_runner_registration_token }} --executor shell --tag-list {{ > ansible_facts[\"architecture\"] }},{{ ansible_facts[\"distribution\"]|lower > }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ > ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] > }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] > }})'" > > - name: Install the gitlab-runner service using its own functionality > - command: /usr/local/bin/gitlab-runner install --user gitlab-runner > --working-directory /home/gitlab-runner > + command: "/usr/bin/gitlab-runner install --user gitlab-runner > --working-directory /home/gitlab-runner/{{ ansible_facts[\"architecture\"] }}" >register: gitlab_runner_install_service_result >failed_when: "gitlab_runner_install_service_result.rc != 0 and > \"already exists\" not in gitlab_runner_install_service_result.stderr" > > @@ -70,33 +75,30 @@ > state: started > enabled: yes > > -- name: Download secondary gitlab-runner > - get_url: > -dest: /usr/local/bin/gitlab-runner-arm > -url: "https://s3.amazonaws.com/gitlab-runner-downloads/v{{ > gitlab_runner_version }}/binaries/gitlab-runner-{{ gitlab_runner_os }}-arm" > -owner: gitlab-runner > -group: gitlab-runner > -mode: u=rwx,g=rwx,o=rx > +- name: Create working directories secondary runner > + file: > +path: "/home/gitlab-runner/arm" > +state: directory >when: > - ansible_facts['distribution'] == 'Ubuntu' > - ansible_facts['architecture'] == 'aarch64' > -- ansible_facts['distribution_version'] == '20.04' > +- ansible_facts['distribution_version'] == '22.04' > > - name: Register secondary gitlab-runner > - command: "/usr/local/bin/gitlab-runner-arm register --non-interactive > --url {{ gitlab_runner_server_url }} --registration-token {{ > gitlab_runner_registration_token }} --executor shell --tag-list aarch32,{{ > ansible_facts[\"distribution\"]|lower }}_{{ > ansible_facts[\"distribution_version\"] }} --description '{{ > ansible_facts[\"distribution\"] }} {{
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
Evgeny Iakovlev writes: > Windows open(2) implementation opens files in text mode by default and > needs a Windows-only O_BINARY flag to open files as binary. QEMU already > knows about that flag in osdep and it is defined to 0 on non-Windows, > so we can just add it to the host_flags for better compatibility. > > Signed-off-by: Evgeny Iakovlev > Reviewed-by: Philippe Mathieu-Daudé > Reviewed-by: Bin Meng Queued to semihosting/next, thanks. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] semihosting: Write back semihosting data before completion callback
Keith Packard writes: > 'lock_user' allocates a host buffer to shadow a target buffer, > 'unlock_user' copies that host buffer back to the target and frees the > host memory. If the completion function uses the target buffer, it > must be called after unlock_user to ensure the data are present. > > This caused the arm-compatible TARGET_SYS_READC to fail as the > completion function, common_semi_readc_cb, pulled data from the target > buffer which would not have been gotten the console data. > > I decided to fix all instances of this pattern instead of just the > console_read function to make things consistent and potentially fix > bugs in other cases. > > Signed-off-by: Keith Packard Queued to semihosting/next, thanks. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: intermittent hang, s390x host, bios-tables-test test, TPM
On 1/6/23 07:10, Peter Maydell wrote: I'm seeing an intermittent hang on the s390 CI runner in the Is this a new hang or has this been occurring for a while? I am asking because the test case is not new. Stefan
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On 1/6/23 5:52 AM, Anthony PERARD wrote: > On Tue, Jan 03, 2023 at 05:58:01PM -0500, Chuck Zmudzinski wrote: >> Hello Anthony and Paul, > > Hi Chuck, > >> I am requesting your feedback to Alex Williamson's suggestion that this >> problem with assigning the correct slot address to the igd on xen should >> be fixed in libxl instead of in qemu. >> >> It seems to me that the xen folks and the kvm folks have two different >> philosophies regarding how a tool stack should be designed. kvm/libvirt >> provides much greater flexibility in configuring the guest which puts >> the burden on the administrator to set all the options correctly for >> a given feature set, while xen/xenlight does not provide so much >> flexibility and tries to automatically configure the guest based on >> a high-level feature option such as the igd-passthrough=on option that >> is available for xen guests using qemu but not for kvm guests using >> qemu. >> >> What do you think? Should libxl be patched instead of fixing the problem >> with this patch to qemu, which is contrary to Alex's suggestion? > > I do think that libxl should be able to deal with having to put a > graphic card on slot 2. QEMU already provides every API necessary for a > toolstack to be able to start a Xen guest with all the PCI card in the > right slot. But it would just be a bit more complicated to implement in > libxl. > > At the moment, libxl makes use of the QEMU machine 'xenfv', libxl should > instead start to use the 'pc' machine and add the "xen-platform" pci > device. (libxl already uses 'pc' when the "xen-platform" pci card isn't > needed.) Also probably add the other pci devices to specific slot to be > able to add the passthrough graphic card at the right slot. > > Next is to deal with migration when using the 'pc' machine, as it's just > an alias to a specific version of the machine. We need to use the same > machine on the receiving end, that is start with e.g. "pc-i440fx-7.1" if > 'pc' was an alias for it at guest creation. > > > I wonder if we can already avoid to patch the 'xenfv' machine with some > xl config: > # avoid 'xenfv' machine and use 'pc' instead > xen_platform_pci=0 > # add xen-platform pci device back > device_model_args_hvm = [ > "-device", "xen-platform,addr=3", > ] > But there's probably another device which is going to be auto-assigned > to slot 2. > > > If you feel like dealing with the technical dept in libxl, that is to > stop using 'xenfv' and use 'pc' instead, then go for it, I can help with > that. Otherwise, if the patch to QEMU only changes the behavior of the > 'xenfv' machine then I think I would be ok with it. > > I'll do a review of that QEMU patch in another email. > > Cheers, > Hello Anthony, Thanks for responding! The first part of my v6 of the patch only affects the xenfv machine. Guests created with the pc machine type will not call the new function that reserves slot 2 for the igd because that function is only called when the machine type is xenfv (or xenfv-4.2). But the new functions I added to configure the TYPE_XEN_PT_DEVICE when igd-passthru=on will be called to check if the device is an Intel igd and clear the slot if it is, but this will not have any effect on the behavior in this case because the slot was never reserved. Still, this would add some unnecessary processing in the case of machines other than xenfv, which is undesirable. So I can add a check for the machine type to a v7 of the patch that will skip the new functions that clear the reserved slot if slot 2 is not reserved and therefore does not need to be cleared. Would that be OK? Kind regards, Chuck
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote: > Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus, > as noted in docs/igd-assign.txt in the Qemu source code. > > Currently, when the xl toolstack is used to configure a Xen HVM guest with > Intel IGD passthrough to the guest with the Qemu upstream device model, > a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy > a different slot. This problem often prevents the guest from booting. > > The only available workaround is not good: Configure Xen HVM guests to use > the old and no longer maintained Qemu traditional device model available > from xenbits.xen.org which does reserve slot 2 for the Intel IGD. > > To implement this feature in the Qemu upstream device model for Xen HVM > guests, introduce the following new functions, types, and macros: > > * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE > * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS > * typedef XenPTQdevRealize function pointer > * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2 > * xen_igd_reserve_slot and xen_igd_clear_slot functions > > The new xen_igd_reserve_slot function uses the existing slot_reserved_mask > member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using > the xl toolstack with the gfx_passthru option enabled, which sets the > igd-passthru=on option to Qemu for the Xen HVM machine type. > > The new xen_igd_reserve_slot function also needs to be implemented in > hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case > when Qemu is configured with --enable-xen and --disable-xen-pci-passthrough, > in which case it does nothing. > > The new xen_igd_clear_slot function overrides qdev->realize of the parent > PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus > since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was > created in hw/i386/pc_piix.c for the case when igd-passthru=on. > > Move the call to xen_host_pci_device_get, and the associated error > handling, from xen_pt_realize to the new xen_igd_clear_slot function to > initialize the device class and vendor values which enables the checks for > the Intel IGD to succeed. The verification that the host device is an > Intel IGD to be passed through is done by checking the domain, bus, slot, > and function values as well as by checking that gfx_passthru is enabled, > the device class is VGA, and the device vendor in Intel. > > Signed-off-by: Chuck Zmudzinski This patch looks good enough. It only changes the "xenfv" machine so it doesn't prevent a proper fix to be done in the toolstack libxl. The change in xen_pci_passthrough_class_init() to try to run some code before pci_qdev_realize() could potentially break in the future due to been uncommon but hopefully that will be ok. So if no work to fix libxl appear soon, I'm ok with this patch: Reviewed-by: Anthony PERARD Thanks, -- Anthony PERARD
Re: intermittent hang, s390x host, bios-tables-test test, TPM
On Fri, 6 Jan 2023 at 13:53, Stefan Berger wrote: > > > > On 1/6/23 07:10, Peter Maydell wrote: > > I'm seeing an intermittent hang on the s390 CI runner in the > Is this a new hang or has this been occurring for a while? I am asking > because the test case is not new. It's intermittent, so no idea. -- PMM
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On 1/6/23 9:03 AM, Anthony PERARD wrote: > On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote: >> Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus, >> as noted in docs/igd-assign.txt in the Qemu source code. >> >> Currently, when the xl toolstack is used to configure a Xen HVM guest with >> Intel IGD passthrough to the guest with the Qemu upstream device model, >> a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy >> a different slot. This problem often prevents the guest from booting. >> >> The only available workaround is not good: Configure Xen HVM guests to use >> the old and no longer maintained Qemu traditional device model available >> from xenbits.xen.org which does reserve slot 2 for the Intel IGD. >> >> To implement this feature in the Qemu upstream device model for Xen HVM >> guests, introduce the following new functions, types, and macros: >> >> * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE >> * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS >> * typedef XenPTQdevRealize function pointer >> * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2 >> * xen_igd_reserve_slot and xen_igd_clear_slot functions >> >> The new xen_igd_reserve_slot function uses the existing slot_reserved_mask >> member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using >> the xl toolstack with the gfx_passthru option enabled, which sets the >> igd-passthru=on option to Qemu for the Xen HVM machine type. >> >> The new xen_igd_reserve_slot function also needs to be implemented in >> hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case >> when Qemu is configured with --enable-xen and --disable-xen-pci-passthrough, >> in which case it does nothing. >> >> The new xen_igd_clear_slot function overrides qdev->realize of the parent >> PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus >> since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was >> created in hw/i386/pc_piix.c for the case when igd-passthru=on. >> >> Move the call to xen_host_pci_device_get, and the associated error >> handling, from xen_pt_realize to the new xen_igd_clear_slot function to >> initialize the device class and vendor values which enables the checks for >> the Intel IGD to succeed. The verification that the host device is an >> Intel IGD to be passed through is done by checking the domain, bus, slot, >> and function values as well as by checking that gfx_passthru is enabled, >> the device class is VGA, and the device vendor in Intel. >> >> Signed-off-by: Chuck Zmudzinski > > > This patch looks good enough. It only changes the "xenfv" machine so it > doesn't prevent a proper fix to be done in the toolstack libxl. > > The change in xen_pci_passthrough_class_init() to try to run some code > before pci_qdev_realize() could potentially break in the future due to > been uncommon but hopefully that will be ok. > > So if no work to fix libxl appear soon, I'm ok with this patch: > > Reviewed-by: Anthony PERARD > > Thanks, > Well, our messages almost collided! I just proposed a v7 that adds a check to prevent the extra processing for cases when machine is not xenfv and the slot does not need to be cleared because it was never reserved. The proposed v7 would not change the behavior of the patch at all but it would avoid some unnecessary processing. Do you want me to submit that v7? Chuck
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
On Fri, 6 Jan 2023 at 10:21, Evgeny Iakovlev wrote: > > Windows open(2) implementation opens files in text mode by default and > needs a Windows-only O_BINARY flag to open files as binary. QEMU already > knows about that flag in osdep and it is defined to 0 on non-Windows, > so we can just add it to the host_flags for better compatibility. > > Signed-off-by: Evgeny Iakovlev > Reviewed-by: Philippe Mathieu-Daudé > Reviewed-by: Bin Meng > --- > semihosting/syscalls.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c > index 508a0ad88c..b621d78c2d 100644 > --- a/semihosting/syscalls.c > +++ b/semihosting/syscalls.c > @@ -253,7 +253,7 @@ static void host_open(CPUState *cs, > gdb_syscall_complete_cb complete, > { > CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; > char *p; > -int ret, host_flags; > +int ret, host_flags = O_BINARY; The semihosting API, at least for Arm, has a modeflags string so the guest can say whether it wants to open O_BINARY or not: https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#sys-open-0x01 So we need to plumb that down through the common semihosting code into this function and set O_BINARY accordingly. Otherwise guest code that asks for a text-mode file won't get one. I don't know about other semihosting APIs, so those would need to be checked to see what they should do. thanks -- PMM
Re: [PULL v4 76/83] vhost-user: Support vhost_dev_start
Hi, it seems this patch breaks vhost-user with DPDK. See https://bugzilla.redhat.com/show_bug.cgi?id=2155173 it seems QEMU doesn't receive the expected commands sequence: Received unexpected msg type. Expected 22 received 40 Fail to update device iotlb Received unexpected msg type. Expected 40 received 22 Received unexpected msg type. Expected 22 received 11 Fail to update device iotlb Received unexpected msg type. Expected 11 received 22 vhost VQ 1 ring restore failed: -71: Protocol error (71) Received unexpected msg type. Expected 22 received 11 Fail to update device iotlb Received unexpected msg type. Expected 11 received 22 vhost VQ 0 ring restore failed: -71: Protocol error (71) unable to start vhost net: 71: falling back on userspace virtio It receives VHOST_USER_GET_STATUS (40) when it expects VHOST_USER_IOTLB_MSG (22) and VHOST_USER_IOTLB_MSG when it expects VHOST_USER_GET_STATUS. and VHOST_USER_GET_VRING_BASE (11) when it expect VHOST_USER_GET_STATUS and so on. Any idea? Thanks, Laurent On 11/7/22 23:53, Michael S. Tsirkin wrote: From: Yajun Wu The motivation of adding vhost-user vhost_dev_start support is to improve backend configuration speed and reduce live migration VM downtime. Today VQ configuration is issued one by one. For virtio net with multi-queue support, backend needs to update RSS (Receive side scaling) on every rx queue enable. Updating RSS is time-consuming (typical time like 7ms). Implement already defined vhost status and message in the vhost specification [1]. (a) VHOST_USER_PROTOCOL_F_STATUS (b) VHOST_USER_SET_STATUS (c) VHOST_USER_GET_STATUS Send message VHOST_USER_SET_STATUS with VIRTIO_CONFIG_S_DRIVER_OK for device start and reset(0) for device stop. On reception of the DRIVER_OK message, backend can apply the needed setting only once (instead of incremental) and also utilize parallelism on enabling queues. This improves QEMU's live migration downtime with vhost user backend implementation by great margin, specially for the large number of VQs of 64 from 800 msec to 250 msec. [1] https://qemu-project.gitlab.io/qemu/interop/vhost-user.html Signed-off-by: Yajun Wu Acked-by: Parav Pandit Message-Id: <20221017064452.1226514-3-yaj...@nvidia.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-user.c | 74 +- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index d256ce589b..abe23d4ebe 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -81,6 +81,7 @@ enum VhostUserProtocolFeature { VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13, /* Feature 14 reserved for VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS. */ VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15, +VHOST_USER_PROTOCOL_F_STATUS = 16, VHOST_USER_PROTOCOL_F_MAX }; @@ -126,6 +127,8 @@ typedef enum VhostUserRequest { VHOST_USER_GET_MAX_MEM_SLOTS = 36, VHOST_USER_ADD_MEM_REG = 37, VHOST_USER_REM_MEM_REG = 38, +VHOST_USER_SET_STATUS = 39, +VHOST_USER_GET_STATUS = 40, VHOST_USER_MAX } VhostUserRequest; @@ -1452,6 +1455,43 @@ static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t u64, return 0; } +static int vhost_user_set_status(struct vhost_dev *dev, uint8_t status) +{ +return vhost_user_set_u64(dev, VHOST_USER_SET_STATUS, status, false); +} + +static int vhost_user_get_status(struct vhost_dev *dev, uint8_t *status) +{ +uint64_t value; +int ret; + +ret = vhost_user_get_u64(dev, VHOST_USER_GET_STATUS, &value); +if (ret < 0) { +return ret; +} +*status = value; + +return 0; +} + +static int vhost_user_add_status(struct vhost_dev *dev, uint8_t status) +{ +uint8_t s; +int ret; + +ret = vhost_user_get_status(dev, &s); +if (ret < 0) { +return ret; +} + +if ((s & status) == status) { +return 0; +} +s |= status; + +return vhost_user_set_status(dev, s); +} + static int vhost_user_set_features(struct vhost_dev *dev, uint64_t features) { @@ -1460,6 +1500,7 @@ static int vhost_user_set_features(struct vhost_dev *dev, * backend is actually logging changes */ bool log_enabled = features & (0x1ULL << VHOST_F_LOG_ALL); +int ret; /* * We need to include any extra backend only feature bits that @@ -1467,9 +1508,18 @@ static int vhost_user_set_features(struct vhost_dev *dev, * VHOST_USER_F_PROTOCOL_FEATURES bit for enabling protocol * features. */ -return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, +ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features | dev->backend_features, log_enabled); + +if (virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_STATUS)) { +
[PATCH] configure: Expand test which disable -Wmissing-braces
From: Anthony PERARD With "clang 6.0.0-1ubuntu2" on Ubuntu Bionic, the test with build fine, but clang still suggest braces around the zero initializer in a few places, where there is a subobject. Expand test to include a sub struct which doesn't build on clang 6.0.0-1ubuntu2, and give: config-temp/qemu-conf.c:7:8: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces] } x = {0}; ^ {} These are the error reported by clang on QEMU's code (v7.2.0): hw/pci-bridge/cxl_downstream.c:101:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces] dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 }; hw/pci-bridge/cxl_root_port.c:62:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces] dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 }; tests/qtest/virtio-net-test.c:322:34: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces] QOSGraphTestOptions opts = { 0 }; Reported-by: Andrew Cooper Signed-off-by: Anthony PERARD --- While Ubuntu Bionic isn't supposed to be supported anymore, clang v6 is still the minimum required as tested by ./configure. --- configure | 4 1 file changed, 4 insertions(+) diff --git a/configure b/configure index 9f0bc57546..3cd9b8bad4 100755 --- a/configure +++ b/configure @@ -1290,7 +1290,11 @@ fi # Disable -Wmissing-braces on older compilers that warn even for # the "universal" C zero initializer {0}. cat > $TMPC << EOF +struct s { + void *a; +}; struct { + struct s s; int a[2]; } x = {0}; EOF -- Anthony PERARD
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On 1/6/23 9:10 AM, Chuck Zmudzinski wrote: > On 1/6/23 9:03 AM, Anthony PERARD wrote: >> On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote: >>> Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus, >>> as noted in docs/igd-assign.txt in the Qemu source code. >>> >>> Currently, when the xl toolstack is used to configure a Xen HVM guest with >>> Intel IGD passthrough to the guest with the Qemu upstream device model, >>> a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy >>> a different slot. This problem often prevents the guest from booting. >>> >>> The only available workaround is not good: Configure Xen HVM guests to use >>> the old and no longer maintained Qemu traditional device model available >>> from xenbits.xen.org which does reserve slot 2 for the Intel IGD. >>> >>> To implement this feature in the Qemu upstream device model for Xen HVM >>> guests, introduce the following new functions, types, and macros: >>> >>> * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE >>> * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS >>> * typedef XenPTQdevRealize function pointer >>> * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2 >>> * xen_igd_reserve_slot and xen_igd_clear_slot functions >>> >>> The new xen_igd_reserve_slot function uses the existing slot_reserved_mask >>> member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using >>> the xl toolstack with the gfx_passthru option enabled, which sets the >>> igd-passthru=on option to Qemu for the Xen HVM machine type. >>> >>> The new xen_igd_reserve_slot function also needs to be implemented in >>> hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case >>> when Qemu is configured with --enable-xen and --disable-xen-pci-passthrough, >>> in which case it does nothing. >>> >>> The new xen_igd_clear_slot function overrides qdev->realize of the parent >>> PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus >>> since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was >>> created in hw/i386/pc_piix.c for the case when igd-passthru=on. >>> >>> Move the call to xen_host_pci_device_get, and the associated error >>> handling, from xen_pt_realize to the new xen_igd_clear_slot function to >>> initialize the device class and vendor values which enables the checks for >>> the Intel IGD to succeed. The verification that the host device is an >>> Intel IGD to be passed through is done by checking the domain, bus, slot, >>> and function values as well as by checking that gfx_passthru is enabled, >>> the device class is VGA, and the device vendor in Intel. >>> >>> Signed-off-by: Chuck Zmudzinski >> >> >> This patch looks good enough. It only changes the "xenfv" machine so it >> doesn't prevent a proper fix to be done in the toolstack libxl. >> >> The change in xen_pci_passthrough_class_init() to try to run some code >> before pci_qdev_realize() could potentially break in the future due to >> been uncommon but hopefully that will be ok. >> >> So if no work to fix libxl appear soon, I'm ok with this patch: Well, I can tell you and others who use qemu are more comfortable fixing this in libxl, so hold off for a week or so. I should have a patch to fix this in libxl written and tested by then. If for some reason that does not work out, then we can fix it in qemu. Cheers, Chuck >> >> Reviewed-by: Anthony PERARD >> >> Thanks, >>
Support -O
Hi QEMU, I am trying to use the -O function to convert a file format to vmdk, but get the below response. Please could you help? Bens-iMac:~ ben$ qemu-img -O output_fmt vmdk /Users/ben/Documents/Windows11_InsiderPreview_Client_ARM64_en-us_22598.VHDX ~/Desktop/Install/Windows11ARM.vmdk qemu-img: unrecognized option '-O' Many thanks, Ben
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On Fri, Jan 06, 2023 at 09:10:55AM -0500, Chuck Zmudzinski wrote: > Well, our messages almost collided! I just proposed a v7 that adds > a check to prevent the extra processing for cases when machine is > not xenfv and the slot does not need to be cleared because it was > never reserved. The proposed v7 would not change the behavior of the > patch at all but it would avoid some unnecessary processing. Do you > want me to submit that v7? Well, preventing an simple assignment and a message from getting logged isn't going to get us far. On the other end, the message "using slot 2" when we don't even if slot 2 is actually going to be used could be confusing, so I guess preventing that message from been logged could be useful indeed. So your proposed v7 would be fine. Cheers, -- Anthony PERARD
Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru
On 1/6/23 9:31 AM, Chuck Zmudzinski wrote: > On 1/6/23 9:10 AM, Chuck Zmudzinski wrote: >> On 1/6/23 9:03 AM, Anthony PERARD wrote: >>> On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote: Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus, as noted in docs/igd-assign.txt in the Qemu source code. Currently, when the xl toolstack is used to configure a Xen HVM guest with Intel IGD passthrough to the guest with the Qemu upstream device model, a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy a different slot. This problem often prevents the guest from booting. The only available workaround is not good: Configure Xen HVM guests to use the old and no longer maintained Qemu traditional device model available from xenbits.xen.org which does reserve slot 2 for the Intel IGD. To implement this feature in the Qemu upstream device model for Xen HVM guests, introduce the following new functions, types, and macros: * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS * typedef XenPTQdevRealize function pointer * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2 * xen_igd_reserve_slot and xen_igd_clear_slot functions The new xen_igd_reserve_slot function uses the existing slot_reserved_mask member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using the xl toolstack with the gfx_passthru option enabled, which sets the igd-passthru=on option to Qemu for the Xen HVM machine type. The new xen_igd_reserve_slot function also needs to be implemented in hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case when Qemu is configured with --enable-xen and --disable-xen-pci-passthrough, in which case it does nothing. The new xen_igd_clear_slot function overrides qdev->realize of the parent PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was created in hw/i386/pc_piix.c for the case when igd-passthru=on. Move the call to xen_host_pci_device_get, and the associated error handling, from xen_pt_realize to the new xen_igd_clear_slot function to initialize the device class and vendor values which enables the checks for the Intel IGD to succeed. The verification that the host device is an Intel IGD to be passed through is done by checking the domain, bus, slot, and function values as well as by checking that gfx_passthru is enabled, the device class is VGA, and the device vendor in Intel. Signed-off-by: Chuck Zmudzinski >>> >>> >>> This patch looks good enough. It only changes the "xenfv" machine so it >>> doesn't prevent a proper fix to be done in the toolstack libxl. >>> >>> The change in xen_pci_passthrough_class_init() to try to run some code >>> before pci_qdev_realize() could potentially break in the future due to >>> been uncommon but hopefully that will be ok. >>> >>> So if no work to fix libxl appear soon, I'm ok with this patch: > > Well, I can tell you and others who use qemu are more comfortable > fixing this in libxl, so hold off for a week or so. I should have > a patch to fix this in libxl written and tested by then. If for > some reason that does not work out, then we can fix it in qemu. One last thought: the only donwnside to fixing this in libxl is that other toolstacks that configure qemu to use the xenfv machine will not benefit from the fix in qemu that would simplify configuring the guest correctly for the igd. Other toolstacks would still need to override the default behavior of adding the xen platform device at slot 2. I think no matter what, we should at least patch qemu to have the xen-platform device use slot 3 instead of being automatically assigned to slot 2 when igd-passthru=on. The rest of the fix could then be implemented in libxl so that other pci devices such as emulated network devices, other passed through pci devices, etc., do not take slot 2 when gfx_passthru in xl.cfg is set. So, unless I hear any objection, my plan is to patch qemu to use slot 3 for the xen platform device when igd-passthru is on, and implement the rest of the fix in libxl. I should have it ready within a week. Thanks for your help, Chuck
Re: intermittent hang, s390x host, bios-tables-test test, TPM
On 1/6/23 07:10, Peter Maydell wrote: I'm seeing an intermittent hang on the s390 CI runner in the bios-tables-test test. It looks like we've deadlocked because: * the TPM device is waiting for data on its socket that never arrives, and it's holding the iothread lock * QEMU is therefore not making forward progress; in particular it is unable to handle qtest queries/responses * the test binary thread 1 is waiting to get a response to its qtest command, which is not going to arrive * test binary thread 3 (tpm_emu_ctrl_thread) is has hit an assertion and is trying to kill QEMU via qtest_kill_qemu() * qtest_kill_qemu() is only a "SIGTERM and wait", so will wait forever, because QEMU won't respond to the SIGTERM while it's blocked waiting for the TPM device to release the iothread lock * because the ctrl-thread is waiting for QEMU to exit, it's never going to send the data that would unblock the TPM device emulation [...] Thread 3 (Thread 0x3ff8dafe900 (LWP 2661316)): #0 0x03ff8e9c6002 in __GI___wait4 (pid=, stat_loc=stat_loc@entry=0x2aa0b42c9bc, options=, usage=usage@entry=0x0) at ../sysdeps/unix/sysv/linux/wait4.c:27 #1 0x03ff8e9c5f72 in __GI___waitpid (pid=, stat_loc=stat_loc@entry=0x2aa0b42c9bc, options=options@entry=0) at waitpid.c:38 #2 0x02aa0952a516 in qtest_wait_qemu (s=0x2aa0b42c9b0) at ../tests/qtest/libqtest.c:206 #3 0x02aa0952a58a in qtest_kill_qemu (s=0x2aa0b42c9b0) at ../tests/qtest/libqtest.c:229 #4 0x03ff8f0c288e in g_hook_list_invoke () from /lib/s390x-linux-gnu/libglib-2.0.so.0 #5 #6 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #7 0x03ff8e9240a2 in __GI_abort () at abort.c:79 #8 0x03ff8f0feda8 in g_assertion_message () from /lib/s390x-linux-gnu/libglib-2.0.so.0 #9 0x03ff8f0fedfe in g_assertion_message_expr () from /lib/s390x-linux-gnu/libglib-2.0.so.0 #10 0x02aa09522904 in tpm_emu_ctrl_thread (data=0x3fff5ffa160) at ../tests/qtest/tpm-emu.c:189 This here seems to be the root cause. An unknown control channel command was received from the TPM emulator backend by the control channel thread and we end up in g_assert_not_reached(). https://github.com/qemu/qemu/blob/master/tests/qtest/tpm-emu.c#L189 ret = qio_channel_read(ioc, (char *)&cmd, sizeof(cmd), NULL); if (ret <= 0) { break; } cmd = be32_to_cpu(cmd); switch (cmd) { [...] default: g_debug("unimplemented %u", cmd); g_assert_not_reached();<-- } I will run this test case in an endless loop on an x86_64 host and see what we get there ... Stefan #11 0x03ff8f0ffb7c in ?? () from /lib/s390x-linux-gnu/libglib-2.0.so.0 #12 0x03ff8eb07e66 in start_thread (arg=0x3ff8dafe900) at pthread_create.c:477 #13 0x03ff8e9fcbe6 in thread_start () at ../sysdeps/unix/sysv/linux/s390/s390-64/clone.S:65
[PATCH v2] scripts/ci: update gitlab-runner playbook to use latest runner
We were using quite and old runner on our machines and running into issues with stalling jobs. Gitlab in the meantime now reliably provide the latest packaged versions of the runner under a stable URL. This update: - creates a per-arch subdir for builds - switches from binary tarballs to deb packages - re-uses the same binary for the secondary runner - updates distro check for second to 22.04 Note this script isn't fully idempotent as we end up accumulating runners especially during testing. However we also want to be able to run twice with different GitLab keys (e.g. project and personal) so I think we just have to be mindful of that during testing. Signed-off-by: Alex Bennée --- v2 - only register aarch32 runner, move service start post both registers - tested on s390x --- scripts/ci/setup/gitlab-runner.yml | 56 +++--- scripts/ci/setup/vars.yml.template | 2 -- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/scripts/ci/setup/gitlab-runner.yml b/scripts/ci/setup/gitlab-runner.yml index 33128be85d..95d4199c03 100644 --- a/scripts/ci/setup/gitlab-runner.yml +++ b/scripts/ci/setup/gitlab-runner.yml @@ -50,60 +50,30 @@ - name: Download the matching gitlab-runner get_url: -dest: /usr/local/bin/gitlab-runner -url: "https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version }}/binaries/gitlab-runner-{{ gitlab_runner_os }}-{{ gitlab_runner_arch }}" -owner: gitlab-runner -group: gitlab-runner -mode: u=rwx,g=rwx,o=rx - -- name: Register the gitlab-runner - command: "/usr/local/bin/gitlab-runner register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell --tag-list {{ ansible_facts[\"architecture\"] }},{{ ansible_facts[\"distribution\"]|lower }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] }})'" - -- name: Install the gitlab-runner service using its own functionality - command: /usr/local/bin/gitlab-runner install --user gitlab-runner --working-directory /home/gitlab-runner - register: gitlab_runner_install_service_result - failed_when: "gitlab_runner_install_service_result.rc != 0 and \"already exists\" not in gitlab_runner_install_service_result.stderr" +dest: "/root/" +url: "https://gitlab-runner-downloads.s3.amazonaws.com/latest/deb/gitlab-runner_{{ gitlab_runner_arch }}.deb" -- name: Enable the gitlab-runner service - service: -name: gitlab-runner -state: started -enabled: yes +- name: Install gitlab-runner via package manager + apt: deb="/root/gitlab-runner_{{ gitlab_runner_arch }}.deb" -- name: Download secondary gitlab-runner - get_url: -dest: /usr/local/bin/gitlab-runner-arm -url: "https://s3.amazonaws.com/gitlab-runner-downloads/v{{ gitlab_runner_version }}/binaries/gitlab-runner-{{ gitlab_runner_os }}-arm" -owner: gitlab-runner -group: gitlab-runner -mode: u=rwx,g=rwx,o=rx - when: -- ansible_facts['distribution'] == 'Ubuntu' -- ansible_facts['architecture'] == 'aarch64' -- ansible_facts['distribution_version'] == '20.04' +- name: Register the gitlab-runner + command: "/usr/bin/gitlab-runner register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell --tag-list {{ ansible_facts[\"architecture\"] }},{{ ansible_facts[\"distribution\"]|lower }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] }})'" +# The secondary runner will still run under the single gitlab-runner service - name: Register secondary gitlab-runner - command: "/usr/local/bin/gitlab-runner-arm register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell --tag-list aarch32,{{ ansible_facts[\"distribution\"]|lower }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansible_facts[\"architecture\"] }} ({{ ansible_facts[\"os_family\"] }})'" + command: "/usr/bin/gitlab-runner register --non-interactive --url {{ gitlab_runner_server_url }} --registration-token {{ gitlab_runner_registration_token }} --executor shell --tag-list aarch32,{{ ansible_facts[\"distribution\"]|lower }}_{{ ansible_facts[\"distribution_version\"] }} --description '{{ ansible_facts[\"distribution\"] }} {{ ansible_facts[\"distribution_version\"] }} {{ ansib
Re: [PULL 00/51] virtio,pc,pci: features, cleanups, fixes
On Thu, 5 Jan 2023 at 21:53, Michael S. Tsirkin wrote: > > On Thu, Jan 05, 2023 at 09:04:37PM +, Peter Maydell wrote: > > On Thu, 5 Jan 2023 at 16:32, Michael S. Tsirkin wrote: > > > > > > On Thu, Jan 05, 2023 at 04:56:39AM -0500, Michael S. Tsirkin wrote: > > > > On Thu, Jan 05, 2023 at 04:14:20AM -0500, Michael S. Tsirkin wrote: > > > > > The following changes since commit > > > > > cb9c6a8e5ad6a1f0ce164d352e3102df46986e22: > > > > > > > > > > .gitlab-ci.d/windows: Work-around timeout and OpenGL problems of > > > > > the MSYS2 jobs (2023-01-04 18:58:33 +) > > > > > > > > > > are available in the Git repository at: > > > > > > > > > > https://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git > > > > > tags/for_upstream > > > > > > > > > > for you to fetch changes up to > > > > > 6529cb46fa76bb4b4f217d6fcc68b61b543062c4: > > > > > > > > 7c77271205339d3b161bdf925f5ead799b582e47 now - I dropped one patch > > > > as v2 is forthcoming. > > > > > > And now it's c46dcec9f699508e811cb6a140250d07486b0e41 as I replaced that > > > patch with it's v2. Sorry about the back and forth, but it seemed > > > important :( > > > > > > > > > > > > vhost-scsi: fix memleak of vsc->inflight (2023-01-05 04:07:39 -0500) > > > > > > > > > > > > > > > virtio,pc,pci: features, cleanups, fixes > > > > > > > > > > mostly vhost-vdpa: > > > > > guest announce feature emulation when using shadow virtqueue > > > > > support for configure interrupt > > > > > startup speed ups > > > > > > > > > > an acpi change to only generate cluster node in PPTT when specified > > > > > for arm > > > > > > > > > > misc fixes, cleanups > > > > > > > > > > Signed-off-by: Michael S. Tsirkin > > > > > > > > > > > > > > > > > > > > Note: linux-user build is failing for me on master, I just > > > > > disabled it for now as nothing I'm doing should affect linux-user. > > > > > Didn't debug yet. > > > > Compile failures on freebsd in the bsd-user build: > > > > https://gitlab.com/qemu-project/qemu/-/jobs/3561556072 > > https://gitlab.com/qemu-project/qemu/-/jobs/3561556071 > > > > Probably something in Markus' include-file cleanup, I suspect > > some file is missing its osdep.h include. > > > > thanks > > -- PMM > > > Pushed a fixup, commit 1df76fab679e9a673b71531925fe12ceb89eaecb now. > Pls let me know, thanks! Still failing on FreeBSD, for a different reason: https://gitlab.com/qemu-project/qemu/-/jobs/3565200188 thanks -- PMM
Re: [PATCH] hw/pci-host/mv64361: Reuse pci_swizzle_map_irq_fn
On Fri, 6 Jan 2023, Bernhard Beschow wrote: mv64361_pcihost_map_irq() is a reimplementation of pci_swizzle_map_irq_fn(). Resolve this redundancy. Signed-off-by: Bernhard Beschow Reviewed-by: BALATON Zoltan --- Testing done: * `qemu-system-ppc -machine pegasos2 \ -rtc base=localtime \ -device ati-vga,guest_hwcursor=true,romfile="" \ -cdrom morphos-3.17.iso \ -kernel morphos-3.17/boot.img` --- hw/pci-host/mv64361.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c index cc9c4d6d3b..70db142ec3 100644 --- a/hw/pci-host/mv64361.c +++ b/hw/pci-host/mv64361.c @@ -72,11 +72,6 @@ struct MV64361PCIState { uint64_t remap[5]; }; -static int mv64361_pcihost_map_irq(PCIDevice *pci_dev, int n) -{ -return (n + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS; -} - static void mv64361_pcihost_set_irq(void *opaque, int n, int level) { MV64361PCIState *s = opaque; @@ -97,7 +92,7 @@ static void mv64361_pcihost_realize(DeviceState *dev, Error **errp) g_free(name); name = g_strdup_printf("pci.%d", s->index); h->bus = pci_register_root_bus(dev, name, mv64361_pcihost_set_irq, - mv64361_pcihost_map_irq, dev, + pci_swizzle_map_irq_fn, dev, &s->mem, &s->io, 0, 4, TYPE_PCI_BUS); g_free(name); pci_create_simple(h->bus, 0, TYPE_MV64361_PCI_BRIDGE);
Re: intermittent hang, s390x host, bios-tables-test test, TPM
On Fri, 6 Jan 2023 at 15:16, Stefan Berger wrote: > > > > On 1/6/23 07:10, Peter Maydell wrote: > > I'm seeing an intermittent hang on the s390 CI runner in the > > bios-tables-test test. It looks like we've deadlocked because: > > > > * the TPM device is waiting for data on its socket that never arrives, > > and it's holding the iothread lock > > * QEMU is therefore not making forward progress; > > in particular it is unable to handle qtest queries/responses > > * the test binary thread 1 is waiting to get a response to its > > qtest command, which is not going to arrive > > * test binary thread 3 (tpm_emu_ctrl_thread) is has hit an > > assertion and is trying to kill QEMU via qtest_kill_qemu() > > * qtest_kill_qemu() is only a "SIGTERM and wait", so will wait > > forever, because QEMU won't respond to the SIGTERM while it's > > blocked waiting for the TPM device to release the iothread lock > > * because the ctrl-thread is waiting for QEMU to exit, it's never > > going to send the data that would unblock the TPM device emulation > > > [...] > > > > > Thread 3 (Thread 0x3ff8dafe900 (LWP 2661316)): > > #0 0x03ff8e9c6002 in __GI___wait4 (pid=, > > stat_loc=stat_loc@entry=0x2aa0b42c9bc, options=, > > usage=usage@entry=0x0) at ../sysdeps/unix/sysv/linux/wait4.c:27 > > #1 0x03ff8e9c5f72 in __GI___waitpid (pid=, > > stat_loc=stat_loc@entry=0x2aa0b42c9bc, options=options@entry=0) at > > waitpid.c:38 > > #2 0x02aa0952a516 in qtest_wait_qemu (s=0x2aa0b42c9b0) at > > ../tests/qtest/libqtest.c:206 > > #3 0x02aa0952a58a in qtest_kill_qemu (s=0x2aa0b42c9b0) at > > ../tests/qtest/libqtest.c:229 > > #4 0x03ff8f0c288e in g_hook_list_invoke () from > > /lib/s390x-linux-gnu/libglib-2.0.so.0 > > #5 > > #6 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 > > #7 0x03ff8e9240a2 in __GI_abort () at abort.c:79 > > #8 0x03ff8f0feda8 in g_assertion_message () from > > /lib/s390x-linux-gnu/libglib-2.0.so.0 > > #9 0x03ff8f0fedfe in g_assertion_message_expr () from > > /lib/s390x-linux-gnu/libglib-2.0.so.0 > > #10 0x02aa09522904 in tpm_emu_ctrl_thread (data=0x3fff5ffa160) at > > ../tests/qtest/tpm-emu.c:189 > > This here seems to be the root cause. An unknown control channel command > was received from the TPM emulator backend by the control channel thread > and we end up in g_assert_not_reached(). Yeah. It would be good if we didn't deadlock without printing the assertion, though... I guess we could improve qtest_kill_qemu() so it doesn't wait indefinitely for QEMU to exit but instead sends a SIGKILL 20 seconds after the SIGTERM. (Annoyingly, there is no convenient "waitpid but with a timeout" function...) thanks -- PMM
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
Peter Maydell writes: > On Fri, 6 Jan 2023 at 10:21, Evgeny Iakovlev > wrote: >> >> Windows open(2) implementation opens files in text mode by default and >> needs a Windows-only O_BINARY flag to open files as binary. QEMU already >> knows about that flag in osdep and it is defined to 0 on non-Windows, >> so we can just add it to the host_flags for better compatibility. >> >> Signed-off-by: Evgeny Iakovlev >> Reviewed-by: Philippe Mathieu-Daudé >> Reviewed-by: Bin Meng >> --- >> semihosting/syscalls.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c >> index 508a0ad88c..b621d78c2d 100644 >> --- a/semihosting/syscalls.c >> +++ b/semihosting/syscalls.c >> @@ -253,7 +253,7 @@ static void host_open(CPUState *cs, >> gdb_syscall_complete_cb complete, >> { >> CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; >> char *p; >> -int ret, host_flags; >> +int ret, host_flags = O_BINARY; > > The semihosting API, at least for Arm, has a modeflags string so the > guest can say whether it wants to open O_BINARY or not: > https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#sys-open-0x01 > > So we need to plumb that down through the common semihosting code > into this function and set O_BINARY accordingly. Otherwise guest > code that asks for a text-mode file won't get one. We used to, in fact we still have a remnant of the code where we do: #ifndef O_BINARY #define O_BINARY 0 #endif I presume because the only places it exists in libc is wrapped in stuff like: #if defined (__CYGWIN__) #define O_BINARY _FBINARY So the mapping got removed in a1a2a3e609 (semihosting: Remove GDB_O_BINARY) because GDB knows nothing of this and as far as I can tell neither does Linux whatever ISO C might say about it. Is this a host detail leakage to the guest? Should a semihosting app be caring about what fopen() modes the underlying host supports? At least a default O_BINARY for windows is most likely to DTRT. > I don't know about other semihosting APIs, so those would need > to be checked to see what they should do. > > thanks > -- PMM -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] target/arm: Allow users to set the number of VFP registers
On Mon, 2 Jan 2023 at 17:52, Cédric Le Goater wrote: > > Cortex A7 CPUs with an FPU implementing VFPv4 without NEON support > have 16 64-bit FPU registers and not 32 registers. Let users set the > number of VFP registers with a CPU property. > > The primary use case of this property is for the Cortex A7 of the > Aspeed AST2600 SoC. > > Signed-off-by: Cédric Le Goater > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 2fa022f62b..27af57ea9a 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1258,6 +1258,9 @@ static Property arm_cpu_cfgend_property = > static Property arm_cpu_has_vfp_property = > DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true); > > +static Property arm_cpu_has_vfp_d32_property = > +DEFINE_PROP_BOOL("vfp-d32", ARMCPU, has_vfp_d32, true); > + > static Property arm_cpu_has_neon_property = > DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true); > > @@ -1384,8 +1387,11 @@ void arm_cpu_post_init(Object *obj) > ? cpu_isar_feature(aa64_fp_simd, cpu) > : cpu_isar_feature(aa32_vfp, cpu)) { > cpu->has_vfp = true; > +cpu->has_vfp_d32 = true; We shouldn't default to true if the CPU default is D16 already. You should be able to use cpu_isar_feature(aa32_simd_r32, cpu) to check this. The setup of the property should probably be in its own if(), rather than tucked into the has_vfp if(), so if (cpu->has_vfp && cpu_isar_feature(aa32_simd_r32, cpu)) { cpu->has_vfp_d32 = true; add property; } This would mean that if the CPU is D16-only then the user can't make it D32, which I think is OK. You could write it to allow that if you wanted I guess, but then you'd need to special-case M-profile, which doesn't permit D32 at all. > if (!kvm_enabled()) { > qdev_property_add_static(DEVICE(obj), &arm_cpu_has_vfp_property); > +qdev_property_add_static(DEVICE(obj), > + &arm_cpu_has_vfp_d32_property); > } > } > > @@ -1650,6 +1656,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error > **errp) > return; > } > > +if (!cpu->has_vfp_d32) { > +uint32_t u; > + > +u = cpu->isar.mvfr0; > +u = FIELD_DP32(u, MVFR0, SIMDREG, 1); /* 16 registers */ > +cpu->isar.mvfr0 = u; > +} > + > if (!cpu->has_vfp) { > uint64_t t; > uint32_t u; There should be a check so the user can't both disable D32 and enable Neon (Neon always has 32 dregs). Armv8A doesn't permit D16, so we shouldn't allow that combination either (but note that v8R, support for which just landed, *does* permit it). thanks -- PMM
Re: intermittent hang, s390x host, bios-tables-test test, TPM
On 1/6/23 10:39, Peter Maydell wrote: On Fri, 6 Jan 2023 at 15:16, Stefan Berger wrote: On 1/6/23 07:10, Peter Maydell wrote: I'm seeing an intermittent hang on the s390 CI runner in the bios-tables-test test. It looks like we've deadlocked because: Thread 3 (Thread 0x3ff8dafe900 (LWP 2661316)): #0 0x03ff8e9c6002 in __GI___wait4 (pid=, stat_loc=stat_loc@entry=0x2aa0b42c9bc, options=, usage=usage@entry=0x0) at ../sysdeps/unix/sysv/linux/wait4.c:27 #1 0x03ff8e9c5f72 in __GI___waitpid (pid=, stat_loc=stat_loc@entry=0x2aa0b42c9bc, options=options@entry=0) at waitpid.c:38 #2 0x02aa0952a516 in qtest_wait_qemu (s=0x2aa0b42c9b0) at ../tests/qtest/libqtest.c:206 #3 0x02aa0952a58a in qtest_kill_qemu (s=0x2aa0b42c9b0) at ../tests/qtest/libqtest.c:229 #4 0x03ff8f0c288e in g_hook_list_invoke () from /lib/s390x-linux-gnu/libglib-2.0.so.0 #5 #6 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #7 0x03ff8e9240a2 in __GI_abort () at abort.c:79 #8 0x03ff8f0feda8 in g_assertion_message () from /lib/s390x-linux-gnu/libglib-2.0.so.0 #9 0x03ff8f0fedfe in g_assertion_message_expr () from /lib/s390x-linux-gnu/libglib-2.0.so.0 #10 0x02aa09522904 in tpm_emu_ctrl_thread (data=0x3fff5ffa160) at ../tests/qtest/tpm-emu.c:189 This here seems to be the root cause. An unknown control channel command was received from the TPM emulator backend by the control channel thread and we end up in g_assert_not_reached(). Yeah. It would be good if we didn't deadlock without printing the assertion, though... I guess we could improve qtest_kill_qemu() so it doesn't wait indefinitely for QEMU to exit but instead sends a SIGKILL 20 seconds after the SIGTERM. (Annoyingly, there is no convenient "waitpid but with a timeout" function...) Yes, wait5(&to,...) doesn't exist, yet. I guess one would have to use a loop sending signal 0 to the pid for 20 seconds? Though I'd really like to know where that data race is coming from and why we get an unknown command. I am now running this on a ppc64 and x86_64 host over the weekend to see what happens. All good so far. Stefan thanks -- PMM
Re: [PATCH v3 0/7] Enable Cubieboard A10 boot SPL from SD card
On Mon, 26 Dec 2022 at 22:03, Strahinja Jankovic wrote: > > This patch series adds missing Allwinner A10 modules needed for > successful SPL boot: > - Clock controller module > - DRAM controller > - I2C0 controller (added also for Allwinner H3 since it is the same) > - AXP-209 connected to I2C0 bus > > It also updates Allwinner A10 emulation so SPL is copied from attached > SD card if `-kernel` parameter is not passed when starting QEMU > (approach adapted from Allwinner H3 implementation). > > Boot from SD card has been tested with Cubieboard Armbian SD card image and > custom > Yocto image built for Cubieboard. > Example usage for Armbian image: > qemu-system-arm -M cubieboard -nographic -sd > ~/Armbian_22.11.0-trunk_Cubieboard_kinetic_edge_6.0.7.img Applied to target-arm.next, thanks. -- PMM
Re: [PATCH] target/arm: Fix sve_probe_page
On Wed, 4 Jan 2023 at 19:01, Richard Henderson wrote: > > Don't dereference CPUTLBEntryFull until we verify that > the page is valid. Move the other user-only info field > updates after the valid check to match. > > Cc: qemu-sta...@nongnu.org > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1412 > Signed-off-by: Richard Henderson > --- > target/arm/sve_helper.c | 14 +- Applied to target-arm.next, thanks. -- PMM
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
On Fri, 6 Jan 2023 at 15:44, Alex Bennée wrote: > Peter Maydell writes: > > The semihosting API, at least for Arm, has a modeflags string so the > > guest can say whether it wants to open O_BINARY or not: > > https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#sys-open-0x01 > > > > So we need to plumb that down through the common semihosting code > > into this function and set O_BINARY accordingly. Otherwise guest > > code that asks for a text-mode file won't get one. > > We used to, in fact we still have a remnant of the code where we do: > > #ifndef O_BINARY > #define O_BINARY 0 > #endif > > I presume because the only places it exists in libc is wrapped in stuff > like: > > #if defined (__CYGWIN__) > #define O_BINARY _FBINARY > > So the mapping got removed in a1a2a3e609 (semihosting: Remove > GDB_O_BINARY) because GDB knows nothing of this and as far as I can tell > neither does Linux whatever ISO C might say about it. > > Is this a host detail leakage to the guest? Should a semihosting app be > caring about what fopen() modes the underlying host supports? At least a > default O_BINARY for windows is most likely to DTRT. I think the theory when the semihosting API was originally designed decades ago was basically "when the guest does fopen(...) this should act like it does on the host". So as a bit of portable guest code you would say whether you wanted a binary or a text file, and the effect would be that if you were running on Windows and you output a text file then you'd get \r\n like the user probably expected, and if on Linux you get \n. The gdb remote protocol, on the other hand, assumes "all files are binary", and the gdb source that implements the gdb remote file I/O operations does "always set O_BINARY if it's defined": https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/remote-fileio.c;h=3ff2a65b0ec6c7695f8659690a8f1dce9b5cdf5f;hb=HEAD#l141 So this is kind of an impedance mismatch problem -- the semihosting API wants functionality that the gdb protocol can't give us. But we don't have that mismatch issue if we're directly making host filesystem calls, because there we can set O_BINARY or not as we choose. Alternatively, we could decide that our implementation of semihosting consistently uses \n for the newline character on all hosts, such that guests which try to write text files on Windows hosts get the "wrong" newline type, but OTOH get consistently the same file regardless of host and regardless of whether semihosting is going via gdb or not. But if we want to do that we should at least note in a comment somewhere that that's a behaviour we've chosen, not something that's happened by accident. Given Windows is less unfriendly about dealing with \n-terminated files these days that might not be an unreasonable choice. -- PMM
Re: [PATCH v4] riscv: Allow user to set the satp mode
On Mon, Dec 12, 2022 at 11:22:50AM +0100, Alexandre Ghiti wrote: > RISC-V specifies multiple sizes for addressable memory and Linux probes for > the machine's support at startup via the satp CSR register (done in > csr.c:validate_vm). > > As per the specification, sv64 must support sv57, which in turn must > support sv48...etc. So we can restrict machine support by simply setting the > "highest" supported mode and the bare mode is always supported. > > You can set the satp mode using the new properties "mbare", "sv32", > "sv39", "sv48", "sv57" and "sv64" as follows: > -cpu rv64,sv57=on # Linux will boot using sv57 scheme > -cpu rv64,sv39=on # Linux will boot using sv39 scheme > > We take the highest level set by the user: > -cpu rv64,sv48=on,sv57=on # Linux will boot using sv57 scheme > > We make sure that invalid configurations are rejected: > -cpu rv64,sv32=on # Can't enable 32-bit satp mode in 64-bit > -cpu rv64,sv39=off,sv48=on # sv39 must be supported if higher modes are > # enabled > > We accept "redundant" configurations: > -cpu rv64,sv48=on,sv57=off # sv39 must be supported if higher modes are ^ from this #, it looks like a copy+paste mistake > > In addition, we now correctly set the device-tree entry 'mmu-type' using > those new properties. > > Co-Developed-by: Ludovic Henry > Signed-off-by: Ludovic Henry > Signed-off-by: Alexandre Ghiti > --- > v4: > - Use custom boolean properties instead of OnOffAuto properties, based > on ARMVQMap, as suggested by Andrew > > v3: > - Free sv_name as pointed by Bin > - Replace satp-mode with boolean properties as suggested by Andrew > - Removed RB from Atish as the patch considerably changed > > v2: > - Use error_setg + return as suggested by Alistair > - Add RB from Atish > - Fixed checkpatch issues missed in v1 > - Replaced Ludovic email address with the rivos one > > hw/riscv/virt.c| 20 +++-- > target/riscv/cpu.c | 217 +++-- > target/riscv/cpu.h | 25 ++ > target/riscv/csr.c | 13 ++- > 4 files changed, 256 insertions(+), 19 deletions(-) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index a5bc7353b4..9bb5ba7366 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -228,7 +228,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int > socket, > int cpu; > uint32_t cpu_phandle; > MachineState *mc = MACHINE(s); > -char *name, *cpu_name, *core_name, *intc_name; > +uint8_t satp_mode_max; > +char *name, *cpu_name, *core_name, *intc_name, *sv_name; > > for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) { > cpu_phandle = (*phandle)++; > @@ -236,14 +237,15 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, > int socket, > cpu_name = g_strdup_printf("/cpus/cpu@%d", > s->soc[socket].hartid_base + cpu); > qemu_fdt_add_subnode(mc->fdt, cpu_name); > -if (riscv_feature(&s->soc[socket].harts[cpu].env, > - RISCV_FEATURE_MMU)) { > -qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", > -(is_32_bit) ? "riscv,sv32" : > "riscv,sv48"); > -} else { > -qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", > -"riscv,none"); > -} > + > +satp_mode_max = satp_mode_max_from_map( > +s->soc[socket].harts[cpu].cfg.satp_mode.map, > +is_32_bit); > +sv_name = g_strdup_printf("riscv,%s", > + satp_mode_str(satp_mode_max, is_32_bit)); > +qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", sv_name); > +g_free(sv_name); > + > name = riscv_isa_string(&s->soc[socket].harts[cpu]); > qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name); > g_free(name); > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index d14e95c9dc..639231ce2e 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -27,6 +27,7 @@ > #include "time_helper.h" > #include "exec/exec-all.h" > #include "qapi/error.h" > +#include "qapi/visitor.h" > #include "qemu/error-report.h" > #include "hw/qdev-properties.h" > #include "migration/vmstate.h" > @@ -199,7 +200,7 @@ static const char * const riscv_intr_names[] = { > "reserved" > }; > > -static void register_cpu_props(DeviceState *dev); > +static void register_cpu_props(Object *obj); Please do this dev -> obj change in a separate ("no functional change intended") patch. > > const char *riscv_cpu_get_trap_name(target_ulong cause, bool async) > { > @@ -237,7 +238,7 @@ static void riscv_any_cpu_init(Object *obj) > set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU); > #endif > set_priv_version(env, PRIV_VERSION_1_12_0); > -register_cpu_props(DEVICE(obj)); > +register_
Re: [PATCH] hvf: arm: Add support for GICv3
On Mon, 19 Dec 2022 at 22:08, Alexander Graf wrote: > > We currently only support GICv2 emulation. To also support GICv3, we will > need to pass a few system registers into their respective handler functions. > > This patch adds support for HVF to call into the TCG callbacks for GICv3 > system register handlers. This is safe because the GICv3 TCG code is generic > as long as we limit ourselves to EL0 and EL1 - which are the only modes > supported by HVF. > > To make sure nobody trips over that, we also annotate callbacks that don't > work in HVF mode, such as EL state change hooks. > > With GICv3 support in place, we can run with more than 8 vCPUs. > > Signed-off-by: Alexander Graf > --- > hw/intc/arm_gicv3_cpuif.c | 8 +- > target/arm/hvf/hvf.c| 151 > target/arm/hvf/trace-events | 2 + > 3 files changed, 160 insertions(+), 1 deletion(-) > > diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c > index b17b29288c..b4e387268c 100644 > --- a/hw/intc/arm_gicv3_cpuif.c > +++ b/hw/intc/arm_gicv3_cpuif.c > @@ -21,6 +21,7 @@ > #include "hw/irq.h" > #include "cpu.h" > #include "target/arm/cpregs.h" > +#include "sysemu/tcg.h" > > /* > * Special case return value from hppvi_index(); must be larger than > @@ -2810,6 +2811,8 @@ void gicv3_init_cpuif(GICv3State *s) > * which case we'd get the wrong value. > * So instead we define the regs with no ri->opaque info, and > * get back to the GICv3CPUState from the CPUARMState. > + * > + * These CP regs callbacks can be called from either TCG or HVF code. > */ > define_arm_cp_regs(cpu, gicv3_cpuif_reginfo); > > @@ -2905,6 +2908,9 @@ void gicv3_init_cpuif(GICv3State *s) > define_arm_cp_regs(cpu, gicv3_cpuif_ich_apxr23_reginfo); > } > } > -arm_register_el_change_hook(cpu, gicv3_cpuif_el_change_hook, cs); > +if (tcg_enabled()) { > +/* We can only trap EL changes with TCG for now */ We could expand this a bit: We can only trap EL changes with TCG. However the GIC interrupt state only changes on EL changes involving EL2 or EL3, so for the non-TCG case this is OK, as EL2 and EL3 can't exist. and assert: assert(!arm_feature(&cpu->env, ARM_FEATURE_EL2)); assert(!arm_feature(&cpu->env, ARM_FEATURE_EL3)); > +static uint32_t hvf_reg2cp_reg(uint32_t reg) > +{ > +return ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP, > + (reg >> 10) & 0xf, > + (reg >> 1) & 0xf, > + (reg >> 20) & 0x3, > + (reg >> 14) & 0x7, > + (reg >> 17) & 0x7); This file has #defines for these shift and mask constants (SYSREG_OP0_SHIFT etc). > +} > + > +static bool hvf_sysreg_read_cp(CPUState *cpu, uint32_t reg, uint64_t *val) > +{ > +ARMCPU *arm_cpu = ARM_CPU(cpu); > +CPUARMState *env = &arm_cpu->env; > +const ARMCPRegInfo *ri; > + > +ri = get_arm_cp_reginfo(arm_cpu->cp_regs, hvf_reg2cp_reg(reg)); > +if (ri) { > +if (ri->accessfn) { > +if (ri->accessfn(env, ri, true) != CP_ACCESS_OK) { > +return false; > +} > +} > +if (ri->type & ARM_CP_CONST) { > +*val = ri->resetvalue; > +} else if (ri->readfn) { > +*val = ri->readfn(env, ri); > +} else { > +*val = CPREG_FIELD64(env, ri); > +} > +trace_hvf_vgic_read(ri->name, *val); > +return true; > +} Can we get here for attempts by EL0 to access EL1-only sysregs, or does hvf send the exception to EL1 without trapping out to us? If we can get here for EL0 accesses we need to check against ri->access as well as ri->accessfn. Otherwise looks OK. thanks -- PMM
Re: [PATCH] .gitlab-ci.d/windows: Work-around timeout and OpenGL problems of the MSYS2 jobs
Am 04.01.23 um 13:35 schrieb Thomas Huth: The windows jobs (especially the 32-bit job) recently started to hit the timeout limit. Bump it a little bit to ease the situation (80 minutes is quite long already - OTOH, these jobs do not have to wait for a job from the container stage to finish, so this should still be OK). Additionally, some update on the container side recently enabled OpenGL in these jobs - but the corresponding code fails to compile. Thus disable OpenGL here for the time being until someone figured out the proper fix in the shader code for this. This is strange. On my Windows msys2 system, I didn't even notice the OpenGL code was silently enabled. The code compiles without issues. Today I enabled the GtkGLArea code initialization in ui/gtk.c to test OpenGL acceleration on Windows. >--- a/ui/gtk.c >+++ b/ui/gtk.c >@@ -2435,6 +2435,12 @@ static void early_gtk_display_init(DisplayOptions *opts) > gtk_use_gl_area = true; > gtk_gl_area_init(); > } else >+#endif >+#if defined(GDK_WINDOWING_WIN32) >+ if (GDK_IS_WIN32_DISPLAY(gdk_display_get_default())) { >+ gtk_use_gl_area = true; >+ gtk_gl_area_init(); >+ } else > #endif > { > #ifdef CONFIG_X11 Well, it's a start. On a Linux guest system the WebGL Aquarium frame rate increased from 6fps to 14fps while the host processor load went down from 100% to 65%. QEMU was started with: ./qemu-system-x86_64.exe -accel whpx \ -machine pc,usb=off,vmport=off,kernel-irqchip=off \ -cpu Skylake-Client-v4,tsc-deadline=off,x2apic=off \ -smp 4,sockets=1,cores=4,threads=1 \ -device virtio-vga-gl,xres=1280,yres=768,bus=pci.0 \ -display gtk,zoom-to-fit=off,gl=on \ -trace "gd_gl_area_*_context" \ ... This is the start of the QEMU log file: Windows Hypervisor Platform accelerator is operational qemu: GtkGLArea console lacks DMABUF support. Realize gdk gl context failed: Unable to create a GL context Realize gdk gl context failed: Unable to create a GL context gd_gl_area_create_context ctx=02934703bc10, major=4, minor=4 gl_version 44 - core profile enabled gd_gl_area_destroy_context ctx=02934703bc10, current_ctx=02934703bc10 gd_gl_area_create_context ctx=02934703bc10, major=4, minor=4 gd_gl_area_create_context ctx=02934703bba0, major=4, minor=4 gd_gl_area_create_context ctx=02934703bb30, major=4, minor=4 GLSL feature level 440 gd_gl_area_create_context ctx=02934703b890, major=4, minor=4 gd_gl_area_create_context ctx=02934703bc80, major=4, minor=4 gd_gl_area_create_context ctx=02934703bcf0, major=4, minor=4 gd_gl_area_destroy_context ctx=02934703bcf0, current_ctx=02934703bcf0 ... With best regards, Volker Signed-off-by: Thomas Huth ---
[PATCH 1/2] hw/char/pl011: better handling of FIFO flags on LCR reset
Current FIFO handling code does not reset RXFE/RXFF flags when guest resets FIFO by writing to UARTLCR register, although internal FIFO state is reset to 0 read count. Actual flag update will happen only on next read or write to UART. As a result of that any guest that expects RXFE flag to be set (and RXFF to be cleared) after resetting FIFO will just hang. Correctly reset FIFO flags on FIFO reset. Also, clean up some FIFO depth handling code based on current FIFO mode. Signed-off-by: Evgeny Iakovlev --- hw/char/pl011.c | 35 +-- include/hw/char/pl011.h | 5 - 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/hw/char/pl011.c b/hw/char/pl011.c index c076813423..9108ed2be9 100644 --- a/hw/char/pl011.c +++ b/hw/char/pl011.c @@ -81,6 +81,18 @@ static void pl011_update(PL011State *s) } } +static inline unsigned pl011_get_fifo_depth(PL011State *s) +{ +return s->lcr & 0x10 ? PL011_FIFO_DEPTH : 1; +} + +static inline void pl011_reset_pipe(PL011State *s) +{ +s->read_count = 0; +s->read_pos = 0; +s->flags = PL011_FLAG_RXFE | PL011_FLAG_TXFE; +} + static uint64_t pl011_read(void *opaque, hwaddr offset, unsigned size) { @@ -94,7 +106,7 @@ static uint64_t pl011_read(void *opaque, hwaddr offset, c = s->read_fifo[s->read_pos]; if (s->read_count > 0) { s->read_count--; -if (++s->read_pos == 16) +if (++s->read_pos == PL011_FIFO_DEPTH) s->read_pos = 0; } if (s->read_count == 0) { @@ -229,8 +241,7 @@ static void pl011_write(void *opaque, hwaddr offset, case 11: /* UARTLCR_H */ /* Reset the FIFO state on FIFO enable or disable */ if ((s->lcr ^ value) & 0x10) { -s->read_count = 0; -s->read_pos = 0; +pl011_reset_pipe(s); } if ((s->lcr ^ value) & 0x1) { int break_enable = value & 0x1; @@ -273,11 +284,7 @@ static int pl011_can_receive(void *opaque) PL011State *s = (PL011State *)opaque; int r; -if (s->lcr & 0x10) { -r = s->read_count < 16; -} else { -r = s->read_count < 1; -} +r = s->read_count < pl011_get_fifo_depth(s); trace_pl011_can_receive(s->lcr, s->read_count, r); return r; } @@ -286,15 +293,15 @@ static void pl011_put_fifo(void *opaque, uint32_t value) { PL011State *s = (PL011State *)opaque; int slot; +unsigned pipe_depth; -slot = s->read_pos + s->read_count; -if (slot >= 16) -slot -= 16; +pipe_depth = pl011_get_fifo_depth(s); +slot = (s->read_pos + s->read_count) % pipe_depth; s->read_fifo[slot] = value; s->read_count++; s->flags &= ~PL011_FLAG_RXFE; trace_pl011_put_fifo(value, s->read_count); -if (!(s->lcr & 0x10) || s->read_count == 16) { +if (s->read_count == pipe_depth) { trace_pl011_put_fifo_full(); s->flags |= PL011_FLAG_RXFF; } @@ -359,7 +366,7 @@ static const VMStateDescription vmstate_pl011 = { VMSTATE_UINT32(dmacr, PL011State), VMSTATE_UINT32(int_enabled, PL011State), VMSTATE_UINT32(int_level, PL011State), -VMSTATE_UINT32_ARRAY(read_fifo, PL011State, 16), +VMSTATE_UINT32_ARRAY(read_fifo, PL011State, PL011_FIFO_DEPTH), VMSTATE_UINT32(ilpr, PL011State), VMSTATE_UINT32(ibrd, PL011State), VMSTATE_UINT32(fbrd, PL011State), @@ -399,7 +406,7 @@ static void pl011_init(Object *obj) s->read_trigger = 1; s->ifl = 0x12; s->cr = 0x300; -s->flags = 0x90; +pl011_reset_pipe(s); s->id = pl011_id_arm; } diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h index dc2c90eedc..926322e242 100644 --- a/include/hw/char/pl011.h +++ b/include/hw/char/pl011.h @@ -27,6 +27,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(PL011State, PL011) /* This shares the same struct (and cast macro) as the base pl011 device */ #define TYPE_PL011_LUMINARY "pl011_luminary" +/* Depth of UART FIFO in bytes, when FIFO mode is enabled (else depth == 1) */ +#define PL011_FIFO_DEPTH 16 + struct PL011State { SysBusDevice parent_obj; @@ -39,7 +42,7 @@ struct PL011State { uint32_t dmacr; uint32_t int_enabled; uint32_t int_level; -uint32_t read_fifo[16]; +uint32_t read_fifo[PL011_FIFO_DEPTH]; uint32_t ilpr; uint32_t ibrd; uint32_t fbrd; -- 2.34.1
[PATCH 0/2] Series of fixes for PL011 char device
Evgeny Iakovlev (2): hw/char/pl011: better handling of FIFO flags on LCR reset hw/char/pl011: check if UART is enabled before RX or TX operation hw/char/pl011.c | 51 ++--- include/hw/char/pl011.h | 5 +++- 2 files changed, 41 insertions(+), 15 deletions(-) -- 2.34.1
[PATCH 2/2] hw/char/pl011: check if UART is enabled before RX or TX operation
UART should be enabled in general and have RX enabled specifically to be able to receive data from peripheral device. Same goes for transmitting data to peripheral device and a TXE flag. Check if UART CR register has EN and RXE or TXE bits enabled before trying to receive or transmit data. Signed-off-by: Evgeny Iakovlev --- hw/char/pl011.c | 22 +++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/hw/char/pl011.c b/hw/char/pl011.c index 9108ed2be9..fcc2600944 100644 --- a/hw/char/pl011.c +++ b/hw/char/pl011.c @@ -54,6 +54,11 @@ #define INT_E (INT_OE | INT_BE | INT_PE | INT_FE) #define INT_MS (INT_RI | INT_DSR | INT_DCD | INT_CTS) +/* UARTCR bits */ +#define PL011_CR_UARTEN (1 << 0) +#define PL011_CR_TXE(1 << 8) +#define PL011_CR_RXE(1 << 9) + static const unsigned char pl011_id_arm[8] = { 0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1 }; static const unsigned char pl011_id_luminary[8] = @@ -203,6 +208,11 @@ static void pl011_trace_baudrate_change(const PL011State *s) s->ibrd, s->fbrd); } +static inline bool pl011_can_transmit(PL011State *s) +{ +return s->cr & PL011_CR_UARTEN && s->cr & PL011_CR_TXE; +} + static void pl011_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { @@ -213,7 +223,9 @@ static void pl011_write(void *opaque, hwaddr offset, switch (offset >> 2) { case 0: /* UARTDR */ -/* ??? Check if transmitter is enabled. */ +if (!pl011_can_transmit(s)) { +break; +} ch = value; /* XXX this blocks entire thread. Rewrite to use * qemu_chr_fe_write and background I/O callbacks */ @@ -284,7 +296,11 @@ static int pl011_can_receive(void *opaque) PL011State *s = (PL011State *)opaque; int r; -r = s->read_count < pl011_get_fifo_depth(s); +if (!(s->cr & PL011_CR_UARTEN) || !(s->cr & PL011_CR_RXE)) { +r = 0; +} else { +r = s->read_count < pl011_get_fifo_depth(s); +} trace_pl011_can_receive(s->lcr, s->read_count, r); return r; } @@ -405,7 +421,7 @@ static void pl011_init(Object *obj) s->read_trigger = 1; s->ifl = 0x12; -s->cr = 0x300; +s->cr = PL011_CR_RXE | PL011_CR_TXE; pl011_reset_pipe(s); s->id = pl011_id_arm; -- 2.34.1
Re: [PATCH v2 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
On Wed, 2023-01-04 at 15:44 +0100, Bernhard Beschow wrote: > + if (xen_enabled()) { Could this perhaps be if (xen_mode != XEN_DISABLED) once we merge the Xen-on-KVM series? smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
On 1/6/23 12:35 PM, David Woodhouse wrote: > On Wed, 2023-01-04 at 15:44 +0100, Bernhard Beschow wrote: >> + if (xen_enabled()) { > > Could this perhaps be if (xen_mode != XEN_DISABLED) once we merge the > Xen-on-KVM series? I am not an expert and just on here as a user/tester, but I think it would depend on whether or not the Xen-on-KVM mode uses Xen PCI IRQ handling or Linux/KVM PCI IRQ handling. Chuck
Re: [PATCH 00/20] hw: Remove implicit sysbus_mmio_map() from pflash APIs
On Wed, 4 Jan 2023 at 22:04, Philippe Mathieu-Daudé wrote: > > Paving the road toward heterogeneous QEMU, the limitations of > having a single machine sysbus become more apparent. > > The sysbus_mmio_map() API forces the caller to map a sysbus > device to an address on the system bus (system bus here is > the root MemoryRegion returned by get_system_memory() ). > > This is not practical when each core has its own address > space and group of cores have access to a part of the > peripherals. > > Experimenting with the PFLASH devices. Here the fix is > quite easy, we split the pflash_cfi_register() -- which > does the implicit sysbus mapping -- into an explicit qdev > pflash_cfi_create() followed by the sysbus_mmio_map() call. pflash_cfi_register() is a legacy convenience function. If you don't like the sysbus_mmio_map() it does then you can create, configure, realize and map the device directly. This is what hw/arm/virt.c does, for instance (it wants to map the flash devices into either secure or non secure RAM). (This also lets you embed the device struct into some other struct if you want rather than using qdev_new(), though we don't have any code that does that currently.) thanks -- PMM
Re: [PATCH v2 1/1] tcg: add perfmap and jitdump
Ilya Leoshkevich writes: > Add ability to dump /tmp/perf-.map and jit-.dump. > The first one allows the perf tool to map samples to each individual > translation block. The second one adds the ability to resolve symbol > names, line numbers and inspect JITed code. > > Example of use: > > perf record qemu-x86_64 -perfmap ./a.out > perf report > > or > > perf record -k 1 qemu-x86_64 -jitdump ./a.out > perf inject -j -i perf.data -o perf.data.jitted > perf report -i perf.data.jitted > > Co-developed-by: Vanderson M. do Rosario > Co-developed-by: Alex Bennée > Signed-off-by: Ilya Leoshkevich > Message-Id: <20221012051846.1432050-2-...@linux.ibm.com> > --- > accel/tcg/debuginfo.c | 99 +++ > accel/tcg/debuginfo.h | 52 ++ > accel/tcg/meson.build | 2 + > accel/tcg/perf.c | 334 ++ > accel/tcg/perf.h | 28 > accel/tcg/translate-all.c | 8 + > docs/devel/tcg.rst| 23 +++ > hw/core/loader.c | 5 + > include/tcg/tcg.h | 4 +- > linux-user/elfload.c | 3 + > linux-user/exit.c | 2 + > linux-user/main.c | 15 ++ > linux-user/meson.build| 1 + > meson.build | 8 + > qemu-options.hx | 20 +++ > softmmu/vl.c | 11 ++ > tcg/region.c | 2 +- > tcg/tcg.c | 2 + > 18 files changed, 616 insertions(+), 3 deletions(-) > create mode 100644 accel/tcg/debuginfo.c > create mode 100644 accel/tcg/debuginfo.h > create mode 100644 accel/tcg/perf.c > create mode 100644 accel/tcg/perf.h > > diff --git a/accel/tcg/debuginfo.c b/accel/tcg/debuginfo.c > new file mode 100644 > index 000..c312db77146 > --- /dev/null > +++ b/accel/tcg/debuginfo.c > @@ -0,0 +1,99 @@ > +/* > + * Debug information support. > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/lockable.h" > + > +#include > + > +#include "debuginfo.h" > + > +static QemuMutex lock; > +static Dwfl *dwfl; > +static const Dwfl_Callbacks dwfl_callbacks = { > +.find_elf = NULL, > +.find_debuginfo = dwfl_standard_find_debuginfo, > +.section_address = NULL, > +.debuginfo_path = NULL, > +}; > + > +__attribute__((constructor)) > +static void debuginfo_init(void) > +{ > +qemu_mutex_init(&lock); > +} > + > +bool debuginfo_report_elf(const char *image_name, int image_fd, > + unsigned long long load_bias) > +{ > +QEMU_LOCK_GUARD(&lock); > + > +if (dwfl == NULL) { > +dwfl = dwfl_begin(&dwfl_callbacks); > +} else { > +dwfl_report_begin_add(dwfl); > +} > + > +if (dwfl == NULL) { > +return false; > +} > + > +dwfl_report_elf(dwfl, image_name, image_name, image_fd, load_bias, true); > +dwfl_report_end(dwfl, NULL, NULL); > +return true; > +} > + > +bool debuginfo_get_symbol(unsigned long long address, > + const char **symbol, unsigned long long *offset) > +{ > +Dwfl_Module *dwfl_module; > +GElf_Off dwfl_offset; > +GElf_Sym dwfl_sym; > + > +QEMU_LOCK_GUARD(&lock); > + > +if (dwfl == NULL) { > +return false; > +} > + > +dwfl_module = dwfl_addrmodule(dwfl, address); > +if (dwfl_module == NULL) { > +return false; > +} > + > +*symbol = dwfl_module_addrinfo(dwfl_module, address, &dwfl_offset, > + &dwfl_sym, NULL, NULL, NULL); > +if (*symbol == NULL) { > +return false; > +} > +*offset = dwfl_offset; > +return true; > +} > + > +bool debuginfo_get_line(unsigned long long address, > +const char **file, int *line) > +{ > +Dwfl_Module *dwfl_module; > +Dwfl_Line *dwfl_line; > + > +QEMU_LOCK_GUARD(&lock); > + > +if (dwfl == NULL) { > +return false; > +} > + > +dwfl_module = dwfl_addrmodule(dwfl, address); > +if (dwfl_module == NULL) { > +return false; > +} > + > +dwfl_line = dwfl_module_getsrc(dwfl_module, address); > +if (dwfl_line == NULL) { > +return false; > +} > +*file = dwfl_lineinfo(dwfl_line, NULL, line, 0, NULL, NULL); > +return true; > +} > diff --git a/accel/tcg/debuginfo.h b/accel/tcg/debuginfo.h > new file mode 100644 > index 000..d41d9d8d9b4 > --- /dev/null > +++ b/accel/tcg/debuginfo.h > @@ -0,0 +1,52 @@ > +/* > + * Debug information support. > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#ifndef ACCEL_TCG_DEBUGINFO_H > +#define ACCEL_TCG_DEBUGINFO_H > + > +#if defined(CONFIG_TCG) && defined(CONFIG_LIBDW) > +/* > + * Load debuginfo for the specified guest ELF image. > + * Return true on success, false on failure. > + */ > +bool debuginfo_report_elf(const char *image_name, int image_fd, > + unsigned long long load_bias); > + > +/* > + * Find a symbol name associated with the specified guest PC.
Re: [PATCH v5 03/14] migration: Simplify migration_iteration_run()
On Thu, 29 Dec 2022 13:03:34 +0200 Avihai Horon wrote: > From: Juan Quintela IMHO, there should always be a commit log description. Why is this a simplification? What does it allow us to do? Nothing later obviously depends on this, why is it part of this series? Thanks, Alex > Signed-off-by: Juan Quintela > Signed-off-by: Avihai Horon > --- > migration/migration.c | 25 + > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 9795d0ec5c..61b9ce0fe8 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -3758,23 +3758,24 @@ static MigIterateState > migration_iteration_run(MigrationState *s) > trace_migrate_pending(pending_size, s->threshold_size, >pend_pre, pend_compat, pend_post); > > -if (pending_size && pending_size >= s->threshold_size) { > -/* Still a significant amount to transfer */ > -if (!in_postcopy && pend_pre <= s->threshold_size && > -qatomic_read(&s->start_postcopy)) { > -if (postcopy_start(s)) { > -error_report("%s: postcopy failed to start", __func__); > -} > -return MIG_ITERATE_SKIP; > -} > -/* Just another iteration step */ > -qemu_savevm_state_iterate(s->to_dst_file, in_postcopy); > -} else { > + > +if (!pending_size || pending_size < s->threshold_size) { > trace_migration_thread_low_pending(pending_size); > migration_completion(s); > return MIG_ITERATE_BREAK; > } > > +/* Still a significant amount to transfer */ > +if (!in_postcopy && pend_pre <= s->threshold_size && > +qatomic_read(&s->start_postcopy)) { > +if (postcopy_start(s)) { > +error_report("%s: postcopy failed to start", __func__); > +} > +return MIG_ITERATE_SKIP; > +} > + > +/* Just another iteration step */ > +qemu_savevm_state_iterate(s->to_dst_file, in_postcopy); > return MIG_ITERATE_RESUME; > } >
Re: [PATCH] memory: dump HPA and access type of ramblocks
Since I applied this twice already to my local trees, let me ping for Ted to make sure it's not lost.. On Mon, Dec 05, 2022 at 08:07:12PM +0800, Ted Chen wrote: > It's convenient to dump HVA and RW/RO status of a ramblock in "info ramblock" > for debug purpose. > > Before: > Offset Used Total > 0x 0x0004 0x0004 > > After: > Offset Used TotalHVA > RO > 0x 0x0004 0x0004 0x7f12ebe0 > rw > > Signed-off-by: Ted Chen > --- > softmmu/physmem.c | 14 ++ > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/softmmu/physmem.c b/softmmu/physmem.c > index 1b606a3002..fed4dfb72c 100644 > --- a/softmmu/physmem.c > +++ b/softmmu/physmem.c > @@ -1316,15 +1316,21 @@ GString *ram_block_format(void) > GString *buf = g_string_new(""); > > RCU_READ_LOCK_GUARD(); > -g_string_append_printf(buf, "%24s %8s %18s %18s %18s\n", > - "Block Name", "PSize", "Offset", "Used", "Total"); > +g_string_append_printf(buf, "%24s %8s %18s %18s %18s %18s %3s\n", > + "Block Name", "PSize", "Offset", "Used", "Total", > + "HVA", "RO"); > + > RAMBLOCK_FOREACH(block) { > psize = size_to_str(block->page_size); > g_string_append_printf(buf, "%24s %8s 0x%016" PRIx64 " 0x%016" > PRIx64 > - " 0x%016" PRIx64 "\n", block->idstr, psize, > + " 0x%016" PRIx64 " 0x%016" PRIx64 " %3s\n", > + block->idstr, psize, > (uint64_t)block->offset, > (uint64_t)block->used_length, > - (uint64_t)block->max_length); > + (uint64_t)block->max_length, > + (uint64_t)block->host, > + block->mr->readonly ? "ro" : "rw"); > + > g_free(psize); > } > > -- > 2.34.1 > -- Peter Xu
Re: [PATCH v5 04/14] vfio/migration: Fix NULL pointer dereference bug
On Tue, 3 Jan 2023 11:13:21 + "Dr. David Alan Gilbert" wrote: > * Avihai Horon (avih...@nvidia.com) wrote: > > As part of its error flow, vfio_vmstate_change() accesses > > MigrationState->to_dst_file without any checks. This can cause a NULL > > pointer dereference if the error flow is taken and > > MigrationState->to_dst_file is not set. > > > > For example, this can happen if VM is started or stopped not during > > migration and vfio_vmstate_change() error flow is taken, as > > MigrationState->to_dst_file is not set at that time. > > > > Fix it by checking that MigrationState->to_dst_file is set before using > > it. > > > > Fixes: 02a7e71b1e5b ("vfio: Add VM state change handler to know state of > > VM") > > Signed-off-by: Avihai Horon > > Reviewed-by: Juan Quintela > > Reviewed-by: Vladimir Sementsov-Ogievskiy > > It might be worth posting this patch separately since it's a simple fix > and should go in sooner. There's no upstream implementation of a v1 migration driver, it's deprecated in the kernel, and only enabled in QEMU via an experimental option. It seems sufficient as done here to make it a separate fix for downstreams that may want to backport separately, but given the remaining v1 status, I don't think I'd bother hurrying it otherwise. Thanks, Alex > > --- > > hw/vfio/migration.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > > index e1413ac90c..09fe7c1de2 100644 > > --- a/hw/vfio/migration.c > > +++ b/hw/vfio/migration.c > > @@ -743,7 +743,9 @@ static void vfio_vmstate_change(void *opaque, bool > > running, RunState state) > > */ > > error_report("%s: Failed to set device state 0x%x", vbasedev->name, > > (migration->device_state & mask) | value); > > -qemu_file_set_error(migrate_get_current()->to_dst_file, ret); > > +if (migrate_get_current()->to_dst_file) { > > +qemu_file_set_error(migrate_get_current()->to_dst_file, ret); > > +} > > } > > vbasedev->migration->vm_running = running; > > trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state), > > -- > > 2.26.3 > >
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
On 1/6/2023 17:28, Peter Maydell wrote: On Fri, 6 Jan 2023 at 15:44, Alex Bennée wrote: Peter Maydell writes: The semihosting API, at least for Arm, has a modeflags string so the guest can say whether it wants to open O_BINARY or not: https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#sys-open-0x01 So we need to plumb that down through the common semihosting code into this function and set O_BINARY accordingly. Otherwise guest code that asks for a text-mode file won't get one. We used to, in fact we still have a remnant of the code where we do: #ifndef O_BINARY #define O_BINARY 0 #endif I presume because the only places it exists in libc is wrapped in stuff like: #if defined (__CYGWIN__) #define O_BINARY _FBINARY So the mapping got removed in a1a2a3e609 (semihosting: Remove GDB_O_BINARY) because GDB knows nothing of this and as far as I can tell neither does Linux whatever ISO C might say about it. Is this a host detail leakage to the guest? Should a semihosting app be caring about what fopen() modes the underlying host supports? At least a default O_BINARY for windows is most likely to DTRT. I think the theory when the semihosting API was originally designed decades ago was basically "when the guest does fopen(...) this should act like it does on the host". So as a bit of portable guest code you would say whether you wanted a binary or a text file, and the effect would be that if you were running on Windows and you output a text file then you'd get \r\n like the user probably expected, and if on Linux you get \n. The gdb remote protocol, on the other hand, assumes "all files are binary", and the gdb source that implements the gdb remote file I/O operations does "always set O_BINARY if it's defined": https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/remote-fileio.c;h=3ff2a65b0ec6c7695f8659690a8f1dce9b5cdf5f;hb=HEAD#l141 So this is kind of an impedance mismatch problem -- the semihosting API wants functionality that the gdb protocol can't give us. But we don't have that mismatch issue if we're directly making host filesystem calls, because there we can set O_BINARY or not as we choose. Alternatively, we could decide that our implementation of semihosting consistently uses \n for the newline character on all hosts, such that guests which try to write text files on Windows hosts get the "wrong" newline type, but OTOH get consistently the same file regardless of host and regardless of whether semihosting is going via gdb or not. But if we want to do that we should at least note in a comment somewhere that that's a behaviour we've chosen, not something that's happened by accident. Given Windows is less unfriendly about dealing with \n-terminated files these days that might not be an unreasonable choice. -- PMM If SYS_OPEN is supposed to call fopen (i didn't actually know that..) then it does make more sense for binary/text mode to be propagated from guest. Qemu's implementation calls open(2) though, which is not correct at all then. Well, as long as qemu does that, there is no posix-compliant way to tell open(2) if it should use binary or text mode, there is no notion of that as far as posix (and most implementations) is concerned. My change then acts as a way to at least have predictable behavior across platforms, but i guess a more correct approach would be to follow actual semi-hosting spec and switch to fopen.
Re: [PATCH v2 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
On 1/6/23 12:46 PM, Chuck Zmudzinski wrote: > On 1/6/23 12:35 PM, David Woodhouse wrote: >> On Wed, 2023-01-04 at 15:44 +0100, Bernhard Beschow wrote: >>> + if (xen_enabled()) { >> >> Could this perhaps be if (xen_mode != XEN_DISABLED) once we merge the >> Xen-on-KVM series? > > I am not an expert and just on here as a user/tester, but I think it > would depend on whether or not the Xen-on-KVM mode uses Xen PCI IRQ > handling or Linux/KVM PCI IRQ handling. > > Chuck I could try Bernhard's patches in a Xen-on-KVM configuration if that might help. I would need help configuring the Xen-on-KVM mode to do it, though, because I have never tried Xen-on-KVM before. The test I could do would be to: 1) Test my Xen guest that uses the current PIIX3-xen device in the Xen-on-KVM environment and get that working. 2) Apply Bernhard's patch series and see what, if anything, needs to be done to make Bernhard's patch work in Xen-on-KVM. I have already verified that Bernhard's patches work well with Xen on bare metal, so I don't think we need to answer this question before going forward with Bernhard's patches. This can be patched later to support Xen-on-KVM if necessary as part of or in addition to the Xen-on-KVM series. Chuck
Re: [PATCH 1/8] hw/pci-host/bonito: Convert to 3-phase reset
On 1/5/23 05:07, Philippe Mathieu-Daudé wrote: From: Philippe Mathieu-Daudé Convert the TYPE_PCI_BONITO class to use 3-phase reset. Signed-off-by: Philippe Mathieu-Daudé --- hw/pci-host/bonito.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 2/8] hw/pci-host/bonito: Use 'bonito_host' for PCI host bridge code
On 1/5/23 05:07, Philippe Mathieu-Daudé wrote: To make it easier to differentiate between the Host Bridge object and its PCI function #0, rename bonito_pcihost* as bonito_host*. Signed-off-by: Philippe Mathieu-Daudé --- hw/pci-host/bonito.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson r~
Re: Re: [PING PATCH 0/1] Fix some typos
On Thu, Jan 5, 2023 at 8:50 PM Dongdong Zhang wrote: > > Hi John, > > Could you help me relay these fixes? > If I submit a pull request, I will go through company's internal review > process again. No problem at all. My apologies for the delay. Staged for QEMU, with additional fix spotted by Max: https://gitlab.com/jsnow/qemu/-/commit/4cd8db9f367e12ccd7d3980f2573fe65f9949562 Staged for qemu.qmp, preserving credit: https://gitlab.com/jsnow/python-qemu-qmp/-/commit/cd190a79bcee975d00b0334003e6dcbaf8ec9630 > > Thanks a lot! > Thank *you* :) --js > Dongdong > > > > -原始邮件-发件人:"John Snow" 发送时间:2023-01-06 07:25:43 > > (星期五)收件人:"Dongdong Zhang" > > 抄送:qemu-devel@nongnu.org, > > cr...@redhat.com, bl...@redhat.com主题:Re: [PING PATCH 0/1] Fix some typos > > > > On Thu, Dec 15, 2022 at 10:22 PM Dongdong Zhang > > wrote: > > > > > > Hi all, > > > > > > I would like to ping a patch > > > > > > https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg04568.html > > > https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg04570.html > > > > > > > > > > -Original Messages-From:"Dongdong Zhang" > > > > Sent Time:2022-11-30 09:53:57 > > > > (Wednesday)To:qemu-devel@nongnu.orgCc:js...@redhat.com, > > > > cr...@redhat.com, bl...@redhat.com, "Dongdong Zhang" > > > > Subject:[PATCH 0/1] Fix some typos > > > > > > > > This patch mainly fixes some typos in the 'python' directory. > > > > > > > > Dongdong Zhang (1): > > > > Fix some typos > > > > > > > > python/qemu/machine/console_socket.py | 2 +- > > > > python/qemu/machine/qtest.py | 2 +- > > > > python/qemu/qmp/protocol.py | 2 +- > > > > python/qemu/qmp/qmp_tui.py| 6 +++--- > > > > 4 files changed, 6 insertions(+), 6 deletions(-) > > > > > > > > -- > > > > 2.17.1 > > > > ACK to this patch. > > > > For fixes under python/qemu/qmp/, I need to relay these fixes over to > > https://gitlab.com/qemu-project/python-qemu-qmp -- you can do it > > yourself and send a small merge request, or I can do it for you, if > > you'd like. Please let me know what you'd prefer, and then I will > > stage this patch. > > > > (Apologies that the code is duplicated in two repositories right > > now I'm working on fixing that.) > > > > --js
Re: [PATCH 3/8] hw/pci-host/bonito: Use 'bonito_pci' for PCI function #0 code
On 1/5/23 05:07, Philippe Mathieu-Daudé wrote: To make it easier to differentiate between the Host Bridge object and its PCI function #0, rename bonito* as bonito_pci*. Signed-off-by: Philippe Mathieu-Daudé --- hw/pci-host/bonito.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 4/8] hw/pci-host/bonito: Set reference using object_property_add_const_link()
On 1/5/23 05:07, Philippe Mathieu-Daudé wrote: A QOM object shouldn't poke at another object internals. Here the PCI host bridge instantiates its PCI function #0 and sets a reference to itself (so the function can access the bridge fields). Pass this reference with object_property_add_const_link(), since the reference won't change during the object lifetime. Signed-off-by: Philippe Mathieu-Daudé --- hw/pci-host/bonito.c | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index 80ec424f86..d881c85509 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -656,10 +656,17 @@ static void bonito_host_realize(DeviceState *dev, Error **errp) static void bonito_pci_realize(PCIDevice *dev, Error **errp) { PCIBonitoState *s = PCI_BONITO(dev); -SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost); -PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost); -BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost); MemoryRegion *pcimem_alias = g_new(MemoryRegion, 1); +SysBusDevice *sysbus; +PCIHostState *phb; +BonitoState *bs; +Object *obj; + +obj = object_property_get_link(OBJECT(dev), "host-bridge", &error_abort); +s->pcihost = BONITO_PCI_HOST_BRIDGE(obj); +sysbus = SYS_BUS_DEVICE(obj); +phb = PCI_HOST_BRIDGE(obj); +bs = BONITO_PCI_HOST_BRIDGE(obj); It would be nice to re-order these so that you only perform the dynamic cast once: s->pcihost = bs; Regardless, Reviewed-by: Richard Henderson r~
Re: [PATCH 5/8] hw/pci-host/bonito: Create PCI function #0 in bridge realize() handler
On 1/5/23 05:07, Philippe Mathieu-Daudé wrote: The PCI function #0 is an integral part of the PCI bridge, instantiate it internally during the bridge creation. Signed-off-by: Philippe Mathieu-Daudé --- hw/pci-host/bonito.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index d881c85509..7722636e9e 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -651,6 +651,11 @@ static void bonito_host_realize(DeviceState *dev, Error **errp) } create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB); + +bs->pci_dev = PCI_BONITO(pci_new(PCI_DEVFN(0, 0), TYPE_PCI_BONITO)); +object_property_add_const_link(OBJECT(bs->pci_dev), "host-bridge", + OBJECT(bs)); +pci_realize_and_unref(PCI_DEVICE(bs->pci_dev), phb->bus, &error_fatal); You can avoid this final dynamic cast by saving the result of pci_new. Otherwise, Reviewed-by: Richard Henderson r~
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility
On Fri, 6 Jan 2023 at 18:22, Evgeny Iakovlev wrote: > > > On 1/6/2023 17:28, Peter Maydell wrote: > > On Fri, 6 Jan 2023 at 15:44, Alex Bennée wrote: > >> Peter Maydell writes: > > I think the theory when the semihosting API was originally designed > > decades ago was basically "when the guest does fopen(...) this > > should act like it does on the host". So as a bit of portable > > guest code you would say whether you wanted a binary or a text > > file, and the effect would be that if you were running on Windows > > and you output a text file then you'd get \r\n like the user > > probably expected, and if on Linux you get \n. > If SYS_OPEN is supposed to call fopen (i didn't actually know that..) > then it does make more sense for binary/text mode to be propagated from > guest. It's not required to literally call fopen(). It just has to give the specified semantics for when the guest passes it a mode integer, which is defined in terms of the ISO C fopen() string semantics for "r", "rb", "r+", "r+b", etc. > Qemu's implementation calls open(2) though, which is not correct > at all then. Well, as long as qemu does that, there is no > posix-compliant way to tell open(2) if it should use binary or text > mode, there is no notion of that as far as posix (and most > implementations) is concerned. QEMU doesn't have to be pure POSIX compliant: we know what our supported host platforms are and we can freely use extensions they provide. If we want to achieve the semantics that semihosting asks for then we can do that with open(), by passing O_BINARY when the mode integer from the guest corresponds to a string with "b" in it. I'm about 50:50 on whether we should do that vs documenting and commenting that we deliberately produce the same behaviour on all platforms by ignoring the 'b' flag, though. thanks -- PMM
Re: [PATCH v2 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
On 1/6/23 7:25 AM, Philippe Mathieu-Daudé wrote: > On 6/1/23 12:57, Bernhard Beschow wrote: >> >> >> Am 4. Januar 2023 15:35:33 UTC schrieb "Philippe Mathieu-Daudé" >> : >>> +Markus/Thomas >>> >>> On 4/1/23 15:44, Bernhard Beschow wrote: During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of TYPE_PIIX3_DEVICE. Remove this redundancy. Signed-off-by: Bernhard Beschow --- hw/i386/pc_piix.c | 4 +--- hw/isa/piix.c | 20 include/hw/southbridge/piix.h | 1 - 3 files changed, 1 insertion(+), 24 deletions(-) > > -static void piix3_xen_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - -k->realize = piix3_realize; -/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */ -k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0; -dc->vmsd = &vmstate_piix3; >>> >>> IIUC, since this device is user-creatable, we can't simply remove it >>> without going thru the deprecation process. >> >> AFAICS this device is actually not user-creatable since dc->user_creatable >> is set to false once in the base class. I think it is safe to remove the Xen >> class unless there are ABI issues. > Great news! I don't know if this means the device is user-creatable: chuckz@bullseye:~$ qemu-system-i386 -device piix3-ide-xen,help piix3-ide-xen options: addr= - Slot and optional function number, example: 06.0 or 06 (default: -1) failover_pair_id= multifunction= - on/off (default: false) rombar=- (default: 1) romfile= x-pcie-extcap-init= - on/off (default: true) x-pcie-lnksta-dllla= - on/off (default: true) Today I am running qemu-5.2 on Debian 11, so this output is for qemu 5.2, and that version of qemu has a piix3-ide-xen device. Is that this same device that is being removed? If so, it seems to me that at least as of qemu 5.2, the device was user-creatable. Chuck
Re: [RFC PATCH 00/40] Toward class init of cpu features
On Tue, 3 Jan 2023 at 18:27, Richard Henderson wrote: > The specific problem I'm trying to solve is the location and > representation of the coprocessor register hash table for ARM cpus, > but in the process affects how cpu initialization might be done for > all targets. > > At present, each cpu (for all targets) is initialized separately. > For some, like ARM, we apply QOM properties and then build a hash > table of all coprocessor regs that are valid for the cpu. For others, > like m68k and ppc, we build tables of all valid instructions > (ppc is slowly moving away from constructed tables to decodetree, > but the migration is not complete). > > Importantly, this happens N times for smp system emulation, for each > cpu instance, meaning we do N times the work on startup as necessary. > For system emulation this isn't so bad, as N is generally small-ish, > but it certainly could be improved. > > More importantly, this happens for each thread in user-only emulation. > This is much more significant because threads can be short-lived, and > there can be many hundreds of them, each one performing what amounts > to redundant initialization. > > The "obvious" solution is to make better use of the cpu class object. > Construct data structures once to be shared with all instances. The trouble with this idea is that not all instances of the same class are actually necessarily the same. For instance, if you have a system with both (a) a Cortex-A53 with a PMU, and (b) a Cortex-A53 without a PMU, then they're both instances of the same class, but they shouldn't be sharing the coprocessor register hashtable because they don't have an identical set of system registers. This kind of same-CPU-type-heterogenous-configuration system is not something we're currently using on A-profile, but we do have it for M-profile (the sse200 has a dual-core setup where only one of the CPUs has an FPU), so it's not totally outlandish. thanks -- PMM