Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <04cdd26307a1eaebeacc039b207db92e0b6820bb.camel@neuling.org>
Subject: Re: [PATCH] powerpc/chrp: Fix enter_rtas() with CONFIG_VMAP_STACK
From:   Michael Neuling <mikey@neuling.org>
To:     Christophe Leroy <christophe.leroy@c-s.fr>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>,
        Michael Ellerman <mpe@ellerman.id.au>
Cc:     linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Date:   Mon, 17 Feb 2020 09:40:08 +1100
In-Reply-To: <159ecb0ab021c07fd2f383d4a083a43d16d67b92.1581669187.git.christophe.leroy@c-s.fr>
References: <159ecb0ab021c07fd2f383d4a083a43d16d67b92.1581669187.git.christophe.leroy@c-s.fr>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.34.3 (3.34.3-1.fc31) 
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 2020-02-14 at 08:33 +0000, Christophe Leroy wrote:
> With CONFIG_VMAP_STACK, data MMU has to be enabled
> to read data on the stack.

Can you describe what goes wrong without this? Some oops message? rtas blow=
s up?
Get corrupt data?

Also can you say what you're actually doing (ie turning on MSR[DR])


> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/kernel/entry_32.S | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_3=
2.S
> index 0713daa651d9..bc056d906b51 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -1354,12 +1354,17 @@ _GLOBAL(enter_rtas)
>  	mtspr	SPRN_SRR0,r8
>  	mtspr	SPRN_SRR1,r9
>  	RFI
> -1:	tophys(r9,r1)
> +1:	tophys_novmstack r9, r1
> +#ifdef CONFIG_VMAP_STACK
> +	li	r0, MSR_KERNEL & ~MSR_IR	/* can take DTLB miss */

You're potentially turning on more than MSR DR here. This should be clear i=
n the
commit message.

> +	mtmsr	r0
> +	isync
> +#endif
>  	lwz	r8,INT_FRAME_SIZE+4(r9)	/* get return address */
>  	lwz	r9,8(r9)	/* original msr value */
>  	addi	r1,r1,INT_FRAME_SIZE
>  	li	r0,0
> -	tophys(r7, r2)
> +	tophys_novmstack r7, r2
>  	stw	r0, THREAD + RTAS_SP(r7)
>  	mtspr	SPRN_SRR0,r8
>  	mtspr	SPRN_SRR1,r9