[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-user: ppc64: don't use volatile register
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH] linux-user: ppc64: don't use volatile register during safe_syscall |
Date: |
Fri, 27 Jul 2018 10:01:06 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
Le 27/07/2018 à 06:47, Richard Henderson a écrit :
> On 07/26/2018 10:39 AM, Laurent Vivier wrote:
>> Le 26/07/2018 à 19:15, Richard Henderson a écrit :
>>> On 07/25/2018 11:48 PM, 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 and is appropriate for local use in
>>>> safe_syscall.
>>>>
>>>> Signed-off-by: Shivaprasad G Bhat <address@hidden>
>>>> ---
>>>> Steps to reproduce:
>>>> On PPC host, issue `qemu-ppc64le /usr/bin/cc -E -`
>>>> Attempt Ctrl-C, the issue is reproduced.
>>>>
>>>> Reference:
>>>> https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#REG
>>>>
>>>> linux-user/host/ppc64/safe-syscall.inc.S | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/linux-user/host/ppc64/safe-syscall.inc.S
>>>> b/linux-user/host/ppc64/safe-syscall.inc.S
>>>> index d30050a67c..b0cbbe6a69 100644
>>>> --- a/linux-user/host/ppc64/safe-syscall.inc.S
>>>> +++ b/linux-user/host/ppc64/safe-syscall.inc.S
>>>> @@ -49,7 +49,7 @@ safe_syscall_base:
>>>> * and returns the result in r3
>>>> * Shuffle everything around appropriately.
>>>> */
>>>> - mr 11, 3 /* signal_pending */
>>>> + mr 14, 3 /* signal_pending */
>>>
>>> I do see that I was incorrect in assuming that r11 would be unmodified. But
>>> you can't simply write to a call-saved register -- you must preserve its
>>> value
>>> for the caller.
>>>
>>> Saving the value requires that you find some space on, or create, a stack
>>> frame. Note that there are two different conventions for _CALL_AIX and
>>> _CALL_ELF==2.
>>
>> Can we guess the syscall ('sc') will not modify neither r11 nor r14...
>
> But sc does modify r11.
>
> li r11,0
> std r11,GPR9(r1)
> std r11,GPR10(r1)
> std r11,GPR11(r1)
>
> (Incidentally, unless I'm misreading, the kernel could have saved r11 with
> exactly as much effort as it took to clobber it to zero. That's just rude.)
>
>> but the function caller expects that r11 is not modified because it's the
>> environment pointer
>
> Huh? What do you think an "environment pointer" is in this context?
It's the comment for r11 in the section "2.2.1.1 Register Roles" of the
document.
> In the ppc64 abi, r11 is one of the ones that are clobbered by plt entries; it
> is not special in any way except as a scratch.
>
>> and saves r14 because it's one of its local
>> variable it knows it has to preserve?
>
> Huh? The caller of safe_syscall does not save r14, and does not expect it
> clobbered.
Yes, you're right. The callee has to save nonvolatile registers, and r14
is a nonvolatile register. And r11 is volatile, it's why it can be
modified by sc.
I should read the doc more carefully.
Thanks,
Laurent