Re: [RFC PATCH 5/6] powerpc/powernv/idle: Convert stack pointer to physical address

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 04/11/2022 à 18:27, Andrew Donnellan a écrit :
> When we go into idle, we must disable the MMU. Currently, we can still
> access the stack once the MMU is disabled, because the stack is in the
> linear map.
> 
> Once we enable CONFIG_VMAP_STACK, the normal stack pointer will be in the
> vmalloc area. To cope with this, manually convert the stack pointer to a
> physical address using stack_pa() before going into idle, and restore the
> original pointer on the way back out.
> 
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> 
> ---
> 
> This currently doesn't boot on my POWER9. I'm also going to clean this up
> to use the helpers from earlier in this series.
> ---
>   arch/powerpc/platforms/powernv/idle.c | 47 +++++++++++++++++++++++++--
>   1 file changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 841cb7f31f4f..6430fb488981 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -22,6 +22,7 @@
>   #include <asm/smp.h>
>   #include <asm/runlatch.h>
>   #include <asm/dbell.h>
> +#include <asm/reg.h>
>   
>   #include "powernv.h"
>   #include "subcore.h"
> @@ -509,6 +510,11 @@ static unsigned long power7_offline(void)
>   {
>   	unsigned long srr1;
>   
> +#ifdef CONFIG_VMAP_STACK
> +	unsigned long ksp_ea = current_stack_pointer;
> +	current_stack_pointer = (unsigned long)stack_pa((void *)ksp_ea);

Same as other patch, I think you can't just change stack pointer on the 
fly, you have to change it carefully via assembly and perform a function 
call, just like done for irqs.

> +#endif
> +
>   	mtmsr(MSR_IDLE);
>   
>   #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> @@ -543,6 +549,9 @@ static unsigned long power7_offline(void)
>   		srr1 = idle_kvm_start_guest(srr1);
>   #endif
>   
> +#ifdef CONFIG_VMAP_STACK

You could avoid many of the #ifdef and replace them with IS_ENABLED()

> +	current_stack_pointer = ksp_ea;
> +#endif
>   	mtmsr(MSR_KERNEL);
>   
>   	return srr1;
> @@ -552,14 +561,24 @@ static unsigned long power7_offline(void)
>   void power7_idle_type(unsigned long type)
>   {
>   	unsigned long srr1;
> +#ifdef CONFIG_VMAP_STACK
> +	unsigned long ksp_ea;
> +#endif
>   
>   	if (!prep_irq_for_idle_irqsoff())
>   		return;
>   
> +#ifdef CONFIG_VMAP_STACK
> +	ksp_ea = current_stack_pointer;
> +	current_stack_pointer = (unsigned long)stack_pa((void *)ksp_ea);
> +#endif
>   	mtmsr(MSR_IDLE);
>   	__ppc64_runlatch_off();
>   	srr1 = power7_idle_insn(type);
>   	__ppc64_runlatch_on();
> +#ifdef CONFIG_VMAP_STACK
> +	current_stack_pointer = ksp_ea;
> +#endif
>   	mtmsr(MSR_KERNEL);
>   
>   	fini_irq_for_idle_irqsoff();
> @@ -615,6 +634,9 @@ static unsigned long power9_idle_stop(unsigned long psscr)
>   	unsigned long mmcra = 0;
>   	struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
>   	bool sprs_saved = false;
> +#ifdef CONFIG_VMAP_STACK
> +	unsigned long ksp_ea;
> +#endif
>   
>   	if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
>   		/* EC=ESL=0 case */
> @@ -633,7 +655,7 @@ static unsigned long power9_idle_stop(unsigned long psscr)
>   		 */
>   		BUG_ON((srr1 & SRR1_WAKESTATE) != SRR1_WS_NOLOSS);
>   
> -		goto out;
> +		goto out_noloss;
>   	}
>   
>   	/* EC=ESL=1 case */
> @@ -688,6 +710,10 @@ static unsigned long power9_idle_stop(unsigned long psscr)
>   	sprs.iamr	= mfspr(SPRN_IAMR);
>   	sprs.uamor	= mfspr(SPRN_UAMOR);
>   
> +#ifdef CONFIG_VMAP_STACK
> +	ksp_ea = current_stack_pointer;
> +	current_stack_pointer = (unsigned long)stack_pa((void *)ksp_ea);
> +#endif
>   	srr1 = isa300_idle_stop_mayloss(psscr);		/* go idle */
>   
>   #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> @@ -797,6 +823,10 @@ static unsigned long power9_idle_stop(unsigned long psscr)
>   		__slb_restore_bolted_realmode();
>   
>   out:
> +#ifdef CONFIG_VMAP_STACK
> +	current_stack_pointer = ksp_ea;
> +#endif
> +out_noloss:
>   	mtmsr(MSR_KERNEL);
>   
>   	return srr1;
> @@ -898,6 +928,9 @@ static unsigned long power10_idle_stop(unsigned long psscr)
>   	unsigned long pls;
>   //	struct p10_sprs sprs = {}; /* avoid false used-uninitialised */
>   	bool sprs_saved = false;
> +#ifdef CONFIG_VMAP_STACK
> +	unsigned long ksp_ea;
> +#endif
>   
>   	if (!(psscr & (PSSCR_EC|PSSCR_ESL))) {
>   		/* EC=ESL=0 case */
> @@ -916,7 +949,7 @@ static unsigned long power10_idle_stop(unsigned long psscr)
>   		 */
>   		BUG_ON((srr1 & SRR1_WAKESTATE) != SRR1_WS_NOLOSS);
>   
> -		goto out;
> +		goto out_noloss;
>   	}
>   
>   	/* EC=ESL=1 case */
> @@ -927,7 +960,11 @@ static unsigned long power10_idle_stop(unsigned long psscr)
>   
>   		atomic_start_thread_idle();
>   	}
> -
> +#ifdef CONFIG_VMAP_STACK
> +	ksp_ea = current_stack_pointer;
> +	current_stack_pointer = (unsigned long)stack_pa((void *)ksp_ea);
> +#endif /* CONFIG_VMAP_STACK */
> +	mtmsr(MSR_IDLE);
>   	srr1 = isa300_idle_stop_mayloss(psscr);		/* go idle */
>   
>   	psscr = mfspr(SPRN_PSSCR);
> @@ -982,6 +1019,10 @@ static unsigned long power10_idle_stop(unsigned long psscr)
>   		__slb_restore_bolted_realmode();
>   
>   out:
> +#ifdef CONFIG_VMAP_STACK
> +	current_stack_pointer = ksp_ea;
> +#endif /* CONFIG_VMAP_STACK */
> +out_noloss:
>   	mtmsr(MSR_KERNEL);
>   
>   	return srr1;