On 7/12/22 16:25, Matheus Ferst wrote:
When using "-machine none", env->tb_env is not allocated, causing the segmentation fault reported in issue #85 (launchpad bug #811683). To avoid this problem, check if the pointer != NULL before calling the methods to print TBU/TBL/DECR. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/85 Signed-off-by: Matheus Ferst <matheus.fe...@eldorado.org.br> --- This patch fixes the reported problem, but may be an incomplete solution since many other places dereference env->tb_env without checking for NULL. AFAICS, "-machine none" is the only way to trigger this problem, and I'm not familiar with the use-cases for this option.
The "none" machine type is mainly used by libvirt to do instrospection of the available options/capabilities of the QEMU binary. It starts a QEMU process like the following: ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults \ -nographic -machine none,accel=kvm:tcg -daemonize And then it uses QMP to probe the binary. Aside from this libvirt usage I am not aware of anyone else using -machine none extensively.
Should we stop assuming env->tb_env != NULL and add checks everywhere? Or should we find a way to provide Time Base/Decrementer for "-machine none"? ---
Are there other cases where env->tb_env can be NULL, aside from the case reported in the bug? I don't mind the bug fix, but I'm not fond of the idea of adding additional checks because of this particular issue. I mean, the bug is using the 'prep' machine that Thomas removed year ago in b2ce76a0730. If there's no other foreseeable problem, that we care about, with env->tb_env being NULL, IMO let's fix the bug and move on. Thanks, Daniel
target/ppc/cpu_init.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index 86ad28466a..7e96baac9f 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -7476,18 +7476,18 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags) "%08x iidx %d didx %d\n", env->msr, env->spr[SPR_HID0], env->hflags, cpu_mmu_index(env, true), cpu_mmu_index(env, false)); -#if !defined(NO_TIMER_DUMP) - qemu_fprintf(f, "TB %08" PRIu32 " %08" PRIu64 + if (env->tb_env) { + qemu_fprintf(f, "TB %08" PRIu32 " %08" PRIu64 #if !defined(CONFIG_USER_ONLY) - " DECR " TARGET_FMT_lu + " DECR " TARGET_FMT_lu #endif - "\n", - cpu_ppc_load_tbu(env), cpu_ppc_load_tbl(env) + "\n", + cpu_ppc_load_tbu(env), cpu_ppc_load_tbl(env) #if !defined(CONFIG_USER_ONLY) - , cpu_ppc_load_decr(env) -#endif - ); + , cpu_ppc_load_decr(env) #endif + ); + } for (i = 0; i < 32; i++) { if ((i & (RGPL - 1)) == 0) { qemu_fprintf(f, "GPR%02d", i);