Re: [Xen-devel] [RFC PATCH v3 19/24] ARM: NUMA: Extract MPIDR from MADT table

[Julien Grall <julien.grall@xxxxxxx>]

Hi Vijay,

On 18/07/17 12:41, vijay.kilari@xxxxxxxxx wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
>
> Parse MADT table and extract MPIDR for all
> CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries
> and store in cpuid_to_hwid_map[]
>
> This mapping is used by SRAT table parsing to extract MPIDR
> of the CPU ID.
>
> MADT table is also parsed in arm/acpi/boot.c during smp boot.
> However cannot wait till smp boot as SRAT table is parsed
> much before during numa_init. Hence MADT is parsed twice
> during boot. Once in numa_init and another in smp init.
The mapping CPU IDs <-> MIDR is not NUMA specific. I still can't see why 
you didn't rework the code as I asked in v1.
I would really appreciate if you try to add the support of NUMA on ARM 
using other things than a hammer. This will not be sustainable to 
maintain in long-term.
> Signed-off-by: Vijaya Kumar <Vijaya.Kumar@xxxxxxxxxx>
> ---
> v3: - acpi_numa is set to -1 on numa failure.
> ---
>  xen/arch/arm/numa/Makefile    |  1 +
>  xen/arch/arm/numa/acpi_numa.c | 94 +++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/numa/numa.c      |  6 +++
>  3 files changed, 101 insertions(+)
>
> diff --git a/xen/arch/arm/numa/Makefile b/xen/arch/arm/numa/Makefile
> index 3af3aff..b549459 100644
> --- a/xen/arch/arm/numa/Makefile
> +++ b/xen/arch/arm/numa/Makefile
> @@ -1,2 +1,3 @@
>  obj-y += dt_numa.o
>  obj-y += numa.o
> +obj-$(CONFIG_ACPI_NUMA) += acpi_numa.o
> diff --git a/xen/arch/arm/numa/acpi_numa.c b/xen/arch/arm/numa/acpi_numa.c
> new file mode 100644
> index 0000000..d9ad547
> --- /dev/null
> +++ b/xen/arch/arm/numa/acpi_numa.c
> @@ -0,0 +1,94 @@
> +/*
> + * ACPI based NUMA setup
> + *
> + * Copyright (C) 2016 - Cavium Inc.
> + * Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
> + *
> + * Reads the ACPI MADT and SRAT table to setup NUMA information.
> + * Contains Excerpts from x86 implementation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/init.h>
> +#include <xen/mm.h>
> +#include <xen/inttypes.h>
> +#include <xen/nodemask.h>
> +#include <xen/acpi.h>
> +#include <xen/numa.h>
> +#include <xen/pfn.h>
> +#include <xen/acpi.h>
> +#include <acpi/srat.h>
> +#include <asm/page.h>
> +
> +/* Holds CPUID to MPIDR mapping read from MADT table. */
> +struct cpuid_to_hwid {
> +    uint32_t cpuid;
> +    uint64_t hwid;
> +};
> +
> +#define PHYS_CPUID_INVALID 0xff
> +
> +/* Holds mapping of CPU id to MPIDR read from MADT */
> +static struct cpuid_to_hwid __read_mostly cpuid_to_hwid_map[NR_CPUS] =
> +    { [0 ... NR_CPUS - 1] = {PHYS_CPUID_INVALID, MPIDR_INVALID} };
> +static unsigned int num_cpuid_to_hwid;
> +
> +static void __init acpi_map_cpu_to_hwid(uint32_t cpuid, uint64_t mpidr)
> +{
> +    if ( mpidr == MPIDR_INVALID )
> +    {
> +        printk("Skip MADT cpu entry with invalid MPIDR\n");
> +        numa_failed();
> +        return;
> +    }
> +
> +    cpuid_to_hwid_map[num_cpuid_to_hwid].hwid = mpidr;
> +    cpuid_to_hwid_map[num_cpuid_to_hwid].cpuid = cpuid;
> +    num_cpuid_to_hwid++;
> +}
> +
> +static int __init acpi_parse_madt_handler(struct acpi_subtable_header *header,
> +                                          const unsigned long end)
> +{
> +    uint64_t mpidr;
> +    struct acpi_madt_generic_interrupt *p =
> +               container_of(header, struct acpi_madt_generic_interrupt, 
> header);
> +
> +    if ( BAD_MADT_ENTRY(p, end) )
> +    {
> +        /* MADT is invalid, we disable NUMA by calling numa_failed() */
> +        numa_failed();
> +        return -EINVAL;
> +    }
> +
> +    acpi_table_print_madt_entry(header);
Why do you always print the MADT entry?

> +    mpidr = p->arm_mpidr & MPIDR_HWID_MASK;
> +    acpi_map_cpu_to_hwid(p->uid, mpidr);
> +
> +    return 0;
> +}
> +
> +void __init acpi_map_uid_to_mpidr(void)
> +{
> +    acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
> +                    acpi_parse_madt_handler, NR_CPUS);
> +}
> +
> +void __init acpi_numa_arch_fixup(void) {}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/numa/numa.c b/xen/arch/arm/numa/numa.c
> index 85352dc..26aa4c0 100644
> --- a/xen/arch/arm/numa/numa.c
> +++ b/xen/arch/arm/numa/numa.c
> @@ -19,6 +19,7 @@
>  #include <xen/nodemask.h>
>  #include <xen/numa.h>
>  #include <xen/pfn.h>
> +#include <acpi/srat.h>
>  #include <asm/acpi.h>
>
>  static uint8_t (*node_distance_fn)(nodeid_t a, nodeid_t b);
> @@ -40,6 +41,11 @@ void numa_failed(void)
>      init_dt_numa_distance();
>      node_distance_fn = NULL;
>      init_cpu_to_node();
> +
> +#ifdef CONFIG_ACPI_NUMA
> +    acpi_numa = -1;
> +    reset_pxm2node();
> +#endif
ARM support both DT and ACPI, so it sounds a bit weird to always do that.

>  }
>
>  void __init numa_set_cpu_node(int cpu, unsigned int nid)
Cheers,

--
Julien Grall

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