Re: [PATCH v2 10/10] xen/x86: add detection of memory interleaves for different nodes

[Wei Chen <Wei.Chen@arm.com>]

Hi Jan,

On 2022/4/26 17:20, Jan Beulich wrote:
> On 18.04.2022 11:07, Wei Chen wrote:
>> --- a/xen/arch/x86/srat.c
>> +++ b/xen/arch/x86/srat.c
>> @@ -271,6 +271,35 @@ acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *pa)
>>   		       pxm, pa->apic_id, node);
>>   }
>>   
>> +/*
>> + * Check to see if there are other nodes within this node's range.
>> + * We just need to check full contains situation. Because overlaps
>> + * have been checked before by conflicting_memblks.
>> + */
>> +static bool __init check_node_memory_interleave(nodeid_t nid,
>> +                                                paddr_t start, paddr_t end)
>> +{
>> +	nodeid_t i;
>> +	const struct node *nd = &nodes[nid];
>> +
>> +	for_each_node_mask(i, memory_nodes_parsed)
>> +	{
>> +		/* Skip itself */
>> +		if (i == nid)
>> +			continue;
>> +
>> +		nd = &nodes[i];
>> +		if (start < nd->start && nd->end < end) {
>> +			printk(KERN_ERR
>> +			       "Node %u: (%"PRIpaddr"-%"PRIpaddr") interleaves with node %u (%"PRIpaddr"-%"PRIpaddr")\n",
>> +			       nid, start, end, i, nd->start, nd->end);
>> +			return true;
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>   /* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
>>   void __init
>>   acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma)
>> @@ -340,10 +369,22 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma)
> 
> Just up from here there already is overlap detection (via a call to
> conflicting_memblks(), and you even mention that in the earlier
> comment). If that doesn't cover all cases, I think it wants fixing
> there rather than introducing a 2nd checking function. But afaics
> that code covers the "fully contains" case.
> 

Yes, that makes sense, I will try to add this case check in 
conflicting_memblks.

Thanks,
Wei Chen

> Jan
> 
>>   			nd->start = start;
>>   			nd->end = end;
>>   		} else {
>> -			if (start < nd->start)
>> -				nd->start = start;
>> -			if (nd->end < end)
>> -				nd->end = end;
>> +			paddr_t new_start = nd->start;
>> +			paddr_t new_end = nd->end;
>> +
>> +			if (start < new_start)
>> +				new_start = start;
>> +			if (new_end < end)
>> +				new_end = end;
>> +
>> +			/* Check whether new range contains memory for other nodes */
>> +			if (check_node_memory_interleave(node, new_start, new_end)) {
>> +				bad_srat();
>> +				return;
>> +			}
>> +
>> +			nd->start = new_start;
>> +			nd->end = new_end;
>>   		}
>>   	}
>>   	printk(KERN_INFO "SRAT: Node %u PXM %u %"PRIpaddr"-%"PRIpaddr"%s\n",
>