Re: [RFC 41/52] machine: Introduce core_type() hook

[Zhao Liu]

On Mon, Feb 13, 2023 at 02:35:24PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Mon, 13 Feb 2023 14:35:24 +0100
> From: Philippe Mathieu-Daudé <phi...@linaro.org>
> Subject: Re: [RFC 41/52] machine: Introduce core_type() hook
> 
> On 13/2/23 10:50, Zhao Liu wrote:
> > From: Zhao Liu <zhao1....@intel.com>
> > 
> > Since supported core types are architecture specific, we need this hook
> > to allow archs define its own parsing or validation method.
> > 
> > As the example, add the x86 core_type() which will be used in "-hybrid"
> > parameter parsing.
> > 
> > Signed-off-by: Zhao Liu <zhao1....@intel.com>
> > ---
> >   hw/core/machine-topo.c | 14 ++++++++++++++
> >   hw/core/machine.c      |  1 +
> >   hw/i386/x86.c          | 15 +++++++++++++++
> >   include/hw/boards.h    |  7 +++++++
> >   4 files changed, 37 insertions(+)
> > 
> > diff --git a/hw/core/machine-topo.c b/hw/core/machine-topo.c
> > index 12c05510c1b5..f9ab08a1252e 100644
> > --- a/hw/core/machine-topo.c
> > +++ b/hw/core/machine-topo.c
> > @@ -352,3 +352,17 @@ void machine_parse_smp_config(MachineState *ms,
> >           return;
> >       }
> >   }
> > +
> > +/*
> > + * machine_parse_hybrid_core_type: the default hook to parse hybrid core
> > + *                                 type corresponding to the coretype
> > + *                                 string option.
> > + */
> > +int machine_parse_hybrid_core_type(MachineState *ms, const char *coretype)
> > +{
> > +    if (strcmp(coretype, "") == 0 || strcmp(coretype, "none") == 0) {
> > +        return 0;
> > +    }
> > +
> > +    return -1;
> 
> Shouldn't this use mc->core_type()? Where is it used?

This interface is used in "[RFC 44/52] machine: Add "-hybrid" parsing
rule"[1] to check the "coretype" info passed from command line:

static void insert_core_into_cluster(MachineState *ms,
                                     HybridCluster *cluster,
                                     const HybridCoreOptions *config,
                                     Error **errp)
{
    ...
    ret = mc->core_type(ms, config->coretype);
    if (!ret) {
        error_setg(errp, "Invalid coretype=%s", config->coretype);
    }
    core_pack->core.core_type = ret;
    ...
}

I use that machine_parse_hybrid_core_type() as the default mc->core_type()
implementation. Other arch can override this default core_type() if
necessary, for example x86 has its own core_type implementation:
x86_parse_hybrid_core_type().

This default core_type() avoids useless and wrong core_type information
from the command line so that I can expose core type info in query_cpus_fast.

[1]: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03250.html


Zhao

> 
> > +}