Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
the start-powered-off property which makes cpu_common_reset() initialize it
to 1 in common code.
Also change creation of CPU object from cpu_create() to object_new() and
qdev_realize_and_unref() because cpu_create() realizes the CPU and it's not
possible to set a property after the object is realized.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/ppc/e500.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index ab9884e315..d7b803ef26 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
cpu_reset(cs);
- /* Secondary CPU starts in halted state for now. Needs to change when
- implementing non-kernel boot. */
- cs->halted = 1;
cs->exception_index = EXCP_HLT;
}
@@ -865,7 +862,7 @@ void ppce500_init(MachineState *machine)
CPUState *cs;
qemu_irq *input;
- cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
+ cpu = POWERPC_CPU(object_new(machine->cpu_type));
env = &cpu->env;
cs = CPU(cpu);
@@ -897,7 +894,16 @@ void ppce500_init(MachineState *machine)
} else {
/* Secondary CPUs */
qemu_register_reset(ppce500_cpu_reset_sec, cpu);
+
+ /*
+ * Secondary CPU starts in halted state for now. Needs to change
+ * when implementing non-kernel boot.
+ */
+ object_property_set_bool(OBJECT(cs), "start-powered-off", true,
+ &error_fatal);
}
+
+ qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
}
env = firstenv;
Hello,
On 8/19/20 6:43 PM, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
> the start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
>
> Also change creation of CPU object from cpu_create() to object_new() and
> qdev_realize_and_unref() because cpu_create() realizes the CPU and it's not
> possible to set a property after the object is realized.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
This is breaking make check :
tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped)
ERROR boot-serial-test - too few tests run (expected 7, got 0)
make: *** [/home/legoater/work/qemu/qemu-powernv-5.2.git/tests/Makefile.include:650: check-qtest-ppc64] Error 1
make: *** Waiting for unfinished jobs....
gdb --args build/ppc64-softmmu/qemu-system-ppc64 -display none -M ppce500
...
Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
0x000055555596ebf2 in ppce500_init (machine=0x5555567aa6e0)
at /home/legoater/work/qemu/qemu-powernv-5.2.git/hw/ppc/e500.c:880
880 irqs[i].irq[OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
AFAIUI, 'input is not initialized since the CPU is not yet realized.
C.
> ---
> hw/ppc/e500.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index ab9884e315..d7b803ef26 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
>
> cpu_reset(cs);
>
> - /* Secondary CPU starts in halted state for now. Needs to change when
> - implementing non-kernel boot. */
> - cs->halted = 1;
> cs->exception_index = EXCP_HLT;
> }
>
> @@ -865,7 +862,7 @@ void ppce500_init(MachineState *machine)
> CPUState *cs;
> qemu_irq *input;
>
> - cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> + cpu = POWERPC_CPU(object_new(machine->cpu_type));
> env = &cpu->env;
> cs = CPU(cpu);
>
> @@ -897,7 +894,16 @@ void ppce500_init(MachineState *machine)
> } else {
> /* Secondary CPUs */
> qemu_register_reset(ppce500_cpu_reset_sec, cpu);
> +
> + /*
> + * Secondary CPU starts in halted state for now. Needs to change
> + * when implementing non-kernel boot.
> + */
> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_fatal);
> }
> +
> + qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
> }
>
> env = firstenv;
>
On 8/22/20 10:59 AM, Cédric Le Goater wrote:
> Hello,
>
> On 8/19/20 6:43 PM, Thiago Jung Bauermann wrote:
>> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
>> the start-powered-off property which makes cpu_common_reset() initialize it
>> to 1 in common code.
>>
>> Also change creation of CPU object from cpu_create() to object_new() and
>> qdev_realize_and_unref() because cpu_create() realizes the CPU and it's not
>> possible to set a property after the object is realized.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
>
> This is breaking make check :
>
> tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped)
> ERROR boot-serial-test - too few tests run (expected 7, got 0)
> make: *** [/home/legoater/work/qemu/qemu-powernv-5.2.git/tests/Makefile.include:650: check-qtest-ppc64] Error 1
> make: *** Waiting for unfinished jobs....
>
>
> gdb --args build/ppc64-softmmu/qemu-system-ppc64 -display none -M ppce500
> ...
> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
> 0x000055555596ebf2 in ppce500_init (machine=0x5555567aa6e0)
> at /home/legoater/work/qemu/qemu-powernv-5.2.git/hw/ppc/e500.c:880
> 880 irqs[i].irq[OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
>
>
> AFAIUI, 'input is not initialized since the CPU is not yet realized.
Thiago, see ad938fc1d53 ("hw/arm/palm.c: Encapsulate misc GPIO handling
in a device") and eventually f8a865d36dc ("hw/arm/allwinner-a10:
Simplify by passing IRQs with qdev_pass_gpios") to get an idea how you
can fix that.
>
> C.
>
>> ---
>> hw/ppc/e500.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index ab9884e315..d7b803ef26 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
>>
>> cpu_reset(cs);
>>
>> - /* Secondary CPU starts in halted state for now. Needs to change when
>> - implementing non-kernel boot. */
>> - cs->halted = 1;
>> cs->exception_index = EXCP_HLT;
>> }
>>
>> @@ -865,7 +862,7 @@ void ppce500_init(MachineState *machine)
>> CPUState *cs;
>> qemu_irq *input;
>>
>> - cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>> + cpu = POWERPC_CPU(object_new(machine->cpu_type));
>> env = &cpu->env;
>> cs = CPU(cpu);
>>
>> @@ -897,7 +894,16 @@ void ppce500_init(MachineState *machine)
>> } else {
>> /* Secondary CPUs */
>> qemu_register_reset(ppce500_cpu_reset_sec, cpu);
>> +
>> + /*
>> + * Secondary CPU starts in halted state for now. Needs to change
>> + * when implementing non-kernel boot.
>> + */
>> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
>> + &error_fatal);
>> }
>> +
>> + qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
>> }
>>
>> env = firstenv;
>>
>
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 8/22/20 10:59 AM, Cédric Le Goater wrote:
>> Hello,
>>
>> On 8/19/20 6:43 PM, Thiago Jung Bauermann wrote:
>>> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
>>> the start-powered-off property which makes cpu_common_reset() initialize it
>>> to 1 in common code.
>>>
>>> Also change creation of CPU object from cpu_create() to object_new() and
>>> qdev_realize_and_unref() because cpu_create() realizes the CPU and it's not
>>> possible to set a property after the object is realized.
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>>
>>
>> This is breaking make check :
>>
>> tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped)
>> ERROR boot-serial-test - too few tests run (expected 7, got 0)
>> make: *** [/home/legoater/work/qemu/qemu-powernv-5.2.git/tests/Makefile.include:650: check-qtest-ppc64] Error 1
>> make: *** Waiting for unfinished jobs....
>>
>>
>> gdb --args build/ppc64-softmmu/qemu-system-ppc64 -display none -M ppce500
>> ...
>> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
>> 0x000055555596ebf2 in ppce500_init (machine=0x5555567aa6e0)
>> at /home/legoater/work/qemu/qemu-powernv-5.2.git/hw/ppc/e500.c:880
>> 880 irqs[i].irq[OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
>>
>>
>> AFAIUI, 'input is not initialized since the CPU is not yet realized.
>
> Thiago, see ad938fc1d53 ("hw/arm/palm.c: Encapsulate misc GPIO handling
> in a device") and eventually f8a865d36dc ("hw/arm/allwinner-a10:
> Simplify by passing IRQs with qdev_pass_gpios") to get an idea how you
> can fix that.
Thanks for the references, those are very useful.
--
Thiago Jung Bauermann
IBM Linux Technology Center
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 8/22/20 10:59 AM, Cédric Le Goater wrote:
>> Hello,
>>
>> On 8/19/20 6:43 PM, Thiago Jung Bauermann wrote:
>>> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
>>> the start-powered-off property which makes cpu_common_reset() initialize it
>>> to 1 in common code.
>>>
>>> Also change creation of CPU object from cpu_create() to object_new() and
>>> qdev_realize_and_unref() because cpu_create() realizes the CPU and it's not
>>> possible to set a property after the object is realized.
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>>
>>
>> This is breaking make check :
>>
>> tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped)
>> ERROR boot-serial-test - too few tests run (expected 7, got 0)
>> make: *** [/home/legoater/work/qemu/qemu-powernv-5.2.git/tests/Makefile.include:650: check-qtest-ppc64] Error 1
>> make: *** Waiting for unfinished jobs....
>>
>>
>> gdb --args build/ppc64-softmmu/qemu-system-ppc64 -display none -M ppce500
>> ...
>> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
>> 0x000055555596ebf2 in ppce500_init (machine=0x5555567aa6e0)
>> at /home/legoater/work/qemu/qemu-powernv-5.2.git/hw/ppc/e500.c:880
>> 880 irqs[i].irq[OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
>>
>>
>> AFAIUI, 'input is not initialized since the CPU is not yet realized.
>
> Thiago, see ad938fc1d53 ("hw/arm/palm.c: Encapsulate misc GPIO handling
> in a device") and eventually f8a865d36dc ("hw/arm/allwinner-a10:
> Simplify by passing IRQs with qdev_pass_gpios") to get an idea how you
> can fix that.
I ended up not following this route. There were other patches in this
series which also caused problems in make check, but in those cases it
wasn't related to IRQ setup.
I started feeling like I had fallen into a rabbit hole so I opted
instead to solve these problems by minimizing the consequences of the
changes made by this patch series.
--
Thiago Jung Bauermann
IBM Linux Technology Center
On Sat, Aug 22, 2020 at 10:59:56AM +0200, Cédric Le Goater wrote: > Hello, > > On 8/19/20 6:43 PM, Thiago Jung Bauermann wrote: > > Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use > > the start-powered-off property which makes cpu_common_reset() initialize it > > to 1 in common code. > > > > Also change creation of CPU object from cpu_create() to object_new() and > > qdev_realize_and_unref() because cpu_create() realizes the CPU and it's not > > possible to set a property after the object is realized. > > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> > > > This is breaking make check : > > tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped) > ERROR boot-serial-test - too few tests run (expected 7, got 0) > make: *** [/home/legoater/work/qemu/qemu-powernv-5.2.git/tests/Makefile.include:650: check-qtest-ppc64] Error 1 > make: *** Waiting for unfinished jobs.... > > > gdb --args build/ppc64-softmmu/qemu-system-ppc64 -display none -M ppce500 > ... > Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault. > 0x000055555596ebf2 in ppce500_init (machine=0x5555567aa6e0) > at /home/legoater/work/qemu/qemu-powernv-5.2.git/hw/ppc/e500.c:880 > 880 irqs[i].irq[OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT]; > > > AFAIUI, 'input is not initialized since the CPU is not yet > realized. Sigh. For future reference, Thiago, running an all-targets make check is pretty much a minimum bar to test before posting. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
David Gibson <david@gibson.dropbear.id.au> writes: > On Sat, Aug 22, 2020 at 10:59:56AM +0200, Cédric Le Goater wrote: >> Hello, >> >> On 8/19/20 6:43 PM, Thiago Jung Bauermann wrote: >> > Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use >> > the start-powered-off property which makes cpu_common_reset() initialize it >> > to 1 in common code. >> > >> > Also change creation of CPU object from cpu_create() to object_new() and >> > qdev_realize_and_unref() because cpu_create() realizes the CPU and it's not >> > possible to set a property after the object is realized. >> > >> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> >> >> >> This is breaking make check : >> >> tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped) >> ERROR boot-serial-test - too few tests run (expected 7, got 0) >> make: *** [/home/legoater/work/qemu/qemu-powernv-5.2.git/tests/Makefile.include:650: check-qtest-ppc64] Error 1 >> make: *** Waiting for unfinished jobs.... >> >> >> gdb --args build/ppc64-softmmu/qemu-system-ppc64 -display none -M ppce500 >> ... >> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault. >> 0x000055555596ebf2 in ppce500_init (machine=0x5555567aa6e0) >> at /home/legoater/work/qemu/qemu-powernv-5.2.git/hw/ppc/e500.c:880 >> 880 irqs[i].irq[OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT]; >> >> >> AFAIUI, 'input is not initialized since the CPU is not yet >> realized. > > Sigh. For future reference, Thiago, running an all-targets make check > is pretty much a minimum bar to test before posting. Sorry for the breakage. I'm working on it. -- Thiago Jung Bauermann IBM Linux Technology Center
© 2016 - 2026 Red Hat, Inc.