Excerpts from Michael Ellerman's message of November 8, 2021 3:20 pm:
> Nicholas Piggin <npig...@gmail.com> writes:
>> In case the FORM2 distance table from firmware is not the expected size,
>> there is fallback code that just populates the lookup table as local vs
>> remote.
>>
>> However it then continues on to use the distance table. Fix.
>>
>> Cc: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com>
>> Fixes: 1c6b5a7e7405 ("powerpc/pseries: Add support for FORM2 associativity")
>> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
>> ---
>> arch/powerpc/mm/numa.c | 29 +++++++++++++----------------
>> 1 file changed, 13 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index 6f14c8fb6359..0789cde7f658 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -380,6 +380,7 @@ static void
>> initialize_form2_numa_distance_lookup_table(void)
>> const __be32 *numa_lookup_index;
>> int numa_dist_table_length;
>> int max_numa_index, distance_index;
>> + bool good = true;
>
> numa_dist_table is a pointer, so couldn't we just set it to NULL if the
> info it's pointing at is invalid?
Yeah probably could just do that.
>
>>
>> if (firmware_has_feature(FW_FEATURE_OPAL))
>> root = of_find_node_by_path("/ibm,opal");
>> @@ -407,30 +408,26 @@ static void
>> initialize_form2_numa_distance_lookup_table(void)
>>
>> if (numa_dist_table_length != max_numa_index * max_numa_index) {
>> WARN(1, "Wrong NUMA distance information\n");
>> - /* consider everybody else just remote. */
>> - for (i = 0; i < max_numa_index; i++) {
>> - for (j = 0; j < max_numa_index; j++) {
>> - int nodeA = numa_id_index_table[i];
>> - int nodeB = numa_id_index_table[j];
>> -
>> - if (nodeA == nodeB)
>> - numa_distance_table[nodeA][nodeB] =
>> LOCAL_DISTANCE;
>> - else
>> - numa_distance_table[nodeA][nodeB] =
>> REMOTE_DISTANCE;
>> - }
>> - }
>> + good = false;
>
> ie. numa_dist_table = NULL;
>
>> }
>> -
>> distance_index = 0;
>> for (i = 0; i < max_numa_index; i++) {
>> for (j = 0; j < max_numa_index; j++) {
>> int nodeA = numa_id_index_table[i];
>> int nodeB = numa_id_index_table[j];
>> -
>> - numa_distance_table[nodeA][nodeB] =
>> numa_dist_table[distance_index++];
>> - pr_debug("dist[%d][%d]=%d ", nodeA, nodeB,
>> numa_distance_table[nodeA][nodeB]);
>> + int dist;
>> +
>> + if (good)
>
> if (numa_dist_table)
>
>> + dist = numa_dist_table[distance_index++];
>> + else if (nodeA == nodeB)
>> + dist = LOCAL_DISTANCE;
>> + else
>> + dist = REMOTE_DISTANCE;
>> + numa_distance_table[nodeA][nodeB] = dist;
>> + pr_debug("dist[%d][%d]=%d ", nodeA, nodeB, dist);
>> }
>> }
>> +
>> of_node_put(root);
>> }
>
>
> But maybe before we do that we can rename it, because it is really easy
> to confuse numa_dist_table and numa_distance_table if you don't look
> closely.
Maybe dt_form2_distances?
Thanks,
Nick
