linux-kernel - Re: [PATCH] ARM: dt: check MPIDR on MP devices built without SMP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <17976e82-04da-d22d-5779-f50db63f98a2@gmail.com>
Date:   Thu, 3 Oct 2019 11:08:35 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        linux-kernel@...r.kernel.org
Cc:     "kernelci.org bot" <bot@...nelci.org>,
        linux-arm-kernel@...ts.infradead.org, wahrenst@....net,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH] ARM: dt: check MPIDR on MP devices built without SMP

On 10/2/19 4:45 AM, Nicolas Saenz Julienne wrote:
> Currently, in arm_dt_init_cpu_maps(), the hwid of the boot CPU is read
> from MPIDR on SMP devices and set to 0 for non SMP. This value is then
> matched with the DT cpu nodes' reg property in order to find the boot
> CPU in DT.

The code you change is about the "mpidr" variable, yet in your commit
message you refer to "hwid", that is a tad confusing for the reader.

> 
> On MP devices build without SMP the cpu DT node contains the expected
> MPIDR yet the hwid is set to 0. With this the function fails to match
> the cpus and uses the default CPU logical map. Making it impossible to
> get the CPU's DT node further down the line. This causes issues with
> cpufreq-dt, as it triggers warnings when not finding a suitable DT node
> on CPU0.
> 
> Change the way we choose whether to get MPIDR or not. Instead of
> depending on SMP check the number of CPUs defined in DT. Anything > 1
> means MPIDR will be available.

Except if someone accidentally wrote their Device Tree such as to have >
1 CPU nodes, yet the CPU is not MP capable and reading the MPIDR
register does return the expected value, but that is wrong anyway.

> 
> This was seen on a Raspberry Pi 2 build with bcm2835_defconfig.
> 
> Reported-by: "kernelci.org bot" <bot@...nelci.org>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
> ---
>  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..a924fda9abc8 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_child_count(cpus) > 1)
> +		mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
> +
>  	for_each_of_cpu_node(cpu) {
>  		const __be32 *cell;
>  		int prop_bytes;
> 


-- 
Florian