Re: [PATCH 3/3] vdpa: Expose VIRTIO_NET_F_STATUS unconditionally
On Thu, Nov 3, 2022 at 4:12 PM Eugenio Perez Martin wrote:
>
> On Thu, Nov 3, 2022 at 4:21 AM Jason Wang wrote:
> >
> > On Wed, Nov 2, 2022 at 7:19 PM Eugenio Perez Martin
> > wrote:
> > >
> > > On Tue, Nov 1, 2022 at 9:10 AM Jason Wang wrote:
> > > >
> > > > On Fri, Oct 28, 2022 at 5:30 PM Eugenio Perez Martin
> > > > wrote:
> > > > >
> > > > > On Fri, Oct 28, 2022 at 3:59 AM Jason Wang
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Oct 27, 2022 at 6:18 PM Eugenio Perez Martin
> > > > > > wrote:
> > > > > > >
> > > > > > > On Thu, Oct 27, 2022 at 8:54 AM Jason Wang
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 27, 2022 at 2:47 PM Eugenio Perez Martin
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Oct 27, 2022 at 6:32 AM Jason Wang
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > 在 2022/10/26 17:53, Eugenio Pérez 写道:
> > > > > > > > > > > Now that qemu can handle and emulate it if the vdpa
> > > > > > > > > > > backend does not
> > > > > > > > > > > support it we can offer it always.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Eugenio Pérez
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I may miss something but isn't more easier to simply remove
> > > > > > > > > > the
> > > > > > > > > > _F_STATUS from vdpa_feature_bits[]?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > How is that? if we remove it, the guest cannot ack it so it
> > > > > > > > > cannot
> > > > > > > > > access the net status, isn't it?
> > > > > > > >
> > > > > > > > My understanding is that the bits stored in the
> > > > > > > > vdpa_feature_bits[]
> > > > > > > > are the features that must be explicitly supported by the vhost
> > > > > > > > device.
> > > > > > >
> > > > > > > (Non English native here, so maybe I don't get what you mean :) )
> > > > > > > The
> > > > > > > device may not support them. net simulator lacks some of them
> > > > > > > actually, and it works.
> > > > > >
> > > > > > Speaking too fast, I think I meant that, if the bit doesn't belong
> > > > > > to
> > > > > > vdpa_feature_bits[], it is assumed to be supported by the Qemu
> > > > > > without
> > > > > > the support of the vhost. So Qemu won't even try to validate if
> > > > > > vhost
> > > > > > has this support. E.g for vhost-net, we only have:
> > > > > >
> > > > > > static const int kernel_feature_bits[] = {
> > > > > > VIRTIO_F_NOTIFY_ON_EMPTY,
> > > > > > VIRTIO_RING_F_INDIRECT_DESC,
> > > > > > VIRTIO_RING_F_EVENT_IDX,
> > > > > > VIRTIO_NET_F_MRG_RXBUF,
> > > > > > VIRTIO_F_VERSION_1,
> > > > > > VIRTIO_NET_F_MTU,
> > > > > > VIRTIO_F_IOMMU_PLATFORM,
> > > > > > VIRTIO_F_RING_PACKED,
> > > > > > VIRTIO_NET_F_HASH_REPORT,
> > > > > > VHOST_INVALID_FEATURE_BIT
> > > > > > };
> > > > > >
> > > > > > You can see there's no STATUS bit there since it is emulated by
> > > > > > Qemu.
> > > > > >
> > > > >
> > > > > Ok now I get what you mean, and yes we may modify the patches in that
> > > > > direction.
> > > > >
> > > > > But if we go then we need to modify how qemu ack the features, because
> > > > > the features that are not in vdpa_feature_bits are not acked to the
> > > > > device. More on this later.
> > > > >
> > > > > > >
> > > > > > > From what I see these are the only features that will be
> > > > > > > forwarded to
> > > > > > > the guest as device_features. If it is not in the list, the
> > > > > > > feature
> > > > > > > will be masked out,
> > > > > >
> > > > > > Only when there's no support for this feature from the vhost.
> > > > > >
> > > > > > > as if the device does not support it.
> > > > > > >
> > > > > > > So now _F_STATUS it was forwarded only if the device supports it.
> > > > > > > If
> > > > > > > we remove it from bit_mask, it will never be offered to the
> > > > > > > guest. But
> > > > > > > we want to offer it always, since we will need it for
> > > > > > > _F_GUEST_ANNOUNCE.
> > > > > > >
> > > > > > > Things get more complex because we actually need to ack it back
> > > > > > > if the
> > > > > > > device offers it, so the vdpa device can report link_down. We will
> > > > > > > only emulate LINK_UP always in the case the device does not
> > > > > > > support
> > > > > > > _F_STATUS.
> > > > > > >
> > > > > > > > So if we remove _F_STATUS, Qemu vhost code won't validate if
> > > > > > > > vhost-vdpa device has this support:
> > > > > > > >
> > > > > > > > uint64_t vhost_get_features(struct vhost_dev *hdev, const int
> > > > > > > > *feature_bits,
> > > > > > > > uint64_t features)
> > > > > > > > {
> > > > > > > > const int *bit = feature_bits;
> > > > > > > > while (*bit != VHOST_INVALID_FEATURE_BIT) {
> > > > > > > > uint64_t bit_mask = (1ULL << *bit);
> > > > > > > > if (!(hdev->features & bit_mask)) {
> > > > > > > > features &= ~bit_mask;
> > > > > > > > }
> > > > > > >
Re: [PATCH] tests/qtest/libqos/e1000e: Use e1000_regs.h
On Tue, Oct 25, 2022 at 10:54 PM Thomas Huth wrote: > > On 13/10/2022 07.52, Akihiko Odaki wrote: > > The register definitions in tests/qtest/libqos/e1000e.c had names > > different from hw/net/e1000_regs.h, which made it hard to understand > > what test codes corresponds to the implementation. Use > > hw/net/e1000_regs.h from tests/qtest/libqos/e1000e.c to remove > > these duplications. > > > > E1000E_CTRL_EXT_TXLSFLOW is removed from E1000E_CTRL_EXT settings > > because hw/net/e1000_regs.h does not have the definition and it is for > > TCP segmentation offload, which does not matter for the implemented > > tests. > > > > Signed-off-by: Akihiko Odaki > > --- > > hw/net/e1000_regs.h | 1 + > > tests/qtest/libqos/e1000e.c | 119 +--- > > 2 files changed, 45 insertions(+), 75 deletions(-) > > Acked-by: Thomas Huth > > I can take it through my testing-next tree: Acked-by: Jason Wang > > https://gitlab.com/thuth/qemu/-/commits/testing-next > > Thomas >
Re: [PULL v3 49/81] acpi: pc: vga: use AcpiDevAmlIf interface to build VGA device descriptors
On Mon, Nov 7, 2022 at 3:09 AM Bernhard Beschow wrote: > > > > On Sun, Nov 6, 2022 at 10:16 PM Bernhard Beschow wrote: >> >> >> >> On Sat, Nov 5, 2022 at 6:45 PM Michael S. Tsirkin wrote: >>> >>> From: Igor Mammedov >>> >>> Signed-off-by: Igor Mammedov >>> Message-Id: <20221017102146.2254096-2-imamm...@redhat.com> >>> Reviewed-by: Michael S. Tsirkin >>> Signed-off-by: Michael S. Tsirkin >>> NB: we do not expect any functional change in >>> any ACPI tables with this change. It's only a refactoring. >>> >>> Reviewed-by: Ani Sinha >>> --- >>> hw/display/vga_int.h | 2 ++ >>> hw/display/acpi-vga-stub.c | 7 +++ >>> hw/display/acpi-vga.c | 26 ++ >>> hw/display/vga-pci.c | 4 >>> hw/i386/acpi-build.c | 26 +- >>> hw/display/meson.build | 17 + >>> 6 files changed, 57 insertions(+), 25 deletions(-) >>> create mode 100644 hw/display/acpi-vga-stub.c >>> create mode 100644 hw/display/acpi-vga.c >> >> >> With this "qemu:qtest+qtest-hppa / qtest-hppa/display-vga-test" fails due to >> the symbol "aml_return" being undefined: >> >> # starting QEMU: exec ./qemu-system-hppa -qtest unix:/tmp/qtest-515650.sock >> -qtest-log /dev/null -chardev socket,path=/tmp/qtest-515650.qmp,id=char0 >> -mon chardev=char0,mode=control -display none -vga none -device virtio-vga >> -accel qtest >> --- stderr >> --- >> Failed to open module: >> qemu/build/qemu-bundle/usr/lib/qemu/hw-display-virtio-vga.so: undefined >> symbol: aml_return >> qemu-system-hppa: -device virtio-vga: 'virtio-vga' is not a valid device >> model name >> Broken pipe >> ../src/tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU >> process but encountered exit status 1 (expected 0) >> >> (test program exited with status code -6) > > > It doesn't only affect hppa: > > grep -e "undefined symbol: aml_return" meson-logs/testlog.txt | wc -l > 139 > Hmm. I see it here too: https://gitlab.com/qemu-project/qemu/-/jobs/3281425457 >>> QTEST_QEMU_IMG=./qemu-img MALLOC_PERTURB_=60 >>> G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon.sh >>> QTEST_QEMU_BINARY=./qemu-system-or1k >>> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon >>> /builds/qemu-project/qemu/build/tests/qtest/device-introspect-test --tap -k 219― ✀ ― 220stderr: 221failed to open module: /builds/qemu-project/qemu/build/qemu-bundle/usr/local/lib64/qemu/hw-display-virtio-vga.so: undefined symbol: aml_return 222qemu-system-or1k: ../util/error.c:59: error_setv: Assertion `*errp == NULL' failed. 223Broken pipe 224../tests/qtest/libqtest.c:188: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped) 225TAP parsing error: Too few tests run (expected 6, got 0) 226(test program exited with status code -6) 227―― 228154/274 qemu:qtest+qtest-or1k / qtest-or1k/machine-none-test OK 0.05s 1 subtests passed 229155/274 qemu:qtest+qtest-or1k / qtest-or1k/qmp-test OK 0.19s 4 subtests passed 230156/274 qemu:qtest+qtest-or1k / qtest-or1k/qmp-cmd-test ERROR 1.72s killed by signal 6 SIGABRT 231>>> QTEST_QEMU_IMG=./qemu-img G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_BINARY=./qemu-system-or1k QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon MALLOC_PERTURB_=53 /builds/qemu-project/qemu/build/tests/qtest/qmp-cmd-test --tap -k 232― ✀ ― 233stderr: 234failed to open module: /builds/qemu-project/qemu/build/qemu-bundle/usr/local/lib64/qemu/hw-display-virtio-vga.so: undefined symbol: aml_return 235qemu-system-or1k: ../util/error.c:59: error_setv: Assertion `*errp == NULL' failed. 236Broken pipe 237../tests/qtest/libqtest.c:188: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped) 238TAP parsing error: Too few tests run (expected 62, got 31) 239(test program exited with status code -6) 240
Re: [PATCH v2] tests/qtest: netdev: test stream and dgram backends
On Mon, Nov 7, 2022 at 2:59 PM Jason Wang wrote:
>
> On Fri, Nov 4, 2022 at 11:01 PM Laurent Vivier wrote:
> >
> > Signed-off-by: Laurent Vivier
> > Acked-by: Michael S. Tsirkin
> > ---
> >
> > Notes:
> > v2:
> > - Fix ipv6 free port allocation
> > - Check for IPv4, IPv6, AF_UNIX
> > - Use g_mkdtemp() rather than g_file_open_tmp()
> > - Use socketpair() in test_stream_fd()
> >
> > v1: compared to v14 of "qapi: net: add unix socket type support to
> > netdev backend":
> > - use IP addresses 127.0.0.1 and ::1 rather than localhost
> >
> > tests/qtest/meson.build | 2 +
> > tests/qtest/netdev-socket.c | 435
> > 2 files changed, 437 insertions(+)
> > create mode 100644 tests/qtest/netdev-socket.c
> >
> > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> > index c07a5b1a5f43..43d075b76280 100644
> > --- a/tests/qtest/meson.build
> > +++ b/tests/qtest/meson.build
> > @@ -27,6 +27,7 @@ qtests_generic = [
> >'test-hmp',
> >'qos-test',
> >'readconfig-test',
> > + 'netdev-socket',
> > ]
> > if config_host.has_key('CONFIG_MODULES')
> >qtests_generic += [ 'modules-test' ]
> > @@ -304,6 +305,7 @@ qtests = {
> >'tpm-tis-device-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
> >'tpm-tis-device-test': [io, tpmemu_files, 'tpm-tis-util.c'],
> >'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
> > + 'netdev-socket': files('netdev-socket.c', '../unit/socket-helpers.c'),
> > }
> >
> > gvnc = dependency('gvnc-1.0', required: false)
> > diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
> > new file mode 100644
> > index ..dd46214f69c7
> > --- /dev/null
> > +++ b/tests/qtest/netdev-socket.c
> > @@ -0,0 +1,435 @@
> > +/*
> > + * QTest testcase for netdev stream and dgram
> > + *
> > + * Copyright (c) 2022 Red Hat, Inc.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include
> > +#include "../unit/socket-helpers.h"
> > +#include "libqtest.h"
> > +
> > +#define CONNECTION_TIMEOUT5
> > +
> > +#define EXPECT_STATE(q, e, t) \
> > +do { \
> > +char *resp = qtest_hmp(q, "info network");\
> > +if (t) { \
> > +strrchr(resp, t)[0] = 0; \
> > +} \
> > +g_test_timer_start(); \
> > +while (g_test_timer_elapsed() < CONNECTION_TIMEOUT) { \
> > +if (strcmp(resp, e) == 0) { \
> > +break;\
> > +} \
> > +g_free(resp); \
> > +resp = qtest_hmp(q, "info network"); \
> > +if (t) { \
> > +strrchr(resp, t)[0] = 0; \
> > +} \
> > +} \
> > +g_assert_cmpstr(resp, ==, e); \
> > +g_free(resp); \
> > +} while (0)
> > +
> > +static char *tmpdir;
> > +
> > +static int inet_get_free_port_socket_ipv4(int sock)
> > +{
> > +struct sockaddr_in addr;
> > +socklen_t len;
> > +
> > +memset(, 0, sizeof(addr));
> > +addr.sin_family = AF_INET;
> > +addr.sin_addr.s_addr = INADDR_ANY;
> > +addr.sin_port = 0;
> > +if (bind(sock, (struct sockaddr *), sizeof(addr)) < 0) {
> > +return -1;
> > +}
> > +
> > +len = sizeof(addr);
> > +if (getsockname(sock, (struct sockaddr *), ) < 0) {
> > +return -1;
> > +}
> > +
> > +return ntohs(addr.sin_port);
> > +}
> > +
> > +static int inet_get_free_port_socket_ipv6(int sock)
> > +{
> > +struct sockaddr_in6 addr;
> > +socklen_t len;
> > +
> > +memset(, 0, sizeof(addr));
> > +addr.sin6_family = AF_INET6;
> > +addr.sin6_addr = in6addr_any;
> > +addr.sin6_port = 0;
> > +if (bind(sock, (struct sockaddr *), sizeof(addr)) < 0) {
> > +return -1;
> > +}
> > +
> > +len = sizeof(addr);
> > +if (getsockname(sock, (struct sockaddr *), ) < 0) {
> > +return -1;
> > +}
> > +
> > +return ntohs(addr.sin6_port);
> > +}
> > +
> > +static int inet_get_free_port_multiple(int nb, int *port, bool ipv6)
> > +{
> > +int sock[nb];
> > +int i;
> > +
> > +for (i = 0; i < nb; i++) {
> > +sock[i] = socket(ipv6 ? AF_INET6 : AF_INET, SOCK_STREAM, 0);
> > +if (sock[i] < 0) {
> > +break;
> > +}
> > +port[i] = ipv6 ? inet_get_free_port_socket_ipv6(sock[i]) :
> > + inet_get_free_port_socket_ipv4(sock[i]);
> > +
Re: [PATCH V5 2/4] intel-iommu: drop VTDBus
On Sun, Nov 6, 2022 at 1:37 AM Michael S. Tsirkin wrote: > > On Fri, Oct 28, 2022 at 02:14:34PM +0800, Jason Wang wrote: > > > > -GHashTable *vtd_as_by_busptr; /* VTDBus objects indexed by PCIBus* > > reference */ > > -VTDBus *vtd_as_by_bus_num[VTD_PCI_BUS_MAX]; /* VTDBus objects indexed > > by bus number */ > > +GHashTable *vtd_address_spaces; /* VTD address spaces */ > > +VTDAddressSpace *vtd_as_cache[VTD_PCI_BUS_MAX]; /* VTD address space > > cache */ > > /* list of registered notifiers */ > > QLIST_HEAD(, VTDAddressSpace) vtd_as_with_notifiers; > > > BTW this triggers a bunch of checkpatch errors. Pls fix up with > a follow-up patch. Thanks! > Ok. Thanks > > -- > > 2.25.1 >
Re: [PATCH v2] tests/qtest: netdev: test stream and dgram backends
On Fri, Nov 4, 2022 at 11:01 PM Laurent Vivier wrote:
>
> Signed-off-by: Laurent Vivier
> Acked-by: Michael S. Tsirkin
> ---
>
> Notes:
> v2:
> - Fix ipv6 free port allocation
> - Check for IPv4, IPv6, AF_UNIX
> - Use g_mkdtemp() rather than g_file_open_tmp()
> - Use socketpair() in test_stream_fd()
>
> v1: compared to v14 of "qapi: net: add unix socket type support to netdev
> backend":
> - use IP addresses 127.0.0.1 and ::1 rather than localhost
>
> tests/qtest/meson.build | 2 +
> tests/qtest/netdev-socket.c | 435
> 2 files changed, 437 insertions(+)
> create mode 100644 tests/qtest/netdev-socket.c
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index c07a5b1a5f43..43d075b76280 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -27,6 +27,7 @@ qtests_generic = [
>'test-hmp',
>'qos-test',
>'readconfig-test',
> + 'netdev-socket',
> ]
> if config_host.has_key('CONFIG_MODULES')
>qtests_generic += [ 'modules-test' ]
> @@ -304,6 +305,7 @@ qtests = {
>'tpm-tis-device-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
>'tpm-tis-device-test': [io, tpmemu_files, 'tpm-tis-util.c'],
>'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
> + 'netdev-socket': files('netdev-socket.c', '../unit/socket-helpers.c'),
> }
>
> gvnc = dependency('gvnc-1.0', required: false)
> diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
> new file mode 100644
> index ..dd46214f69c7
> --- /dev/null
> +++ b/tests/qtest/netdev-socket.c
> @@ -0,0 +1,435 @@
> +/*
> + * QTest testcase for netdev stream and dgram
> + *
> + * Copyright (c) 2022 Red Hat, Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include
> +#include "../unit/socket-helpers.h"
> +#include "libqtest.h"
> +
> +#define CONNECTION_TIMEOUT5
> +
> +#define EXPECT_STATE(q, e, t) \
> +do { \
> +char *resp = qtest_hmp(q, "info network");\
> +if (t) { \
> +strrchr(resp, t)[0] = 0; \
> +} \
> +g_test_timer_start(); \
> +while (g_test_timer_elapsed() < CONNECTION_TIMEOUT) { \
> +if (strcmp(resp, e) == 0) { \
> +break;\
> +} \
> +g_free(resp); \
> +resp = qtest_hmp(q, "info network"); \
> +if (t) { \
> +strrchr(resp, t)[0] = 0; \
> +} \
> +} \
> +g_assert_cmpstr(resp, ==, e); \
> +g_free(resp); \
> +} while (0)
> +
> +static char *tmpdir;
> +
> +static int inet_get_free_port_socket_ipv4(int sock)
> +{
> +struct sockaddr_in addr;
> +socklen_t len;
> +
> +memset(, 0, sizeof(addr));
> +addr.sin_family = AF_INET;
> +addr.sin_addr.s_addr = INADDR_ANY;
> +addr.sin_port = 0;
> +if (bind(sock, (struct sockaddr *), sizeof(addr)) < 0) {
> +return -1;
> +}
> +
> +len = sizeof(addr);
> +if (getsockname(sock, (struct sockaddr *), ) < 0) {
> +return -1;
> +}
> +
> +return ntohs(addr.sin_port);
> +}
> +
> +static int inet_get_free_port_socket_ipv6(int sock)
> +{
> +struct sockaddr_in6 addr;
> +socklen_t len;
> +
> +memset(, 0, sizeof(addr));
> +addr.sin6_family = AF_INET6;
> +addr.sin6_addr = in6addr_any;
> +addr.sin6_port = 0;
> +if (bind(sock, (struct sockaddr *), sizeof(addr)) < 0) {
> +return -1;
> +}
> +
> +len = sizeof(addr);
> +if (getsockname(sock, (struct sockaddr *), ) < 0) {
> +return -1;
> +}
> +
> +return ntohs(addr.sin6_port);
> +}
> +
> +static int inet_get_free_port_multiple(int nb, int *port, bool ipv6)
> +{
> +int sock[nb];
> +int i;
> +
> +for (i = 0; i < nb; i++) {
> +sock[i] = socket(ipv6 ? AF_INET6 : AF_INET, SOCK_STREAM, 0);
> +if (sock[i] < 0) {
> +break;
> +}
> +port[i] = ipv6 ? inet_get_free_port_socket_ipv6(sock[i]) :
> + inet_get_free_port_socket_ipv4(sock[i]);
> +if (port[i] == -1) {
> +break;
> +}
> +}
> +
> +nb = i;
> +for (i = 0; i < nb; i++) {
> +closesocket(sock[i]);
> +}
> +
> +return nb;
> +}
> +
> +static int inet_get_free_port(bool ipv6)
> +{
> +int nb, port;
> +
> +nb = inet_get_free_port_multiple(1, , ipv6);
> +
Re: Intermittent hang on x86 replay avocado test?
On 04.11.2022 21:53, Peter Maydell wrote: On my machine this avocado test: ./build/all/tests/venv/bin/avocado run ./build/all/tests/avocado/replay_kernel.py:ReplayKernelNormal.test_x86_64_pc seems to hang intermittently (maybe 1 time in 3?). Does anybody else see this? Looking at the avocado logs suggests the record part runs fine but the replay part hangs very early in the kernel bootup. (Or possibly Avocado has got confused and isn't logging all the output. > I couldn't trigger it outside avocado. I sometimes have the same problem with one of the replay tests (I don't remember which one). It hangs with avocado, but does not hang when I run it with the same command line without avocado. It could be some replay issue (like infinite waiting for input in main_loop_wait), but I couldn't trigger this behavior with logging/debugging enabled. Pavel Dovgalyuk
Re: [PATCH] hw/riscv: virt: Remove size restriction for pflash
On Sun, Nov 06, 2022 at 10:20:57PM +0300, Mike Maslenkin wrote: > Hello Sunil! > > What about virt_machine_done() function? > kernel_entry variable still points to the second flash started from > virt_memmap[VIRT_FLASH].size / 2. > The base address of the flash has not changed to keep things flexible. So, I didn't change this portion of the code to keep the changes minimal. Thanks Sunil
[PATCH v4 1/2] vhost: Change the sequence of device start
This patch is part of adding vhost-user vhost_dev_start support. The
motivation is to improve backend configuration speed and reduce live
migration VM downtime.
Moving the device start routines after finishing all the necessary device
and VQ configuration, further aligning to the virtio specification for
"device initialization sequence".
Following patch will add vhost-user vhost_dev_start support.
Signed-off-by: Yajun Wu
Acked-by: Parav Pandit
---
hw/block/vhost-user-blk.c | 18 +++---
hw/net/vhost_net.c| 14 --
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 13bf5cc47a..28409c90f7 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -168,13 +168,6 @@ static int vhost_user_blk_start(VirtIODevice *vdev, Error
**errp)
goto err_guest_notifiers;
}
-ret = vhost_dev_start(>dev, vdev);
-if (ret < 0) {
-error_setg_errno(errp, -ret, "Error starting vhost");
-goto err_guest_notifiers;
-}
-s->started_vu = true;
-
/* guest_notifier_mask/pending not used yet, so just unmask
* everything here. virtio-pci will do the right thing by
* enabling/disabling irqfd.
@@ -183,9 +176,20 @@ static int vhost_user_blk_start(VirtIODevice *vdev, Error
**errp)
vhost_virtqueue_mask(>dev, vdev, i, false);
}
+s->dev.vq_index_end = s->dev.nvqs;
+ret = vhost_dev_start(>dev, vdev);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "Error starting vhost");
+goto err_guest_notifiers;
+}
+s->started_vu = true;
+
return ret;
err_guest_notifiers:
+for (i = 0; i < s->dev.nvqs; i++) {
+vhost_virtqueue_mask(>dev, vdev, i, true);
+}
k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
err_host_notifiers:
vhost_dev_disable_notifiers(>dev, vdev);
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index d28f8b974b..0fe71ed309 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -387,21 +387,23 @@ int vhost_net_start(VirtIODevice *dev, NetClientState
*ncs,
} else {
peer = qemu_get_peer(ncs, n->max_queue_pairs);
}
-r = vhost_net_start_one(get_vhost_net(peer), dev);
-
-if (r < 0) {
-goto err_start;
-}
if (peer->vring_enable) {
/* restore vring enable state */
r = vhost_set_vring_enable(peer, peer->vring_enable);
if (r < 0) {
-vhost_net_stop_one(get_vhost_net(peer), dev);
goto err_start;
}
}
+
+r = vhost_net_start_one(get_vhost_net(peer), dev);
+if (r < 0) {
+if (peer->vring_enable) {
+vhost_set_vring_enable(peer, false);
+}
+goto err_start;
+}
}
return 0;
--
2.27.0
[PATCH v4 0/2] vhost-user: Support vhost_dev_start
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. Another change is to move the device start routines after finishing all the necessary device and VQ configuration, further aligning to the virtio specification for "device initialization sequence". [1] https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#introduction v4: - fix vhost_net_start_one fallback code v3: - rebase v2: - add setting status bit VIRTIO_CONFIG_S_FEATURES_OK - avoid adding status bits already set Yajun Wu (2): vhost: Change the sequence of device start vhost-user: Support vhost_dev_start hw/block/vhost-user-blk.c | 18 ++ hw/net/vhost_net.c| 14 hw/virtio/vhost-user.c| 74 ++- 3 files changed, 92 insertions(+), 14 deletions(-) -- 2.27.0
[PATCH v4 2/2] vhost-user: Support vhost_dev_start
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
---
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 03415b6c95..bb5164b753 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, );
+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, );
+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)) {
+if (!ret) {
+return vhost_user_add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+}
+}
+
+return ret;
}
static int vhost_user_set_protocol_features(struct vhost_dev *dev,
@@ -2615,6 +2665,27 @@ void vhost_user_cleanup(VhostUserState *user)
user->chr = NULL;
}
+static int vhost_user_dev_start(struct vhost_dev *dev, bool started)
+{
+if (!virtio_has_feature(dev->protocol_features,
+VHOST_USER_PROTOCOL_F_STATUS)) {
+return 0;
+}
+
+/* Set device status only for last queue pair */
+if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
+return 0;
+}
+
+if (started) {
+return vhost_user_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
+ VIRTIO_CONFIG_S_DRIVER |
+ VIRTIO_CONFIG_S_DRIVER_OK);
+} else {
+return vhost_user_set_status(dev, 0);
+}
+}
+
const VhostOps user_ops = {
.backend_type = VHOST_BACKEND_TYPE_USER,
.vhost_backend_init = vhost_user_backend_init,
@@ -2649,4 +2720,5 @@ const VhostOps user_ops = {
.vhost_backend_mem_section_filter = vhost_user_mem_section_filter,
[PULL 2/2] target/loongarch: Fix return value of CHECK_FPE
From: Rui Wang
Regarding the patchset v3 has been merged into main line, and not
approved, this patch updates to patchset v4.
Fixes: 2419978c ("target/loongarch: Fix emulation of float-point disable
exception")
Link: https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg00808.html
Reviewed-by: Richard Henderson
Signed-off-by: Rui Wang
Message-Id: <20221107024526.702297-3-wang...@loongson.cn>
Signed-off-by: Song Gao
---
target/loongarch/insn_trans/trans_farith.c.inc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/loongarch/insn_trans/trans_farith.c.inc
b/target/loongarch/insn_trans/trans_farith.c.inc
index e2dec75dfb..7081fbb89b 100644
--- a/target/loongarch/insn_trans/trans_farith.c.inc
+++ b/target/loongarch/insn_trans/trans_farith.c.inc
@@ -7,7 +7,7 @@
#define CHECK_FPE do { \
if ((ctx->base.tb->flags & HW_FLAGS_EUEN_FPE) == 0) { \
generate_exception(ctx, EXCCODE_FPD); \
-return false; \
+return true; \
} \
} while (0)
#else
--
2.31.1
[PULL 1/2] target/loongarch: Separate the hardware flags into MMU index and PLV
From: Rui Wang
Regarding the patchset v3 has been merged into main line, and not
approved, this patch updates to patchset v4.
Fixes: b4bda200 ("target/loongarch: Adjust the layout of hardware flags bit
fields")
Link: https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg00808.html
Reviewed-by: Richard Henderson
Signed-off-by: Rui Wang
Message-Id: <20221107024526.702297-2-wang...@loongson.cn>
Signed-off-by: Song Gao
---
target/loongarch/cpu.h | 18 +-
.../insn_trans/trans_privileged.c.inc | 4 ++--
target/loongarch/tlb_helper.c | 4 ++--
target/loongarch/translate.c | 5 +++--
target/loongarch/translate.h | 3 ++-
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 08c1f6baa1..e15c633b0b 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -374,21 +374,21 @@ struct LoongArchCPUClass {
* 0 for kernel mode, 3 for user mode.
* Define an extra index for DA(direct addressing) mode.
*/
-#define MMU_KERNEL_IDX 0
-#define MMU_USER_IDX 3
-#define MMU_DA_IDX 4
+#define MMU_PLV_KERNEL 0
+#define MMU_PLV_USER 3
+#define MMU_IDX_KERNEL MMU_PLV_KERNEL
+#define MMU_IDX_USER MMU_PLV_USER
+#define MMU_IDX_DA 4
static inline int cpu_mmu_index(CPULoongArchState *env, bool ifetch)
{
#ifdef CONFIG_USER_ONLY
-return MMU_USER_IDX;
+return MMU_IDX_USER;
#else
-uint8_t pg = FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PG);
-
-if (!pg) {
-return MMU_DA_IDX;
+if (FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PG)) {
+return FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV);
}
-return FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV);
+return MMU_IDX_DA;
#endif
}
diff --git a/target/loongarch/insn_trans/trans_privileged.c.inc
b/target/loongarch/insn_trans/trans_privileged.c.inc
index ff3a6d95ae..40f82becb0 100644
--- a/target/loongarch/insn_trans/trans_privileged.c.inc
+++ b/target/loongarch/insn_trans/trans_privileged.c.inc
@@ -159,7 +159,7 @@ static const CSRInfo csr_info[] = {
static bool check_plv(DisasContext *ctx)
{
-if (ctx->mem_idx == MMU_USER_IDX) {
+if (ctx->plv == MMU_PLV_USER) {
generate_exception(ctx, EXCCODE_IPE);
return true;
}
@@ -335,7 +335,7 @@ TRANS(iocsrwr_d, gen_iocsrwr, gen_helper_iocsrwr_d)
static void check_mmu_idx(DisasContext *ctx)
{
-if (ctx->mem_idx != MMU_DA_IDX) {
+if (ctx->mem_idx != MMU_IDX_DA) {
tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
ctx->base.is_jmp = DISAS_EXIT;
}
diff --git a/target/loongarch/tlb_helper.c b/target/loongarch/tlb_helper.c
index d2f8fb0c60..c6d1de50fe 100644
--- a/target/loongarch/tlb_helper.c
+++ b/target/loongarch/tlb_helper.c
@@ -170,8 +170,8 @@ static int get_physical_address(CPULoongArchState *env,
hwaddr *physical,
int *prot, target_ulong address,
MMUAccessType access_type, int mmu_idx)
{
-int user_mode = mmu_idx == MMU_USER_IDX;
-int kernel_mode = mmu_idx == MMU_KERNEL_IDX;
+int user_mode = mmu_idx == MMU_IDX_USER;
+int kernel_mode = mmu_idx == MMU_IDX_KERNEL;
uint32_t plv, base_c, base_v;
int64_t addr_high;
uint8_t da = FIELD_EX64(env->CSR_CRMD, CSR_CRMD, DA);
diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
index 31462b2b61..38ced69803 100644
--- a/target/loongarch/translate.c
+++ b/target/loongarch/translate.c
@@ -75,10 +75,11 @@ static void
loongarch_tr_init_disas_context(DisasContextBase *dcbase,
DisasContext *ctx = container_of(dcbase, DisasContext, base);
ctx->page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
+ctx->plv = ctx->base.tb->flags & HW_FLAGS_PLV_MASK;
if (ctx->base.tb->flags & HW_FLAGS_CRMD_PG) {
-ctx->mem_idx = ctx->base.tb->flags & HW_FLAGS_PLV_MASK;
+ctx->mem_idx = ctx->plv;
} else {
-ctx->mem_idx = MMU_DA_IDX;
+ctx->mem_idx = MMU_IDX_DA;
}
/* Bound the number of insns to execute to those left on the page. */
diff --git a/target/loongarch/translate.h b/target/loongarch/translate.h
index 9cc12512d1..6d2e382e8b 100644
--- a/target/loongarch/translate.h
+++ b/target/loongarch/translate.h
@@ -29,7 +29,8 @@ typedef struct DisasContext {
DisasContextBase base;
target_ulong page_start;
uint32_t opcode;
-int mem_idx;
+uint16_t mem_idx;
+uint16_t plv;
TCGv zero;
/* Space for 3 operands plus 1 extra for address computation. */
TCGv temp[4];
--
2.31.1
[PULL 0/2] loongarch for 7.2 patches
The following changes since commit 466e81ff12013d026e2d0154266fce82bce2ee9b: Merge tag 'vfio-fixes-v7.2-rc0.0' of https://gitlab.com/alex.williamson/qemu into staging (2022-11-05 08:41:01 -0400) are available in the Git repository at: https://gitlab.com/gaosong/qemu.git tags/pull-loongarch-20221107 for you to fetch changes up to e913bace61c539a88feb489b424554ebb2d5d3a3: target/loongarch: Fix return value of CHECK_FPE (2022-11-07 10:54:11 +0800) pull-loongarch-20221107 Rui Wang (2): target/loongarch: Separate the hardware flags into MMU index and PLV target/loongarch: Fix return value of CHECK_FPE target/loongarch/cpu.h | 18 +- target/loongarch/insn_trans/trans_farith.c.inc | 2 +- target/loongarch/insn_trans/trans_privileged.c.inc | 4 ++-- target/loongarch/tlb_helper.c | 4 ++-- target/loongarch/translate.c | 5 +++-- target/loongarch/translate.h | 3 ++- 6 files changed, 19 insertions(+), 17 deletions(-)
Re: [PATCH 4/5] target/riscv: No need to re-start QEMU timer when timecmp == UINT64_MAX
On Wed, Nov 2, 2022 at 5:40 AM Alistair Francis wrote:
>
> On Mon, Oct 31, 2022 at 1:49 PM Anup Patel wrote:
> >
> > On Mon, Oct 31, 2022 at 6:25 AM Alistair Francis
> > wrote:
> > >
> > > On Fri, Oct 28, 2022 at 2:53 AM Anup Patel
> > > wrote:
> > > >
> > > > The time CSR will wrap-around immediately after reaching UINT64_MAX
> > > > so we don't need to re-start QEMU timer when timecmp == UINT64_MAX
> > > > in riscv_timer_write_timecmp().
> > >
> > > I'm not clear what this is fixing?
> > >
> > > If the guest sets a timer for UINT64_MAX shouldn't that still trigger
> > > an event at some point?
> >
> > Here's what Sstc says about timer interrupt using Sstc:
> > "A supervisor timer interrupt becomes pending - as reflected in the
> > STIP bit in the mip and sip registers - whenever time contains a
> > value greater than or equal to stimecmp, treating the values as
> > unsigned integers. Writes to stimecmp are guaranteed to be
> > reflected in STIP eventually, but not necessarily immediately.
> > The interrupt remains posted until stimecmp becomes greater
> > than time - typically as a result of writing stimecmp."
> >
> > When timecmp = UINT64_MAX, the time CSR will eventually reach
> > timecmp value but on next timer tick the time CSR will wrap-around
> > and become zero which is less than UINT64_MAX. Now, the timer
> > interrupt behaves like a level triggered interrupt so it will become 1
> > when time = timecmp = UINT64_MAX and next timer tick it will
> > become 0 again because time = 0 < timecmp = UINT64_MAX.
>
> Ah, I didn't realise this. Can you add this to the code comment and
> maybe add this description to the commit message. Otherwise:
>
> Reviewed-by: Alistair Francis
Sure, I will add a detailed comment block in the code itself.
Thanks,
Anup
>
> Alistair
>
> >
> > This time CSR wrap-around comparison with timecmp is natural
> > to implement in HW but not straight forward in QEMU hence this
> > patch.
> >
> > Software can potentially use timecmp = UINT64_MAX as a way
> > to clear the timer interrupt and keep timer disabled instead of
> > enabling/disabling sie.STIP. This timecmp = UINT64_MAX helps:
> > 1) Linux RISC-V timer driver keep timer interrupt enable/disable
> > state in-sync with Linux interrupt subsystem.
> > 2) Reduce number of traps taken when emulating Sstc for the
> > "Nested Guest" (i.e. Guest running under some "Guest Hypervisor"
> > which in-turn runs under a "Host Hypervisor").
> >
> > In fact, the SBI set_timer() call also defines similar mechanism to
> > disable timer: "If the supervisor wishes to clear the timer interrupt
> > without scheduling the next timer event, it can either request a timer
> > interrupt infinitely far into the future (i.e., (uint64_t)-1), ...".
> >
> > Regards,
> > Anup
> >
> > >
> > > Alistair
> > >
> > > >
> > > > Signed-off-by: Anup Patel
> > > > ---
> > > > target/riscv/time_helper.c | 8
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/target/riscv/time_helper.c b/target/riscv/time_helper.c
> > > > index 4fb2a471a9..1ee9f94813 100644
> > > > --- a/target/riscv/time_helper.c
> > > > +++ b/target/riscv/time_helper.c
> > > > @@ -72,6 +72,14 @@ void riscv_timer_write_timecmp(RISCVCPU *cpu,
> > > > QEMUTimer *timer,
> > > > riscv_cpu_update_mip(cpu, timer_irq, BOOL_TO_MASK(0));
> > > > }
> > > >
> > > > +/*
> > > > + * Don't re-start the QEMU timer when timecmp == UINT64_MAX because
> > > > + * time CSR will wrap-around immediately after reaching UINT64_MAX.
> > > > + */
> > > > +if (timecmp == UINT64_MAX) {
> > > > +return;
> > > > +}
> > > > +
> > > > /* otherwise, set up the future timer interrupt */
> > > > diff = timecmp - rtc_r;
> > > > /* back to ns (note args switched in muldiv64) */
> > > > --
> > > > 2.34.1
> > > >
> > > >
[PATCH 2/2] target/loongarch: Fix return value of CHECK_FPE
Regarding the patchset v3 has been merged into main line, and not
approved, this patch updates to patchset v4.
Fixes: 2419978c ("target/loongarch: Fix emulation of float-point disable
exception")
Link: https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg00808.html
Reviewed-by: Richard Henderson
Signed-off-by: Rui Wang
---
target/loongarch/insn_trans/trans_farith.c.inc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/loongarch/insn_trans/trans_farith.c.inc
b/target/loongarch/insn_trans/trans_farith.c.inc
index e2dec75dfb..7081fbb89b 100644
--- a/target/loongarch/insn_trans/trans_farith.c.inc
+++ b/target/loongarch/insn_trans/trans_farith.c.inc
@@ -7,7 +7,7 @@
#define CHECK_FPE do { \
if ((ctx->base.tb->flags & HW_FLAGS_EUEN_FPE) == 0) { \
generate_exception(ctx, EXCCODE_FPD); \
-return false; \
+return true; \
} \
} while (0)
#else
--
2.38.1
[PATCH 0/2] Updates emulation of float-point to v4
Regarding the patchset v3 has been merged into main line, and not approved, this patch updates to patchset v4. Link: https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg00808.html Rui Wang (2): target/loongarch: Separate the hardware flags into MMU index and PLV target/loongarch: Fix return value of CHECK_FPE target/loongarch/cpu.h | 18 +- target/loongarch/insn_trans/trans_farith.c.inc | 2 +- .../insn_trans/trans_privileged.c.inc | 4 ++-- target/loongarch/tlb_helper.c | 4 ++-- target/loongarch/translate.c | 5 +++-- target/loongarch/translate.h | 3 ++- 6 files changed, 19 insertions(+), 17 deletions(-) -- 2.38.1
[PATCH 1/2] target/loongarch: Separate the hardware flags into MMU index and PLV
Regarding the patchset v3 has been merged into main line, and not
approved, this patch updates to patchset v4.
Fixes: b4bda200 ("target/loongarch: Adjust the layout of hardware flags bit
fields")
Link: https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg00808.html
Reviewed-by: Richard Henderson
Signed-off-by: Rui Wang
---
target/loongarch/cpu.h | 18 +-
.../insn_trans/trans_privileged.c.inc | 4 ++--
target/loongarch/tlb_helper.c | 4 ++--
target/loongarch/translate.c | 5 +++--
target/loongarch/translate.h | 3 ++-
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 08c1f6baa1..e15c633b0b 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -374,21 +374,21 @@ struct LoongArchCPUClass {
* 0 for kernel mode, 3 for user mode.
* Define an extra index for DA(direct addressing) mode.
*/
-#define MMU_KERNEL_IDX 0
-#define MMU_USER_IDX 3
-#define MMU_DA_IDX 4
+#define MMU_PLV_KERNEL 0
+#define MMU_PLV_USER 3
+#define MMU_IDX_KERNEL MMU_PLV_KERNEL
+#define MMU_IDX_USER MMU_PLV_USER
+#define MMU_IDX_DA 4
static inline int cpu_mmu_index(CPULoongArchState *env, bool ifetch)
{
#ifdef CONFIG_USER_ONLY
-return MMU_USER_IDX;
+return MMU_IDX_USER;
#else
-uint8_t pg = FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PG);
-
-if (!pg) {
-return MMU_DA_IDX;
+if (FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PG)) {
+return FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV);
}
-return FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV);
+return MMU_IDX_DA;
#endif
}
diff --git a/target/loongarch/insn_trans/trans_privileged.c.inc
b/target/loongarch/insn_trans/trans_privileged.c.inc
index ff3a6d95ae..40f82becb0 100644
--- a/target/loongarch/insn_trans/trans_privileged.c.inc
+++ b/target/loongarch/insn_trans/trans_privileged.c.inc
@@ -159,7 +159,7 @@ static const CSRInfo csr_info[] = {
static bool check_plv(DisasContext *ctx)
{
-if (ctx->mem_idx == MMU_USER_IDX) {
+if (ctx->plv == MMU_PLV_USER) {
generate_exception(ctx, EXCCODE_IPE);
return true;
}
@@ -335,7 +335,7 @@ TRANS(iocsrwr_d, gen_iocsrwr, gen_helper_iocsrwr_d)
static void check_mmu_idx(DisasContext *ctx)
{
-if (ctx->mem_idx != MMU_DA_IDX) {
+if (ctx->mem_idx != MMU_IDX_DA) {
tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
ctx->base.is_jmp = DISAS_EXIT;
}
diff --git a/target/loongarch/tlb_helper.c b/target/loongarch/tlb_helper.c
index d2f8fb0c60..c6d1de50fe 100644
--- a/target/loongarch/tlb_helper.c
+++ b/target/loongarch/tlb_helper.c
@@ -170,8 +170,8 @@ static int get_physical_address(CPULoongArchState *env,
hwaddr *physical,
int *prot, target_ulong address,
MMUAccessType access_type, int mmu_idx)
{
-int user_mode = mmu_idx == MMU_USER_IDX;
-int kernel_mode = mmu_idx == MMU_KERNEL_IDX;
+int user_mode = mmu_idx == MMU_IDX_USER;
+int kernel_mode = mmu_idx == MMU_IDX_KERNEL;
uint32_t plv, base_c, base_v;
int64_t addr_high;
uint8_t da = FIELD_EX64(env->CSR_CRMD, CSR_CRMD, DA);
diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
index 31462b2b61..38ced69803 100644
--- a/target/loongarch/translate.c
+++ b/target/loongarch/translate.c
@@ -75,10 +75,11 @@ static void
loongarch_tr_init_disas_context(DisasContextBase *dcbase,
DisasContext *ctx = container_of(dcbase, DisasContext, base);
ctx->page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
+ctx->plv = ctx->base.tb->flags & HW_FLAGS_PLV_MASK;
if (ctx->base.tb->flags & HW_FLAGS_CRMD_PG) {
-ctx->mem_idx = ctx->base.tb->flags & HW_FLAGS_PLV_MASK;
+ctx->mem_idx = ctx->plv;
} else {
-ctx->mem_idx = MMU_DA_IDX;
+ctx->mem_idx = MMU_IDX_DA;
}
/* Bound the number of insns to execute to those left on the page. */
diff --git a/target/loongarch/translate.h b/target/loongarch/translate.h
index 9cc12512d1..6d2e382e8b 100644
--- a/target/loongarch/translate.h
+++ b/target/loongarch/translate.h
@@ -29,7 +29,8 @@ typedef struct DisasContext {
DisasContextBase base;
target_ulong page_start;
uint32_t opcode;
-int mem_idx;
+uint16_t mem_idx;
+uint16_t plv;
TCGv zero;
/* Space for 3 operands plus 1 extra for address computation. */
TCGv temp[4];
--
2.38.1
Re: [PATCH v1 1/4] target/riscv: Add itrigger support when icount is not enabled
On 2022/11/7 9:37, Alistair Francis wrote:
On Thu, Oct 13, 2022 at 4:32 PM LIU Zhiwei wrote:
When icount is not enabled, there is no API in QEMU that can get the
guest instruction number.
Translate the guest code in a way that each TB only has one instruction.
I don't think this is a great idea.
Why can't we just require icount be enabled if a user wants this? Or singlestep?
This feature will only be used by users who want to run the native gdb
on Linux. If we run QEMU as a service, after booting the kernel, we
can't predicate whether the users will use native gdb.
Besides, icount can't be enabled on MTTCG currently (I am working on
this problem) and I don't want to constraint the use of MTTCG even when
it is possible the users use native gdb (which may only occupy just a
little time).
Thus, I give this fallback way to implement the itrigger. The icount
parameter can be used as an accelerated way.
Thanks,
Zhiwei
Alistair
After executing the instruction, decrease the count by 1 until it reaches 0
where the itrigger fires.
Note that only when priviledge matches the itrigger configuration,
the count will decrease.
Signed-off-by: LIU Zhiwei
---
target/riscv/cpu.h| 2 +
target/riscv/cpu_helper.c | 6 ++
target/riscv/debug.c | 71 +++
target/riscv/debug.h | 12
target/riscv/helper.h | 2 +
.../riscv/insn_trans/trans_privileged.c.inc | 4 +-
target/riscv/insn_trans/trans_rvi.c.inc | 8 +--
target/riscv/insn_trans/trans_rvv.c.inc | 4 +-
target/riscv/translate.c | 33 -
9 files changed, 131 insertions(+), 11 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index b131fa8c8e..24bafda27d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -621,6 +621,8 @@ FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
FIELD(TB_FLAGS, VTA, 24, 1)
FIELD(TB_FLAGS, VMA, 25, 1)
+/* Native debug itrigger */
+FIELD(TB_FLAGS, ITRIGGER, 26, 1)
#ifdef TARGET_RISCV32
#define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 278d163803..263282f230 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -27,7 +27,9 @@
#include "tcg/tcg-op.h"
#include "trace.h"
#include "semihosting/common-semi.h"
+#include "sysemu/cpu-timers.h"
#include "cpu_bits.h"
+#include "debug.h"
int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
{
@@ -103,6 +105,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong
*pc,
flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
get_field(env->mstatus_hs, MSTATUS_VS));
}
+if (riscv_feature(env, RISCV_FEATURE_DEBUG) && !icount_enabled()) {
+flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER,
+ riscv_itrigger_enabled(env));
+}
#endif
flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 26ea764407..45a3537d5c 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -29,6 +29,7 @@
#include "cpu.h"
#include "trace.h"
#include "exec/exec-all.h"
+#include "exec/helper-proto.h"
/*
* The following M-mode trigger CSRs are implemented:
@@ -498,6 +499,76 @@ static void type6_reg_write(CPURISCVState *env,
target_ulong index,
return;
}
+/* icount trigger type */
+static inline int
+itrigger_get_count(CPURISCVState *env, int index)
+{
+return get_field(env->tdata1[index], ITRIGGER_COUNT);
+}
+
+static inline void
+itrigger_set_count(CPURISCVState *env, int index, int value)
+{
+env->tdata1[index] = set_field(env->tdata1[index],
+ ITRIGGER_COUNT, value);
+}
+
+static bool check_itrigger_priv(CPURISCVState *env, int index)
+{
+target_ulong tdata1 = env->tdata1[index];
+if (riscv_cpu_virt_enabled(env)) {
+/* check VU/VS bit against current privilege level */
+return (get_field(tdata1, ITRIGGER_VS) == env->priv) ||
+ (get_field(tdata1, ITRIGGER_VU) == env->priv);
+} else {
+/* check U/S/M bit against current privilege level */
+return (get_field(tdata1, ITRIGGER_M) == env->priv) ||
+ (get_field(tdata1, ITRIGGER_S) == env->priv) ||
+ (get_field(tdata1, ITRIGGER_U) == env->priv);
+}
+}
+
+bool riscv_itrigger_enabled(CPURISCVState *env)
+{
+int count;
+for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
+if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
+continue;
+}
+if (check_itrigger_priv(env, i)) {
+continue;
+}
+count = itrigger_get_count(env, i);
+if (!count) {
+continue;
+}
+return true;
+}
+
+return false;
+}
Re: [PATCH v1 1/4] target/riscv: Add itrigger support when icount is not enabled
On Thu, Oct 13, 2022 at 4:32 PM LIU Zhiwei wrote:
>
> When icount is not enabled, there is no API in QEMU that can get the
> guest instruction number.
>
> Translate the guest code in a way that each TB only has one instruction.
I don't think this is a great idea.
Why can't we just require icount be enabled if a user wants this? Or singlestep?
Alistair
> After executing the instruction, decrease the count by 1 until it reaches 0
> where the itrigger fires.
>
> Note that only when priviledge matches the itrigger configuration,
> the count will decrease.
>
> Signed-off-by: LIU Zhiwei
> ---
> target/riscv/cpu.h| 2 +
> target/riscv/cpu_helper.c | 6 ++
> target/riscv/debug.c | 71 +++
> target/riscv/debug.h | 12
> target/riscv/helper.h | 2 +
> .../riscv/insn_trans/trans_privileged.c.inc | 4 +-
> target/riscv/insn_trans/trans_rvi.c.inc | 8 +--
> target/riscv/insn_trans/trans_rvv.c.inc | 4 +-
> target/riscv/translate.c | 33 -
> 9 files changed, 131 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index b131fa8c8e..24bafda27d 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -621,6 +621,8 @@ FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
> FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
> FIELD(TB_FLAGS, VTA, 24, 1)
> FIELD(TB_FLAGS, VMA, 25, 1)
> +/* Native debug itrigger */
> +FIELD(TB_FLAGS, ITRIGGER, 26, 1)
>
> #ifdef TARGET_RISCV32
> #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 278d163803..263282f230 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -27,7 +27,9 @@
> #include "tcg/tcg-op.h"
> #include "trace.h"
> #include "semihosting/common-semi.h"
> +#include "sysemu/cpu-timers.h"
> #include "cpu_bits.h"
> +#include "debug.h"
>
> int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
> {
> @@ -103,6 +105,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env,
> target_ulong *pc,
> flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
> get_field(env->mstatus_hs, MSTATUS_VS));
> }
> +if (riscv_feature(env, RISCV_FEATURE_DEBUG) && !icount_enabled()) {
> +flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER,
> + riscv_itrigger_enabled(env));
> +}
> #endif
>
> flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 26ea764407..45a3537d5c 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -29,6 +29,7 @@
> #include "cpu.h"
> #include "trace.h"
> #include "exec/exec-all.h"
> +#include "exec/helper-proto.h"
>
> /*
> * The following M-mode trigger CSRs are implemented:
> @@ -498,6 +499,76 @@ static void type6_reg_write(CPURISCVState *env,
> target_ulong index,
> return;
> }
>
> +/* icount trigger type */
> +static inline int
> +itrigger_get_count(CPURISCVState *env, int index)
> +{
> +return get_field(env->tdata1[index], ITRIGGER_COUNT);
> +}
> +
> +static inline void
> +itrigger_set_count(CPURISCVState *env, int index, int value)
> +{
> +env->tdata1[index] = set_field(env->tdata1[index],
> + ITRIGGER_COUNT, value);
> +}
> +
> +static bool check_itrigger_priv(CPURISCVState *env, int index)
> +{
> +target_ulong tdata1 = env->tdata1[index];
> +if (riscv_cpu_virt_enabled(env)) {
> +/* check VU/VS bit against current privilege level */
> +return (get_field(tdata1, ITRIGGER_VS) == env->priv) ||
> + (get_field(tdata1, ITRIGGER_VU) == env->priv);
> +} else {
> +/* check U/S/M bit against current privilege level */
> +return (get_field(tdata1, ITRIGGER_M) == env->priv) ||
> + (get_field(tdata1, ITRIGGER_S) == env->priv) ||
> + (get_field(tdata1, ITRIGGER_U) == env->priv);
> +}
> +}
> +
> +bool riscv_itrigger_enabled(CPURISCVState *env)
> +{
> +int count;
> +for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
> +if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
> +continue;
> +}
> +if (check_itrigger_priv(env, i)) {
> +continue;
> +}
> +count = itrigger_get_count(env, i);
> +if (!count) {
> +continue;
> +}
> +return true;
> +}
> +
> +return false;
> +}
> +
> +void helper_itrigger_match(CPURISCVState *env)
> +{
> +int count;
> +for (int i = 0; i < RV_MAX_TRIGGERS; i++) {
> +if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
> +continue;
> +}
> +if (check_itrigger_priv(env, i)) {
> +continue;
> +}
> +count = itrigger_get_count(env, i);
> +if (!count) {
> +
Re: [PATCH v3 11/11] Hexagon (target/hexagon) Use direct block chaining for tight loops
On 11/7/22 08:52, Taylor Simpson wrote: I coded this originally with manual handling but decided this would be easier to read/understand/maintain - especially as we add more flags and some have more than 1 bit. I haven't noticed the flags in any of the logs. Where are they printed? Third field of -d exec, e.g. Trace 0: 0x7f09a40004c0 [/fc44/0101/ff00] __start r~
RE: [PATCH v3 11/11] Hexagon (target/hexagon) Use direct block chaining for tight loops
> -Original Message-
> From: Richard Henderson
> Sent: Friday, November 4, 2022 8:44 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: phi...@linaro.org; a...@rev.ng; a...@rev.ng; Brian Cain
> ; Matheus Bernardino (QUIC)
>
> Subject: Re: [PATCH v3 11/11] Hexagon (target/hexagon) Use direct block
> chaining for tight loops
>
> On 11/5/22 06:26, Taylor Simpson wrote:
> > Direct block chaining is documented here
> > https://qemu.readthedocs.io/en/latest/devel/tcg.html#direct-block-chai
> > ning
> >
> > Hexagon inner loops end with the endloop0 instruction To go back to
> > the beginning of the loop, this instructions writes to PC from
> > register SA0 (start address 0). To use direct block chaining, we have
> > to assign PC with a constant value. So, we specialize the code
> > generation when the start of the translation block is equal to SA0.
> >
> > When this is the case, we defer the compare/branch from endloop0 to
> > gen_end_tb. When this is done, we can assign the start address of the
> > TB to PC.
> >
> > Signed-off-by: Taylor Simpson
> > ---
> > target/hexagon/cpu.h | 17
> > target/hexagon/gen_tcg.h | 3 ++
> > target/hexagon/translate.h | 1 +
> > target/hexagon/genptr.c| 57
> ++
> > target/hexagon/translate.c | 34 +++
> > 5 files changed, 107 insertions(+), 5 deletions(-)
> >
> > diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h index
> > ff8c26272d..5260e0f127 100644
> > --- a/target/hexagon/cpu.h
> > +++ b/target/hexagon/cpu.h
> > @@ -152,16 +152,23 @@ struct ArchCPU {
> >
> > #include "cpu_bits.h"
> >
> > +typedef union {
> > +uint32_t i;
> > +struct {
> > +bool is_tight_loop:1;
> > +};
> > +} HexStateFlags;
>
> I don't see this as an improvement on manual flags handling, as it makes the
> flags value be dependent on host bit-field ordering. This makes it more
> difficult to compare traces across hosts.
I coded this originally with manual handling but decided this would be easier
to read/understand/maintain - especially as we add more flags and some have
more than 1 bit.
I haven't noticed the flags in any of the logs. Where are they printed?
>
> Otherwise,
> Reviewed-by: Richard Henderson
>
>
> r~
Re: [PULL v3 50/81] tests: acpi: whitelist DSDT before generating PCI-ISA bridge AML automatically
On Sat, Nov 5, 2022 at 6:27 PM Michael S. Tsirkin wrote: > From: Igor Mammedov > > Signed-off-by: Igor Mammedov > Message-Id: <20221017102146.2254096-3-imamm...@redhat.com> > Reviewed-by: Michael S. Tsirkin > Signed-off-by: Michael S. Tsirkin > --- > tests/qtest/bios-tables-test-allowed-diff.h | 34 + > 1 file changed, 34 insertions(+) > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h > b/tests/qtest/bios-tables-test-allowed-diff.h > index dfb8523c8b..570b17478e 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1 +1,35 @@ > /* List of comma-separated changed AML files to ignore */ > +"tests/data/acpi/pc/DSDT", > +"tests/data/acpi/pc/DSDT.acpierst", > +"tests/data/acpi/pc/DSDT.acpihmat", > +"tests/data/acpi/pc/DSDT.bridge", > +"tests/data/acpi/pc/DSDT.cphp", > +"tests/data/acpi/pc/DSDT.dimmpxm", > +"tests/data/acpi/pc/DSDT.hpbridge", > +"tests/data/acpi/pc/DSDT.hpbrroot", > +"tests/data/acpi/pc/DSDT.ipmikcs", > +"tests/data/acpi/pc/DSDT.memhp", > +"tests/data/acpi/pc/DSDT.nohpet", > +"tests/data/acpi/pc/DSDT.numamem", > +"tests/data/acpi/pc/DSDT.roothp", > +"tests/data/acpi/q35/DSDT", > +"tests/data/acpi/q35/DSDT.acpierst", > +"tests/data/acpi/q35/DSDT.acpihmat", > +"tests/data/acpi/q35/DSDT.applesmc", > +"tests/data/acpi/q35/DSDT.bridge", > +"tests/data/acpi/q35/DSDT.core-count2" ... and probably in more patches down the road. Best regards, Bernhard +"tests/data/acpi/q35/DSDT.cphp", > +"tests/data/acpi/q35/DSDT.cxl", > +"tests/data/acpi/q35/DSDT.dimmpxm", > +"tests/data/acpi/q35/DSDT.ipmibt", > +"tests/data/acpi/q35/DSDT.ipmismbus", > +"tests/data/acpi/q35/DSDT.ivrs", > +"tests/data/acpi/q35/DSDT.memhp", > +"tests/data/acpi/q35/DSDT.mmio64", > +"tests/data/acpi/q35/DSDT.multi-bridge", > +"tests/data/acpi/q35/DSDT.nohpet", > +"tests/data/acpi/q35/DSDT.numamem", > +"tests/data/acpi/q35/DSDT.pvpanic-isa", > +"tests/data/acpi/q35/DSDT.tis.tpm12", > +"tests/data/acpi/q35/DSDT.tis.tpm2", > +"tests/data/acpi/q35/DSDT.viot", > +"tests/data/acpi/q35/DSDT.xapic", > -- > MST > > >
RE: [PATCH v3 10/11] Hexagon (target/hexagon) Use direct block chaining for direct jump/branch
> -Original Message-
> From: Richard Henderson
> Sent: Friday, November 4, 2022 8:33 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: phi...@linaro.org; a...@rev.ng; a...@rev.ng; Brian Cain
> ; Matheus Bernardino (QUIC)
>
> Subject: Re: [PATCH v3 10/11] Hexagon (target/hexagon) Use direct block
> chaining for direct jump/branch
>
> On 11/5/22 06:26, Taylor Simpson wrote:
> > Direct block chaining is documented here
> > https://qemu.readthedocs.io/en/latest/devel/tcg.html#direct-block-chai
> > ning
> >
> > Recall that Hexagon allows packets with multiple jumps where only the
> > first one with a true predicate will actually jump. So, we can only
> > use direct block chaining when the packet contains a single PC-relative
> jump.
>
> Not quite accurate.
>
> Only the first two direct branches can use direct block chaining. Other exits
> from the translation block could use indirect block chaining
> (tcg_gen_lookup_and_goto_ptr). You just have to remember which is
> taken.
>
I'll work on the wording in the commit message. When there is a single
PC-relative branch or jump in the packet, we use
tcg_gen_goto_tb/tcg_gen_exit_tb. Otherwise, we use tcg_gen_lookup_and_goto_ptr.
> That said, this is certainly an improvement.
>
> > +if (ctx->pkt->pkt_has_multi_cof) {
> > +gen_write_new_pc_addr(ctx, tcg_constant_tl(dest), pred);
> > +} else {
> > +/* Defer this jump to the end of the TB */
> > +g_assert(ctx->branch_cond == NULL);
> > +ctx->has_single_direct_branch = true;
> > +if (pred != NULL) {
> > +ctx->branch_cond = tcg_temp_local_new();
> > +tcg_gen_mov_tl(ctx->branch_cond, pred);
> > +}
> > +ctx->branch_dest = dest;
>
> Perhaps re-use hex_branch_taken as branch_cond?
Good idea. That will save the allocation/deallocation of the TCGv. I'll
change it to a TCGCond to indicate the comparison to be done (if any). It will
work nicely with your other suggestion to pass the branch condition along.
>
> Anyway,
> Reviewed-by: Richard Henderson
>
>
> r~
Re: [PULL v3 49/81] acpi: pc: vga: use AcpiDevAmlIf interface to build VGA device descriptors
On Sun, Nov 6, 2022 at 10:16 PM Bernhard Beschow wrote: > > > On Sat, Nov 5, 2022 at 6:45 PM Michael S. Tsirkin wrote: > >> From: Igor Mammedov >> >> Signed-off-by: Igor Mammedov >> Message-Id: <20221017102146.2254096-2-imamm...@redhat.com> >> Reviewed-by: Michael S. Tsirkin >> Signed-off-by: Michael S. Tsirkin >> NB: we do not expect any functional change in >> any ACPI tables with this change. It's only a refactoring. >> >> Reviewed-by: Ani Sinha >> --- >> hw/display/vga_int.h | 2 ++ >> hw/display/acpi-vga-stub.c | 7 +++ >> hw/display/acpi-vga.c | 26 ++ >> hw/display/vga-pci.c | 4 >> hw/i386/acpi-build.c | 26 +- >> hw/display/meson.build | 17 + >> 6 files changed, 57 insertions(+), 25 deletions(-) >> create mode 100644 hw/display/acpi-vga-stub.c >> create mode 100644 hw/display/acpi-vga.c >> > > With this "qemu:qtest+qtest-hppa / qtest-hppa/display-vga-test" fails due > to the symbol "aml_return" being undefined: > > # starting QEMU: exec ./qemu-system-hppa -qtest > unix:/tmp/qtest-515650.sock -qtest-log /dev/null -chardev > socket,path=/tmp/qtest-515650.qmp,id=char0 -mon chardev=char0,mode=control > -display none -vga none -device virtio-vga -accel qtest > --- stderr > --- > Failed to open module: > qemu/build/qemu-bundle/usr/lib/qemu/hw-display-virtio-vga.so: undefined > symbol: aml_return > qemu-system-hppa: -device virtio-vga: 'virtio-vga' is not a valid device > model name > Broken pipe > ../src/tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU > process but encountered exit status 1 (expected 0) > > (test program exited with status code -6) > It doesn't only affect hppa: grep -e "undefined symbol: aml_return" meson-logs/testlog.txt | wc -l 139 Best regards, Bernhard
[PATCH v2 6/6] accel/tcg: Split out setjmp_gen_code
Isolate the code protected by setjmp. Fixes:
translate-all.c: In function ‘tb_gen_code’:
translate-all.c:748:51: error: argument ‘cflags’ might be clobbered by
‘longjmp’ or ‘vfork’ [-Werror=clobbered]
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
accel/tcg/translate-all.c | 58 ++-
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 9ee21f7f52..ac3ee3740c 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -742,6 +742,37 @@ void page_collection_unlock(struct page_collection *set)
#endif /* !CONFIG_USER_ONLY */
+/*
+ * Isolate the portion of code gen which can setjmp/longjmp.
+ * Return the size of the generated code, or negative on error.
+ */
+static int setjmp_gen_code(CPUArchState *env, TranslationBlock *tb,
+ target_ulong pc, void *host_pc,
+ int *max_insns, int64_t *ti)
+{
+int ret = sigsetjmp(tcg_ctx->jmp_trans, 0);
+if (unlikely(ret != 0)) {
+return ret;
+}
+
+tcg_func_start(tcg_ctx);
+
+tcg_ctx->cpu = env_cpu(env);
+gen_intermediate_code(env_cpu(env), tb, *max_insns, pc, host_pc);
+assert(tb->size != 0);
+tcg_ctx->cpu = NULL;
+*max_insns = tb->icount;
+
+#ifdef CONFIG_PROFILER
+qatomic_set(_ctx->prof.tb_count, tcg_ctx->prof.tb_count + 1);
+qatomic_set(_ctx->prof.interm_time,
+tcg_ctx->prof.interm_time + profile_getclock() - *ti);
+*ti = profile_getclock();
+#endif
+
+return tcg_gen_code(tcg_ctx, tb, pc);
+}
+
/* Called with mmap_lock held for user mode emulation. */
TranslationBlock *tb_gen_code(CPUState *cpu,
target_ulong pc, target_ulong cs_base,
@@ -754,8 +785,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
int gen_code_size, search_size, max_insns;
#ifdef CONFIG_PROFILER
TCGProfile *prof = _ctx->prof;
-int64_t ti;
#endif
+int64_t ti;
void *host_pc;
assert_memory_lock();
@@ -805,33 +836,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
ti = profile_getclock();
#endif
-gen_code_size = sigsetjmp(tcg_ctx->jmp_trans, 0);
-if (unlikely(gen_code_size != 0)) {
-goto error_return;
-}
-
-tcg_func_start(tcg_ctx);
-
-tcg_ctx->cpu = env_cpu(env);
-gen_intermediate_code(cpu, tb, max_insns, pc, host_pc);
-assert(tb->size != 0);
-tcg_ctx->cpu = NULL;
-max_insns = tb->icount;
-
trace_translate_block(tb, pc, tb->tc.ptr);
-/* generate machine code */
-
-#ifdef CONFIG_PROFILER
-qatomic_set(>tb_count, prof->tb_count + 1);
-qatomic_set(>interm_time,
-prof->interm_time + profile_getclock() - ti);
-ti = profile_getclock();
-#endif
-
-gen_code_size = tcg_gen_code(tcg_ctx, tb, pc);
+gen_code_size = setjmp_gen_code(env, tb, pc, host_pc, _insns, );
if (unlikely(gen_code_size < 0)) {
- error_return:
switch (gen_code_size) {
case -1:
/*
--
2.34.1
[PATCH v2 5/6] tcg: Move TCG_TARGET_HAS_direct_jump init to tb_gen_code
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
accel/tcg/translate-all.c | 10 --
tcg/tcg.c | 12
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 921944a5ab..9ee21f7f52 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -821,16 +821,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
trace_translate_block(tb, pc, tb->tc.ptr);
/* generate machine code */
-tb->jmp_reset_offset[0] = TB_JMP_RESET_OFFSET_INVALID;
-tb->jmp_reset_offset[1] = TB_JMP_RESET_OFFSET_INVALID;
-tcg_ctx->tb_jmp_reset_offset = tb->jmp_reset_offset;
-if (TCG_TARGET_HAS_direct_jump) {
-tcg_ctx->tb_jmp_insn_offset = tb->jmp_target_arg;
-tcg_ctx->tb_jmp_target_addr = NULL;
-} else {
-tcg_ctx->tb_jmp_insn_offset = NULL;
-tcg_ctx->tb_jmp_target_addr = tb->jmp_target_arg;
-}
#ifdef CONFIG_PROFILER
qatomic_set(>tb_count, prof->tb_count + 1);
diff --git a/tcg/tcg.c b/tcg/tcg.c
index b43b6a7981..436fcf6ebd 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4228,6 +4228,18 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb,
target_ulong pc_start)
}
#endif
+/* Initialize goto_tb jump offsets. */
+tb->jmp_reset_offset[0] = TB_JMP_RESET_OFFSET_INVALID;
+tb->jmp_reset_offset[1] = TB_JMP_RESET_OFFSET_INVALID;
+tcg_ctx->tb_jmp_reset_offset = tb->jmp_reset_offset;
+if (TCG_TARGET_HAS_direct_jump) {
+tcg_ctx->tb_jmp_insn_offset = tb->jmp_target_arg;
+tcg_ctx->tb_jmp_target_addr = NULL;
+} else {
+tcg_ctx->tb_jmp_insn_offset = NULL;
+tcg_ctx->tb_jmp_target_addr = tb->jmp_target_arg;
+}
+
tcg_reg_alloc_start(s);
/*
--
2.34.1
[PATCH v2 2/6] disas/nanomips: Merge insn{1,2,3} into words[3]
Since Disassemble wants the data in this format, collect
it that way. This allows using a loop to print the bytes.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
disas/nanomips.c | 44 +---
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/disas/nanomips.c b/disas/nanomips.c
index ea3e9202ac..1645d6d7aa 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21905,26 +21905,22 @@ static const Pool MAJOR[2] = {
0x0 },/* P16 */
};
-static bool nanomips_dis(char **buf, Dis_info *info,
- unsigned short one,
- unsigned short two,
- unsigned short three)
+static bool nanomips_dis(const uint16_t *data, char **buf, Dis_info *info)
{
-uint16 bits[3] = {one, two, three};
TABLE_ENTRY_TYPE type;
/* Handle runtime errors. */
if (unlikely(sigsetjmp(info->buf, 0) != 0)) {
return false;
}
-return Disassemble(bits, buf, , MAJOR, ARRAY_SIZE(MAJOR), info) >= 0;
+return Disassemble(data, buf, , MAJOR, ARRAY_SIZE(MAJOR), info) >= 0;
}
int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
{
int status, length;
bfd_byte buffer[2];
-uint16_t insn1 = 0, insn2 = 0, insn3 = 0;
+uint16_t words[3] = { };
g_autofree char *buf = NULL;
info->bytes_per_chunk = 2;
@@ -21948,15 +21944,14 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
}
if (info->endian == BFD_ENDIAN_BIG) {
-insn1 = bfd_getb16(buffer);
+words[0] = bfd_getb16(buffer);
} else {
-insn1 = bfd_getl16(buffer);
+words[0] = bfd_getl16(buffer);
}
length = 2;
-(*info->fprintf_func)(info->stream, "%04x ", insn1);
/* Handle 32-bit opcodes. */
-if ((insn1 & 0x1000) == 0) {
+if ((words[0] & 0x1000) == 0) {
status = (*info->read_memory_func)(memaddr + 2, buffer, 2, info);
if (status != 0) {
(*info->memory_error_func)(status, memaddr + 2, info);
@@ -21964,17 +21959,15 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
}
if (info->endian == BFD_ENDIAN_BIG) {
-insn2 = bfd_getb16(buffer);
+words[1] = bfd_getb16(buffer);
} else {
-insn2 = bfd_getl16(buffer);
+words[1] = bfd_getl16(buffer);
}
length = 4;
-(*info->fprintf_func)(info->stream, "%04x ", insn2);
-} else {
-(*info->fprintf_func)(info->stream, " ");
}
+
/* Handle 48-bit opcodes. */
-if ((insn1 >> 10) == 0x18) {
+if ((words[0] >> 10) == 0x18) {
status = (*info->read_memory_func)(memaddr + 4, buffer, 2, info);
if (status != 0) {
(*info->memory_error_func)(status, memaddr + 4, info);
@@ -21982,17 +21975,22 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
}
if (info->endian == BFD_ENDIAN_BIG) {
-insn3 = bfd_getb16(buffer);
+words[2] = bfd_getb16(buffer);
} else {
-insn3 = bfd_getl16(buffer);
+words[2] = bfd_getl16(buffer);
}
length = 6;
-(*info->fprintf_func)(info->stream, "%04x ", insn3);
-} else {
-(*info->fprintf_func)(info->stream, " ");
}
-if (nanomips_dis(, _info, insn1, insn2, insn3)) {
+for (int i = 0; i < ARRAY_SIZE(words); i++) {
+if (i * 2 < length) {
+(*info->fprintf_func)(info->stream, "%04x ", words[i]);
+} else {
+(*info->fprintf_func)(info->stream, " ");
+}
+}
+
+if (nanomips_dis(words, , _info)) {
(*info->fprintf_func) (info->stream, "%s", buf);
}
--
2.34.1
[PATCH v2 4/6] disas/nanomips: Tidy read for 48-bit opcodes
There is no point in looking for a 48-bit opcode if we've
not read the second word for a 32-bit opcode.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
disas/nanomips.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/disas/nanomips.c b/disas/nanomips.c
index 717fafb739..abc78ae165 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21962,14 +21962,14 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
return -1;
}
length = 4;
-}
-/* Handle 48-bit opcodes. */
-if ((words[0] >> 10) == 0x18) {
-if (!read_u16([1], memaddr + 4, info)) {
-return -1;
+/* Handle 48-bit opcodes. */
+if ((words[0] >> 10) == 0x18) {
+if (!read_u16([1], memaddr + 4, info)) {
+return -1;
+}
+length = 6;
}
-length = 6;
}
for (int i = 0; i < ARRAY_SIZE(words); i++) {
--
2.34.1
[PATCH v2 3/6] disas/nanomips: Split out read_u16
Split out a helper function for reading a uint16_t
with the correct endianness.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
disas/nanomips.c | 48 +++-
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/disas/nanomips.c b/disas/nanomips.c
index 1645d6d7aa..717fafb739 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21916,10 +21916,24 @@ static bool nanomips_dis(const uint16_t *data, char
**buf, Dis_info *info)
return Disassemble(data, buf, , MAJOR, ARRAY_SIZE(MAJOR), info) >= 0;
}
+static bool read_u16(uint16_t *ret, bfd_vma memaddr,
+ struct disassemble_info *info)
+{
+int status = (*info->read_memory_func)(memaddr, (bfd_byte *)ret, 2, info);
+if (status != 0) {
+(*info->memory_error_func)(status, memaddr, info);
+return false;
+}
+
+if ((info->endian == BFD_ENDIAN_BIG) != HOST_BIG_ENDIAN) {
+bswap16s(ret);
+}
+return true;
+}
+
int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
{
-int status, length;
-bfd_byte buffer[2];
+int length;
uint16_t words[3] = { };
g_autofree char *buf = NULL;
@@ -21937,48 +21951,24 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
disassm_info.fprintf_func = info->fprintf_func;
disassm_info.stream = info->stream;
-status = (*info->read_memory_func)(memaddr, buffer, 2, info);
-if (status != 0) {
-(*info->memory_error_func)(status, memaddr, info);
+if (!read_u16([0], memaddr, info)) {
return -1;
}
-
-if (info->endian == BFD_ENDIAN_BIG) {
-words[0] = bfd_getb16(buffer);
-} else {
-words[0] = bfd_getl16(buffer);
-}
length = 2;
/* Handle 32-bit opcodes. */
if ((words[0] & 0x1000) == 0) {
-status = (*info->read_memory_func)(memaddr + 2, buffer, 2, info);
-if (status != 0) {
-(*info->memory_error_func)(status, memaddr + 2, info);
+if (!read_u16([1], memaddr + 2, info)) {
return -1;
}
-
-if (info->endian == BFD_ENDIAN_BIG) {
-words[1] = bfd_getb16(buffer);
-} else {
-words[1] = bfd_getl16(buffer);
-}
length = 4;
}
/* Handle 48-bit opcodes. */
if ((words[0] >> 10) == 0x18) {
-status = (*info->read_memory_func)(memaddr + 4, buffer, 2, info);
-if (status != 0) {
-(*info->memory_error_func)(status, memaddr + 4, info);
+if (!read_u16([1], memaddr + 4, info)) {
return -1;
}
-
-if (info->endian == BFD_ENDIAN_BIG) {
-words[2] = bfd_getb16(buffer);
-} else {
-words[2] = bfd_getl16(buffer);
-}
length = 6;
}
--
2.34.1
[PATCH v2 1/6] disas/nanomips: Move setjmp into nanomips_dis
Reduce the number of local variables within the scope of the
setjmp by moving it to the existing helper. The actual length
returned from Disassemble is not used, because we have already
determined the length while reading bytes. Fixes:
nanomips.c: In function ‘print_insn_nanomips’:
nanomips.c:21925:14: error: variable ‘insn1’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
nanomips.c:21925:25: error: variable ‘insn2’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
nanomips.c:21925:36: error: variable ‘insn3’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
nanomips.c:21926:22: error: variable ‘buf’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
disas/nanomips.c | 42 +-
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/disas/nanomips.c b/disas/nanomips.c
index 9647f1a8e3..ea3e9202ac 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21905,22 +21905,24 @@ static const Pool MAJOR[2] = {
0x0 },/* P16 */
};
-static int nanomips_dis(char **buf,
- Dis_info *info,
- unsigned short one,
- unsigned short two,
- unsigned short three)
+static bool nanomips_dis(char **buf, Dis_info *info,
+ unsigned short one,
+ unsigned short two,
+ unsigned short three)
{
uint16 bits[3] = {one, two, three};
-
TABLE_ENTRY_TYPE type;
-int size = Disassemble(bits, buf, , MAJOR, 2, info);
-return size;
+
+/* Handle runtime errors. */
+if (unlikely(sigsetjmp(info->buf, 0) != 0)) {
+return false;
+}
+return Disassemble(bits, buf, , MAJOR, ARRAY_SIZE(MAJOR), info) >= 0;
}
int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
{
-int status;
+int status, length;
bfd_byte buffer[2];
uint16_t insn1 = 0, insn2 = 0, insn3 = 0;
g_autofree char *buf = NULL;
@@ -21950,6 +21952,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
} else {
insn1 = bfd_getl16(buffer);
}
+length = 2;
(*info->fprintf_func)(info->stream, "%04x ", insn1);
/* Handle 32-bit opcodes. */
@@ -21965,6 +21968,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
} else {
insn2 = bfd_getl16(buffer);
}
+length = 4;
(*info->fprintf_func)(info->stream, "%04x ", insn2);
} else {
(*info->fprintf_func)(info->stream, " ");
@@ -21982,27 +21986,15 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
} else {
insn3 = bfd_getl16(buffer);
}
+length = 6;
(*info->fprintf_func)(info->stream, "%04x ", insn3);
} else {
(*info->fprintf_func)(info->stream, " ");
}
-/* Handle runtime errors. */
-if (sigsetjmp(disassm_info.buf, 0) != 0) {
-info->insn_type = dis_noninsn;
-return insn3 ? 6 : insn2 ? 4 : 2;
+if (nanomips_dis(, _info, insn1, insn2, insn3)) {
+(*info->fprintf_func) (info->stream, "%s", buf);
}
-int length = nanomips_dis(, _info, insn1, insn2, insn3);
-
-/* FIXME: Should probably use a hash table on the major opcode here. */
-
-(*info->fprintf_func) (info->stream, "%s", buf);
-if (length > 0) {
-return length / 8;
-}
-
-info->insn_type = dis_noninsn;
-
-return insn3 ? 6 : insn2 ? 4 : 2;
+return length;
}
--
2.34.1
[PATCH v2 0/6] Two -Wclobbered fixes, plus other cleanup
Stefan reported for accel/tcg, and I reproduced on Ubuntu 22.04.
Changes for v2:
* Incorporate suggested changes to nanomips.c (phil, balaton).
r~
Richard Henderson (6):
disas/nanomips: Move setjmp into nanomips_dis
disas/nanomips: Merge insn{1,2,3} into words[3]
disas/nanomips: Split out read_u16
disas/nanomips: Tidy read for 48-bit opcodes
tcg: Move TCG_TARGET_HAS_direct_jump init to tb_gen_code
accel/tcg: Split out setjmp_gen_code
accel/tcg/translate-all.c | 68 ---
disas/nanomips.c | 110 --
tcg/tcg.c | 12 +
3 files changed, 90 insertions(+), 100 deletions(-)
--
2.34.1
Re: [PULL v3 49/81] acpi: pc: vga: use AcpiDevAmlIf interface to build VGA device descriptors
On Sat, Nov 5, 2022 at 6:45 PM Michael S. Tsirkin wrote: > From: Igor Mammedov > > Signed-off-by: Igor Mammedov > Message-Id: <20221017102146.2254096-2-imamm...@redhat.com> > Reviewed-by: Michael S. Tsirkin > Signed-off-by: Michael S. Tsirkin > NB: we do not expect any functional change in > any ACPI tables with this change. It's only a refactoring. > > Reviewed-by: Ani Sinha > --- > hw/display/vga_int.h | 2 ++ > hw/display/acpi-vga-stub.c | 7 +++ > hw/display/acpi-vga.c | 26 ++ > hw/display/vga-pci.c | 4 > hw/i386/acpi-build.c | 26 +- > hw/display/meson.build | 17 + > 6 files changed, 57 insertions(+), 25 deletions(-) > create mode 100644 hw/display/acpi-vga-stub.c > create mode 100644 hw/display/acpi-vga.c > With this "qemu:qtest+qtest-hppa / qtest-hppa/display-vga-test" fails due to the symbol "aml_return" being undefined: # starting QEMU: exec ./qemu-system-hppa -qtest unix:/tmp/qtest-515650.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-515650.qmp,id=char0 -mon chardev=char0,mode=control -display none -vga none -device virtio-vga -accel qtest --- stderr --- Failed to open module: qemu/build/qemu-bundle/usr/lib/qemu/hw-display-virtio-vga.so: undefined symbol: aml_return qemu-system-hppa: -device virtio-vga: 'virtio-vga' is not a valid device model name Broken pipe ../src/tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) (test program exited with status code -6) Best regards, Bernhard
Re: [PATCH] hw/riscv: virt: Remove size restriction for pflash
Hello Sunil!
What about virt_machine_done() function?
kernel_entry variable still points to the second flash started from
virt_memmap[VIRT_FLASH].size / 2.
On Sun, Nov 6, 2022 at 5:41 PM Sunil V L wrote:
>
> The pflash implementation currently assumes fixed size of the
> backend storage. Due to this, the backend storage file needs to be
> exactly of size 32M. Otherwise, there will be an error like below.
>
> "device requires 33554432 bytes, block backend provides 3145728 bytes"
>
> Fix this issue by using the actual size of the backing store.
>
> Signed-off-by: Sunil V L
> ---
> hw/riscv/virt.c | 33 +
> 1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a5bc7353b4..aad175fa31 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -49,6 +49,7 @@
> #include "hw/pci/pci.h"
> #include "hw/pci-host/gpex.h"
> #include "hw/display/ramfb.h"
> +#include "sysemu/block-backend.h"
>
> /*
> * The virt machine physical address space used by some of the devices
> @@ -144,10 +145,17 @@ static void virt_flash_map1(PFlashCFI01 *flash,
> MemoryRegion *sysmem)
> {
> DeviceState *dev = DEVICE(flash);
> +BlockBackend *blk;
> +hwaddr real_size;
>
> -assert(QEMU_IS_ALIGNED(size, VIRT_FLASH_SECTOR_SIZE));
> -assert(size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);
> -qdev_prop_set_uint32(dev, "num-blocks", size / VIRT_FLASH_SECTOR_SIZE);
> +blk = pflash_cfi01_get_blk(flash);
> +
> +real_size = blk ? blk_getlength(blk): size;
> +
> +assert(real_size);
> +assert(QEMU_IS_ALIGNED(real_size, VIRT_FLASH_SECTOR_SIZE));
> +assert(real_size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);
> +qdev_prop_set_uint32(dev, "num-blocks", real_size /
> VIRT_FLASH_SECTOR_SIZE);
> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
>
> memory_region_add_subregion(sysmem, base,
> @@ -971,15 +979,24 @@ static void create_fdt_flash(RISCVVirtState *s, const
> MemMapEntry *memmap)
> {
> char *name;
> MachineState *mc = MACHINE(s);
> -hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
> -hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
> +MemoryRegion *flash_mem;
> +hwaddr flashsize[2];
> +hwaddr flashbase[2];
> +
> +flash_mem = pflash_cfi01_get_memory(s->flash[0]);
> +flashbase[0] = flash_mem->addr;
> +flashsize[0] = flash_mem->size;
> +
> +flash_mem = pflash_cfi01_get_memory(s->flash[1]);
> +flashbase[1] = flash_mem->addr;
> +flashsize[1] = flash_mem->size;
>
> -name = g_strdup_printf("/flash@%" PRIx64, flashbase);
> +name = g_strdup_printf("/flash@%" PRIx64, flashbase[0]);
> qemu_fdt_add_subnode(mc->fdt, name);
> qemu_fdt_setprop_string(mc->fdt, name, "compatible", "cfi-flash");
> qemu_fdt_setprop_sized_cells(mc->fdt, name, "reg",
> - 2, flashbase, 2, flashsize,
> - 2, flashbase + flashsize, 2, flashsize);
> + 2, flashbase[0], 2, flashsize[0],
> + 2, flashbase[1], 2, flashsize[1]);
> qemu_fdt_setprop_cell(mc->fdt, name, "bank-width", 4);
> g_free(name);
> }
> --
> 2.38.0
>
>
Re: [RESEND PATCH 1/6] disas/nanomips: Move setjmp into nanomips_dis
On 6/11/22 03:37, Richard Henderson wrote: Reduce the number of local variables within the scope of the setjmp by moving it to the existing helper. The actual length returned from Disassemble is not used, because we have already determined the length while reading bytes. Fixes: nanomips.c: In function ‘print_insn_nanomips’: nanomips.c:21925:14: error: variable ‘insn1’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] nanomips.c:21925:25: error: variable ‘insn2’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] nanomips.c:21925:36: error: variable ‘insn3’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] nanomips.c:21926:22: error: variable ‘buf’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] Signed-off-by: Richard Henderson --- disas/nanomips.c | 44 1 file changed, 20 insertions(+), 24 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PULL v2 31/82] vhost: Change the sequence of device start
Hi,
On Sat, Nov 05, 2022 at 12:43:05PM -0400, Michael S. Tsirkin wrote:
> On Sat, Nov 05, 2022 at 05:35:57PM +0100, Bernhard Beschow wrote:
> >
> >
> > On Wed, Nov 2, 2022 at 5:24 PM Michael S. Tsirkin wrote:
> >
> > From: Yajun Wu
> >
> > This patch is part of adding vhost-user vhost_dev_start support. The
> > motivation is to improve backend configuration speed and reduce live
> > migration VM downtime.
> >
> > Moving the device start routines after finishing all the necessary
> > device
> > and VQ configuration, further aligning to the virtio specification for
> > "device initialization sequence".
> >
> > Following patch will add vhost-user vhost_dev_start support.
> >
> > Signed-off-by: Yajun Wu
> > Acked-by: Parav Pandit
> >
> > Message-Id: <20221017064452.1226514-2-yaj...@nvidia.com>
> > Reviewed-by: Michael S. Tsirkin
> > Signed-off-by: Michael S. Tsirkin
> > ---
> > hw/block/vhost-user-blk.c | 18 +++---
> > hw/net/vhost_net.c | 12 ++--
> > 2 files changed, 17 insertions(+), 13 deletions(-)
> >
> >
> > A git bisect tells me that this is the first bad commit for failing
> > qos-tests
> > which only fail when parallel jobs are enabled, e.g. `make check-qtest -j8`:
Parallel test run is not required provided that the test machine is
sufficiently busy (load > number of CPU threads). In this case a single
invocation of the qos test will fail reliably with this change.
However, the change is not really the root cause of the failures.
> > Summary of Failures:
> >
> > 76/541 qemu:qtest+qtest-aarch64 / qtest-aarch64/qos-test
> >
> > ERROR 18.68s killed by signal 6 SIGABRT
> > 77/541 qemu:qtest+qtest-arm / qtest-arm/qos-test
> >
> > ERROR 17.60s killed by signal 6 SIGABRT
> > 93/541 qemu:qtest+qtest-i386 / qtest-i386/qos-test
> >
> > ERROR 18.98s killed by signal 6 SIGABRT
> > 108/541 qemu:qtest+qtest-ppc64 / qtest-ppc64/qos-test
> >
> > ERROR 16.40s killed by signal 6 SIGABRT
> > 112/541 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test
> >
> > ERROR 145.94s killed by signal 6 SIGABRT
> > 130/541 qemu:qtest+qtest-x86_64 / qtest-x86_64/qos-test
> >
> > ERROR 17.32s killed by signal 6 SIGABRT
> > 243/541 qemu:qtest+qtest-x86_64 / qtest-x86_64/bios-tables-test
> >
> > ERROR 127.70s killed by signal 6 SIGABRT
> >
> > Ok: 500
> > Expected Fail: 0
> > Fail: 7
> > Unexpected Pass: 0
> > Skipped: 34
> > Timeout: 0
> >
> > Can anyone else reproduce this?
>
> Could you pls try latest for_upstream in my tree?
> That should have this fixed.
Your new pull request simply drops this change and this does fix
make check-qtest. However, this looks accidental to me and the real
bug is there in plain origin/master, too.
What happens is this backtrace a recursive call to vu_gpio_stop
via the backtrace below. It is caused by a delayed of the TCP
connection (the delayed part only triggers with heavy load on the
machine).
You can get the failure back (probably in upstream) if the test is
forced to us "use-started=off" which can be set on the command line.
E.g. like this:
diff --git a/tests/qtest/libqos/virtio-gpio.c b/tests/qtest/libqos/virtio-gpio.c
index 762aa6695b..17c6b71e8b 100644
--- a/tests/qtest/libqos/virtio-gpio.c
+++ b/tests/qtest/libqos/virtio-gpio.c
@@ -154,14 +154,14 @@ static void virtio_gpio_register_nodes(void)
QOSGraphEdgeOptions edge_opts = { };
/* vhost-user-gpio-device */
-edge_opts.extra_device_opts = "id=gpio0,chardev=chr-vhost-user-test";
+edge_opts.extra_device_opts =
"id=gpio0,chardev=chr-vhost-user-test,use-started=off";
qos_node_create_driver("vhost-user-gpio-device",
virtio_gpio_device_create);
qos_node_consumes("vhost-user-gpio-device", "virtio-bus", _opts);
qos_node_produces("vhost-user-gpio-device", "vhost-user-gpio");
/* virtio-gpio-pci */
-edge_opts.extra_device_opts =
"id=gpio0,addr=04.0,chardev=chr-vhost-user-test";
+edge_opts.extra_device_opts =
"id=gpio0,addr=04.0,chardev=chr-vhost-user-test,use-started=on";
add_qpci_address(_opts, );
qos_node_create_driver("vhost-user-gpio-pci", virtio_gpio_pci_create);
qos_node_consumes("vhost-user-gpio-pci", "pci-bus", _opts);
I haven't verified this but from looking at the code other types of
vhost devices seem to have the same problem (e.g. vhost-user-i2c looks
suspicious).
Ok, here's the backtrace:
#0 vu_gpio_stop (vdev=vdev@entry=0x560e0ae449d0) at
../hw/virtio/vhost-user-gpio.c:143
#1 0x560e0768fb1f in vu_gpio_disconnect (dev=) at
../hw/virtio/vhost-user-gpio.c:260
#2 vu_gpio_event
Re: [RESEND PATCH 4/6] disas/nanomips: Tidy read for 48-bit opcodes
On 6/11/22 03:37, Richard Henderson wrote: There is no point in looking for a 48-bit opcode if we've not read the second word for a 32-bit opcode. Signed-off-by: Richard Henderson --- disas/nanomips.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [RESEND PATCH 5/6] tcg: Move TCG_TARGET_HAS_direct_jump init to tb_gen_code
On 6/11/22 03:37, Richard Henderson wrote: Signed-off-by: Richard Henderson --- accel/tcg/translate-all.c | 10 -- tcg/tcg.c | 12 2 files changed, 12 insertions(+), 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [RESEND PATCH 6/6] accel/tcg: Split out setjmp_gen_code
On 6/11/22 03:37, Richard Henderson wrote: Isolate the code protected by setjmp. Fixes: translate-all.c: In function ‘tb_gen_code’: translate-all.c:748:51: error: argument ‘cflags’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] Signed-off-by: Richard Henderson --- accel/tcg/translate-all.c | 58 ++- 1 file changed, 33 insertions(+), 25 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [RESEND PATCH 3/6] disas/nanomips: Split out read_u16
On 6/11/22 03:37, Richard Henderson wrote: Split out a helper function for reading a uint16_t with the correct endianness. Eh I was thinking about that when reviewing the previous patch :) Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- disas/nanomips.c | 48 +++- 1 file changed, 19 insertions(+), 29 deletions(-)
Re: [RESEND PATCH 2/6] disas/nanomips: Merge insn{1,2,3} into words[3]
On 6/11/22 03:37, Richard Henderson wrote:
Since Disassemble wants the data in this format, collect
it that way. This allows using a loop to print the bytes.
Signed-off-by: Richard Henderson
---
disas/nanomips.c | 44 +---
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/disas/nanomips.c b/disas/nanomips.c
index 9a69e6880a..5438def9af 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21905,12 +21905,8 @@ static const Pool MAJOR[2] = {
0x0 },/* P16 */
};
-static bool nanomips_dis(char **buf, Dis_info *info,
- unsigned short one,
- unsigned short two,
- unsigned short three)
+static bool nanomips_dis(char **buf, Dis_info *info, uint16_t words[3])
words[] can be const.
+for (int i = 0; i < 6; i += 2) {
I'd rather convert this magic 6 and iterate over ARRAY_SIZE(words).
Anyhow,
Reviewed-by: Philippe Mathieu-Daudé
+if (i < length) {
+(*info->fprintf_func)(info->stream, "%04x ", words[i / 2]);
+} else {
+(*info->fprintf_func)(info->stream, " ");
+}
}
[PULL 03/12] tests/qtest/e1000e-test: Use e1000_regs.h
From: Akihiko Odaki
The register definitions in tests/qtest/e1000e-test.c had names
different from hw/net/e1000_regs.h, which made it hard to understand
what test codes corresponds to the implementation. Use
hw/net/e1000_regs.h from tests/qtest/libqos/e1000e.c to remove
these duplications.
Signed-off-by: Akihiko Odaki
Message-Id: <20221103095416.110162-1-akihiko.od...@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Thomas Huth
---
tests/qtest/e1000e-test.c | 66 ++-
1 file changed, 10 insertions(+), 56 deletions(-)
diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c
index 4cdd8238f2..08adc5226d 100644
--- a/tests/qtest/e1000e-test.c
+++ b/tests/qtest/e1000e-test.c
@@ -33,34 +33,11 @@
#include "qemu/bitops.h"
#include "libqos/libqos-malloc.h"
#include "libqos/e1000e.h"
+#include "hw/net/e1000_regs.h"
static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator
*alloc)
{
-struct {
-uint64_t buffer_addr;
-union {
-uint32_t data;
-struct {
-uint16_t length;
-uint8_t cso;
-uint8_t cmd;
-} flags;
-} lower;
-union {
-uint32_t data;
-struct {
-uint8_t status;
-uint8_t css;
-uint16_t special;
-} fields;
-} upper;
-} descr;
-
-static const uint32_t dtyp_data = BIT(20);
-static const uint32_t dtyp_ext = BIT(29);
-static const uint32_t dcmd_rs = BIT(27);
-static const uint32_t dcmd_eop = BIT(24);
-static const uint32_t dsta_dd = BIT(0);
+struct e1000_tx_desc descr;
static const int data_len = 64;
char buffer[64];
int ret;
@@ -73,10 +50,10 @@ static void e1000e_send_verify(QE1000E *d, int
*test_sockets, QGuestAllocator *a
/* Prepare TX descriptor */
memset(, 0, sizeof(descr));
descr.buffer_addr = cpu_to_le64(data);
-descr.lower.data = cpu_to_le32(dcmd_rs |
- dcmd_eop |
- dtyp_ext |
- dtyp_data |
+descr.lower.data = cpu_to_le32(E1000_TXD_CMD_RS |
+ E1000_TXD_CMD_EOP |
+ E1000_TXD_CMD_DEXT |
+ E1000_TXD_DTYP_D |
data_len);
/* Put descriptor to the ring */
@@ -86,7 +63,8 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets,
QGuestAllocator *a
e1000e_wait_isr(d, E1000E_TX0_MSG_ID);
/* Check DD bit */
-g_assert_cmphex(le32_to_cpu(descr.upper.data) & dsta_dd, ==, dsta_dd);
+g_assert_cmphex(le32_to_cpu(descr.upper.data) & E1000_TXD_STAT_DD, ==,
+E1000_TXD_STAT_DD);
/* Check data sent to the backend */
ret = recv(test_sockets[0], _len, sizeof(recv_len), 0);
@@ -101,31 +79,7 @@ static void e1000e_send_verify(QE1000E *d, int
*test_sockets, QGuestAllocator *a
static void e1000e_receive_verify(QE1000E *d, int *test_sockets,
QGuestAllocator *alloc)
{
-union {
-struct {
-uint64_t buffer_addr;
-uint64_t reserved;
-} read;
-struct {
-struct {
-uint32_t mrq;
-union {
-uint32_t rss;
-struct {
-uint16_t ip_id;
-uint16_t csum;
-} csum_ip;
-} hi_dword;
-} lower;
-struct {
-uint32_t status_error;
-uint16_t length;
-uint16_t vlan;
-} upper;
-} wb;
-} descr;
-
-static const uint32_t esta_dd = BIT(0);
+union e1000_rx_desc_extended descr;
char test[] = "TEST";
int len = htonl(sizeof(test));
@@ -162,7 +116,7 @@ static void e1000e_receive_verify(QE1000E *d, int
*test_sockets, QGuestAllocator
/* Check DD bit */
g_assert_cmphex(le32_to_cpu(descr.wb.upper.status_error) &
-esta_dd, ==, esta_dd);
+E1000_RXD_STAT_DD, ==, E1000_RXD_STAT_DD);
/* Check data sent to the backend */
memread(data, buffer, sizeof(buffer));
--
2.31.1
[PULL 12/12] s390x/cpu topology: add max_threads machine class attribute
From: Pierre Morel
The S390 CPU topology accepts the smp.threads argument while
in reality it does not effectively allow multthreading.
Let's keep this behavior for machines older than 7.2 and
refuse to use threads in newer machines until multithreading
is really exposed to the guest by the machine.
Signed-off-by: Pierre Morel
Message-Id: <20221103170150.20789-3-pmo...@linux.ibm.com>
[thuth: Small fixes to the commit description]
Signed-off-by: Thomas Huth
---
include/hw/s390x/s390-virtio-ccw.h | 1 +
hw/s390x/s390-virtio-ccw.c | 11 +++
2 files changed, 12 insertions(+)
diff --git a/include/hw/s390x/s390-virtio-ccw.h
b/include/hw/s390x/s390-virtio-ccw.h
index 8a0090a071..4f8a39abda 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -40,6 +40,7 @@ struct S390CcwMachineClass {
bool cpu_model_allowed;
bool css_migration_enabled;
bool hpage_1m_allowed;
+int max_threads;
};
/* runtime-instrumentation allowed by the machine */
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 196773c833..560ddbb6fb 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -85,8 +85,15 @@ out:
static void s390_init_cpus(MachineState *machine)
{
MachineClass *mc = MACHINE_GET_CLASS(machine);
+S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
int i;
+if (machine->smp.threads > s390mc->max_threads) {
+error_report("S390 does not support more than %d threads.",
+ s390mc->max_threads);
+exit(1);
+}
+
/* initialize possible_cpus */
mc->possible_cpu_arch_ids(machine);
@@ -731,6 +738,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void
*data)
s390mc->cpu_model_allowed = true;
s390mc->css_migration_enabled = true;
s390mc->hpage_1m_allowed = true;
+s390mc->max_threads = 1;
mc->init = ccw_init;
mc->reset = s390_machine_reset;
mc->block_default_type = IF_VIRTIO;
@@ -859,8 +867,11 @@ static void ccw_machine_7_1_instance_options(MachineState
*machine)
static void ccw_machine_7_1_class_options(MachineClass *mc)
{
+S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
+
ccw_machine_7_2_class_options(mc);
compat_props_add(mc->compat_props, hw_compat_7_1, hw_compat_7_1_len);
+s390mc->max_threads = S390_MAX_CPUS;
}
DEFINE_CCW_MACHINE(7_1, "7.1", false);
--
2.31.1
[PULL 08/12] gitlab-ci: increase clang-user timeout
From: Stefan Hajnoczi The clang-user test exceeds the 1 hour timeout occassionally. Philippe Mathieu-Daudé has pointed out that the number of tcg tests has increased since QEMU 7.1. The execution time therefore probably reflects a legitimate increase in tests rather than a performance regression. Bump the timeout to prevent CI failures. Suggested-by: Thomas Huth Signed-off-by: Stefan Hajnoczi Reviewed-by: Thomas Huth Message-Id: <20221104113659.427690-1-stefa...@redhat.com> Signed-off-by: Thomas Huth --- .gitlab-ci.d/buildtest.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 6c05c46397..7173749c52 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -327,6 +327,7 @@ clang-user: extends: .native_build_job_template needs: job: amd64-debian-user-cross-container + timeout: 70m variables: IMAGE: debian-all-test-cross CONFIGURE_ARGS: --cc=clang --cxx=clang++ --disable-system -- 2.31.1
[PULL 06/12] tests/qtest: Fix two format strings
From: Stefan Weil
Signed-off-by: Stefan Weil
Message-Id: <20221105115525.623059-1...@weilnetz.de>
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Thomas Huth
---
tests/qtest/migration-test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d2eb107f0c..f574331b7b 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2188,7 +2188,7 @@ static void calc_dirty_rate(QTestState *who, uint64_t
calc_time)
qobject_unref(qmp_command(who,
"{ 'execute': 'calc-dirty-rate',"
"'arguments': { "
- "'calc-time': %ld,"
+ "'calc-time': %" PRIu64 ","
"'mode': 'dirty-ring' }}",
calc_time));
}
@@ -2203,7 +2203,7 @@ static void dirtylimit_set_all(QTestState *who, uint64_t
dirtyrate)
qobject_unref(qmp_command(who,
"{ 'execute': 'set-vcpu-dirty-limit',"
"'arguments': { "
- "'dirty-rate': %ld } }",
+ "'dirty-rate': %" PRIu64 " } }",
dirtyrate));
}
--
2.31.1
[PULL 02/12] tests/qtest/libqos/e1000e: Set E1000_CTRL_SLU
From: Akihiko Odaki The later device status check depends on E1000_STATUS_LU, which is enabled by E1000_CTRL_SLU. Though E1000_STATUS_LU is not implemented and E1000_STATUS_LU is always available in the current implementation, be a bit nicer and set E1000_CTRL_SLU just in case the bit is implemented in the future. Signed-off-by: Akihiko Odaki Message-Id: <20221103025451.27446-1-akihiko.od...@daynix.com> Signed-off-by: Thomas Huth --- tests/qtest/libqos/e1000e.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index 5f80035859..4fd0bd5311 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -122,7 +122,7 @@ static void e1000e_pci_start_hw(QOSGraphObject *obj) /* Reset the device */ val = e1000e_macreg_read(>e1000e, E1000_CTRL); -e1000e_macreg_write(>e1000e, E1000_CTRL, val | E1000_CTRL_RST); +e1000e_macreg_write(>e1000e, E1000_CTRL, val | E1000_CTRL_RST | E1000_CTRL_SLU); /* Enable and configure MSI-X */ qpci_msix_enable(>pci_dev); -- 2.31.1
[PULL 10/12] s390x/pci: RPCIT second pass when mappings exhausted
From: Matthew Rosato
If we encounter a new mapping while the number of available DMA entries
in vfio is 0, we are currently skipping that mapping which is a problem
if we manage to free up DMA space after that within the same RPCIT --
we will return to the guest with CC0 and have not mapped everything
within the specified range. This issue was uncovered while testing
changes to the s390 linux kernel iommu/dma code, where a different
usage pattern was employed (new mappings start at the end of the
aperture and work back towards the front, making us far more likely
to encounter new mappings before invalidated mappings during a
global refresh).
Fix this by tracking whether any mappings were skipped due to vfio
DMA limit hitting 0; when this occurs, we still continue the range
and unmap/map anything we can - then we must re-run the range again
to pickup anything that was missed. This must occur in a loop until
all requests are satisfied (success) or we detect that we are still
unable to complete all mappings (return ZPCI_RPCIT_ST_INSUFF_RES).
Link:
https://lore.kernel.org/linux-s390/20221019144435.369902-1-schne...@linux.ibm.com/
Fixes: 37fa32de70 ("s390x/pci: Honor DMA limits set by vfio")
Reported-by: Niklas Schnelle
Signed-off-by: Matthew Rosato
Message-Id: <20221028194758.204007-2-mjros...@linux.ibm.com>
Reviewed-by: Eric Farman
Signed-off-by: Thomas Huth
---
hw/s390x/s390-pci-inst.c | 29 ++---
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 20a9bcc7af..7cc4bcf850 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -677,8 +677,9 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2, uintptr_t ra)
S390PCIBusDevice *pbdev;
S390PCIIOMMU *iommu;
S390IOTLBEntry entry;
-hwaddr start, end;
+hwaddr start, end, sstart;
uint32_t dma_avail;
+bool again;
if (env->psw.mask & PSW_MASK_PSTATE) {
s390_program_interrupt(env, PGM_PRIVILEGED, ra);
@@ -691,7 +692,7 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2, uintptr_t ra)
}
fh = env->regs[r1] >> 32;
-start = env->regs[r2];
+sstart = start = env->regs[r2];
end = start + env->regs[r2 + 1];
pbdev = s390_pci_find_dev_by_fh(s390_get_phb(), fh);
@@ -732,6 +733,9 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2, uintptr_t ra)
goto err;
}
+ retry:
+start = sstart;
+again = false;
while (start < end) {
error = s390_guest_io_table_walk(iommu->g_iota, start, );
if (error) {
@@ -739,13 +743,24 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2, uintptr_t ra)
}
start += entry.len;
-while (entry.iova < start && entry.iova < end &&
- (dma_avail > 0 || entry.perm == IOMMU_NONE)) {
-dma_avail = s390_pci_update_iotlb(iommu, );
-entry.iova += TARGET_PAGE_SIZE;
-entry.translated_addr += TARGET_PAGE_SIZE;
+while (entry.iova < start && entry.iova < end) {
+if (dma_avail > 0 || entry.perm == IOMMU_NONE) {
+dma_avail = s390_pci_update_iotlb(iommu, );
+entry.iova += TARGET_PAGE_SIZE;
+entry.translated_addr += TARGET_PAGE_SIZE;
+} else {
+/*
+ * We are unable to make a new mapping at this time, continue
+ * on and hopefully free up more space. Then attempt another
+ * pass.
+ */
+again = true;
+break;
+}
}
}
+if (again && dma_avail > 0)
+goto retry;
err:
if (error) {
pbdev->state = ZPCI_FS_ERROR;
--
2.31.1
[PULL 11/12] s390x: Register TYPE_S390_CCW_MACHINE properties as class properties
From: Pierre Morel
Currently, when running 'qemu-system-s390x -M s390-ccw-virtio,help'
the s390x-specific properties are not listed anymore. This happens
because since commit d8fb7d0969 ("vl: switch -M parsing to keyval")
the properties have to be defined at the class level and not at the
instance level anymore. Fix it on s390x now, too, by moving the
registration of the properties to the class level"
Fixes: d8fb7d0969 ("vl: switch -M parsing to keyval")
Signed-off-by: Pierre Morel
Message-Id: <20221103170150.20789-2-pmo...@linux.ibm.com>
[thuth: Add patch description]
Signed-off-by: Thomas Huth
---
hw/s390x/s390-virtio-ccw.c | 127 +
1 file changed, 72 insertions(+), 55 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 806de32034..196773c833 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -43,6 +43,7 @@
#include "sysemu/sysemu.h"
#include "hw/s390x/pv.h"
#include "migration/blocker.h"
+#include "qapi/visitor.h"
static Error *pv_mig_blocker;
@@ -589,38 +590,6 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
return newsz;
}
-static void ccw_machine_class_init(ObjectClass *oc, void *data)
-{
-MachineClass *mc = MACHINE_CLASS(oc);
-NMIClass *nc = NMI_CLASS(oc);
-HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
-S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
-
-s390mc->ri_allowed = true;
-s390mc->cpu_model_allowed = true;
-s390mc->css_migration_enabled = true;
-s390mc->hpage_1m_allowed = true;
-mc->init = ccw_init;
-mc->reset = s390_machine_reset;
-mc->block_default_type = IF_VIRTIO;
-mc->no_cdrom = 1;
-mc->no_floppy = 1;
-mc->no_parallel = 1;
-mc->no_sdcard = 1;
-mc->max_cpus = S390_MAX_CPUS;
-mc->has_hotpluggable_cpus = true;
-assert(!mc->get_hotplug_handler);
-mc->get_hotplug_handler = s390_get_hotplug_handler;
-mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
-mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;
-/* it is overridden with 'host' cpu *in kvm_arch_init* */
-mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu");
-hc->plug = s390_machine_device_plug;
-hc->unplug_request = s390_machine_device_unplug_request;
-nc->nmi_monitor_handler = s390_nmi;
-mc->default_ram_id = "s390.ram";
-}
-
static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
{
S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -710,19 +679,29 @@ bool hpage_1m_allowed(void)
return get_machine_class()->hpage_1m_allowed;
}
-static char *machine_get_loadparm(Object *obj, Error **errp)
+static void machine_get_loadparm(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
{
S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
+char *str = g_strndup((char *) ms->loadparm, sizeof(ms->loadparm));
-/* make a NUL-terminated string */
-return g_strndup((char *) ms->loadparm, sizeof(ms->loadparm));
+visit_type_str(v, name, , errp);
+g_free(str);
}
-static void machine_set_loadparm(Object *obj, const char *val, Error **errp)
+static void machine_set_loadparm(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
{
S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
+char *val;
int i;
+if (!visit_type_str(v, name, , errp)) {
+return;
+}
+
for (i = 0; i < sizeof(ms->loadparm) && val[i]; i++) {
uint8_t c = qemu_toupper(val[i]); /* mimic HMC */
@@ -740,34 +719,72 @@ static void machine_set_loadparm(Object *obj, const char
*val, Error **errp)
ms->loadparm[i] = ' '; /* pad right with spaces */
}
}
-static inline void s390_machine_initfn(Object *obj)
+
+static void ccw_machine_class_init(ObjectClass *oc, void *data)
{
-object_property_add_bool(obj, "aes-key-wrap",
- machine_get_aes_key_wrap,
- machine_set_aes_key_wrap);
-object_property_set_description(obj, "aes-key-wrap",
+MachineClass *mc = MACHINE_CLASS(oc);
+NMIClass *nc = NMI_CLASS(oc);
+HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
+
+s390mc->ri_allowed = true;
+s390mc->cpu_model_allowed = true;
+s390mc->css_migration_enabled = true;
+s390mc->hpage_1m_allowed = true;
+mc->init = ccw_init;
+mc->reset = s390_machine_reset;
+mc->block_default_type = IF_VIRTIO;
+mc->no_cdrom = 1;
+mc->no_floppy = 1;
+mc->no_parallel = 1;
+mc->no_sdcard = 1;
+mc->max_cpus = S390_MAX_CPUS;
+mc->has_hotpluggable_cpus = true;
+assert(!mc->get_hotplug_handler);
+mc->get_hotplug_handler = s390_get_hotplug_handler;
+mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
[PULL 09/12] s390x/css: revert SCSW ctrl/flag bits on error
From: Peter Jin
Revert the control and flag bits in the subchannel status word in case
the SSCH operation fails with non-zero CC (ditto for CSCH and HSCH).
According to POPS, the control and flag bits are only changed if SSCH,
CSCH, and HSCH return CC 0, and no other action should be taken otherwise.
In order to simulate that after the fact, the bits need to be reverted on
non-zero CC.
While the do_subchannel_work logic for virtual (virtio) devices will
return condition code 0, passthrough (vfio) devices may encounter
errors from either the host kernel or real hardware that need to be
accounted for after this point. This includes restoring the state of
the Subchannel Status Word to reflect the subchannel, as these bits
would not be set in the event of a non-zero condition code from the
affected instructions.
Experimentation has shown that a failure on a START SUBCHANNEL (SSCH)
to a passthrough device would leave the subchannel with the START
PENDING activity control bit set, thus blocking subsequent SSCH
operations in css_do_ssch() until some form of error recovery was
undertaken since no interrupt would be expected.
Signed-off-by: Peter Jin
Message-Id: <20221027212341.2904795-1-p...@linux.ibm.com>
Reviewed-by: Eric Farman
Reviewed-by: Matthew Rosato
[thuth: Updated the commit description to Eric's suggestion]
Signed-off-by: Thomas Huth
---
hw/s390x/css.c | 51 +++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 7d9523f811..95d1b3a3ce 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1522,21 +1522,37 @@ IOInstEnding css_do_xsch(SubchDev *sch)
IOInstEnding css_do_csch(SubchDev *sch)
{
SCHIB *schib = >curr_status;
+uint16_t old_scsw_ctrl;
+IOInstEnding ccode;
if (~(schib->pmcw.flags) & (PMCW_FLAGS_MASK_DNV | PMCW_FLAGS_MASK_ENA)) {
return IOINST_CC_NOT_OPERATIONAL;
}
+/*
+ * Save the current scsw.ctrl in case CSCH fails and we need
+ * to revert the scsw to the status quo ante.
+ */
+old_scsw_ctrl = schib->scsw.ctrl;
+
/* Trigger the clear function. */
schib->scsw.ctrl &= ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL);
schib->scsw.ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_ACTL_CLEAR_PEND;
-return do_subchannel_work(sch);
+ccode = do_subchannel_work(sch);
+
+if (ccode != IOINST_CC_EXPECTED) {
+schib->scsw.ctrl = old_scsw_ctrl;
+}
+
+return ccode;
}
IOInstEnding css_do_hsch(SubchDev *sch)
{
SCHIB *schib = >curr_status;
+uint16_t old_scsw_ctrl;
+IOInstEnding ccode;
if (~(schib->pmcw.flags) & (PMCW_FLAGS_MASK_DNV | PMCW_FLAGS_MASK_ENA)) {
return IOINST_CC_NOT_OPERATIONAL;
@@ -1553,6 +1569,12 @@ IOInstEnding css_do_hsch(SubchDev *sch)
return IOINST_CC_BUSY;
}
+/*
+ * Save the current scsw.ctrl in case HSCH fails and we need
+ * to revert the scsw to the status quo ante.
+ */
+old_scsw_ctrl = schib->scsw.ctrl;
+
/* Trigger the halt function. */
schib->scsw.ctrl |= SCSW_FCTL_HALT_FUNC;
schib->scsw.ctrl &= ~SCSW_FCTL_START_FUNC;
@@ -1564,7 +1586,13 @@ IOInstEnding css_do_hsch(SubchDev *sch)
}
schib->scsw.ctrl |= SCSW_ACTL_HALT_PEND;
-return do_subchannel_work(sch);
+ccode = do_subchannel_work(sch);
+
+if (ccode != IOINST_CC_EXPECTED) {
+schib->scsw.ctrl = old_scsw_ctrl;
+}
+
+return ccode;
}
static void css_update_chnmon(SubchDev *sch)
@@ -1605,6 +1633,8 @@ static void css_update_chnmon(SubchDev *sch)
IOInstEnding css_do_ssch(SubchDev *sch, ORB *orb)
{
SCHIB *schib = >curr_status;
+uint16_t old_scsw_ctrl, old_scsw_flags;
+IOInstEnding ccode;
if (~(schib->pmcw.flags) & (PMCW_FLAGS_MASK_DNV | PMCW_FLAGS_MASK_ENA)) {
return IOINST_CC_NOT_OPERATIONAL;
@@ -1626,11 +1656,26 @@ IOInstEnding css_do_ssch(SubchDev *sch, ORB *orb)
}
sch->orb = *orb;
sch->channel_prog = orb->cpa;
+
+/*
+ * Save the current scsw.ctrl and scsw.flags in case SSCH fails and we need
+ * to revert the scsw to the status quo ante.
+ */
+old_scsw_ctrl = schib->scsw.ctrl;
+old_scsw_flags = schib->scsw.flags;
+
/* Trigger the start function. */
schib->scsw.ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND);
schib->scsw.flags &= ~SCSW_FLAGS_MASK_PNO;
-return do_subchannel_work(sch);
+ccode = do_subchannel_work(sch);
+
+if (ccode != IOINST_CC_EXPECTED) {
+schib->scsw.ctrl = old_scsw_ctrl;
+schib->scsw.flags = old_scsw_flags;
+}
+
+return ccode;
}
static void copy_irb_to_guest(IRB *dest, const IRB *src, const PMCW *pmcw,
--
2.31.1
[PULL 00/12] qtest and s390x patches
Hi Stefan! The following changes since commit 6295a58ad1b73985b9c32d184de7d2ed1fbe1774: Merge tag 'pull-target-arm-20221104' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-11-04 11:01:17 -0400) are available in the Git repository at: https://gitlab.com/thuth/qemu.git tags/pull-request-2022-11-06 for you to fetch changes up to 6393b29966fce3c0e47746a9646ae439e7fd0728: s390x/cpu topology: add max_threads machine class attribute (2022-11-06 12:38:57 +0100) * e1000e qtest improvements * Allow TLS PSK tests on win32 * Increase the timeout of the clang-user CI job * Some s390x fixes for QEMU 7.2 Akihiko Odaki (5): tests/qtest/libqos/e1000e: Refer common PCI ID definitions tests/qtest/libqos/e1000e: Set E1000_CTRL_SLU tests/qtest/e1000e-test: Use e1000_regs.h tests/qtest/libqos/e1000e: Use E1000_STATUS_ASDV_1000 tests/qtest/libqos/e1000e: Use IVAR shift definitions Bin Meng (1): tests/qtest: migration-test: Enable TLS PSK tests for win32 Matthew Rosato (1): s390x/pci: RPCIT second pass when mappings exhausted Peter Jin (1): s390x/css: revert SCSW ctrl/flag bits on error Pierre Morel (2): s390x: Register TYPE_S390_CCW_MACHINE properties as class properties s390x/cpu topology: add max_threads machine class attribute Stefan Hajnoczi (1): gitlab-ci: increase clang-user timeout Stefan Weil (1): tests/qtest: Fix two format strings include/hw/s390x/s390-virtio-ccw.h | 1 + hw/s390x/css.c | 51 +- hw/s390x/s390-pci-inst.c | 29 ++-- hw/s390x/s390-virtio-ccw.c | 138 ++--- tests/qtest/e1000e-test.c | 66 +++--- tests/qtest/libqos/e1000e.c| 17 ++--- tests/qtest/migration-test.c | 18 + .gitlab-ci.d/buildtest.yml | 1 + 8 files changed, 176 insertions(+), 145 deletions(-)
[PULL 04/12] tests/qtest/libqos/e1000e: Use E1000_STATUS_ASDV_1000
From: Akihiko Odaki Nemonics E1000_STATUS_LAN_INIT_DONE and E1000_STATUS_ASDV_1000 have the same value, and E1000_STATUS_ASDV_1000 should be used here because E1000_STATUS_ASDV_1000 represents the auto-detected speed tested here while E1000_STATUS_LAN_INIT_DONE is a value used for a different purpose with a variant of e1000e family different from the one implemented in QEMU. Signed-off-by: Akihiko Odaki Message-Id: <20221103083425.100590-1-akihiko.od...@daynix.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/libqos/e1000e.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index 4fd0bd5311..05af6f2118 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -130,8 +130,8 @@ static void e1000e_pci_start_hw(QOSGraphObject *obj) /* Check the device status - link and speed */ val = e1000e_macreg_read(>e1000e, E1000_STATUS); -g_assert_cmphex(val & (E1000_STATUS_LU | E1000_STATUS_LAN_INIT_DONE), -==, E1000_STATUS_LU | E1000_STATUS_LAN_INIT_DONE); +g_assert_cmphex(val & (E1000_STATUS_LU | E1000_STATUS_ASDV_1000), +==, E1000_STATUS_LU | E1000_STATUS_ASDV_1000); /* Initialize TX/RX logic */ e1000e_macreg_write(>e1000e, E1000_RCTL, 0); -- 2.31.1
[PULL 07/12] tests/qtest: migration-test: Enable TLS PSK tests for win32
From: Bin Meng
Since commit f1018ea0a30f ("tests: avoid DOS line endings in PSK file"),
the bug of the helper test_tls_psk_init_common() that caused TLS PSK
tests to fail on Windows was fixed. Let's enable these tests on win32.
Signed-off-by: Bin Meng
Message-Id: <20221101035021.729669-1-bin.m...@windriver.com>
Signed-off-by: Thomas Huth
---
tests/qtest/migration-test.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f574331b7b..442998d9eb 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1402,7 +1402,6 @@ static void test_precopy_unix_dirty_ring(void)
}
#ifdef CONFIG_GNUTLS
-#ifndef _WIN32
static void test_precopy_unix_tls_psk(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -1415,7 +1414,6 @@ static void test_precopy_unix_tls_psk(void)
test_precopy_common();
}
-#endif /* _WIN32 */
#ifdef CONFIG_TASN1
static void test_precopy_unix_tls_x509_default_host(void)
@@ -1524,7 +1522,6 @@ static void test_precopy_tcp_plain(void)
}
#ifdef CONFIG_GNUTLS
-#ifndef _WIN32
static void test_precopy_tcp_tls_psk_match(void)
{
MigrateCommon args = {
@@ -1535,7 +1532,6 @@ static void test_precopy_tcp_tls_psk_match(void)
test_precopy_common();
}
-#endif /* _WIN32 */
static void test_precopy_tcp_tls_psk_mismatch(void)
{
@@ -1933,7 +1929,6 @@ static void test_multifd_tcp_zstd(void)
#endif
#ifdef CONFIG_GNUTLS
-#ifndef _WIN32
static void *
test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
QTestState *to)
@@ -1941,7 +1936,6 @@ test_migrate_multifd_tcp_tls_psk_start_match(QTestState
*from,
test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
return test_migrate_tls_psk_start_match(from, to);
}
-#endif /* _WIN32 */
static void *
test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
@@ -1993,7 +1987,6 @@
test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from,
}
#endif /* CONFIG_TASN1 */
-#ifndef _WIN32
static void test_multifd_tcp_tls_psk_match(void)
{
MigrateCommon args = {
@@ -2003,7 +1996,6 @@ static void test_multifd_tcp_tls_psk_match(void)
};
test_precopy_common();
}
-#endif /* _WIN32 */
static void test_multifd_tcp_tls_psk_mismatch(void)
{
@@ -2505,10 +2497,8 @@ int main(int argc, char **argv)
qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle);
#ifdef CONFIG_GNUTLS
-#ifndef _WIN32
qtest_add_func("/migration/precopy/unix/tls/psk",
test_precopy_unix_tls_psk);
-#endif
if (has_uffd) {
/*
@@ -2534,10 +2524,8 @@ int main(int argc, char **argv)
qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
#ifdef CONFIG_GNUTLS
-#ifndef _WIN32
qtest_add_func("/migration/precopy/tcp/tls/psk/match",
test_precopy_tcp_tls_psk_match);
-#endif
qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
test_precopy_tcp_tls_psk_mismatch);
#ifdef CONFIG_TASN1
@@ -2581,10 +2569,8 @@ int main(int argc, char **argv)
test_multifd_tcp_zstd);
#endif
#ifdef CONFIG_GNUTLS
-#ifndef _WIN32
qtest_add_func("/migration/multifd/tcp/tls/psk/match",
test_multifd_tcp_tls_psk_match);
-#endif
qtest_add_func("/migration/multifd/tcp/tls/psk/mismatch",
test_multifd_tcp_tls_psk_mismatch);
#ifdef CONFIG_TASN1
--
2.31.1
[PULL 05/12] tests/qtest/libqos/e1000e: Use IVAR shift definitions
From: Akihiko Odaki There were still some constants defined in e1000_regs.h. Signed-off-by: Akihiko Odaki Message-Id: <20221105053010.38037-1-akihiko.od...@daynix.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/libqos/e1000e.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index 05af6f2118..80b3e3db90 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -30,9 +30,9 @@ #include "e1000e.h" #define E1000E_IVAR_TEST_CFG \ -(E1000E_RX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID | \ - ((E1000E_TX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << 8)| \ - ((E1000E_OTHER_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << 16) | \ +(((E1000E_RX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << E1000_IVAR_RXQ0_SHIFT) | \ + ((E1000E_TX0_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << E1000_IVAR_TXQ0_SHIFT) | \ + ((E1000E_OTHER_MSG_ID | E1000_IVAR_INT_ALLOC_VALID) << E1000_IVAR_OTHER_SHIFT) | \ E1000_IVAR_TX_INT_EVERY_WB) #define E1000E_RING_LEN (0x1000) -- 2.31.1
[PULL 01/12] tests/qtest/libqos/e1000e: Refer common PCI ID definitions
From: Akihiko Odaki
This is yet another minor cleanup to ease understanding and
future refactoring of the tests.
Signed-off-by: Akihiko Odaki
Message-Id: <20221103015017.19947-1-akihiko.od...@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Thomas Huth
---
tests/qtest/libqos/e1000e.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c
index ed47e34044..5f80035859 100644
--- a/tests/qtest/libqos/e1000e.c
+++ b/tests/qtest/libqos/e1000e.c
@@ -18,6 +18,7 @@
#include "qemu/osdep.h"
#include "hw/net/e1000_regs.h"
+#include "hw/pci/pci_ids.h"
#include "../libqtest.h"
#include "pci-pc.h"
#include "qemu/sockets.h"
@@ -217,8 +218,8 @@ static void *e1000e_pci_create(void *pci_bus,
QGuestAllocator *alloc,
static void e1000e_register_nodes(void)
{
QPCIAddress addr = {
-.vendor_id = 0x8086,
-.device_id = 0x10D3,
+.vendor_id = PCI_VENDOR_ID_INTEL,
+.device_id = E1000_DEV_ID_82574L,
};
/* FIXME: every test using this node needs to setup a -netdev socket,id=hs0
--
2.31.1
Re: [PATCH] hw/riscv: virt: Remove size restriction for pflash
On Sun, Nov 06, 2022 at 08:09:00PM +0530, Sunil V L wrote:
> The pflash implementation currently assumes fixed size of the
> backend storage. Due to this, the backend storage file needs to be
> exactly of size 32M. Otherwise, there will be an error like below.
>
> "device requires 33554432 bytes, block backend provides 3145728 bytes"
>
> Fix this issue by using the actual size of the backing store.
>
> Signed-off-by: Sunil V L
> ---
> hw/riscv/virt.c | 33 +
> 1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a5bc7353b4..aad175fa31 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -49,6 +49,7 @@
> #include "hw/pci/pci.h"
> #include "hw/pci-host/gpex.h"
> #include "hw/display/ramfb.h"
> +#include "sysemu/block-backend.h"
>
> /*
> * The virt machine physical address space used by some of the devices
> @@ -144,10 +145,17 @@ static void virt_flash_map1(PFlashCFI01 *flash,
> MemoryRegion *sysmem)
> {
> DeviceState *dev = DEVICE(flash);
> +BlockBackend *blk;
> +hwaddr real_size;
>
> -assert(QEMU_IS_ALIGNED(size, VIRT_FLASH_SECTOR_SIZE));
> -assert(size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);
> -qdev_prop_set_uint32(dev, "num-blocks", size / VIRT_FLASH_SECTOR_SIZE);
> +blk = pflash_cfi01_get_blk(flash);
> +
> +real_size = blk ? blk_getlength(blk): size;
> +
> +assert(real_size);
> +assert(QEMU_IS_ALIGNED(real_size, VIRT_FLASH_SECTOR_SIZE));
> +assert(real_size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);
> +qdev_prop_set_uint32(dev, "num-blocks", real_size /
> VIRT_FLASH_SECTOR_SIZE);
> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
>
> memory_region_add_subregion(sysmem, base,
> @@ -971,15 +979,24 @@ static void create_fdt_flash(RISCVVirtState *s, const
> MemMapEntry *memmap)
> {
> char *name;
> MachineState *mc = MACHINE(s);
> -hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
> -hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
> +MemoryRegion *flash_mem;
> +hwaddr flashsize[2];
> +hwaddr flashbase[2];
> +
> +flash_mem = pflash_cfi01_get_memory(s->flash[0]);
> +flashbase[0] = flash_mem->addr;
> +flashsize[0] = flash_mem->size;
> +
> +flash_mem = pflash_cfi01_get_memory(s->flash[1]);
> +flashbase[1] = flash_mem->addr;
> +flashsize[1] = flash_mem->size;
>
> -name = g_strdup_printf("/flash@%" PRIx64, flashbase);
> +name = g_strdup_printf("/flash@%" PRIx64, flashbase[0]);
> qemu_fdt_add_subnode(mc->fdt, name);
> qemu_fdt_setprop_string(mc->fdt, name, "compatible", "cfi-flash");
> qemu_fdt_setprop_sized_cells(mc->fdt, name, "reg",
> - 2, flashbase, 2, flashsize,
> - 2, flashbase + flashsize, 2, flashsize);
> + 2, flashbase[0], 2, flashsize[0],
> + 2, flashbase[1], 2, flashsize[1]);
> qemu_fdt_setprop_cell(mc->fdt, name, "bank-width", 4);
> g_free(name);
> }
> --
> 2.38.0
>
>
Reviewed-by: Andrew Jones
Re: [RESEND PATCH 1/6] disas/nanomips: Move setjmp into nanomips_dis
On Sun, 6 Nov 2022, Richard Henderson wrote:
Reduce the number of local variables within the scope of the
setjmp by moving it to the existing helper. The actual length
returned from Disassemble is not used, because we have already
determined the length while reading bytes. Fixes:
nanomips.c: In function ‘print_insn_nanomips’:
nanomips.c:21925:14: error: variable ‘insn1’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
nanomips.c:21925:25: error: variable ‘insn2’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
nanomips.c:21925:36: error: variable ‘insn3’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
nanomips.c:21926:22: error: variable ‘buf’ might be clobbered by ‘longjmp’ or
‘vfork’ [-Werror=clobbered]
Signed-off-by: Richard Henderson
---
disas/nanomips.c | 44
1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/disas/nanomips.c b/disas/nanomips.c
index 9647f1a8e3..9a69e6880a 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21905,22 +21905,27 @@ static const Pool MAJOR[2] = {
0x0 },/* P16 */
};
-static int nanomips_dis(char **buf,
- Dis_info *info,
- unsigned short one,
- unsigned short two,
- unsigned short three)
+static bool nanomips_dis(char **buf, Dis_info *info,
+ unsigned short one,
+ unsigned short two,
+ unsigned short three)
{
uint16 bits[3] = {one, two, three};
-
TABLE_ENTRY_TYPE type;
-int size = Disassemble(bits, buf, , MAJOR, 2, info);
-return size;
+int ret;
+
+ret = sigsetjmp(info->buf, 0);
+if (ret != 0) {
+return false;
+}
+
+ret = Disassemble(bits, buf, , MAJOR, 2, info);
+return ret >= 0;
}
Maybe you could lose ret too and simplify it to something like this?
if (sigsetjmp(info->buf, 0)) {
return false;
}
return Disassemble(bits, buf, , MAJOR, 2, info) >= 0;
Storing the return value in a local car just to use it in the next line
does not seem necessary to me but it's just an idea, not really important
so as you like.
Regards,
BALATON Zoltan
int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
{
-int status;
+int status, length;
bfd_byte buffer[2];
uint16_t insn1 = 0, insn2 = 0, insn3 = 0;
g_autofree char *buf = NULL;
@@ -21950,6 +21955,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
} else {
insn1 = bfd_getl16(buffer);
}
+length = 2;
(*info->fprintf_func)(info->stream, "%04x ", insn1);
/* Handle 32-bit opcodes. */
@@ -21965,6 +21971,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
} else {
insn2 = bfd_getl16(buffer);
}
+length = 4;
(*info->fprintf_func)(info->stream, "%04x ", insn2);
} else {
(*info->fprintf_func)(info->stream, " ");
@@ -21982,27 +21989,16 @@ int print_insn_nanomips(bfd_vma memaddr, struct
disassemble_info *info)
} else {
insn3 = bfd_getl16(buffer);
}
+length = 6;
(*info->fprintf_func)(info->stream, "%04x ", insn3);
} else {
(*info->fprintf_func)(info->stream, " ");
}
/* Handle runtime errors. */
-if (sigsetjmp(disassm_info.buf, 0) != 0) {
-info->insn_type = dis_noninsn;
-return insn3 ? 6 : insn2 ? 4 : 2;
+if (nanomips_dis(, _info, insn1, insn2, insn3)) {
+(*info->fprintf_func) (info->stream, "%s", buf);
}
-int length = nanomips_dis(, _info, insn1, insn2, insn3);
-
-/* FIXME: Should probably use a hash table on the major opcode here. */
-
-(*info->fprintf_func) (info->stream, "%s", buf);
-if (length > 0) {
-return length / 8;
-}
-
-info->insn_type = dis_noninsn;
-
-return insn3 ? 6 : insn2 ? 4 : 2;
+return length;
}
[Bug 1034423] Re: Guests running OpenIndiana (and relatives) fail to boot on AMD hardware
This bug tracker here is not used anymore. Could you please open a new ticket here: https://gitlab.com/qemu-project/qemu/-/issues Thanks! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1034423 Title: Guests running OpenIndiana (and relatives) fail to boot on AMD hardware Status in QEMU: Expired Bug description: First observed with OpenSolaris 2009.06, and also applies to the latest OpenIndiana release. Version: qemu-kvm 1.1.1 Hardware: 2 x AMD Opteron 6128 8-core processors, 64GB RAM. These guests boot on equivalent Intel hardware. To reproduce: qemu-kvm -nodefaults -m 512 -cpu host -vga cirrus -usbdevice tablet -vnc :99 -monitor stdio -hda drive.img -cdrom oi- dev-151a5-live-x86.iso -boot order=dc I've tested with "-vga std" and various different emulated CPU types, to no effect. What happens: GRUB loads, and offers multiple boot options, but none work. Some kind of kernel panic flies by very fast before restarting the VM, and careful use of the screenshot button reveals that it reads as follows: panic[cpu0]/thread=fec22de0: BAD TRAP: type=8 (#df Double fault) rp=fec2b48c add r=0 #df Double fault pid=0, pc=0xault pid=0, pc=0xfe800377, sp=0xfec40090, eflags=0x202 cr0: 80050011 cr4:b8 cr2: 0cr3: ae2f000 gs:1b0fs: 0 es: 160 ds: 160 edi:0 esi: 0 ebp: 0 esp: fec2b4c4 ebx: c0010015 edx: 0 ecx: 0 eax: fec40400 trp: 8 err: 0 eip: fe800377 cs: 158 efl: 202 usp: fec40090 ss: 160 tss.tss_link: 0x0 tss.tss_esp0: 0x0 tss.tss_ss0: 0x160 tss.tss_esp1: 0x0 tss.tss_ss1: 0x0 tss.tss esp2: 0x0 tss.tss_ss2: 0x0 tss.tss_cr3: 0xae2f000 tss.tss_eip: 0xfec40400 tss.tss_eflags: 0x202 tss.tss_eax: 0xfec40400 tss.tss_ebx: 0xc0010015 tss.tss_ecx: 0xc001 tss.tss_edx: 0x0 tss.tss_esp: 0xfec40090 Warning - stack not written to the dumpbuf fec2b3c8 unix:due+e4 (8, fec2b48c, 0, 0) fec2b478 unix:trap+12fa (fec2b48c, 0, 0) fec2b48c unix:_cmntrap+7c (1b0, 0, 160, 160, 0) If there's any more, I haven't managed to catch it. Solaris 11 does not seem to suffer from the same issue, although the first message that appears at boot (after the version info) is "trap: Unkown trap type 8 in user mode". Could be related? As always, thanks in advance and please let me know if I can help to test, or provide any more information. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1034423/+subscriptions
Re: [PATCH v7 resend 0/4] add generic vDPA device support
在 2022/11/6 21:47, Michael S. Tsirkin 写道: On Sun, Nov 06, 2022 at 09:11:39PM +0800, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: 在 2022/11/6 13:22, Michael S. Tsirkin 写道: On Sun, Nov 06, 2022 at 08:17:07AM +0800, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: 在 2022/11/6 0:43, Michael S. Tsirkin 写道: On Sat, Nov 05, 2022 at 04:36:25PM +0800, Longpeng(Mike) wrote: From: Longpeng Hi guys, With the generic vDPA device, QEMU won't need to touch the device types any more, such like vfio. With this kind of passthrough migration is completely MIA right? Better add a blocker... Oh, I missed the "vdpa-dev: mark the device as unmigratable" since v4 and I'll add it in the next version. We'll support passthrough migration in the next step. We have already written a demo that can migrate between some offloading cards. Hmm ok. Backend disconnect can't work though, can it? State is by necessity lost when backend crashes. Yes, it can't. And given this is there an advantage over VFIO? I think the answer is the same as "why we need vDPA" if we compare it with VFIO. The answer is mostly because you can migrate and support backend disconnect, no? Migrating between different hardware is the first consideration in our requirement, supporting backend disconnect is a low priority. I dislike non-orthogonal features though ... And the advantage of keeping it out of process with qemu is I presume security? Yes, this is one of the reasons. The TCB of the generic vdpa device is smaller than the existing vdpa device (needs to use the virtio-net/blk/scsi emulation codes). Besides, the generic vdpa device can support any virtio device, but the existing vdpa device only supports virtio-net yet. Though the existing vdpa device is more powerful and the generic vdpa device would miss some features, it can be an alternative for some users. We can use the generic vDPA device as follow: -device vhost-vdpa-device-pci,vhostdev=/dev/vhost-vdpa-X Or -M microvm -m 512m -smp 2 -kernel ... -initrd ... -device \ vhost-vdpa-device,vhostdev=/dev/vhost-vdpa-x Changes v6 -> v7: (v6: https://mail.gnu.org/archive/html/qemu-devel/2022-05/msg02821.html) - rebase. [Jason] - add documentation . [Stefan] Changes v5 -> v6: Patch 2: - Turn to the original approach in the RFC to initialize the virtio_pci_id_info array. [Michael] https://lore.kernel.org/all/20220105005900.860-2-longpe...@huawei.com/ Patch 3: - Fix logical error of exception handler around the post_init. [Stefano] - Fix some coding style warnings. [Stefano] Patch 4: - Fix some coding style warnings. [Stefano] Changes v4 -> v5: Patch 3: - remove vhostfd [Jason] - support virtio-mmio [Jason] Changes v3 -> v4: v3: https://www.mail-archive.com/qemu-devel@nongnu.org/msg877015.html - reorganize the series [Stefano] - fix some typos [Stefano] - fix logical error in vhost_vdpa_device_realize [Stefano] Changes v2 -> v3 Patch 4 & 5: - only call vdpa ioctls in vdpa-dev.c [Stefano, Longpeng] - s/VQS_NUM/VQS_COUNT [Stefano] - check both vdpa_dev_fd and vdpa_dev [Stefano] Patch 6: - move all steps into vhost_vdpa_device_unrealize. [Stefano] Changes RFC -> v2 Patch 1: - rename 'pdev_id' to 'trans_devid' [Michael] - only use transitional device id for the devices listed in the spec [Michael] - use macros to make the id_info table clearer [Longpeng] - add some modern devices in the id_info table [Longpeng] Patch 2: - remove the GET_VECTORS_NUM command [Jason] Patch 4: - expose vdpa_dev_fd as a QOM preperty [Stefan] - introduce vhost_vdpa_device_get_u32 as a common function to make the code clearer [Stefan] - fix the misleading description of 'dc->desc' [Stefano] Patch 5: - check returned number of virtqueues [Stefan] Patch 6: - init s->num_queues [Stefano] - free s->dev.vqs [Stefano] Longpeng (Mike) (4): virtio: get class_id and pci device id by the virtio id vdpa: add vdpa-dev support vdpa: add vdpa-dev-pci support docs: Add generic vhost-vdpa device documentation docs/system/devices/vhost-vdpa-device.rst | 43 +++ hw/virtio/Kconfig | 5 + hw/virtio/meson.build | 2 + hw/virtio/vdpa-dev-pci.c | 102 ++ hw/virtio/vdpa-dev.c | 377 ++ hw/virtio/virtio-pci.c| 88 + include/hw/virtio/vdpa-dev.h | 43 +++ include/hw/virtio/virtio-pci.h| 5 + 8 files changed, 665 insertions(+) create mode 100644 docs/system/devices/vhost-vdpa-device.rst create mode 100644 hw/virtio/vdpa-dev-pci.c
[PATCH] hw/riscv: virt: Remove size restriction for pflash
The pflash implementation currently assumes fixed size of the
backend storage. Due to this, the backend storage file needs to be
exactly of size 32M. Otherwise, there will be an error like below.
"device requires 33554432 bytes, block backend provides 3145728 bytes"
Fix this issue by using the actual size of the backing store.
Signed-off-by: Sunil V L
---
hw/riscv/virt.c | 33 +
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index a5bc7353b4..aad175fa31 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -49,6 +49,7 @@
#include "hw/pci/pci.h"
#include "hw/pci-host/gpex.h"
#include "hw/display/ramfb.h"
+#include "sysemu/block-backend.h"
/*
* The virt machine physical address space used by some of the devices
@@ -144,10 +145,17 @@ static void virt_flash_map1(PFlashCFI01 *flash,
MemoryRegion *sysmem)
{
DeviceState *dev = DEVICE(flash);
+BlockBackend *blk;
+hwaddr real_size;
-assert(QEMU_IS_ALIGNED(size, VIRT_FLASH_SECTOR_SIZE));
-assert(size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);
-qdev_prop_set_uint32(dev, "num-blocks", size / VIRT_FLASH_SECTOR_SIZE);
+blk = pflash_cfi01_get_blk(flash);
+
+real_size = blk ? blk_getlength(blk): size;
+
+assert(real_size);
+assert(QEMU_IS_ALIGNED(real_size, VIRT_FLASH_SECTOR_SIZE));
+assert(real_size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);
+qdev_prop_set_uint32(dev, "num-blocks", real_size /
VIRT_FLASH_SECTOR_SIZE);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
memory_region_add_subregion(sysmem, base,
@@ -971,15 +979,24 @@ static void create_fdt_flash(RISCVVirtState *s, const
MemMapEntry *memmap)
{
char *name;
MachineState *mc = MACHINE(s);
-hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
-hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
+MemoryRegion *flash_mem;
+hwaddr flashsize[2];
+hwaddr flashbase[2];
+
+flash_mem = pflash_cfi01_get_memory(s->flash[0]);
+flashbase[0] = flash_mem->addr;
+flashsize[0] = flash_mem->size;
+
+flash_mem = pflash_cfi01_get_memory(s->flash[1]);
+flashbase[1] = flash_mem->addr;
+flashsize[1] = flash_mem->size;
-name = g_strdup_printf("/flash@%" PRIx64, flashbase);
+name = g_strdup_printf("/flash@%" PRIx64, flashbase[0]);
qemu_fdt_add_subnode(mc->fdt, name);
qemu_fdt_setprop_string(mc->fdt, name, "compatible", "cfi-flash");
qemu_fdt_setprop_sized_cells(mc->fdt, name, "reg",
- 2, flashbase, 2, flashsize,
- 2, flashbase + flashsize, 2, flashsize);
+ 2, flashbase[0], 2, flashsize[0],
+ 2, flashbase[1], 2, flashsize[1]);
qemu_fdt_setprop_cell(mc->fdt, name, "bank-width", 4);
g_free(name);
}
--
2.38.0
Re: [PATCH] LockGuards: replace manual lock()/unlock() calls to WITH_QEMU_LOCK_GUARD()
On Fri, 4 Nov 2022 at 21:04, wrote:
>
> From: Samker
...
> Signed-off-by: M N Gachu
The author and Signed-off-by name/email are different. Do you want to
use a single name/email?
> ---
> softmmu/physmem.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index d9578ccfd4..fb00596777 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -24,6 +24,7 @@
> #include "qemu/cutils.h"
> #include "qemu/cacheflush.h"
> #include "qemu/madvise.h"
> +#include "qemu/lockable.h"
>
> #ifdef CONFIG_TCG
> #include "hw/core/tcg-cpu-ops.h"
> @@ -3114,13 +3115,12 @@ void cpu_register_map_client(QEMUBH *bh)
> {
> MapClient *client = g_malloc(sizeof(*client));
>
> -qemu_mutex_lock(_client_list_lock);
> +WITH_QEMU_LOCK_GUARD(_client_list_lock);
There is a bug here: the lock won't be held after this line because
WITH_QEMU_LOCK_GUARD() is block scoped. It requires curly braces:
WITH_QEMU_LOCK_GUARD() {
...protected code...
}
...unprotected code...
Use QEMU_LOCK_GUARD(); when don't want block scope. It holds the
lock for the remainder of the function.
Re: [PATCH v7 resend 0/4] add generic vDPA device support
On Sun, Nov 06, 2022 at 09:11:39PM +0800, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: > > > 在 2022/11/6 13:22, Michael S. Tsirkin 写道: > > On Sun, Nov 06, 2022 at 08:17:07AM +0800, Longpeng (Mike, Cloud > > Infrastructure Service Product Dept.) wrote: > > > > > > > > > 在 2022/11/6 0:43, Michael S. Tsirkin 写道: > > > > On Sat, Nov 05, 2022 at 04:36:25PM +0800, Longpeng(Mike) wrote: > > > > > From: Longpeng > > > > > > > > > > Hi guys, > > > > > > > > > > With the generic vDPA device, QEMU won't need to touch the device > > > > > types any more, such like vfio. > > > > > > > > With this kind of passthrough migration is completely MIA right? > > > > Better add a blocker... > > > > > > Oh, I missed the "vdpa-dev: mark the device as unmigratable" since v4 and > > > I'll add it in the next version. > > > > > > We'll support passthrough migration in the next step. We have already > > > written a demo that can migrate between some offloading cards. > > > > Hmm ok. Backend disconnect can't work though, can it? State > > is by necessity lost when backend crashes. > > Yes, it can't. > > > > > And given this is there an advantage over VFIO? > > > > > > I think the answer is the same as "why we need vDPA" if we compare it with > > > VFIO. > > > > The answer is mostly because you can migrate and support backend > > disconnect, no? > > > Migrating between different hardware is the first consideration in our > requirement, supporting backend disconnect is a low priority. I dislike non-orthogonal features though ... And the advantage of keeping it out of process with qemu is I presume security? > > > > > > > > > We can use the generic vDPA device as follow: > > > > > -device vhost-vdpa-device-pci,vhostdev=/dev/vhost-vdpa-X > > > > > Or > > > > > -M microvm -m 512m -smp 2 -kernel ... -initrd ... -device \ > > > > > vhost-vdpa-device,vhostdev=/dev/vhost-vdpa-x > > > > > > > > > Changes v6 -> v7: > > > > > (v6: > > > > > https://mail.gnu.org/archive/html/qemu-devel/2022-05/msg02821.html) > > > > > - rebase. [Jason] > > > > > - add documentation . [Stefan] > > > > > > > > > > Changes v5 -> v6: > > > > > Patch 2: > > > > > - Turn to the original approach in the RFC to initialize the > > > > > virtio_pci_id_info array. [Michael] > > > > > > > > > > https://lore.kernel.org/all/20220105005900.860-2-longpe...@huawei.com/ > > > > > Patch 3: > > > > > - Fix logical error of exception handler around the post_init. > > > > > [Stefano] > > > > > - Fix some coding style warnings. [Stefano] > > > > > Patch 4: > > > > > - Fix some coding style warnings. [Stefano] > > > > > > > > > > Changes v4 -> v5: > > > > > Patch 3: > > > > > - remove vhostfd [Jason] > > > > > - support virtio-mmio [Jason] > > > > > > > > > > Changes v3 -> v4: > > > > > v3: > > > > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg877015.html > > > > > - reorganize the series [Stefano] > > > > > - fix some typos [Stefano] > > > > > - fix logical error in vhost_vdpa_device_realize [Stefano] > > > > > > > > > > Changes v2 -> v3 > > > > > Patch 4 & 5: > > > > > - only call vdpa ioctls in vdpa-dev.c [Stefano, Longpeng] > > > > > - s/VQS_NUM/VQS_COUNT [Stefano] > > > > > - check both vdpa_dev_fd and vdpa_dev [Stefano] > > > > > Patch 6: > > > > > - move all steps into vhost_vdpa_device_unrealize. [Stefano] > > > > > > > > > > Changes RFC -> v2 > > > > > Patch 1: > > > > > - rename 'pdev_id' to 'trans_devid' [Michael] > > > > > - only use transitional device id for the devices > > > > > listed in the spec [Michael] > > > > > - use macros to make the id_info table clearer [Longpeng] > > > > > - add some modern devices in the id_info table [Longpeng] > > > > > Patch 2: > > > > > - remove the GET_VECTORS_NUM command [Jason] > > > > > Patch 4: > > > > > - expose vdpa_dev_fd as a QOM preperty [Stefan] > > > > > - introduce vhost_vdpa_device_get_u32 as a common > > > > > function to make the code clearer [Stefan] > > > > > - fix the misleading description of 'dc->desc' [Stefano] > > > > > Patch 5: > > > > > - check returned number of virtqueues [Stefan] > > > > > Patch 6: > > > > > - init s->num_queues [Stefano] > > > > > - free s->dev.vqs [Stefano] > > > > > > > > > > > > > > > Longpeng (Mike) (4): > > > > > virtio: get class_id and pci device id by the virtio id > > > > > vdpa: add vdpa-dev support > > > > > vdpa: add vdpa-dev-pci support > > > > > docs: Add generic vhost-vdpa device documentation > > > > > > > > > >docs/system/devices/vhost-vdpa-device.rst | 43 +++ > > > > >hw/virtio/Kconfig | 5 + > > > > >hw/virtio/meson.build | 2 + > > > > >hw/virtio/vdpa-dev-pci.c
Re: [PATCH v7 resend 0/4] add generic vDPA device support
在 2022/11/6 13:22, Michael S. Tsirkin 写道: On Sun, Nov 06, 2022 at 08:17:07AM +0800, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: 在 2022/11/6 0:43, Michael S. Tsirkin 写道: On Sat, Nov 05, 2022 at 04:36:25PM +0800, Longpeng(Mike) wrote: From: Longpeng Hi guys, With the generic vDPA device, QEMU won't need to touch the device types any more, such like vfio. With this kind of passthrough migration is completely MIA right? Better add a blocker... Oh, I missed the "vdpa-dev: mark the device as unmigratable" since v4 and I'll add it in the next version. We'll support passthrough migration in the next step. We have already written a demo that can migrate between some offloading cards. Hmm ok. Backend disconnect can't work though, can it? State is by necessity lost when backend crashes. Yes, it can't. And given this is there an advantage over VFIO? I think the answer is the same as "why we need vDPA" if we compare it with VFIO. The answer is mostly because you can migrate and support backend disconnect, no? Migrating between different hardware is the first consideration in our requirement, supporting backend disconnect is a low priority. We can use the generic vDPA device as follow: -device vhost-vdpa-device-pci,vhostdev=/dev/vhost-vdpa-X Or -M microvm -m 512m -smp 2 -kernel ... -initrd ... -device \ vhost-vdpa-device,vhostdev=/dev/vhost-vdpa-x Changes v6 -> v7: (v6: https://mail.gnu.org/archive/html/qemu-devel/2022-05/msg02821.html) - rebase. [Jason] - add documentation . [Stefan] Changes v5 -> v6: Patch 2: - Turn to the original approach in the RFC to initialize the virtio_pci_id_info array. [Michael] https://lore.kernel.org/all/20220105005900.860-2-longpe...@huawei.com/ Patch 3: - Fix logical error of exception handler around the post_init. [Stefano] - Fix some coding style warnings. [Stefano] Patch 4: - Fix some coding style warnings. [Stefano] Changes v4 -> v5: Patch 3: - remove vhostfd [Jason] - support virtio-mmio [Jason] Changes v3 -> v4: v3: https://www.mail-archive.com/qemu-devel@nongnu.org/msg877015.html - reorganize the series [Stefano] - fix some typos [Stefano] - fix logical error in vhost_vdpa_device_realize [Stefano] Changes v2 -> v3 Patch 4 & 5: - only call vdpa ioctls in vdpa-dev.c [Stefano, Longpeng] - s/VQS_NUM/VQS_COUNT [Stefano] - check both vdpa_dev_fd and vdpa_dev [Stefano] Patch 6: - move all steps into vhost_vdpa_device_unrealize. [Stefano] Changes RFC -> v2 Patch 1: - rename 'pdev_id' to 'trans_devid' [Michael] - only use transitional device id for the devices listed in the spec [Michael] - use macros to make the id_info table clearer [Longpeng] - add some modern devices in the id_info table [Longpeng] Patch 2: - remove the GET_VECTORS_NUM command [Jason] Patch 4: - expose vdpa_dev_fd as a QOM preperty [Stefan] - introduce vhost_vdpa_device_get_u32 as a common function to make the code clearer [Stefan] - fix the misleading description of 'dc->desc' [Stefano] Patch 5: - check returned number of virtqueues [Stefan] Patch 6: - init s->num_queues [Stefano] - free s->dev.vqs [Stefano] Longpeng (Mike) (4): virtio: get class_id and pci device id by the virtio id vdpa: add vdpa-dev support vdpa: add vdpa-dev-pci support docs: Add generic vhost-vdpa device documentation docs/system/devices/vhost-vdpa-device.rst | 43 +++ hw/virtio/Kconfig | 5 + hw/virtio/meson.build | 2 + hw/virtio/vdpa-dev-pci.c | 102 ++ hw/virtio/vdpa-dev.c | 377 ++ hw/virtio/virtio-pci.c| 88 + include/hw/virtio/vdpa-dev.h | 43 +++ include/hw/virtio/virtio-pci.h| 5 + 8 files changed, 665 insertions(+) create mode 100644 docs/system/devices/vhost-vdpa-device.rst create mode 100644 hw/virtio/vdpa-dev-pci.c create mode 100644 hw/virtio/vdpa-dev.c create mode 100644 include/hw/virtio/vdpa-dev.h -- 2.23.0 . .
[Bug 1034423] Re: Guests running OpenIndiana (and relatives) fail to boot on AMD hardware
Despite the age of the report, I am also reproducing the issue. I am using Qemu 6.2.0 with KVM on Linux kernel 6.0.5 under Linux Mint 21. The guest is OpenIndiana Hipster 2021.10. A guest console capture is attached. The guest is managed using libvirt 8.0.0 The dump of the libvirt domain configuration is as follows: openindiana 7a7adcc0-889c-4daf-a73b-21a3fac3d8e7 http://libosinfo.org/xmlns/libvirt/domain/1.0;> http://libosinfo.org/linux/2020"/> 2097152 2097152 4 /machine hvm /usr/share/OVMF/OVMF_CODE_4M.fd /var/lib/libvirt/qemu/nvram/openindiana_VARS.fd destroy restart destroy /usr/bin/qemu-system-x86_64 libvirt-7a7adcc0-889c-4daf-a73b-21a3fac3d8e7 libvirt-7a7adcc0-889c-4daf-a73b-21a3fac3d8e7 +64055:+130 +64055:+130 ** Attachment added: "Screenshot_openindiana_2022-11-06_07:30:05.png" https://bugs.launchpad.net/qemu/+bug/1034423/+attachment/5629412/+files/Screenshot_openindiana_2022-11-06_07%3A30%3A05.png -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1034423 Title: Guests running OpenIndiana (and relatives) fail to boot on AMD hardware Status in QEMU: Expired Bug description: First observed with OpenSolaris 2009.06, and also applies to the latest OpenIndiana release. Version: qemu-kvm 1.1.1 Hardware: 2 x AMD Opteron 6128 8-core processors, 64GB RAM. These guests boot on equivalent Intel hardware. To reproduce: qemu-kvm -nodefaults -m 512 -cpu host -vga cirrus -usbdevice tablet -vnc :99 -monitor stdio -hda drive.img -cdrom oi- dev-151a5-live-x86.iso -boot order=dc I've tested with "-vga std" and various different emulated CPU types, to no effect. What happens: GRUB loads, and offers multiple boot options, but none work. Some kind of kernel panic flies by very fast before restarting the VM, and careful use of the screenshot button reveals that it reads as follows: panic[cpu0]/thread=fec22de0: BAD TRAP: type=8 (#df Double fault) rp=fec2b48c add r=0 #df Double fault pid=0, pc=0xault pid=0, pc=0xfe800377, sp=0xfec40090, eflags=0x202 cr0: 80050011 cr4:b8 cr2: 0cr3: ae2f000 gs:1b0fs: 0 es: 160 ds: 160 edi:0 esi: 0 ebp: 0 esp: fec2b4c4 ebx: c0010015 edx: 0 ecx: 0 eax: fec40400 trp: 8 err: 0 eip: fe800377 cs: 158 efl: 202 usp: fec40090 ss: 160 tss.tss_link: 0x0 tss.tss_esp0: 0x0 tss.tss_ss0: 0x160 tss.tss_esp1: 0x0 tss.tss_ss1: 0x0 tss.tss esp2: 0x0 tss.tss_ss2: 0x0 tss.tss_cr3: 0xae2f000 tss.tss_eip: 0xfec40400 tss.tss_eflags: 0x202 tss.tss_eax: 0xfec40400 tss.tss_ebx: 0xc0010015 tss.tss_ecx: 0xc001 tss.tss_edx: 0x0 tss.tss_esp: 0xfec40090 Warning - stack not written to the dumpbuf fec2b3c8 unix:due+e4 (8, fec2b48c, 0, 0) fec2b478 unix:trap+12fa (fec2b48c, 0, 0) fec2b48c unix:_cmntrap+7c (1b0, 0, 160, 160, 0) If there's any more, I haven't managed to catch it. Solaris 11 does not seem to suffer from the same issue, although the first message that appears at boot (after the version info) is "trap: Unkown trap type 8 in user mode". Could be related? As always, thanks in advance and please let me know if I can help to test, or provide any more information. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1034423/+subscriptions
Re: [PATCH v11 01/11] s390x: Register TYPE_S390_CCW_MACHINE properties as class properties
On 04/11/2022 15.57, Pierre Morel wrote:
On 11/4/22 15:29, Thomas Huth wrote:
On 04/11/2022 11.53, Cédric Le Goater wrote:
On 11/4/22 11:16, Pierre Morel wrote:
On 11/4/22 07:32, Thomas Huth wrote:
On 03/11/2022 18.01, Pierre Morel wrote:
Signed-off-by: Pierre Morel
---
hw/s390x/s390-virtio-ccw.c | 127 +
1 file changed, 72 insertions(+), 55 deletions(-)
-EMISSINGPATCHDESCRIPTION
... please add some words *why* this is a good idea / necessary.
I saw that the i386 patch had no description for the same patch so...
To be honest I do not know why it is necessary.
The only reason I see is to be in sync with the PC implementation.
So what about:
"
Register TYPE_S390_CCW_MACHINE properties as class properties
to be conform with the X architectures
"
?
@Cédric , any official recommendation for doing that?
There was a bunch of commits related to QOM in this series :
91def7b83 arm/virt: Register most properties as class properties
f5730c69f0 i386: Register feature bit properties as class properties
which moved property definitions at the class level.
Then,
commit d8fb7d0969 ("vl: switch -M parsing to keyval")
changed machine_help_func() to use a machine class and not machine
instance anymore.
I would use the same kind of commit log and add a Fixes tag to get it
merged in 7.2
Ah, so this fixes the problem that running QEMU with " -M
s390-ccw-virtio,help" does not show the s390x-specific properties anymore?
... that's certainly somethings that should be mentioned in the commit
message! What about something like this:
"Currently, when running 'qemu-system-s390x -M -M s390-ccw-virtio,help'
the s390x-specific properties are not listed anymore. This happens because
since commit d8fb7d0969 ("vl: switch -M parsing to keyval") the properties
have to be defined at the class level and not at the instance level
anymore. Fix it on s390x now, too, by moving the registration of the
properties to the class level"
Fixes: d8fb7d0969 ("vl: switch -M parsing to keyval")
?
Thomas
That seems really good :)
All right, I've queued this patch (with the updated commit description) and
the next one on my s390x-branch for QEMU 7.2:
https://gitlab.com/thuth/qemu/-/commits/s390x-next/
Thomas
Re: [PATCH v2] s390x/css: revert SCSW ctrl/flag bits on error
On 28/10/2022 22.22, Eric Farman wrote: On Thu, 2022-10-27 at 23:23 +0200, Peter Jin wrote: Revert the control and flag bits in the subchannel status word in case the SSCH operation fails with non-zero CC (ditto for CSCH and HSCH). According to POPS, the control and flag bits are only changed if SSCH, CSCH, and HSCH return CC 0, and no other action should be taken otherwise. In order to simulate that after the fact, the bits need to be reverted on non-zero CC. I'm okay to this point... This change is necessary due to the fact that the pwrite() in vfio- ccw which triggers the SSCH can fail at any time. Previously, there was only virtio-ccw, whose do_subchannel_work function was only able to return CC0. However, once vfio-ccw went into the mix, it has become necessary to handle errors in code paths that were previously assumed to always return success. In our case, we found that in case of pwrite() failure (which was discovered by strace injection), the subchannel could be stuck in start pending state, which could be problematic if the pwrite() call returns CC2. Experimentation shows that the guest tries to retry the SSCH call as normal for CC2, but it actually continously fails due to the fact that the subchannel is stuck in start pending state even though no start function is actually taking place. ...but the two paragraphs above are a bit cumbersome to digest. Maybe it's just too late in the week for me. What about something like this? """ While the do_subchannel_work logic for virtual (virtio) devices will return condition code 0, passthrough (vfio) devices may encounter errors from either the host kernel or real hardware that need to be accounted for after this point. This includes restoring the state of the Subchannel Status Word to reflect the subchannel, as these bits would not be set in the event of a non-zero condition code from the affected instructions. Experimentation has shown that a failure on a START SUBCHANNEL (SSCH) to a passthrough device would leave the subchannel with the START PENDING activity control bit set, thus blocking subsequent SSCH operations in css_do_ssch() until some form of error recovery was undertaken since no interrupt would be expected. """ Signed-off-by: Peter Jin We've talked previously about clearing this within the do_subchannel_work_passthrough routine in order to keep the _virtual paths untouched, but this seems like a reasonable approach to me. The commit message is probably fine either way, but as far as the code goes: Reviewed-by: Eric Farman Thanks, I've queued the patch now to my s390x-next branch with the updated commit message. Please double-check whether that looks OK now: https://gitlab.com/thuth/qemu/-/commits/s390x-next/ Thomas
[PULL 10/12] module: add Error arguments to module_load and module_load_qom
From: Claudio Fontana
improve error handling during module load, by changing:
bool module_load(const char *prefix, const char *lib_name);
void module_load_qom(const char *type);
to:
int module_load(const char *prefix, const char *name, Error **errp);
int module_load_qom(const char *type, Error **errp);
where the return value is:
-1 on module load error, and errp is set with the error
0 on module or one of its dependencies are not installed
1 on module load success
2 on module load success (module already loaded or built-in)
module_load_qom_one has been introduced in:
commit 28457744c345 ("module: qom module support"), which built on top of
module_load_one, but discarded the bool return value. Restore it.
Adapt all callers to emit errors, or ignore them, or fail hard,
as appropriate in each context.
Replace the previous emission of errors via fprintf in _some_ error
conditions with Error and error_report, so as to emit to the appropriate
target.
A memory leak is also fixed as part of the module_load changes.
audio: when attempting to load an audio module, report module load errors.
Note that still for some callers, a single issue may generate multiple
error reports, and this could be improved further.
Regarding the audio code itself, audio_add() seems to ignore errors,
and this should probably be improved.
block: when attempting to load a block module, report module load errors.
For the code paths that already use the Error API, take advantage of those
to report module load errors into the Error parameter.
For the other code paths, we currently emit the error, but this could be
improved further by adding Error parameters to all possible code paths.
console: when attempting to load a display module, report module load errors.
qdev: when creating a new qdev Device object (DeviceState), report load errors.
If a module cannot be loaded to create that device, now abort execution
(if no CONFIG_MODULE) or exit (if CONFIG_MODULE).
qom/object.c: when initializing a QOM object, or looking up class_by_name,
report module load errors.
qtest: when processing the "module_load" qtest command, report errors
in the load of the module.
Signed-off-by: Claudio Fontana
Reviewed-by: Richard Henderson
Message-Id: <20220929093035.4231-4-cfont...@suse.de>
Signed-off-by: Paolo Bonzini
---
audio/audio.c | 16 ++--
block.c | 20 +++-
block/dmg.c | 14 ++-
hw/core/qdev.c| 17 +++-
include/qemu/module.h | 37 +++-
qom/object.c | 18 +++-
softmmu/qtest.c | 8 +-
ui/console.c | 18 +++-
util/module.c | 209 +++---
9 files changed, 234 insertions(+), 123 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 379f19dc891f..065602ce1b95 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -73,20 +73,24 @@ void audio_driver_register(audio_driver *drv)
audio_driver *audio_driver_lookup(const char *name)
{
struct audio_driver *d;
+Error *local_err = NULL;
+int rv;
QLIST_FOREACH(d, _drivers, next) {
if (strcmp(name, d->name) == 0) {
return d;
}
}
-
-audio_module_load(name);
-QLIST_FOREACH(d, _drivers, next) {
-if (strcmp(name, d->name) == 0) {
-return d;
+rv = audio_module_load(name, _err);
+if (rv > 0) {
+QLIST_FOREACH(d, _drivers, next) {
+if (strcmp(name, d->name) == 0) {
+return d;
+}
}
+} else if (rv < 0) {
+error_report_err(local_err);
}
-
return NULL;
}
diff --git a/block.c b/block.c
index ddd743c44735..c5e20c0beae3 100644
--- a/block.c
+++ b/block.c
@@ -464,12 +464,18 @@ BlockDriver *bdrv_find_format(const char *format_name)
/* The driver isn't registered, maybe we need to load a module */
for (i = 0; i < (int)ARRAY_SIZE(block_driver_modules); ++i) {
if (!strcmp(block_driver_modules[i].format_name, format_name)) {
-block_module_load(block_driver_modules[i].library_name);
+Error *local_err = NULL;
+int rv = block_module_load(block_driver_modules[i].library_name,
+ _err);
+if (rv > 0) {
+return bdrv_do_find_format(format_name);
+} else if (rv < 0) {
+error_report_err(local_err);
+}
break;
}
}
-
-return bdrv_do_find_format(format_name);
+return NULL;
}
static int bdrv_format_is_whitelisted(const char *format_name, bool read_only)
@@ -981,12 +987,16 @@ BlockDriver *bdrv_find_protocol(const char *filename,
for (i = 0; i < (int)ARRAY_SIZE(block_driver_modules); ++i) {
if (block_driver_modules[i].protocol_name &&
!strcmp(block_driver_modules[i].protocol_name, protocol)) {
-block_module_load(block_driver_modules[i].library_name);
+int rv =
[PULL 12/12] accel: abort if we fail to load the accelerator plugin
From: Claudio Fontana
if QEMU is configured with modules enabled, it is possible that the
load of an accelerator module will fail.
Exit in this case, relying on module_object_class_by_name to report
the specific load error if any.
Signed-off-by: Claudio Fontana
Reviewed-by: Richard Henderson
[claudio: changed abort() to exit(1)]
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Markus Armbruster
Message-Id: <20220929093035.4231-6-cfont...@suse.de>
Signed-off-by: Paolo Bonzini
---
accel/accel-softmmu.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
index 67276e4f5222..f9cdafb148ac 100644
--- a/accel/accel-softmmu.c
+++ b/accel/accel-softmmu.c
@@ -66,6 +66,7 @@ void accel_init_ops_interfaces(AccelClass *ac)
{
const char *ac_name;
char *ops_name;
+ObjectClass *oc;
AccelOpsClass *ops;
ac_name = object_class_get_name(OBJECT_CLASS(ac));
@@ -73,8 +74,13 @@ void accel_init_ops_interfaces(AccelClass *ac)
ops_name = g_strdup_printf("%s" ACCEL_OPS_SUFFIX, ac_name);
ops = ACCEL_OPS_CLASS(module_object_class_by_name(ops_name));
+oc = module_object_class_by_name(ops_name);
+if (!oc) {
+error_report("fatal: could not load module for type '%s'", ops_name);
+exit(1);
+}
g_free(ops_name);
-
+ops = ACCEL_OPS_CLASS(oc);
/*
* all accelerators need to define ops, providing at least a mandatory
* non-NULL create_vcpu_thread operation.
--
2.38.1
[PULL 11/12] dmg: warn when opening dmg images containing blocks of unknown type
From: Kevin Wolf
Signed-off-by: Kevin Wolf
Signed-off-by: Claudio Fontana
Reviewed-by: Richard Henderson
Message-Id: <20220929093035.4231-5-cfont...@suse.de>
Signed-off-by: Paolo Bonzini
---
block/dmg.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/block/dmg.c b/block/dmg.c
index ba8ec344d479..675e840ca587 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -254,6 +254,25 @@ static int dmg_read_mish_block(BDRVDMGState *s,
DmgHeaderState *ds,
for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
s->types[i] = buff_read_uint32(buffer, offset);
if (!dmg_is_known_block_type(s->types[i])) {
+switch (s->types[i]) {
+case UDBZ:
+warn_report_once("dmg-bzip2 module is missing, accessing bzip2
"
+ "compressed blocks will result in I/O
errors");
+break;
+case ULFO:
+warn_report_once("dmg-lzfse module is missing, accessing lzfse
"
+ "compressed blocks will result in I/O
errors");
+break;
+case UDCM:
+case UDLE:
+/* Comments and last entry can be ignored without problems */
+break;
+default:
+warn_report_once("Image contains chunks of unknown type %x, "
+ "accessing them will result in I/O errors",
+ s->types[i]);
+break;
+}
chunk_count--;
i--;
offset += 40;
--
2.38.1
[PULL 05/12] Fix broken configure with -Wunused-parameter
From: Stefan Weil
The configure script fails because it tries to compile small C programs
with a main function which is declared with arguments argc and argv
although those arguments are unused.
Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
configure for a native build does abort but shows the error in config.log.
A cross build configure for Windows with Debian stable aborts with an
error.
Avoiding unused arguments fixes this.
Signed-off-by: Stefan Weil
Message-Id: <20221102202258.456359-1...@weilnetz.de>
Signed-off-by: Paolo Bonzini
---
configure | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/configure b/configure
index 4275f5419fae..66928692b058 100755
--- a/configure
+++ b/configure
@@ -1258,7 +1258,7 @@ if test "$stack_protector" != "no"; then
cat > $TMPC << EOF
int main(int argc, char *argv[])
{
-char arr[64], *p = arr, *c = argv[0];
+char arr[64], *p = arr, *c = argv[argc - 1];
while (*c) {
*p++ = *c++;
}
@@ -1607,7 +1607,7 @@ fi
if test "$safe_stack" = "yes"; then
cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
{
#if ! __has_feature(safe_stack)
#error SafeStack Disabled
@@ -1629,7 +1629,7 @@ EOF
fi
else
cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
{
#if defined(__has_feature)
#if __has_feature(safe_stack)
@@ -1675,7 +1675,7 @@ static const int Z = 1;
#define TAUT(X) ((X) == Z)
#define PAREN(X, Y) (X == Y)
#define ID(X) (X)
-int main(int argc, char *argv[])
+int main(void)
{
int x = 0, y = 0;
x = ID(x);
--
2.38.1
[PULL 08/12] module: removed unused function argument "mayfail"
From: Claudio Fontana
mayfail is always passed as false for every invocation throughout the program.
It controls whether to printf or not to printf an error on
g_module_open failure.
Remove this unused argument.
Signed-off-by: Claudio Fontana
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20220929093035.4231-2-cfont...@suse.de>
Signed-off-by: Paolo Bonzini
---
include/qemu/module.h | 8
softmmu/qtest.c | 2 +-
util/module.c | 20 +---
3 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/include/qemu/module.h b/include/qemu/module.h
index bd73607104c9..8c012bbe038d 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -61,15 +61,15 @@ typedef enum {
#define fuzz_target_init(function) module_init(function, \
MODULE_INIT_FUZZ_TARGET)
#define migration_init(function) module_init(function, MODULE_INIT_MIGRATION)
-#define block_module_load_one(lib) module_load_one("block-", lib, false)
-#define ui_module_load_one(lib) module_load_one("ui-", lib, false)
-#define audio_module_load_one(lib) module_load_one("audio-", lib, false)
+#define block_module_load_one(lib) module_load_one("block-", lib)
+#define ui_module_load_one(lib) module_load_one("ui-", lib)
+#define audio_module_load_one(lib) module_load_one("audio-", lib)
void register_module_init(void (*fn)(void), module_init_type type);
void register_dso_module_init(void (*fn)(void), module_init_type type);
void module_call_init(module_init_type type);
-bool module_load_one(const char *prefix, const char *lib_name, bool mayfail);
+bool module_load_one(const char *prefix, const char *lib_name);
void module_load_qom_one(const char *type);
void module_load_qom_all(void);
void module_allow_arch(const char *arch);
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index afea7693d0cd..ff74c5d7092e 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -756,7 +756,7 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
g_assert(words[1] && words[2]);
qtest_send_prefix(chr);
-if (module_load_one(words[1], words[2], false)) {
+if (module_load_one(words[1], words[2])) {
qtest_sendf(chr, "OK\n");
} else {
qtest_sendf(chr, "FAIL\n");
diff --git a/util/module.c b/util/module.c
index 8ddb0e18f517..8563edd6267c 100644
--- a/util/module.c
+++ b/util/module.c
@@ -144,7 +144,7 @@ static bool module_check_arch(const QemuModinfo *modinfo)
return true;
}
-static int module_load_file(const char *fname, bool mayfail, bool
export_symbols)
+static int module_load_file(const char *fname, bool export_symbols)
{
GModule *g_module;
void (*sym)(void);
@@ -172,10 +172,8 @@ static int module_load_file(const char *fname, bool
mayfail, bool export_symbols
}
g_module = g_module_open(fname, flags);
if (!g_module) {
-if (!mayfail) {
-fprintf(stderr, "Failed to open module: %s\n",
-g_module_error());
-}
+fprintf(stderr, "Failed to open module: %s\n",
+g_module_error());
ret = -EINVAL;
goto out;
}
@@ -208,7 +206,7 @@ out:
}
#endif
-bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
+bool module_load_one(const char *prefix, const char *lib_name)
{
bool success = false;
@@ -256,7 +254,7 @@ bool module_load_one(const char *prefix, const char
*lib_name, bool mayfail)
if (strcmp(modinfo->name, module_name) == 0) {
/* we depend on other module(s) */
for (sl = modinfo->deps; *sl != NULL; sl++) {
-module_load_one("", *sl, false);
+module_load_one("", *sl);
}
} else {
for (sl = modinfo->deps; *sl != NULL; sl++) {
@@ -287,7 +285,7 @@ bool module_load_one(const char *prefix, const char
*lib_name, bool mayfail)
for (i = 0; i < n_dirs; i++) {
fname = g_strdup_printf("%s/%s%s",
dirs[i], module_name, CONFIG_HOST_DSOSUF);
-ret = module_load_file(fname, mayfail, export_symbols);
+ret = module_load_file(fname, export_symbols);
g_free(fname);
fname = NULL;
/* Try loading until loaded a module file */
@@ -333,7 +331,7 @@ void module_load_qom_one(const char *type)
}
for (sl = modinfo->objs; *sl != NULL; sl++) {
if (strcmp(type, *sl) == 0) {
-module_load_one("", modinfo->name, false);
+module_load_one("", modinfo->name);
}
}
}
@@ -354,7 +352,7 @@ void module_load_qom_all(void)
if (!module_check_arch(modinfo)) {
continue;
}
-module_load_one("", modinfo->name, false);
+module_load_one("", modinfo->name);
}
module_loaded_qom_all = true;
}
@@ -370,7 +368,7 @@ void
[PULL 07/12] Add missing include statement for global xml_builtin
From: Stefan Weil
This fixes some compiler warnings with compiler flag
-Wmissing-variable-declarations (tested with clang):
aarch64_be-linux-user-gdbstub-xml.c:564:19: warning: no previous extern
declaration for non-static variable 'xml_builtin'
[-Wmissing-variable-declarations]
aarch64-linux-user-gdbstub-xml.c:564:19: warning: no previous extern
declaration for non-static variable 'xml_builtin'
[-Wmissing-variable-declarations]
aarch64-softmmu-gdbstub-xml.c:1763:19: warning: no previous extern
declaration for non-static variable 'xml_builtin'
[-Wmissing-variable-declarations]
Signed-off-by: Stefan Weil
Signed-off-by: Paolo Bonzini
---
scripts/feature_to_c.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/feature_to_c.sh b/scripts/feature_to_c.sh
index b1169899c19d..c1f67c8f6a57 100644
--- a/scripts/feature_to_c.sh
+++ b/scripts/feature_to_c.sh
@@ -56,6 +56,7 @@ for input; do
done
echo
+echo '#include "exec/gdbstub.h"'
echo "const char *const xml_builtin[][2] = {"
for input; do
--
2.38.1
[PULL 06/12] meson: avoid unused arguments of main() in compiler tests
meson.build has one test where "main" is declared unnecessarily
with argc and argv arguments, but does not use them. Because
the test needs -Werror too, HAVE_BROKEN_SIZE_MAX is defined
incorrectly.
Fix the test and, for consistency, remove argc and argv whenever
they are not needed.
Signed-off-by: Paolo Bonzini
---
meson.build | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/meson.build b/meson.build
index 1d448272ab88..cf3e517e56d8 100644
--- a/meson.build
+++ b/meson.build
@@ -2165,7 +2165,7 @@ config_host_data.set('CONFIG_SPLICE',
cc.links(gnu_source_prefix + '''
config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''
#include
- int main(int argc, char *argv[]) {
+ int main(void) {
return mlockall(MCL_FUTURE);
}'''))
@@ -2210,7 +2210,7 @@ config_host_data.set('HAVE_FSXATTR', cc.links('''
config_host_data.set('HAVE_BROKEN_SIZE_MAX', not cc.compiles('''
#include
#include
-int main(int argc, char *argv[]) {
+int main(void) {
return printf("%zu", SIZE_MAX);
}''', args: ['-Werror']))
@@ -2327,7 +2327,7 @@ config_host_data.set('CONFIG_AVX2_OPT',
get_option('avx2') \
__m256i x = *(__m256i *)a;
return _mm256_testz_si256(x, x);
}
-int main(int argc, char *argv[]) { return bar(argv[0]); }
+int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
'''), error_message: 'AVX2 not available').allowed())
config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \
@@ -2341,7 +2341,7 @@ config_host_data.set('CONFIG_AVX512F_OPT',
get_option('avx512f') \
__m512i x = *(__m512i *)a;
return _mm512_test_epi64_mask(x, x);
}
-int main(int argc, char *argv[]) { return bar(argv[0]); }
+int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
'''), error_message: 'AVX512F not available').allowed())
have_pvrdma = get_option('pvrdma') \
--
2.38.1
[PULL 09/12] module: rename module_load_one to module_load
From: Claudio Fontana
Signed-off-by: Claudio Fontana
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20220929093035.4231-3-cfont...@suse.de>
Signed-off-by: Paolo Bonzini
---
audio/audio.c | 2 +-
block.c | 4 ++--
block/dmg.c | 4 ++--
hw/core/qdev.c| 2 +-
include/qemu/module.h | 10 +-
qom/object.c | 4 ++--
softmmu/qtest.c | 2 +-
ui/console.c | 6 +++---
util/module.c | 14 +++---
9 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index cc664271ebb5..379f19dc891f 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -80,7 +80,7 @@ audio_driver *audio_driver_lookup(const char *name)
}
}
-audio_module_load_one(name);
+audio_module_load(name);
QLIST_FOREACH(d, _drivers, next) {
if (strcmp(name, d->name) == 0) {
return d;
diff --git a/block.c b/block.c
index 3bd594eb2aed..ddd743c44735 100644
--- a/block.c
+++ b/block.c
@@ -464,7 +464,7 @@ BlockDriver *bdrv_find_format(const char *format_name)
/* The driver isn't registered, maybe we need to load a module */
for (i = 0; i < (int)ARRAY_SIZE(block_driver_modules); ++i) {
if (!strcmp(block_driver_modules[i].format_name, format_name)) {
-block_module_load_one(block_driver_modules[i].library_name);
+block_module_load(block_driver_modules[i].library_name);
break;
}
}
@@ -981,7 +981,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
for (i = 0; i < (int)ARRAY_SIZE(block_driver_modules); ++i) {
if (block_driver_modules[i].protocol_name &&
!strcmp(block_driver_modules[i].protocol_name, protocol)) {
-block_module_load_one(block_driver_modules[i].library_name);
+block_module_load(block_driver_modules[i].library_name);
break;
}
}
diff --git a/block/dmg.c b/block/dmg.c
index 422136276aa4..b5a93b086b20 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -445,8 +445,8 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
return ret;
}
-block_module_load_one("dmg-bz2");
-block_module_load_one("dmg-lzfse");
+block_module_load("dmg-bz2");
+block_module_load("dmg-lzfse");
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0806d8fcaaac..25dfc0846801 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -148,7 +148,7 @@ bool qdev_set_parent_bus(DeviceState *dev, BusState *bus,
Error **errp)
DeviceState *qdev_new(const char *name)
{
if (!object_class_by_name(name)) {
-module_load_qom_one(name);
+module_load_qom(name);
}
return DEVICE(object_new(name));
}
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 8c012bbe038d..b7911ce79161 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -61,16 +61,16 @@ typedef enum {
#define fuzz_target_init(function) module_init(function, \
MODULE_INIT_FUZZ_TARGET)
#define migration_init(function) module_init(function, MODULE_INIT_MIGRATION)
-#define block_module_load_one(lib) module_load_one("block-", lib)
-#define ui_module_load_one(lib) module_load_one("ui-", lib)
-#define audio_module_load_one(lib) module_load_one("audio-", lib)
+#define block_module_load(lib) module_load("block-", lib)
+#define ui_module_load(lib) module_load("ui-", lib)
+#define audio_module_load(lib) module_load("audio-", lib)
void register_module_init(void (*fn)(void), module_init_type type);
void register_dso_module_init(void (*fn)(void), module_init_type type);
void module_call_init(module_init_type type);
-bool module_load_one(const char *prefix, const char *lib_name);
-void module_load_qom_one(const char *type);
+bool module_load(const char *prefix, const char *lib_name);
+void module_load_qom(const char *type);
void module_load_qom_all(void);
void module_allow_arch(const char *arch);
diff --git a/qom/object.c b/qom/object.c
index e5cef30f6d1a..aba942bdf31c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -526,7 +526,7 @@ void object_initialize(void *data, size_t size, const char
*typename)
#ifdef CONFIG_MODULES
if (!type) {
-module_load_qom_one(typename);
+module_load_qom(typename);
type = type_get_by_name(typename);
}
#endif
@@ -1033,7 +1033,7 @@ ObjectClass *module_object_class_by_name(const char
*typename)
oc = object_class_by_name(typename);
#ifdef CONFIG_MODULES
if (!oc) {
-module_load_qom_one(typename);
+module_load_qom(typename);
oc = object_class_by_name(typename);
}
#endif
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index ff74c5d7092e..774354565165 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -756,7 +756,7 @@ static void
[PULL 00/12] Misc bugfix patches (+ improved module errors) for QEMU 7.2
The following changes since commit 6295a58ad1b73985b9c32d184de7d2ed1fbe1774: Merge tag 'pull-target-arm-20221104' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-11-04 11:01:17 -0400) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 5141e9a23fc9a890d66a5700920a5ffd8885121f: accel: abort if we fail to load the accelerator plugin (2022-11-06 09:48:50 +0100) * bug fixes for Win32 event loop * bug fixes for -Wextra * fix gdb XML for 32-bit x86 * improve error handling for module load Kevin's patch below is a bugfix that Claudio picked up, and became part of his series to improve error reporting for modules. Thanks, Paolo Bin Meng (3): util/main-loop: Fix maximum number of wait objects for win32 util/main-loop: Avoid adding the same HANDLE twice util/aio-win32: Correct the event array size in aio_poll() Claudio Fontana (4): module: removed unused function argument "mayfail" module: rename module_load_one to module_load module: add Error arguments to module_load and module_load_qom accel: abort if we fail to load the accelerator plugin Kevin Wolf (1): dmg: warn when opening dmg images containing blocks of unknown type Paolo Bonzini (1): meson: avoid unused arguments of main() in compiler tests Stefan Weil (2): Fix broken configure with -Wunused-parameter Add missing include statement for global xml_builtin TaiseiIto (1): gdb-xml: Fix size of EFER register on i386 architecture when debugged by GDB accel/accel-softmmu.c| 8 +- audio/audio.c| 16 ++-- block.c | 20 +++-- block/dmg.c | 33 +++- configure| 8 +- gdb-xml/i386-32bit.xml | 2 +- hw/core/qdev.c | 17 +++- include/qemu/main-loop.h | 2 + include/qemu/module.h| 37 +++-- meson.build | 8 +- qom/object.c | 18 +++- scripts/feature_to_c.sh | 1 + softmmu/qtest.c | 8 +- ui/console.c | 18 +++- util/aio-win32.c | 5 +- util/main-loop.c | 20 +++-- util/module.c| 211 ++- 17 files changed, 290 insertions(+), 142 deletions(-) -- 2.38.1
[PULL 04/12] gdb-xml: Fix size of EFER register on i386 architecture when debugged by GDB
From: TaiseiIto Before this commit, there were contradictory descriptions about size of EFER register. Line 113 says the size is 8 bytes. Line 129 says the size is 4 bytes. As a result, when GDB is debugging an OS running on QEMU, the GDB cannot read 'g' packets correctly. This 'g' packet transmits values of each registers of machine emulated by QEMU to GDB. QEMU, the packet sender, assign 4 bytes for EFER in 'g' packet based on the line 113. GDB, the packet receiver, extract 8 bytes for EFER in 'g' packet based on the line 129. Therefore, all registers located behind EFER in 'g' packet has been shifted 4 bytes in GDB. After this commit, GDB can read 'g' packets correctly. Signed-off-by: TaiseiIto Message-Id: Signed-off-by: Paolo Bonzini --- gdb-xml/i386-32bit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb-xml/i386-32bit.xml b/gdb-xml/i386-32bit.xml index 872fcea9c25a..7a66a02b67e3 100644 --- a/gdb-xml/i386-32bit.xml +++ b/gdb-xml/i386-32bit.xml @@ -110,7 +110,7 @@ - + -- 2.38.1
[PULL 02/12] util/main-loop: Avoid adding the same HANDLE twice
From: Bin Meng
Fix the logic in qemu_add_wait_object() to avoid adding the same
HANDLE twice, as the behavior is undefined when passing an array
that contains same HANDLEs to WaitForMultipleObjects() API.
Signed-off-by: Bin Meng
Message-Id: <20221019102015.2441622-2-bmeng...@gmail.com>
Signed-off-by: Paolo Bonzini
---
include/qemu/main-loop.h | 2 ++
util/main-loop.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index aac707d073a1..3c9a9a982def 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -157,6 +157,8 @@ typedef void WaitObjectFunc(void *opaque);
* in the main loop's calls to WaitForMultipleObjects. When the handle
* is in a signaled state, QEMU will call @func.
*
+ * If the same HANDLE is added twice, this function returns -1.
+ *
* @handle: The Windows handle to be observed.
* @func: A function to be called when @handle is in a signaled state.
* @opaque: A pointer-size value that is passed to @func.
diff --git a/util/main-loop.c b/util/main-loop.c
index de38876064e4..10fa74c6e319 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -373,10 +373,20 @@ static WaitObjects wait_objects = {0};
int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
{
+int i;
WaitObjects *w = _objects;
+
if (w->num >= MAXIMUM_WAIT_OBJECTS) {
return -1;
}
+
+for (i = 0; i < w->num; i++) {
+/* check if the same handle is added twice */
+if (w->events[i] == handle) {
+return -1;
+}
+}
+
w->events[w->num] = handle;
w->func[w->num] = func;
w->opaque[w->num] = opaque;
--
2.38.1
[PULL 01/12] util/main-loop: Fix maximum number of wait objects for win32
From: Bin Meng
The maximum number of wait objects for win32 should be
MAXIMUM_WAIT_OBJECTS, not MAXIMUM_WAIT_OBJECTS + 1.
Signed-off-by: Bin Meng
Message-Id: <20221019102015.2441622-1-bmeng...@gmail.com>
Signed-off-by: Paolo Bonzini
---
util/main-loop.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/util/main-loop.c b/util/main-loop.c
index f00a25451bdc..de38876064e4 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -363,10 +363,10 @@ void qemu_del_polling_cb(PollingFunc *func, void *opaque)
/* Wait objects support */
typedef struct WaitObjects {
int num;
-int revents[MAXIMUM_WAIT_OBJECTS + 1];
-HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
-WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS + 1];
-void *opaque[MAXIMUM_WAIT_OBJECTS + 1];
+int revents[MAXIMUM_WAIT_OBJECTS];
+HANDLE events[MAXIMUM_WAIT_OBJECTS];
+WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS];
+void *opaque[MAXIMUM_WAIT_OBJECTS];
} WaitObjects;
static WaitObjects wait_objects = {0};
@@ -395,7 +395,7 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc
*func, void *opaque)
if (w->events[i] == handle) {
found = 1;
}
-if (found) {
+if (found && i < (MAXIMUM_WAIT_OBJECTS - 1)) {
w->events[i] = w->events[i + 1];
w->func[i] = w->func[i + 1];
w->opaque[i] = w->opaque[i + 1];
--
2.38.1
[PULL 03/12] util/aio-win32: Correct the event array size in aio_poll()
From: Bin Meng
WaitForMultipleObjects() can only wait for MAXIMUM_WAIT_OBJECTS
object handles. Correct the event array size in aio_poll() and
add a assert() to ensure it does not cause out of bound access.
Signed-off-by: Bin Meng
Reviewed-by: Stefan Weil
Reviewed-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20221019102015.2441622-3-bmeng...@gmail.com>
Signed-off-by: Paolo Bonzini
---
util/aio-win32.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 44003d645ecd..80cfe012ad8f 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -326,9 +326,9 @@ void aio_dispatch(AioContext *ctx)
bool aio_poll(AioContext *ctx, bool blocking)
{
AioHandler *node;
-HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
+HANDLE events[MAXIMUM_WAIT_OBJECTS];
bool progress, have_select_revents, first;
-int count;
+unsigned count;
int timeout;
/*
@@ -369,6 +369,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
QLIST_FOREACH_RCU(node, >aio_handlers, node) {
if (!node->deleted && node->io_notify
&& aio_node_check(ctx, node->is_external)) {
+assert(count < MAXIMUM_WAIT_OBJECTS);
events[count++] = event_notifier_get_handle(node->e);
}
}
--
2.38.1
Re: [PATCH] tests/qtest/ac97-test: add up-/downsampling tests
Am 04.11.22 um 18:33 schrieb Thomas Huth:
On 26/10/2022 21.34, Volker Rümelin wrote:
Am 25.10.22 um 09:44 schrieb Marc-André Lureau:
Hi
On Tue, Oct 25, 2022 at 12:31 AM Volker Rümelin
wrote:
Am 24.10.22 um 10:13 schrieb Marc-André Lureau:
Hi
On Mon, Oct 24, 2022 at 9:28 AM Volker Rümelin
wrote:
Test if the audio subsystem can handle extreme up- and down-
sampling ratios like 44100/1 and 1/44100. For some time these
used to trigger QEMU aborts. The test was taken from
https://gitlab.com/qemu-project/qemu/-/issues/71 where it was
used to demonstrate a very different issue.
Suggested-by: Marc-André Lureau
Signed-off-by: Volker Rümelin
Thanks for working on this
It seems to show something different though:
"
A bug was just triggered in audio_calloc
Save all your work and restart without audio
I am sorry
"
AUD_open_out() is called with audsettings: {freq = 1, nchannels = 2,
fmt = AUDIO_FORMAT_S16, endianness = 0}
And that's it. Any idea?
Hi,
the scary message is expected and doesn't mean this qos-test failed.
This is the currently not so silent 'the audio subsystem should (...)
silently give up' case.
Ok, but it's not silent. According to the AC97 spec, "if the value
written to the register is supported that value will be echoed back
when read, otherwise the closest (higher in case of a tie) sample rate
supported is returned". We should probably pick a low sample rate,
like 8000 (see Table 32 in spec 2.1) for anything below it.
Hi,
I don't think we should limit the lowest sample rate to 8000 Hz. The
sample rates in AC97 revision 2.1 Table 32 are sample rates the codec
should support at minimum. We are free to support the whole 1-65535
Hz sample rate range.
FWIW, a minimum sample rate of 1 Hz also does not make much sense. You
cannot hear that frequency anymore... so it does not really make that
much sense to support such low frequencies here. Just my 0.02 €.
Hi,
sample rates below a minimum sample rate are currently not supported.
The audio device gets disabled. This is why you see the confusing
audio_bug() message. The minimum sample rate depends on the selected
audio backend and can be changed indirectly with -audiodev arguments. If
we change the AC97 minimum sample rate to 8000 Hz, it's much more
difficult to test this code path.
This is a convenient way to test edge cases. If you think the
audio_bug message is an issue, I'll improve the error handling in
AUD_open_* first and then resend this qos test.
I agree with Marc-André - the error message looks confusing when
running the test. Maybe you could simply fence it with qtest_enable()
at least?
I have written two patches for the audio subsystem to address this
issue. This was two days before QEMU 7.2 soft freeze. I'll send the
patches to the mailing list after the release of QEMU 7.2.
The noaudio backend uses a mixing-engine buffer size of 1024 audio
frames and AUD_open_* tries to allocate memory for 1024/44100 = 0.0232
audio frames for the resample buffer in
audio_pcm_sw_alloc_resources_*.
This allocation fails and produces the scary message. The error is
handled correctly and AUD_open_* returns NULL. AUD_read and AUD_write
return early if this pointer is NULL and the audio frontend callback
functions will also not be called because the audio_frontend_frames_*
functions return 0 in this case.
Thanks, it'd be nice to have such a description in the commit message.
I'll improve the commit message of patch version 2.
A v2 would be appreciated!
I won't forget it.
With best regards,
Volker
Thanks,
Thomas
Re: [PATCH trivial for 7.2 1/2] hw/usb/hcd-xhci.c: spelling: tranfer
On 05/11/2022 12.48, Michael Tokarev wrote: Fixes: effaf5a240e03020f4ae953e10b764622c3e87cc Signed-off-by: Michael Tokarev --- hw/usb/hcd-xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 8299f35e66..b89b618ec2 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -796,7 +796,7 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring) */ } while (length < TRB_LINK_LIMIT * 65536 / TRB_SIZE); -qemu_log_mask(LOG_GUEST_ERROR, "%s: exceeded maximum tranfer ring size!\n", +qemu_log_mask(LOG_GUEST_ERROR, "%s: exceeded maximum transfer ring size!\n", __func__); return -1; Reviewed-by: Thomas Huth
