linux-kernel - Re: [PATCH] arm64/cpuinfo: Drop boot_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: <20200504124321.GA73375@C02TD0UTHF1T.local>
Date:   Mon, 4 May 2020 13:43:21 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Anshuman Khandual <anshuman.khandual@....com>
Cc:     linux-arm-kernel@...ts.infradead.org,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Mark Brown <broonie@...nel.org>,
        Suzuki Poulose <suzuki.poulose@....com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64/cpuinfo: Drop boot_cpu_data

On Mon, May 04, 2020 at 06:00:00PM +0530, Anshuman Khandual wrote:
> A global boot_cpu_data is not really required. Lets drop this.

I don't think it's true that this isn't required today.

One reason that we have both boot_cpu_data and a cpu_data variable for
CPU0 is that CPU0 itself can be hotplugged out then back in, and this
allows us to detect if CPU0's features have changed (e.g. due to FW
failing to configure it appropriately, or real physical hotplug
occurring).

So NAK to the patch as it stands. If we're certain we capture all of
those details even without boot_cpu_data, then we should make other
changes to make that clear (e.g. removing it as an argument to
update_cpu_features()).

Thanks,
Mark.

> While here, rename the local variable as boot_cpu_info when it is
> fetched for the boot cpu.
> 
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Mark Brown <broonie@...nel.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Suzuki Poulose <suzuki.poulose@....com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
> ---
> Based on 5.7-rc4
> 
>  arch/arm64/kernel/cpuinfo.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index a515d8f3639e..dabcdc132e56 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -31,7 +31,6 @@
>   * values depending on configuration at or after reset.
>   */
>  DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
> -static struct cpuinfo_arm64 boot_cpu_data;
>  
>  static const char *icache_policy_str[] = {
>  	[0 ... ICACHE_POLICY_PIPT]	= "RESERVED/UNKNOWN",
> @@ -393,15 +392,16 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
>  void cpuinfo_store_cpu(void)
>  {
>  	struct cpuinfo_arm64 *info = this_cpu_ptr(&cpu_data);
> +	struct cpuinfo_arm64 *boot_cpu_info = &per_cpu(cpu_data, 0);
> +
>  	__cpuinfo_store_cpu(info);
> -	update_cpu_features(smp_processor_id(), info, &boot_cpu_data);
> +	update_cpu_features(smp_processor_id(), info, boot_cpu_info);
>  }
>  
>  void __init cpuinfo_store_boot_cpu(void)
>  {
> -	struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0);
> -	__cpuinfo_store_cpu(info);
> +	struct cpuinfo_arm64 *boot_cpu_info = &per_cpu(cpu_data, 0);
>  
> -	boot_cpu_data = *info;
> -	init_cpu_features(&boot_cpu_data);
> +	__cpuinfo_store_cpu(boot_cpu_info);
> +	init_cpu_features(boot_cpu_info);
>  }
> -- 
> 2.20.1
>