Re: [PATCH v6 6/8] sparc/sun4m: Remove main_cpu_reset()

[Thiago Jung Bauermann]

Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:

> We rely on cpu_common_reset() to set cs->halted to 0, so main_cpu_reset()
> is pointless.
>
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
>  hw/sparc/sun4m.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index cf7dfa4af5..22c51dac8a 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -218,15 +218,6 @@ static void dummy_cpu_set_irq(void *opaque, int irq, int 
> level)
>  {
>  }
>  
> -static void main_cpu_reset(void *opaque)
> -{
> -    SPARCCPU *cpu = opaque;
> -    CPUState *cs = CPU(cpu);
> -
> -    cpu_reset(cs);
> -    cs->halted = 0;
> -}
> -
>  static void secondary_cpu_reset(void *opaque)
>  {
>      SPARCCPU *cpu = opaque;
> @@ -827,9 +818,7 @@ static void cpu_devinit(const char *cpu_type, unsigned 
> int id,
>      env = &cpu->env;
>  
>      cpu_sparc_set_id(env, id);
> -    if (id == 0) {
> -        qemu_register_reset(main_cpu_reset, cpu);
> -    } else {
> +    if (id != 0) {
>          qemu_register_reset(secondary_cpu_reset, cpu);
>          cs = CPU(cpu);
>          cs->halted = 1;

Surprisingly, this patch also causes a make check failure:

$ make && make check-qtest
  GEN     docs/index.html
  CC      qga/main.o
  CC      qemu-nbd.o
  CC      qemu-storage-daemon.o
  CC      monitor/qmp-cmds-control.o
  CC      qemu-img.o
  CC      qemu-io.o
  CC      sparc-softmmu/hw/sparc/sun4m.o
  CC      sparc-softmmu/softmmu/vl.o
  LINK    qemu-ga
  LINK    qemu-nbd
  LINK    qemu-storage-daemon
  LINK    qemu-img
  LINK    sparc-softmmu/qemu-system-sparc
  LINK    qemu-io
  TEST    check-qtest-sparc: tests/qtest/prom-env-test
Broken pipe
/home/bauermann/trabalho/src/qemu/tests/qtest/libqtest.c:175: kill_qemu() 
detected QEMU death from signal 11 (Segmentation fault) (core dumped)
ERROR prom-env-test - too few tests run (expected 3, got 0)
make: *** [/home/bauermann/trabalho/src/qemu/tests/Makefile.include:650: 
check-qtest-sparc] Fehler 1

Here's what I got from the core file:

$ gdb sparc-softmmu/qemu-system-sparc core.645493
Reading symbols from sparc-softmmu/qemu-system-sparc...
[New LWP 645497]
[New LWP 645496]
[New LWP 645493]
[New LWP 645495]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Core was generated by `sparc-softmmu/qemu-system-sparc -qtest 
unix:/tmp/qtest-645490.sock -qtest-log /'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x0000000000000000 in ?? ()
[Current thread is 1 (Thread 0x7f713ce60700 (LWP 645497))]
(gdb) bt
#0  0x0000000000000000 in  ()
#1  0x0000557b3d6155c5 in helper_compute_psr (env=env@entry=0x557b3f444030) at 
/home/bauermann/trabalho/src/qemu/target/sparc/cc_helper.c:459
#2  0x0000557b3d6156e9 in cpu_get_psr (env=env@entry=0x557b3f444030) at 
/home/bauermann/trabalho/src/qemu/target/sparc/win_helper.c:56
#3  0x0000557b3d61779c in sparc_cpu_do_interrupt (cs=0x557b3f43b7f0) at 
/home/bauermann/trabalho/src/qemu/target/sparc/int32_helper.c:76
#4  0x0000557b3d5e29a6 in cpu_handle_exception (ret=<synthetic pointer>, 
cpu=0x557b3f43b7f0) at 
/home/bauermann/trabalho/src/qemu/accel/tcg/cpu-exec.c:504
#5  cpu_exec (cpu=cpu@entry=0x557b3f43b7f0) at 
/home/bauermann/trabalho/src/qemu/accel/tcg/cpu-exec.c:729
#6  0x0000557b3d5f6c85 in tcg_cpu_exec (cpu=<optimized out>) at 
/home/bauermann/trabalho/src/qemu/softmmu/cpus.c:1356
#7  qemu_tcg_rr_cpu_thread_fn (arg=arg@entry=0x557b3f43b7f0) at 
/home/bauermann/trabalho/src/qemu/softmmu/cpus.c:1458
#8  0x0000557b3d81e919 in qemu_thread_start (args=0x7f713ce5e930) at 
/home/bauermann/trabalho/src/qemu/util/qemu-thread-posix.c:521
#9  0x00007f717dc6b432 in start_thread () at /lib64/libpthread.so.0
#10 0x00007f717db99913 in clone () at /lib64/libc.so.6
(gdb) up
#1  0x0000557b3d6155c5 in helper_compute_psr (env=env@entry=0x557b3f444030) at 
/home/bauermann/trabalho/src/qemu/target/sparc/cc_helper.c:459
459         new_psr = icc_table[CC_OP].compute_all(env);
(gdb) list
454
455     void helper_compute_psr(CPUSPARCState *env)
456     {
457         uint32_t new_psr;
458
459         new_psr = icc_table[CC_OP].compute_all(env);
460         env->psr = new_psr;
461     #ifdef TARGET_SPARC64
462         new_psr = xcc_table[CC_OP].compute_all(env);
463         env->xcc = new_psr;

CC_OP is:

#define CC_OP  (env->cc_op)

So:

(gdb) p env->cc_op
$1 = 0

0 is CC_OP_DYNAMIC, but the icc_table definition says:

static const CCTable icc_table[CC_OP_NB] = {
    /* CC_OP_DYNAMIC should never happen */

I don't know what is going on. So I will change this patch to keep
main_cpu_reset() but drop the line which sets cs->halted to 0 (which
does pass make check).

-- 
Thiago Jung Bauermann
IBM Linux Technology Center