This reverts commit 2d6cfbaf17.
The patch is supposed to be part of ARM CPU hotplug series and has not value
on its own without it. The series however is still in RFC stage and outside
of scope 9.2 release.
On top of that it introduces not needed callback that pokes directly into
CPU state without any need for that. Instead properties and AML generator
option should be used to configure static platform depended vCPU presence
state.
Drop the patch so that corrected version could be posted along with
ARM CPU hotplug series and properly reviewed in relevant context.
That also helps us to keep history cleaner with new patch being
against original code vs a string of fixups on top of current mess.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20241112170258.2996640-4-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
When the backend of vhost_net restarts during the vm is running, vhost_net
is stopped and started. The virtio_device_grab_ioeventfd() fucntion in
vhost_net_enable_notifiers() will result in a call to
virtio_bus_set_host_notifier()(assign=false).
And now virtio_device_grab_ioeventfd() is batched in a single transaction
with virtio_bus_set_host_notifier()(assign=true).
This triggers the following assertion:
kvm_mem_ioeventfd_del: error deleting ioeventfd: Bad file descriptor
This patch moves virtio_device_grab_ioeventfd() out of the batch to fix
this problem.
To be noted that the for loop to release ioeventfd should start from i+1,
not i, because the i-th ioeventfd has already been released in
vhost_dev_disable_notifiers_nvqs().
Fixes: 6166799f6 ("vhost_net: configure all host notifiers in a single MR transaction")
Signed-off-by: Zuo Boqun <zuoboqun@baidu.com>
Reported-by: Gao Shiyuan <gaoshiyuan@baidu.com>
Message-Id: <20241115080312.3184-1-zuoboqun@baidu.com>
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Coverity reports a possible buffer overrun due to a non-NUL-terminated
string in scsi_property_set_loadparm(). While things are not so easy,
because qdev_prop_sanitize_s390x_loadparm is designed to operate on a
buffer that is not NUL-terminated, in this case the string *does* have
to be NUL-terminated because it is read by scsi_property_get_loadparm
and s390_build_iplb.
Reviewed-by: jrossi@linux.ibm.com
Cc: thuth@redhat.com
Fixes: 429442e52d ("hw: Add "loadparm" property to scsi disk devices for booting on s390x", 2024-11-18)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Add subsubsections for possible boot methods and introduce a new
section on eMMC boot support for the ast2600-evb and rainier-emmc
machines, boot partitions assumptions and limitations.
Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Jan Luebbe <jlu@pengutronix.de>
Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
The trace-root.h file has the definitions of trace events for
the top-level trace-events file (i.e. for those events which are
used in source files in the root of the source tree). There's
no particular need for trace/control.c or trace/control-target.c
to include this.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20241108162909.4080314-4-peter.maydell@linaro.org
The documentation for trace events says that every subdirectory which
has trace events should have a trace.h header, whose only content is
an include of the trace/trace-<subdir>.h file.
When we added the trace events in target/arm/hvf/ we forgot to create
this file and instead hvf.c directly includes
trace/trace-target_arm_hvf.h.
Create the standard trace.h file to bring this into line with the
convention.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20241108162909.4080314-3-peter.maydell@linaro.org
In extioi_setirq() we try to operate on a bit array stored as an
array of uint32_t using the set_bit() and clear_bit() functions
by casting the pointer to 'unsigned long *'.
This has two problems:
* the alignment of 'uint32_t' is less than that of 'unsigned long'
so we pass an insufficiently aligned pointer, which is
undefined behaviour
* on big-endian hosts the 64-bit 'unsigned long' will have
its two halves the wrong way around, and we will produce
incorrect results
The undefined behaviour is shown by the clang undefined-behaviour
sanitizer when running the loongarch64-virt functional test:
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/bitops.h:41:5: runtime error: store to misaligned address 0x555559745d9c for type 'unsigned long', which requires 8 byte alignment
0x555559745d9c: note: pointer points here
ff ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
#0 0x555556fb81c4 in set_bit /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/bitops.h:41:9
#1 0x555556fb81c4 in extioi_setirq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../hw/intc/loongarch_extioi.c:65:9
#2 0x555556fb6e90 in pch_pic_irq_handler /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../hw/intc/loongarch_pch_pic.c:75:5
#3 0x555556710265 in serial_ioport_write /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../hw/char/serial.c
Fix these problems by using set_bit32() and clear_bit32(),
which work with bit arrays stored as an array of uint32_t.
Cc: qemu-stable@nongnu.org
Fixes: cbff2db1e9 ("hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Bibo Mao <maobibo@loongson.cn>
Message-id: 20241108135514.4006953-4-peter.maydell@linaro.org
Now we have official uint32_t bit array functions in bitops.h, use
them instead of the hand-rolled local versions.
We retain gic_bmp_replace_bit() because bitops doesn't provide that
specific functionality.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20241108135514.4006953-3-peter.maydell@linaro.org
Currently bitops.h defines a set of operations that work on
arbitrary-length bit arrays. However (largely because they
originally came from the Linux kernel) the bit array storage is an
array of 'unsigned long'. This is OK for the kernel and even for
parts of QEMU where we don't really care about the underlying storage
format, but it is not good for devices, where we often want to expose
the storage to the guest and so need a type that is not
variably-sized between host OSes.
We already have a workaround for this in the GICv3 model:
arm_gicv3_common.h defines equivalents of the bit operations that
work on uint32_t. It turns out that we should also be using
something similar in hw/intc/loongarch_extioi.c, which currently
casts a pointer to a uint32_t array to 'unsigned long *' in
extio_setirq(), which is both undefined behaviour and not correct on
a big-endian host.
Define equivalents of the set_bit() function family which work
with a uint32_t array.
(Cc stable because we're about to provide a bugfix to
loongarch_extioi which will depend on this commit.)
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20241108135514.4006953-2-peter.maydell@linaro.org
Currently the handling of page protection in the short-format
descriptor is open-coded. This means that we forgot to update
it to handle some newer architectural features, including:
* handling of SCTLR.{UWXN,WXN}
* handling of SCR.SIF
Make the short-format descriptor code call the same get_S1prot()
that we already use for the LPAE descriptor format. This makes
the code simpler and means it now correctly honours the WXN/UWXN
and SIF bits.
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Message-id: 20241118152537.45277-1-paskripkin@gmail.com
[PMM: fixed a couple of checkpatch nits, tweaked commit message]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
AP in armv7 short descriptor mode has 3 bits and also domain, which
makes it incompatible with other arm schemas.
To make it possible to share get_S1prot between armv8, armv7 long
format, armv7 short format and armv6 it's easier to make caller
decode AP.
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Message-id: 20241118152526.45185-1-paskripkin@gmail.com
[PMM: fixed checkpatch nit]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Currently the watchdog test has a behavior in which the first test
assertion that fails will make the test abort making it impossible to
see the result of other tests:
# ERROR:../tests/qtest/cmsdk-apb-watchdog-test.c:87:test_watchdog:
assertion failed ...
Bail out!
Aborted
Changing the behavior in order to let the test finish other tests and
report the ones that pass and fail:
# ERROR:../tests/qtest/cmsdk-apb-watchdog-test.c:101:test_watchdog:
assertion failed ...
not ok 1 /arm/cmsdk-apb-watchdog/watchdog
Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
Message-id: 20241115160328.1650269-5-roqueh@google.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Currently the CMSDK APB watchdog tests target an specialized version
of the device (luminaris using the lm3s811evb machine) that prevents
the development of tests for the more generic device documented in:
https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model
This patch allows the execution of the watchdog tests in an MPS2
machine (when applicable) which uses the generic version of the CMSDK
APB watchdog.
Finally the rules for compiling the test have to change because it is
possible not to have CONFIG_STELLARIS (required for the lm3s811evb
machine) while still having CONFIG_CMSDK_APB_WATCHDOG and the test
will fail. Due to the addition of the MPS2 machine CONFIG_MPS2
becomes also a dependency for the test compilation.
Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
Reviewed-by: Stephen Longfield <slongfield@google.com>
Message-id: 20241115160328.1650269-4-roqueh@google.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Current watchdog is free running out of reset, this combined with the
fact that current implementation also ensures the counter is running
when programing WDOGLOAD creates issues when the firmware defer the
programing of WDOGCONTROL.INTEN much later after WDOGLOAD. Arm
Programmer's Model documentation states that INTEN is also the
counter enable:
> INTEN
>
> Enable the interrupt event, WDOGINT. Set HIGH to enable the counter
> and the interrupt, or LOW to disable the counter and interrupt.
> Reloads the counter from the value in WDOGLOAD when the interrupt
> is enabled, after previously being disabled.
Source of the time of writing:
https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model
Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
Reviewed-by: Stephen Longfield <slongfield@google.com>
Reviewed-by: Joe Komlodi <komlodi@google.com>
Message-id: 20241115160328.1650269-3-roqueh@google.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
The clang sanitizer complains about the code in the EOI handling
of openpic_cpu_write_internal():
UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1 ./build/clang/qemu-system-ppc -M mac99,graphics=off -display none -kernel day15/invaders.elf
../../hw/intc/openpic.c:1034:16: runtime error: index -1 out of bounds for type 'IRQSource[264]' (aka 'struct IRQSource[264]')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/intc/openpic.c:1034:16 in
This is because we do
src = &opp->src[n_IRQ];
when n_IRQ may be -1. This is in practice harmless because if n_IRQ
is -1 then we don't do anything with the src pointer, but it is
undefined behaviour. (This has been present since this device
was first added to QEMU.)
Rearrange the code so we only do the array index when n_IRQ is not -1.
Cc: qemu-stable@nongnu.org
Fixes: e9df014c0b ("Implement embedded IRQ controller for PowerPC 6xx/740 & 75")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-id: 20241105180205.3074071-1-peter.maydell@linaro.org
* Fixes & doc updates for the new "boot order" s390x bios feature
* Provide a "loadparm" property for scsi-hd & scsi-cd devices on s390x
(required for the "boot order" feature)
* Fix the floating-point multiply-and-add NaN rules on s390x
* Raise timeout on cross-accel build jobs to 60m
# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEEJ7iIR+7gJQEY8+q5LtnXdP5wLbUFAmc7ercRHHRodXRoQHJl
# ZGhhdC5jb20ACgkQLtnXdP5wLbVjyg//ZuhSDCj+oBSU6vwM7Lwh3CS6GwZvGECU
# h60V3tizKypiRNtTJRXHoWcx95brXmoZgI+QQhDEXe3fFLkOEKT6AIlDhrKZRUsd
# rpLPr6O8TVKO+rSE7JVJAP3X1tpOOQDxnq83uWBv53b0S+Da0VwDRtI9gcugRMmh
# d58P8Q1bV344fQdcrebejstpSUG7RxSA4Plj2uSQx4mSHT7cy/hN+vA34Ha7reE3
# tcN9yfQq3Rmfvt0MV5I9Umd6JXEoDlEAwjSNsWRsCzo69jBZwiMtXSH8LyLtwRTp
# C919G/MIRuhvImF74dStLVCr82sNq54YR1NP6CGcmqPH76FOH8Mx3vmx9Cxj9ckA
# 6NI6SvIg++bW2O1efG2apz8p5fjbDzYXSAbHnaWTcEu3gPgH4PQ5QXoyKaDymvWV
# JIh5/gXEy+twEXgIBsdWQ44A9E06lL/tNfKnqGdXK4ZYF2JIrI+Lq7AKBee7tebP
# +72I4PljHLSHQ3GxdkoOeJ8ahu70IBdSz2/VEIwOWK1wIf5C5WFNBerLJyDmkyx8
# xIvIm0vlRLwPcuOC711nlaMaKqTNT+8W4DIqIY6fHs2Jy0psMdgey1uHQxYEj9Kh
# fg7CvalK8n3MkGAwTqAvRJIwMFe0a4Ss6c6CaemSaYa38ud/pCNnv+IT+Eqr+mjq
# 6y5PZWNrZi0=
# =UaDH
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 18 Nov 2024 17:34:47 GMT
# gpg: using RSA key 27B88847EEE0250118F3EAB92ED9D774FE702DB5
# gpg: issuer "thuth@redhat.com"
# gpg: Good signature from "Thomas Huth <th.huth@gmx.de>" [full]
# gpg: aka "Thomas Huth <thuth@redhat.com>" [full]
# gpg: aka "Thomas Huth <huth@tuxfamily.org>" [full]
# gpg: aka "Thomas Huth <th.huth@posteo.de>" [unknown]
# Primary key fingerprint: 27B8 8847 EEE0 2501 18F3 EAB9 2ED9 D774 FE70 2DB5
* tag 'pull-request-2024-11-18' of https://gitlab.com/thuth/qemu:
.gitlab-ci.d: Raise timeout on cross-accel build jobs to 60m
pc-bios: Update the s390 bios images with the recent fixes
pc-bios/s390-ccw: Re-initialize receive queue index before each boot attempt
pc-bios/s390x: Initialize machine loadparm before probing IPL devices
pc-bios/s390x: Initialize cdrom type to false for each IPL device
hw: Add "loadparm" property to scsi disk devices for booting on s390x
hw/s390x: Restrict "loadparm" property to devices that can be used for booting
docs/system/bootindex: Make it clear that s390x can also boot from virtio-net
docs/system/s390x/bootdevices: Update loadparm documentation
tests/tcg/s390x: Add the floating-point multiply-and-add test
target/s390x: Fix the floating-point multiply-and-add NaN rules
hw/usb: Use __attribute__((packed)) vs __packed
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
The current 30 minute timeout on the cross_accel_build_job template
is a bit low: sometimes if the k8s runners are running slow the
can hit it, for example this cross-arm64-xen-only job hit the
30 minute timeout while still not quite finished with the compile:
https://gitlab.com/qemu-project/qemu/-/jobs/8401277985
This is partly a "runner performance can be unpredictable" issue:
https://gitlab.com/qemu-project/qemu/-/jobs/8391726482
is the same job from just a day earlier and it finished in
16 minutes. But we already have build jobs that are higher
timeouts than 30 minutes, so we have headroom to raise the
timeout here to something we're less likely to hit on a slow
runner.
Bump the cross_accel_build_job timeout to 60 mins.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20241118153226.1524542-1-peter.maydell@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Now that we can boot from multiple boot devices, we have to make sure
to reinitialize static variables like rx_last_idx to avoid that they
contain garbage data during the second boot attempt (which can lead to
crashes when the code tries to access the wrong ring data).
Message-ID: <20241111131120.317796-1-thuth@redhat.com>
Reviewed-by: Jared Rossi <jrossi@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Commit bb185de423 ("s390x: Add individual loadparm assignment to
CCW device") allowed boot devices to be assigned a loadparm value independent
of the machine value, however, when no boot devices are defined, the machine
loadparm becomes ignored. Therefore, let's check the machine loadparm
prior to probing the devices.
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20241114161952.3508554-1-jrossi@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
While adding the new flexible boot order feature on s390x recently,
we missed to add the "loadparm" property to the scsi-hd and scsi-cd
devices. This property is required on s390x to pass the information
to the boot loader about which kernel should be started or whether
the boot menu should be shown. But even more serious: The missing
property is now causing trouble with the corresponding libvirt patches
that assume that the "loadparm" property is either settable for all
bootable devices (when the "boot order" feature is implemented in
QEMU), or none (meaning the behaviour of older QEMUs that only allowed
one "loadparm" at the machine level). To fix this broken situation,
let's implement the "loadparm" property in for the SCSI devices, too.
Message-ID: <20241115141202.1877294-1-thuth@redhat.com>
Acked-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
GDB 15 does not like exit() anymore:
(gdb) python exit(0)
Python Exception <class 'SystemExit'>: 0
Error occurred in Python: 0
Use the GDB's own exit command, like it's already done in a couple
places, everywhere. This is the same fix as commit 93a3048dcf
("tests: Gently exit from GDB when tests complete"), but applied to
more places.
Acked-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20241022113939.19989-1-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
While testing the use of qemu-nbd in a Pod of a Kubernetes cluster, I
got LOTS of log messages of the forms:
qemu-nbd: option negotiation failed: Failed to read flags: Unexpected end-of-file before all data were read
qemu-nbd: option negotiation failed: Failed to read flags: Unable to read from socket: Connection reset by peer
While it is nice to warn about clients that aren't following protocol
(in case it helps diagnosing bugs in those clients), a mere port probe
(where the client never write()s any bytes, and where we might even
hit EPIPE in trying to send our greeting to the client) is NOT
abnormal, but merely serves to pollute the log. And Kubernetes
_really_ likes to do port probes to determine whether a given Pod is
up and running.
Easy ways to demonstrate the above port probes:
$ qemu-nbd -r -f raw path/to/file &
$ nc localhost 10809 </dev/null
$ bash -c 'exec </dev/tcp/localhost/10809'
$ kill $!
Silence the noise by not capturing errors until after our first
successful read() from a client.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20241115195638.1132007-2-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
The ip_header is not actually guaranteed to be aligned. We attempt to
deal with this in some places such as net_checksum_calculate() by
using stw_be_p and so on to access the fields, but this is not
sufficient to be correct, because even accessing a byte member
within an unaligned struct is undefined behaviour. The clang
sanitizer will emit warnings like these if net_checksum_calculate()
is called:
Stopping network: ../../net/checksum.c:106:9: runtime error: member access within misaligned address 0x556aad9b502e for type 'struct ip_header', which requires 4 byte alignment
0x556aad9b502e: note: pointer points here
34 56 08 00 45 00 01 48 a5 09 40 00 40 11 7c 8b 0a 00 02 0f 0a 00 02 02 00 44 00 43 01 34 19 56
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../net/checksum.c:106:9 in
../../net/checksum.c:106:9: runtime error: load of misaligned address 0x556aad9b502e for type 'uint8_t' (aka 'unsigned char'), which requires 4 byte alignment
0x556aad9b502e: note: pointer points here
34 56 08 00 45 00 01 48 a5 09 40 00 40 11 7c 8b 0a 00 02 0f 0a 00 02 02 00 44 00 43 01 34 19 56
^
Fix this by marking the ip_header struct as QEMU_PACKED, so that
the compiler knows that it might be unaligned and will generate
the right code for accessing fields.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20241114141619.806652-3-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Vladimir Sementsov-Ogievskiy
Jonathan Cameron
Igor Mammedov
Salil Mehta
zuoboqun
Peter Maydell
Paolo Bonzini
Cédric Le Goater
Zhao Liu
Pierrick Bouvier
Kamil Szczęk
Pavel Skripkin
Roque Arcudia Hernandez
Rodrigo Dias Correa
Dmitry Frolov
Thomas Huth
Jared Rossi
Alex Bennée
Manos Pitsidianakis
Ilya Leoshkevich
Eric Blake