diff mbox

[v3,1/3] exec: split cpu_exec_init()

Message ID 1476485569-6744-2-git-send-email-lvivier@redhat.com
State New
Headers show

Commit Message

Laurent Vivier Oct. 14, 2016, 10:52 p.m. UTC
Put in cpu_exec_initfn() what initializes the CPU,
and let in cpu_exec_init() what adds it to the environment.

As cpu_exec_initfn() is called by all XX_cpu_initfn() call it
directly in cpu_common_initfn().
cpu_exec_init() is now a realize function, it will be renamed
to cpu_exec_realizefn() and moved to the XX_cpu_realizefn()
function in a following patch.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 exec.c            | 10 ++++++----
 include/qom/cpu.h |  1 +
 qom/cpu.c         |  2 ++
 3 files changed, 9 insertions(+), 4 deletions(-)

Comments

David Gibson Oct. 17, 2016, 3:43 a.m. UTC | #1
On Sat, Oct 15, 2016 at 12:52:47AM +0200, Laurent Vivier wrote:
> Put in cpu_exec_initfn() what initializes the CPU,
> and let in cpu_exec_init() what adds it to the environment.
> 
> As cpu_exec_initfn() is called by all XX_cpu_initfn() call it
> directly in cpu_common_initfn().
> cpu_exec_init() is now a realize function, it will be renamed
> to cpu_exec_realizefn() and moved to the XX_cpu_realizefn()
> function in a following patch.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  exec.c            | 10 ++++++----
>  include/qom/cpu.h |  1 +
>  qom/cpu.c         |  2 ++
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 374c364..d1e57c4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -610,11 +610,8 @@ void cpu_exec_exit(CPUState *cpu)
>      }
>  }
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp)
> +void cpu_exec_initfn(CPUState *cpu)
>  {
> -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> -
>      cpu->as = NULL;
>      cpu->num_ases = 0;
>  
> @@ -635,6 +632,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>      cpu->memory = system_memory;
>      object_ref(OBJECT(cpu->memory));
>  #endif
> +}
> +
> +void cpu_exec_init(CPUState *cpu, Error **errp)
> +{
> +    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>  
>      cpu_list_add(cpu);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 6d481a1..d7648a9 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -946,6 +946,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx);
>  
>  void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
>      GCC_FMT_ATTR(2, 3);
> +void cpu_exec_initfn(CPUState *cpu);
>  void cpu_exec_exit(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
> diff --git a/qom/cpu.c b/qom/cpu.c
> index c40f774..85f1132 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -362,6 +362,8 @@ static void cpu_common_initfn(Object *obj)
>      QTAILQ_INIT(&cpu->watchpoints);
>  
>      cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
> +
> +    cpu_exec_initfn(cpu);
>  }
>  
>  static void cpu_common_finalize(Object *obj)
Igor Mammedov Oct. 17, 2016, 11:15 a.m. UTC | #2
On Sat, 15 Oct 2016 00:52:47 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Put in cpu_exec_initfn() what initializes the CPU,
> and let in cpu_exec_init() what adds it to the environment.
s/let/leave/
> 
> As cpu_exec_initfn() is called by all XX_cpu_initfn()
maybe add comma before "call it"
> call it
> directly in cpu_common_initfn().
> cpu_exec_init() is now a realize function, it will be renamed
> to cpu_exec_realizefn() and moved to the XX_cpu_realizefn()
> function in a following patch.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
With commit message amended

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  exec.c            | 10 ++++++----
>  include/qom/cpu.h |  1 +
>  qom/cpu.c         |  2 ++
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 374c364..d1e57c4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -610,11 +610,8 @@ void cpu_exec_exit(CPUState *cpu)
>      }
>  }
>  
> -void cpu_exec_init(CPUState *cpu, Error **errp)
> +void cpu_exec_initfn(CPUState *cpu)
>  {
> -    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> -    Error *local_err ATTRIBUTE_UNUSED = NULL;
> -
>      cpu->as = NULL;
>      cpu->num_ases = 0;
>  
> @@ -635,6 +632,11 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
>      cpu->memory = system_memory;
>      object_ref(OBJECT(cpu->memory));
>  #endif
> +}
> +
> +void cpu_exec_init(CPUState *cpu, Error **errp)
> +{
> +    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
>  
>      cpu_list_add(cpu);
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 6d481a1..d7648a9 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -946,6 +946,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx);
>  
>  void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
>      GCC_FMT_ATTR(2, 3);
> +void cpu_exec_initfn(CPUState *cpu);
>  void cpu_exec_exit(CPUState *cpu);
>  
>  #ifdef CONFIG_SOFTMMU
> diff --git a/qom/cpu.c b/qom/cpu.c
> index c40f774..85f1132 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -362,6 +362,8 @@ static void cpu_common_initfn(Object *obj)
>      QTAILQ_INIT(&cpu->watchpoints);
>  
>      cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
> +
> +    cpu_exec_initfn(cpu);
>  }
>  
>  static void cpu_common_finalize(Object *obj)
Eduardo Habkost Oct. 17, 2016, 6:46 p.m. UTC | #3
On Sat, Oct 15, 2016 at 12:52:47AM +0200, Laurent Vivier wrote:
> Put in cpu_exec_initfn() what initializes the CPU,
> and let in cpu_exec_init() what adds it to the environment.
> 
> As cpu_exec_initfn() is called by all XX_cpu_initfn() call it
> directly in cpu_common_initfn().
> cpu_exec_init() is now a realize function, it will be renamed
> to cpu_exec_realizefn() and moved to the XX_cpu_realizefn()
> function in a following patch.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Confirmed that:

* cpu->num_ases and cpu->as are never changed by architecture
  code before calling cpu_exec_init()
* cpu_exec_exit() is called on cpu_common_finalize()
* The cpu->memory reference will be dropped automatically
  because the property is registered using
  OBJ_PROP_LINK_UNREF_ON_RELEASE

BTW, the cpu->as and cpu->num_ases lines are redundant, because
QOM objects are guaranteed to be zeroed when allocated.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
diff mbox

Patch

diff --git a/exec.c b/exec.c
index 374c364..d1e57c4 100644
--- a/exec.c
+++ b/exec.c
@@ -610,11 +610,8 @@  void cpu_exec_exit(CPUState *cpu)
     }
 }
 
-void cpu_exec_init(CPUState *cpu, Error **errp)
+void cpu_exec_initfn(CPUState *cpu)
 {
-    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
-    Error *local_err ATTRIBUTE_UNUSED = NULL;
-
     cpu->as = NULL;
     cpu->num_ases = 0;
 
@@ -635,6 +632,11 @@  void cpu_exec_init(CPUState *cpu, Error **errp)
     cpu->memory = system_memory;
     object_ref(OBJECT(cpu->memory));
 #endif
+}
+
+void cpu_exec_init(CPUState *cpu, Error **errp)
+{
+    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
 
     cpu_list_add(cpu);
 
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 6d481a1..d7648a9 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -946,6 +946,7 @@  AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx);
 
 void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
+void cpu_exec_initfn(CPUState *cpu);
 void cpu_exec_exit(CPUState *cpu);
 
 #ifdef CONFIG_SOFTMMU
diff --git a/qom/cpu.c b/qom/cpu.c
index c40f774..85f1132 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -362,6 +362,8 @@  static void cpu_common_initfn(Object *obj)
     QTAILQ_INIT(&cpu->watchpoints);
 
     cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
+
+    cpu_exec_initfn(cpu);
 }
 
 static void cpu_common_finalize(Object *obj)