diff mbox series

[v5,4/4] spapr: move FORM1 verifications to do_client_architecture_support()

Message ID 20210907002527.412013-5-danielhb413@gmail.com (mailing list archive)
State New, archived
Headers show
Series pSeries FORM2 affinity support | expand

Commit Message

Daniel Henrique Barboza Sept. 7, 2021, 12:25 a.m. UTC
FORM2 NUMA affinity is prepared to deal with empty (memory/cpu less)
NUMA nodes. This is used by the DAX KMEM driver to locate a PAPR SCM
device that has a different latency than the original NUMA node from the
regular memory. FORM2 is also enable to deal with asymmetric NUMA
distances gracefully, something that our FORM1 implementation doesn't
do.

Move these FORM1 verifications to a new function and wait until after
CAS, when we're sure that we're sticking with FORM1, to enforce them.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/spapr.c              | 33 -------------------------
 hw/ppc/spapr_hcall.c        |  6 +++++
 hw/ppc/spapr_numa.c         | 49 ++++++++++++++++++++++++++++++++-----
 include/hw/ppc/spapr_numa.h |  1 +
 4 files changed, 50 insertions(+), 39 deletions(-)

Comments

David Gibson Sept. 7, 2021, 1:04 a.m. UTC | #1
On Mon, Sep 06, 2021 at 09:25:27PM -0300, Daniel Henrique Barboza wrote:
> FORM2 NUMA affinity is prepared to deal with empty (memory/cpu less)
> NUMA nodes. This is used by the DAX KMEM driver to locate a PAPR SCM
> device that has a different latency than the original NUMA node from the
> regular memory. FORM2 is also enable to deal with asymmetric NUMA
> distances gracefully, something that our FORM1 implementation doesn't
> do.
> 
> Move these FORM1 verifications to a new function and wait until after
> CAS, when we're sure that we're sticking with FORM1, to enforce them.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/spapr.c              | 33 -------------------------
>  hw/ppc/spapr_hcall.c        |  6 +++++
>  hw/ppc/spapr_numa.c         | 49 ++++++++++++++++++++++++++++++++-----
>  include/hw/ppc/spapr_numa.h |  1 +
>  4 files changed, 50 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8d98e3b08a..c974c07fb8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2797,39 +2797,6 @@ static void spapr_machine_init(MachineState *machine)
>      /* init CPUs */
>      spapr_init_cpus(spapr);
>  
> -    /*
> -     * check we don't have a memory-less/cpu-less NUMA node
> -     * Firmware relies on the existing memory/cpu topology to provide the
> -     * NUMA topology to the kernel.
> -     * And the linux kernel needs to know the NUMA topology at start
> -     * to be able to hotplug CPUs later.
> -     */
> -    if (machine->numa_state->num_nodes) {
> -        for (i = 0; i < machine->numa_state->num_nodes; ++i) {
> -            /* check for memory-less node */
> -            if (machine->numa_state->nodes[i].node_mem == 0) {
> -                CPUState *cs;
> -                int found = 0;
> -                /* check for cpu-less node */
> -                CPU_FOREACH(cs) {
> -                    PowerPCCPU *cpu = POWERPC_CPU(cs);
> -                    if (cpu->node_id == i) {
> -                        found = 1;
> -                        break;
> -                    }
> -                }
> -                /* memory-less and cpu-less node */
> -                if (!found) {
> -                    error_report(
> -                       "Memory-less/cpu-less nodes are not supported (node %d)",
> -                                 i);
> -                    exit(1);
> -                }
> -            }
> -        }
> -
> -    }
> -
>      spapr->gpu_numa_id = spapr_numa_initial_nvgpu_numa_id(machine);
>  
>      if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 7efbe93f4b..27ee713600 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1202,9 +1202,15 @@ target_ulong do_client_architecture_support(PowerPCCPU *cpu,
>       * If the guest chooses FORM2 we need to reset the associativity
>       * information - it is being defaulted to FORM1 during
>       * spapr_machine_reset().
> +     *
> +     * If we're sure that we'll be using FORM1, verify now if we have
> +     * a configuration or condition that is not available for FORM1
> +     * (namely asymmetric NUMA topologies and empty NUMA nodes).
>       */
>      if (spapr_ovec_test(spapr->ov5_cas, OV5_FORM2_AFFINITY)) {
>          spapr_numa_associativity_reset(spapr);
> +    } else {
> +        spapr_numa_check_FORM1_constraints(MACHINE(spapr));

Couldn't you put this call into one of the existing FORM1 functions?

>      }
>  
>      /*
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index ca276e16cb..0c57d03184 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -155,6 +155,49 @@ static void spapr_numa_define_associativity_domains(SpaprMachineState *spapr)
>  
>  }
>  
> +void spapr_numa_check_FORM1_constraints(MachineState *machine)
> +{
> +    int i;
> +
> +    if (!spapr_numa_is_symmetrical(machine)) {
> +        error_report("Asymmetrical NUMA topologies aren't supported "
> +                     "in the pSeries machine");

Error message needs an update since they are now possible with FORM2.

> +        exit(EXIT_FAILURE);
> +    }
> +
> +    /*
> +     * check we don't have a memory-less/cpu-less NUMA node
> +     * Firmware relies on the existing memory/cpu topology to provide the
> +     * NUMA topology to the kernel.
> +     * And the linux kernel needs to know the NUMA topology at start
> +     * to be able to hotplug CPUs later.
> +     */
> +    if (machine->numa_state->num_nodes) {
> +        for (i = 0; i < machine->numa_state->num_nodes; ++i) {
> +            /* check for memory-less node */
> +            if (machine->numa_state->nodes[i].node_mem == 0) {
> +                CPUState *cs;
> +                int found = 0;
> +                /* check for cpu-less node */
> +                CPU_FOREACH(cs) {
> +                    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +                    if (cpu->node_id == i) {
> +                        found = 1;
> +                        break;
> +                    }
> +                }
> +                /* memory-less and cpu-less node */
> +                if (!found) {
> +                    error_report(
> +                       "Memory-less/cpu-less nodes are not supported (node %d)",
> +                                 i);
> +                    exit(EXIT_FAILURE);
> +                }
> +            }
> +        }
> +    }
> +}
> +
>  /*
>   * Set NUMA machine state data based on FORM1 affinity semantics.
>   */
> @@ -172,12 +215,6 @@ static void spapr_numa_FORM1_affinity_init(SpaprMachineState *spapr,
>          return;
>      }
>  
> -    if (!spapr_numa_is_symmetrical(machine)) {
> -        error_report("Asymmetrical NUMA topologies aren't supported "
> -                     "in the pSeries machine");
> -        exit(EXIT_FAILURE);
> -    }
> -
>      spapr_numa_define_associativity_domains(spapr);
>  }
>  
> diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
> index 0e457bba57..b5a19cb3f1 100644
> --- a/include/hw/ppc/spapr_numa.h
> +++ b/include/hw/ppc/spapr_numa.h
> @@ -25,5 +25,6 @@ int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
>  int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt,
>                                           int offset);
>  unsigned int spapr_numa_initial_nvgpu_numa_id(MachineState *machine);
> +void spapr_numa_check_FORM1_constraints(MachineState *machine);
>  
>  #endif /* HW_SPAPR_NUMA_H */
diff mbox series

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8d98e3b08a..c974c07fb8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2797,39 +2797,6 @@  static void spapr_machine_init(MachineState *machine)
     /* init CPUs */
     spapr_init_cpus(spapr);
 
-    /*
-     * check we don't have a memory-less/cpu-less NUMA node
-     * Firmware relies on the existing memory/cpu topology to provide the
-     * NUMA topology to the kernel.
-     * And the linux kernel needs to know the NUMA topology at start
-     * to be able to hotplug CPUs later.
-     */
-    if (machine->numa_state->num_nodes) {
-        for (i = 0; i < machine->numa_state->num_nodes; ++i) {
-            /* check for memory-less node */
-            if (machine->numa_state->nodes[i].node_mem == 0) {
-                CPUState *cs;
-                int found = 0;
-                /* check for cpu-less node */
-                CPU_FOREACH(cs) {
-                    PowerPCCPU *cpu = POWERPC_CPU(cs);
-                    if (cpu->node_id == i) {
-                        found = 1;
-                        break;
-                    }
-                }
-                /* memory-less and cpu-less node */
-                if (!found) {
-                    error_report(
-                       "Memory-less/cpu-less nodes are not supported (node %d)",
-                                 i);
-                    exit(1);
-                }
-            }
-        }
-
-    }
-
     spapr->gpu_numa_id = spapr_numa_initial_nvgpu_numa_id(machine);
 
     if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 7efbe93f4b..27ee713600 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1202,9 +1202,15 @@  target_ulong do_client_architecture_support(PowerPCCPU *cpu,
      * If the guest chooses FORM2 we need to reset the associativity
      * information - it is being defaulted to FORM1 during
      * spapr_machine_reset().
+     *
+     * If we're sure that we'll be using FORM1, verify now if we have
+     * a configuration or condition that is not available for FORM1
+     * (namely asymmetric NUMA topologies and empty NUMA nodes).
      */
     if (spapr_ovec_test(spapr->ov5_cas, OV5_FORM2_AFFINITY)) {
         spapr_numa_associativity_reset(spapr);
+    } else {
+        spapr_numa_check_FORM1_constraints(MACHINE(spapr));
     }
 
     /*
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index ca276e16cb..0c57d03184 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -155,6 +155,49 @@  static void spapr_numa_define_associativity_domains(SpaprMachineState *spapr)
 
 }
 
+void spapr_numa_check_FORM1_constraints(MachineState *machine)
+{
+    int i;
+
+    if (!spapr_numa_is_symmetrical(machine)) {
+        error_report("Asymmetrical NUMA topologies aren't supported "
+                     "in the pSeries machine");
+        exit(EXIT_FAILURE);
+    }
+
+    /*
+     * check we don't have a memory-less/cpu-less NUMA node
+     * Firmware relies on the existing memory/cpu topology to provide the
+     * NUMA topology to the kernel.
+     * And the linux kernel needs to know the NUMA topology at start
+     * to be able to hotplug CPUs later.
+     */
+    if (machine->numa_state->num_nodes) {
+        for (i = 0; i < machine->numa_state->num_nodes; ++i) {
+            /* check for memory-less node */
+            if (machine->numa_state->nodes[i].node_mem == 0) {
+                CPUState *cs;
+                int found = 0;
+                /* check for cpu-less node */
+                CPU_FOREACH(cs) {
+                    PowerPCCPU *cpu = POWERPC_CPU(cs);
+                    if (cpu->node_id == i) {
+                        found = 1;
+                        break;
+                    }
+                }
+                /* memory-less and cpu-less node */
+                if (!found) {
+                    error_report(
+                       "Memory-less/cpu-less nodes are not supported (node %d)",
+                                 i);
+                    exit(EXIT_FAILURE);
+                }
+            }
+        }
+    }
+}
+
 /*
  * Set NUMA machine state data based on FORM1 affinity semantics.
  */
@@ -172,12 +215,6 @@  static void spapr_numa_FORM1_affinity_init(SpaprMachineState *spapr,
         return;
     }
 
-    if (!spapr_numa_is_symmetrical(machine)) {
-        error_report("Asymmetrical NUMA topologies aren't supported "
-                     "in the pSeries machine");
-        exit(EXIT_FAILURE);
-    }
-
     spapr_numa_define_associativity_domains(spapr);
 }
 
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index 0e457bba57..b5a19cb3f1 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -25,5 +25,6 @@  int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
 int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt,
                                          int offset);
 unsigned int spapr_numa_initial_nvgpu_numa_id(MachineState *machine);
+void spapr_numa_check_FORM1_constraints(MachineState *machine);
 
 #endif /* HW_SPAPR_NUMA_H */