Re: [Qemu-devel] [PATCH v3] linux-user: ppc64: don't use volatile register during safe_syscall

[Richard Henderson <richard.henderson@linaro.org>]

On 07/30/2018 06:09 AM, Shivaprasad G Bhat wrote:
> r11 is a volatile register on PPC as per calling conventions.
> The safe_syscall code uses it to check if the signal_pending
> is set during the safe_syscall. When a syscall is interrupted
> on return from signal handling, the r11 might be corrupted
> before we retry the syscall leading to a crash. The registers
> r0-r13 are not to be used here as they have
> volatile/designated/reserved usages.
> 
> Change the code to use r14 which is non-volatile.
> Use SP+16 which is a slot for LR, for save/restore of previous value
> of r14. SP+16 can be used, as LR is preserved across the syscall.
> 
> Steps to reproduce:
> On PPC host, issue `qemu-x86_64 /usr/bin/cc -E -`
> Attempt Ctrl-C, the issue is reproduced.
> 
> Reference:
> https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#REG
> https://openpowerfoundation.org/wp-content/uploads/2016/03/ABI64BitOpenPOWERv1.1_16July2015_pub4.pdf
> 
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>

This does fix the bug, so
Tested-by: Richard Henderson <richard.henderson@linaro.org>

But we can do slightly better:

> @@ -49,7 +49,8 @@ safe_syscall_base:
>  	 *               and returns the result in r3
>  	 * Shuffle everything around appropriately.
>  	 */
> -	mr	11, 3	/* signal_pending */
> +	std     14, 16(1) /* Preserve r14 in SP+16 */

Above this context, we have a .cfi_startproc directive, which indicates we are
providing unwind information for this function (trivial up to this point).

To preserve that, you need to add

	.cfi_offset 14, 16

right here, indicating r14 is saved 16 bytes "up" the stack frame.

Since this portion of the stack frame is allocated by the caller, this saved
value remains valid through the end of the function, so we do not need to do
anything at the points where the value is restored.

>  safe_syscall_end:
> +	ld 14, 16(1) /* restore r14 to its original value */
>  	/* code path when we did execute the syscall */

Swap these two lines.

With those two changes,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~