| Message ID | 20200814205424.543857-8-danielhb413@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pseries NUMA distance rework | expand |
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 <danielhb413@gmail.com> Reviewed-by: David Gibson <david@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? > --- > 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" >
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 <danielhb413@gmail.com> > > Reviewed-by: David Gibson <david@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" >> >
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"
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 <danielhb413@gmail.com> --- 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(-)