[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e447bd53-3682-3808-f256-3463a5cfdec6@csgroup.eu>
Date: Tue, 8 Nov 2022 16:17:56 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Andrew Donnellan <ajd@...ux.ibm.com>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
CC: "ruscur@...sell.cc" <ruscur@...sell.cc>,
"cmr@...escreens.de" <cmr@...escreens.de>,
"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>
Subject: Re: [RFC PATCH 5/6] powerpc/powernv/idle: Convert stack pointer to
physical address
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@...ux.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;
Powered by blists - more mailing lists