linux-kernel - Re: [PATCH v2 1/2] arm64: topology: move store_cpu

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: <efa89122-b428-7691-49d3-f5867206f05a@microchip.com>
Date:   Sat, 9 Jul 2022 12:58:57 +0000
From:   <Conor.Dooley@...rochip.com>
To:     <paul.walmsley@...ive.com>, <palmer@...belt.com>,
        <palmer@...osinc.com>, <aou@...s.berkeley.edu>,
        <sudeep.holla@....com>, <catalin.marinas@....com>,
        <will@...nel.org>, <gregkh@...uxfoundation.org>,
        <rafael@...nel.org>, <linux@...linux.org.uk>, <arnd@...db.de>
CC:     <Daire.McNamara@...rochip.com>, <niklas.cassel@....com>,
        <damien.lemoal@...nsource.wdc.com>, <geert@...ux-m68k.org>,
        <zong.li@...ive.com>, <kernel@...il.dk>, <hahnjo@...njo.de>,
        <guoren@...nel.org>, <anup@...infault.org>,
        <atishp@...shpatra.org>, <heiko@...ech.de>,
        <philipp.tomsich@...ll.eu>, <robh@...nel.org>, <maz@...nel.org>,
        <viresh.kumar@...aro.org>, <linux-riscv@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <Brice.Goglin@...ia.fr>
Subject: Re: [PATCH v2 1/2] arm64: topology: move store_cpu_topology() to
 shared code

+CC Russel, Arnd

On 08/07/2022 21:45, Conor Dooley - M52691 wrote:
> On 08/07/2022 21:33, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@...rochip.com>
>>
>> arm64's method of defining a default cpu topology requires only minimal
>> changes to apply to RISC-V also. The current arm64 implementation exits
>> early in a uniprocessor configuration by reading MPIDR & claiming that
>> uniprocessor can rely on the default values.
>>
>> This is appears to be a hangover from prior to '3102bc0e6ac7 ("arm64:
>> topology: Stop using MPIDR for topology information")', because the
>> current code just assigns default values for multiprocessor systems.
>>
>> With the MPIDR references removed, store_cpu_topolgy() can be moved to
>> the common arch_topology code.
>>
>> CC: stable@...r.kernel.org
>> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
>> ---
---8<---
>>  #ifdef CONFIG_ACPI
>>  static bool __init acpi_cpu_is_threaded(int cpu)
>>  {
>> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
>> index 441e14ac33a4..07e84c6ac5c2 100644
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -765,6 +765,25 @@ void update_siblings_masks(unsigned int cpuid)
>>  	}
>>  }
>>  
>> +void __weak store_cpu_topology(unsigned int cpuid)
> 
> Ahh crap, I forgot to remove the __weak.
> I won't immediately respin since it is minor. I've pushed it (without
> the __weak) to https://git.kernel.org/conor/h/arch-topo so it'll get
> the lkp coverage.

And build failure for arm32:

> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git arch-topo
> branch HEAD: df379c4b12f6b22fb8c07c2be16fd821a4fcbfc5  riscv: topology: fix default topology reporting
> 
> Error/Warning: (recently discovered and may have been fixed)
> 
> arch_topology.c:(.text+0xbac): multiple definition of `store_cpu_topology'; arch/arm/kernel/topology.o:topology.c:(.text+0x0): first defined here
> 
> Error/Warning ids grouped by kconfigs:
> 
> gcc_recent_errors
> `-- arm-defconfig
>     `-- multiple-definition-of-store_cpu_topology-arch-arm-kernel-topology.o:topology.c:(.text):first-defined-here
> 
> elapsed time: 721m

Looking at the arm32 implementation - it appears to be mostly the sort of MPIDR
stuff that was removed from the arm64 implementation in 3102bc0e6ac7 ("arm64:
topology: Stop using MPIDR for topology information"). Could arm32 benefit from
the same shared implemenation too, or is usage of MPIDR only invalid for arm64?

The other difference is a call to update_cpu_capacity() in the arm32
implementation. Could that be moved to smp_store_cpu_info() which is the only
callsite of store_cpu_topology()?

Either way, will respin a v3 that doesn't break the arm32 build when
CONFIG_GENERIC_ARCH_TOPOLOGY is enabled :)

Thanks,
Conor.