[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 10/34] linux-user: Support for restarting system
Edgar E. Iglesias
Re: [Qemu-devel] [PATCH 10/34] linux-user: Support for restarting system calls for Microblaze targets
Fri, 4 Mar 2016 01:27:19 +0100
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
> * 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
> Is that right?
Yes, I think so.
> (For context, the original patch is this one:
> 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 <address@hidden>