ARM: multi-cluster aware boot protocol
Subsystems (2)
cover ARM: multi-cluster aware boot protocol
This series is an updated version of a previous posting available here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/080868.html
The introduction of multi-cluster ARM systems in SoC designs requires the kernel to become cluster aware, so that it can be booted on every CPU in the system and it can build an appropriate HW-to-logical cpu map.
Current code in the kernel, in particular the boot sequence, hinges upon a sequential mapping of MPIDR values for cpus and related interrupt controller CPU interfaces to logical cpu indexing.
This hypothesis is not valid when the concept of cluster is introduced since the MPIDR cannot be represented as a single index and interrupt controller CPU interfaces can be wired with a numbering scheme following per-SoC design parameters which can be only detected through probing or device tree representation.
Through the device tree and "cpu" nodes bindings, the kernel is provided with HW values for MPIDR registers that allow the kernel to identify the HW CPU ids that are present in the platform.
The GIC code has been extended to allow automatic detection of GIC CPU IF ids at boot. IPIs are broadcast to all possible CPUs, and every time a secondary CPU is booted, it initializes its own mask and clears itself from the mask of all other logical CPUs.
The device tree bindings and GIC probing allow to boot the Linux kernel on any CPU of a multi-cluster system without relying on a platform specific hook to identify the number of CPUs and hypothesis on the sequential pattern of MPIDRs and relative GIC CPU IF ids.
Pen release code for all converted platforms will need patching to extend the current MPIDR range check; this will be done as soon as the bindings and code for the multi-cluster boot protocol will be reviewed and accepted.
The patchset has been tested on:
- TC2 testchip
to boot a 5-core dual-cluster system on every possible CPU.
Lorenzo Pieralisi (3):
ARM: kernel: add device tree init map function
ARM: kernel: add cpu logical map DT init in setup_arch
ARM: kernel: add logical mappings look-up
Nicolas Pitre (1):
ARM: gic: use a private mapping for CPU target interfaces
Documentation/devicetree/bindings/arm/cpus.txt | 42 +++++++++++++++++++++++ arch/arm/common/gic.c | 42 ++++++++++++++++++----- arch/arm/include/asm/prom.h | 2 ++ arch/arm/include/asm/smp_plat.h | 17 ++++++++++ arch/arm/kernel/devtree.c | 47 ++++++++++++++++++++++++++ arch/arm/kernel/setup.c | 1 + 6 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/cpus.txt
Signature · 1 line
1.7.12
RFC PATCH 1/4 ARM: kernel: add device tree init map function
When booting through a device tree, the kernel cpu logical id map can be initialized using device tree data passed by FW or through an embedded blob.
This patch adds a function that parses device tree "cpu" nodes and retrieves the corresponding CPUs hardware identifiers (MPIDR). It sets the possible cpus and the cpu logical map values according to the number of CPUs defined in the device tree and respective properties.
The primary CPU is assigned cpu logical number 0 to keep the current convention valid.
Current bindings documentation is included in the patch:
Documentation/devicetree/bindings/arm/cpus.txt
--- Documentation/devicetree/bindings/arm/cpus.txt | 42 +++++++++++++++++++++++ arch/arm/include/asm/prom.h | 2 ++ arch/arm/kernel/devtree.c | 47 ++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/cpus.txt
Documentation/devicetree/bindings/arm/cpus.txt new file +42 -0
@@ -0,0 +1,42 @@ +* ARM CPUs binding description + +The device tree allows to describe the layout of CPUs in a system through +the "cpus" node, which in turn contains a number of subnodes (ie "cpu") +defining properties for every cpu. + +Bindings for CPU nodes follow the ePAPR standard, available from: + +http://devicetree.org + +For the ARM architecture every CPU node must contain the following property: + +- reg : property defining the CPU MPIDR[23:0] register bits + +Every cpu node is required to set its device_type to "cpu". + +Example: + + cpus { + #size-cells = <0>; + #address-cells = <1>; + + CPU0: cpu at 0 { + device_type = "cpu"; + reg = <0x0>; + }; + + CPU1: cpu at 1 { + device_type = "cpu"; + reg = <0x1>; + }; + + CPU2: cpu at 100 { + device_type = "cpu"; + reg = <0x100>; + }; + + CPU3: cpu at 101 { + device_type = "cpu"; + reg = <0x101>; + }; + };
arch/arm/include/asm/prom.h +2 -0
@@ -15,6 +15,7 @@extern struct machine_desc *setup_machine_fdt (unsigned int dt_phys);extern void arm_dt_memblock_reserve (void ); +extern void __init arm_dt_init_cpu_maps (void );#else /* CONFIG_OF */
@@ -24,6 +25,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys) }static inline void arm_dt_memblock_reserve (void ) { } +static inline void arm_dt_init_cpu_maps (void ) { }#endif /* CONFIG_OF */ #endif /* ASMARM_PROM_H */
arch/arm/kernel/devtree.c +47 -0
61 lines not on this page · raw message
Signature · 1 line
1.7.12
RFC PATCH 1/4 ARM: kernel: add device tree init map function
Quoted 32 lines · Lorenzo Pieralisi
When booting through a device tree, the kernel cpu logical id map can be initialized using device tree data passed by FW or through an embedded blob.
This patch adds a function that parses device tree "cpu" nodes and retrieves the corresponding CPUs hardware identifiers (MPIDR). It sets the possible cpus and the cpu logical map values according to the number of CPUs defined in the device tree and respective properties.
The primary CPU is assigned cpu logical number 0 to keep the current convention valid.
Current bindings documentation is included in the patch:
Documentation/devicetree/bindings/arm/cpus.txtSigned-off-by Lorenzo Pieralisi <[email protected]>--- Documentation/devicetree/bindings/arm/cpus.txt | 42 +++++++++++++++++++++++ arch/arm/include/asm/prom.h | 2 ++ arch/arm/kernel/devtree.c | 47 ++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/cpus.txtDocumentation/devicetree/bindings/arm/cpus.txt new file +3
-0@@ -0,0 +1,42 @@ +* ARM CPUs binding description + +The device tree allows to describe the layout of CPUs in a system through
Documentation/devicetree/bindings/arm/cpus.txt line 13
Quoted 10 lines
Documentation/devicetree/bindings/arm/cpus.txt new file +10
-0+the "cpus" node, which in turn contains a number of subnodes (ie "cpu") +defining properties for every cpu. + +Bindings for CPU nodes follow the ePAPR standard, available from: + +http://devicetree.org + +For the ARM architecture every CPU node must contain the following property: + +- reg : property defining the CPU MPIDR[23:0] register bits
defining or matching the MPIDR?
Documentation/devicetree/bindings/arm/cpus.txt line 15
Quoted 2 lines
Documentation/devicetree/bindings/arm/cpus.txt new file +2
-0+ +Every cpu node is required to set its device_type to "cpu".
This is a bit questionable as device_type is deprecated for FDT. However, since the ePAPR defines using it
You should add a compatible property for the cpu model.
Quoted 69 lines
Documentation/devicetree/bindings/arm/cpus.txt new file +27
-0+ +Example: + + cpus { + #size-cells = <0>; + #address-cells = <1>; + + CPU0: cpu at 0 { + device_type = "cpu"; + reg = <0x0>; + }; + + CPU1: cpu at 1 { + device_type = "cpu"; + reg = <0x1>; + }; + + CPU2: cpu at 100 { + device_type = "cpu"; + reg = <0x100>; + }; + + CPU3: cpu at 101 { + device_type = "cpu"; + reg = <0x101>; + }; + };arch/arm/include/asm/prom.h +2
-0@@ -15,6 +15,7 @@extern struct machine_desc *setup_machine_fdt (unsigned int dt_phys);extern void arm_dt_memblock_reserve (void ); +extern void __init arm_dt_init_cpu_maps (void );#else /* CONFIG_OF */ @@ -24,6 +25,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys) }static inline void arm_dt_memblock_reserve (void ) { } +static inline void arm_dt_init_cpu_maps (void ) { }#endif /* CONFIG_OF */ #endif /* ASMARM_PROM_H */ arch/arm/kernel/devtree.c +5
-0@@ -19,8 +19,10 @@#include <linux/of_irq.h> #include <linux/of_platform.h> +#include <asm/cputype.h> #include <asm/setup.h> #include <asm/page.h> +#include <asm/smp_plat.h> #include <asm/mach/arch.h> #include <asm/mach-types.h> @@ -61,6 +63,51 @@ void __init arm_dt_memblock_reserve(void) } } +/* +* arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree +* and builds the cpu logical map array containing MPIDR values related to
arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 78
Quoted 10 lines
arch/arm/kernel/devtree.c +10
-0+* logical cpus +* +* Updates the cpu possible mask with the number of parsed cpu nodes +*/ +void __init arm_dt_init_cpu_maps (void ) +{ +struct device_node *dn =NULL ; +int i, cpu =1 ; + +while ((dn =of_find_node_by_type (dn,"cpu" )) && cpu <= nr_cpu_ids) {
I think all /cpu nodes would have the right type. You could use for_each_child_of_node here.
arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 85
Quoted 7 lines
arch/arm/kernel/devtree.c +7
-0+const u32 *hwid; +int len; + +pr_debug (" * %s...\n" , dn->full_name); + + hwid =of_get_property (dn,"reg" , &len); +
Use of_property_read_u32.
Quoted 29 lines
arch/arm/kernel/devtree.c +25
-0+if (!hwid || len !=4 ) { +pr_err (" * %s missing reg property\n" , dn->full_name); +continue ; + } +/* +* We want to assign the boot CPU logical id 0. +* Boot CPU checks its own MPIDR and if matches the current +* cpu node "reg" value it sets the logical cpu index to 0 +* and stores the physical id accordingly. +* If MPIDR does not match, assign sequential cpu logical +* id (starting from 1) and increments it. +*/ + i = (be32_to_cpup (hwid) == (read_cpuid_mpidr () &0xffffff )) + ?0 : cpu++; + +if (!i) +printk (KERN_INFO "Booting Linux on CPU HWID 0x%x\n" , +be32_to_cpup (hwid)); + +cpu_logical_map (i) =be32_to_cpup (hwid); + +set_cpu_possible (i, true); + } +} +/** * setup_machine_fdt - Machine setup when an dtb was passed to the kernel * @dt_phys: physical address of dt blob
RFC PATCH 1/4 ARM: kernel: add device tree init map function
Hi Rob,
thanks for having a look.
Quoted 37 lines · Rob Herring
Quoted 36 lines · Lorenzo Pieralisi
When booting through a device tree, the kernel cpu logical id map can be initialized using device tree data passed by FW or through an embedded blob.
This patch adds a function that parses device tree "cpu" nodes and retrieves the corresponding CPUs hardware identifiers (MPIDR). It sets the possible cpus and the cpu logical map values according to the number of CPUs defined in the device tree and respective properties.
The primary CPU is assigned cpu logical number 0 to keep the current convention valid.
Current bindings documentation is included in the patch:
Documentation/devicetree/bindings/arm/cpus.txtSigned-off-by Lorenzo Pieralisi <[email protected]>--- Documentation/devicetree/bindings/arm/cpus.txt | 42 +++++++++++++++++++++++ arch/arm/include/asm/prom.h | 2 ++ arch/arm/kernel/devtree.c | 47 ++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/cpus.txtDocumentation/devicetree/bindings/arm/cpus.txt new file +7
-0@@ -0,0 +1,42 @@ +* ARM CPUs binding description + +The device tree allows to describe the layout of CPUs in a system through +the "cpus" node, which in turn contains a number of subnodes (ie "cpu") +defining properties for every cpu. + +Bindings for CPU nodes follow the ePAPR standard, available from:
Quoted 8 lines
Documentation/devicetree/bindings/arm/cpus.txt line 13
Quoted 6 lines
Documentation/devicetree/bindings/arm/cpus.txt new file +6
-0+ +http://devicetree.org + +For the ARM architecture every CPU node must contain the following property: + +- reg : property defining the CPU MPIDR[23:0] register bitsdefining or matching the MPIDR?
I would say matching, otherwise it reads as if the MPIDR were writable.
Quoted 7 lines
Documentation/devicetree/bindings/arm/cpus.txt line 15
Quoted 2 lines
Documentation/devicetree/bindings/arm/cpus.txt new file +2
-0+ +Every cpu node is required to set its device_type to "cpu".This is a bit questionable as
device_typeis deprecated for FDT. However, since the ePAPR defines using itYou should add a compatible property for the cpu model.
Ok, I will.
Quoted 66 lines
Quoted 66 lines
Documentation/devicetree/bindings/arm/cpus.txt new file +27
-0+ +Example: + + cpus { + #size-cells = <0>; + #address-cells = <1>; + + CPU0: cpu at 0 { + device_type = "cpu"; + reg = <0x0>; + }; + + CPU1: cpu at 1 { + device_type = "cpu"; + reg = <0x1>; + }; + + CPU2: cpu at 100 { + device_type = "cpu"; + reg = <0x100>; + }; + + CPU3: cpu at 101 { + device_type = "cpu"; + reg = <0x101>; + }; + };arch/arm/include/asm/prom.h +2
-0@@ -15,6 +15,7 @@extern struct machine_desc *setup_machine_fdt (unsigned int dt_phys);extern void arm_dt_memblock_reserve (void ); +extern void __init arm_dt_init_cpu_maps (void );#else /* CONFIG_OF */ @@ -24,6 +25,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys) }static inline void arm_dt_memblock_reserve (void ) { } +static inline void arm_dt_init_cpu_maps (void ) { }#endif /* CONFIG_OF */ #endif /* ASMARM_PROM_H */ arch/arm/kernel/devtree.c +2
-0@@ -19,8 +19,10 @@#include <linux/of_irq.h> #include <linux/of_platform.h> +#include <asm/cputype.h> #include <asm/setup.h> #include <asm/page.h> +#include <asm/smp_plat.h> #include <asm/mach/arch.h> #include <asm/mach-types.h> @@ -61,6 +63,51 @@ void __init arm_dt_memblock_reserve(void) } }
Quoted 16 lines
arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 78
Quoted 13 lines
arch/arm/kernel/devtree.c +13
-0+/* +* arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree +* and builds the cpu logical map array containing MPIDR values related to +* logical cpus +* +* Updates the cpu possible mask with the number of parsed cpu nodes +*/ +void __init arm_dt_init_cpu_maps (void ) +{ +struct device_node *dn =NULL ; +int i, cpu =1 ; + +while ((dn =of_find_node_by_type (dn,"cpu" )) && cpu <= nr_cpu_ids) {I think all /cpu nodes would have the right type. You could use
for_each_child_of_nodehere.
Starting from /cpus, looked-up by path, right ?
Quoted 9 lines
arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 85
Quoted 7 lines
arch/arm/kernel/devtree.c +7
-0+const u32 *hwid; +int len; + +pr_debug (" * %s...\n" , dn->full_name); + + hwid =of_get_property (dn,"reg" , &len); +Use
of_property_read_u32.
Ok.
Thanks,
Lorenzo
RFC PATCH 1/4 ARM: kernel: add device tree init map function
In reply to Lorenzo PieralisiQuoted 16 lines · Lorenzo Pieralisi
When booting through a device tree, the kernel cpu logical id map can be initialized using device tree data passed by FW or through an embedded blob.
This patch adds a function that parses device tree "cpu" nodes and retrieves the corresponding CPUs hardware identifiers (MPIDR). It sets the possible cpus and the cpu logical map values according to the number of CPUs defined in the device tree and respective properties.
The primary CPU is assigned cpu logical number 0 to keep the current convention valid.
Current bindings documentation is included in the patch:
Documentation/devicetree/bindings/arm/cpus.txtSigned-off-by Lorenzo Pieralisi <[email protected]>
[...]
Quoted 21 lines
arch/arm/kernel/devtree.c +21
-0+void __init arm_dt_init_cpu_maps (void ) +{ +struct device_node *dn =NULL ; +int i, cpu =1 ; + +while ((dn =of_find_node_by_type (dn,"cpu" )) && cpu <= nr_cpu_ids) { +const u32 *hwid; +int len; + +pr_debug (" * %s...\n" , dn->full_name); + + hwid =of_get_property (dn,"reg" , &len); + +if (!hwid || len !=4 ) { +pr_err (" * %s missing reg property\n" , dn->full_name); +continue ; + } +/* +* We want to assign the boot CPU logical id 0. +* Boot CPU checks its own MPIDR and if matches the current +* cpu node "reg" value it sets the logical cpu index to 0
arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 103
Quoted 10 lines
arch/arm/kernel/devtree.c +10
-0+* and stores the physical id accordingly. +* If MPIDR does not match, assign sequential cpu logical +* id (starting from 1) and increments it. +*/ + i = (be32_to_cpup (hwid) == (read_cpuid_mpidr () &0xffffff )) + ?0 : cpu++; + +if (!i) +printk (KERN_INFO "Booting Linux on CPU HWID 0x%x\n" , +be32_to_cpup (hwid));
I don't think we should bother with this print -- we already print something in smp_setup_processor_id, which cannot differ for the boot CPU, If you want the full mpidr, we could change that code to include it as well.
We might also want some sanity checking that we do indeed end up with logical id 0 for the boot CPU. If not, I think we should scream and fall back on the simple mapping created earlier.
Will
RFC PATCH 1/4 ARM: kernel: add device tree init map function
Hi Will,
thanks for the review on the series.
Quoted 47 lines · Will Deacon
Quoted 16 lines · Lorenzo Pieralisi
When booting through a device tree, the kernel cpu logical id map can be initialized using device tree data passed by FW or through an embedded blob.
This patch adds a function that parses device tree "cpu" nodes and retrieves the corresponding CPUs hardware identifiers (MPIDR). It sets the possible cpus and the cpu logical map values according to the number of CPUs defined in the device tree and respective properties.
The primary CPU is assigned cpu logical number 0 to keep the current convention valid.
Current bindings documentation is included in the patch:
Documentation/devicetree/bindings/arm/cpus.txtSigned-off-by Lorenzo Pieralisi <[email protected]>[...]
Quoted 27 lines
arch/arm/kernel/devtree.c +27
-0+void __init arm_dt_init_cpu_maps (void ) +{ +struct device_node *dn =NULL ; +int i, cpu =1 ; + +while ((dn =of_find_node_by_type (dn,"cpu" )) && cpu <= nr_cpu_ids) { +const u32 *hwid; +int len; + +pr_debug (" * %s...\n" , dn->full_name); + + hwid =of_get_property (dn,"reg" , &len); + +if (!hwid || len !=4 ) { +pr_err (" * %s missing reg property\n" , dn->full_name); +continue ; + } +/* +* We want to assign the boot CPU logical id 0. +* Boot CPU checks its own MPIDR and if matches the current +* cpu node "reg" value it sets the logical cpu index to 0 +* and stores the physical id accordingly. +* If MPIDR does not match, assign sequential cpu logical +* id (starting from 1) and increments it. +*/ + i = (be32_to_cpup (hwid) == (read_cpuid_mpidr () &0xffffff )) + ?0 : cpu++;
Quoted 8 lines
arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 103
Quoted 4 lines
arch/arm/kernel/devtree.c +4
-0+ +if (!i) +printk (KERN_INFO "Booting Linux on CPU HWID 0x%x\n" , +be32_to_cpup (hwid));I don't think we should bother with this print -- we already print something in
smp_setup_processor_id, which cannot differ for the boot CPU, If you want the full mpidr, we could change that code to include it as well.
Yes, it is duplicate code, I will remove it. Extending the printk in smp_setup_processor_id() to include the entire MPIDR should be done though, otherwise we might have printk aliases on system with multiple clusters.
Quoted 4 lines
We might also want some sanity checking that we do indeed end up with logical id 0 for the boot CPU. If not, I think we should scream and fall back on the simple mapping created earlier.
Well, this basically means that we have to make sure this function is executed on the boot CPU (and it is as the code stands now), right ?
Since we are reading the MPIDR of the CPU parsing the tree and assign logical cpu 0 accordingly I think we have this check carried out automatically, unless for any given reason someone calls this function on a CPU that is not the boot one (Why ?).
Basically I could add a check like:
if (WARN_ON(smp_processor_id())
return;
to fall back to the previous mapping, but that's overkill IMHO.
Thanks,
Lorenzo
RFC PATCH 1/4 ARM: kernel: add device tree init map function
Quoted 32 lines · Lorenzo Pieralisi
Quoted 22 lines · Will Deacon
Quoted 22 lines
arch/arm/kernel/devtree.c +22
-0+while ((dn =of_find_node_by_type (dn,"cpu" )) && cpu <= nr_cpu_ids) { +const u32 *hwid; +int len; + +pr_debug (" * %s...\n" , dn->full_name); + + hwid =of_get_property (dn,"reg" , &len); + +if (!hwid || len !=4 ) { +pr_err (" * %s missing reg property\n" , dn->full_name); +continue ; + } +/* +* We want to assign the boot CPU logical id 0. +* Boot CPU checks its own MPIDR and if matches the current +* cpu node "reg" value it sets the logical cpu index to 0 +* and stores the physical id accordingly. +* If MPIDR does not match, assign sequential cpu logical +* id (starting from 1) and increments it. +*/ + i = (be32_to_cpup (hwid) == (read_cpuid_mpidr () &0xffffff )) + ?0 : cpu++;Quoted 8 lines
arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 103
Quoted 4 lines
arch/arm/kernel/devtree.c +4
-0+ +if (!i) +printk (KERN_INFO "Booting Linux on CPU HWID 0x%x\n" , +be32_to_cpup (hwid));I don't think we should bother with this print -- we already print something in
smp_setup_processor_id, which cannot differ for the boot CPU, If you want the full mpidr, we could change that code to include it as well.
Quoted 4 lines
Yes, it is duplicate code, I will remove it. Extending the printk in
smp_setup_processor_id()to include the entire MPIDR should be done though, otherwise we might have printk aliases on system with multiple clusters.
Feel free to make that change. You could also replace NR_CPUS in smp_setup_processor_id with nr_cpu_ids for consistency (they'll be the same this early on).
Quoted 7 lines
Quoted 4 lines
We might also want some sanity checking that we do indeed end up with logical id 0 for the boot CPU. If not, I think we should scream and fall back on the simple mapping created earlier.
Well, this basically means that we have to make sure this function is executed on the boot CPU (and it is as the code stands now), right ?
Yes, smp is not up yet which is why we're allowed to play with the logical map.
Quoted 11 lines
Since we are reading the MPIDR of the CPU parsing the tree and assign logical cpu 0 accordingly I think we have this check carried out automatically, unless for any given reason someone calls this function on a CPU that is not the boot one (Why ?).
Basically I could add a check like:
if (WARN_ON(smp_processor_id())
return;to fall back to the previous mapping, but that's overkill IMHO.
No, I was thinking about what happens if the devicetree doesn't contain an mpidr property that matches the boot cpu. In this case, we will fail to assign logical ID 0, right? If this happens, we should complain about an invalid devicetree and try to fall back on the logical_map that was generated earlier on.
Will
RFC PATCH 1/4 ARM: kernel: add device tree init map function
Quoted 33 lines · Will Deacon
Quoted 32 lines · Lorenzo Pieralisi
Quoted 30 lines · Will Deacon
Quoted 16 lines
arch/arm/kernel/devtree.c +16
-0+while ((dn =of_find_node_by_type (dn,"cpu" )) && cpu <= nr_cpu_ids) { +const u32 *hwid; +int len; + +pr_debug (" * %s...\n" , dn->full_name); + + hwid =of_get_property (dn,"reg" , &len); + +if (!hwid || len !=4 ) { +pr_err (" * %s missing reg property\n" , dn->full_name); +continue ; + } +/* +* We want to assign the boot CPU logical id 0. +* Boot CPU checks its own MPIDR and if matches the current +* cpu node "reg" value it sets the logical cpu index to 0 arch/arm/kernel/devtree.c void __init arm_dt_memblock_reserve(void) · line 103
Quoted 10 lines
arch/arm/kernel/devtree.c +10
-0+* and stores the physical id accordingly. +* If MPIDR does not match, assign sequential cpu logical +* id (starting from 1) and increments it. +*/ + i = (be32_to_cpup (hwid) == (read_cpuid_mpidr () &0xffffff )) + ?0 : cpu++; + +if (!i) +printk (KERN_INFO "Booting Linux on CPU HWID 0x%x\n" , +be32_to_cpup (hwid));I don't think we should bother with this print -- we already print something in
smp_setup_processor_id, which cannot differ for the boot CPU, If you want the full mpidr, we could change that code to include it as well.
Quoted 8 lines
Quoted 4 lines
Yes, it is duplicate code, I will remove it. Extending the printk in
smp_setup_processor_id()to include the entire MPIDR should be done though, otherwise we might have printk aliases on system with multiple clusters.Feel free to make that change. You could also replace
NR_CPUSinsmp_setup_processor_idwithnr_cpu_idsfor consistency (they'll be the same this early on).
Ok.
Quoted 20 lines
Quoted 6 lines
Quoted 3 lines
We might also want some sanity checking that we do indeed end up with logical id 0 for the boot CPU. If not, I think we should scream and fall back on the simple mapping created earlier.
Well, this basically means that we have to make sure this function is executed on the boot CPU (and it is as the code stands now), right ?
Yes, smp is not up yet which is why we're allowed to play with the logical map.
Quoted 10 lines
Since we are reading the MPIDR of the CPU parsing the tree and assign logical cpu 0 accordingly I think we have this check carried out automatically, unless for any given reason someone calls this function on a CPU that is not the boot one (Why ?).
Basically I could add a check like:
if (WARN_ON(smp_processor_id())
return;
Quoted 7 lines
Quoted 1 line
to fall back to the previous mapping, but that's overkill IMHO.
No, I was thinking about what happens if the devicetree doesn't contain an mpidr property that matches the boot cpu. In this case, we will fail to assign logical ID 0, right? If this happens, we should complain about an invalid devicetree and try to fall back on the
logical_mapthat was generated earlier on.
Good point. What I could do, I can assign the MPIDR of the boot CPU to the logical index 0 before even starting to parse the DT (that's what it is done in smp_setup_processor_id(), with a couple of twists). Then, if I find a node that matches the boot CPU mpidr I just skip over it. This way the boot CPU MPIDR will always be correct the only difference with the current approach will be that instead of generating the secondaries MPIDRs we will read them from DT.
The problem with this approach is that if we need a pointer (phandle) to the boot CPU DT node through the MPIDR and the boot CPU node is botched or missing we still behave as if the DT CPU nodes were ok.
I think I'd better stick a warning condition in there if the boot CPU node is not present or botched (from a MPIDR perspective at least).
Thoughts ?
Lorenzo
RFC PATCH 1/4 ARM: kernel: add device tree init map function
Quoted 7 lines · Lorenzo Pieralisi
Quoted 5 lines · Will Deacon
No, I was thinking about what happens if the devicetree doesn't contain an mpidr property that matches the boot cpu. In this case, we will fail to assign logical ID 0, right? If this happens, we should complain about an invalid devicetree and try to fall back on the
logical_mapthat was generated earlier on.
Quoted 7 lines
Good point. What I could do, I can assign the MPIDR of the boot CPU to the logical index 0 before even starting to parse the DT (that's what it is done in
smp_setup_processor_id(), with a couple of twists). Then, if I find a node that matches the boot CPU mpidr I just skip over it. This way the boot CPU MPIDR will always be correct the only difference with the current approach will be that instead of generating the secondaries MPIDRs we will read them from DT.
That should work, although I'm not sure why you can't just give up altogether and use the initial mapping from smp_setup_processor_id?
Quoted 3 lines
The problem with this approach is that if we need a pointer (phandle) to the boot CPU DT node through the MPIDR and the boot CPU node is botched or missing we still behave as if the DT CPU nodes were ok.
Does any code do this? Wouldn't it be much better to lookup logical CPU 0 if you want to find anything out about the boot CPU?
Quoted 2 lines
I think I'd better stick a warning condition in there if the boot CPU node is not present or botched (from a MPIDR perspective at least).
Definitely!
Will
RFC PATCH 1/4 ARM: kernel: add device tree init map function
Quoted 16 lines · Will Deacon
Quoted 7 lines · Lorenzo Pieralisi
Quoted 5 lines · Will Deacon
No, I was thinking about what happens if the devicetree doesn't contain an mpidr property that matches the boot cpu. In this case, we will fail to assign logical ID 0, right? If this happens, we should complain about an invalid devicetree and try to fall back on the
logical_mapthat was generated earlier on.Quoted 7 lines
Good point. What I could do, I can assign the MPIDR of the boot CPU to the logical index 0 before even starting to parse the DT (that's what it is done in
smp_setup_processor_id(), with a couple of twists). Then, if I find a node that matches the boot CPU mpidr I just skip over it. This way the boot CPU MPIDR will always be correct the only difference with the current approach will be that instead of generating the secondaries MPIDRs we will read them from DT.
Quoted 2 lines
That should work, although I'm not sure why you can't just give up altogether and use the initial mapping from
smp_setup_processor_id?
Since I need to either stash the values to avoid reparsing the tree or at first I just parse to check for correctness, second pass I update the map.
I will stash the reg values, and if the boot CPU MPIDR is correct I will copy the stashed map to the cpu_logical_map. If that's unacceptable we will change it.
Quoted 6 lines
Quoted 3 lines
The problem with this approach is that if we need a pointer (phandle) to the boot CPU DT node through the MPIDR and the boot CPU node is botched or missing we still behave as if the DT CPU nodes were ok.
Does any code do this? Wouldn't it be much better to lookup logical CPU 0 if you want to find anything out about the boot CPU?
Formulated my point in a horrible way sorry.
In order to retrieve the logical id of a CPU (eg IRQ affinity list) we need its MPIDR for the reverse look-up and for that to work the reg property in the /cpu nodes must be correct. Let's gloss over this for now.
Quoted 2 lines
Quoted 2 lines
I think I'd better stick a warning condition in there if the boot CPU node is not present or botched (from a MPIDR perspective at least).
Done, IMHO I wrote some code that is too convoluted, I will post it anyway for review to get further feeback.
Thanks !!
Lorenzo
RFC PATCH 2/4 ARM: kernel: add cpu logical map DT init in setup_arch
In reply to Lorenzo PieralisiAs soon as the device tree is unflattened the cpu logical to physical mapping is carried out in setup_arch to build a proper array of MPIDR and corresponding logical indexes.
The mapping could have been carried out using the flattened DT blob and related primitives, but since the mapping is not needed by early boot code it can safely be executed when the device tree has been uncompressed to its tree data structure.
This patch adds the arm_dt_init_cpu maps() function call in setup_arch().
If the kernel is not compiled with DT support the function is empty and no logical mapping takes place through it; the mapping carried out in smp_setup_processor_id() is left unchanged.
If DT is supported the mapping created in smp_setup_processor_id() is overriden. The DT mapping also sets the possible cpus mask, hence platform code need not set it again in the respective smp_init_cpus() functions.
--- arch/arm/kernel/setup.c | 1 + 1 file changed, 1 insertion(+)
arch/arm/kernel/setup.c +1 -0
@@ -758,6 +758,7 @@ void __init setup_arch(char **cmdline_p)unflatten_device_tree (); +arm_dt_init_cpu_maps ();#ifdef CONFIG_SMP if (is_smp ()) {smp_set_ops (mdesc->smp);
Signature · 1 line
1.7.12
RFC PATCH 2/4 ARM: kernel: add cpu logical map DT init in setup_arch
Quoted 37 lines · Lorenzo Pieralisi
As soon as the device tree is unflattened the cpu logical to physical mapping is carried out in
setup_archto build a proper array of MPIDR and corresponding logical indexes.The mapping could have been carried out using the flattened DT blob and related primitives, but since the mapping is not needed by early boot code it can safely be executed when the device tree has been uncompressed to its tree data structure.
This patch adds the
arm_dt_init_cpumaps()function call insetup_arch().If the kernel is not compiled with DT support the function is empty and no logical mapping takes place through it; the mapping carried out in
smp_setup_processor_id()is left unchanged.
If DT is supported the mapping created insmp_setup_processor_id()is overriden. The DT mapping also sets the possible cpus mask, hence platform code need not set it again in the respectivesmp_init_cpus()functions.Signed-off-by Lorenzo Pieralisi <[email protected]>--- arch/arm/kernel/setup.c | 1 + 1 file changed, 1 insertion(+)arch/arm/kernel/setup.c +1
-0@@ -758,6 +758,7 @@ void __init setup_arch(char **cmdline_p)unflatten_device_tree (); +arm_dt_init_cpu_maps ();#ifdef CONFIG_SMP if (is_smp ()) {smp_set_ops (mdesc->smp);Signature · 1 line
1.7.12
Will
RFC PATCH 3/4 ARM: kernel: add logical mappings look-up
In reply to Lorenzo PieralisiIn ARM SMP systems the MPIDR register ([23:0] bits) is used to uniquely identify CPUs.
In order to retrieve the logical CPU index corresponding to a given MPIDR value and guarantee a consistent translation throughout the kernel, this patch adds a look-up based on the MPIDR[23:0] so that kernel subsystems can use it whenever the logical cpu index corresponding to a given MPIDR value is needed.
--- arch/arm/include/asm/smp_plat.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
arch/arm/include/asm/smp_plat.h +17 -0
@@ -5,6 +5,9 @@#ifndef __ASMARM_SMP_PLAT_H#define __ASMARM_SMP_PLAT_H +#include <linux/cpumask.h> +#include <linux/err.h> +#include <asm/cputype.h> /*
@@ -48,5 +51,19 @@ static inline int cache_ops_need_broadcast(void)*/ extern int __cpu_logical_map[];#define cpu_logical_map (cpu) __cpu_logical_map[cpu] +/* +* Retrieve logical cpu index corresponding to a given MPIDR[23:0] +* - mpidr: MPIDR[23:0] to be used for the look-up +* +* Returns the cpu logical index or -EINVAL on look-up error +*/ +static inline int get_logical_index (u32 mpidr) +{ +int cpu; +for (cpu =0 ; cpu < nr_cpu_ids; cpu++) +if (cpu_logical_map (cpu) == mpidr) +return cpu; +return -EINVAL ; +}#endif
Signature · 1 line
1.7.12
RFC PATCH 3/4 ARM: kernel: add logical mappings look-up
Quoted 38 lines · Lorenzo Pieralisi
In ARM SMP systems the MPIDR register ([23:0] bits) is used to uniquely identify CPUs.
In order to retrieve the logical CPU index corresponding to a given MPIDR value and guarantee a consistent translation throughout the kernel, this patch adds a look-up based on the MPIDR[23:0] so that kernel subsystems can use it whenever the logical cpu index corresponding to a given MPIDR value is needed.
Signed-off-by Lorenzo Pieralisi <[email protected]>--- arch/arm/include/asm/smp_plat.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)arch/arm/include/asm/smp_plat.h +9
-0@@ -5,6 +5,9 @@#ifndef __ASMARM_SMP_PLAT_H#define __ASMARM_SMP_PLAT_H +#include <linux/cpumask.h> +#include <linux/err.h> +#include <asm/cputype.h> /* @@ -48,5 +51,19 @@ static inline int cache_ops_need_broadcast(void)*/ extern int __cpu_logical_map[];#define cpu_logical_map (cpu) __cpu_logical_map[cpu] +/* +* Retrieve logical cpu index corresponding to a given MPIDR[23:0] +* - mpidr: MPIDR[23:0] to be used for the look-up +* +* Returns the cpu logical index or -EINVAL on look-up error +*/
arch/arm/include/asm/smp_plat.h static inline int cache_ops_need_broadcast(void) · line 69
Quoted 10 lines
arch/arm/include/asm/smp_plat.h +8
-0+static inline int get_logical_index (u32 mpidr) +{ +int cpu; +for (cpu =0 ; cpu < nr_cpu_ids; cpu++) +if (cpu_logical_map (cpu) == mpidr) +return cpu; +return -EINVAL ; +}#endif
Will
RFC PATCH 4/4 ARM: gic: use a private mapping for CPU target interfaces
In reply to Lorenzo PieralisiFrom: Nicolas Pitre <[email protected]>
The GIC interface numbering does not necessarily follow the logical CPU numbering, especially for complex topologies such as multi-cluster systems.
Fortunately we can easily probe the GIC to create a mapping as the Interrupt Processor Targets Registers for the first 32 interrupts are read-only, and each field returns a value that always corresponds to the processor reading the register.
Initially all mappings target all CPUs in case an IPI is required to boot secondary CPUs. It is refined as those CPUs discover what their actual mapping is.
--- arch/arm/common/gic.c | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-)
arch/arm/common/gic.c +34 -8
@@ -70,6 +70,13 @@ struct gic_chip_data {static DEFINE_RAW_SPINLOCK (irq_controller_lock);/* +* The GIC mapping of CPU interfaces does not necessarily match +* the logical CPU numbering. Let's use a mapping as returned +* by the GIC itself. +*/ +static u8 gic_cpu_map[8 ]__read_mostly ; + +/* * Supported arch specific GIC irq extension. * Default make them NULL. */
@@ -242,7 +249,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,return -EINVAL ; mask =0xff << shift; - bit =1 << (cpu_logical_map (cpu) + shift); + bit = gic_cpu_map[cpu] << shift;raw_spin_lock (&irq_controller_lock); val =readl_relaxed (reg) & ~mask;
@@ -349,11 +356,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic)u32 cpumask;unsigned int gic_irqs = gic->gic_irqs;void __iomem *base =gic_data_dist_base (gic); -u32 cpu =cpu_logical_map (smp_processor_id ()); - - cpumask =1 << cpu; - cpumask |= cpumask <<8 ; - cpumask |= cpumask <<16 ;writel_relaxed (0 , base +GIC_DIST_CTRL );
@@ -366,6 +368,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic)/* * Set all global interrupts to this CPU only. */ + cpumask =readl_relaxed (base +GIC_DIST_TARGET +0 );for (i =32 ; i < gic_irqs; i +=4 )writel_relaxed (cpumask, base +GIC_DIST_TARGET + i *4 /4 );
@@ -389,9 +392,25 @@ static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) {void __iomem *dist_base =gic_data_dist_base (gic);void __iomem *base =gic_data_cpu_base (gic); +unsigned int cpu_mask, cpu =smp_processor_id ();int i;/* +* Get what the GIC says our CPU mask is. +*/ +BUG_ON (cpu >=8 ); + cpu_mask =readl_relaxed (dist_base +GIC_DIST_TARGET +0 ); + gic_cpu_map[cpu] = cpu_mask; + +/* +* Clear our mask from the other map entries in case they're +* still undefined. +*/ +for (i =0 ; i <8 ; i++) +if (i != cpu) + gic_cpu_map[i] &= ~cpu_mask; + +/* * Deal with the banked PPI and SGI interrupts - disable all * PPI interrupts, ensure all SGI interrupts are enabled. */
@@ -646,7 +665,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, {irq_hw_number_t hwirq_base;struct gic_chip_data *gic; -int gic_irqs, irq_base; +int gic_irqs, irq_base, i;BUG_ON (gic_nr >=MAX_GIC_NR );
@@ -683,6 +702,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, }/* +* Initialize the CPU interface map to all CPUs. +* It will be refined as each CPU probes its ID. +*/ +for (i =0 ; i <8 ; i++) + gic_cpu_map[i] =0xff ; + +/* * For primary GICs, skip over SGIs. * For secondary GICs, skip over PPIs, too. */
@@ -737,7 +763,7 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)/* Convert our logical CPU mask into a physical one. */ for_each_cpu (cpu, mask) - map |=1 <<cpu_logical_map (cpu); + map |= gic_cpu_map[cpu];/* * Ensure that stores to Normal memory are visible to the
Signature · 1 line
1.7.12
RFC PATCH 4/4 ARM: gic: use a private mapping for CPU target interfaces
Quoted 11 lines · Lorenzo Pieralisi
From: Nicolas Pitre <[email protected]>
The GIC interface numbering does not necessarily follow the logical CPU numbering, especially for complex topologies such as multi-cluster systems.
Fortunately we can easily probe the GIC to create a mapping as the Interrupt Processor Targets Registers for the first 32 interrupts are read-only, and each field returns a value that always corresponds to the processor reading the register.
Quoted 5 lines
Initially all mappings target all CPUs in case an IPI is required to boot secondary CPUs. It is refined as those CPUs discover what their actual mapping is.
Signed-off-by Nicolas Pitre <[email protected]>
This is a really neat idea!
Quoted 9 lines
--- arch/arm/common/gic.c | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-)arch/arm/common/gic.c +0
-0
arch/arm/common/gic.c struct gic_chip_data { · line 77
Quoted 8 lines
arch/arm/common/gic.c +5
-0@@ -70,6 +70,13 @@ struct gic_chip_data {static DEFINE_RAW_SPINLOCK (irq_controller_lock);/* +* The GIC mapping of CPU interfaces does not necessarily match +* the logical CPU numbering. Let's use a mapping as returned +* by the GIC itself. +*/ +static u8 gic_cpu_map[8 ]__read_mostly ;
Can we have a #define for the number CPUs supported by the GIC? It gets used a fair amount in this patch for loop bounds etc.
Quoted 35 lines
arch/arm/common/gic.c +3
-6+/* * Supported arch specific GIC irq extension. * Default make them NULL. */ @@ -242,7 +249,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,return -EINVAL ; mask =0xff << shift; - bit =1 << (cpu_logical_map (cpu) + shift); + bit = gic_cpu_map[cpu] << shift;raw_spin_lock (&irq_controller_lock); val =readl_relaxed (reg) & ~mask;@@ -349,11 +356,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic)u32 cpumask;unsigned int gic_irqs = gic->gic_irqs;void __iomem *base =gic_data_dist_base (gic); -u32 cpu =cpu_logical_map (smp_processor_id ()); - - cpumask =1 << cpu; - cpumask |= cpumask <<8 ; - cpumask |= cpumask <<16 ;writel_relaxed (0 , base +GIC_DIST_CTRL );@@ -366,6 +368,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic)/* * Set all global interrupts to this CPU only. */ + cpumask =readl_relaxed (base +GIC_DIST_TARGET +0 );for (i =32 ; i < gic_irqs; i +=4 )writel_relaxed (cpumask, base +GIC_DIST_TARGET + i *4 /4 );@@ -389,9 +392,25 @@ static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) {
arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 402
Quoted 10 lines
arch/arm/common/gic.c +5
-0void __iomem *dist_base =gic_data_dist_base (gic);void __iomem *base =gic_data_cpu_base (gic); +unsigned int cpu_mask, cpu =smp_processor_id ();int i;/* +* Get what the GIC says our CPU mask is. +*/ +BUG_ON (cpu >=8 ); + cpu_mask =readl_relaxed (dist_base +GIC_DIST_TARGET +0 );
Making the mask a u8 and using readb_relaxed here makes this bit of code clearer to me (and the GIC apparently allows such an access to this register).
Quoted 38 lines
arch/arm/common/gic.c +19
-1+ gic_cpu_map[cpu] = cpu_mask; + +/* +* Clear our mask from the other map entries in case they're +* still undefined. +*/ +for (i =0 ; i <8 ; i++) +if (i != cpu) + gic_cpu_map[i] &= ~cpu_mask; + +/* * Deal with the banked PPI and SGI interrupts - disable all * PPI interrupts, ensure all SGI interrupts are enabled. */ @@ -646,7 +665,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, {irq_hw_number_t hwirq_base;struct gic_chip_data *gic; -int gic_irqs, irq_base; +int gic_irqs, irq_base, i;BUG_ON (gic_nr >=MAX_GIC_NR );@@ -683,6 +702,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, }/* +* Initialize the CPU interface map to all CPUs. +* It will be refined as each CPU probes its ID. +*/ +for (i =0 ; i <8 ; i++) + gic_cpu_map[i] =0xff ; + +/* * For primary GICs, skip over SGIs. * For secondary GICs, skip over PPIs, too. */
arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) · line 769
Quoted 8 lines
arch/arm/common/gic.c +1
-1@@ -737,7 +763,7 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)/* Convert our logical CPU mask into a physical one. */ for_each_cpu (cpu, mask) - map |=1 <<cpu_logical_map (cpu); + map |= gic_cpu_map[cpu];/* * Ensure that stores to Normal memory are visible to the
With those changes:
Will
RFC PATCH 4/4 ARM: gic: use a private mapping for CPU target interfaces
Quoted 31 lines · Will Deacon
Quoted 11 lines · Lorenzo Pieralisi
From: Nicolas Pitre <[email protected]>
The GIC interface numbering does not necessarily follow the logical CPU numbering, especially for complex topologies such as multi-cluster systems.
Fortunately we can easily probe the GIC to create a mapping as the Interrupt Processor Targets Registers for the first 32 interrupts are read-only, and each field returns a value that always corresponds to the processor reading the register.
Quoted 5 lines
Initially all mappings target all CPUs in case an IPI is required to boot secondary CPUs. It is refined as those CPUs discover what their actual mapping is.
Signed-off-by Nicolas Pitre <[email protected]>This is a really neat idea!
Quoted 11 lines
--- arch/arm/common/gic.c | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-)arch/arm/common/gic.c +0
-0@@ -70,6 +70,13 @@ struct gic_chip_data {static DEFINE_RAW_SPINLOCK (irq_controller_lock);
Quoted 9 lines
arch/arm/common/gic.c struct gic_chip_data { · line 77
Quoted 6 lines
arch/arm/common/gic.c +5
-0/* +* The GIC mapping of CPU interfaces does not necessarily match +* the logical CPU numbering. Let's use a mapping as returned +* by the GIC itself. +*/ +static u8 gic_cpu_map[8 ]__read_mostly ;Can we have a #define for the number CPUs supported by the GIC? It gets used a fair amount in this patch for loop bounds etc.
Sure. I'll respin the patch.
Quoted 41 lines
Quoted 41 lines
arch/arm/common/gic.c +4
-6+/* * Supported arch specific GIC irq extension. * Default make them NULL. */ @@ -242,7 +249,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,return -EINVAL ; mask =0xff << shift; - bit =1 << (cpu_logical_map (cpu) + shift); + bit = gic_cpu_map[cpu] << shift;raw_spin_lock (&irq_controller_lock); val =readl_relaxed (reg) & ~mask;@@ -349,11 +356,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic)u32 cpumask;unsigned int gic_irqs = gic->gic_irqs;void __iomem *base =gic_data_dist_base (gic); -u32 cpu =cpu_logical_map (smp_processor_id ()); - - cpumask =1 << cpu; - cpumask |= cpumask <<8 ; - cpumask |= cpumask <<16 ;writel_relaxed (0 , base +GIC_DIST_CTRL );@@ -366,6 +368,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic)/* * Set all global interrupts to this CPU only. */ + cpumask =readl_relaxed (base +GIC_DIST_TARGET +0 );for (i =32 ; i < gic_irqs; i +=4 )writel_relaxed (cpumask, base +GIC_DIST_TARGET + i *4 /4 );@@ -389,9 +392,25 @@ static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) {void __iomem *dist_base =gic_data_dist_base (gic);void __iomem *base =gic_data_cpu_base (gic); +unsigned int cpu_mask, cpu =smp_processor_id ();int i;
Quoted 9 lines
arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 402
Quoted 5 lines
arch/arm/common/gic.c +4
-0/* +* Get what the GIC says our CPU mask is. +*/ +BUG_ON (cpu >=8 ); + cpu_mask =readl_relaxed (dist_base +GIC_DIST_TARGET +0 );Making the mask a u8 and using
readb_relaxedhere makes this bit of code clearer to me (and the GIC apparently allows such an access to this register).
Not always. At least RTSM throws an exception if you do so. Been there.
Quoted 53 lines
Quoted 38 lines
arch/arm/common/gic.c +19
-1+ gic_cpu_map[cpu] = cpu_mask; + +/* +* Clear our mask from the other map entries in case they're +* still undefined. +*/ +for (i =0 ; i <8 ; i++) +if (i != cpu) + gic_cpu_map[i] &= ~cpu_mask; + +/* * Deal with the banked PPI and SGI interrupts - disable all * PPI interrupts, ensure all SGI interrupts are enabled. */ @@ -646,7 +665,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, {irq_hw_number_t hwirq_base;struct gic_chip_data *gic; -int gic_irqs, irq_base; +int gic_irqs, irq_base, i;BUG_ON (gic_nr >=MAX_GIC_NR );@@ -683,6 +702,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, }/* +* Initialize the CPU interface map to all CPUs. +* It will be refined as each CPU probes its ID. +*/ +for (i =0 ; i <8 ; i++) + gic_cpu_map[i] =0xff ; + +/* * For primary GICs, skip over SGIs. * For secondary GICs, skip over PPIs, too. */ arch/arm/common/gic.c void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) · line 769
Quoted 8 lines
arch/arm/common/gic.c +1
-1@@ -737,7 +763,7 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)/* Convert our logical CPU mask into a physical one. */ for_each_cpu (cpu, mask) - map |=1 <<cpu_logical_map (cpu); + map |= gic_cpu_map[cpu];/* * Ensure that stores to Normal memory are visible to the With those changes:
Acked-by Will Deacon <[email protected]>Will
RFC PATCH 4/4 ARM: gic: use a private mapping for CPU target interfaces
Quoted 21 lines · Nicolas Pitre
Quoted 10 lines · Will Deacon
Quoted 10 lines
arch/arm/common/gic.c | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-)arch/arm/common/gic.c +0
-0@@ -70,6 +70,13 @@ struct gic_chip_data {static DEFINE_RAW_SPINLOCK (irq_controller_lock);Quoted 9 lines
arch/arm/common/gic.c struct gic_chip_data { · line 77
Quoted 6 lines
arch/arm/common/gic.c +5
-0/* +* The GIC mapping of CPU interfaces does not necessarily match +* the logical CPU numbering. Let's use a mapping as returned +* by the GIC itself. +*/ +static u8 gic_cpu_map[8 ]__read_mostly ;Can we have a #define for the number CPUs supported by the GIC? It gets used a fair amount in this patch for loop bounds etc.
Quoted 1 line
Sure. I'll respin the patch.
Cheers Nicolas.
Quoted 10 lines
Quoted 9 lines
arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 402
Quoted 5 lines
arch/arm/common/gic.c +4
-0/* +* Get what the GIC says our CPU mask is. +*/ +BUG_ON (cpu >=8 ); + cpu_mask =readl_relaxed (dist_base +GIC_DIST_TARGET +0 );Making the mask a u8 and using
readb_relaxedhere makes this bit of code clearer to me (and the GIC apparently allows such an access to this register).
Quoted 2 lines
Not always. At least RTSM throws an exception if you do so. Been there.
That would be a bug in the RTSM then. Have you reported it to support? (if not, I can chase this one up). I'd rather we just fix the model than work around it in Linux.
Will
RFC PATCH 4/4 ARM: gic: use a private mapping for CPU target interfaces
Quoted 12 lines · Will Deacon
Quoted 11 lines · Nicolas Pitre
Quoted 9 lines · Will Deacon
arch/arm/common/gic.c static void __cpuinit gic_cpu_init(struct gic_chip_data *gic) · line 402
Quoted 5 lines
arch/arm/common/gic.c +4
-0/* +* Get what the GIC says our CPU mask is. +*/ +BUG_ON (cpu >=8 ); + cpu_mask =readl_relaxed (dist_base +GIC_DIST_TARGET +0 );Making the mask a u8 and using
readb_relaxedhere makes this bit of code clearer to me (and the GIC apparently allows such an access to this register).
Quoted 6 lines
Quoted 2 lines
Not always. At least RTSM throws an exception if you do so. Been there.
That would be a bug in the RTSM then. Have you reported it to support? (if not, I can chase this one up). I'd rather we just fix the model than work around it in Linux.
I have no problem with you chasing it down with the support people.
I don't want to wait for fixed RTSM versions to be released and the whole world to migrate to them though.
While the readl is maybe marginally unintuitive compared to a readb here, the code is always using readl everywhere else already, even using bit masking and shifting when a readb/writeb could have made the code much simpler (see gic_set_affinity() for example). I therefore much prefer to stick to a proven 32-bit access than risking regression on some possible implementation where the 8-bit access wasn't properly implemented as the doc says it should and never exercised before.
In other words, I prefer erring on the safe side here.
Nicolas