Re: [PATCH] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job

2023-01-06 Thread Stefan Weil via

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

2023-01-06 Thread Thomas Huth

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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
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()

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
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()

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Thomas Huth
 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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Lei Wang
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Lei Wang
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()

2023-01-06 Thread Lei Wang
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

2023-01-06 Thread Lei Wang
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Lei Wang
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Andrew Jones
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

2023-01-06 Thread Lei Wang
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

2023-01-06 Thread Thomas Huth
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

2023-01-06 Thread Lei Wang
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

2023-01-06 Thread Philippe Mathieu-Daudé

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

2023-01-06 Thread Bin Meng
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

2023-01-06 Thread Chao Peng
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

2023-01-06 Thread Bin Meng
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?

2023-01-06 Thread Bin Meng
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

2023-01-06 Thread Evgeny Iakovlev
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

2023-01-06 Thread Evgeny Iakovlev



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

2023-01-06 Thread Alex Bennée


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

2023-01-06 Thread Evgeny Iakovlev



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

2023-01-06 Thread Klaus Jensen
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

2023-01-06 Thread Alex Bennée


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

2023-01-06 Thread Anthony PERARD via
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

2023-01-06 Thread Stefan Weil via

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

2023-01-06 Thread Bernhard Beschow
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

2023-01-06 Thread Philippe Mathieu-Daudé

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

2023-01-06 Thread Bernhard Beschow



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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Philippe Mathieu-Daudé

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

2023-01-06 Thread Michael S. Tsirkin
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

2023-01-06 Thread Eric Auger
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

2023-01-06 Thread Alex Bennée
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Alex Bennée


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

2023-01-06 Thread Alex Bennée


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

2023-01-06 Thread Alex Bennée


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

2023-01-06 Thread Stefan Berger




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

2023-01-06 Thread Chuck Zmudzinski
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

2023-01-06 Thread Anthony PERARD via
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Chuck Zmudzinski
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Laurent Vivier

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

2023-01-06 Thread Anthony PERARD via
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

2023-01-06 Thread Chuck Zmudzinski
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

2023-01-06 Thread Benjamin Ellis
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

2023-01-06 Thread Anthony PERARD via
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

2023-01-06 Thread Chuck Zmudzinski
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

2023-01-06 Thread Stefan Berger




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

2023-01-06 Thread Alex Bennée
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread BALATON Zoltan

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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Alex Bennée


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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Stefan Berger




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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Andrew Jones
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Volker Rümelin

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

2023-01-06 Thread Evgeny Iakovlev
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

2023-01-06 Thread Evgeny Iakovlev
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

2023-01-06 Thread Evgeny Iakovlev
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

2023-01-06 Thread David Woodhouse
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

2023-01-06 Thread Chuck Zmudzinski
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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Alex Bennée


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()

2023-01-06 Thread Alex Williamson
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

2023-01-06 Thread Peter Xu
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

2023-01-06 Thread Alex Williamson
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

2023-01-06 Thread Evgeny Iakovlev



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

2023-01-06 Thread Chuck Zmudzinski
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

2023-01-06 Thread Richard Henderson

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

2023-01-06 Thread Richard Henderson

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

2023-01-06 Thread John Snow
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

2023-01-06 Thread Richard Henderson

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()

2023-01-06 Thread Richard Henderson

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

2023-01-06 Thread Richard Henderson

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

2023-01-06 Thread Peter Maydell
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

2023-01-06 Thread Chuck Zmudzinski
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

2023-01-06 Thread Peter Maydell
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



  1   2   >