[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model |
Date: |
Fri, 19 Jan 2018 11:14:30 -0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Fri, Jan 19, 2018 at 11:14:39AM +0100, Igor Mammedov wrote:
> On Thu, 18 Jan 2018 17:18:09 -0200
> Eduardo Habkost <address@hidden> wrote:
>
> > On Thu, Jan 18, 2018 at 11:10:35AM +0100, Igor Mammedov wrote:
> > > On Wed, 17 Jan 2018 23:48:46 -0200
> > > Eduardo Habkost <address@hidden> wrote:
> > >
> > > > On Wed, Jan 17, 2018 at 04:43:32PM +0100, Igor Mammedov wrote:
> > > > > The last user of it was machine type 'none', which used field
> > > > > to create CPU id user requested it on CLI with -cpu option.
> [...]
>
> > It looks like default_cpu_type is being overloaded for two
> > different roles: 1) specifying the default CPU type; 2) finding
> > the arch-specific class to be used to parse -cpu.
> >
> > In the case of null-machine, these two roles conflict with each
> > other. I believe we can find other solutions instead of this
> > hack that involves lying on MachineClass::default_cpu_type (and
> > then having to work around the lie on machine_none_init()).
> >
> > I see multiple options: adding a new MachineClass field for that
> > (e.g. resolving_cpu_type, which defaults to default_cpu_type if
> > NULL); moving the CPU parsing code to arch_init.c (so it could
> > use CPU_RESOLVING_TYPE or something similar); adding a optional
> > MachineClass::parse_cpu_model hook. We could even try to get rid
> > of CPUClass::parse_features completely
> Adding hooks just for the sake on null-machine seems to be overkill,
> I'd go for arch_init.c but it won't work for linux-user, how about
> exec.c as following:
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 93bd546..0185589 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -661,8 +661,7 @@ ObjectClass *cpu_class_by_name(const char *typename,
> const char *cpu_model);
> [...]
>
> diff --git a/exec.c b/exec.c
> index d28fc0c..4543f06 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -817,6 +817,29 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
> #endif
> }
>
> +const char *parse_cpu_model(const char *cpu_model)
> +{
> + ObjectClass *oc;
> + CPUClass *cc;
> + gchar **model_pieces;
> + const char *cpu_type;
> +
> + model_pieces = g_strsplit(cpu_model, ",", 2);
> +
> + oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
> + if (oc == NULL) {
> + error_report("unable to find CPU model '%s'", model_pieces[0]);
> + g_strfreev(model_pieces);
> + exit(EXIT_FAILURE);
> + }
> +
> + cpu_type = object_class_get_name(oc);
> + cc = CPU_CLASS(oc);
> + cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
> + g_strfreev(model_pieces);
> + return cpu_type;
> +}
Sounds good to me. Only two comments:
This looks like duplication of cpu_parse_cpu_model(). Should
this function body be replaced with:
cpu_parse_cpu_model(CPU_RESOLVING_TYPE, cpu_model)
?
I would move this to arch_init.c, because that's where existing
target-dependent initialization code lives.
> [...]
--
Eduardo
- [Qemu-devel] [PATCH 17/24] xtensa: cpu: rename XTENSA_DEFAULT_CPU_TYPE to TARGET_DEFAULT_CPU_TYPE, (continued)
- [Qemu-devel] [PATCH 17/24] xtensa: cpu: rename XTENSA_DEFAULT_CPU_TYPE to TARGET_DEFAULT_CPU_TYPE, Igor Mammedov, 2018/01/17
- [Qemu-devel] [PATCH 19/24] tilegx: cpu: add TARGET_DEFAULT_CPU_TYPE macro, Igor Mammedov, 2018/01/17
- [Qemu-devel] [PATCH 18/24] hppa: cpu: add TARGET_DEFAULT_CPU_TYPE macro, Igor Mammedov, 2018/01/17
- [Qemu-devel] [PATCH 21/24] linux/bsd-user: drop cpu_init() and use cpu_create() instead, Igor Mammedov, 2018/01/17
- [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model, Igor Mammedov, 2018/01/17
- Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model, Eduardo Habkost, 2018/01/17
- Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model, Igor Mammedov, 2018/01/18
- Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model, Eduardo Habkost, 2018/01/18
- Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model, Igor Mammedov, 2018/01/19
- Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model,
Eduardo Habkost <=
- Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model, Igor Mammedov, 2018/01/19
- Re: [Qemu-devel] [PATCH 20/24] machine: drop MachineState::cpu_model, Eduardo Habkost, 2018/01/19
[Qemu-devel] [PATCH 23/24] nios2: 10m50_devboard: replace cpu_model with cpu_type, Igor Mammedov, 2018/01/17
[Qemu-devel] [PATCH 24/24] cpu: get rid of cpu_generic_init(), Igor Mammedov, 2018/01/17
[Qemu-devel] [PATCH 22/24] cpu: get rid of unused cpu_init() defines, Igor Mammedov, 2018/01/17
Re: [Qemu-devel] [PATCH 00/24] generalize parsing of cpu_model (part 4), Peter Maydell, 2018/01/17