Re: [PATCH 2/3] dump: add crash_occurred flag to QEMUCPUState

On 2026/07/22 10:48, 李峥嵘(村叔) wrote:
> Add is_crash_occurred_cpu field to QEMUCPUState so that
> dump-guest-memory can record which CPU triggered the guest
> panic. Bump QEMUCPUSTATE_VERSION to 2.
> 
> The new field is appended after kernel_gs_base, so existing
> tools that check the 'size' field can safely ignore it when
> reading older dumps.

QEMUCPUSTATE_VERSION was not bumped with commit 46fac17dca19 ("dump: add 
kernel_gs_base to QEMU CPU state"), and tools just check the size. 
Probably there is no reason to bump it this time either.

> 
> Signed-off-by: Zhengrong Li <[email protected]>

docs/devel/code-provenance.rst says:
 > It is generally expected that the name and email addresses used in one
 > of the ``Signed-off-by`` lines, matches that of the git commit
 > ``Author`` field.
 > It's okay if you subscribe or contribute to the list via more than one
 > address, but using multiple addresses in one commit just confuses
 > things.

The "From" (which will be imported as the "Author" field when applying 
the patch) has Chinese characters while the Signed-off-by line has roman 
characters. Strictly speaking, they should be consistent.

> ---
>   contrib/elf2dmp/qemu_elf.h | 2 ++
>   target/i386/arch_dump.c    | 8 +++++++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
> index adc50238b4..3b28332e67 100644
> --- a/contrib/elf2dmp/qemu_elf.h
> +++ b/contrib/elf2dmp/qemu_elf.h
> @@ -27,6 +27,8 @@ typedef struct QEMUCPUState {
>       QEMUCPUSegment ldt, tr, gdt, idt;
>       uint64_t cr[5];
>       uint64_t kernel_gs_base;
> +    uint8_t  is_crash_occurred_cpu;
> +    uint8_t  pad[7];

Growing QEMUCPUState from 440 to 448 bytes confuses init_states() in 
contrib/elf2dmp/qemu_elf.c and makes valid version-1 notes and appear to 
lack kernel_gs_base, despite that field occupying bytes 432–439. Please 
update the function.

>   } QEMUCPUState;
>   
>   int is_system(QEMUCPUState *s);
> diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c
> index 16e47c4747..f1c31984c5 100644
> --- a/target/i386/arch_dump.c
> +++ b/target/i386/arch_dump.c
> @@ -237,7 +237,7 @@ int x86_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>    * please count up QEMUCPUSTATE_VERSION if you have changed definition of
>    * QEMUCPUState, and modify the tools using this information accordingly.
>    */
> -#define QEMUCPUSTATE_VERSION (1)
> +#define QEMUCPUSTATE_VERSION (2)>
>   struct QEMUCPUSegment {
>       uint32_t selector;
> @@ -264,6 +264,8 @@ struct QEMUCPUState {
>        * by checking 'size' field.
>        */
>       uint64_t kernel_gs_base;
> +    uint8_t  is_crash_occurred_cpu;
> +    uint8_t  pad[7];
>   };
>   
>   typedef struct QEMUCPUState QEMUCPUState;
> @@ -279,6 +281,7 @@ static void copy_segment(QEMUCPUSegment *d, SegmentCache *s)
>   
>   static void qemu_get_cpustate(QEMUCPUState *s, CPUX86State *env)
>   {
> +    CPUState *cs = env_cpu(env);
>       memset(s, 0, sizeof(QEMUCPUState));
>   
>       s->version = QEMUCPUSTATE_VERSION;
> @@ -324,6 +327,9 @@ static void qemu_get_cpustate(QEMUCPUState *s, CPUX86State *env)
>   
>   #ifdef TARGET_X86_64
>       s->kernel_gs_base = env->kernelgsbase;
> +    if (cs->crash_occurred) {
> +        s->is_crash_occurred_cpu = 1;
> +    }

cs is declared unconditionally but used only under TARGET_X86_64.
make qemu-system-i386 fails with -Werror=unused-variable. Please Move it 
outside the #ifdef block if you don't have a reason not to do so.

Regards,
Akihiko Odaki

>   #endif
>   }
>