From ba1ba5cca3962a9cc400c713c736b4fb8db1f38e Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Wed, 13 Sep 2017 18:04:57 +0200 Subject: arm: drop intermediate cpu_model -> cpu type parsing and use cpu type directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit there are 2 use cases to deal with: 1: fixed CPU models per board/soc 2: boards with user configurable cpu_model and fallback to default cpu_model if user hasn't specified one explicitly For the 1st drop intermediate cpu_model parsing and use const cpu type directly, which replaces: typename = object_class_get_name( cpu_class_by_name(TYPE_ARM_CPU, cpu_model)) object_new(typename) with object_new(FOO_CPU_TYPE_NAME) or cpu_generic_init(BASE_CPU_TYPE, "my cpu model") with cpu_create(FOO_CPU_TYPE_NAME) as result 1st use case doesn't have to invoke not necessary translation and not needed code is removed. For the 2nd 1: set default cpu type with MachineClass::default_cpu_type and 2: use generic cpu_model parsing that done before machine_init() is run and: 2.1: drop custom cpu_model parsing where pattern is: typename = object_class_get_name( cpu_class_by_name(TYPE_ARM_CPU, cpu_model)) [parse_features(typename, cpu_model, &err) ] 2.2: or replace cpu_generic_init() which does what 2.1 does + create_cpu(typename) with just create_cpu(machine->cpu_type) as result cpu_name -> cpu_type translation is done using generic machine code one including parsing optional features if supported/present (removes a bunch of duplicated cpu_model parsing code) and default cpu type is defined in an uniform way within machine_class_init callbacks instead of adhoc places in boadr's machine_init code. Signed-off-by: Igor Mammedov Reviewed-by: Eduardo Habkost Message-Id: <1505318697-77161-6-git-send-email-imammedo@redhat.com> Reviewed-by: Alistair Francis Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Eduardo Habkost --- hw/arm/virt.c | 46 +++++++++------------------------------------- 1 file changed, 9 insertions(+), 37 deletions(-) (limited to 'hw/arm/virt.c') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index cfd834d0cc..65d68bc50d 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -163,13 +163,13 @@ static const int a15irqmap[] = { }; static const char *valid_cpus[] = { - "cortex-a15", - "cortex-a53", - "cortex-a57", - "host", + ARM_CPU_TYPE_NAME("cortex-a15"), + ARM_CPU_TYPE_NAME("cortex-a53"), + ARM_CPU_TYPE_NAME("cortex-a57"), + ARM_CPU_TYPE_NAME("host"), }; -static bool cpuname_valid(const char *cpu) +static bool cpu_type_valid(const char *cpu) { int i; @@ -1259,18 +1259,8 @@ static void machvirt_init(MachineState *machine) MemoryRegion *secure_sysmem = NULL; int n, virt_max_cpus; MemoryRegion *ram = g_new(MemoryRegion, 1); - const char *cpu_model = machine->cpu_model; - char **cpustr; - ObjectClass *oc; - const char *typename; - CPUClass *cc; - Error *err = NULL; bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); - if (!cpu_model) { - cpu_model = "cortex-a15"; - } - /* We can probe only here because during property set * KVM is not available yet */ @@ -1287,11 +1277,8 @@ static void machvirt_init(MachineState *machine) } } - /* Separate the actual CPU model name from any appended features */ - cpustr = g_strsplit(cpu_model, ",", 2); - - if (!cpuname_valid(cpustr[0])) { - error_report("mach-virt: CPU %s not supported", cpustr[0]); + if (!cpu_type_valid(machine->cpu_type)) { + error_report("mach-virt: CPU type %s not supported", machine->cpu_type); exit(1); } @@ -1361,22 +1348,6 @@ static void machvirt_init(MachineState *machine) create_fdt(vms); - oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]); - if (!oc) { - error_report("Unable to find CPU definition"); - exit(1); - } - typename = object_class_get_name(oc); - - /* convert -smp CPU options specified by the user into global props */ - cc = CPU_CLASS(oc); - cc->parse_features(typename, cpustr[1], &err); - g_strfreev(cpustr); - if (err) { - error_report_err(err); - exit(1); - } - possible_cpus = mc->possible_cpu_arch_ids(machine); for (n = 0; n < possible_cpus->len; n++) { Object *cpuobj; @@ -1386,7 +1357,7 @@ static void machvirt_init(MachineState *machine) break; } - cpuobj = object_new(typename); + cpuobj = object_new(machine->cpu_type); object_property_set_int(cpuobj, possible_cpus->cpus[n].arch_id, "mp-affinity", NULL); @@ -1631,6 +1602,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) mc->minimum_page_bits = 12; mc->possible_cpu_arch_ids = virt_possible_cpu_arch_ids; mc->cpu_index_to_instance_props = virt_cpu_index_to_props; + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"); } static const TypeInfo virt_machine_info = { -- cgit 1.4.1 From 79e0793614fcd6b5674fa96180b66971c37d1dfd Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Thu, 1 Jun 2017 12:53:28 +0200 Subject: numa: cpu: calculate/set default node-ids after all -numa CLI options are parsed Calculating default node-ids for CPUs in possible_cpu_arch_ids() is rather fragile since defaults calculation uses nb_numa_nodes but callback might be potentially called early before all -numa CLI options are parsed, which would lead to cpus assigned only upto nb_numa_nodes at the time possible_cpu_arch_ids() is called. Issue was introduced by (7c88e65 numa: mirror cpu to node mapping in MachineState::possible_cpus) and for example CLI: -smp 4 -numa node,cpus=0 -numa node would set props.node-id in possible_cpus array for every non explicitly mapped CPU to the first node. Issue is not visible to guest nor to mgmt interface due to 1) implictly mapped cpus are forced to the first node in case of partial mapping 2) in case of default mapping possible_cpu_arch_ids() is called after all -numa options are parsed (resulting in correct mapping). However it's fragile to rely on late execution of possible_cpu_arch_ids(), therefore add machine specific callback that returns node-id for CPU and use it to calculate/ set defaults at machine_numa_finish_init() time when all -numa options are parsed. Reported-by: Eduardo Habkost Signed-off-by: Igor Mammedov Message-Id: <1496314408-163972-1-git-send-email-imammedo@redhat.com> Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- hw/arm/virt.c | 14 ++++++-------- hw/core/machine.c | 1 + hw/i386/pc.c | 20 +++++++++++--------- hw/ppc/spapr.c | 15 ++++++--------- include/hw/boards.h | 4 ++++ 5 files changed, 28 insertions(+), 26 deletions(-) (limited to 'hw/arm/virt.c') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 65d68bc50d..9e18b410d7 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1554,6 +1554,11 @@ virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index) return possible_cpus->cpus[cpu_index].props; } +static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx) +{ + return idx % nb_numa_nodes; +} + static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) { int n; @@ -1572,14 +1577,6 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) virt_cpu_mp_affinity(vms, n); ms->possible_cpus->cpus[n].props.has_thread_id = true; ms->possible_cpus->cpus[n].props.thread_id = n; - - /* default distribution of CPUs over NUMA nodes */ - if (nb_numa_nodes) { - /* preset values but do not enable them i.e. 'has_node_id = false', - * numa init code will enable them later if manual mapping wasn't - * present on CLI */ - ms->possible_cpus->cpus[n].props.node_id = n % nb_numa_nodes; - } } return ms->possible_cpus; } @@ -1603,6 +1600,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) mc->possible_cpu_arch_ids = virt_possible_cpu_arch_ids; mc->cpu_index_to_instance_props = virt_cpu_index_to_props; mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"); + mc->get_default_cpu_node_id = virt_get_default_cpu_node_id; } static const TypeInfo virt_machine_info = { diff --git a/hw/core/machine.c b/hw/core/machine.c index 41b53a17ad..80647edc2a 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -724,6 +724,7 @@ static void machine_numa_finish_init(MachineState *machine) /* fetch default mapping from board and enable it */ CpuInstanceProperties props = cpu_slot->props; + props.node_id = mc->get_default_cpu_node_id(machine, i); if (!default_mapping) { /* record slots with not set mapping, * TODO: make it hard error in future */ diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 2247ac0a01..610c65aeab 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2234,6 +2234,16 @@ pc_cpu_index_to_props(MachineState *ms, unsigned cpu_index) return possible_cpus->cpus[cpu_index].props; } +static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx) +{ + X86CPUTopoInfo topo; + + assert(idx < ms->possible_cpus->len); + x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id, + smp_cores, smp_threads, &topo); + return topo.pkg_id % nb_numa_nodes; +} + static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms) { int i; @@ -2263,15 +2273,6 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms) ms->possible_cpus->cpus[i].props.core_id = topo.core_id; ms->possible_cpus->cpus[i].props.has_thread_id = true; ms->possible_cpus->cpus[i].props.thread_id = topo.smt_id; - - /* default distribution of CPUs over NUMA nodes */ - if (nb_numa_nodes) { - /* preset values but do not enable them i.e. 'has_node_id = false', - * numa init code will enable them later if manual mapping wasn't - * present on CLI */ - ms->possible_cpus->cpus[i].props.node_id = - topo.pkg_id % nb_numa_nodes; - } } return ms->possible_cpus; } @@ -2316,6 +2317,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc->linuxboot_dma_enabled = true; mc->get_hotplug_handler = pc_get_hotpug_handler; mc->cpu_index_to_instance_props = pc_cpu_index_to_props; + mc->get_default_cpu_node_id = pc_get_default_cpu_node_id; mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids; mc->has_hotpluggable_cpus = true; mc->default_boot_order = "cad"; diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index f680f28a15..17ea77618c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3404,6 +3404,11 @@ spapr_cpu_index_to_props(MachineState *machine, unsigned cpu_index) return core_slot->props; } +static int64_t spapr_get_default_cpu_node_id(const MachineState *ms, int idx) +{ + return idx / smp_cores % nb_numa_nodes; +} + static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine) { int i; @@ -3428,15 +3433,6 @@ static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine) machine->possible_cpus->cpus[i].arch_id = core_id; machine->possible_cpus->cpus[i].props.has_core_id = true; machine->possible_cpus->cpus[i].props.core_id = core_id; - - /* default distribution of CPUs over NUMA nodes */ - if (nb_numa_nodes) { - /* preset values but do not enable them i.e. 'has_node_id = false', - * numa init code will enable them later if manual mapping wasn't - * present on CLI */ - machine->possible_cpus->cpus[i].props.node_id = - core_id / smp_threads / smp_cores % nb_numa_nodes; - } } return machine->possible_cpus; } @@ -3587,6 +3583,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) hc->pre_plug = spapr_machine_device_pre_plug; hc->plug = spapr_machine_device_plug; mc->cpu_index_to_instance_props = spapr_cpu_index_to_props; + mc->get_default_cpu_node_id = spapr_get_default_cpu_node_id; mc->possible_cpu_arch_ids = spapr_possible_cpu_arch_ids; hc->unplug_request = spapr_machine_device_unplug_request; diff --git a/include/hw/boards.h b/include/hw/boards.h index 6b67adaef6..156e0a5701 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -123,6 +123,9 @@ typedef struct { * Returns an array of @CPUArchId architecture-dependent CPU IDs * which includes CPU IDs for present and possible to hotplug CPUs. * Caller is responsible for freeing returned list. + * @get_default_cpu_node_id: + * returns default board specific node_id value for CPU slot specified by + * index @idx in @ms->possible_cpus[] * @has_hotpluggable_cpus: * If true, board supports CPUs creation with -device/device_add. * @default_cpu_type: @@ -196,6 +199,7 @@ struct MachineClass { CpuInstanceProperties (*cpu_index_to_instance_props)(MachineState *machine, unsigned cpu_index); const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); + int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); }; /** -- cgit 1.4.1