Re: [Qemu-devel] [PATCH 1/4] cpu: add cpu_generic_new()

[Eduardo Habkost]

On Mon, Feb 13, 2017 at 02:28:16PM +0000, Peter Maydell wrote:
> From: Michael Davidsaver <address@hidden>
> 
> Add a new API cpu_generic_new() which creates a QOM CPU object
> (including calling the CPU class parse_features method) but
> does not realize it, and reimplement cpu_generic_init() to
> simply call cpu_generic_new() and then immediately realize.
> the CPU.
> 
> The motivation for this is that there is currently no
> way for board code to take advantage of the cpu_generic_init()
> convenience function if it needs to set QOM properties
> on the created CPU. Instead it has to do it all manually, which
> is prone to bugs like that fixed in commit 00909b585861 (where
> a board code forgot to call parse_features which meant that
> command line +feature,-feature flags were ignored).
> 
> Signed-off-by: Michael Davidsaver <address@hidden>
> [PMM: renamed new function to cpu_generic_new(), rewrote
>  commit message]
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  include/qom/cpu.h | 17 +++++++++++++++++
>  qom/cpu.c         | 27 ++++++++++++++++++---------
>  2 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 45bcf21..e900586 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -599,11 +599,28 @@ void cpu_reset(CPUState *cpu);
>  ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
>  
>  /**
> + * cpu_generic_new:
> + * @typename: The CPU base type.
> + * @cpu_model: The model string including optional parameters.
> + *
> + * Instantiates a CPU, processes optional parameters but does not realize it.
> + * This is the recommended way to create a CPU object which needs to be
> + * configured by then setting QOM properties on it. The configured CPU can
> + * then be realized in the usual way by calling
> + *    object_property_set_bool(cpuobj, true, "realized", &err);
> + *
> + * Returns: A #CPUState or %NULL if an error occurred.
> + */
> +CPUState *cpu_generic_new(const char *typename, const char *cpu_model);

I am not sure we will stlil add this, but in case we are going to
add this helper temporarily until we have Igor's generic
cpu_model parsing, it would be useful to make the function
signature be:
  CPUState *cpu_generic_new(const char *typename, MachineState *machine)
and parse machine->cpu_model inside the function implementation.

This way we could replace the helper with
  object_new(machine->cpu_typename)
more easily once we add Igor's cleanup.

-- 
Eduardo