All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <danielhb413@gmail.com>
To: qemu-devel@nongnu.org
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>,
	qemu-ppc@nongnu.org, david@gibson.dropbear.id.au
Subject: [PATCH 03/10] spapr: robustify NVLink2 NUMA node logic
Date: Fri, 14 Aug 2020 17:54:17 -0300	[thread overview]
Message-ID: <20200814205424.543857-4-danielhb413@gmail.com> (raw)
In-Reply-To: <20200814205424.543857-1-danielhb413@gmail.com>

NVLink2 GPUs are allocated in their own NUMA node, at maximum
distance from every other resource in the board. The existing
logic makes some assumptions that don't scale well:

- only NVLink2 GPUs will ever require such mechanism, meaning
that the GPU logic is tightly coupled with the NUMA setup of
the machine, via how ibm,max-associativity-domains is set.

- the code is relying on the lack of support for sparse NUMA
nodes in QEMU. Eventually this support can be implemented, and
then the assumption that spapr->gpu_numa_id represents the total
of NUMA nodes plus all generated NUMA ids for the GPUs, which
relies on all QEMU NUMA nodes not being sparsed, has a good
potential for disaster.

This patch aims to fix both assumptions by creating a generic
mechanism to get an available NUMA node, regardless of the
NUMA setup being sparse or not. The idea is to rename the existing
spapr->gpu_numa_id to spapr->current_numa_id and add a new
spapr->extra_numa_nodes attribute. They are used in a new function
called spapr_pci_get_available_numa_id(), that takes into account
that the NUMA conf can be sparsed or not, to retrieve an available
NUMA id for the caller. Each consecutive call of
spapr_pci_get_available_numa_id() will generate a new ID, up
to the limit of numa_state->num_nodes + spapr->extra_numa_nodes
exceeding MAX_NODES. This is a generic code being used only by
NVLink2 ATM, being available to be used in the future by any
other device.

With this new function in place, we can decouple
ibm,max-associativity-domains logic from NVLink2 logic by
using the new spapr->extra_numa_nodes to define the maxdomains
of the forth NUMA level. Instead of defining it as gpu_numa_id,
use num_nodes + extra_numa_nodes. This also makes it resilient
to any future change in the support of sparse NUMA nodes.

Despite all the code juggling, no functional change was made
because sparse NUMA nodes isn't a thing and we do not support
distinct NUMA distances via user input. Next patches will
change that.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/spapr.c              | 15 ++++++++++-----
 hw/ppc/spapr_pci.c          | 33 +++++++++++++++++++++++++++++++++
 hw/ppc/spapr_pci_nvlink2.c  | 10 ++++++----
 include/hw/pci-host/spapr.h |  2 ++
 include/hw/ppc/spapr.h      |  4 +++-
 5 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3b16edaf4c..22e78cfc84 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -910,13 +910,13 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
         cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE & 0xffffffff),
         cpu_to_be32(ms->smp.max_cpus / ms->smp.threads),
     };
-    uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0);
+    uint32_t maxdomain = cpu_to_be32(spapr->extra_numa_nodes > 1 ? 1 : 0);
     uint32_t maxdomains[] = {
         cpu_to_be32(4),
         maxdomain,
         maxdomain,
         maxdomain,
-        cpu_to_be32(spapr->gpu_numa_id),
+        cpu_to_be32(ms->numa_state->num_nodes + spapr->extra_numa_nodes),
     };
 
     _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
@@ -2824,13 +2824,18 @@ static void spapr_machine_init(MachineState *machine)
     /*
      * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
      * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
-     * called from vPHB reset handler so we initialize the counter here.
+     * called from vPHB reset handler. We have code to generate an extra numa
+     * id to place the GPU via 'extra_numa_nodes' and 'current_numa_node', which
+     * are initialized here.
+     *
      * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
      * must be equally distant from any other node.
-     * The final value of spapr->gpu_numa_id is going to be written to
+     *
+     * The extra NUMA node ids generated for GPU usage will be written to
      * max-associativity-domains in spapr_build_fdt().
      */
-    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
+    spapr->current_numa_id = 0;
+    spapr->extra_numa_nodes = 0;
 
     if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
         ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 0a418f1e67..09ac58fd7f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2492,3 +2492,36 @@ void spapr_pci_switch_vga(bool big_endian)
                            &big_endian);
     }
 }
+
+unsigned spapr_pci_get_available_numa_id(Error **errp)
+{
+    MachineState *machine = MACHINE(qdev_get_machine());
+    SpaprMachineState *spapr = SPAPR_MACHINE(machine);
+    NodeInfo *numa_info = machine->numa_state->nodes;
+    unsigned i, start;
+
+    if (machine->numa_state->num_nodes + spapr->extra_numa_nodes >= MAX_NODES) {
+        error_setg(errp,
+                   "Unable to get an extra NUMA node beyond MAX_NODES = %d",
+                   MAX_NODES);
+        return spapr->current_numa_id;
+    }
+
+    if (spapr->extra_numa_nodes == 0) {
+        start = 0;
+    } else {
+        start = spapr->current_numa_id + 1;
+    }
+
+    for (i = start; i < MAX_NODES; i++) {
+        if (!numa_info[i].present) {
+            spapr->extra_numa_nodes++;
+            spapr->current_numa_id = i;
+            return i;
+        }
+    }
+
+    error_setg(errp, "Unable to find a valid NUMA id");
+
+    return spapr->current_numa_id;
+}
diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c
index 76ae77ebc8..611c8a2957 100644
--- a/hw/ppc/spapr_pci_nvlink2.c
+++ b/hw/ppc/spapr_pci_nvlink2.c
@@ -87,9 +87,8 @@ static void spapr_pci_collect_nvgpu(SpaprPhbPciNvGpuConfig *nvgpus,
                                     PCIDevice *pdev, uint64_t tgt,
                                     MemoryRegion *mr, Error **errp)
 {
-    MachineState *machine = MACHINE(qdev_get_machine());
-    SpaprMachineState *spapr = SPAPR_MACHINE(machine);
     SpaprPhbPciNvGpuSlot *nvslot = spapr_nvgpu_get_slot(nvgpus, tgt);
+    Error *local_err = NULL;
 
     if (!nvslot) {
         error_setg(errp, "Found too many GPUs per vPHB");
@@ -100,8 +99,11 @@ static void spapr_pci_collect_nvgpu(SpaprPhbPciNvGpuConfig *nvgpus,
 
     nvslot->gpa = nvgpus->nv2_ram_current;
     nvgpus->nv2_ram_current += memory_region_size(mr);
-    nvslot->numa_id = spapr->gpu_numa_id;
-    ++spapr->gpu_numa_id;
+
+    nvslot->numa_id = spapr_pci_get_available_numa_id(&local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
 }
 
 static void spapr_pci_collect_nvnpu(SpaprPhbPciNvGpuConfig *nvgpus,
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 600eb55c34..8d93223a76 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -129,6 +129,8 @@ struct SpaprPhbState {
 #define SPAPR_PCI_NV2ATSD_WIN_SIZE   (NVGPU_MAX_NUM * NVGPU_MAX_LINKS * \
                                       64 * KiB)
 
+unsigned spapr_pci_get_available_numa_id(Error **errp);
+
 int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb,
                  uint32_t intc_phandle, void *fdt, int *node_offset);
 
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 3134d339e8..739a6a4942 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -227,7 +227,9 @@ struct SpaprMachineState {
     bool cmd_line_caps[SPAPR_CAP_NUM];
     SpaprCapabilities def, eff, mig;
 
-    unsigned gpu_numa_id;
+    unsigned current_numa_id;
+    unsigned extra_numa_nodes;
+
     SpaprTpmProxy *tpm_proxy;
 
     Error *fwnmi_migration_blocker;
-- 
2.26.2



  parent reply	other threads:[~2020-08-15 15:48 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14 20:54 [PATCH 00/10] pseries NUMA distance rework Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 01/10] hw: add compat machines for 5.2 Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 02/10] numa: introduce MachineClass::forbid_asymmetrical_numa Daniel Henrique Barboza
2020-08-20  1:17   ` David Gibson
2020-08-20  2:11     ` Eduardo Habkost
2020-08-20  4:15       ` David Gibson
2020-08-20 10:33         ` Daniel Henrique Barboza
2020-08-20 14:29           ` Igor Mammedov
2020-08-20 16:51         ` Eduardo Habkost
2020-08-21  8:55           ` Igor Mammedov
2020-08-21 12:47             ` Daniel Henrique Barboza
2020-08-24  6:08               ` David Gibson
2020-08-24 11:45                 ` Daniel Henrique Barboza
2020-08-24 23:49                   ` David Gibson
2020-08-25  9:56                     ` Daniel Henrique Barboza
2020-08-25 11:12                       ` David Gibson
2020-09-23 15:21           ` John Snow
2020-08-14 20:54 ` Daniel Henrique Barboza [this message]
2020-08-20  2:14   ` [PATCH 03/10] spapr: robustify NVLink2 NUMA node logic David Gibson
2020-08-26 21:49     ` Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 04/10] spapr: add spapr_machine_using_legacy_numa() helper Daniel Henrique Barboza
2020-08-20  2:15   ` David Gibson
2020-08-14 20:54 ` [PATCH 05/10] spapr: make ibm, max-associativity-domains scale with user input Daniel Henrique Barboza
2020-08-20  2:55   ` [PATCH 05/10] spapr: make ibm,max-associativity-domains " David Gibson
2020-08-26 21:17     ` Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 06/10] spapr: allow 4 NUMA levels in ibm, associativity-reference-points Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 07/10] spapr: create helper to set ibm,associativity Daniel Henrique Barboza
2020-08-20  3:00   ` David Gibson
2020-08-20 10:39     ` Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 08/10] spapr: introduce SpaprMachineClass::numa_assoc_domains Daniel Henrique Barboza
2020-08-20  4:26   ` David Gibson
2020-08-26 20:06     ` Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 09/10] spapr: consider user input when defining spapr guest NUMA Daniel Henrique Barboza
2020-08-14 20:54 ` [PATCH 10/10] specs/ppc-spapr-numa: update with new NUMA support Daniel Henrique Barboza

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200814205424.543857-4-danielhb413@gmail.com \
    --to=danielhb413@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.