linux-kernel - Re: [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.

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: <b30f1d3a-3a74-b562-afb6-da88a547468b@amazon.com>
Date:   Wed, 20 Jan 2021 17:46:28 +0100
From:   Alexander Graf <graf@...zon.com>
To:     Mohamed Mediouni <mohamed.mediouni@...amail.com>,
        <linux-arm-kernel@...ts.infradead.org>
CC:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Marc Zyngier <maz@...nel.org>,
        "Hector Martin" <marcan@...can.st>, <linux-kernel@...r.kernel.org>,
        Stan Skowronek <stan@...ellium.com>
Subject: Re: [RFC PATCH 2/7] arm64: kernel: Add a WFI hook.

On 20.01.21 14:27, Mohamed Mediouni wrote:
> From: Stan Skowronek <stan@...ellium.com>
> 
> WFI drops register state on Apple Silicon for SMP systems.

It probably drops the register because it loses power on WFI, right?

Have you tried to set the ARM64_REG_CYC_OVRD_ok2pwrdn_force_up bit in 
ARM64_REG_CYC_OVRD yet? XNU has a call for that[1]. Maybe that's enough 
to convince the core to preserve its register state for now.

For real power savings, we will probably want much more sophisticated 
deep sleep capabilities later that would reach beyond just register 
saving on WFI (different wakeup mechanisms, IRQ balancing, etc).


Alex

[1] 
https://github.com/opensource-apple/xnu/blob/master/osfmk/arm64/machine_routines_asm.s#L797

> 
> This hook will be used for a hardware workaround in the
> Apple CPU start driver.
> 
> Signed-off-by: Stan Skowronek <stan@...ellium.com>
> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@...amail.com>
> ---
>   arch/arm64/include/asm/cpu_ops.h |  2 ++
>   arch/arm64/kernel/cpu_ops.c      |  6 ++++++
>   arch/arm64/kernel/process.c      | 11 +++++++++--
>   3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h
> index e95c4df83911..4be0fc5bcaf9 100644
> --- a/arch/arm64/include/asm/cpu_ops.h
> +++ b/arch/arm64/include/asm/cpu_ops.h
> @@ -23,6 +23,7 @@
>    * @cpu_boot:	Boots a cpu into the kernel.
>    * @cpu_postboot: Optionally, perform any post-boot cleanup or necessary
>    *		synchronisation. Called from the cpu being booted.
> + * @cpu_wfi:    Optionally, replace calls to WFI in default idle with this.
>    * @cpu_can_disable: Determines whether a CPU can be disabled based on
>    *		mechanism-specific information.
>    * @cpu_disable: Prepares a cpu to die. May fail for some mechanism-specific
> @@ -43,6 +44,7 @@ struct cpu_operations {
>   	int		(*cpu_prepare)(unsigned int);
>   	int		(*cpu_boot)(unsigned int);
>   	void		(*cpu_postboot)(void);
> +	void		(*cpu_wfi)(void);
>   #ifdef CONFIG_HOTPLUG_CPU
>   	bool		(*cpu_can_disable)(unsigned int cpu);
>   	int		(*cpu_disable)(unsigned int cpu);
> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
> index e133011f64b5..6979fc4490b2 100644
> --- a/arch/arm64/kernel/cpu_ops.c
> +++ b/arch/arm64/kernel/cpu_ops.c
> @@ -19,12 +19,18 @@ extern const struct cpu_operations smp_spin_table_ops;
>   extern const struct cpu_operations acpi_parking_protocol_ops;
>   #endif
>   extern const struct cpu_operations cpu_psci_ops;
> +#ifdef CONFIG_ARCH_APPLE
> +extern const struct cpu_operations cpu_apple_start_ops;
> +#endif
> 
>   static const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;
> 
>   static const struct cpu_operations *const dt_supported_cpu_ops[] __initconst = {
>   	&smp_spin_table_ops,
>   	&cpu_psci_ops,
> +#ifdef CONFIG_ARCH_APPLE
> +	&cpu_apple_start_ops,
> +#endif
>   	NULL,
>   };
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 34ec400288d0..611c639e20be 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -57,6 +57,7 @@
>   #include <asm/processor.h>
>   #include <asm/pointer_auth.h>
>   #include <asm/stacktrace.h>
> +#include <asm/cpu_ops.h>
> 
>   #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
>   #include <linux/stackprotector.h>
> @@ -74,8 +75,14 @@ void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
> 
>   static void noinstr __cpu_do_idle(void)
>   {
> -	dsb(sy);
> -	wfi();
> +	const struct cpu_operations *ops = get_cpu_ops(task_cpu(current));
> +
> +	if (ops->cpu_wfi) {
> +		ops->cpu_wfi();
> +	} else {
> +		dsb(sy);
> +		wfi();
> +	}
>   }
> 
>   static void noinstr __cpu_do_idle_irqprio(void)
> --
> 2.29.2
> 




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879