| Message ID | 1486655834-9708-16-git-send-email-vijay.kilari@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Vijay, On 09/02/17 15:57, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > > Parse MADT table and extract MPIDR for all > CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries > and store in cpu_uid_to_hwid[]. > > This mapping is used by SRAT table parsing to > extract MPIDR of the CPU ID. > > Signed-off-by: Vijaya Kumar <Vijaya.Kumar@cavium.com> > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/acpi_numa.c | 122 +++++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/numa.c | 1 + This new file should go in xen/arch/arm/acpi/ > xen/include/asm-arm/acpi.h | 2 + > 4 files changed, 126 insertions(+) > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 7694485..8c5e67b 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -51,6 +51,7 @@ obj-y += vpsci.o > obj-y += vuart.o > obj-$(CONFIG_NUMA) += numa.o > obj-$(CONFIG_NUMA) += dt_numa.o > +obj-$(CONFIG_ACPI_NUMA) += acpi_numa.o > > #obj-bin-y += ....o > > diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c > new file mode 100644 > index 0000000..3ee87f2 > --- /dev/null > +++ b/xen/arch/arm/acpi_numa.c > @@ -0,0 +1,122 @@ > +/* > + * ACPI based NUMA setup > + * > + * Copyright (C) 2016 - Cavium Inc. > + * Vijaya Kumar K <Vijaya.Kumar@cavium.com> > + * > + * 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 as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. Xen is GPLv2, please update the license accordingly. > + * > + * 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/srat.h> > +#include <asm/page.h> > +#include <asm/acpi.h> > + > +extern nodemask_t numa_nodes_parsed; > +struct uid_to_mpidr { > + u32 uid; > + u64 mpidr; > +}; > + > +/* Holds mapping of CPU id to MPIDR read from MADT */ > +static struct uid_to_mpidr cpu_uid_to_hwid[NR_CPUS] __read_mostly; > + > +static __init void bad_srat(void) > +{ > + int i; > + > + printk(KERN_ERR "SRAT: SRAT not used.\n"); > + acpi_numa = -1; > + for (i = 0; i < ARRAY_SIZE(pxm2node); i++) > + pxm2node[i].node = NUMA_NO_NODE; > +} > + > +static u64 acpi_get_cpu_mpidr(int uid) > +{ > + int i; > + > + if ( uid < ARRAY_SIZE(cpu_uid_to_hwid) && cpu_uid_to_hwid[uid].uid == uid && > + cpu_uid_to_hwid[uid].mpidr != MPIDR_INVALID ) > + return cpu_uid_to_hwid[uid].mpidr; Please don't make a special case. This makes more complicate to read the code. We should just loop to find the entry matching the UID. > + > + for ( i = 0; i < NR_CPUS; i++ ) You can limit the loop by keeping an the number of element in the array. > + { > + if ( cpu_uid_to_hwid[i].uid == uid ) > + return cpu_uid_to_hwid[i].mpidr; > + } > + > + return MPIDR_INVALID; > +} > + > +static void __init > +acpi_map_cpu_to_mpidr(struct acpi_madt_generic_interrupt *processor) > +{ > + static int cpus = 0; > + > + u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK; > + > + if ( mpidr == MPIDR_INVALID ) > + { > + printk("Skip MADT cpu entry with invalid MPIDR\n"); > + bad_srat(); > + return; > + } > + > + cpu_uid_to_hwid[cpus].mpidr = mpidr; > + cpu_uid_to_hwid[cpus].uid = processor->uid; > + > + cpus++; > +} > + > +static int __init acpi_parse_madt_handler(struct acpi_subtable_header *header, > + const unsigned long end) > +{ > + struct acpi_madt_generic_interrupt *p = > + container_of(header, struct acpi_madt_generic_interrupt, header); > + > + if ( BAD_MADT_ENTRY(p, end) ) > + { > + /* Though MADT is invalid, we disable NUMA by calling bad_srat() */ > + bad_srat(); > + return -EINVAL; > + } > + > + acpi_table_print_madt_entry(header); > + acpi_map_cpu_to_mpidr(p); > + > + return 0; > +} Why do you need to parse the MADT again? Can't this be done in the parsing made in acpi/boot.c? > + > +void __init acpi_map_uid_to_mpidr(void) > +{ > + int i; unsigned int. > + > + for ( i = 0; i < NR_CPUS; i++ ) > + { > + cpu_uid_to_hwid[i].mpidr = MPIDR_INVALID; > + cpu_uid_to_hwid[i].uid = -1; > + } > + > + acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, > + acpi_parse_madt_handler, 0); > +} > + > +void __init acpi_numa_arch_fixup(void) {} Missing emacs magic. > diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c > index 31dc552..5c49347 100644 > --- a/xen/arch/arm/numa.c > +++ b/xen/arch/arm/numa.c > @@ -20,6 +20,7 @@ > #include <xen/mm.h> > #include <xen/nodemask.h> > #include <xen/pfn.h> > +#include <xen/acpi.h> Why this include? This patch should compile without it. > #include <asm/mm.h> > #include <xen/numa.h> > #include <asm/acpi.h> > diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h > index 9f954d3..b1f36f4 100644 > --- a/xen/include/asm-arm/acpi.h > +++ b/xen/include/asm-arm/acpi.h > @@ -68,6 +68,8 @@ static inline void enable_acpi(void) > { > acpi_disabled = 0; > } > + > +void acpi_map_uid_to_mpidr(void); > #else > #define acpi_disabled (1) > #define disable_acpi() > Regards,
On Thu, Mar 2, 2017 at 9:58 PM, Julien Grall <julien.grall@arm.com> wrote: > Hello Vijay, > > On 09/02/17 15:57, vijay.kilari@gmail.com wrote: >> >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> >> Parse MADT table and extract MPIDR for all >> CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries >> and store in cpu_uid_to_hwid[]. >> >> This mapping is used by SRAT table parsing to >> extract MPIDR of the CPU ID. >> >> Signed-off-by: Vijaya Kumar <Vijaya.Kumar@cavium.com> >> --- >> xen/arch/arm/Makefile | 1 + >> xen/arch/arm/acpi_numa.c | 122 >> +++++++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/numa.c | 1 + > > > This new file should go in xen/arch/arm/acpi/ shouldn't be in xen/arch/arm/numa/? > > >> xen/include/asm-arm/acpi.h | 2 + >> 4 files changed, 126 insertions(+) >> >> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile >> index 7694485..8c5e67b 100644 >> --- a/xen/arch/arm/Makefile >> +++ b/xen/arch/arm/Makefile >> @@ -51,6 +51,7 @@ obj-y += vpsci.o >> obj-y += vuart.o >> obj-$(CONFIG_NUMA) += numa.o >> obj-$(CONFIG_NUMA) += dt_numa.o >> +obj-$(CONFIG_ACPI_NUMA) += acpi_numa.o >> >> #obj-bin-y += ....o >> >> diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c >> new file mode 100644 >> index 0000000..3ee87f2 >> --- /dev/null >> +++ b/xen/arch/arm/acpi_numa.c >> @@ -0,0 +1,122 @@ >> +/* >> + * ACPI based NUMA setup >> + * >> + * Copyright (C) 2016 - Cavium Inc. >> + * Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> + * >> + * 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 as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. > > > Xen is GPLv2, please update the license accordingly. > > >> + * >> + * 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/srat.h> >> +#include <asm/page.h> >> +#include <asm/acpi.h> >> + >> +extern nodemask_t numa_nodes_parsed; >> +struct uid_to_mpidr { >> + u32 uid; >> + u64 mpidr; >> +}; >> + >> +/* Holds mapping of CPU id to MPIDR read from MADT */ >> +static struct uid_to_mpidr cpu_uid_to_hwid[NR_CPUS] __read_mostly; >> + >> +static __init void bad_srat(void) >> +{ >> + int i; >> + >> + printk(KERN_ERR "SRAT: SRAT not used.\n"); >> + acpi_numa = -1; >> + for (i = 0; i < ARRAY_SIZE(pxm2node); i++) >> + pxm2node[i].node = NUMA_NO_NODE; >> +} >> + >> +static u64 acpi_get_cpu_mpidr(int uid) >> +{ >> + int i; >> + >> + if ( uid < ARRAY_SIZE(cpu_uid_to_hwid) && cpu_uid_to_hwid[uid].uid == >> uid && >> + cpu_uid_to_hwid[uid].mpidr != MPIDR_INVALID ) >> + return cpu_uid_to_hwid[uid].mpidr; > > > Please don't make a special case. This makes more complicate to read the > code. > > We should just loop to find the entry matching the UID. > >> + >> + for ( i = 0; i < NR_CPUS; i++ ) > > > You can limit the loop by keeping an the number of element in the array. OK. > > >> + { >> + if ( cpu_uid_to_hwid[i].uid == uid ) >> + return cpu_uid_to_hwid[i].mpidr; >> + } >> + >> + return MPIDR_INVALID; >> +} >> + >> +static void __init >> +acpi_map_cpu_to_mpidr(struct acpi_madt_generic_interrupt *processor) >> +{ >> + static int cpus = 0; >> + >> + u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK; >> + >> + if ( mpidr == MPIDR_INVALID ) >> + { >> + printk("Skip MADT cpu entry with invalid MPIDR\n"); >> + bad_srat(); >> + return; >> + } >> + >> + cpu_uid_to_hwid[cpus].mpidr = mpidr; >> + cpu_uid_to_hwid[cpus].uid = processor->uid; >> + >> + cpus++; >> +} >> + >> +static int __init acpi_parse_madt_handler(struct acpi_subtable_header >> *header, >> + const unsigned long end) >> +{ >> + struct acpi_madt_generic_interrupt *p = >> + container_of(header, struct acpi_madt_generic_interrupt, >> header); >> + >> + if ( BAD_MADT_ENTRY(p, end) ) >> + { >> + /* Though MADT is invalid, we disable NUMA by calling bad_srat() >> */ >> + bad_srat(); >> + return -EINVAL; >> + } >> + >> + acpi_table_print_madt_entry(header); >> + acpi_map_cpu_to_mpidr(p); >> + >> + return 0; >> +} > > > Why do you need to parse the MADT again? Can't this be done in the parsing > made in acpi/boot.c? I will check. But I see that this is done quite late in smp_init().
On 02/03/17 16:41, Vijay Kilari wrote: > On Thu, Mar 2, 2017 at 9:58 PM, Julien Grall <julien.grall@arm.com> wrote: >> Hello Vijay, >> >> On 09/02/17 15:57, vijay.kilari@gmail.com wrote: >>> >>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >>> >>> Parse MADT table and extract MPIDR for all >>> CPU IDs in MADT ACPI_MADT_TYPE_GENERIC_INTERRUPT entries >>> and store in cpu_uid_to_hwid[]. >>> >>> This mapping is used by SRAT table parsing to >>> extract MPIDR of the CPU ID. >>> >>> Signed-off-by: Vijaya Kumar <Vijaya.Kumar@cavium.com> >>> --- >>> xen/arch/arm/Makefile | 1 + >>> xen/arch/arm/acpi_numa.c | 122 >>> +++++++++++++++++++++++++++++++++++++++++++++ >>> xen/arch/arm/numa.c | 1 + >> >> >> This new file should go in xen/arch/arm/acpi/ > > shouldn't be in xen/arch/arm/numa/? If you introduce a numa directory then move in it. Otherwise acpi/. >>> +static int __init acpi_parse_madt_handler(struct acpi_subtable_header >>> *header, >>> + const unsigned long end) >>> +{ >>> + struct acpi_madt_generic_interrupt *p = >>> + container_of(header, struct acpi_madt_generic_interrupt, >>> header); >>> + >>> + if ( BAD_MADT_ENTRY(p, end) ) >>> + { >>> + /* Though MADT is invalid, we disable NUMA by calling bad_srat() >>> */ >>> + bad_srat(); >>> + return -EINVAL; >>> + } >>> + >>> + acpi_table_print_madt_entry(header); >>> + acpi_map_cpu_to_mpidr(p); >>> + >>> + return 0; >>> +} >> >> >> Why do you need to parse the MADT again? Can't this be done in the parsing >> made in acpi/boot.c? > I will check. But I see that this is done quite late in smp_init(). Then rework the code. Regards,
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 7694485..8c5e67b 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -51,6 +51,7 @@ obj-y += vpsci.o obj-y += vuart.o obj-$(CONFIG_NUMA) += numa.o obj-$(CONFIG_NUMA) += dt_numa.o +obj-$(CONFIG_ACPI_NUMA) += acpi_numa.o #obj-bin-y += ....o diff --git a/xen/arch/arm/acpi_numa.c b/xen/arch/arm/acpi_numa.c new file mode 100644 index 0000000..3ee87f2 --- /dev/null +++ b/xen/arch/arm/acpi_numa.c @@ -0,0 +1,122 @@ +/* + * ACPI based NUMA setup + * + * Copyright (C) 2016 - Cavium Inc. + * Vijaya Kumar K <Vijaya.Kumar@cavium.com> + * + * 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 as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * 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/srat.h> +#include <asm/page.h> +#include <asm/acpi.h> + +extern nodemask_t numa_nodes_parsed; +struct uid_to_mpidr { + u32 uid; + u64 mpidr; +}; + +/* Holds mapping of CPU id to MPIDR read from MADT */ +static struct uid_to_mpidr cpu_uid_to_hwid[NR_CPUS] __read_mostly; + +static __init void bad_srat(void) +{ + int i; + + printk(KERN_ERR "SRAT: SRAT not used.\n"); + acpi_numa = -1; + for (i = 0; i < ARRAY_SIZE(pxm2node); i++) + pxm2node[i].node = NUMA_NO_NODE; +} + +static u64 acpi_get_cpu_mpidr(int uid) +{ + int i; + + if ( uid < ARRAY_SIZE(cpu_uid_to_hwid) && cpu_uid_to_hwid[uid].uid == uid && + cpu_uid_to_hwid[uid].mpidr != MPIDR_INVALID ) + return cpu_uid_to_hwid[uid].mpidr; + + for ( i = 0; i < NR_CPUS; i++ ) + { + if ( cpu_uid_to_hwid[i].uid == uid ) + return cpu_uid_to_hwid[i].mpidr; + } + + return MPIDR_INVALID; +} + +static void __init +acpi_map_cpu_to_mpidr(struct acpi_madt_generic_interrupt *processor) +{ + static int cpus = 0; + + u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK; + + if ( mpidr == MPIDR_INVALID ) + { + printk("Skip MADT cpu entry with invalid MPIDR\n"); + bad_srat(); + return; + } + + cpu_uid_to_hwid[cpus].mpidr = mpidr; + cpu_uid_to_hwid[cpus].uid = processor->uid; + + cpus++; +} + +static int __init acpi_parse_madt_handler(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_interrupt *p = + container_of(header, struct acpi_madt_generic_interrupt, header); + + if ( BAD_MADT_ENTRY(p, end) ) + { + /* Though MADT is invalid, we disable NUMA by calling bad_srat() */ + bad_srat(); + return -EINVAL; + } + + acpi_table_print_madt_entry(header); + acpi_map_cpu_to_mpidr(p); + + return 0; +} + +void __init acpi_map_uid_to_mpidr(void) +{ + int i; + + for ( i = 0; i < NR_CPUS; i++ ) + { + cpu_uid_to_hwid[i].mpidr = MPIDR_INVALID; + cpu_uid_to_hwid[i].uid = -1; + } + + acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, + acpi_parse_madt_handler, 0); +} + +void __init acpi_numa_arch_fixup(void) {} diff --git a/xen/arch/arm/numa.c b/xen/arch/arm/numa.c index 31dc552..5c49347 100644 --- a/xen/arch/arm/numa.c +++ b/xen/arch/arm/numa.c @@ -20,6 +20,7 @@ #include <xen/mm.h> #include <xen/nodemask.h> #include <xen/pfn.h> +#include <xen/acpi.h> #include <asm/mm.h> #include <xen/numa.h> #include <asm/acpi.h> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h index 9f954d3..b1f36f4 100644 --- a/xen/include/asm-arm/acpi.h +++ b/xen/include/asm-arm/acpi.h @@ -68,6 +68,8 @@ static inline void enable_acpi(void) { acpi_disabled = 0; } + +void acpi_map_uid_to_mpidr(void); #else #define acpi_disabled (1) #define disable_acpi()