[2/4] powerpc/64: remove support for kernel-mode syscalls

There is support for the kernel to execute the 'sc 0' instruction and
make a system call to itself. This is a relic that is unused in the
tree, therefore untested. It's also highly questionable for modules to
be doing this.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/entry_64.S       | 21 ++++++---------------
 arch/powerpc/kernel/exceptions-64s.S |  2 --
 2 files changed, 6 insertions(+), 17 deletions(-)

Le 27/08/2019 à 05:30, Nicholas Piggin a écrit :
> There is support for the kernel to execute the 'sc 0' instruction and
> make a system call to itself. This is a relic that is unused in the
> tree, therefore untested. It's also highly questionable for modules to
> be doing this.

I like it.

I dropped support for that in PPC32 when I added fast-path syscalls.

> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/kernel/entry_64.S       | 21 ++++++---------------
>   arch/powerpc/kernel/exceptions-64s.S |  2 --
>   2 files changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 0a0b5310f54a..6467bdab8d40 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -69,24 +69,20 @@ BEGIN_FTR_SECTION
>   	bne	.Ltabort_syscall
>   END_FTR_SECTION_IFSET(CPU_FTR_TM)
>   #endif
> -	andi.	r10,r12,MSR_PR
>   	mr	r10,r1
> -	addi	r1,r1,-INT_FRAME_SIZE
> -	beq-	1f
>   	ld	r1,PACAKSAVE(r13)
> -1:	std	r10,0(r1)
> +	std	r10,0(r1)
>   	std	r11,_NIP(r1)
>   	std	r12,_MSR(r1)
>   	std	r0,GPR0(r1)
>   	std	r10,GPR1(r1)
> -	beq	2f			/* if from kernel mode */
>   #ifdef CONFIG_PPC_FSL_BOOK3E
>   START_BTB_FLUSH_SECTION
>   	BTB_FLUSH(r10)
>   END_BTB_FLUSH_SECTION
>   #endif
>   	ACCOUNT_CPU_USER_ENTRY(r13, r10, r11)
> -2:	std	r2,GPR2(r1)
> +	std	r2,GPR2(r1)
>   	std	r3,GPR3(r1)
>   	mfcr	r2
>   	std	r4,GPR4(r1)
> @@ -122,14 +118,13 @@ END_BTB_FLUSH_SECTION
>   
>   #if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && defined(CONFIG_PPC_SPLPAR)
>   BEGIN_FW_FTR_SECTION
> -	beq	33f
> -	/* if from user, see if there are any DTL entries to process */
> +	/* see if there are any DTL entries to process */
>   	ld	r10,PACALPPACAPTR(r13)	/* get ptr to VPA */
>   	ld	r11,PACA_DTL_RIDX(r13)	/* get log read index */
>   	addi	r10,r10,LPPACA_DTLIDX
>   	LDX_BE	r10,0,r10		/* get log write index */
> -	cmpd	cr1,r11,r10
> -	beq+	cr1,33f
> +	cmpd	r11,r10
> +	beq+	33f

Any need to do this change ? Why not keep it as is ?

Christophe

>   	bl	accumulate_stolen_time
>   	REST_GPR(0,r1)
>   	REST_4GPRS(3,r1)
> @@ -203,6 +198,7 @@ system_call:			/* label this so stack traces look sane */
>   	mtctr   r12
>   	bctrl			/* Call handler */
>   
> +	/* syscall_exit can exit to kernel mode, via ret_from_kernel_thread */
>   .Lsyscall_exit:
>   	std	r3,RESULT(r1)
>   
> @@ -216,11 +212,6 @@ system_call:			/* label this so stack traces look sane */
>   	ld	r12, PACA_THREAD_INFO(r13)
>   
>   	ld	r8,_MSR(r1)
> -#ifdef CONFIG_PPC_BOOK3S
> -	/* No MSR:RI on BookE */
> -	andi.	r10,r8,MSR_RI
> -	beq-	.Lunrecov_restore
> -#endif
>   
>   /*
>    * This is a few instructions into the actual syscall exit path (which actually
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 6ba3cc2ef8ab..768f133de4f1 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -1521,8 +1521,6 @@ EXC_COMMON(trap_0b_common, 0xb00, unknown_exception)
>    * system call / hypercall (0xc00, 0x4c00)
>    *
>    * The system call exception is invoked with "sc 0" and does not alter HV bit.
> - * There is support for kernel code to invoke system calls but there are no
> - * in-tree users.
>    *
>    * The hypercall is invoked with "sc 1" and sets HV=1.
>    *
>

Christophe Leroy's on August 27, 2019 4:13 pm:
> 
> 
> Le 27/08/2019 à 05:30, Nicholas Piggin a écrit :
>> There is support for the kernel to execute the 'sc 0' instruction and
>> make a system call to itself. This is a relic that is unused in the
>> tree, therefore untested. It's also highly questionable for modules to
>> be doing this.
> 
> I like it.
> 
> I dropped support for that in PPC32 when I added fast-path syscalls.

Good, then we'll match again.

>> -	beq	2f			/* if from kernel mode */
>>   #ifdef CONFIG_PPC_FSL_BOOK3E
>>   START_BTB_FLUSH_SECTION
>>   	BTB_FLUSH(r10)
>>   END_BTB_FLUSH_SECTION
>>   #endif
>>   	ACCOUNT_CPU_USER_ENTRY(r13, r10, r11)
>> -2:	std	r2,GPR2(r1)

Btw. there is a hunk which restores this optimisation but it leaked
into a later patch, I'll move it back here.

>> @@ -122,14 +118,13 @@ END_BTB_FLUSH_SECTION
>>   
>>   #if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && defined(CONFIG_PPC_SPLPAR)
>>   BEGIN_FW_FTR_SECTION
>> -	beq	33f
>> -	/* if from user, see if there are any DTL entries to process */
>> +	/* see if there are any DTL entries to process */
>>   	ld	r10,PACALPPACAPTR(r13)	/* get ptr to VPA */
>>   	ld	r11,PACA_DTL_RIDX(r13)	/* get log read index */
>>   	addi	r10,r10,LPPACA_DTLIDX
>>   	LDX_BE	r10,0,r10		/* get log write index */
>> -	cmpd	cr1,r11,r10
>> -	beq+	cr1,33f
>> +	cmpd	r11,r10
>> +	beq+	33f
> 
> Any need to do this change ? Why not keep it as is ?

We don't need to keep cr0 alive with the MSR_PR compare, so I prefer
to keep the cr usage more compact, and I prefer to have cr0 for short
lived results and others for long running ones as a rule.

Thanks,
Nick

Nicholas Piggin's on August 27, 2019 8:20 pm:
> Christophe Leroy's on August 27, 2019 4:13 pm:
>> 
>> 
>> Le 27/08/2019 à 05:30, Nicholas Piggin a écrit :
>>> There is support for the kernel to execute the 'sc 0' instruction and
>>> make a system call to itself. This is a relic that is unused in the
>>> tree, therefore untested. It's also highly questionable for modules to
>>> be doing this.
>> 
>> I like it.
>> 
>> I dropped support for that in PPC32 when I added fast-path syscalls.
> 
> Good, then we'll match again.
> 
>>> -	beq	2f			/* if from kernel mode */
>>>   #ifdef CONFIG_PPC_FSL_BOOK3E
>>>   START_BTB_FLUSH_SECTION
>>>   	BTB_FLUSH(r10)
>>>   END_BTB_FLUSH_SECTION
>>>   #endif
>>>   	ACCOUNT_CPU_USER_ENTRY(r13, r10, r11)
>>> -2:	std	r2,GPR2(r1)
> 
> Btw. there is a hunk which restores this optimisation but it leaked
> into a later patch, I'll move it back here.

I'm wrong. Now I look at it again, the hunk should be removed
completely of course.

Thanks,
Nick