Re: [PATCH v2] ARM: dt: check MPIDR on MP devices built without SMP

[Nicolas Saenz Julienne]

On Fri, 2019-10-04 at 17:52 +0200, Nicolas Saenz Julienne wrote:
> On SMP builds, in order to properly link CPU devices with their
> respective DT nodes we start by matching the boot CPU. This is achieved
> by comparing the 'reg' property on each of the CPU DT nodes with the
> MPIDR. The association is necessary as to validate the whole CPU logical
> map, which ultimately links CPU devices and their DT nodes.
> 
> On setups built without SMP, no MPIDR read is performed. The only thing
> expected is for the 'reg' property in the CPU DT node to contain the
> value 0x0.
> 
> This causes problems on MP setups built without SMP. As their boot CPU
> DT node contains the relevant MPIDR as opposed to 0x0. No match is then
> possible. This causes troubles further down the line as drivers are
> unable to get the CPU's DT node.
> 
> Change the way we choose whether to get the MPIDR or not. On builds
> without SMP check the number of CPUs defined in DT. Anything > 1 means
> the MPIDR should be available and matched accordingly.
> 
> Note that if there was a rogue UP device exposing multiple active CPUs
> in its DT the possible outcomes depend on the ARM series. For example
> Cortex-A9 specifies that the MPIDR is returns 0x0 on UP devices. On the
> other hand ARM1176JZ's TRM doesn't define a MPIDR register and specifies
> that any unwarranted register access will raise an undefined exception.
> Overall, given the bogus DT, a reasonable outcome.
> 
> This was originally seen on a Raspberry Pi 2 built with bcm2835_defconfig.
> 
> Reported-by: "kernelci.org bot" <bot@xxxxxxxxxxxx>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> Tested-by: Stefan Wahren <wahrenst@xxxxxxx>

ping :)

> ---
> 
> Changes since v1:
>   - Rewrite patch description
>   - Use of_get_available_child_count()
> 
> Note: kept Setfan's Tested-by as the changes only affect a corner case.
> 
>  arch/arm/kernel/devtree.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index 39c978698406..cd11742d9bc2 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -74,7 +74,7 @@ void __init arm_dt_init_cpu_maps(void)
>  	struct device_node *cpu, *cpus;
>  	int found_method = 0;
>  	u32 i, j, cpuidx = 1;
> -	u32 mpidr = is_smp() ? read_cpuid_mpidr() & MPIDR_HWID_BITMASK : 0;
> +	u32 mpidr = 0;
>  
>  	u32 tmp_map[NR_CPUS] = { [0 ... NR_CPUS-1] = MPIDR_INVALID };
>  	bool bootcpu_valid = false;
> @@ -83,6 +83,9 @@ void __init arm_dt_init_cpu_maps(void)
>  	if (!cpus)
>  		return;
>  
> +	if (is_smp() || of_get_available_child_count(cpus) > 1)
> +		mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
> +
>  	for_each_of_cpu_node(cpu) {
>  		const __be32 *cell;
>  		int prop_bytes;

Attachment: signature.asc
Description: This is a digitally signed message part