Re: [PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step

2012-08-01 Thread Ananth N Mavinakayanahalli
On Wed, Aug 01, 2012 at 03:43:37PM +0200, Oleg Nesterov wrote: ... > However, I am not sure we can trust it. We are in kernel mode, > DEBUGCTLMSR_BTF can be cleared by kprobes (Ananth, please correct me). > I think we need to check TIF_BLOCKSTEP. Kprobes resets DEBUGCTLMSR_BTF only if we have to

Re: [PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step

2012-08-01 Thread Oleg Nesterov
See my previous emails... and a couple of other nits. On 07/31, Sebastian Andrzej Siewior wrote: > > +static int insn_is_popf(const u8 *insn) > +{ > + /* popf */ > + if (insn[0] == 0x9d) > + return 1; > + return 0; > +} I can't believe I am going to blame the naming ;) Bu

Re: [PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step

2012-08-01 Thread Oleg Nesterov
On 07/31, Sebastian Andrzej Siewior wrote: > > On 07/31/2012 07:51 PM, Oleg Nesterov wrote: >> However, honestly I do not like it. I think we should change this >> step-by-step, that is why I suggested to use TIF_SINGLESTEP and >> user_enable_single_step() like your initial patch did. With this >>

Re: [PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step

2012-07-31 Thread Sebastian Andrzej Siewior
On 07/31/2012 07:51 PM, Oleg Nesterov wrote: However, honestly I do not like it. I think we should change this step-by-step, that is why I suggested to use TIF_SINGLESTEP and user_enable_single_step() like your initial patch did. With this patch at least the debugger doesn't lose the control over

Re: [PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step

2012-07-31 Thread Oleg Nesterov
Oh, Sebastian, I'll try to read this patch tomorrow, but I am not expert anyway. However, honestly I do not like it. I think we should change this step-by-step, that is why I suggested to use TIF_SINGLESTEP and user_enable_single_step() like your initial patch did. With this patch at least the deb

[PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step

2012-07-31 Thread Sebastian Andrzej Siewior
The arch specific implementation enables single stepping directly by setting the trap flag. "Single-Step on branches" is always disabled since only one opcode has to be executed. The disable call removes the trap flag unless it was there before. It does not touch the flags register if the executed