[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH 10/10] pc: parse cpu features only once
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-arm] [PATCH 10/10] pc: parse cpu features only once |
Date: |
Wed, 8 Jun 2016 14:03:15 -0300 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Mon, Jun 06, 2016 at 05:16:52PM +0200, Igor Mammedov wrote:
> considering that features are converted to
> global properties and global properties are
> automatically applied to every new instance
> of created CPU (at object_new() time), there
> is no point in parsing cpu_model string every
> time a CPU created.
> So move parsing outside CPU creation loop and
> do it only once.
> Parsing also should be done before any CPU is
> created so that features would affect the first
> CPU a well.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> hw/i386/pc.c | 37 ++++++++++++++++++++++++++++---------
> target-i386/cpu.c | 44 --------------------------------------------
> target-i386/cpu.h | 1 -
> 3 files changed, 28 insertions(+), 54 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c48322b..0331e6d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1041,21 +1041,17 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int
> level)
> }
> }
>
> -static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> +static X86CPU *pc_new_cpu(const char *typename, int64_t apic_id,
> Error **errp)
> {
> X86CPU *cpu = NULL;
> Error *local_err = NULL;
>
> - cpu = cpu_x86_create(cpu_model, &local_err);
> - if (local_err != NULL) {
> - goto out;
> - }
> + cpu = X86_CPU(object_new(typename));
Nice. :)
>
> object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
> object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
>
> -out:
> if (local_err) {
> error_propagate(errp, local_err);
> object_unref(OBJECT(cpu));
> @@ -1067,7 +1063,8 @@ out:
> void pc_hot_add_cpu(const int64_t id, Error **errp)
> {
> X86CPU *cpu;
> - MachineState *machine = MACHINE(qdev_get_machine());
> + ObjectClass *oc;
> + PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> int64_t apic_id = x86_cpu_apic_id_from_index(id);
> Error *local_err = NULL;
>
> @@ -1095,7 +1092,9 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> return;
> }
>
> - cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);
> + assert(pcms->possible_cpus->cpus[0].cpu); /* BSP is always present */
> + oc = OBJECT_CLASS(CPU_GET_CLASS(pcms->possible_cpus->cpus[0].cpu));
The same pattern will probably repeat in other machines. I
wouldn't mind adding a new MachineState::cpu_type field, as we
already have MachineState::cpu_model.
MachineState::cpu_model could eventually go away if we move all
parse_features() calls to generic code.
> + cpu = pc_new_cpu(object_class_get_name(oc), apic_id, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> @@ -1106,6 +1105,10 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> void pc_cpus_init(PCMachineState *pcms)
> {
> int i, j;
> + CPUClass *cc;
> + ObjectClass *oc;
> + const char *typename;
> + gchar **model_pieces;
> X86CPU *cpu = NULL;
> MachineState *machine = MACHINE(pcms);
>
> @@ -1118,6 +1121,22 @@ void pc_cpus_init(PCMachineState *pcms)
> #endif
> }
>
> + model_pieces = g_strsplit(machine->cpu_model, ",", 2);
> + if (!model_pieces[0]) {
> + error_report("Invalid/empty CPU model name");
> + exit(1);
> + }
> +
> + oc = cpu_class_by_name(TYPE_X86_CPU, model_pieces[0]);
> + if (oc == NULL) {
> + error_report("Unable to find CPU definition: %s", model_pieces[0]);
> + exit(1);
> + }
> + typename = object_class_get_name(oc);
> + cc = CPU_CLASS(oc);
> + cc->parse_features(typename, model_pieces[1], &error_fatal);
> + g_strfreev(model_pieces);
Can we move this to a generic function to be reused by other
machines?
> +
> /* Calculates the limit to CPU APIC ID values
> *
> * Limit for the APIC ID value, so that all
> @@ -1148,7 +1167,7 @@ void pc_cpus_init(PCMachineState *pcms)
> }
>
> if (i < smp_cpus) {
> - cpu = pc_new_cpu(machine->cpu_model,
> x86_cpu_apic_id_from_index(i),
> + cpu = pc_new_cpu(typename, x86_cpu_apic_id_from_index(i),
> &error_fatal);
> pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
> object_unref(OBJECT(cpu));
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 43b22e6..c633579 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2211,50 +2211,6 @@ static void x86_cpu_load_def(X86CPU *cpu,
> X86CPUDefinition *def, Error **errp)
>
> }
>
> -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
> -{
> - X86CPU *cpu = NULL;
> - ObjectClass *oc;
> - CPUClass *cc;
> - gchar **model_pieces;
> - char *name, *features;
> - Error *error = NULL;
> - const char *typename;
> -
> - model_pieces = g_strsplit(cpu_model, ",", 2);
> - if (!model_pieces[0]) {
> - error_setg(&error, "Invalid/empty CPU model name");
> - goto out;
> - }
> - name = model_pieces[0];
> - features = model_pieces[1];
> -
> - oc = x86_cpu_class_by_name(name);
> - if (oc == NULL) {
> - error_setg(&error, "Unable to find CPU definition: %s", name);
> - goto out;
> - }
> - cc = CPU_CLASS(oc);
> - typename = object_class_get_name(oc);
> -
> - cc->parse_features(typename, features, &error);
> - cpu = X86_CPU(object_new(typename));
> - if (error) {
> - goto out;
> - }
> -
> -out:
> - if (error != NULL) {
> - error_propagate(errp, error);
> - if (cpu) {
> - object_unref(OBJECT(cpu));
> - cpu = NULL;
> - }
> - }
> - g_strfreev(model_pieces);
> - return cpu;
> -}
Nice. :)
> -
> X86CPU *cpu_x86_init(const char *cpu_model)
> {
> return X86_CPU(cpu_generic_init(TYPE_X86_CPU, cpu_model));
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index a257c53..a9a3b87 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1227,7 +1227,6 @@ void x86_cpu_exec_enter(CPUState *cpu);
> void x86_cpu_exec_exit(CPUState *cpu);
>
> X86CPU *cpu_x86_init(const char *cpu_model);
> -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp);
> int cpu_x86_exec(CPUState *cpu);
> void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
> int cpu_x86_support_mca_broadcast(CPUX86State *env);
> --
> 1.8.3.1
>
--
Eduardo
- Re: [Qemu-arm] [PATCH 06/10] target-i386: print obsolete warnings if +-features are used, (continued)
[Qemu-arm] [PATCH 10/10] pc: parse cpu features only once, Igor Mammedov, 2016/06/06
- Re: [Qemu-arm] [PATCH 10/10] pc: parse cpu features only once,
Eduardo Habkost <=
[Qemu-arm] [PATCH 08/10] cpu: use CPUClass->parse_features() as convertor to global properties, Igor Mammedov, 2016/06/06
[Qemu-arm] [PATCH 05/10] target-i386: cpu: consolidate calls of object_property_parse() in x86_cpu_parse_featurestr, Igor Mammedov, 2016/06/06
[Qemu-arm] [PATCH 07/10] target-sparc: cpu: use sparc_cpu_parse_features() directly, Igor Mammedov, 2016/06/06