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

[Wirth, Allan]

Pranith,

Thanks for doing this. I totally forgot about this (my work has moved 
elsewhere) so thank you for picking it back up.

Please don’t worry about the attribution.

The patch LGTM. :)

Cheers,
Allan

On 2/3/17, 10:55 AM, "Pranith Kumar" <address@hidden> wrote:

    
    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