From 79b38d61b53ffba95dcbf5ff6a1ba8f6f4eefbef Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 7 Jun 2024 15:29:33 -0700 Subject: [PATCH 01/13] hw/openrisc: Fixed undercounting of TTCR in continuous mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the existing design, TTCR is prone to undercounting when running in continuous mode. This manifests as a timer interrupt appearing to trigger a few cycles prior to the deadline set in SPR_TTMR_TP. When the timer triggers, the virtual time delta in nanoseconds between the time when the timer was set, and when it triggers is calculated. This nanoseconds value is then divided by TIMER_PERIOD (50) to compute an increment of cycles to apply to TTCR. However, this calculation rounds down the number of cycles causing the undercounting. A simplistic solution would be to instead round up the number of cycles, however this will result in the accumulation of timing error over time. This patch corrects the issue by calculating the time delta in nanoseconds between when the timer was last reset and the timer event. This approach allows the TTCR value to be rounded up, but without accumulating error over time. Signed-off-by: Joel Holdsworth [stafford: Incremented version in vmstate_or1k_timer, checkpatch fixes] Signed-off-by: Stafford Horne Message-ID: <20241203110536.402131-3-shorne@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/openrisc/cputimer.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c index 835986c4db..87aa353323 100644 --- a/hw/openrisc/cputimer.c +++ b/hw/openrisc/cputimer.c @@ -29,7 +29,8 @@ /* Tick Timer global state to allow all cores to be in sync */ typedef struct OR1KTimerState { uint32_t ttcr; - uint64_t last_clk; + uint32_t ttcr_offset; + uint64_t clk_offset; } OR1KTimerState; static OR1KTimerState *or1k_timer; @@ -37,6 +38,8 @@ static OR1KTimerState *or1k_timer; void cpu_openrisc_count_set(OpenRISCCPU *cpu, uint32_t val) { or1k_timer->ttcr = val; + or1k_timer->ttcr_offset = val; + or1k_timer->clk_offset = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); } uint32_t cpu_openrisc_count_get(OpenRISCCPU *cpu) @@ -53,9 +56,8 @@ void cpu_openrisc_count_update(OpenRISCCPU *cpu) return; } now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); - or1k_timer->ttcr += (uint32_t)((now - or1k_timer->last_clk) - / TIMER_PERIOD); - or1k_timer->last_clk = now; + or1k_timer->ttcr = or1k_timer->ttcr_offset + + DIV_ROUND_UP(now - or1k_timer->clk_offset, TIMER_PERIOD); } /* Update the next timeout time as difference between ttmr and ttcr */ @@ -69,7 +71,7 @@ void cpu_openrisc_timer_update(OpenRISCCPU *cpu) } cpu_openrisc_count_update(cpu); - now = or1k_timer->last_clk; + now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); if ((cpu->env.ttmr & TTMR_TP) <= (or1k_timer->ttcr & TTMR_TP)) { wait = TTMR_TP - (or1k_timer->ttcr & TTMR_TP) + 1; @@ -110,7 +112,8 @@ static void openrisc_timer_cb(void *opaque) case TIMER_NONE: break; case TIMER_INTR: - or1k_timer->ttcr = 0; + /* Zero the count by applying a negative offset to the counter */ + or1k_timer->ttcr_offset -= (cpu->env.ttmr & TTMR_TP); break; case TIMER_SHOT: cpu_openrisc_count_stop(cpu); @@ -137,17 +140,18 @@ static void openrisc_count_reset(void *opaque) /* Reset the global timer state. */ static void openrisc_timer_reset(void *opaque) { - or1k_timer->ttcr = 0x00000000; - or1k_timer->last_clk = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + OpenRISCCPU *cpu = opaque; + cpu_openrisc_count_set(cpu, 0); } static const VMStateDescription vmstate_or1k_timer = { .name = "or1k_timer", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (const VMStateField[]) { VMSTATE_UINT32(ttcr, OR1KTimerState), - VMSTATE_UINT64(last_clk, OR1KTimerState), + VMSTATE_UINT32(ttcr_offset, OR1KTimerState), + VMSTATE_UINT64(clk_offset, OR1KTimerState), VMSTATE_END_OF_LIST() } }; From 5d8a250f906815a56051aa402e80c0824ba9b9de Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Thu, 22 Aug 2024 18:38:38 +0200 Subject: [PATCH 02/13] hw/openrisc/openrisc_sim: keep serial@90000000 as default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We used to only have a single UART on the platform and it was located at address 0x90000000. When the number of UARTs was increased to 4, the first UART remained at it's location, but instead of being the first one to be registered, it became the last. This caused QEMU to pick 0x90000300 as the default UART, which broke software that hardcoded the address of 0x90000000 and expected it's output to be visible when the user configured only a single console. This caused regressions[1] in the barebox test suite when updating to a newer QEMU. As there seems to be no good reason to register the UARTs in inverse order, let's register them by ascending address, so existing software can remain oblivious to the additional UART ports. Changing the order of uart registration alone breaks Linux which was choosing the UART at 0x90000300 as the default for ttyS0. To fix Linux we fix three things in the device tree: 1. Define stdout-path only one time for the first registered UART instead of incorrectly defining for each UART. 2. Change the UART alias name from 'uart0' to 'serial0' as almost all Linux tty drivers look for an alias starting with "serial". 3. Add the UART nodes so they appear in the final DTB in the order starting with the lowest address and working upwards. In summary these changes mean that the QEMU default UART (serial_hd(0)) is now setup where: * serial_hd(0) is the lowest-address UART * serial_hd(0) is listed first in the DTB * serial_hd(0) is the /chosen/stdout-path one * the /aliases/serial0 alias points at serial_hd(0) [1]: https://lore.barebox.org/barebox/707e7c50-aad1-4459-8796-0cc54bab32e2@pengutronix.de/T/#m5da26e8a799033301489a938b5d5667b81cef6ad [stafford: Change to serial0 alias and update change message, reverse uart registration order] Fixes: 777784bda468 ("hw/openrisc: support 4 serial ports in or1ksim") Cc: qemu-stable@nongnu.org Signed-off-by: Ahmad Fatoum Signed-off-by: Stafford Horne Reviewed-by: Peter Maydell Message-ID: <20241203110536.402131-2-shorne@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/openrisc/openrisc_sim.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c index 9fb63515ef..42f002985b 100644 --- a/hw/openrisc/openrisc_sim.c +++ b/hw/openrisc/openrisc_sim.c @@ -250,7 +250,7 @@ static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base, void *fdt = state->fdt; char *nodename; qemu_irq serial_irq; - char alias[sizeof("uart0")]; + char alias[sizeof("serial0")]; int i; if (num_cpus > 1) { @@ -265,7 +265,7 @@ static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base, serial_irq = get_cpu_irq(cpus, 0, irq_pin); } serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200, - serial_hd(OR1KSIM_UART_COUNT - uart_idx - 1), + serial_hd(uart_idx), DEVICE_NATIVE_ENDIAN); /* Add device tree node for serial. */ @@ -277,10 +277,13 @@ static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base, qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency", OR1KSIM_CLK_MHZ); qemu_fdt_setprop(fdt, nodename, "big-endian", NULL, 0); - /* The /chosen node is created during fdt creation. */ - qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename); - snprintf(alias, sizeof(alias), "uart%d", uart_idx); + if (uart_idx == 0) { + /* The /chosen node is created during fdt creation. */ + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename); + } + snprintf(alias, sizeof(alias), "serial%d", uart_idx); qemu_fdt_setprop_string(fdt, "/aliases", alias, nodename); + g_free(nodename); } @@ -326,11 +329,22 @@ static void openrisc_sim_init(MachineState *machine) smp_cpus, cpus, OR1KSIM_OMPIC_IRQ); } - for (n = 0; n < OR1KSIM_UART_COUNT; ++n) + /* + * We create the UART nodes starting with the highest address and + * working downwards, because in QEMU the DTB nodes end up in the + * DTB in reverse order of creation. Correctly-written guest software + * will not care about the node order (it will look at stdout-path + * or the alias nodes), but for the benefit of guest software which + * just looks for the first UART node in the DTB, make sure the + * lowest-address UART (which is QEMU's first serial port) appears + * first in the DTB. + */ + for (n = OR1KSIM_UART_COUNT - 1; n >= 0; n--) { openrisc_sim_serial_init(state, or1ksim_memmap[OR1KSIM_UART].base + or1ksim_memmap[OR1KSIM_UART].size * n, or1ksim_memmap[OR1KSIM_UART].size, smp_cpus, cpus, OR1KSIM_UART_IRQ, n); + } load_addr = openrisc_load_kernel(ram_size, kernel_filename, &boot_info.bootstrap_pc); From 9cf6e41fe293dd56089faac94c36ff5cb3d96726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 17 Sep 2024 14:07:56 +0200 Subject: [PATCH 03/13] ui/cocoa: Temporarily ignore annoying deprecated declaration warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These warnings are breaking some build configurations since 2 months now (https://gitlab.com/qemu-project/qemu/-/issues/2575): ui/cocoa.m:662:14: error: 'CVDisplayLinkCreateWithCGDisplay' is deprecated: first deprecated in macOS 15.0 - use NSView.displayLink(target:selector:), NSWindow.displayLink(target:selector:), or NSScreen.displayLink(target:selector:) [-Werror,-Wdeprecated-declarations] 662 | if (!CVDisplayLinkCreateWithCGDisplay(display, &displayLink)) { | ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVDisplayLink.h:89:20: note: 'CVDisplayLinkCreateWithCGDisplay' has been explicitly marked deprecated here 89 | CV_EXPORT CVReturn CVDisplayLinkCreateWithCGDisplay( | ^ ui/cocoa.m:663:29: error: 'CVDisplayLinkGetNominalOutputVideoRefreshPeriod' is deprecated: first deprecated in macOS 15.0 - use NSView.displayLink(target:selector:), NSWindow.displayLink(target:selector:), or NSScreen.displayLink(target:selector:) [-Werror,-Wdeprecated-declarations] 663 | CVTime period = CVDisplayLinkGetNominalOutputVideoRefreshPeriod(displayLink); | ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVDisplayLink.h:182:18: note: 'CVDisplayLinkGetNominalOutputVideoRefreshPeriod' has been explicitly marked deprecated here 182 | CV_EXPORT CVTime CVDisplayLinkGetNominalOutputVideoRefreshPeriod( CVDisplayLinkRef CV_NONNULL displayLink ); | ^ ui/cocoa.m:664:13: error: 'CVDisplayLinkRelease' is deprecated: first deprecated in macOS 15.0 - use NSView.displayLink(target:selector:), NSWindow.displayLink(target:selector:), or NSScreen.displayLink(target:selector:) [-Werror,-Wdeprecated-declarations] 664 | CVDisplayLinkRelease(displayLink); | ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVDisplayLink.h:249:16: note: 'CVDisplayLinkRelease' has been explicitly marked deprecated here 249 | CV_EXPORT void CVDisplayLinkRelease( CV_RELEASES_ARGUMENT CVDisplayLinkRef CV_NULLABLE displayLink ); | ^ 3 errors generated. For the next release, ignore the warnings using #pragma directives. At least until we figure the correct new API usage. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Phil Dennis-Jordan Tested-by: Phil Dennis-Jordan Message-Id: <20241121131954.98949-1-philmd@linaro.org> --- ui/cocoa.m | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ui/cocoa.m b/ui/cocoa.m index 4c2dd33532..dd88115dc6 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -639,6 +639,9 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven [self setBoundsSize:NSMakeSize(screen.width, screen.height)]; } +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + - (void) updateUIInfoLocked { /* Must be called with the BQL, i.e. via updateUIInfo */ @@ -685,6 +688,8 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven dpy_set_ui_info(dcl.con, &info, TRUE); } +#pragma clang diagnostic pop + - (void) updateUIInfo { if (!allow_events) { From 9c3934b33c90efcb167354b9e86f294109706ccc Mon Sep 17 00:00:00 2001 From: Bibo Mao Date: Tue, 12 Nov 2024 15:37:14 +0800 Subject: [PATCH 04/13] MAINTAINERS: add myself as the maintainer for LoongArch VirtMachine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Song Gao is will be sick leave for a long time, I apply for maintainer for LoongArch Virt Machine during this period, LoongArch TCG keeps unchanged since I am not familiar with it. The maintainer duty will transfer to him after he comes back to work. Signed-off-by: Bibo Mao Acked-by: Philippe Mathieu-Daudé Message-ID: <20241112073714.1953481-1-maobibo@loongson.cn> Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 2b1c4abed6..3d6194684f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1199,6 +1199,7 @@ LoongArch Machines ------------------ Virt M: Song Gao +M: Bibo Mao R: Jiaxun Yang S: Maintained F: docs/system/loongarch/virt.rst From 51625575747943ca118abfb179ee06e4b103fe0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 20 Nov 2024 12:36:43 +0100 Subject: [PATCH 05/13] meson: Add missing SDL dependency to system/main.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building QEMU configure with --disable-gtk --disable-cocoa on macOS we get: User interface Cocoa support : NO SDL support : YES 2.30.5 SDL image support : NO GTK support : NO pixman : YES 0.42.2 VTE support : NO PNG support : YES 1.6.43 VNC support : YES VNC SASL support : YES VNC JPEG support : YES 3.0.3 spice protocol support : YES 0.14.4 spice server support : NO curses support : YES brlapi support : NO User defined options cocoa : disabled docs : disabled gtk : disabled ../system/main.c:30:10: fatal error: 'SDL.h' file not found 30 | #include | ^~~~~~~ 1 error generated. Fix by adding the SDL dependency to main.c it's CFLAGS contains the SDL include directory. Fixes: 64ed6f92ff ("meson: link emulators without Makefile.target") Signed-off-by: Philippe Mathieu-Daudé Acked-by: Paolo Bonzini Message-Id: <20241120114943.85080-1-philmd@linaro.org> --- meson.build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/meson.build b/meson.build index a290dbfa33..147097c652 100644 --- a/meson.build +++ b/meson.build @@ -4235,14 +4235,14 @@ foreach target : target_dirs 'name': 'qemu-system-' + target_name, 'win_subsystem': 'console', 'sources': files('system/main.c'), - 'dependencies': [] + 'dependencies': [sdl] }] if host_os == 'windows' and (sdl.found() or gtk.found()) execs += [{ 'name': 'qemu-system-' + target_name + 'w', 'win_subsystem': 'windows', 'sources': files('system/main.c'), - 'dependencies': [] + 'dependencies': [sdl] }] endif if get_option('fuzzing') From 2dfe93699cd447224fa88d5890ad1d2397b7a9f6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Sat, 23 Nov 2024 08:46:40 -0800 Subject: [PATCH 06/13] MAINTAINERS: update email addr for Brian Cain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also: add mapping for "quic_bcain@quicinc.com" which was ~briefly used for some replies to mailing list traffic. Signed-off-by: Brian Cain Signed-off-by: Brian Cain Tested-by: Philippe Mathieu-Daudé Message-ID: <20241123164641.364748-2-bcain@quicinc.com> Signed-off-by: Philippe Mathieu-Daudé --- .mailmap | 2 ++ MAINTAINERS | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index ef1b8a53f4..727ce204b2 100644 --- a/.mailmap +++ b/.mailmap @@ -75,6 +75,8 @@ Aleksandar Rikalo Alexander Graf Ani Sinha Anthony Liguori Anthony Liguori +Brian Cain +Brian Cain Christian Borntraeger Damien Hedde Filip Bozuta diff --git a/MAINTAINERS b/MAINTAINERS index 3d6194684f..f92be04775 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -226,7 +226,7 @@ F: target/avr/ F: tests/functional/test_avr_mega2560.py Hexagon TCG CPUs -M: Brian Cain +M: Brian Cain S: Supported F: target/hexagon/ X: target/hexagon/idef-parser/ From 235560b3a771f4bc05cf3e1c267ba8e7451576ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Wed, 27 Nov 2024 11:40:57 +0000 Subject: [PATCH 07/13] hw/core/machine: diagnose wrapping of maxmem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'maxmem' parameter parsed on the command line is held in uint64_t and then assigned to the MachineState field that is 'ram_addr_t'. This assignment will wrap on 32-bit hosts, silently changing the user's config request if it were over-sized. Improve the existing diagnositics for validating 'size', and add the same diagnostics for 'maxmem' Signed-off-by: Daniel P. Berrangé Tested-by: Ani Sinha Reviewed-by: Ani Sinha Message-ID: <20241127114057.255995-1-berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/core/machine.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index a35c4a8fae..f29fe95964 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -598,11 +598,19 @@ static void machine_set_mem(Object *obj, Visitor *v, const char *name, mem->size = mc->fixup_ram_size(mem->size); } if ((ram_addr_t)mem->size != mem->size) { - error_setg(errp, "ram size too large"); + error_setg(errp, "ram size %llu exceeds permitted maximum %llu", + (unsigned long long)mem->size, + (unsigned long long)RAM_ADDR_MAX); goto out_free; } if (mem->has_max_size) { + if ((ram_addr_t)mem->max_size != mem->max_size) { + error_setg(errp, "ram size %llu exceeds permitted maximum %llu", + (unsigned long long)mem->max_size, + (unsigned long long)RAM_ADDR_MAX); + goto out_free; + } if (mem->max_size < mem->size) { error_setg(errp, "invalid value of maxmem: " "maximum memory size (0x%" PRIx64 ") must be at least " From 5311599cdc48337f2f27b1b51a80d46d75b05ed0 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 28 Nov 2024 10:38:31 +0000 Subject: [PATCH 08/13] target/riscv: Avoid bad shift in riscv_cpu_do_interrupt() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In riscv_cpu_do_interrupt() we use the 'cause' value we got out of cs->exception as a shift value. However this value can be larger than 31, which means that "1 << cause" is undefined behaviour, because we do the shift on an 'int' type. This causes the undefined behaviour sanitizer to complain on one of the check-tcg tests: $ UBSAN_OPTIONS=print_stacktrace=1:abort_on_error=1:halt_on_error=1 ./build/clang/qemu-system-riscv64 -M virt -semihosting -display none -device loader,file=build/clang/tests/tcg/riscv64-softmmu/issue1060 ../../target/riscv/cpu_helper.c:1805:38: runtime error: shift exponent 63 is too large for 32-bit type 'int' #0 0x55f2dc026703 in riscv_cpu_do_interrupt /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../target/riscv/cpu_helper.c:1805:38 #1 0x55f2dc3d170e in cpu_handle_exception /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../accel/tcg/cpu-exec.c:752:9 In this case cause is RISCV_EXCP_SEMIHOST, which is 0x3f. Use 1ULL instead to ensure that the shift is in range. Signed-off-by: Peter Maydell Fixes: 1697837ed9 ("target/riscv: Add M-mode virtual interrupt and IRQ filtering support.") Fixes: 40336d5b1d ("target/riscv: Add HS-mode virtual interrupt and IRQ filtering support.") Reviewed-by: Daniel Henrique Barboza Reviewed-by: Richard Henderson Reviewed-by: Alistair Francis Message-ID: <20241128103831.3452572-1-peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé --- target/riscv/cpu_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 0a3ead69ea..45806f5ab0 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -1802,10 +1802,10 @@ void riscv_cpu_do_interrupt(CPUState *cs) bool async = !!(cs->exception_index & RISCV_EXCP_INT_FLAG); target_ulong cause = cs->exception_index & RISCV_EXCP_INT_MASK; uint64_t deleg = async ? env->mideleg : env->medeleg; - bool s_injected = env->mvip & (1 << cause) & env->mvien && - !(env->mip & (1 << cause)); - bool vs_injected = env->hvip & (1 << cause) & env->hvien && - !(env->mip & (1 << cause)); + bool s_injected = env->mvip & (1ULL << cause) & env->mvien && + !(env->mip & (1ULL << cause)); + bool vs_injected = env->hvip & (1ULL << cause) & env->hvien && + !(env->mip & (1ULL << cause)); target_ulong tval = 0; target_ulong tinst = 0; target_ulong htval = 0; From 302075f85e29d6e658aeec75f50a90eec23f3726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 28 Nov 2024 18:54:09 +0100 Subject: [PATCH 09/13] hw/display/vga: Do not reset 'big_endian_fb' in vga_common_reset() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'pci-vga' device allow setting a 'big-endian-framebuffer' property since commit 3c2784fc864 ("vga: Expose framebuffer byteorder as a QOM property"). Similarly, the 'virtio-vga' device since commit 8be61ce2ce3 ("virtio-vga: implement big-endian-framebuffer property"). Both call vga_common_reset() in their reset handler, respectively pci_secondary_vga_reset() and virtio_vga_base_reset_hold(), which reset 'big_endian_fb', overwritting the property. This is not correct: the hardware is expected to keep its configured endianness during resets. Move 'big_endian_fb' assignment from vga_common_reset() to vga_common_init() which is called once when the common VGA state is initialized. Signed-off-by: Philippe Mathieu-Daudé Acked-by: Benjamin Herrenschmidt Message-Id: <20241129101721.17836-2-philmd@linaro.org> --- hw/display/vga.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 892fedc8dc..b074b58c90 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1873,7 +1873,6 @@ void vga_common_reset(VGACommonState *s) s->cursor_start = 0; s->cursor_end = 0; s->cursor_offset = 0; - s->big_endian_fb = s->default_endian_fb; memset(s->invalidated_y_table, '\0', sizeof(s->invalidated_y_table)); memset(s->last_palette, '\0', sizeof(s->last_palette)); memset(s->last_ch_attr, '\0', sizeof(s->last_ch_attr)); @@ -2266,6 +2265,7 @@ bool vga_common_init(VGACommonState *s, Object *obj, Error **errp) * all target endian dependencies from this file. */ s->default_endian_fb = target_words_bigendian(); + s->big_endian_fb = s->default_endian_fb; vga_dirty_log_start(s); From bff1050a5630ce5da6f43ed002725d52140bb9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 29 Nov 2024 13:55:05 +0000 Subject: [PATCH 10/13] hw/virtio: fix crash in processing balloon stats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit balloon_stats_get_all will iterate over guest stats upto the max VIRTIO_BALLOON_S_NR value, calling visit_type_uint64 to populate the QObject dict. The dict keys are obtained from the static array balloon_stat_names which is VIRTIO_BALLOON_S_NR in size. Unfortunately the way that array is declared results in any unassigned stats getting a NULL name, which will then cause visit_type_uint64 to trigger an assert in qobject_output_add_obj. The balloon_stat_names array was fortunately fully populated with names until recently: commit 0d2eeef77a33315187df8519491a900bde4a3d83 Author: Bibo Mao Date: Mon Oct 28 10:38:09 2024 +0800 linux-headers: Update to Linux v6.12-rc5 pulled a change to include/standard-headers/linux/virtio_balloon.h which increased VIRTIO_BALLOON_S_NR by 6, and failed to add the new names to balloon_stat_names. This commit fills in the missing names, and uses a static assert to guarantee that any future changes to VIRTIO_BALLOON_S_NR will cause a build failure until balloon_stat_names is updated. This problem was detected by the Cockpit Project's automated integration tests on QEMU 9.2.0-rc1. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2329448 Fixes: 0d2eeef77a3 ("linux-headers: Update to Linux v6.12-rc5") Reported-by: Martin Pitt Reviewed-by: Richard W.M. Jones Signed-off-by: Daniel P. Berrangé Reviewed-by: David Hildenbrand Reviewed-by: Michael Tokarev Acked-by: Michael S. Tsirkin Message-ID: <20241129135507.699030-2-berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/virtio/virtio-balloon.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 609e39a821..afd2ad6dd6 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -167,19 +167,33 @@ static void balloon_deflate_page(VirtIOBalloon *balloon, } } +/* + * All stats upto VIRTIO_BALLOON_S_NR /must/ have a + * non-NULL name declared here, since these are used + * as keys for populating the QDict with stats + */ static const char *balloon_stat_names[] = { [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in", [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out", [VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults", [VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults", [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory", + [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory", [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory", [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches", [VIRTIO_BALLOON_S_HTLB_PGALLOC] = "stat-htlb-pgalloc", [VIRTIO_BALLOON_S_HTLB_PGFAIL] = "stat-htlb-pgfail", - [VIRTIO_BALLOON_S_NR] = NULL + + [VIRTIO_BALLOON_S_OOM_KILL] = "stat-oom-kills", + [VIRTIO_BALLOON_S_ALLOC_STALL] = "stat-alloc-stalls", + [VIRTIO_BALLOON_S_ASYNC_SCAN] = "stat-async-scans", + [VIRTIO_BALLOON_S_DIRECT_SCAN] = "stat-direct-scans", + [VIRTIO_BALLOON_S_ASYNC_RECLAIM] = "stat-async-reclaims", + + [VIRTIO_BALLOON_S_DIRECT_RECLAIM] = "stat-direct-reclaims", }; +G_STATIC_ASSERT(G_N_ELEMENTS(balloon_stat_names) == VIRTIO_BALLOON_S_NR); /* * reset_stats - Mark all items in the stats array as unset From 8460459529215e29c7f84efec69a329194ad4f87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 29 Nov 2024 13:55:06 +0000 Subject: [PATCH 11/13] tests/qtest: drop 'fuzz-' prefix from virtio-balloon test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test file is expected to be extended for arbitrary virtio-balloon related tests, not merely those discovered by fuzzing. Signed-off-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Fabiano Rosas Message-ID: <20241129135507.699030-3-berrange@redhat.com> [PMD: Update MAINTAINERS] Signed-off-by: Philippe Mathieu-Daudé Acked-by: Michael S. Tsirkin --- MAINTAINERS | 1 + tests/qtest/meson.build | 2 +- .../{fuzz-virtio-balloon-test.c => virtio-balloon-test.c} | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) rename tests/qtest/{fuzz-virtio-balloon-test.c => virtio-balloon-test.c} (84%) diff --git a/MAINTAINERS b/MAINTAINERS index f92be04775..aaf0505a21 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2228,6 +2228,7 @@ F: hw/virtio/virtio-balloon*.c F: include/hw/virtio/virtio-balloon.h F: system/balloon.c F: include/sysemu/balloon.h +F: tests/qtest/virtio-balloon-test.c virtio-9p M: Greg Kurz diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index f2f35367ae..bd41c9da5f 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -88,7 +88,7 @@ qtests_i386 = \ (config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ? ['fuzz-megasas-test'] : []) + \ (config_all_devices.has_key('CONFIG_LSI_SCSI_PCI') ? ['fuzz-lsi53c895a-test'] : []) + \ (config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? ['fuzz-virtio-scsi-test'] : []) + \ - (config_all_devices.has_key('CONFIG_VIRTIO_BALLOON') ? ['fuzz-virtio-balloon-test'] : []) + \ + (config_all_devices.has_key('CONFIG_VIRTIO_BALLOON') ? ['virtio-balloon-test'] : []) + \ (config_all_devices.has_key('CONFIG_Q35') ? ['q35-test'] : []) + \ (config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \ (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) + \ diff --git a/tests/qtest/fuzz-virtio-balloon-test.c b/tests/qtest/virtio-balloon-test.c similarity index 84% rename from tests/qtest/fuzz-virtio-balloon-test.c rename to tests/qtest/virtio-balloon-test.c index ecb597fbee..6bea33b590 100644 --- a/tests/qtest/fuzz-virtio-balloon-test.c +++ b/tests/qtest/virtio-balloon-test.c @@ -1,5 +1,5 @@ /* - * QTest fuzzer-generated testcase for virtio balloon device + * QTest test cases for virtio balloon device * * Copyright (c) 2024 Gao Shiyuan * @@ -30,7 +30,7 @@ int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); - qtest_add_func("fuzz/virtio/oss_fuzz_71649", oss_fuzz_71649); + qtest_add_func("virtio-balloon/oss_fuzz_71649", oss_fuzz_71649); return g_test_run(); } From d65c890a5806e1f3d608a4218295d4729d697cd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 29 Nov 2024 13:55:07 +0000 Subject: [PATCH 12/13] tests/qtest: add test for querying balloon guest stats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test would have identified the crash caused by the addition of new balloon stats fields. Signed-off-by: Daniel P. Berrangé Reviewed-by: Fabiano Rosas Acked-by: Michael S. Tsirkin Message-ID: <20241129135507.699030-4-berrange@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/qtest/virtio-balloon-test.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/qtest/virtio-balloon-test.c b/tests/qtest/virtio-balloon-test.c index 6bea33b590..ecdd363b06 100644 --- a/tests/qtest/virtio-balloon-test.c +++ b/tests/qtest/virtio-balloon-test.c @@ -8,6 +8,7 @@ #include "qemu/osdep.h" #include "libqtest.h" +#include "standard-headers/linux/virtio_balloon.h" /* * https://gitlab.com/qemu-project/qemu/-/issues/2576 @@ -26,11 +27,30 @@ static void oss_fuzz_71649(void) qtest_quit(s); } +static void query_stats(void) +{ + QTestState *s = qtest_init("-device virtio-balloon,id=balloon" + " -nodefaults"); + QDict *ret = qtest_qmp_assert_success_ref( + s, + "{ 'execute': 'qom-get', 'arguments': " \ + "{ 'path': '/machine/peripheral/balloon', " \ + " 'property': 'guest-stats' } }"); + QDict *stats = qdict_get_qdict(ret, "stats"); + + /* We expect 1 entry in the dict for each known kernel stat */ + assert(qdict_size(stats) == VIRTIO_BALLOON_S_NR); + + qobject_unref(ret); + qtest_quit(s); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); qtest_add_func("virtio-balloon/oss_fuzz_71649", oss_fuzz_71649); + qtest_add_func("virtio-balloon/query-stats", query_stats); return g_test_run(); } From 964d2a0cf8136cdafc07f6fd847ebf897965dc35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 3 Dec 2024 10:36:16 +0100 Subject: [PATCH 13/13] system: Select HVF by default when no other accelerator is available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When testing with a HVF-only binary, we get: 3/12 qemu:func-quick+func-aarch64 / func-aarch64-version ERROR 0.29s exit status 1 stderr: Traceback (most recent call last): File "tests/functional/test_version.py", line 22, in test_qmp_human_info_version self.vm.launch() File "machine/machine.py", line 461, in launch raise VMLaunchFailure( qemu.machine.machine.VMLaunchFailure: ConnectError: Failed to establish session: EOFError Exit code: 1 Command: build/qemu-system-aarch64 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -machine none -nodefaults Output: qemu-system-aarch64: No accelerator selected and no default accelerator available Fix by checking for HVF in configure_accelerators() and using it by default when no other accelerator is available. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Daniel P. Berrangé Reviewed-by: Thomas Huth Message-Id: <20241203094232.62232-1-philmd@linaro.org> --- system/vl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/system/vl.c b/system/vl.c index 54998fdbc7..2f855d83fb 100644 --- a/system/vl.c +++ b/system/vl.c @@ -2362,6 +2362,7 @@ static void configure_accelerators(const char *progname) /* Select the default accelerator */ bool have_tcg = accel_find("tcg"); bool have_kvm = accel_find("kvm"); + bool have_hvf = accel_find("hvf"); if (have_tcg && have_kvm) { if (g_str_has_suffix(progname, "kvm")) { @@ -2374,6 +2375,8 @@ static void configure_accelerators(const char *progname) accelerators = "kvm"; } else if (have_tcg) { accelerators = "tcg"; + } else if (have_hvf) { + accelerators = "hvf"; } else { error_report("No accelerator selected and" " no default accelerator available");