Re: Crashes with qemu-system-ppc64

[Greg Kurz <groug@kaod.org>]

On Wed, 24 Mar 2021 10:17:55 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 24/03/21 00:35, Philippe Mathieu-Daudé wrote:
> > Hmmm does this assert() matches your comment?
> > 
> > -- >8 --
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index cefc5eaa0a9..41cbee77d14 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -1130,6 +1130,8 @@ Object *qdev_get_machine(void)
> >   {
> >       static Object *dev;
> > 
> > +    assert(phase_check(PHASE_MACHINE_CREATED));
> > +
> 
> Very nice use of phase_check!  Kudos.
> 

It seems promising at first sight but qdev_get_machine() gets
called under qemu_create_machine() long before phase is advanced
to PHASE_MACHINE_CREATED.

qemu-system-ppc64: ../../hw/core/qdev.c:1133: qdev_get_machine: Assertion `phase_check(PHASE_MACHINE_CREATED)' failed.

(gdb) bt
#0  0x00007ffff64a3708 in raise () at /lib64/power9/libc.so.6
#1  0x00007ffff6483bcc in abort () at /lib64/power9/libc.so.6
#2  0x00007ffff6497210 in __assert_fail_base () at /lib64/power9/libc.so.6
#3  0x00007ffff64972b4 in __assert_fail () at /lib64/power9/libc.so.6
#4  0x00000001009e7820 in qdev_get_machine () at ../../hw/core/qdev.c:1133
#5  0x00000001009e7820 in qdev_get_machine () at ../../hw/core/qdev.c:1129
#6  0x0000000100747894 in memory_region_do_init (mr=0x101261200, owner=0x0, name=<optimized out>, size=<optimized out>) at ../../softmmu/memory.c:1177
#7  0x00000001007fccc4 in memory_map_init () at ../../softmmu/physmem.c:2630
#8  0x00000001007fccc4 in cpu_exec_init_all () at ../../softmmu/physmem.c:3034
#9  0x00000001007e9c9c in qemu_create_machine (machine_class=machine_class@entry=0x1014b96d0) at ../../softmmu/vl.c:2086
#10 0x00000001007eb8c0 in qemu_init (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at ../../softmmu/vl.c:1640
#11 0x00000001002f53c8 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at ../../softmmu/main.c:49


static void memory_region_do_init(MemoryRegion *mr,
                                  Object *owner,
                                  const char *name,
                                  uint64_t size)
{
[...]
        if (!owner) {
            owner = container_get(qdev_get_machine(), "/unattached");
        }

The true condition for qdev_get_machine() to be functional is
actually that the following happened already:

    object_property_add_child(object_get_root(), "machine",
                              OBJECT(current_machine));

This is the case with the call stack ^^ and I don't see any valid
reason to forbid use of qdev_get_machine() here.

So I'm wondering if we shouldn't rather check the existence of the
"/machine" path in the QOM tree instead of checking the phase.

> Paolo
>