| Message ID | 20250602090726.41315-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] system/cpus: Only kick running vCPUs | expand |
On Mon, 2 Jun 2025 at 10:08, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > We shouldn't kick a stopped vCPU, as it will be resumed. What is this trying to fix? Do we get wrong-behaviour, or is it just a bit inefficient as the vcpu thread goes round its loop and decides it still has nothing to do? thanks -- PMM
On 2/6/25 11:16, Peter Maydell wrote: > On Mon, 2 Jun 2025 at 10:08, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >> We shouldn't kick a stopped vCPU, as it will be resumed. > > What is this trying to fix? Hmm I guess the bug is in the accelerator I'm trying (split). > Do we get wrong-behaviour, Not really. > or is it just a bit inefficient as the vcpu thread > goes round its loop and decides it still has nothing to do? So far with mainstream, yes. Adding an assertion triggers: frame #3: 0x000000018a5aaeec libsystem_c.dylib`__assert_rtn + 284 * frame #4: 0x000000010094b8e0 qemu-system-aarch64-unsigned`qemu_cpu_kick.cold.1 at cpus.c:494:5 frame #5: 0x00000001002e8380 qemu-system-aarch64-unsigned`qemu_cpu_kick(cpu=0x0000000140028000) at cpus.c:494:5 frame #6: 0x00000001006d2a7c qemu-system-aarch64-unsigned`memory_listener_register [inlined] listener_add_address_space(listener=0x00006000015e9dd8, as=0x00006000019eaa00) at memory.c:3107:9 frame #7: 0x00000001006d28e4 qemu-system-aarch64-unsigned`memory_listener_register(listener=0x00006000015e9dd8, as=0x00006000019eaa00) at memory.c:3195:5 frame #8: 0x00000001006d6fd8 qemu-system-aarch64-unsigned`cpu_address_space_init(cpu=<unavailable>, asidx=<unavailable>, prefix=<unavailable>, mr=<unavailable>) at physmem.c:783:9 [artificial] frame #9: 0x0000000100412c6c qemu-system-aarch64-unsigned`arm_cpu_realizefn(dev=0x0000000140028000, errp=0x000000016fdfe250) at cpu.c:2633:5 frame #10: 0x0000000100708b9c qemu-system-aarch64-unsigned`device_set_realized(obj=<unavailable>, value=<unavailable>, errp=0x000000016fdfe308) at qdev.c:494:13 frame #11: 0x000000010071054c qemu-system-aarch64-unsigned`property_set_bool(obj=0x0000000140028000, v=<unavailable>, name=<unavailable>, opaque=0x0000600002ee03c0, errp=0x000000016fdfe308) at object.c:2374:5 frame #12: 0x000000010070e27c qemu-system-aarch64-unsigned`object_property_set(obj=0x0000000140028000, name="realized", v=0x00006000010eb400, errp=0x000000016fdfe308) at object.c:1449:5 frame #13: 0x00000001007128b0 qemu-system-aarch64-unsigned`object_property_set_qobject(obj=<unavailable>, name=<unavailable>, value=<unavailable>, errp=<unavailable>) at qom-qobject.c:28:10 frame #14: 0x000000010070e788 qemu-system-aarch64-unsigned`object_property_set_bool(obj=<unavailable>, name=<unavailable>, value=<unavailable>, errp=<unavailable>) at object.c:1519:15 frame #15: 0x0000000100707bf8 qemu-system-aarch64-unsigned`qdev_realize(dev=<unavailable>, bus=<unavailable>, errp=<unavailable>) at qdev.c:276:12 [artificial] frame #16: 0x000000010039fec4 qemu-system-aarch64-unsigned`machvirt_init(machine=0x000000014e1071c0) at virt.c:2355:9 frame #17: 0x00000001000c313c qemu-system-aarch64-unsigned`machine_run_board_init(machine=<unavailable>, mem_path=<unavailable>, errp=<unavailable>) at machine.c:1682:5 frame #18: 0x00000001002f3000 qemu-system-aarch64-unsigned`qmp_x_exit_preconfig [inlined] qemu_init_board at vl.c:2716:5 frame #19: 0x00000001002f2fb4 qemu-system-aarch64-unsigned`qmp_x_exit_preconfig(errp=0x0000000101996590) at vl.c:2812:5 frame #20: 0x00000001002f6da4 qemu-system-aarch64-unsigned`qemu_init(argc=<unavailable>, argv=<unavailable>) at vl.c:3848:9 frame #21: 0x000000010081aaac qemu-system-aarch64-unsigned`main(argc=<unavailable>, argv=<unavailable>) at main.c:71:5 Please ignore this patch, I'll keep debugging and eventually repost the same content described as simple optimization. Regards, Phil.
diff --git a/system/cpus.c b/system/cpus.c index d16b0dff989..4835e5ced48 100644 --- a/system/cpus.c +++ b/system/cpus.c @@ -494,6 +494,11 @@ void cpus_kick_thread(CPUState *cpu) void qemu_cpu_kick(CPUState *cpu) { qemu_cond_broadcast(cpu->halt_cond); + + if (!cpu_can_run(cpu)) { + return; + } + if (cpus_accel->kick_vcpu_thread) { cpus_accel->kick_vcpu_thread(cpu); } else { /* default */
We shouldn't kick a stopped vCPU, as it will be resumed. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- RFC: Better to assert and fix call sites? --- system/cpus.c | 5 +++++ 1 file changed, 5 insertions(+)