Re: [PATCH 07/10] spapr: create helper to set ibm,associativity

[Daniel Henrique Barboza]

On 8/20/20 12:00 AM, David Gibson wrote:

On Fri, Aug 14, 2020 at 05:54:21PM -0300, Daniel Henrique Barboza wrote:

We have several places around hw/ppc files where we use the
same code to set the ibm,associativity array. This patch
creates a helper called spapr_set_associativity() to do
that in a single place. It'll also make it saner to change
the value of ibm,associativity in the next patches.

After this patch, only 2 places are left with open code
ibm,associativity assignment:

- spapr_dt_dynamic_reconfiguration_memory()
- h_home_node_associativity() in spapr_hcall.c

The update of associativity values will be made in these places
manually later on.

Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>

Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>

I like this - any chance you could move this to the front of the
series so that we can make this code easier to follow while we're
still discussing the more meaningful changes?

No problem. I'll move this up front in v2.


DHB

---
  hw/ppc/spapr.c         | 32 +++++++++++++++++++++-----------
  hw/ppc/spapr_nvdimm.c  |  8 +++-----
  hw/ppc/spapr_pci.c     |  8 +++-----
  include/hw/ppc/spapr.h |  1 +
  4 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bc51d2db90..b80a6f6936 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -201,15 +201,27 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, 
PowerPCCPU *cpu,
      return ret;
  }

+void spapr_set_associativity(uint32_t *assoc, int node_id, int cpu_index)

+{
+    uint8_t assoc_size = 0x4;
+
+    if (cpu_index >= 0) {
+        assoc_size = 0x5;
+        assoc[5] = cpu_to_be32(cpu_index);
+    }
+
+    assoc[0] = cpu_to_be32(assoc_size);
+    assoc[1] = cpu_to_be32(0x0);
+    assoc[2] = cpu_to_be32(0x0);
+    assoc[3] = cpu_to_be32(0x0);
+    assoc[4] = cpu_to_be32(node_id);
+}
+
  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)};
+    uint32_t associativity[6];
+    spapr_set_associativity(associativity, cpu->node_id, index);

/* Advertise NUMA via ibm,associativity */

      return fdt_setprop(fdt, offset, "ibm,associativity", associativity,
@@ -325,15 +337,13 @@ static void add_str(GString *s, const gchar *s1)
  static int spapr_dt_memory_node(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)
-    };
+    uint32_t associativity[5];
      char mem_name[32];
      uint64_t mem_reg_property[2];
      int off;

+ spapr_set_associativity(associativity, nodeid, -1);

+
      mem_reg_property[0] = cpu_to_be64(start);
      mem_reg_property[1] = cpu_to_be64(size);

diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c

index 81410aa63f..bd109bfc00 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -115,15 +115,13 @@ 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)
-    };
+    uint32_t associativity[5];
      uint64_t lsize = nvdimm->label_size;
      uint64_t size = object_property_get_int(OBJECT(nvdimm), PC_DIMM_SIZE_PROP,
                                              NULL);

+ spapr_set_associativity(associativity, node, -1);

+
      drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PMEM, slot);
      g_assert(drc);

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c

index 09ac58fd7f..c02ace226c 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2321,11 +2321,8 @@ 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)};
+    uint32_t associativity[5];
+
      SpaprTceTable *tcet;
      SpaprDrc *drc;
      Error *err = NULL;
@@ -2358,6 +2355,7 @@ int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState 
*phb,

/* Advertise NUMA via ibm,associativity */

      if (phb->numa_node != -1) {
+        spapr_set_associativity(associativity, phb->numa_node, -1);
          _FDT(fdt_setprop(fdt, bus_off, "ibm,associativity", associativity,
                           sizeof(associativity)));
      }
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index d9f1afa8b2..cd158bf95a 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -863,6 +863,7 @@ int spapr_phb_dt_populate(SpaprDrc *drc, SpaprMachineState 
*spapr,

void spapr_rtc_read(SpaprRtcState *rtc, struct tm *tm, uint32_t *ns);

  int spapr_rtc_import_offset(SpaprRtcState *rtc, int64_t legacy_offset);
+void spapr_set_associativity(uint32_t *assoc, int node_id, int cpu_index);

#define TYPE_SPAPR_RNG "spapr-rng"