On Wed, 22 Sep 2021 09:28:52 -0300
Daniel Henrique Barboza <danielhb...@gmail.com> wrote:
> This patch has a handful of modifications for the recent added
> FORM2 support:
>
> - to not allocate more than the necessary size in 'distance_table'.
> At this moment the array is oversized due to allocating uint32_t for
> all elements, when most of them fits in an uint8_t. Fix it by
> changing the array to uint8_t and allocating the exact size;
>
> - use stl_be_p() to store the uint32_t at the start of 'distance_table';
>
> - use sizeof(uint32_t) to skip the uint32_t length when populating the
> distances;
>
> - use the NUMA_DISTANCE_MIN macro from sysemu/numa.h to avoid hardcoding
> the local distance value.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
> ---
> hw/ppc/spapr_numa.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index 58d5dc7084..5822938448 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -502,9 +502,8 @@ static void
> spapr_numa_FORM2_write_rtas_tables(SpaprMachineState *spapr,
> int nb_numa_nodes = ms->numa_state->num_nodes;
> int distance_table_entries = nb_numa_nodes * nb_numa_nodes;
> g_autofree uint32_t *lookup_index_table = NULL;
> - g_autofree uint32_t *distance_table = NULL;
> + g_autofree uint8_t *distance_table = NULL;
> int src, dst, i, distance_table_size;
> - uint8_t *node_distances;
>
> /*
> * ibm,numa-lookup-index-table: array with length and a
> @@ -531,11 +530,13 @@ static void
> spapr_numa_FORM2_write_rtas_tables(SpaprMachineState *spapr,
> * array because NUMA ids can be sparse (node 0 is the first,
> * node 8 is the second ...).
> */
> - distance_table = g_new0(uint32_t, distance_table_entries + 1);
> - distance_table[0] = cpu_to_be32(distance_table_entries);
I personally liked the comment from v1:
+ /*
+ * Distance table is an uint8_t array with a leading uint32_t
+ * containing its length.
+ */
but I guess the code is simple enough to understand that, so :
Reviewed-by: Greg Kurz <gr...@kaod.org>
> + distance_table_size = distance_table_entries * sizeof(uint8_t) +
> + sizeof(uint32_t);
> + distance_table = g_new0(uint8_t, distance_table_size);
> + stl_be_p(distance_table, distance_table_entries);
>
> - node_distances = (uint8_t *)&distance_table[1];
> - i = 0;
> + /* Skip the uint32_t array length at the start */
> + i = sizeof(uint32_t);
>
> for (src = 0; src < nb_numa_nodes; src++) {
> for (dst = 0; dst < nb_numa_nodes; dst++) {
> @@ -546,16 +547,14 @@ static void
> spapr_numa_FORM2_write_rtas_tables(SpaprMachineState *spapr,
> * adding the numa_info to retrieve distance info from.
> */
> if (src == dst) {
> - node_distances[i++] = 10;
> + distance_table[i++] = NUMA_DISTANCE_MIN;
> continue;
> }
>
> - node_distances[i++] = numa_info[src].distance[dst];
> + distance_table[i++] = numa_info[src].distance[dst];
> }
> }
>
> - distance_table_size = distance_table_entries * sizeof(uint8_t) +
> - sizeof(uint32_t);
> _FDT(fdt_setprop(fdt, rtas, "ibm,numa-distance-table",
> distance_table, distance_table_size));
> }
