* [PATCH v3 1/7] spapr: introduce SpaprMachineState::numa_assoc_array
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
@ 2020-09-03 22:06 ` Daniel Henrique Barboza
2020-09-03 22:06 ` [PATCH v3 2/7] spapr, spapr_numa: handle vcpu ibm,associativity Daniel Henrique Barboza
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-03 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, david
The next step to centralize all NUMA/associativity handling in
the spapr machine is to create a 'one stop place' for all
things ibm,associativity.
This patch introduces numa_assoc_array, a 2 dimensional array
that will store all ibm,associativity arrays of all NUMA nodes.
This array is initialized in a new spapr_numa_associativity_init()
function, called in spapr_machine_init(). It is being initialized
with the same values used in other ibm,associativity properties
around spapr files (i.e. all zeros, last value is node_id).
The idea is to remove all hardcoded definitions and FDT writes
of ibm,associativity arrays, doing instead a call to the new
helper spapr_numa_write_associativity_dt() helper, that will
be able to write the DT with the correct values.
We'll start small, handling the trivial cases first. The
remaining instances of ibm,associativity will be handled
next.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr.c | 23 ++++++++++-------------
hw/ppc/spapr_numa.c | 30 ++++++++++++++++++++++++++++++
hw/ppc/spapr_nvdimm.c | 19 +++++++------------
hw/ppc/spapr_pci.c | 9 ++-------
include/hw/ppc/spapr.h | 12 ++++++++++++
include/hw/ppc/spapr_numa.h | 11 +++++++++++
include/hw/ppc/spapr_nvdimm.h | 2 +-
7 files changed, 73 insertions(+), 33 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a45912acac..1ad6f59863 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -314,14 +314,9 @@ static void add_str(GString *s, const gchar *s1)
g_string_append_len(s, s1, strlen(s1) + 1);
}
-static int spapr_dt_memory_node(void *fdt, int nodeid, hwaddr start,
- hwaddr size)
+static int spapr_dt_memory_node(SpaprMachineState *spapr, void *fdt, int nodeid,
+ hwaddr start, hwaddr size)
{
- uint32_t associativity[] = {
- cpu_to_be32(0x4), /* length */
- cpu_to_be32(0x0), cpu_to_be32(0x0),
- cpu_to_be32(0x0), cpu_to_be32(nodeid)
- };
char mem_name[32];
uint64_t mem_reg_property[2];
int off;
@@ -335,8 +330,7 @@ static int spapr_dt_memory_node(void *fdt, int nodeid, hwaddr start,
_FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
_FDT((fdt_setprop(fdt, off, "reg", mem_reg_property,
sizeof(mem_reg_property))));
- _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
- sizeof(associativity))));
+ spapr_numa_write_associativity_dt(spapr, fdt, off, nodeid);
return off;
}
@@ -649,7 +643,7 @@ static int spapr_dt_memory(SpaprMachineState *spapr, void *fdt)
if (!mem_start) {
/* spapr_machine_init() checks for rma_size <= node0_size
* already */
- spapr_dt_memory_node(fdt, i, 0, spapr->rma_size);
+ spapr_dt_memory_node(spapr, fdt, i, 0, spapr->rma_size);
mem_start += spapr->rma_size;
node_size -= spapr->rma_size;
}
@@ -661,7 +655,7 @@ static int spapr_dt_memory(SpaprMachineState *spapr, void *fdt)
sizetmp = 1ULL << ctzl(mem_start);
}
- spapr_dt_memory_node(fdt, i, mem_start, sizetmp);
+ spapr_dt_memory_node(spapr, fdt, i, mem_start, sizetmp);
node_size -= sizetmp;
mem_start += sizetmp;
}
@@ -1275,7 +1269,7 @@ void *spapr_build_fdt(SpaprMachineState *spapr, bool reset, size_t space)
/* NVDIMM devices */
if (mc->nvdimm_supported) {
- spapr_dt_persistent_memory(fdt);
+ spapr_dt_persistent_memory(spapr, fdt);
}
return fdt;
@@ -2810,6 +2804,9 @@ static void spapr_machine_init(MachineState *machine)
*/
spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
+ /* Init numa_assoc_array */
+ spapr_numa_associativity_init(spapr, machine);
+
if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
spapr->max_compat_pvr)) {
@@ -3394,7 +3391,7 @@ int spapr_lmb_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
addr = spapr_drc_index(drc) * SPAPR_MEMORY_BLOCK_SIZE;
node = object_property_get_uint(OBJECT(drc->dev), PC_DIMM_NODE_PROP,
&error_abort);
- *fdt_start_offset = spapr_dt_memory_node(fdt, node, addr,
+ *fdt_start_offset = spapr_dt_memory_node(spapr, fdt, node, addr,
SPAPR_MEMORY_BLOCK_SIZE);
return 0;
}
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index cdf3288cbd..f6b6fe648f 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -15,6 +15,36 @@
#include "hw/ppc/spapr_numa.h"
#include "hw/ppc/fdt.h"
+
+void spapr_numa_associativity_init(SpaprMachineState *spapr,
+ MachineState *machine)
+{
+ int nb_numa_nodes = machine->numa_state->num_nodes;
+ int i;
+
+ /*
+ * For all associativity arrays: first position is the size,
+ * position MAX_DISTANCE_REF_POINTS is always the numa_id,
+ * represented by the index 'i'.
+ *
+ * This will break on sparse NUMA setups, when/if QEMU starts
+ * to support it, because there will be no more guarantee that
+ * 'i' will be a valid node_id set by the user.
+ */
+ for (i = 0; i < nb_numa_nodes; i++) {
+ spapr->numa_assoc_array[i][0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS);
+ spapr->numa_assoc_array[i][MAX_DISTANCE_REF_POINTS] = cpu_to_be32(i);
+ }
+}
+
+void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
+ int offset, int nodeid)
+{
+ _FDT((fdt_setprop(fdt, offset, "ibm,associativity",
+ spapr->numa_assoc_array[nodeid],
+ sizeof(spapr->numa_assoc_array[nodeid]))));
+}
+
/*
* Helper that writes ibm,associativity-reference-points and
* max-associativity-domains in the RTAS pointed by @rtas
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index 5188e2f503..63872054f3 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -31,6 +31,7 @@
#include "hw/ppc/fdt.h"
#include "qemu/range.h"
#include "sysemu/sysemu.h"
+#include "hw/ppc/spapr_numa.h"
void spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
uint64_t size, Error **errp)
@@ -117,8 +118,8 @@ void spapr_create_nvdimm_dr_connectors(SpaprMachineState *spapr)
}
-static int spapr_dt_nvdimm(void *fdt, int parent_offset,
- NVDIMMDevice *nvdimm)
+static int spapr_dt_nvdimm(SpaprMachineState *spapr, void *fdt,
+ int parent_offset, NVDIMMDevice *nvdimm)
{
int child_offset;
char *buf;
@@ -128,11 +129,6 @@ static int spapr_dt_nvdimm(void *fdt, int parent_offset,
&error_abort);
uint64_t slot = object_property_get_uint(OBJECT(nvdimm), PC_DIMM_SLOT_PROP,
&error_abort);
- uint32_t associativity[] = {
- cpu_to_be32(0x4), /* length */
- cpu_to_be32(0x0), cpu_to_be32(0x0),
- cpu_to_be32(0x0), cpu_to_be32(node)
- };
uint64_t lsize = nvdimm->label_size;
uint64_t size = object_property_get_int(OBJECT(nvdimm), PC_DIMM_SIZE_PROP,
NULL);
@@ -152,8 +148,7 @@ static int spapr_dt_nvdimm(void *fdt, int parent_offset,
_FDT((fdt_setprop_string(fdt, child_offset, "compatible", "ibm,pmemory")));
_FDT((fdt_setprop_string(fdt, child_offset, "device_type", "ibm,pmemory")));
- _FDT((fdt_setprop(fdt, child_offset, "ibm,associativity", associativity,
- sizeof(associativity))));
+ spapr_numa_write_associativity_dt(spapr, fdt, child_offset, node);
buf = qemu_uuid_unparse_strdup(&nvdimm->uuid);
_FDT((fdt_setprop_string(fdt, child_offset, "ibm,unit-guid", buf)));
@@ -179,12 +174,12 @@ int spapr_pmem_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
{
NVDIMMDevice *nvdimm = NVDIMM(drc->dev);
- *fdt_start_offset = spapr_dt_nvdimm(fdt, 0, nvdimm);
+ *fdt_start_offset = spapr_dt_nvdimm(spapr, fdt, 0, nvdimm);
return 0;
}
-void spapr_dt_persistent_memory(void *fdt)
+void spapr_dt_persistent_memory(SpaprMachineState *spapr, void *fdt)
{
int offset = fdt_subnode_offset(fdt, 0, "persistent-memory");
GSList *iter, *nvdimms = nvdimm_get_device_list();
@@ -202,7 +197,7 @@ void spapr_dt_persistent_memory(void *fdt)
for (iter = nvdimms; iter; iter = iter->next) {
NVDIMMDevice *nvdimm = iter->data;
- spapr_dt_nvdimm(fdt, offset, nvdimm);
+ spapr_dt_nvdimm(spapr, fdt, offset, nvdimm);
}
g_slist_free(nvdimms);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 0a418f1e67..4d97ff6c70 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -52,6 +52,7 @@
#include "sysemu/kvm.h"
#include "sysemu/hostmem.h"
#include "sysemu/numa.h"
+#include "hw/ppc/spapr_numa.h"
/* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
#define RTAS_QUERY_FN 0
@@ -2321,11 +2322,6 @@ int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb,
cpu_to_be32(1),
cpu_to_be32(RTAS_IBM_RESET_PE_DMA_WINDOW)
};
- uint32_t associativity[] = {cpu_to_be32(0x4),
- cpu_to_be32(0x0),
- cpu_to_be32(0x0),
- cpu_to_be32(0x0),
- cpu_to_be32(phb->numa_node)};
SpaprTceTable *tcet;
SpaprDrc *drc;
Error *err = NULL;
@@ -2358,8 +2354,7 @@ int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb,
/* Advertise NUMA via ibm,associativity */
if (phb->numa_node != -1) {
- _FDT(fdt_setprop(fdt, bus_off, "ibm,associativity", associativity,
- sizeof(associativity)));
+ spapr_numa_write_associativity_dt(spapr, fdt, bus_off, phb->numa_node);
}
/* Build the interrupt-map, this must matches what is done
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index a1e230ad39..9a63380801 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -105,6 +105,16 @@ typedef enum {
#define FDT_MAX_SIZE 0x100000
+/*
+ * NUMA related macros. MAX_DISTANCE_REF_POINTS was taken
+ * from Taken from Linux kernel arch/powerpc/mm/numa.h.
+ *
+ * NUMA_ASSOC_SIZE is the base array size of an ibm,associativity
+ * array for any non-CPU resource.
+ */
+#define MAX_DISTANCE_REF_POINTS 4
+#define NUMA_ASSOC_SIZE (MAX_DISTANCE_REF_POINTS + 1)
+
typedef struct SpaprCapabilities SpaprCapabilities;
struct SpaprCapabilities {
uint8_t caps[SPAPR_CAP_NUM];
@@ -231,6 +241,8 @@ struct SpaprMachineState {
unsigned gpu_numa_id;
SpaprTpmProxy *tpm_proxy;
+ uint32_t numa_assoc_array[MAX_NODES][NUMA_ASSOC_SIZE];
+
Error *fwnmi_migration_blocker;
};
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index 7a370a8768..a2a4df55f7 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -13,8 +13,19 @@
#ifndef HW_SPAPR_NUMA_H
#define HW_SPAPR_NUMA_H
+#include "hw/boards.h"
#include "hw/ppc/spapr.h"
+/*
+ * Having both SpaprMachineState and MachineState as arguments
+ * feels odd, but it will spare a MACHINE() call inside the
+ * function. spapr_machine_init() is the only caller for it, and
+ * it has both pointers resolved already.
+ */
+void spapr_numa_associativity_init(SpaprMachineState *spapr,
+ MachineState *machine);
void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
+void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
+ int offset, int nodeid);
#endif /* HW_SPAPR_NUMA_H */
diff --git a/include/hw/ppc/spapr_nvdimm.h b/include/hw/ppc/spapr_nvdimm.h
index 10a6d9dbbc..3eb344e8e9 100644
--- a/include/hw/ppc/spapr_nvdimm.h
+++ b/include/hw/ppc/spapr_nvdimm.h
@@ -27,7 +27,7 @@ QEMU_BUILD_BUG_ON(SPAPR_MINIMUM_SCM_BLOCK_SIZE % SPAPR_MEMORY_BLOCK_SIZE);
int spapr_pmem_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
void *fdt, int *fdt_start_offset, Error **errp);
-void spapr_dt_persistent_memory(void *fdt);
+void spapr_dt_persistent_memory(SpaprMachineState *spapr, void *fdt);
void spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
uint64_t size, Error **errp);
void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp);
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 2/7] spapr, spapr_numa: handle vcpu ibm,associativity
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
2020-09-03 22:06 ` [PATCH v3 1/7] spapr: introduce SpaprMachineState::numa_assoc_array Daniel Henrique Barboza
@ 2020-09-03 22:06 ` Daniel Henrique Barboza
2020-09-03 23:23 ` David Gibson
2020-09-03 22:06 ` [PATCH v3 3/7] spapr, spapr_numa: move lookup-arrays handling to spapr_numa.c Daniel Henrique Barboza
` (5 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-03 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, david
Vcpus have an additional paramenter to be appended, vcpu_id. This
also changes the size of the of property itself, which is being
represented in index 0 of numa_assoc_array[cpu->node_id],
and defaults to MAX_DISTANCE_REF_POINTS for all cases but
vcpus.
All this logic makes more sense in spapr_numa.c, where we handle
everything NUMA and associativity. A new helper spapr_numa_fixup_cpu_dt()
was added, and spapr.c uses it the same way as it was using the former
spapr_fixup_cpu_numa_dt().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr.c | 17 +----------------
hw/ppc/spapr_numa.c | 27 +++++++++++++++++++++++++++
include/hw/ppc/spapr_numa.h | 2 ++
3 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1ad6f59863..badfa86319 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -202,21 +202,6 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
return ret;
}
-static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
-{
- int index = spapr_get_vcpu_id(cpu);
- uint32_t associativity[] = {cpu_to_be32(0x5),
- cpu_to_be32(0x0),
- cpu_to_be32(0x0),
- cpu_to_be32(0x0),
- cpu_to_be32(cpu->node_id),
- cpu_to_be32(index)};
-
- /* Advertise NUMA via ibm,associativity */
- return fdt_setprop(fdt, offset, "ibm,associativity", associativity,
- sizeof(associativity));
-}
-
static void spapr_dt_pa_features(SpaprMachineState *spapr,
PowerPCCPU *cpu,
void *fdt, int offset)
@@ -785,7 +770,7 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int offset,
pft_size_prop, sizeof(pft_size_prop))));
if (ms->numa_state->num_nodes > 1) {
- _FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cpu));
+ _FDT(spapr_numa_fixup_cpu_dt(spapr, fdt, offset, cpu));
}
_FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index f6b6fe648f..1a1ec8bcff 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -45,6 +45,33 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
sizeof(spapr->numa_assoc_array[nodeid]))));
}
+int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
+ int offset, PowerPCCPU *cpu)
+{
+ uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
+ uint32_t vcpu_assoc[vcpu_assoc_size];
+ int index = spapr_get_vcpu_id(cpu);
+ int i;
+
+ /*
+ * VCPUs have an extra 'cpu_id' value in ibm,associativity
+ * compared to other resources. Increment the size at index
+ * 0, copy all associativity domains already set, then put
+ * cpu_id last.
+ */
+ vcpu_assoc[0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS + 1);
+
+ for (i = 1; i <= MAX_DISTANCE_REF_POINTS; i++) {
+ vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
+ }
+
+ vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
+
+ /* Advertise NUMA via ibm,associativity */
+ return fdt_setprop(fdt, offset, "ibm,associativity",
+ vcpu_assoc, sizeof(vcpu_assoc));
+}
+
/*
* Helper that writes ibm,associativity-reference-points and
* max-associativity-domains in the RTAS pointed by @rtas
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index a2a4df55f7..43c6a16fe3 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -27,5 +27,7 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr,
void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
int offset, int nodeid);
+int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
+ int offset, PowerPCCPU *cpu);
#endif /* HW_SPAPR_NUMA_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 2/7] spapr, spapr_numa: handle vcpu ibm,associativity
2020-09-03 22:06 ` [PATCH v3 2/7] spapr, spapr_numa: handle vcpu ibm,associativity Daniel Henrique Barboza
@ 2020-09-03 23:23 ` David Gibson
2020-09-04 0:32 ` Daniel Henrique Barboza
0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2020-09-03 23:23 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4801 bytes --]
On Thu, Sep 03, 2020 at 07:06:34PM -0300, Daniel Henrique Barboza wrote:
> Vcpus have an additional paramenter to be appended, vcpu_id. This
> also changes the size of the of property itself, which is being
> represented in index 0 of numa_assoc_array[cpu->node_id],
> and defaults to MAX_DISTANCE_REF_POINTS for all cases but
> vcpus.
>
> All this logic makes more sense in spapr_numa.c, where we handle
> everything NUMA and associativity. A new helper spapr_numa_fixup_cpu_dt()
> was added, and spapr.c uses it the same way as it was using the former
> spapr_fixup_cpu_numa_dt().
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/spapr.c | 17 +----------------
> hw/ppc/spapr_numa.c | 27 +++++++++++++++++++++++++++
> include/hw/ppc/spapr_numa.h | 2 ++
> 3 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1ad6f59863..badfa86319 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -202,21 +202,6 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
> return ret;
> }
>
> -static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
> -{
> - int index = spapr_get_vcpu_id(cpu);
> - uint32_t associativity[] = {cpu_to_be32(0x5),
> - cpu_to_be32(0x0),
> - cpu_to_be32(0x0),
> - cpu_to_be32(0x0),
> - cpu_to_be32(cpu->node_id),
> - cpu_to_be32(index)};
> -
> - /* Advertise NUMA via ibm,associativity */
> - return fdt_setprop(fdt, offset, "ibm,associativity", associativity,
> - sizeof(associativity));
> -}
> -
> static void spapr_dt_pa_features(SpaprMachineState *spapr,
> PowerPCCPU *cpu,
> void *fdt, int offset)
> @@ -785,7 +770,7 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int offset,
> pft_size_prop, sizeof(pft_size_prop))));
>
> if (ms->numa_state->num_nodes > 1) {
> - _FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cpu));
> + _FDT(spapr_numa_fixup_cpu_dt(spapr, fdt, offset, cpu));
> }
>
> _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index f6b6fe648f..1a1ec8bcff 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -45,6 +45,33 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
> sizeof(spapr->numa_assoc_array[nodeid]))));
> }
>
> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> + int offset, PowerPCCPU *cpu)
> +{
> + uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
> + uint32_t vcpu_assoc[vcpu_assoc_size];
> + int index = spapr_get_vcpu_id(cpu);
> + int i;
> +
> + /*
> + * VCPUs have an extra 'cpu_id' value in ibm,associativity
> + * compared to other resources. Increment the size at index
> + * 0, copy all associativity domains already set, then put
> + * cpu_id last.
> + */
> + vcpu_assoc[0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS + 1);
> +
> + for (i = 1; i <= MAX_DISTANCE_REF_POINTS; i++) {
> + vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
> + }
You could use a single memcpy() here as well.
> +
> + vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
> +
> + /* Advertise NUMA via ibm,associativity */
> + return fdt_setprop(fdt, offset, "ibm,associativity",
> + vcpu_assoc, sizeof(vcpu_assoc));
> +}
> +
> /*
> * Helper that writes ibm,associativity-reference-points and
> * max-associativity-domains in the RTAS pointed by @rtas
> diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
> index a2a4df55f7..43c6a16fe3 100644
> --- a/include/hw/ppc/spapr_numa.h
> +++ b/include/hw/ppc/spapr_numa.h
> @@ -27,5 +27,7 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr,
> void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
> void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
> int offset, int nodeid);
> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> + int offset, PowerPCCPU *cpu);
>
> #endif /* HW_SPAPR_NUMA_H */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 2/7] spapr, spapr_numa: handle vcpu ibm,associativity
2020-09-03 23:23 ` David Gibson
@ 2020-09-04 0:32 ` Daniel Henrique Barboza
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-04 0:32 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 9/3/20 8:23 PM, David Gibson wrote:
> On Thu, Sep 03, 2020 at 07:06:34PM -0300, Daniel Henrique Barboza wrote:
>> Vcpus have an additional paramenter to be appended, vcpu_id. This
>> also changes the size of the of property itself, which is being
>> represented in index 0 of numa_assoc_array[cpu->node_id],
>> and defaults to MAX_DISTANCE_REF_POINTS for all cases but
>> vcpus.
>>
>> All this logic makes more sense in spapr_numa.c, where we handle
>> everything NUMA and associativity. A new helper spapr_numa_fixup_cpu_dt()
>> was added, and spapr.c uses it the same way as it was using the former
>> spapr_fixup_cpu_numa_dt().
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>> hw/ppc/spapr.c | 17 +----------------
>> hw/ppc/spapr_numa.c | 27 +++++++++++++++++++++++++++
>> include/hw/ppc/spapr_numa.h | 2 ++
>> 3 files changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 1ad6f59863..badfa86319 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -202,21 +202,6 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>> return ret;
>> }
>>
>> -static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
>> -{
>> - int index = spapr_get_vcpu_id(cpu);
>> - uint32_t associativity[] = {cpu_to_be32(0x5),
>> - cpu_to_be32(0x0),
>> - cpu_to_be32(0x0),
>> - cpu_to_be32(0x0),
>> - cpu_to_be32(cpu->node_id),
>> - cpu_to_be32(index)};
>> -
>> - /* Advertise NUMA via ibm,associativity */
>> - return fdt_setprop(fdt, offset, "ibm,associativity", associativity,
>> - sizeof(associativity));
>> -}
>> -
>> static void spapr_dt_pa_features(SpaprMachineState *spapr,
>> PowerPCCPU *cpu,
>> void *fdt, int offset)
>> @@ -785,7 +770,7 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int offset,
>> pft_size_prop, sizeof(pft_size_prop))));
>>
>> if (ms->numa_state->num_nodes > 1) {
>> - _FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cpu));
>> + _FDT(spapr_numa_fixup_cpu_dt(spapr, fdt, offset, cpu));
>> }
>>
>> _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
>> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
>> index f6b6fe648f..1a1ec8bcff 100644
>> --- a/hw/ppc/spapr_numa.c
>> +++ b/hw/ppc/spapr_numa.c
>> @@ -45,6 +45,33 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
>> sizeof(spapr->numa_assoc_array[nodeid]))));
>> }
>>
>> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
>> + int offset, PowerPCCPU *cpu)
>> +{
>> + uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
>> + uint32_t vcpu_assoc[vcpu_assoc_size];
>> + int index = spapr_get_vcpu_id(cpu);
>> + int i;
>> +
>> + /*
>> + * VCPUs have an extra 'cpu_id' value in ibm,associativity
>> + * compared to other resources. Increment the size at index
>> + * 0, copy all associativity domains already set, then put
>> + * cpu_id last.
>> + */
>> + vcpu_assoc[0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS + 1);
>> +
>> + for (i = 1; i <= MAX_DISTANCE_REF_POINTS; i++) {
>> + vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
>> + }
>
> You could use a single memcpy() here as well.
I'm moving this code to a new function in patch 06. I'll change this for memcpy()
there for v4.
Thanks,
DHB
>
>> +
>> + vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
>> +
>> + /* Advertise NUMA via ibm,associativity */
>> + return fdt_setprop(fdt, offset, "ibm,associativity",
>> + vcpu_assoc, sizeof(vcpu_assoc));
>> +}
>> +
>> /*
>> * Helper that writes ibm,associativity-reference-points and
>> * max-associativity-domains in the RTAS pointed by @rtas
>> diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
>> index a2a4df55f7..43c6a16fe3 100644
>> --- a/include/hw/ppc/spapr_numa.h
>> +++ b/include/hw/ppc/spapr_numa.h
>> @@ -27,5 +27,7 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr,
>> void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
>> void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
>> int offset, int nodeid);
>> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
>> + int offset, PowerPCCPU *cpu);
>>
>> #endif /* HW_SPAPR_NUMA_H */
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/7] spapr, spapr_numa: move lookup-arrays handling to spapr_numa.c
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
2020-09-03 22:06 ` [PATCH v3 1/7] spapr: introduce SpaprMachineState::numa_assoc_array Daniel Henrique Barboza
2020-09-03 22:06 ` [PATCH v3 2/7] spapr, spapr_numa: handle vcpu ibm,associativity Daniel Henrique Barboza
@ 2020-09-03 22:06 ` Daniel Henrique Barboza
2020-09-03 22:06 ` [PATCH v3 4/7] spapr_numa: move NVLink2 associativity " Daniel Henrique Barboza
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-03 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, david
In a similar fashion as the previous patch, let's move the
handling of ibm,associativity-lookup-arrays from spapr.c to
spapr_numa.c. A spapr_numa_write_assoc_lookup_arrays() helper was
created, and spapr_dt_dynamic_reconfiguration_memory() can now
use it to advertise the lookup-arrays.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr.c | 25 ++-----------------------
hw/ppc/spapr_numa.c | 34 ++++++++++++++++++++++++++++++++++
include/hw/ppc/spapr_numa.h | 2 ++
3 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index badfa86319..9bce1892b5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -535,13 +535,10 @@ static int spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,
void *fdt)
{
MachineState *machine = MACHINE(spapr);
- int nb_numa_nodes = machine->numa_state->num_nodes;
- int ret, i, offset;
+ int ret, offset;
uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
uint32_t prop_lmb_size[] = {cpu_to_be32(lmb_size >> 32),
cpu_to_be32(lmb_size & 0xffffffff)};
- uint32_t *int_buf, *cur_index, buf_len;
- int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
MemoryDeviceInfoList *dimms = NULL;
/*
@@ -582,25 +579,7 @@ static int spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,
return ret;
}
- /* ibm,associativity-lookup-arrays */
- buf_len = (nr_nodes * 4 + 2) * sizeof(uint32_t);
- cur_index = int_buf = g_malloc0(buf_len);
- int_buf[0] = cpu_to_be32(nr_nodes);
- int_buf[1] = cpu_to_be32(4); /* Number of entries per associativity list */
- cur_index += 2;
- for (i = 0; i < nr_nodes; i++) {
- uint32_t associativity[] = {
- cpu_to_be32(0x0),
- cpu_to_be32(0x0),
- cpu_to_be32(0x0),
- cpu_to_be32(i)
- };
- memcpy(cur_index, associativity, sizeof(associativity));
- cur_index += 4;
- }
- ret = fdt_setprop(fdt, offset, "ibm,associativity-lookup-arrays", int_buf,
- (cur_index - int_buf) * sizeof(uint32_t));
- g_free(int_buf);
+ ret = spapr_numa_write_assoc_lookup_arrays(spapr, fdt, offset);
return ret;
}
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index 1a1ec8bcff..5a82a84438 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -72,6 +72,40 @@ int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
vcpu_assoc, sizeof(vcpu_assoc));
}
+
+int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt,
+ int offset)
+{
+ MachineState *machine = MACHINE(spapr);
+ int nb_numa_nodes = machine->numa_state->num_nodes;
+ int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
+ uint32_t *int_buf, *cur_index, buf_len;
+ int ret, i;
+
+ /* ibm,associativity-lookup-arrays */
+ buf_len = (nr_nodes * MAX_DISTANCE_REF_POINTS + 2) * sizeof(uint32_t);
+ cur_index = int_buf = g_malloc0(buf_len);
+ int_buf[0] = cpu_to_be32(nr_nodes);
+ /* Number of entries per associativity list */
+ int_buf[1] = cpu_to_be32(MAX_DISTANCE_REF_POINTS);
+ cur_index += 2;
+ for (i = 0; i < nr_nodes; i++) {
+ /*
+ * For the lookup-array we use the ibm,associativity array,
+ * from numa_assoc_array. without the first element (size).
+ */
+ uint32_t *associativity = spapr->numa_assoc_array[i];
+ memcpy(cur_index, ++associativity,
+ sizeof(uint32_t) * MAX_DISTANCE_REF_POINTS);
+ cur_index += MAX_DISTANCE_REF_POINTS;
+ }
+ ret = fdt_setprop(fdt, offset, "ibm,associativity-lookup-arrays", int_buf,
+ (cur_index - int_buf) * sizeof(uint32_t));
+ g_free(int_buf);
+
+ return ret;
+}
+
/*
* Helper that writes ibm,associativity-reference-points and
* max-associativity-domains in the RTAS pointed by @rtas
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index 43c6a16fe3..b3fd950634 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -29,5 +29,7 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
int offset, int nodeid);
int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
int offset, PowerPCCPU *cpu);
+int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt,
+ int offset);
#endif /* HW_SPAPR_NUMA_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 4/7] spapr_numa: move NVLink2 associativity handling to spapr_numa.c
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
` (2 preceding siblings ...)
2020-09-03 22:06 ` [PATCH v3 3/7] spapr, spapr_numa: move lookup-arrays handling to spapr_numa.c Daniel Henrique Barboza
@ 2020-09-03 22:06 ` Daniel Henrique Barboza
2020-09-03 22:06 ` [PATCH v3 5/7] spapr: move h_home_node_associativity " Daniel Henrique Barboza
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-03 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, david
The NVLink2 GPUs works like a regular NUMA node with its
own associativity values, regardless of user input.
This can be handled inside spapr_numa_associativity_init(),
initializing NVGPU_MAX_NUM associativity arrays that can
be used by the GPUs.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr_numa.c | 28 +++++++++++++++++++++++++++-
hw/ppc/spapr_pci_nvlink2.c | 20 +++-----------------
2 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index 5a82a84438..93a000b729 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -13,14 +13,18 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "hw/ppc/spapr_numa.h"
+#include "hw/pci-host/spapr.h"
#include "hw/ppc/fdt.h"
+/* Moved from hw/ppc/spapr_pci_nvlink2.c */
+#define SPAPR_GPU_NUMA_ID (cpu_to_be32(1))
void spapr_numa_associativity_init(SpaprMachineState *spapr,
MachineState *machine)
{
+ SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
int nb_numa_nodes = machine->numa_state->num_nodes;
- int i;
+ int i, j, max_nodes_with_gpus;
/*
* For all associativity arrays: first position is the size,
@@ -35,6 +39,28 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr,
spapr->numa_assoc_array[i][0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS);
spapr->numa_assoc_array[i][MAX_DISTANCE_REF_POINTS] = cpu_to_be32(i);
}
+
+ /*
+ * Initialize NVLink GPU associativity arrays. We know that
+ * the first GPU will take the first available NUMA id, and
+ * we'll have a maximum of NVGPU_MAX_NUM GPUs in the machine.
+ * At this point we're not sure if there are GPUs or not, but
+ * let's initialize the associativity arrays and allow NVLink
+ * GPUs to be handled like regular NUMA nodes later on.
+ */
+ max_nodes_with_gpus = nb_numa_nodes + NVGPU_MAX_NUM;
+
+ for (i = nb_numa_nodes; i < max_nodes_with_gpus; i++) {
+ spapr->numa_assoc_array[i][0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS);
+
+ for (j = 1; j < MAX_DISTANCE_REF_POINTS; j++) {
+ uint32_t gpu_assoc = smc->pre_5_1_assoc_refpoints ?
+ SPAPR_GPU_NUMA_ID : cpu_to_be32(i);
+ spapr->numa_assoc_array[i][j] = gpu_assoc;
+ }
+
+ spapr->numa_assoc_array[i][MAX_DISTANCE_REF_POINTS] = cpu_to_be32(i);
+ }
}
void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c
index 76ae77ebc8..8ef9b40a18 100644
--- a/hw/ppc/spapr_pci_nvlink2.c
+++ b/hw/ppc/spapr_pci_nvlink2.c
@@ -26,6 +26,7 @@
#include "qemu-common.h"
#include "hw/pci/pci.h"
#include "hw/pci-host/spapr.h"
+#include "hw/ppc/spapr_numa.h"
#include "qemu/error-report.h"
#include "hw/ppc/fdt.h"
#include "hw/pci/pci_bridge.h"
@@ -37,8 +38,6 @@
#define PHANDLE_NVLINK(phb, gn, nn) (0x00130000 | (((phb)->index) << 8) | \
((gn) << 4) | (nn))
-#define SPAPR_GPU_NUMA_ID (cpu_to_be32(1))
-
typedef struct SpaprPhbPciNvGpuSlot {
uint64_t tgt;
uint64_t gpa;
@@ -360,13 +359,6 @@ void spapr_phb_nvgpu_ram_populate_dt(SpaprPhbState *sphb, void *fdt)
Object *nv_mrobj = object_property_get_link(OBJECT(nvslot->gpdev),
"nvlink2-mr[0]",
&error_abort);
- uint32_t associativity[] = {
- cpu_to_be32(0x4),
- cpu_to_be32(nvslot->numa_id),
- cpu_to_be32(nvslot->numa_id),
- cpu_to_be32(nvslot->numa_id),
- cpu_to_be32(nvslot->numa_id)
- };
uint64_t size = object_property_get_uint(nv_mrobj, "size", NULL);
uint64_t mem_reg[2] = { cpu_to_be64(nvslot->gpa), cpu_to_be64(size) };
char *mem_name = g_strdup_printf("memory@%"PRIx64, nvslot->gpa);
@@ -376,14 +368,8 @@ void spapr_phb_nvgpu_ram_populate_dt(SpaprPhbState *sphb, void *fdt)
_FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
_FDT((fdt_setprop(fdt, off, "reg", mem_reg, sizeof(mem_reg))));
- if (sphb->pre_5_1_assoc) {
- associativity[1] = SPAPR_GPU_NUMA_ID;
- associativity[2] = SPAPR_GPU_NUMA_ID;
- associativity[3] = SPAPR_GPU_NUMA_ID;
- }
-
- _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
- sizeof(associativity))));
+ spapr_numa_write_associativity_dt(SPAPR_MACHINE(qdev_get_machine()),
+ fdt, off, nvslot->numa_id);
_FDT((fdt_setprop_string(fdt, off, "compatible",
"ibm,coherent-device-memory")));
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 5/7] spapr: move h_home_node_associativity to spapr_numa.c
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
` (3 preceding siblings ...)
2020-09-03 22:06 ` [PATCH v3 4/7] spapr_numa: move NVLink2 associativity " Daniel Henrique Barboza
@ 2020-09-03 22:06 ` Daniel Henrique Barboza
2020-09-03 23:25 ` David Gibson
2020-09-03 22:06 ` [PATCH v3 6/7] spapr_numa: create a vcpu associativity helper Daniel Henrique Barboza
` (2 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-03 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, david
The implementation of this hypercall will be modified to use
spapr->numa_assoc_arrays input. Moving it to spapr_numa.c makes
make more sense.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr_hcall.c | 37 +------------------------------------
hw/ppc/spapr_numa.c | 36 ++++++++++++++++++++++++++++++++++++
include/hw/ppc/spapr_numa.h | 4 ++++
3 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index c1d01228c6..9e9b959bbd 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -18,6 +18,7 @@
#include "kvm_ppc.h"
#include "hw/ppc/fdt.h"
#include "hw/ppc/spapr_ovec.h"
+#include "hw/ppc/spapr_numa.h"
#include "mmu-book3s-v3.h"
#include "hw/mem/memory-device.h"
@@ -1873,42 +1874,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
return ret;
}
-static target_ulong h_home_node_associativity(PowerPCCPU *cpu,
- SpaprMachineState *spapr,
- target_ulong opcode,
- target_ulong *args)
-{
- target_ulong flags = args[0];
- target_ulong procno = args[1];
- PowerPCCPU *tcpu;
- int idx;
-
- /* only support procno from H_REGISTER_VPA */
- if (flags != 0x1) {
- return H_FUNCTION;
- }
-
- tcpu = spapr_find_cpu(procno);
- if (tcpu == NULL) {
- return H_P2;
- }
-
- /* sequence is the same as in the "ibm,associativity" property */
-
- idx = 0;
-#define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
- ((uint64_t)(b) & 0xffffffff))
- args[idx++] = ASSOCIATIVITY(0, 0);
- args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
- args[idx++] = ASSOCIATIVITY(procno, -1);
- for ( ; idx < 6; idx++) {
- args[idx] = -1;
- }
-#undef ASSOCIATIVITY
-
- return H_SUCCESS;
-}
-
static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
SpaprMachineState *spapr,
target_ulong opcode,
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index 93a000b729..d4dca57321 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -165,3 +165,39 @@ void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas)
_FDT(fdt_setprop(fdt, rtas, "ibm,max-associativity-domains",
maxdomains, sizeof(maxdomains)));
}
+
+target_ulong h_home_node_associativity(PowerPCCPU *cpu,
+ SpaprMachineState *spapr,
+ target_ulong opcode,
+ target_ulong *args)
+{
+ target_ulong flags = args[0];
+ target_ulong procno = args[1];
+ PowerPCCPU *tcpu;
+ int idx;
+
+ /* only support procno from H_REGISTER_VPA */
+ if (flags != 0x1) {
+ return H_FUNCTION;
+ }
+
+ tcpu = spapr_find_cpu(procno);
+ if (tcpu == NULL) {
+ return H_P2;
+ }
+
+ /* sequence is the same as in the "ibm,associativity" property */
+
+ idx = 0;
+#define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
+ ((uint64_t)(b) & 0xffffffff))
+ args[idx++] = ASSOCIATIVITY(0, 0);
+ args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
+ args[idx++] = ASSOCIATIVITY(procno, -1);
+ for ( ; idx < 6; idx++) {
+ args[idx] = -1;
+ }
+#undef ASSOCIATIVITY
+
+ return H_SUCCESS;
+}
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index b3fd950634..5b4d165c06 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -31,5 +31,9 @@ int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
int offset, PowerPCCPU *cpu);
int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt,
int offset);
+target_ulong h_home_node_associativity(PowerPCCPU *cpu,
+ SpaprMachineState *spapr,
+ target_ulong opcode,
+ target_ulong *args);
#endif /* HW_SPAPR_NUMA_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 5/7] spapr: move h_home_node_associativity to spapr_numa.c
2020-09-03 22:06 ` [PATCH v3 5/7] spapr: move h_home_node_associativity " Daniel Henrique Barboza
@ 2020-09-03 23:25 ` David Gibson
0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2020-09-03 23:25 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5085 bytes --]
On Thu, Sep 03, 2020 at 07:06:37PM -0300, Daniel Henrique Barboza wrote:
> The implementation of this hypercall will be modified to use
> spapr->numa_assoc_arrays input. Moving it to spapr_numa.c makes
> make more sense.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/spapr_hcall.c | 37 +------------------------------------
> hw/ppc/spapr_numa.c | 36 ++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr_numa.h | 4 ++++
> 3 files changed, 41 insertions(+), 36 deletions(-)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index c1d01228c6..9e9b959bbd 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -18,6 +18,7 @@
> #include "kvm_ppc.h"
> #include "hw/ppc/fdt.h"
> #include "hw/ppc/spapr_ovec.h"
> +#include "hw/ppc/spapr_numa.h"
> #include "mmu-book3s-v3.h"
> #include "hw/mem/memory-device.h"
>
> @@ -1873,42 +1874,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> return ret;
> }
>
> -static target_ulong h_home_node_associativity(PowerPCCPU *cpu,
> - SpaprMachineState *spapr,
> - target_ulong opcode,
> - target_ulong *args)
> -{
> - target_ulong flags = args[0];
> - target_ulong procno = args[1];
> - PowerPCCPU *tcpu;
> - int idx;
> -
> - /* only support procno from H_REGISTER_VPA */
> - if (flags != 0x1) {
> - return H_FUNCTION;
> - }
> -
> - tcpu = spapr_find_cpu(procno);
> - if (tcpu == NULL) {
> - return H_P2;
> - }
> -
> - /* sequence is the same as in the "ibm,associativity" property */
> -
> - idx = 0;
> -#define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
> - ((uint64_t)(b) & 0xffffffff))
> - args[idx++] = ASSOCIATIVITY(0, 0);
> - args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
> - args[idx++] = ASSOCIATIVITY(procno, -1);
> - for ( ; idx < 6; idx++) {
> - args[idx] = -1;
> - }
> -#undef ASSOCIATIVITY
> -
> - return H_SUCCESS;
> -}
> -
> static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
> SpaprMachineState *spapr,
> target_ulong opcode,
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index 93a000b729..d4dca57321 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -165,3 +165,39 @@ void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas)
> _FDT(fdt_setprop(fdt, rtas, "ibm,max-associativity-domains",
> maxdomains, sizeof(maxdomains)));
> }
> +
> +target_ulong h_home_node_associativity(PowerPCCPU *cpu,
> + SpaprMachineState *spapr,
> + target_ulong opcode,
> + target_ulong *args)
So, if you're moving this function to this file, you should also move
the corresponding spapr_register_hypercall() to this file. That will
let you keep this function static.
> +{
> + target_ulong flags = args[0];
> + target_ulong procno = args[1];
> + PowerPCCPU *tcpu;
> + int idx;
> +
> + /* only support procno from H_REGISTER_VPA */
> + if (flags != 0x1) {
> + return H_FUNCTION;
> + }
> +
> + tcpu = spapr_find_cpu(procno);
> + if (tcpu == NULL) {
> + return H_P2;
> + }
> +
> + /* sequence is the same as in the "ibm,associativity" property */
> +
> + idx = 0;
> +#define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
> + ((uint64_t)(b) & 0xffffffff))
> + args[idx++] = ASSOCIATIVITY(0, 0);
> + args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
> + args[idx++] = ASSOCIATIVITY(procno, -1);
> + for ( ; idx < 6; idx++) {
> + args[idx] = -1;
> + }
> +#undef ASSOCIATIVITY
> +
> + return H_SUCCESS;
> +}
> diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
> index b3fd950634..5b4d165c06 100644
> --- a/include/hw/ppc/spapr_numa.h
> +++ b/include/hw/ppc/spapr_numa.h
> @@ -31,5 +31,9 @@ int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> int offset, PowerPCCPU *cpu);
> int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt,
> int offset);
> +target_ulong h_home_node_associativity(PowerPCCPU *cpu,
> + SpaprMachineState *spapr,
> + target_ulong opcode,
> + target_ulong *args);
>
> #endif /* HW_SPAPR_NUMA_H */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 6/7] spapr_numa: create a vcpu associativity helper
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
` (4 preceding siblings ...)
2020-09-03 22:06 ` [PATCH v3 5/7] spapr: move h_home_node_associativity " Daniel Henrique Barboza
@ 2020-09-03 22:06 ` Daniel Henrique Barboza
2020-09-03 23:27 ` David Gibson
2020-09-03 22:06 ` [PATCH v3 7/7] spapr_numa: use spapr_numa_get_vcpu_assoc() in home_node hcall Daniel Henrique Barboza
2020-09-03 23:23 ` [PATCH v3 0/7] pseries NUMA distance rework David Gibson
7 siblings, 1 reply; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-03 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, david
The work to be done in h_home_node_associativity() intersects
with what is already done in spapr_numa_fixup_cpu_dt(). This
patch creates a new helper, spapr_numa_get_vcpu_assoc(), to
be used for both spapr_numa_fixup_cpu_dt() and
h_home_node_associativity().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr_numa.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index d4dca57321..abc7361921 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -71,14 +71,17 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
sizeof(spapr->numa_assoc_array[nodeid]))));
}
-int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
- int offset, PowerPCCPU *cpu)
+static uint32_t *spapr_numa_get_vcpu_assoc(SpaprMachineState *spapr,
+ PowerPCCPU *cpu,
+ uint *vcpu_assoc_size)
{
- uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
- uint32_t vcpu_assoc[vcpu_assoc_size];
+ uint32_t *vcpu_assoc = NULL;
int index = spapr_get_vcpu_id(cpu);
int i;
+ *vcpu_assoc_size = (NUMA_ASSOC_SIZE + 1) * sizeof(uint32_t);
+ vcpu_assoc = g_malloc(*vcpu_assoc_size);
+
/*
* VCPUs have an extra 'cpu_id' value in ibm,associativity
* compared to other resources. Increment the size at index
@@ -91,11 +94,22 @@ int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
}
- vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
+ vcpu_assoc[MAX_DISTANCE_REF_POINTS + 1] = cpu_to_be32(index);
+
+ return vcpu_assoc;
+}
+
+int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
+ int offset, PowerPCCPU *cpu)
+{
+ g_autofree uint32_t *vcpu_assoc = NULL;
+ uint vcpu_assoc_size;
+
+ vcpu_assoc = spapr_numa_get_vcpu_assoc(spapr, cpu, &vcpu_assoc_size);
/* Advertise NUMA via ibm,associativity */
return fdt_setprop(fdt, offset, "ibm,associativity",
- vcpu_assoc, sizeof(vcpu_assoc));
+ vcpu_assoc, vcpu_assoc_size);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 6/7] spapr_numa: create a vcpu associativity helper
2020-09-03 22:06 ` [PATCH v3 6/7] spapr_numa: create a vcpu associativity helper Daniel Henrique Barboza
@ 2020-09-03 23:27 ` David Gibson
0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2020-09-03 23:27 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2834 bytes --]
On Thu, Sep 03, 2020 at 07:06:38PM -0300, Daniel Henrique Barboza wrote:
> The work to be done in h_home_node_associativity() intersects
> with what is already done in spapr_numa_fixup_cpu_dt(). This
> patch creates a new helper, spapr_numa_get_vcpu_assoc(), to
> be used for both spapr_numa_fixup_cpu_dt() and
> h_home_node_associativity().
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr_numa.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index d4dca57321..abc7361921 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -71,14 +71,17 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
> sizeof(spapr->numa_assoc_array[nodeid]))));
> }
>
> -int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> - int offset, PowerPCCPU *cpu)
> +static uint32_t *spapr_numa_get_vcpu_assoc(SpaprMachineState *spapr,
> + PowerPCCPU *cpu,
> + uint *vcpu_assoc_size)
> {
> - uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
> - uint32_t vcpu_assoc[vcpu_assoc_size];
> + uint32_t *vcpu_assoc = NULL;
> int index = spapr_get_vcpu_id(cpu);
> int i;
>
> + *vcpu_assoc_size = (NUMA_ASSOC_SIZE + 1) * sizeof(uint32_t);
> + vcpu_assoc = g_malloc(*vcpu_assoc_size);
> +
> /*
> * VCPUs have an extra 'cpu_id' value in ibm,associativity
> * compared to other resources. Increment the size at index
> @@ -91,11 +94,22 @@ int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
> }
>
> - vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
> + vcpu_assoc[MAX_DISTANCE_REF_POINTS + 1] = cpu_to_be32(index);
> +
> + return vcpu_assoc;
> +}
> +
> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> + int offset, PowerPCCPU *cpu)
> +{
> + g_autofree uint32_t *vcpu_assoc = NULL;
> + uint vcpu_assoc_size;
> +
> + vcpu_assoc = spapr_numa_get_vcpu_assoc(spapr, cpu, &vcpu_assoc_size);
>
> /* Advertise NUMA via ibm,associativity */
> return fdt_setprop(fdt, offset, "ibm,associativity",
> - vcpu_assoc, sizeof(vcpu_assoc));
> + vcpu_assoc, vcpu_assoc_size);
> }
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 7/7] spapr_numa: use spapr_numa_get_vcpu_assoc() in home_node hcall
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
` (5 preceding siblings ...)
2020-09-03 22:06 ` [PATCH v3 6/7] spapr_numa: create a vcpu associativity helper Daniel Henrique Barboza
@ 2020-09-03 22:06 ` Daniel Henrique Barboza
2020-09-03 23:31 ` David Gibson
2020-09-03 23:23 ` [PATCH v3 0/7] pseries NUMA distance rework David Gibson
7 siblings, 1 reply; 14+ messages in thread
From: Daniel Henrique Barboza @ 2020-09-03 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, david
The current implementation of h_home_node_associativity hard codes
the values of associativity domains of the vcpus. Let's make
it consider the values already initialized in spapr->numa_assoc_array,
via the spapr_numa_get_vcpu_assoc() helper.
We want to set it and forget it, and for that we also need to
assert that we don't overflow the registers of the hypercall.
From R4 to R9 we can squeeze in 12 associativity domains, so
let's assert that MAX_DISTANCE_REF_POINTS isn't greater
than that.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/spapr_numa.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index abc7361921..850e61bf98 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -185,10 +185,12 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
target_ulong opcode,
target_ulong *args)
{
+ g_autofree uint32_t *vcpu_assoc = NULL;
target_ulong flags = args[0];
target_ulong procno = args[1];
PowerPCCPU *tcpu;
- int idx;
+ uint vcpu_assoc_size;
+ int idx, assoc_idx;
/* only support procno from H_REGISTER_VPA */
if (flags != 0x1) {
@@ -200,16 +202,31 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
return H_P2;
}
- /* sequence is the same as in the "ibm,associativity" property */
+ /*
+ * Given that we want to be flexible with the sizes and indexes,
+ * we must consider that there is a hard limit of how many
+ * associativities domain we can fit in R4 up to o R9, which
+ * would be 12. Assert and bail if that's not the case.
+ */
+ g_assert(MAX_DISTANCE_REF_POINTS <= 12);
+
+ vcpu_assoc = spapr_numa_get_vcpu_assoc(spapr, tcpu, &vcpu_assoc_size);
+ vcpu_assoc_size /= sizeof(uint32_t);
+ /* assoc_idx starts at 1 to skip associativity size */
+ assoc_idx = 1;
- idx = 0;
#define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
((uint64_t)(b) & 0xffffffff))
- args[idx++] = ASSOCIATIVITY(0, 0);
- args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
- args[idx++] = ASSOCIATIVITY(procno, -1);
- for ( ; idx < 6; idx++) {
- args[idx] = -1;
+
+ for (idx = 0; idx < 6; idx++) {
+ int8_t a, b;
+
+ a = assoc_idx < vcpu_assoc_size ?
+ be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
+ b = assoc_idx < vcpu_assoc_size ?
+ be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
+
+ args[idx] = ASSOCIATIVITY(a, b);
}
#undef ASSOCIATIVITY
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 7/7] spapr_numa: use spapr_numa_get_vcpu_assoc() in home_node hcall
2020-09-03 22:06 ` [PATCH v3 7/7] spapr_numa: use spapr_numa_get_vcpu_assoc() in home_node hcall Daniel Henrique Barboza
@ 2020-09-03 23:31 ` David Gibson
0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2020-09-03 23:31 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3424 bytes --]
On Thu, Sep 03, 2020 at 07:06:39PM -0300, Daniel Henrique Barboza wrote:
> The current implementation of h_home_node_associativity hard codes
> the values of associativity domains of the vcpus. Let's make
> it consider the values already initialized in spapr->numa_assoc_array,
> via the spapr_numa_get_vcpu_assoc() helper.
>
> We want to set it and forget it, and for that we also need to
> assert that we don't overflow the registers of the hypercall.
> >From R4 to R9 we can squeeze in 12 associativity domains, so
> let's assert that MAX_DISTANCE_REF_POINTS isn't greater
> than that.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/spapr_numa.c | 33 +++++++++++++++++++++++++--------
> 1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index abc7361921..850e61bf98 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -185,10 +185,12 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
> target_ulong opcode,
> target_ulong *args)
> {
> + g_autofree uint32_t *vcpu_assoc = NULL;
> target_ulong flags = args[0];
> target_ulong procno = args[1];
> PowerPCCPU *tcpu;
> - int idx;
> + uint vcpu_assoc_size;
> + int idx, assoc_idx;
>
> /* only support procno from H_REGISTER_VPA */
> if (flags != 0x1) {
> @@ -200,16 +202,31 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
> return H_P2;
> }
>
> - /* sequence is the same as in the "ibm,associativity" property */
> + /*
> + * Given that we want to be flexible with the sizes and indexes,
> + * we must consider that there is a hard limit of how many
> + * associativities domain we can fit in R4 up to o R9, which
typo ............................................... ^
> + * would be 12. Assert and bail if that's not the case.
> + */
> + g_assert(MAX_DISTANCE_REF_POINTS <= 12);
Since MAX_DISTANCE_REF_POINTS is a compile time constant, you could
use G_STATIC_ASSERT() to make this a compile rather than runtime
error.
> +
> + vcpu_assoc = spapr_numa_get_vcpu_assoc(spapr, tcpu, &vcpu_assoc_size);
> + vcpu_assoc_size /= sizeof(uint32_t);
> + /* assoc_idx starts at 1 to skip associativity size */
> + assoc_idx = 1;
>
> - idx = 0;
> #define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
> ((uint64_t)(b) & 0xffffffff))
> - args[idx++] = ASSOCIATIVITY(0, 0);
> - args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
> - args[idx++] = ASSOCIATIVITY(procno, -1);
> - for ( ; idx < 6; idx++) {
> - args[idx] = -1;
> +
> + for (idx = 0; idx < 6; idx++) {
> + int8_t a, b;
Do you really want int8_t, rather than say int32_t?
> +
> + a = assoc_idx < vcpu_assoc_size ?
> + be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
> + b = assoc_idx < vcpu_assoc_size ?
> + be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
> +
> + args[idx] = ASSOCIATIVITY(a, b);
> }
> #undef ASSOCIATIVITY
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/7] pseries NUMA distance rework
2020-09-03 22:06 [PATCH v3 0/7] pseries NUMA distance rework Daniel Henrique Barboza
` (6 preceding siblings ...)
2020-09-03 22:06 ` [PATCH v3 7/7] spapr_numa: use spapr_numa_get_vcpu_assoc() in home_node hcall Daniel Henrique Barboza
@ 2020-09-03 23:23 ` David Gibson
7 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2020-09-03 23:23 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2479 bytes --]
On Thu, Sep 03, 2020 at 07:06:32PM -0300, Daniel Henrique Barboza wrote:
> Hi,
>
> This spin attempts to cover all suggestions and concerns pointed
> out by David Gibson in the v2 review.
>
> The patches were rebased with David's ppc-for-5.2 at 615ae3763144.
> They can also be cloned from
> https://github.com/danielhb/qemu/tree/spapr_numa_v3.
>
> Changes from v2:
> - patches 1 and 2 from v2 -> already pushed to ppc-for-5.2
> - patch 1 (former 3):
> * numa_assoc_array moved to SpaprMachineState
> - patch 3 (former 5):
> * use memcpy
> * fix index increment to use MAX_DISTANCE_REF_POINTS
> - patch 4 (former 6):
> * revamped. NVLink2 associativity is now calculated in
> spapr_numa_associativity_init(). GPU code will use the same
> helper everyone but vcpus uses to write the associativity DT
> - patch 5 - new
> - patch 6 - new
> - patch 7:
> * no more brazilian portuguese notes in the commit message
> * change the code to handle an arbitrary vcpu associativity
> array, retrieved with a new helper added by patch 6.
>
> v2 link:
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00261.html
I've applied 1..4, though I will comment on some nits that could be
cleaned up later on. I have some bigger comments on the remainder.
>
>
> Daniel Henrique Barboza (7):
> spapr: introduce SpaprMachineState::numa_assoc_array
> spapr, spapr_numa: handle vcpu ibm,associativity
> spapr, spapr_numa: move lookup-arrays handling to spapr_numa.c
> spapr_numa: move NVLink2 associativity handling to spapr_numa.c
> spapr: move h_home_node_associativity to spapr_numa.c
> spapr_numa: create a vcpu associativity helper
> spapr_numa: use spapr_numa_get_vcpu_assoc() in home_node hcall
>
> hw/ppc/spapr.c | 65 +++---------
> hw/ppc/spapr_hcall.c | 37 +------
> hw/ppc/spapr_numa.c | 184 ++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_nvdimm.c | 19 ++--
> hw/ppc/spapr_pci.c | 9 +-
> hw/ppc/spapr_pci_nvlink2.c | 20 +---
> include/hw/ppc/spapr.h | 12 +++
> include/hw/ppc/spapr_numa.h | 19 ++++
> include/hw/ppc/spapr_nvdimm.h | 2 +-
> 9 files changed, 242 insertions(+), 125 deletions(-)
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread