Re: [XEN RFC PATCH 20/40] xen/arm: implement node distance helpers for Arm64

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 11 Aug 2021, Wei Chen wrote:
> In current Xen code, __node_distance is a fake API, it always
> returns NUMA_REMOTE_DISTANCE(20). Now we use a matrix to record
> the distance between any two nodes. Accordingly, we provide a
> set_node_distance API to set the distance for any two nodes in
> this patch.
> 
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> ---
>  xen/arch/arm/numa.c        | 44 ++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/numa.h | 12 ++++++++++-
>  xen/include/asm-x86/numa.h |  1 -
>  xen/include/xen/numa.h     |  2 +-
>  4 files changed, 56 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c
> index 566ad1e52b..f61a8df645 100644
> --- a/xen/arch/arm/numa.c
> +++ b/xen/arch/arm/numa.c
> @@ -23,6 +23,11 @@
>  #include <xen/pfn.h>
>  #include <asm/setup.h>
>  
> +static uint8_t __read_mostly
> +node_distance_map[MAX_NUMNODES][MAX_NUMNODES] = {
> +    { NUMA_REMOTE_DISTANCE }
> +};
> +
>  void numa_set_node(int cpu, nodeid_t nid)
>  {
>      if ( nid >= MAX_NUMNODES ||
> @@ -32,6 +37,45 @@ void numa_set_node(int cpu, nodeid_t nid)
>      cpu_to_node[cpu] = nid;
>  }
>  
> +void __init numa_set_distance(nodeid_t from, nodeid_t to, uint32_t distance)
> +{
> +    if ( from >= MAX_NUMNODES || to >= MAX_NUMNODES )
> +    {
> +        printk(KERN_WARNING
> +            "NUMA nodes are out of matrix, from=%u to=%u distance=%u\n",
> +            from, to, distance);

NIT: please align. Example:

printk(KERN_WARNING
       "NUMA nodes are out of matrix, from=%u to=%u distance=%u\n",

Also please use PRIu32 for uint32_t. Probably should use PRIu8 for
nodeids.


> +        return;
> +    }
> +
> +    /* NUMA defines 0xff as an unreachable node and 0-9 are undefined */
> +    if ( distance >= NUMA_NO_DISTANCE ||
> +        (distance >= NUMA_DISTANCE_UDF_MIN &&
> +         distance <= NUMA_DISTANCE_UDF_MAX) ||
> +        (from == to && distance != NUMA_LOCAL_DISTANCE) )
> +    {
> +        printk(KERN_WARNING
> +            "Invalid NUMA node distance, from:%d to:%d distance=%d\n",
> +            from, to, distance);

NIT: please align

Also you used %u before for nodeids, which is better because from and to
are unsigned. Distance should be uint32_t.


> +        return;
> +    }
> +
> +    node_distance_map[from][to] = distance;

Shouldn't we also be setting:

    node_distance_map[to][from] = distance;

?


> +}
> +
> +uint8_t __node_distance(nodeid_t from, nodeid_t to)
> +{
> +    /*
> +     * Check whether the nodes are in the matrix range.
> +     * When any node is out of range, except from and to nodes are the
> +     * same, we treat them as unreachable (return 0xFF)
> +     */
> +    if ( from >= MAX_NUMNODES || to >= MAX_NUMNODES )
> +        return from == to ? NUMA_LOCAL_DISTANCE : NUMA_NO_DISTANCE;
> +
> +    return node_distance_map[from][to];
> +}
> +EXPORT_SYMBOL(__node_distance);
> +
>  void __init numa_init(bool acpi_off)
>  {
>      uint32_t idx;
> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> index bb495a24e1..559b028a01 100644
> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -12,8 +12,19 @@ typedef u8 nodeid_t;
>   * set the number of NUMA memory block number to 128.
>   */
>  #define NODES_SHIFT      6
> +/*
> + * In ACPI spec, 0-9 are the reserved values for node distance,
> + * 10 indicates local node distance, 20 indicates remote node
> + * distance. Set node distance map in device tree will follow
> + * the ACPI's definition.
> + */
> +#define NUMA_DISTANCE_UDF_MIN   0
> +#define NUMA_DISTANCE_UDF_MAX   9
> +#define NUMA_LOCAL_DISTANCE     10
> +#define NUMA_REMOTE_DISTANCE    20
>  
>  extern void numa_init(bool acpi_off);
> +extern void numa_set_distance(nodeid_t from, nodeid_t to, uint32_t distance);
>  
>  /*
>   * Temporary for fake NUMA node, when CPU, memory and distance
> @@ -21,7 +32,6 @@ extern void numa_init(bool acpi_off);
>   * symbols will be removed.
>   */
>  extern mfn_t first_valid_mfn;
> -#define __node_distance(a, b) (20)
>  
>  #else
>  
> diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
> index 5a57a51e26..e0253c20b7 100644
> --- a/xen/include/asm-x86/numa.h
> +++ b/xen/include/asm-x86/numa.h
> @@ -21,7 +21,6 @@ extern nodeid_t apicid_to_node[];
>  extern void init_cpu_to_node(void);
>  
>  void srat_parse_regions(u64 addr);
> -extern u8 __node_distance(nodeid_t a, nodeid_t b);
>  unsigned int arch_get_dma_bitsize(void);
>  
>  #endif
> diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
> index cb08d2eca9..0475823b13 100644
> --- a/xen/include/xen/numa.h
> +++ b/xen/include/xen/numa.h
> @@ -58,7 +58,7 @@ static inline __attribute__((pure)) nodeid_t 
> phys_to_nid(paddr_t addr)
>  #define node_spanned_pages(nid)      (NODE_DATA(nid)->node_spanned_pages)
>  #define node_end_pfn(nid)       (NODE_DATA(nid)->node_start_pfn + \
>                                NODE_DATA(nid)->node_spanned_pages)
> -
> +extern u8 __node_distance(nodeid_t a, nodeid_t b);
>  extern void numa_add_cpu(int cpu);
>  
>  struct node {
> -- 
> 2.25.1
>