Re: [Qemu-devel] [RFC PATCH] linux-user: Add signal handling for x86_64

[Pranith Kumar]

Peter Maydell writes:

> On 25 January 2017 at 00:10, Pranith Kumar <address@hidden> wrote:
>> Adopted from a previous patch posting:
>> https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg02079.html
>>
>> CC: Allan Wirth <address@hidden>
>> CC: Peter Maydell <address@hidden>
>> Signed-off-by: Pranith Kumar <address@hidden>
>
> Thanks for picking this patch up. A nit about commit message format:
> because this  is mostly Allan's work you need to add his signed-off-by:
> line (which he provided on his original patch posting), and make
> a brief not of what was changed, so it looks like:
>
>   Signed-off-by: Original Author <address@hidden>
>   [OP: changed X, Y, Z]
>   Signed-off-by: Other Person <address@hidden>
>
> It's also in this kind of situation worth considering whether the
> patch would be better attributed to Allan as the git commit 'author'.
> If I've taken somebody else's work and made mostly minor overhauls
> to it I tend to go for giving them credit in the git commit log.

OK, I'll add these SOB lines and attribute it to Allan as he did most of the 
work.

>
>> ---
>>  linux-user/signal.c      | 264 
>> ++++++++++++++++++++++++++++++++++++++++-------
>>  target/i386/cpu.h        |   2 +
>>  target/i386/fpu_helper.c |  12 +++
>>  3 files changed, 242 insertions(+), 36 deletions(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index 0a5bb4e26b..0248621d66 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -253,8 +253,7 @@ int do_sigprocmask(int how, const sigset_t *set, 
>> sigset_t *oldset)
>>      return 0;
>>  }
>>
>> -#if !defined(TARGET_OPENRISC) && !defined(TARGET_UNICORE32) && \
>> -    !defined(TARGET_X86_64)
>> +#if !defined(TARGET_OPENRISC) && !defined(TARGET_UNICORE32)
>>  /* Just set the guest's signal mask to the specified value; the
>>   * caller is assumed to have called block_signals() already.
>>   */
>
> There's a minor conflict here with the Nios2 code that's now
> in master (which added another clause to this #if), but it's
> trivial to resolve.

I'll rebase my patch on master and fix up the conflicts and send a v2.

>
> Otherwise:
>
> Reviewed-by: Peter Maydell <address@hidden>

Thanks for the review!
-- 
Pranith