[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 23/25] Use cpu_create(type) instead of cpu_in
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v4 23/25] Use cpu_create(type) instead of cpu_init(cpu_model) |
Date: |
Tue, 6 Feb 2018 20:25:38 -0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Mon, Feb 05, 2018 at 06:08:29PM +0100, Igor Mammedov wrote:
> With all targets defining CPU_RESOLVING_TYPE, refactor
> cpu_parse_cpu_model(type, cpu_model) to parse_cpu_model(cpu_model)
> so that callers won't have to know internal resolving cpu
> type. Place it in exec.c so it could be called from both
> target independed vl.c and *-user/main.c.
>
> That allows us to stop abusing cpu type from
> MachineClass::default_cpu_type
> as resolver class in vl.c which were confusing part of
> cpu_parse_cpu_model().
>
> Also with new parse_cpu_model(), the last users of cpu_init()
> in null-machine.c and bsd/linux-user targets could be switched
> to cpu_create() API and cpu_init() API will be removed by
> follow up patch.
>
> With no longer users left remove MachineState::cpu_model field,
> new code should use MachineState::cpu_type instead and
> leave cpu_model parsing to generic code in vl.c.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> v4:
> - actually remove no longer used MachineState::cpu_model field
> that I've lost somewhere during respins
>
> - squash in [PATCH v3 25/25] cpu: get rid of cpu_generic_init()
> as after rework/rebase cpu_generic_init() is being removed by
> this patch and only check removal was left in 25/25, which
> should be removed together with cpu_generic_init() in this patch
>
> CC: Richard Henderson <address@hidden>
> CC: "Emilio G. Cota" <address@hidden>
> CC: Paolo Bonzini <address@hidden>
> CC: Eduardo Habkost <address@hidden>
> CC: "Alex Bennée" <address@hidden>
> CC: "Philippe Mathieu-Daudé" <address@hidden>
>
> fixup: Use cpu_create(type) instead of cpu_init(cpu_model)
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> include/hw/boards.h | 1 -
> include/qom/cpu.h | 16 ++--------------
> bsd-user/main.c | 4 +++-
> exec.c | 23 +++++++++++++++++++++++
> hw/core/null-machine.c | 6 +++---
> linux-user/main.c | 8 ++++++--
> qom/cpu.c | 48 ++----------------------------------------------
> vl.c | 10 +++-------
> 8 files changed, 42 insertions(+), 74 deletions(-)
Less 32 lines. Nice. :)
[...]
> @@ -335,22 +304,9 @@ static ObjectClass *cpu_common_class_by_name(const char
> *cpu_model)
> static void cpu_common_parse_features(const char *typename, char *features,
> Error **errp)
> {
> - char *featurestr; /* Single "key=value" string being parsed */
> char *val;
> - static bool cpu_globals_initialized;
> -
> - /* TODO: all callers of ->parse_features() need to be changed to
> - * call it only once, so we can remove this check (or change it
> - * to assert(!cpu_globals_initialized).
> - * Current callers of ->parse_features() are:
> - * - cpu_generic_init()
> - */
> - if (cpu_globals_initialized) {
> - return;
Suggestion: replace this with assert(!cpu_globals_initialized)
just to make sure there are no bugs that make us register CPU
globals twice.
This shouldn't block the patch, though:
Reviewed-by: Eduardo Habkost <address@hidden>
> - }
> - cpu_globals_initialized = true;
> -
> - featurestr = features ? strtok(features, ",") : NULL;
> + /* Single "key=value" string being parsed */
> + char *featurestr = features ? strtok(features, ",") : NULL;
>
> while (featurestr) {
> val = strchr(featurestr, '=');
[...]
--
Eduardo