Patchwork [4/5] qom-cpu: during cpu reset, it will reset its child

login
register
mail settings
Submitter pingfan liu
Date July 10, 2012, 6:16 a.m.
Message ID <1341900967-4344-5-git-send-email-qemulist@gmail.com>
Download mbox | patch
Permalink /patch/170063/
State New
Headers show

Comments

pingfan liu - July 10, 2012, 6:16 a.m.
This will give the embeded logic module, such as apic has the
opportunity to reset.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 qom/cpu.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
Paolo Bonzini - July 10, 2012, 8:41 a.m.
Il 10/07/2012 08:16, Liu Ping Fan ha scritto:
> This will give the embeded logic module, such as apic has the
> opportunity to reset.
> 
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
>  qom/cpu.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5b36046..6aea8e6 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -20,10 +20,26 @@
>  
>  #include "qemu/cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev.h"
> +
> +static int cpu_reset_kid(Object *child, void *opaque)
> +{
> +    if (object_is_type_str(child, TYPE_DEVICE)) {
> +        device_reset(DEVICE(child));
> +    } else if (object_is_type_str(child, TYPE_BUS)) {
> +        bus_reset(BUS(child));
> +    } else {
> +        printf("cpu's child must be DEVICE or BUS");
> +        abort();
> +    }
> +    return 0;
> +}
>  
>  void cpu_reset(CPUState *cpu)
>  {
>      CPUClass *klass = CPU_GET_CLASS(cpu);
> +    Object *obj = OBJECT(cpu);
> +    object_child_foreach(obj, cpu_reset_kid, NULL);

Ok, now I see what you want to do.  Next time, please add meaningful
commit messages to all patches in the series, even those that only add
infrastructure.

It really looks like time is ripe to make CPUs children of Device, so
you can just use qdev_reset_all to reset the CPU.

Paolo

>      if (klass->reset != NULL) {
>          (*klass->reset)(cpu);
>
Andreas Färber - July 10, 2012, 10:12 a.m.
Am 10.07.2012 10:41, schrieb Paolo Bonzini:
> Il 10/07/2012 08:16, Liu Ping Fan ha scritto:
>> This will give the embeded logic module, such as apic has the
>> opportunity to reset.
>>
>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>> ---
>>  qom/cpu.c |   16 ++++++++++++++++
>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 5b36046..6aea8e6 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -20,10 +20,26 @@
>>  
>>  #include "qemu/cpu.h"
>>  #include "qemu-common.h"
>> +#include "hw/qdev.h"
>> +
>> +static int cpu_reset_kid(Object *child, void *opaque)
>> +{
>> +    if (object_is_type_str(child, TYPE_DEVICE)) {
>> +        device_reset(DEVICE(child));
>> +    } else if (object_is_type_str(child, TYPE_BUS)) {
>> +        bus_reset(BUS(child));
>> +    } else {
>> +        printf("cpu's child must be DEVICE or BUS");
>> +        abort();
>> +    }
>> +    return 0;
>> +}
>>  
>>  void cpu_reset(CPUState *cpu)
>>  {
>>      CPUClass *klass = CPU_GET_CLASS(cpu);
>> +    Object *obj = OBJECT(cpu);
>> +    object_child_foreach(obj, cpu_reset_kid, NULL);
> 
> Ok, now I see what you want to do.  Next time, please add meaningful
> commit messages to all patches in the series, even those that only add
> infrastructure.
> 
> It really looks like time is ripe to make CPUs children of Device, so
> you can just use qdev_reset_all to reset the CPU.

While we agree on that goal, the way there has proven controversial,
please review and comment on the two approaches around.

One thing we definitely need to do is to split up qdev.h.

Andreas
pingfan liu - July 11, 2012, 1:17 a.m.
On Tue, Jul 10, 2012 at 6:12 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 10.07.2012 10:41, schrieb Paolo Bonzini:
>> Il 10/07/2012 08:16, Liu Ping Fan ha scritto:
>>> This will give the embeded logic module, such as apic has the
>>> opportunity to reset.
>>>
>>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>> ---
>>>  qom/cpu.c |   16 ++++++++++++++++
>>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/qom/cpu.c b/qom/cpu.c
>>> index 5b36046..6aea8e6 100644
>>> --- a/qom/cpu.c
>>> +++ b/qom/cpu.c
>>> @@ -20,10 +20,26 @@
>>>
>>>  #include "qemu/cpu.h"
>>>  #include "qemu-common.h"
>>> +#include "hw/qdev.h"
>>> +
>>> +static int cpu_reset_kid(Object *child, void *opaque)
>>> +{
>>> +    if (object_is_type_str(child, TYPE_DEVICE)) {
>>> +        device_reset(DEVICE(child));
>>> +    } else if (object_is_type_str(child, TYPE_BUS)) {
>>> +        bus_reset(BUS(child));
>>> +    } else {
>>> +        printf("cpu's child must be DEVICE or BUS");
>>> +        abort();
>>> +    }
>>> +    return 0;
>>> +}
>>>
>>>  void cpu_reset(CPUState *cpu)
>>>  {
>>>      CPUClass *klass = CPU_GET_CLASS(cpu);
>>> +    Object *obj = OBJECT(cpu);
>>> +    object_child_foreach(obj, cpu_reset_kid, NULL);
>>
>> Ok, now I see what you want to do.  Next time, please add meaningful
>> commit messages to all patches in the series, even those that only add
>> infrastructure.
>>
>> It really looks like time is ripe to make CPUs children of Device, so
>> you can just use qdev_reset_all to reset the CPU.
>
> While we agree on that goal, the way there has proven controversial,
> please review and comment on the two approaches around.
>
> One thing we definitely need to do is to split up qdev.h.
>
Could you remember the main topic about these?  I miss these discussion.

Thanks,
pingfan

> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
>
pingfan liu - July 11, 2012, 1:17 a.m.
On Tue, Jul 10, 2012 at 4:41 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 10/07/2012 08:16, Liu Ping Fan ha scritto:
>> This will give the embeded logic module, such as apic has the
>> opportunity to reset.
>>
>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>> ---
>>  qom/cpu.c |   16 ++++++++++++++++
>>  1 files changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 5b36046..6aea8e6 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -20,10 +20,26 @@
>>
>>  #include "qemu/cpu.h"
>>  #include "qemu-common.h"
>> +#include "hw/qdev.h"
>> +
>> +static int cpu_reset_kid(Object *child, void *opaque)
>> +{
>> +    if (object_is_type_str(child, TYPE_DEVICE)) {
>> +        device_reset(DEVICE(child));
>> +    } else if (object_is_type_str(child, TYPE_BUS)) {
>> +        bus_reset(BUS(child));
>> +    } else {
>> +        printf("cpu's child must be DEVICE or BUS");
>> +        abort();
>> +    }
>> +    return 0;
>> +}
>>
>>  void cpu_reset(CPUState *cpu)
>>  {
>>      CPUClass *klass = CPU_GET_CLASS(cpu);
>> +    Object *obj = OBJECT(cpu);
>> +    object_child_foreach(obj, cpu_reset_kid, NULL);
>
> Ok, now I see what you want to do.  Next time, please add meaningful
> commit messages to all patches in the series, even those that only add
> infrastructure.
>
OK, I will.

> It really looks like time is ripe to make CPUs children of Device, so
> you can just use qdev_reset_all to reset the CPU.
>
Change CPUState as child of Device? It is cool, but I think it will a
huge work with argue.
> Paolo
>
>>      if (klass->reset != NULL) {
>>          (*klass->reset)(cpu);
>>
>
>
Paolo Bonzini - July 11, 2012, 7:02 a.m.
Il 11/07/2012 03:17, liu ping fan ha scritto:
>> > It really looks like time is ripe to make CPUs children of Device, so
>> > you can just use qdev_reset_all to reset the CPU.
>> >
> Change CPUState as child of Device? It is cool, but I think it will a
> huge work with argue.

Luckily other people (Igor, Andreas, Anthony) are already working on that.

Paolo
Igor Mammedov - July 11, 2012, 12:37 p.m.
On 07/11/2012 03:17 AM, liu ping fan wrote:
> On Tue, Jul 10, 2012 at 6:12 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 10.07.2012 10:41, schrieb Paolo Bonzini:
>>> Il 10/07/2012 08:16, Liu Ping Fan ha scritto:
>>>> This will give the embeded logic module, such as apic has the
>>>> opportunity to reset.
>>>>
>>>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>>> ---
>>>>   qom/cpu.c |   16 ++++++++++++++++
>>>>   1 files changed, 16 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/qom/cpu.c b/qom/cpu.c
>>>> index 5b36046..6aea8e6 100644
>>>> --- a/qom/cpu.c
>>>> +++ b/qom/cpu.c
>>>> @@ -20,10 +20,26 @@
>>>>
>>>>   #include "qemu/cpu.h"
>>>>   #include "qemu-common.h"
>>>> +#include "hw/qdev.h"
>>>> +
>>>> +static int cpu_reset_kid(Object *child, void *opaque)
>>>> +{
>>>> +    if (object_is_type_str(child, TYPE_DEVICE)) {
>>>> +        device_reset(DEVICE(child));
>>>> +    } else if (object_is_type_str(child, TYPE_BUS)) {
>>>> +        bus_reset(BUS(child));
>>>> +    } else {
>>>> +        printf("cpu's child must be DEVICE or BUS");
>>>> +        abort();
>>>> +    }
>>>> +    return 0;
>>>> +}
>>>>
>>>>   void cpu_reset(CPUState *cpu)
>>>>   {
>>>>       CPUClass *klass = CPU_GET_CLASS(cpu);
>>>> +    Object *obj = OBJECT(cpu);
>>>> +    object_child_foreach(obj, cpu_reset_kid, NULL);
>>>
>>> Ok, now I see what you want to do.  Next time, please add meaningful
>>> commit messages to all patches in the series, even those that only add
>>> infrastructure.
>>>
>>> It really looks like time is ripe to make CPUs children of Device, so
>>> you can just use qdev_reset_all to reset the CPU.
>>
>> While we agree on that goal, the way there has proven controversial,
>> please review and comment on the two approaches around.
>>
>> One thing we definitely need to do is to split up qdev.h.
>>
> Could you remember the main topic about these?  I miss these discussion.
http://lists.gnu.org/archive/html/qemu-devel/2012-06/msg03912.html
the rest was on irc

> Thanks,
> pingfan
>
>> Andreas
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>>
>>
>

Patch

diff --git a/qom/cpu.c b/qom/cpu.c
index 5b36046..6aea8e6 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -20,10 +20,26 @@ 
 
 #include "qemu/cpu.h"
 #include "qemu-common.h"
+#include "hw/qdev.h"
+
+static int cpu_reset_kid(Object *child, void *opaque)
+{
+    if (object_is_type_str(child, TYPE_DEVICE)) {
+        device_reset(DEVICE(child));
+    } else if (object_is_type_str(child, TYPE_BUS)) {
+        bus_reset(BUS(child));
+    } else {
+        printf("cpu's child must be DEVICE or BUS");
+        abort();
+    }
+    return 0;
+}
 
 void cpu_reset(CPUState *cpu)
 {
     CPUClass *klass = CPU_GET_CLASS(cpu);
+    Object *obj = OBJECT(cpu);
+    object_child_foreach(obj, cpu_reset_kid, NULL);
 
     if (klass->reset != NULL) {
         (*klass->reset)(cpu);