Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030497AbVJGQck (ORCPT ); Fri, 7 Oct 2005 12:32:40 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1030499AbVJGQck (ORCPT ); Fri, 7 Oct 2005 12:32:40 -0400 Received: from vms046pub.verizon.net ([206.46.252.46]:13609 "EHLO vms046pub.verizon.net") by vger.kernel.org with ESMTP id S1030497AbVJGQcj (ORCPT ); Fri, 7 Oct 2005 12:32:39 -0400 Date: Fri, 07 Oct 2005 12:32:38 -0400 From: Bryan Ford Subject: Re: [PATCH] x86_64 signal handling for 64-bit apps w/ mixed 32-bit code - trivial fix In-reply-to: To: Andi Kleen Cc: linux-kernel@vger.kernel.org Message-id: <200510071232.38344.baford@mit.edu> Organization: Massachusetts Institute of Technology MIME-version: 1.0 Content-type: Multipart/Mixed; boundary="Boundary-00=_mMqRD6yCqjR9xKo" Content-disposition: inline References: <200510051235.41376.baford@mit.edu> User-Agent: KMail/1.8 Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3738 Lines: 94 --Boundary-00=_mMqRD6yCqjR9xKo Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Thursday 06 October 2005 12:46, Andi Kleen wrote: > Bryan Ford writes: > > The proposed patch doesn't affect any performance-critical paths (e.g., > > syscall or interrupt entry/exit), and merely involves a couple more moves > > to/from user space on signal frame setup and sigreturn. It also doesn't > > affect the size or shape of the sigcontext at all, since there already > > was an (unused) slot for CS, and I've assigned the convenient __pad0 > > field as a slot for SS. The existing, unused slots for FS and GS remain > > unused for now, and I don't see any urgent need to change that. The only > > way this might break an existing app is if the app tries to cons up its > > own signal frame (not generated by the kernel) and pass it to sigreturn, > > but this is presumably a no-no anyway. > > I see the point of saving/restore cs, but why ss and not es/ds ? Hmm - I remember at one point thinking there was a reason it was necessary for the kernel to save SS as well as CS, but now I can't remember what it was - and now that you mention it, it does seem unnecessary for the kernel to save SS since its value is irrelevant to the CPU while running the 64-bit signal handler code, and the 64-bit signal handler itself can save and restore it if necessary (as it can with ES/DS/FS/GS). Saving SS along with CS would make the 64-bit signal path more consistent with the 32-bit signal path and with the way the processor's interrupt/IRET mechanism works, but neither of these considerations seem very important. So on the assumption you'd prefer the code path to do the absolute minimum it can get away with, here's a new version of my previously posted patch, trimmed so it only saves and restores CS. This of course makes it unnecessary even to rename the __pad0 field in struct sigcontext. Thanks, Bryan --Boundary-00=_mMqRD6yCqjR9xKo Content-Type: text/x-diff; charset="iso-8859-1"; name="pch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pch" diff -ur orig/arch/x86_64/kernel/signal.c linux-2.6.13.3/arch/x86_64/kernel/signal.c --- orig/arch/x86_64/kernel/signal.c 2005-10-06 16:00:54.000000000 -0400 +++ linux-2.6.13.3/arch/x86_64/kernel/signal.c 2005-10-06 16:02:23.000000000 -0400 @@ -110,6 +110,13 @@ COPY(r14); COPY(r15); + /* Kernel saves and restores only the CS segment register on signals, + * which is the bare minimum needed to allow mixed 32/64-bit code. + * App's signal handler can save/restore other segments if needed. */ + unsigned short cs; + err |= __get_user(cs, &sc->cs); + regs->cs = cs | 3; /* Force into user mode */ + { unsigned int tmpflags; err |= __get_user(tmpflags, &sc->eflags); @@ -187,6 +194,7 @@ { int err = 0; + err |= __put_user(regs->cs, &sc->cs); err |= __put_user(0, &sc->gs); err |= __put_user(0, &sc->fs); @@ -318,7 +326,14 @@ regs->rsp = (unsigned long)frame; + /* Set up the CS register to run signal handlers in 64-bit mode, + even if the handler happens to be interrupting 32-bit code. */ + regs->cs = __USER_CS; + + /* This, by contrast, has nothing to do with segment registers - + see include/asm-x86_64/uaccess.h for details. */ set_fs(USER_DS); + regs->eflags &= ~TF_MASK; if (test_thread_flag(TIF_SINGLESTEP)) ptrace_notify(SIGTRAP); --Boundary-00=_mMqRD6yCqjR9xKo-- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/