On Fri, 17 May 2013, Lorenzo Pieralisi wrote:

> DT cpu map parsing code must be made compliant with the latest cpus/cpu
> nodes bindings updates, hence this patch updates the arm_dt_init_cpu_maps()
> function with checks and additional parsing rules.
> 
> Uniprocessor systems predating v7 do not parse the cpus node at all
> since the reg property is meaningless on those systems.
> 
> Device trees for 64-bit systems can be taken as device tree input also
> for 64-bit CPUs running in 32-bit mode. The code checks that the reg entries
> are zeroed as required in the respective fields and detects automatically
> the cpus node #address-cells value so that device tree written for
> 64-bit ARM platforms (that can have cpus node #address-cells == 2) can still
> be taken as input. The correct device tree entries are to be set up by the
> boot loader, kernel code just checks that device tree entries in the cpus
> node are as expected for a 32-bit CPU (reg[63:24] == 0).
> 
> cpu node entries with invalid reg property or containing duplicates are
> ignored and the device tree parsing is not stopped anymore when such
> entries are encountered, the device tree cpu node entry is just skipped.
> 
> A device tree with cpu nodes missing the boot CPU MPIDR is considered
> an error and the kernel flags this up as such to trigger firmware updates.
> 
> Signed-off-by: Lorenzo Pieralisi <[email protected]>

Acked-by: Nicolas Pitre <[email protected]>


> ---
>  arch/arm/kernel/devtree.c | 146 
> ++++++++++++++++++++++++++++------------------
>  1 file changed, 88 insertions(+), 58 deletions(-)
> 
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index 0905502..80d6cf24 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -23,6 +23,7 @@
>  #include <asm/setup.h>
>  #include <asm/page.h>
>  #include <asm/smp_plat.h>
> +#include <asm/system_info.h>
>  #include <asm/mach/arch.h>
>  #include <asm/mach-types.h>
>  
> @@ -72,100 +73,129 @@ void __init arm_dt_memblock_reserve(void)
>   */
>  void __init arm_dt_init_cpu_maps(void)
>  {
> -     /*
> -      * Temp logical map is initialized with UINT_MAX values that are
> -      * considered invalid logical map entries since the logical map must
> -      * contain a list of MPIDR[23:0] values where MPIDR[31:24] must
> -      * read as 0.
> -      */
>       struct device_node *cpu, *cpus;
> -     u32 i, j, cpuidx = 1;
> +     u32 i, ac, cpuidx = 1;
> +     int len;
>       u32 mpidr = is_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
>  
> -     u32 tmp_map[NR_CPUS] = { [0 ... NR_CPUS-1] = MPIDR_INVALID };
>       bool bootcpu_valid = false;
>       cpus = of_find_node_by_path("/cpus");
>  
> -     if (!cpus)
> +     if (!cpus || ((cpu_architecture() < CPU_ARCH_ARMv7) && !is_smp()))
>               return;
>  
> +     if (of_property_read_u32(cpus, "#address-cells", &ac)) {
> +             pr_warn("%s invalid #address-cells\n", cpus->full_name);
> +             ac = of_n_addr_cells(cpus);
> +     }
> +     /*
> +      * The boot CPU knows its MPIDR and initialize it
> +      * to allow DT boot CPU detection.
> +      */
> +     cpu_logical_map(0) = mpidr;
> +
>       for_each_child_of_node(cpus, cpu) {
> -             u32 hwid;
> +             u64 hwid64;
> +             u32 hwid32;
> +             const __be32 *prop;
>  
>               if (of_node_cmp(cpu->type, "cpu"))
>                       continue;
>  
>               pr_debug(" * %s...\n", cpu->full_name);
>               /*
> -              * A device tree containing CPU nodes with missing "reg"
> -              * properties is considered invalid to build the
> -              * cpu_logical_map.
> +              * A CPU node with missing or wrong "reg" property is
> +              * considered invalid to build a cpu_logical_map entry.
>                */
> -             if (of_property_read_u32(cpu, "reg", &hwid)) {
> -                     pr_debug(" * %s missing reg property\n",
> -                                  cpu->full_name);
> -                     return;
> +             prop = of_get_property(cpu, "reg", &len);
> +             if (!prop || len < (ac * sizeof(*prop))) {
> +                     pr_warn(" * %s node missing/wrong reg property, 
> skipped\n",
> +                             cpu->full_name);
> +                             goto next;
>               }
>  
>               /*
> -              * 8 MSBs must be set to 0 in the DT since the reg property
> -              * defines the MPIDR[23:0].
> +              * Always read reg as u64 value.
> +              * For dts with #address-cells == 1 hwid64[63:32]
> +              * will be set to 0 by of_read_number.
> +              * Toss away the top 32 bits and store value in hwid32.
>                */
> -             if (hwid & ~MPIDR_HWID_BITMASK)
> -                     return;
> -
> +             hwid32 = hwid64 = of_read_number(prop, ac);
> +             /*
> +              * hwid64[63:24] must be always be 0 since the reg
> +              * property defines the MPIDR[23:0] bits regardless
> +              * of the cpus node #address-cells value.
> +              */
> +             if (hwid64 & ~MPIDR_HWID_BITMASK) {
> +                     pr_warn(" * %s node reg[63:24] must be 0 on 32-bit dts, 
> got %#016llx, skipped\n",
> +                             cpu->full_name, hwid64);
> +                     goto next;
> +             }
>               /*
>                * Duplicate MPIDRs are a recipe for disaster.
>                * Scan all initialized entries and check for
> -              * duplicates. If any is found just bail out.
> -              * temp values were initialized to UINT_MAX
> -              * to avoid matching valid MPIDR[23:0] values.
> +              * duplicates. If any is found just ignore the CPU.
> +              * Boot CPU at logical index 0 is not checked to
> +              * allow self contained boot CPU detection logic.
>                */
> -             for (j = 0; j < cpuidx; j++)
> -                     if (WARN(tmp_map[j] == hwid, "Duplicate /cpu reg "
> -                                                  "properties in the DT\n"))
> -                             return;
> +             for (i = 1; i < cpuidx; i++)
> +                     if (cpu_logical_map(i) == hwid32) {
> +                             pr_warn(" * %s node duplicate cpu reg property, 
> skipped\n",
> +                                     cpu->full_name);
> +                             goto next;
> +                     }
>  
>               /*
> -              * Build a stashed array of MPIDR values. Numbering scheme
> -              * requires that if detected the boot CPU must be assigned
> -              * logical id 0. Other CPUs get sequential indexes starting
> -              * from 1. If a CPU node with a reg property matching the
> -              * boot CPU MPIDR is detected, this is recorded so that the
> -              * logical map built from DT is validated and can be used
> -              * to override the map created in smp_setup_processor_id().
> +              * If a CPU node with a reg property matching the
> +              * cpu_logical_map(0) is detected, this is recorded so
> +              * that the bootcpu_valid condition can be checked when
> +              * DT scanning is completed. Duplicate boot cpu entries
> +              * are flagged up if detected.
>                */
> -             if (hwid == mpidr) {
> -                     i = 0;
> -                     bootcpu_valid = true;
> -             } else {
> -                     i = cpuidx++;
> +             if (hwid32 == cpu_logical_map(0)) {
> +                     if (bootcpu_valid) {
> +                             pr_warn(" * %s node duplicate boot cpu reg 
> property, skipped\n",
> +                                     cpu->full_name);
> +                     } else {
> +                             bootcpu_valid = true;
> +                     }
> +                     goto next;
>               }
> +             /*
> +              * Build cpu_logical_map array. Numbering scheme
> +              * requires that boot CPU is assigned logical id 0.
> +              * Other CPUs get sequential indexes starting from 1.
> +              */
> +             i = cpuidx++;
>  
> -             if (WARN(cpuidx > nr_cpu_ids, "DT /cpu %u nodes greater than "
> -                                            "max cores %u, capping them\n",
> -                                            cpuidx, nr_cpu_ids)) {
> +             if (cpuidx > nr_cpu_ids) {
> +                     pr_warn_once("DT cpu %u nodes greater than max cores 
> %u, capping them\n",
> +                             cpuidx, nr_cpu_ids);
>                       cpuidx = nr_cpu_ids;
> -                     break;
> +                     goto next;
>               }
>  
> -             tmp_map[i] = hwid;
> +             cpu_logical_map(i) = hwid32;
> +             set_cpu_possible(i, true);
> +             pr_debug("cpu_logical_map(%u) 0x%x\n", i, cpu_logical_map(i));
> +next:        ;
>       }
> -
> -     if (WARN(!bootcpu_valid, "DT missing boot CPU MPIDR[23:0], "
> -                              "fall back to default cpu_logical_map\n"))
> -             return;
> +     /*
> +      * A DT missing the boot CPU MPIDR is a really bad omen
> +      * Flag it up as such.
> +      */
> +     if (!bootcpu_valid)
> +             pr_warn("DT missing boot cpu node\n");
>  
>       /*
> -      * Since the boot CPU node contains proper data, and all nodes have
> -      * a reg property, the DT CPU list can be considered valid and the
> -      * logical map created in smp_setup_processor_id() can be overridden
> +      * Since the DT might contain fewer entries than NR_CPUS,
> +      * cpu_logical_map entries initialized in smp_setup_processor_id()
> +      * but not found in the DT must be overriden with MPIDR_INVALID
> +      * values to make sure the cpu_logical_map does not contain stale
> +      * MPIDR values.
>        */
> -     for (i = 0; i < cpuidx; i++) {
> -             set_cpu_possible(i, true);
> -             cpu_logical_map(i) = tmp_map[i];
> -             pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
> -     }
> +     for (i = cpuidx; i < nr_cpu_ids; i++)
> +             cpu_logical_map(i) = MPIDR_INVALID;
>  }
>  
>  /**
> -- 
> 1.8.2.2
> 
> 
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss

Reply via email to