LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] powerpc/chrp: Fix enter_rtas() with CONFIG_VMAP_STACK
@ 2020-02-14  8:33 Christophe Leroy
  2020-02-16 22:40 ` Michael Neuling
  0 siblings, 1 reply; 3+ messages in thread
From: Christophe Leroy @ 2020-02-14  8:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

With CONFIG_VMAP_STACK, data MMU has to be enabled
to read data on the stack.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/entry_32.S | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.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 */
+	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
-- 
2.25.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/chrp: Fix enter_rtas() with CONFIG_VMAP_STACK
  2020-02-14  8:33 [PATCH] powerpc/chrp: Fix enter_rtas() with CONFIG_VMAP_STACK Christophe Leroy
@ 2020-02-16 22:40 ` Michael Neuling
  2020-02-17  6:40   ` Christophe Leroy
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Neuling @ 2020-02-16 22:40 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

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 blows 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(-)
> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.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 in 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] powerpc/chrp: Fix enter_rtas() with CONFIG_VMAP_STACK
  2020-02-16 22:40 ` Michael Neuling
@ 2020-02-17  6:40   ` Christophe Leroy
  0 siblings, 0 replies; 3+ messages in thread
From: Christophe Leroy @ 2020-02-17  6:40 UTC (permalink / raw)
  To: Michael Neuling, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman
  Cc: linuxppc-dev, linux-kernel



Le 16/02/2020 à 23:40, Michael Neuling a écrit :
> 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 blows up?
> Get corrupt data?

Larry reported a machine check. Or in fact, he reported a Oops in 
kprobe_handler(), that Oops being a bug in kprobe_handle() triggered by 
this machine check.

By converting a VM address to a phys-like address as if is was linear 
mem, you get in the dark. Either there is some physical memory at that 
address and you corrupt it. Or there is none and you get a machine check.

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

Euh ... I'm saying that data MMU has to be enabled, so I'm enabling it.

> 
> 
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/kernel/entry_32.S | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.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 in the
> commit message.

Am I ?

At the time of the RFI just above, SRR1 contains the value of r9 which 
has been set 2 lines before to MSR_KERNEL & ~(MSR_IR|MSR_DR).

What should be clear in 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

Christophe

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14  8:33 [PATCH] powerpc/chrp: Fix enter_rtas() with CONFIG_VMAP_STACK Christophe Leroy
2020-02-16 22:40 ` Michael Neuling
2020-02-17  6:40   ` Christophe Leroy

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git