| Message ID | 1456467780-8476-9-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 26.02.16 at 07:22, <zhaoshenglong@huawei.com> wrote: > --- a/xen/include/xen/acpi.h > +++ b/xen/include/xen/acpi.h > @@ -39,6 +39,10 @@ > #define ACPI_MADT_GET_POLARITY(inti) ACPI_MADT_GET_(POLARITY, inti) > #define ACPI_MADT_GET_TRIGGER(inti) ACPI_MADT_GET_(TRIGGER, inti) > > +#define BAD_MADT_ENTRY(entry, end) ( \ > + (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) || \ > + ((struct acpi_subtable_header *)(entry))->length < sizeof(*(entry))) Would it be too much to ask for you to eliminate the bogus cast here, in favor of properly using (entry)->header.length? Afaics this should be fine for all users of the macro. Jan
On 2016/2/26 17:49, Jan Beulich wrote: >>>> On 26.02.16 at 07:22, <zhaoshenglong@huawei.com> wrote: >> > --- a/xen/include/xen/acpi.h >> > +++ b/xen/include/xen/acpi.h >> > @@ -39,6 +39,10 @@ >> > #define ACPI_MADT_GET_POLARITY(inti) ACPI_MADT_GET_(POLARITY, inti) >> > #define ACPI_MADT_GET_TRIGGER(inti) ACPI_MADT_GET_(TRIGGER, inti) >> > >> > +#define BAD_MADT_ENTRY(entry, end) ( \ >> > + (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) || \ >> > + ((struct acpi_subtable_header *)(entry))->length < sizeof(*(entry))) > Would it be too much to ask for you to eliminate the bogus cast > here, in favor of properly using (entry)->header.length? Afaics > this should be fine for all users of the macro. Oh, sorry, will update this. Thanks,
On Fri, 26 Feb 2016, Shannon Zhao wrote: > From: Parth Dixit <parth.dixit@linaro.org> > > MADT contains the information for MPIDR which is essential for SMP > initialization, parse the GIC cpu interface structures to get the MPIDR > value and map it to cpu_logical_map(), and add enabled cpu with valid > MPIDR into cpu_possible_map. > > Move BAD_MADT_ENTRY to common place and parenthesize its parameters. > > Cc: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org> > Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > --- > V5: drop bootcpu_valid and parenthesize parameters of BAD_MADT_ENTRY > V4: fix coding style and address some comments > --- > xen/arch/arm/acpi/boot.c | 121 +++++++++++++++++++++++++++++++++++++++++++++ > xen/arch/x86/acpi/boot.c | 4 -- > xen/include/asm-arm/acpi.h | 1 + > xen/include/xen/acpi.h | 4 ++ > 4 files changed, 126 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c > index 6b33fbe..066b590 100644 > --- a/xen/arch/arm/acpi/boot.c > +++ b/xen/arch/arm/acpi/boot.c > @@ -32,6 +32,127 @@ > #include <xen/mm.h> > > #include <asm/acpi.h> > +#include <asm/smp.h> > + > +/* Processors with enabled flag and sane MPIDR */ > +static unsigned int enabled_cpus; > + > +/* total number of cpus in this system */ > +static unsigned int __initdata total_cpus; > + > +/* > + * acpi_map_gic_cpu_interface - generates a logical cpu number > + * and map to MPIDR represented by GICC structure > + */ > +static void __init > +acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor) > +{ > + int i; > + u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK; > + bool_t enabled = !!(processor->flags & ACPI_MADT_ENABLED); > + > + if ( mpidr == MPIDR_INVALID ) > + { > + printk("Skip MADT cpu entry with invalid MPIDR\n"); > + return; > + } > + > + total_cpus++; > + if ( !enabled ) > + return; > + > + if ( enabled_cpus >= NR_CPUS ) > + { > + printk("NR_CPUS limit of %d reached, Processor %d/0x%"PRIx64" ignored.\n", > + NR_CPUS, total_cpus, mpidr); > + return; > + } > + > + /* Check if GICC structure of boot CPU is available in the MADT */ > + if ( (enabled_cpus == 0) && (cpu_logical_map(0) != mpidr) ) > + { > + printk("Firmware bug, duplicate CPU MPIDR: 0x%"PRIx64" in MADT\n", > + mpidr); Please change this error message into something more appropriate, like: printk("Firmware bug, invalid CPU MPIDR for cpu0: 0x%"PRIx64" in MADT\n", mpidr); with this change: Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > + return; > + } > + > + /* > + * Duplicate MPIDRs are a recipe for disaster. Scan > + * all initialized entries and check for > + * duplicates. If any is found just ignore the CPU. > + */ > + for ( i = 0; i < enabled_cpus; i++ ) > + { > + if ( cpu_logical_map(i) == mpidr ) > + { > + printk("Firmware bug, duplicate CPU MPIDR: 0x%"PRIx64" in MADT\n", > + mpidr); > + return; > + } > + } > + > + if ( !acpi_psci_present() ) > + return; > + > + /* CPU 0 was already initialized */ > + if ( enabled_cpus ) > + { > + if ( arch_cpu_init(enabled_cpus, NULL) < 0 ) > + return; > + > + /* map the logical cpu id to cpu MPIDR */ > + cpu_logical_map(enabled_cpus) = mpidr; > + } > + > + enabled_cpus++; > +} > + > +static int __init > +acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header, > + const unsigned long end) > +{ > + struct acpi_madt_generic_interrupt *processor = > + container_of(header, struct acpi_madt_generic_interrupt, header); > + > + if ( BAD_MADT_ENTRY(processor, end) ) > + return -EINVAL; > + > + acpi_table_print_madt_entry(header); > + acpi_map_gic_cpu_interface(processor); > + return 0; > +} > + > +/* Parse GIC cpu interface entries in MADT for SMP init */ > +void __init acpi_smp_init_cpus(void) > +{ > + int count, i; > + > + /* > + * do a partial walk of MADT to determine how many CPUs > + * we have including disabled CPUs, and get information > + * we need for SMP init > + */ > + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, > + acpi_parse_gic_cpu_interface, 0); > + > + if ( count <= 0 ) > + { > + printk("Error parsing GIC CPU interface entry\n"); > + return; > + } > + > + if ( enabled_cpus > 1 ) > + { > + printk("MADT missing boot CPU MPIDR, not enabling secondaries\n"); > + return; > + } > + > + for ( i = 0; i < enabled_cpus; i++ ) > + cpumask_set_cpu(i, &cpu_possible_map); > + > + /* Make boot-up look pretty */ > + printk("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus); > +} > > static int __init acpi_parse_fadt(struct acpi_table_header *table) > { > diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c > index fac36c6..385c0be 100644 > --- a/xen/arch/x86/acpi/boot.c > +++ b/xen/arch/x86/acpi/boot.c > @@ -43,10 +43,6 @@ > #include <mach_apic.h> > #include <mach_mpparse.h> > > -#define BAD_MADT_ENTRY(entry, end) ( \ > - (!entry) || (unsigned long)entry + sizeof(*entry) > end || \ > - ((struct acpi_subtable_header *)entry)->length != sizeof(*entry)) > - > #define PREFIX "ACPI: " > > bool_t __initdata acpi_noirq; /* skip ACPI IRQ initialization */ > diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h > index f13072f..a0f2f24 100644 > --- a/xen/include/asm-arm/acpi.h > +++ b/xen/include/asm-arm/acpi.h > @@ -32,6 +32,7 @@ > > bool_t __init acpi_psci_present(void); > bool_t __init acpi_psci_hvc_present(void); > +void __init acpi_smp_init_cpus(void); > > #ifdef CONFIG_ACPI > extern bool_t acpi_disabled; > diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h > index 65e53a6..894917b 100644 > --- a/xen/include/xen/acpi.h > +++ b/xen/include/xen/acpi.h > @@ -39,6 +39,10 @@ > #define ACPI_MADT_GET_POLARITY(inti) ACPI_MADT_GET_(POLARITY, inti) > #define ACPI_MADT_GET_TRIGGER(inti) ACPI_MADT_GET_(TRIGGER, inti) > > +#define BAD_MADT_ENTRY(entry, end) ( \ > + (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) || \ > + ((struct acpi_subtable_header *)(entry))->length < sizeof(*(entry))) > + > #ifdef CONFIG_ACPI_BOOT > > enum acpi_interrupt_id { > -- > 2.0.4 > >
diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c index 6b33fbe..066b590 100644 --- a/xen/arch/arm/acpi/boot.c +++ b/xen/arch/arm/acpi/boot.c @@ -32,6 +32,127 @@ #include <xen/mm.h> #include <asm/acpi.h> +#include <asm/smp.h> + +/* Processors with enabled flag and sane MPIDR */ +static unsigned int enabled_cpus; + +/* total number of cpus in this system */ +static unsigned int __initdata total_cpus; + +/* + * acpi_map_gic_cpu_interface - generates a logical cpu number + * and map to MPIDR represented by GICC structure + */ +static void __init +acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor) +{ + int i; + u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK; + bool_t enabled = !!(processor->flags & ACPI_MADT_ENABLED); + + if ( mpidr == MPIDR_INVALID ) + { + printk("Skip MADT cpu entry with invalid MPIDR\n"); + return; + } + + total_cpus++; + if ( !enabled ) + return; + + if ( enabled_cpus >= NR_CPUS ) + { + printk("NR_CPUS limit of %d reached, Processor %d/0x%"PRIx64" ignored.\n", + NR_CPUS, total_cpus, mpidr); + return; + } + + /* Check if GICC structure of boot CPU is available in the MADT */ + if ( (enabled_cpus == 0) && (cpu_logical_map(0) != mpidr) ) + { + printk("Firmware bug, duplicate CPU MPIDR: 0x%"PRIx64" in MADT\n", + mpidr); + return; + } + + /* + * Duplicate MPIDRs are a recipe for disaster. Scan + * all initialized entries and check for + * duplicates. If any is found just ignore the CPU. + */ + for ( i = 0; i < enabled_cpus; i++ ) + { + if ( cpu_logical_map(i) == mpidr ) + { + printk("Firmware bug, duplicate CPU MPIDR: 0x%"PRIx64" in MADT\n", + mpidr); + return; + } + } + + if ( !acpi_psci_present() ) + return; + + /* CPU 0 was already initialized */ + if ( enabled_cpus ) + { + if ( arch_cpu_init(enabled_cpus, NULL) < 0 ) + return; + + /* map the logical cpu id to cpu MPIDR */ + cpu_logical_map(enabled_cpus) = mpidr; + } + + enabled_cpus++; +} + +static int __init +acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_interrupt *processor = + container_of(header, struct acpi_madt_generic_interrupt, header); + + if ( BAD_MADT_ENTRY(processor, end) ) + return -EINVAL; + + acpi_table_print_madt_entry(header); + acpi_map_gic_cpu_interface(processor); + return 0; +} + +/* Parse GIC cpu interface entries in MADT for SMP init */ +void __init acpi_smp_init_cpus(void) +{ + int count, i; + + /* + * do a partial walk of MADT to determine how many CPUs + * we have including disabled CPUs, and get information + * we need for SMP init + */ + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, + acpi_parse_gic_cpu_interface, 0); + + if ( count <= 0 ) + { + printk("Error parsing GIC CPU interface entry\n"); + return; + } + + if ( enabled_cpus > 1 ) + { + printk("MADT missing boot CPU MPIDR, not enabling secondaries\n"); + return; + } + + for ( i = 0; i < enabled_cpus; i++ ) + cpumask_set_cpu(i, &cpu_possible_map); + + /* Make boot-up look pretty */ + printk("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus); +} static int __init acpi_parse_fadt(struct acpi_table_header *table) { diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c index fac36c6..385c0be 100644 --- a/xen/arch/x86/acpi/boot.c +++ b/xen/arch/x86/acpi/boot.c @@ -43,10 +43,6 @@ #include <mach_apic.h> #include <mach_mpparse.h> -#define BAD_MADT_ENTRY(entry, end) ( \ - (!entry) || (unsigned long)entry + sizeof(*entry) > end || \ - ((struct acpi_subtable_header *)entry)->length != sizeof(*entry)) - #define PREFIX "ACPI: " bool_t __initdata acpi_noirq; /* skip ACPI IRQ initialization */ diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h index f13072f..a0f2f24 100644 --- a/xen/include/asm-arm/acpi.h +++ b/xen/include/asm-arm/acpi.h @@ -32,6 +32,7 @@ bool_t __init acpi_psci_present(void); bool_t __init acpi_psci_hvc_present(void); +void __init acpi_smp_init_cpus(void); #ifdef CONFIG_ACPI extern bool_t acpi_disabled; diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h index 65e53a6..894917b 100644 --- a/xen/include/xen/acpi.h +++ b/xen/include/xen/acpi.h @@ -39,6 +39,10 @@ #define ACPI_MADT_GET_POLARITY(inti) ACPI_MADT_GET_(POLARITY, inti) #define ACPI_MADT_GET_TRIGGER(inti) ACPI_MADT_GET_(TRIGGER, inti) +#define BAD_MADT_ENTRY(entry, end) ( \ + (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) || \ + ((struct acpi_subtable_header *)(entry))->length < sizeof(*(entry))) + #ifdef CONFIG_ACPI_BOOT enum acpi_interrupt_id {