From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57491) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abdaX-0002Xi-Sk for qemu-devel@nongnu.org; Thu, 03 Mar 2016 19:27:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abdaU-0007By-KU for qemu-devel@nongnu.org; Thu, 03 Mar 2016 19:27:25 -0500 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:36807) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abdaU-0007Bu-CK for qemu-devel@nongnu.org; Thu, 03 Mar 2016 19:27:22 -0500 Received: by mail-wm0-x244.google.com with SMTP id l68so1610668wml.3 for ; Thu, 03 Mar 2016 16:27:22 -0800 (PST) Date: Fri, 4 Mar 2016 01:27:19 +0100 From: "Edgar E. Iglesias" Message-ID: <20160304002719.GI16305@toto> References: <1441497448-32489-1-git-send-email-T.E.Baldwin99@members.leeds.ac.uk> <1441497448-32489-11-git-send-email-T.E.Baldwin99@members.leeds.ac.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 10/34] linux-user: Support for restarting system calls for Microblaze targets List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Riku Voipio , QEMU Developers , Timothy E Baldwin On Thu, Mar 03, 2016 at 08:15:13PM +0000, Peter Maydell wrote: > Hi Edgar -- I'm just looking back at these signal handling > race condition fix patches, and with this one I have a confusion > about the Microblaze Linux syscall code that I hope you can > clear up for me. > > Looking at the kernel entry.S code it looks to me like > the way syscalls work on microblaze is: > * syscall insn is brki r14 > * the insn itself saves the PC of the brki into r14 > * on entry the kernel advances r14 by 4 to skip the brki > * then SAVE_REGS saves r14 into the 'PC' slot in the pt_regs > struct > * for syscall restart handle_restart() may wind the PC > value in the pt_regs back by 4 > * in any case, on syscall exit we pull the PC value out of > pt_regs into r14, and do a return with rtbd r14, 0 Yes, that sounds right. > > I think what this implies is that: > * r14 is a "used by the kernel, may be corrupted at any > time, not to be touched by userspace" register Yes. r14 is not really usable by user-space, interrupts will for example clobber r14 at any time aswell. > * on exit from a syscall PC and r14 are always the same Yes that's how it works but as far as user-space is concerned r14 may have any value at any time as it's not really observable in a safe way. > * this includes do_sigreturn, ie "taking a signal" is one > of the things that can corrupt r14 Yes. > > Is that right? Yes, I think so. > (For context, the original patch is this one: > http://patchwork.ozlabs.org/patch/514879/ > and I now suspect my review comments at the time to be wrong.) I see. Functionally I think the patch is OK. It seems to have some whitespace fixes mixed with functional changes (nitpick). Either way: Reviewed-by: Edgar E. Iglesias Best regards, Edgar