| Message ID | 20240606124010.2460-3-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | plugins: Few debugging cleanups | expand |
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? Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
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!
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.
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);
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(+)