Windows pvpanic driver writes PVPANIC_CRASH_LOADED (bit 1) on
bugcheck, not PVPANIC_PANICKED (bit 0). As a result,
qemu_system_guest_panicked() is never called and crash_occurred
stays false for Windows guests.

Set crash_occurred in qemu_system_guest_crashloaded() as well,
so the faulting CPU can be identified in the guest dump.

Signed-off-by: Zhengrong Li <zhengrong_li@linux.alibaba.com>
---
 system/runstate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/system/runstate.c b/system/runstate.c
index 08acf801b0..c7961cc1df 100644
--- a/system/runstate.c
+++ b/system/runstate.c
@@ -724,6 +724,11 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
 void qemu_system_guest_crashloaded(GuestPanicInformation *info)
 {
     qemu_log_mask(LOG_GUEST_ERROR, "Guest crash loaded");
+
+    if (current_cpu) {
+        current_cpu->crash_occurred = true;
+    }
+
     qapi_event_send_guest_crashloaded(GUEST_PANIC_ACTION_RUN, info);
     qapi_free_GuestPanicInformation(info);
 }
-- 
2.43.0

Hi,

On 5/8/26 11:05, Zhengrong Li wrote:
> Windows pvpanic driver writes PVPANIC_CRASH_LOADED (bit 1) on
> bugcheck, not PVPANIC_PANICKED (bit 0). As a result,
> qemu_system_guest_panicked() is never called and crash_occurred
> stays false for Windows guests.
> 
> Set crash_occurred in qemu_system_guest_crashloaded() as well,
> so the faulting CPU can be identified in the guest dump.
> 
> Signed-off-by: Zhengrong Li <zhengrong_li@linux.alibaba.com>
> ---
>   system/runstate.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/system/runstate.c b/system/runstate.c
> index 08acf801b0..c7961cc1df 100644
> --- a/system/runstate.c
> +++ b/system/runstate.c
> @@ -724,6 +724,11 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
>   void qemu_system_guest_crashloaded(GuestPanicInformation *info)
>   {
>       qemu_log_mask(LOG_GUEST_ERROR, "Guest crash loaded");
> +
> +    if (current_cpu) {
> +        current_cpu->crash_occurred = true;

This is generic code, I'm surprised we need to update the vCPU thread
here. Could it be the reponsibility of the caller? As you described,
pvpanic so add this change in handle_event()? I'm also surprised
qemu_system_guest_panicked() does that -- the change comes from
commit bac05aa9a77 ("cpu: Add crash_occurred flag into CPUState")
which doesn't explain much. Unfortunately the pvpanic code doesn't
have any maintainer listed...

Cc'ing some developers active in this code...

$ git grep @ $(git ls-files|fgrep pvpanic)
hw/misc/pvpanic-pci.c:7: *     Mihai Carabas <mihai.carabas@oracle.com>
hw/misc/pvpanic.c:7: *     Wen Congyang <wency@cn.fujitsu.com>
hw/misc/pvpanic.c:8: *     Hu Tao <hutao@cn.fujitsu.com>

> +    }
> +
>       qapi_event_send_guest_crashloaded(GUEST_PANIC_ACTION_RUN, info);
>       qapi_free_GuestPanicInformation(info);
>   }

Hi Philippe,

Thanks for the review.

I followed the existing pattern in qemu_system_guest_panicked(),
which already sets crash_occurred on current_cpu when a guest
panic is reported. The intent was to keep the panicked and
crash-loaded paths consistent.

Putting it in the generic function also means any future crash
notification mechanism (beyond pvpanic) would automatically
benefit from identifying the faulting CPU in guest dumps.

I agree the original commit bac05aa9a77 ("cpu: Add crash_occurred
flag into CPUState") doesn't explain the design choice well.

If you prefer moving it to handle_event() in the caller, I'm
happy to adjust. What do you think?

Regards,
Zhengrong Li

On 2026/08/05 21:10, Zhengrong Li wrote:
> Hi Philippe,
> 
> Thanks for the review.
> 
> I followed the existing pattern in qemu_system_guest_panicked(),
> which already sets crash_occurred on current_cpu when a guest
> panic is reported. The intent was to keep the panicked and
> crash-loaded paths consistent.
> 
> Putting it in the generic function also means any future crash
> notification mechanism (beyond pvpanic) would automatically
> benefit from identifying the faulting CPU in guest dumps.
> 
> I agree the original commit bac05aa9a77 ("cpu: Add crash_occurred
> flag into CPUState") doesn't explain the design choice well.
> 
> If you prefer moving it to handle_event() in the caller, I'm
> happy to adjust. What do you think?

I find your reasoning makes sense. It is a common requirement of 
qemu_system_guest_panicked() callers to update current_cpu, and having 
the logic to update the CPU in the same function will de-duplicate that 
logic. While there is only one caller of 
qemu_system_guest_crashloaded(), consistency and future prospect 
justifies to do the same.

On the other hand, it is also true that a function that is prefixed 
qemu_system_* implicitly updating current_cpu is obscure. I think the 
intent would be clearer if the functions explicitly take the CPU via a 
parameter. In fact, most callers of qemu_system_guest_panicked() 
explicitly take the CPU via a parameter. e.g., kvm_cpu_exec(). 
qemu_system_guest_panicked() breaks the propagation of the CPU value and 
potentially attributes the panic to a wrong CPU.

We are adding a similar function and its call, so it is a good timing to 
perform such refactoring. I still don't require you to do that since 
updating the signature of qemu_system_guest_panicked() requires a simple 
yet codebase-wide edit that is not strictly necessary for the feature we 
are adding.

Regards,
Akihiko Odaki

Hi Akihiko,

Thanks for the detailed analysis. I agree that explicitly passing
the CPU as a parameter would be cleaner than relying on current_cpu
implicitly. That sounds like a good standalone refactoring to pursue
separately.

For this series, I'll keep the current approach.

Regards,
Zhengrong Li