public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Roland McGrath <roland@redhat.com>,
	Jason Baron <jbaron@redhat.com>,
	linux-kernel@vger.kernel.org, mingo@elte.hu, tglx@linutronix.de,
	rostedt@goodmis.org, ak@suse.de, rth@redhat.com
Subject: Re: [PATCH 0/4] jump label patches
Date: Tue, 6 Oct 2009 10:30:56 -0400	[thread overview]
Message-ID: <20091006143056.GB3951@Krystal> (raw)
In-Reply-To: <4ACB5082.6050902@redhat.com>

* Masami Hiramatsu (mhiramat@redhat.com) wrote:
> Roland McGrath wrote:
> > I think text_poke_fixup() is a good safe place to start, and it seems wise
> > to merge a version using that before worrying anything subtler.  But it's
> > almost surely overkill and makes the enable/disable switching cost pretty
> > huge.  The rules as documented by Intel seem to indicate that simple
> > self-modification can work for UP and for SMP there should be some scheme
> > with IPIs that is not too terrible.  
> > 
> > Those can entail a multi-phase modification like the int3 patching style,
> > but int3 is not the only way to do it.  int3 has the benefit of being a
> > one-byte instruction you can patch in, but also the downside of requiring
> > the trap handling hair.
> 
> Hmm, would you want to put tracepoint on the path of int3 handling?
> 
> >  Another approach is:
> > 
> > start:
> > 	   .balign 2
> > 	2: nopl
> > 	7: ...
> > 
> > phase 1:
> > 	2: jmp 7
> > 	4: <last 3 bytes of nopl>
> > 	7: ...
> > 
> > phase 2:
> > 	2: jmp 7
> > 	4: {last 3 bytes of "jmp .Ldo_trace"}
> > 	7: ...
> > 
> > phase 3:
> > 	2: jmp .Ldo_trace
> > 	7: ...
> > 
> > A scheme like that requires that the instruction to be patched be 2-byte
> > aligned so that the two-byte "jmp .+3" can be an atomic store not
> > straddling a word boundary.  On x86-64 (and, according to the Intel book,
> > everything >= Pentium), you can atomically store 8 bytes when aligned.  So
> > there you will usually actually be able to do this in one or two phases to
> > cover each particular 5 byte range with adequately aligned stores.
> 
> It is unclear whether we can atomically modify 2 bytes in icache (also, it
> can across cache lines or pages.)
> I think int3 bypassing is more generic way to patching if you don't mind
> tracing int3 path :-)

I think their point is that by only changing the 2nd byte of a 2-byte
jmp instruction, leaving the opcode as-is, they think the processor will
either choose one or the other branch, seeing two "coherent"
instructions.

However, as I point out in the immediate values thread, the problem
might run deeper, which would be that CPU need to see consistent
instructions across multiple reads in the same code region (due to
pipeline effects of some sorts). Mere atomicity of the modification does
not seem to be enough. What I gathered by discussing with Richard J
Moore is that we really need a synchronizing instruction between the
moments CPUs see the old and new version. int3/iret is one. Sending an
IPI doing a mfence or cpuid is another. Both seem needed to ensure
SMP-safe code modification.

See the immediate values thread for details. We are waiting for Intel
ruling on the int3+IPI scheme.

Mathieu

> 
> 
> Thank you,
> 
> -- 
> Masami Hiramatsu
> 
> Software Engineer
> Hitachi Computer Products (America), Inc.
> Software Solutions Division
> 
> e-mail: mhiramat@redhat.com
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

      reply	other threads:[~2009-10-06 14:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-24 23:17 [PATCH 0/4] jump label patches Jason Baron
2009-09-24 23:17 ` [PATCH 1/4] jump label - make init_kernel_text() global Jason Baron
2009-10-01 11:20   ` Ingo Molnar
2009-10-01 12:58     ` Mathieu Desnoyers
2009-10-01 20:39     ` Jason Baron
2009-10-03 10:43       ` Ingo Molnar
2009-10-03 12:39         ` Mathieu Desnoyers
2009-10-07  1:54           ` Steven Rostedt
2009-10-07  2:32             ` Mathieu Desnoyers
2009-10-07  3:10               ` Masami Hiramatsu
2009-10-07  3:23                 ` Mathieu Desnoyers
2009-10-07  3:29               ` Mathieu Desnoyers
2009-10-07 12:56               ` Steven Rostedt
2009-10-07 13:35                 ` Mathieu Desnoyers
2009-09-24 23:17 ` [PATCH 2/4] jump label - base patch Jason Baron
2009-09-25  0:49   ` Roland McGrath
2009-09-26 10:21     ` Steven Rostedt
2009-10-01 11:36   ` Ingo Molnar
2009-09-24 23:17 ` [PATCH 3/4] jump label - add module support Jason Baron
2009-09-24 23:18 ` [PATCH 4/4] jump label - tracepoint implementation Jason Baron
2009-10-06  5:39 ` [PATCH 0/4] jump label patches Roland McGrath
2009-10-06 14:07   ` Jason Baron
2009-10-06 23:24   ` Richard Henderson
2009-10-07  0:14     ` Roland McGrath
2009-10-07 15:35       ` Richard Henderson
2009-10-06  6:04 ` Roland McGrath
2009-10-06 14:09   ` Steven Rostedt
2009-10-06 14:13   ` Masami Hiramatsu
2009-10-06 14:30     ` Mathieu Desnoyers [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091006143056.GB3951@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=ak@suse.de \
    --cc=jbaron@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=rth@redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox