Re: [PATCH 2/3] plugins: Free CPUPluginState before destroying vCPU state

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 6/6/24 21:53, Philippe Mathieu-Daudé wrote:
> On 6/6/24 23:14, Pierrick Bouvier wrote:
>> On 6/6/24 05:40, Philippe Mathieu-Daudé wrote:
>>> cpu::plugin_state is allocated in cpu_common_initfn() when
>>> the vCPU state is created. Release it in cpu_common_finalize()
>>> when we are done.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    include/qemu/plugin.h | 3 +++
>>>    hw/core/cpu-common.c  | 5 +++++
>>>    2 files changed, 8 insertions(+)
>>>
>>> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
>>> index bc5aef979e..af5f9db469 100644
>>> --- a/include/qemu/plugin.h
>>> +++ b/include/qemu/plugin.h
>>> @@ -149,6 +149,9 @@ struct CPUPluginState {
>>>    /**
>>>     * qemu_plugin_create_vcpu_state: allocate plugin state
>>> + *
>>> + * The returned data must be released with g_free()
>>> + * when no longer required.
>>>     */
>>>    CPUPluginState *qemu_plugin_create_vcpu_state(void);
>>> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
>>> index bf1a7b8892..cd15402552 100644
>>> --- a/hw/core/cpu-common.c
>>> +++ b/hw/core/cpu-common.c
>>> @@ -283,6 +283,11 @@ static void cpu_common_finalize(Object *obj)
>>>    {
>>>        CPUState *cpu = CPU(obj);
>>> +#ifdef CONFIG_PLUGIN
>>> +    if (tcg_enabled()) {
>>> +        g_free(cpu->plugin_state);
>>> +    }
>>> +#endif
>>>        g_array_free(cpu->gdb_regs, TRUE);
>>>        qemu_lockcnt_destroy(&cpu->in_ioctl_lock);
>>>        qemu_mutex_destroy(&cpu->work_mutex);
>>
>> To ensure I get it right, order of cpu init/deinit is:
>> - init
>> - realize
>> - unrealize
>> - finalize
>> Is that correct?
> 
> Yes, this is valid for all QDev (CPU is based on it).
> 
> + init: allocate state, expose configurable properties
> . user configure properties
> + realize: consume properties to tune the object
> + reset: set default values
> . object is used
> + unrealize: undo stuff from realize because the object
>     might be realized again (unplug - plug)
> + finalize: release resources
> 
> See
> https://lore.kernel.org/qemu-devel/20240209123226.32576-1-philmd@linaro.org/
> 
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> Thanks!

Thanks, it definitely have its place in the official documentation, if 
you feel like adding it.