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

[Alexander Graf]

On 20.01.21 14:27, Mohamed Mediouni wrote:
> From: Stan Skowronek <stan@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
> Signed-off-by: Mohamed Mediouni <mohamed.mediouni@xxxxxxxxxxxx>
> ---
>   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