From nobody Mon Feb 9 04:03:21 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1496161642774536.303561633024; Tue, 30 May 2017 09:27:22 -0700 (PDT) Received: from localhost ([::1]:54960 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFjzM-0008O1-9P for importer@patchew.org; Tue, 30 May 2017 12:27:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32910) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dFjwf-0006Ez-Al for qemu-devel@nongnu.org; Tue, 30 May 2017 12:24:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dFjwd-0005Cu-Qp for qemu-devel@nongnu.org; Tue, 30 May 2017 12:24:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:28890) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dFjwY-0005Ah-HM; Tue, 30 May 2017 12:24:26 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 75D2280F75; Tue, 30 May 2017 16:24:25 +0000 (UTC) Received: from dell-r430-03.lab.eng.brq.redhat.com (dell-r430-03.lab.eng.brq.redhat.com [10.34.112.60]) by smtp.corp.redhat.com (Postfix) with ESMTP id CFF48171A0; Tue, 30 May 2017 16:24:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 75D2280F75 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=imammedo@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 75D2280F75 From: Igor Mammedov To: qemu-devel@nongnu.org Date: Tue, 30 May 2017 18:24:02 +0200 Message-Id: <1496161442-96665-8-git-send-email-imammedo@redhat.com> In-Reply-To: <1496161442-96665-1-git-send-email-imammedo@redhat.com> References: <1496161442-96665-1-git-send-email-imammedo@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 30 May 2017 16:24:25 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 7/7] numa: cpu: calculate/set default node-ids after all -numa CLI options are parsed X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Jones , qemu-arm@nongnu.org, qemu-ppc@nongnu.org, Eduardo Habkost , David Gibson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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=3D0 -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 --- include/hw/boards.h | 3 +++ include/sysemu/numa.h | 9 +++++++++ hw/arm/virt.c | 16 ++++++++-------- hw/core/machine.c | 1 + hw/i386/pc.c | 21 ++++++++++++--------- hw/ppc/spapr.c | 16 +++++++--------- 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index 76ce021..063f329 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -94,6 +94,8 @@ 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 * @has_hotpluggable_cpus: * If true, board supports CPUs creation with -device/device_add. * @minimum_page_bits: @@ -151,6 +153,7 @@ struct MachineClass { CpuInstanceProperties (*cpu_index_to_instance_props)(MachineState *mac= hine, unsigned cpu_inde= x); const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); + int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); }; =20 /** diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h index 610eece..ea123ef 100644 --- a/include/sysemu/numa.h +++ b/include/sysemu/numa.h @@ -36,4 +36,13 @@ void numa_legacy_auto_assign_ram(MachineClass *mc, NodeI= nfo *nodes, void numa_default_auto_assign_ram(MachineClass *mc, NodeInfo *nodes, int nb_nodes, ram_addr_t size); void numa_cpu_pre_plug(const CPUArchId *slot, DeviceState *dev, Error **er= rp); + +static inline void assert_nb_numa_nodes_change(void) +{ + static int saved_nb_numa_nodes; + assert(nb_numa_nodes); + assert(saved_nb_numa_nodes =3D=3D 0 || saved_nb_numa_nodes =3D=3D nb_n= uma_nodes); + saved_nb_numa_nodes =3D nb_numa_nodes; +} + #endif diff --git a/hw/arm/virt.c b/hw/arm/virt.c index ce676df..74f1453 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1553,6 +1553,13 @@ virt_cpu_index_to_props(MachineState *ms, unsigned c= pu_index) return possible_cpus->cpus[cpu_index].props; } =20 +static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int id= x) +{ + assert(nb_numa_nodes); + assert_nb_numa_nodes_change(); + return idx % nb_numa_nodes; +} + static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) { int n; @@ -1571,14 +1578,6 @@ static const CPUArchIdList *virt_possible_cpu_arch_i= ds(MachineState *ms) virt_cpu_mp_affinity(vms, n); ms->possible_cpus->cpus[n].props.has_thread_id =3D true; ms->possible_cpus->cpus[n].props.thread_id =3D n; - - /* default distribution of CPUs over NUMA nodes */ - if (nb_numa_nodes) { - /* preset values but do not enable them i.e. 'has_node_id =3D = false', - * numa init code will enable them later if manual mapping was= n't - * present on CLI */ - ms->possible_cpus->cpus[n].props.node_id =3D n % nb_numa_nodes; - } } return ms->possible_cpus; } @@ -1601,6 +1600,7 @@ static void virt_machine_class_init(ObjectClass *oc, = void *data) mc->minimum_page_bits =3D 12; mc->possible_cpu_arch_ids =3D virt_possible_cpu_arch_ids; mc->cpu_index_to_instance_props =3D virt_cpu_index_to_props; + mc->get_default_cpu_node_id =3D virt_get_default_cpu_node_id; } =20 static const TypeInfo virt_machine_info =3D { diff --git a/hw/core/machine.c b/hw/core/machine.c index 964b75d..01028c8 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -723,6 +723,7 @@ static void machine_numa_finish_init(MachineState *mach= ine) /* fetch default mapping from board and enable it */ CpuInstanceProperties props =3D cpu_slot->props; =20 + props.node_id =3D 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 84f0a6f..51d5a1b 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2253,6 +2253,17 @@ pc_cpu_index_to_props(MachineState *ms, unsigned cpu= _index) return possible_cpus->cpus[cpu_index].props; } =20 +static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx) +{ + X86CPUTopoInfo topo; + + assert_nb_numa_nodes_change(); + assert(ms->possible_cpus && (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; @@ -2282,15 +2293,6 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids= (MachineState *ms) ms->possible_cpus->cpus[i].props.core_id =3D topo.core_id; ms->possible_cpus->cpus[i].props.has_thread_id =3D true; ms->possible_cpus->cpus[i].props.thread_id =3D 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 =3D = false', - * numa init code will enable them later if manual mapping was= n't - * present on CLI */ - ms->possible_cpus->cpus[i].props.node_id =3D - topo.pkg_id % nb_numa_nodes; - } } return ms->possible_cpus; } @@ -2335,6 +2337,7 @@ static void pc_machine_class_init(ObjectClass *oc, vo= id *data) pcmc->linuxboot_dma_enabled =3D true; mc->get_hotplug_handler =3D pc_get_hotpug_handler; mc->cpu_index_to_instance_props =3D pc_cpu_index_to_props; + mc->get_default_cpu_node_id =3D pc_get_default_cpu_node_id; mc->possible_cpu_arch_ids =3D pc_possible_cpu_arch_ids; mc->has_hotpluggable_cpus =3D true; mc->default_boot_order =3D "cad"; diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 96a2a74..06d0fb3 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3002,6 +3002,12 @@ spapr_cpu_index_to_props(MachineState *machine, unsi= gned cpu_index) return core_slot->props; } =20 +static int64_t spapr_get_default_cpu_node_id(const MachineState *ms, int i= dx) +{ + assert_nb_numa_nodes_change(); + return idx / smp_cores % nb_numa_nodes; +} + static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *mach= ine) { int i; @@ -3026,15 +3032,6 @@ static const CPUArchIdList *spapr_possible_cpu_arch_= ids(MachineState *machine) machine->possible_cpus->cpus[i].arch_id =3D core_id; machine->possible_cpus->cpus[i].props.has_core_id =3D true; machine->possible_cpus->cpus[i].props.core_id =3D 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 =3D = false', - * numa init code will enable them later if manual mapping was= n't - * present on CLI */ - machine->possible_cpus->cpus[i].props.node_id =3D - core_id / smp_threads / smp_cores % nb_numa_nodes; - } } return machine->possible_cpus; } @@ -3160,6 +3157,7 @@ static void spapr_machine_class_init(ObjectClass *oc,= void *data) hc->plug =3D spapr_machine_device_plug; hc->unplug =3D spapr_machine_device_unplug; mc->cpu_index_to_instance_props =3D spapr_cpu_index_to_props; + mc->get_default_cpu_node_id =3D spapr_get_default_cpu_node_id; mc->possible_cpu_arch_ids =3D spapr_possible_cpu_arch_ids; hc->unplug_request =3D spapr_machine_device_unplug_request; =20 --=20 2.7.4