[v2,5/5] target-ppc: Add CPU finalizer

free() opcode tables. They are being malloc()'ed in create_new_table().

Resolves Jocelyn's TODO in former cpu_ppc_close().

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-ppc/translate_init.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

On Fri, Apr 06, 2012 at 06:17:12PM +0200, Andreas Färber wrote:
> free() opcode tables. They are being malloc()'ed in create_new_table().

This doesn't seem right.  Unless I've missed something in the call
path, create_new_table() is not called from the instance initializer.
So surely it should not be torn down in the instance finalizer.

> Resolves Jocelyn's TODO in former cpu_ppc_close().
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-ppc/translate_init.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index bb81bbc..5365229 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -10261,6 +10261,19 @@ static void ppc_cpu_initfn(Object *obj)
>  #endif /* !CONFIG_USER_ONLY */
>  }
>  
> +static void ppc_cpu_uninitfn(Object *obj)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(obj);
> +    CPUPPCState *env = &cpu->env;
> +    int i;
> +
> +    for (i = 0; i < 0x40; i++) {
> +        if (env->opcodes[i] != &invalid_handler) {
> +            free(env->opcodes[i]);
> +        }
> +    }
> +}
> +
>  static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> @@ -10275,6 +10288,7 @@ static const TypeInfo ppc_cpu_type_info = {
>      .parent = TYPE_CPU,
>      .instance_size = sizeof(PowerPCCPU),
>      .instance_init = ppc_cpu_initfn,
> +    .instance_finalize = ppc_cpu_uninitfn,
>      .abstract = false,
>      .class_size = sizeof(PowerPCCPUClass),
>      .class_init = ppc_cpu_class_init,

Am 11.04.2012 03:06, schrieb David Gibson:
> On Fri, Apr 06, 2012 at 06:17:12PM +0200, Andreas Färber wrote:
>> free() opcode tables. They are being malloc()'ed in create_new_table().
> 
> This doesn't seem right.  Unless I've missed something in the call
> path, create_new_table() is not called from the instance initializer.
> So surely it should not be torn down in the instance finalizer.

We have this call chain:

helper.c:cpu_ppc_init() -> translate_init.c:cpu_ppc_register_internal()
-> create_ppc_opcodes() -> create_ppc_opcodes() -> { fill_new_table()
(filling it with &invalid_handler),
register_insn() -> { register_[dbl]ind_insn() -> register_ind_in_table()
-> create_new_table(), register_direct_insn() -> insert_in_table()
}(each filling it with non-invalid handlers) }

So you are correct that it is not directly called from the initfn. The
reason not to do that yet is that cpu_ppc_register_internal() still uses
ppc_def_t, which my previous RFC patch series replaces through QOM
subclasses.

Since free() works fine with zero'ed memory such as after the
object_new() in cpu_ppc_init() I still think this patch is fully
correct. But we can postpone it if you prefer.

Andreas