Re: [Xen-devel] [RFC PATCH v3 22/24] ARM: NUMA: Initialize ACPI NUMA

[Julien Grall <julien.grall@xxxxxxx>]

Hi Vijay,

On 18/07/17 12:41, vijay.kilari@xxxxxxxxx wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
>
> Call ACPI NUMA initialization under CONFIG_ACPI_NUMA.
>
> Signed-off-by: Vijaya Kumar <Vijaya.Kumar@xxxxxxxxxx>
> ---
>  xen/arch/arm/numa/acpi_numa.c | 27 ++++++++++++++++++++++++++-
>  xen/arch/arm/numa/numa.c      | 15 +++++++++++++--
>  xen/common/numa.c             | 14 ++++++++++++++
>  xen/include/asm-arm/numa.h    |  1 +
>  xen/include/xen/numa.h        |  1 +
>  5 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/numa/acpi_numa.c b/xen/arch/arm/numa/acpi_numa.c
> index 95617f9..68fff95 100644
> --- a/xen/arch/arm/numa/acpi_numa.c
> +++ b/xen/arch/arm/numa/acpi_numa.c
> @@ -181,7 +181,7 @@ acpi_numa_gicc_affinity_init(const struct 
> acpi_srat_gicc_affinity *pa)
>             pxm, mpidr, node);
>  }
>
> -void __init acpi_map_uid_to_mpidr(void)
> +static void __init acpi_map_uid_to_mpidr(void)
>  {
>      acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
>                      acpi_parse_madt_handler, NR_CPUS);
> @@ -209,6 +209,31 @@ void __init arch_table_parse_srat(void)
>                            acpi_parse_gicc_affinity, NR_CPUS);
>  }
>
> +bool_t __init arch_acpi_numa_init(void)
When I read arch_* I expect this to be called in common code. This is 
actually not the case. Please rename it in arm_acpi_numa_init or 
something else.
Also, as it has been repeated a numerous amount of time, you should use 
bool and not bool_t.
> +{
> +    int ret;
> +
> +    if ( !acpi_disabled )
> +    {
> +        /*
> +         * If firmware has DT, process_memory_node() call
> +         * would have added memory blocks. So reset it before
> +         * ACPI numa init.
> +         */
Likely something is wrong in the handling path if it is happening.

> +        numa_clear_memblks();
> +        nodes_clear(memory_nodes_parsed);
> +        acpi_map_uid_to_mpidr();
> +        ret = acpi_numa_init();
> +        if ( ret || srat_disabled() )
> +            return 1;
> +
> +        /* Register acpi node_distance handler */
> +        register_node_distance(&acpi_node_distance);
> +    }
> +
> +    return 0;
> +}
> +
>  void __init acpi_numa_arch_fixup(void) {}
>
>  /*
> diff --git a/xen/arch/arm/numa/numa.c b/xen/arch/arm/numa/numa.c
> index 26aa4c0..68599c4 100644
> --- a/xen/arch/arm/numa/numa.c
> +++ b/xen/arch/arm/numa/numa.c
> @@ -139,11 +139,22 @@ void __init numa_init(void)
>      if ( numa_off )
>          goto no_numa;
>
> -    ret = dt_numa_init();
> +#ifdef CONFIG_ACPI_NUMA
if ( !acpi_disabled )
{
 ....
}

And you can avoid #ifdef CONFIG_ACPI_NUMA by introducing stubs as you do 
in other places.
> +    ret = arch_acpi_numa_init();
>      if ( ret )
>      {
>          numa_off = true;
> -        printk(XENLOG_WARNING "DT NUMA init failed\n");
> +        printk(XENLOG_WARNING "ACPI NUMA init failed\n");
> +    }
> +#endif
> +    if ( acpi_disabled )
> +    {
> +        ret = dt_numa_init();
> +        if ( ret )
> +        {
> +            numa_off = true;
> +            printk(XENLOG_WARNING "DT NUMA init failed\n");
> +        }
>      }
>
>  no_numa:
> diff --git a/xen/common/numa.c b/xen/common/numa.c
> index 0f79a07..020bc19 100644
> --- a/xen/common/numa.c
> +++ b/xen/common/numa.c
> @@ -76,6 +76,20 @@ nodeid_t get_memblk_nodeid(unsigned int id)
>      return memblk_nodeid[id];
>  }
>
> +void __init numa_clear_memblks(void)
> +{
> +    unsigned int i;
> +
> +    for ( i = 0; i < get_num_node_memblks(); i++ )
> +    {
> +        node_memblk_range[i].start = 0;
> +        node_memblk_range[i].end = 0;
> +        memblk_nodeid[i] = NUMA_NO_NODE;
> +    }
> +
> +    num_node_memblks = 0;
> +}
> +
>  int __init get_mem_nodeid(paddr_t start, paddr_t end)
>  {
>      unsigned int i;
> diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
> index f0a50bd..ff10b31 100644
> --- a/xen/include/asm-arm/numa.h
> +++ b/xen/include/asm-arm/numa.h
> @@ -20,6 +20,7 @@ static inline nodeid_t acpi_get_nodeid(uint64_t hwid)
>  void numa_init(void);
>  int dt_numa_init(void);
>  void numa_set_cpu_node(int cpu, unsigned int nid);
> +bool_t arch_acpi_numa_init(void);
>
>  #else
>  static inline void numa_init(void)
> diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
> index a541eb7..14a7a0c 100644
> --- a/xen/include/xen/numa.h
> +++ b/xen/include/xen/numa.h
> @@ -75,6 +75,7 @@ int get_num_node_memblks(void);
>  bool arch_sanitize_nodes_memory(void);
>  void numa_failed(void);
>  uint8_t __node_distance(nodeid_t a, nodeid_t b);
> +void numa_clear_memblks(void);
>  #else
>  static inline void numa_add_cpu(int cpu) { }
>  static inline void numa_set_node(int cpu, nodeid_t node) { }
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel