Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-21 Thread Glauber de Oliveira Costa
On 8/21/07, Andi Kleen <[EMAIL PROTECTED]> wrote:
> On Tue, Aug 21, 2007 at 04:30:10AM -0300, Glauber de Oliveira Costa wrote:
> > On 8/20/07, Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote:
> > > Chris Wright wrote:
> > > > That did get backed out (at least the part that broke paravirt patching)
> > > > in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
> > > > working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
> > > > committed, and can be further refined with the patch below that's just
> > > > waiting on some further testing.
> > > >
> > >
> > > I don't think this is necessary.  It isn't worth complicating the
> > > interface to avoid the memcpy.
> > >
> > > J
> > Damn,
> >
> > I can't believe I've just lost a night tracking the issue, without
> > seeing the discussion here ;-)
> > I came out to this very same conclusion, and was about to send a patch
> > that fixes it, by doing a memcpy before starting the instruction
> > replacement.
> >
> > (I wouldn't say anything, as this is solved, but my night have to get
> > some value, after all! ;-)
> >
> > So I'm with Jeremy. We don't lose too much by putting a memcpy there,
> > this code is not exactly critical. It also seems cleaner, and less
> > error prone. I have a patch ready here, but I think by this time, you
> > guys have too ;-)
>
> x86-64 also has a __inline_memcpy that is guaranteed inlined. It was
> originally for such cases when memcpy didn't work. Could be added to i386
> too if there is need
>
Andi,

this is not the case that memcpy did not work. (I got the same issue
for paravirt_ops x86_64, and btw, that's why I have not yet sent a new
patch).

The thing is that if that the paravirt replacement function returns,
it will return with the temporary buffer empty, will write the buffer
with text_poke(), and thus, put crap into the code.

So adding a memcpy to the buffer guarantees that in the worst case, it
will contain the original instruction. The normal memcpy works neatly.

-- 
Glauber de Oliveira Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-21 Thread Andi Kleen
On Tue, Aug 21, 2007 at 04:30:10AM -0300, Glauber de Oliveira Costa wrote:
> On 8/20/07, Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote:
> > Chris Wright wrote:
> > > That did get backed out (at least the part that broke paravirt patching)
> > > in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
> > > working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
> > > committed, and can be further refined with the patch below that's just
> > > waiting on some further testing.
> > >
> >
> > I don't think this is necessary.  It isn't worth complicating the
> > interface to avoid the memcpy.
> >
> > J
> Damn,
> 
> I can't believe I've just lost a night tracking the issue, without
> seeing the discussion here ;-)
> I came out to this very same conclusion, and was about to send a patch
> that fixes it, by doing a memcpy before starting the instruction
> replacement.
> 
> (I wouldn't say anything, as this is solved, but my night have to get
> some value, after all! ;-)
> 
> So I'm with Jeremy. We don't lose too much by putting a memcpy there,
> this code is not exactly critical. It also seems cleaner, and less
> error prone. I have a patch ready here, but I think by this time, you
> guys have too ;-)

x86-64 also has a __inline_memcpy that is guaranteed inlined. It was
originally for such cases when memcpy didn't work. Could be added to i386
too if there is need

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-21 Thread Glauber de Oliveira Costa
On 8/20/07, Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote:
> Chris Wright wrote:
> > That did get backed out (at least the part that broke paravirt patching)
> > in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
> > working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
> > committed, and can be further refined with the patch below that's just
> > waiting on some further testing.
> >
>
> I don't think this is necessary.  It isn't worth complicating the
> interface to avoid the memcpy.
>
> J
Damn,

I can't believe I've just lost a night tracking the issue, without
seeing the discussion here ;-)
I came out to this very same conclusion, and was about to send a patch
that fixes it, by doing a memcpy before starting the instruction
replacement.

(I wouldn't say anything, as this is solved, but my night have to get
some value, after all! ;-)

So I'm with Jeremy. We don't lose too much by putting a memcpy there,
this code is not exactly critical. It also seems cleaner, and less
error prone. I have a patch ready here, but I think by this time, you
guys have too ;-)

-- 
Glauber de Oliveira Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-21 Thread Glauber de Oliveira Costa
On 8/20/07, Jeremy Fitzhardinge [EMAIL PROTECTED] wrote:
 Chris Wright wrote:
  That did get backed out (at least the part that broke paravirt patching)
  in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
  working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
  committed, and can be further refined with the patch below that's just
  waiting on some further testing.
 

 I don't think this is necessary.  It isn't worth complicating the
 interface to avoid the memcpy.

 J
Damn,

I can't believe I've just lost a night tracking the issue, without
seeing the discussion here ;-)
I came out to this very same conclusion, and was about to send a patch
that fixes it, by doing a memcpy before starting the instruction
replacement.

(I wouldn't say anything, as this is solved, but my night have to get
some value, after all! ;-)

So I'm with Jeremy. We don't lose too much by putting a memcpy there,
this code is not exactly critical. It also seems cleaner, and less
error prone. I have a patch ready here, but I think by this time, you
guys have too ;-)

-- 
Glauber de Oliveira Costa.
Free as in Freedom
http://glommer.net

The less confident you are, the more serious you have to act.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-21 Thread Andi Kleen
On Tue, Aug 21, 2007 at 04:30:10AM -0300, Glauber de Oliveira Costa wrote:
 On 8/20/07, Jeremy Fitzhardinge [EMAIL PROTECTED] wrote:
  Chris Wright wrote:
   That did get backed out (at least the part that broke paravirt patching)
   in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
   working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
   committed, and can be further refined with the patch below that's just
   waiting on some further testing.
  
 
  I don't think this is necessary.  It isn't worth complicating the
  interface to avoid the memcpy.
 
  J
 Damn,
 
 I can't believe I've just lost a night tracking the issue, without
 seeing the discussion here ;-)
 I came out to this very same conclusion, and was about to send a patch
 that fixes it, by doing a memcpy before starting the instruction
 replacement.
 
 (I wouldn't say anything, as this is solved, but my night have to get
 some value, after all! ;-)
 
 So I'm with Jeremy. We don't lose too much by putting a memcpy there,
 this code is not exactly critical. It also seems cleaner, and less
 error prone. I have a patch ready here, but I think by this time, you
 guys have too ;-)

x86-64 also has a __inline_memcpy that is guaranteed inlined. It was
originally for such cases when memcpy didn't work. Could be added to i386
too if there is need

-Andi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-21 Thread Glauber de Oliveira Costa
On 8/21/07, Andi Kleen [EMAIL PROTECTED] wrote:
 On Tue, Aug 21, 2007 at 04:30:10AM -0300, Glauber de Oliveira Costa wrote:
  On 8/20/07, Jeremy Fitzhardinge [EMAIL PROTECTED] wrote:
   Chris Wright wrote:
That did get backed out (at least the part that broke paravirt patching)
in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
committed, and can be further refined with the patch below that's just
waiting on some further testing.
   
  
   I don't think this is necessary.  It isn't worth complicating the
   interface to avoid the memcpy.
  
   J
  Damn,
 
  I can't believe I've just lost a night tracking the issue, without
  seeing the discussion here ;-)
  I came out to this very same conclusion, and was about to send a patch
  that fixes it, by doing a memcpy before starting the instruction
  replacement.
 
  (I wouldn't say anything, as this is solved, but my night have to get
  some value, after all! ;-)
 
  So I'm with Jeremy. We don't lose too much by putting a memcpy there,
  this code is not exactly critical. It also seems cleaner, and less
  error prone. I have a patch ready here, but I think by this time, you
  guys have too ;-)

 x86-64 also has a __inline_memcpy that is guaranteed inlined. It was
 originally for such cases when memcpy didn't work. Could be added to i386
 too if there is need

Andi,

this is not the case that memcpy did not work. (I got the same issue
for paravirt_ops x86_64, and btw, that's why I have not yet sent a new
patch).

The thing is that if that the paravirt replacement function returns,
it will return with the temporary buffer empty, will write the buffer
with text_poke(), and thus, put crap into the code.

So adding a memcpy to the buffer guarantees that in the worst case, it
will contain the original instruction. The normal memcpy works neatly.

-- 
Glauber de Oliveira Costa.
Free as in Freedom
http://glommer.net

The less confident you are, the more serious you have to act.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-20 Thread Jeremy Fitzhardinge
Chris Wright wrote:
> That did get backed out (at least the part that broke paravirt patching)
> in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
> working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
> committed, and can be further refined with the patch below that's just
> waiting on some further testing.
>   

I don't think this is necessary.  It isn't worth complicating the
interface to avoid the memcpy.

J
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-20 Thread Jeremy Fitzhardinge
Chris Wright wrote:
 That did get backed out (at least the part that broke paravirt patching)
 in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
 working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
 committed, and can be further refined with the patch below that's just
 waiting on some further testing.
   

I don't think this is necessary.  It isn't worth complicating the
interface to avoid the memcpy.

J
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-19 Thread Chris Wright
* Rusty Russell ([EMAIL PROTECTED]) wrote:
> Then back out 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 too, which broke
> lguest booting, and this tried to fix.

That did get backed out (at least the part that broke paravirt patching)
in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
committed, and can be further refined with the patch below that's just
waiting on some further testing.

thanks,
-chris
--

Subject: [PATCH] x86: skip paravirt patching when appropriate
From: Chris Wright <[EMAIL PROTECTED]>

commit d34fda4a84c18402640a1a2342d6e6d9829e6db7 was a little overkill
in the case where a paravirt patcher chooses to leave patch site
unpatched.  Instead of copying original instructions to temp buffer
then back to patch site, simply skip patching those sites altogether.

Cc: Jeremy Fitzhardinge <[EMAIL PROTECTED]>
Cc: Rusty Russell <[EMAIL PROTECTED]>
Cc: Zach Amsden <[EMAIL PROTECTED]>
Signed-off-by: Chris Wright <[EMAIL PROTECTED]>
---
 arch/i386/kernel/alternative.c |4 ++--
 arch/i386/kernel/paravirt.c|   10 +-
 arch/i386/kernel/vmi.c |4 ++--
 include/asm-i386/paravirt.h|3 +++
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/i386/kernel/alternative.c b/arch/i386/kernel/alternative.c
index 9f4ac8b..b81d87e 100644
--- a/arch/i386/kernel/alternative.c
+++ b/arch/i386/kernel/alternative.c
@@ -366,10 +366,10 @@ void apply_paravirt(struct paravirt_patch_site *start,
unsigned int used;
 
BUG_ON(p->len > MAX_PATCH_LEN);
-   /* prep the buffer with the original instructions */
-   memcpy(insnbuf, p->instr, p->len);
used = paravirt_ops.patch(p->instrtype, p->clobbers, insnbuf,
  (unsigned long)p->instr, p->len);
+   if (used == PV_NO_PATCH)
+   continue;
 
BUG_ON(used > p->len);
 
diff --git a/arch/i386/kernel/paravirt.c b/arch/i386/kernel/paravirt.c
index 739cfb2..a36ce34 100644
--- a/arch/i386/kernel/paravirt.c
+++ b/arch/i386/kernel/paravirt.c
@@ -122,7 +122,7 @@ unsigned paravirt_patch_nop(void)
 
 unsigned paravirt_patch_ignore(unsigned len)
 {
-   return len;
+   return PV_NO_PATCH;
 }
 
 struct branch {
@@ -139,9 +139,9 @@ unsigned paravirt_patch_call(void *insnbuf,
unsigned long delta = (unsigned long)target - (addr+5);
 
if (tgt_clobbers & ~site_clobbers)
-   return len; /* target would clobber too much for this site 
*/
+   return PV_NO_PATCH; /* target would clobber too much for 
this site */
if (len < 5)
-   return len; /* call too long for patch site */
+   return PV_NO_PATCH; /* call too long for patch site */
 
b->opcode = 0xe8; /* call */
b->delta = delta;
@@ -157,7 +157,7 @@ unsigned paravirt_patch_jmp(const void *target, void 
*insnbuf,
unsigned long delta = (unsigned long)target - (addr+5);
 
if (len < 5)
-   return len; /* call too long for patch site */
+   return PV_NO_PATCH; /* call too long for patch site */
 
b->opcode = 0xe9;   /* jmp */
b->delta = delta;
@@ -196,7 +196,7 @@ unsigned paravirt_patch_insns(void *insnbuf, unsigned len,
unsigned insn_len = end - start;
 
if (insn_len > len || start == NULL)
-   insn_len = len;
+   insn_len = PV_NO_PATCH;
else
memcpy(insnbuf, start, insn_len);
 
diff --git a/arch/i386/kernel/vmi.c b/arch/i386/kernel/vmi.c
index 18673e0..27ae004 100644
--- a/arch/i386/kernel/vmi.c
+++ b/arch/i386/kernel/vmi.c
@@ -118,7 +118,7 @@ static unsigned patch_internal(int call, unsigned len, void 
*insnbuf,
 
case VMI_RELOCATION_NONE:
/* leave native code in place */
-   break;
+   return PV_NO_PATCH;
 
default:
BUG();
@@ -153,7 +153,7 @@ static unsigned vmi_patch(u8 type, u16 clobbers, void 
*insns,
default:
break;
}
-   return len;
+   return PV_NO_PATCH;
 }
 
 /* CPUID has non-C semantics, and paravirt-ops API doesn't match hardware ISA 
*/
diff --git a/include/asm-i386/paravirt.h b/include/asm-i386/paravirt.h
index 9fa3fa9..b26794f 100644
--- a/include/asm-i386/paravirt.h
+++ b/include/asm-i386/paravirt.h
@@ -252,6 +252,9 @@ extern struct paravirt_ops paravirt_ops;
 #define paravirt_alt(insn_string)  \
_paravirt_alt(insn_string, "%c[paravirt_typenum]", 
"%c[paravirt_clobber]")
 
+enum {
+   PV_NO_PATCH = -1
+};
 unsigned paravirt_patch_nop(void);
 unsigned paravirt_patch_ignore(unsigned len);
 unsigned paravirt_patch_call(void *insnbuf,
-
To unsubscribe from this list: send the line "unsubscribe 

Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-19 Thread Rusty Russell
On Fri, 2007-08-17 at 17:05 -0700, Jeremy Fitzhardinge wrote:
> Andi Kleen wrote:
> > Commit 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 "x86: Fix alternatives
> > and kprobes to remap write-protected kernel text" uses code which is
> > being patched for patching.
> >
> > In particular, paravirt_ops does patching in two stages: first it
> > calls paravirt_ops.patch, then it fills any remaining instructions
> > with nop_out().  nop_out calls text_poke() which calls
> > lookup_address() which calls pgd_val() (aka paravirt_ops.pgd_val):
> > that call site is one of the places we patch.
> >
> > If we always do patching as one single call to text_poke(), we only
> > need make sure we're not patching the memcpy in text_poke itself.
> > This means the prototype to paravirt_ops.patch needs to change, to
> > marshal the new code into a buffer rather than patching in place as it
> > does now.  It also means all patching goes through text_poke(), which
> > is known to be safe (apply_alternatives is also changed to make a
> > single patch).
> >   
> 
> Hi Andi,
> 
> This patch breaks Xen booting.  I get infinite recursive faults during
> patching when this patch is present.  If I boot with
> "noreplace-paravirt" it works OK, and it works as expected if I back
> this patch out.  I haven't tracked down the exact failure mode; its a
> little hard to debug because it overwrites all kernel memory with
> recursive fault stackframes and then finally traps out to Xen when it
> hits the bottom of memory.
> 
> I think we should back this one out before .23.

Then back out 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 too, which broke
lguest booting, and this tried to fix.

Rusty.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-19 Thread Rusty Russell
On Fri, 2007-08-17 at 17:05 -0700, Jeremy Fitzhardinge wrote:
 Andi Kleen wrote:
  Commit 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 x86: Fix alternatives
  and kprobes to remap write-protected kernel text uses code which is
  being patched for patching.
 
  In particular, paravirt_ops does patching in two stages: first it
  calls paravirt_ops.patch, then it fills any remaining instructions
  with nop_out().  nop_out calls text_poke() which calls
  lookup_address() which calls pgd_val() (aka paravirt_ops.pgd_val):
  that call site is one of the places we patch.
 
  If we always do patching as one single call to text_poke(), we only
  need make sure we're not patching the memcpy in text_poke itself.
  This means the prototype to paravirt_ops.patch needs to change, to
  marshal the new code into a buffer rather than patching in place as it
  does now.  It also means all patching goes through text_poke(), which
  is known to be safe (apply_alternatives is also changed to make a
  single patch).

 
 Hi Andi,
 
 This patch breaks Xen booting.  I get infinite recursive faults during
 patching when this patch is present.  If I boot with
 noreplace-paravirt it works OK, and it works as expected if I back
 this patch out.  I haven't tracked down the exact failure mode; its a
 little hard to debug because it overwrites all kernel memory with
 recursive fault stackframes and then finally traps out to Xen when it
 hits the bottom of memory.
 
 I think we should back this one out before .23.

Then back out 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 too, which broke
lguest booting, and this tried to fix.

Rusty.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-19 Thread Chris Wright
* Rusty Russell ([EMAIL PROTECTED]) wrote:
 Then back out 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 too, which broke
 lguest booting, and this tried to fix.

That did get backed out (at least the part that broke paravirt patching)
in 602033ed5907a59ce86f709082a35be047743a86.  Linus' tree should be
working fine right now with d34fda4a84c18402640a1a2342d6e6d9829e6db7
committed, and can be further refined with the patch below that's just
waiting on some further testing.

thanks,
-chris
--

Subject: [PATCH] x86: skip paravirt patching when appropriate
From: Chris Wright [EMAIL PROTECTED]

commit d34fda4a84c18402640a1a2342d6e6d9829e6db7 was a little overkill
in the case where a paravirt patcher chooses to leave patch site
unpatched.  Instead of copying original instructions to temp buffer
then back to patch site, simply skip patching those sites altogether.

Cc: Jeremy Fitzhardinge [EMAIL PROTECTED]
Cc: Rusty Russell [EMAIL PROTECTED]
Cc: Zach Amsden [EMAIL PROTECTED]
Signed-off-by: Chris Wright [EMAIL PROTECTED]
---
 arch/i386/kernel/alternative.c |4 ++--
 arch/i386/kernel/paravirt.c|   10 +-
 arch/i386/kernel/vmi.c |4 ++--
 include/asm-i386/paravirt.h|3 +++
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/i386/kernel/alternative.c b/arch/i386/kernel/alternative.c
index 9f4ac8b..b81d87e 100644
--- a/arch/i386/kernel/alternative.c
+++ b/arch/i386/kernel/alternative.c
@@ -366,10 +366,10 @@ void apply_paravirt(struct paravirt_patch_site *start,
unsigned int used;
 
BUG_ON(p-len  MAX_PATCH_LEN);
-   /* prep the buffer with the original instructions */
-   memcpy(insnbuf, p-instr, p-len);
used = paravirt_ops.patch(p-instrtype, p-clobbers, insnbuf,
  (unsigned long)p-instr, p-len);
+   if (used == PV_NO_PATCH)
+   continue;
 
BUG_ON(used  p-len);
 
diff --git a/arch/i386/kernel/paravirt.c b/arch/i386/kernel/paravirt.c
index 739cfb2..a36ce34 100644
--- a/arch/i386/kernel/paravirt.c
+++ b/arch/i386/kernel/paravirt.c
@@ -122,7 +122,7 @@ unsigned paravirt_patch_nop(void)
 
 unsigned paravirt_patch_ignore(unsigned len)
 {
-   return len;
+   return PV_NO_PATCH;
 }
 
 struct branch {
@@ -139,9 +139,9 @@ unsigned paravirt_patch_call(void *insnbuf,
unsigned long delta = (unsigned long)target - (addr+5);
 
if (tgt_clobbers  ~site_clobbers)
-   return len; /* target would clobber too much for this site 
*/
+   return PV_NO_PATCH; /* target would clobber too much for 
this site */
if (len  5)
-   return len; /* call too long for patch site */
+   return PV_NO_PATCH; /* call too long for patch site */
 
b-opcode = 0xe8; /* call */
b-delta = delta;
@@ -157,7 +157,7 @@ unsigned paravirt_patch_jmp(const void *target, void 
*insnbuf,
unsigned long delta = (unsigned long)target - (addr+5);
 
if (len  5)
-   return len; /* call too long for patch site */
+   return PV_NO_PATCH; /* call too long for patch site */
 
b-opcode = 0xe9;   /* jmp */
b-delta = delta;
@@ -196,7 +196,7 @@ unsigned paravirt_patch_insns(void *insnbuf, unsigned len,
unsigned insn_len = end - start;
 
if (insn_len  len || start == NULL)
-   insn_len = len;
+   insn_len = PV_NO_PATCH;
else
memcpy(insnbuf, start, insn_len);
 
diff --git a/arch/i386/kernel/vmi.c b/arch/i386/kernel/vmi.c
index 18673e0..27ae004 100644
--- a/arch/i386/kernel/vmi.c
+++ b/arch/i386/kernel/vmi.c
@@ -118,7 +118,7 @@ static unsigned patch_internal(int call, unsigned len, void 
*insnbuf,
 
case VMI_RELOCATION_NONE:
/* leave native code in place */
-   break;
+   return PV_NO_PATCH;
 
default:
BUG();
@@ -153,7 +153,7 @@ static unsigned vmi_patch(u8 type, u16 clobbers, void 
*insns,
default:
break;
}
-   return len;
+   return PV_NO_PATCH;
 }
 
 /* CPUID has non-C semantics, and paravirt-ops API doesn't match hardware ISA 
*/
diff --git a/include/asm-i386/paravirt.h b/include/asm-i386/paravirt.h
index 9fa3fa9..b26794f 100644
--- a/include/asm-i386/paravirt.h
+++ b/include/asm-i386/paravirt.h
@@ -252,6 +252,9 @@ extern struct paravirt_ops paravirt_ops;
 #define paravirt_alt(insn_string)  \
_paravirt_alt(insn_string, %c[paravirt_typenum], 
%c[paravirt_clobber])
 
+enum {
+   PV_NO_PATCH = -1
+};
 unsigned paravirt_patch_nop(void);
 unsigned paravirt_patch_ignore(unsigned len);
 unsigned paravirt_patch_call(void *insnbuf,
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to 

Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Jeremy Fitzhardinge
Andi Kleen wrote:
>> This patch breaks Xen booting.  
>> 
>
> Check the latest git head. Does it still break?

Yes, that's with latest git head.

J
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Chris Wright
* Linus Torvalds ([EMAIL PROTECTED]) wrote:
> On Sat, 18 Aug 2007, Chris Wright wrote:
> > > 
> > > Check the latest git head. Does it still break?
> > 
> > Yeah, this is the latest git.  The broken commit is Rusty's patch which,
> > after Linus reverted the write-protected remap changes, is no longer
> > necessary.  AFAICT patching is writing garbage into the insn stream.
> > I suspect it's copying an uninitialized temp buffer.
> 
> Can you send me the revert patch that is verified to work?

Now that I understand the problem, I do have a very simple (slightly
overkill) fix for paravirt patching.  This can be cleaned up to avoid
the copies when they aren't needed, but that will take a little more
auditing of the various patchers.  If you still prefer a revert I've
got one handy, and we can re-visit this all post .23.

thanks,
-chris
--

Subject: [PATCH] x86: properly initialize temp insn buffer for paravirt 
patching 
From: Chris Wright <[EMAIL PROTECTED]>

With commit ab144f5ec64c42218a555ec1dbde6b60cf2982d6 the patching code
now collects the complete new instruction stream into a temp buffer
before finally patching in the new insns.  In some cases the paravirt
patchers will choose to leave the patch site unpatched (length mismatch,
clobbers mismatch, etc).  This causes the new patching code to copy an
uninitialized temp buffer, i.e. garbage, to the callsite.  Simply make
sure to always initialize the buffer with the original instruction stream.
A better fix is to audit all the patchers and return proper length so that
apply_paravirt() can skip copies when we leave the patch site untouched.

Signed-off-by: Chris Wright <[EMAIL PROTECTED]>
---
 arch/i386/kernel/alternative.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/i386/kernel/alternative.c b/arch/i386/kernel/alternative.c
index 1b66d5c..9f4ac8b 100644
--- a/arch/i386/kernel/alternative.c
+++ b/arch/i386/kernel/alternative.c
@@ -366,6 +366,8 @@ void apply_paravirt(struct paravirt_patch_site *start,
unsigned int used;
 
BUG_ON(p->len > MAX_PATCH_LEN);
+   /* prep the buffer with the original instructions */
+   memcpy(insnbuf, p->instr, p->len);
used = paravirt_ops.patch(p->instrtype, p->clobbers, insnbuf,
  (unsigned long)p->instr, p->len);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Linus Torvalds


On Sat, 18 Aug 2007, Chris Wright wrote:
> > 
> > Check the latest git head. Does it still break?
> 
> Yeah, this is the latest git.  The broken commit is Rusty's patch which,
> after Linus reverted the write-protected remap changes, is no longer
> necessary.  AFAICT patching is writing garbage into the insn stream.
> I suspect it's copying an uninitialized temp buffer.

Can you send me the revert patch that is verified to work?

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Chris Wright
* Andi Kleen ([EMAIL PROTECTED]) wrote:
> > This patch breaks Xen booting.  
> 
> Check the latest git head. Does it still break?

Yeah, this is the latest git.  The broken commit is Rusty's patch which,
after Linus reverted the write-protected remap changes, is no longer
necessary.  AFAICT patching is writing garbage into the insn stream.
I suspect it's copying an uninitialized temp buffer.

thanks,
-chris
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Andi Kleen

> This patch breaks Xen booting.  

Check the latest git head. Does it still break?

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Andi Kleen

 This patch breaks Xen booting.  

Check the latest git head. Does it still break?

-Andi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Chris Wright
* Andi Kleen ([EMAIL PROTECTED]) wrote:
  This patch breaks Xen booting.  
 
 Check the latest git head. Does it still break?

Yeah, this is the latest git.  The broken commit is Rusty's patch which,
after Linus reverted the write-protected remap changes, is no longer
necessary.  AFAICT patching is writing garbage into the insn stream.
I suspect it's copying an uninitialized temp buffer.

thanks,
-chris
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Linus Torvalds


On Sat, 18 Aug 2007, Chris Wright wrote:
  
  Check the latest git head. Does it still break?
 
 Yeah, this is the latest git.  The broken commit is Rusty's patch which,
 after Linus reverted the write-protected remap changes, is no longer
 necessary.  AFAICT patching is writing garbage into the insn stream.
 I suspect it's copying an uninitialized temp buffer.

Can you send me the revert patch that is verified to work?

Linus
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Chris Wright
* Linus Torvalds ([EMAIL PROTECTED]) wrote:
 On Sat, 18 Aug 2007, Chris Wright wrote:
   
   Check the latest git head. Does it still break?
  
  Yeah, this is the latest git.  The broken commit is Rusty's patch which,
  after Linus reverted the write-protected remap changes, is no longer
  necessary.  AFAICT patching is writing garbage into the insn stream.
  I suspect it's copying an uninitialized temp buffer.
 
 Can you send me the revert patch that is verified to work?

Now that I understand the problem, I do have a very simple (slightly
overkill) fix for paravirt patching.  This can be cleaned up to avoid
the copies when they aren't needed, but that will take a little more
auditing of the various patchers.  If you still prefer a revert I've
got one handy, and we can re-visit this all post .23.

thanks,
-chris
--

Subject: [PATCH] x86: properly initialize temp insn buffer for paravirt 
patching 
From: Chris Wright [EMAIL PROTECTED]

With commit ab144f5ec64c42218a555ec1dbde6b60cf2982d6 the patching code
now collects the complete new instruction stream into a temp buffer
before finally patching in the new insns.  In some cases the paravirt
patchers will choose to leave the patch site unpatched (length mismatch,
clobbers mismatch, etc).  This causes the new patching code to copy an
uninitialized temp buffer, i.e. garbage, to the callsite.  Simply make
sure to always initialize the buffer with the original instruction stream.
A better fix is to audit all the patchers and return proper length so that
apply_paravirt() can skip copies when we leave the patch site untouched.

Signed-off-by: Chris Wright [EMAIL PROTECTED]
---
 arch/i386/kernel/alternative.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/i386/kernel/alternative.c b/arch/i386/kernel/alternative.c
index 1b66d5c..9f4ac8b 100644
--- a/arch/i386/kernel/alternative.c
+++ b/arch/i386/kernel/alternative.c
@@ -366,6 +366,8 @@ void apply_paravirt(struct paravirt_patch_site *start,
unsigned int used;
 
BUG_ON(p-len  MAX_PATCH_LEN);
+   /* prep the buffer with the original instructions */
+   memcpy(insnbuf, p-instr, p-len);
used = paravirt_ops.patch(p-instrtype, p-clobbers, insnbuf,
  (unsigned long)p-instr, p-len);
 
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-18 Thread Jeremy Fitzhardinge
Andi Kleen wrote:
 This patch breaks Xen booting.  
 

 Check the latest git head. Does it still break?

Yes, that's with latest git head.

J
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-17 Thread Chris Wright
* Jeremy Fitzhardinge ([EMAIL PROTECTED]) wrote:
> This patch breaks Xen booting.  I get infinite recursive faults during
> patching when this patch is present.  If I boot with
> "noreplace-paravirt" it works OK, and it works as expected if I back
> this patch out.  I haven't tracked down the exact failure mode; its a
> little hard to debug because it overwrites all kernel memory with
> recursive fault stackframes and then finally traps out to Xen when it
> hits the bottom of memory.
> 
> I think we should back this one out before .23.

I agree (second time this has broken during .23 devel).

thanks,
-chris
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-17 Thread Jeremy Fitzhardinge
Andi Kleen wrote:
> Commit 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 "x86: Fix alternatives
> and kprobes to remap write-protected kernel text" uses code which is
> being patched for patching.
>
> In particular, paravirt_ops does patching in two stages: first it
> calls paravirt_ops.patch, then it fills any remaining instructions
> with nop_out().  nop_out calls text_poke() which calls
> lookup_address() which calls pgd_val() (aka paravirt_ops.pgd_val):
> that call site is one of the places we patch.
>
> If we always do patching as one single call to text_poke(), we only
> need make sure we're not patching the memcpy in text_poke itself.
> This means the prototype to paravirt_ops.patch needs to change, to
> marshal the new code into a buffer rather than patching in place as it
> does now.  It also means all patching goes through text_poke(), which
> is known to be safe (apply_alternatives is also changed to make a
> single patch).
>   

Hi Andi,

This patch breaks Xen booting.  I get infinite recursive faults during
patching when this patch is present.  If I boot with
"noreplace-paravirt" it works OK, and it works as expected if I back
this patch out.  I haven't tracked down the exact failure mode; its a
little hard to debug because it overwrites all kernel memory with
recursive fault stackframes and then finally traps out to Xen when it
hits the bottom of memory.

I think we should back this one out before .23.

J
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-17 Thread Jeremy Fitzhardinge
Andi Kleen wrote:
 Commit 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 x86: Fix alternatives
 and kprobes to remap write-protected kernel text uses code which is
 being patched for patching.

 In particular, paravirt_ops does patching in two stages: first it
 calls paravirt_ops.patch, then it fills any remaining instructions
 with nop_out().  nop_out calls text_poke() which calls
 lookup_address() which calls pgd_val() (aka paravirt_ops.pgd_val):
 that call site is one of the places we patch.

 If we always do patching as one single call to text_poke(), we only
 need make sure we're not patching the memcpy in text_poke itself.
 This means the prototype to paravirt_ops.patch needs to change, to
 marshal the new code into a buffer rather than patching in place as it
 does now.  It also means all patching goes through text_poke(), which
 is known to be safe (apply_alternatives is also changed to make a
 single patch).
   

Hi Andi,

This patch breaks Xen booting.  I get infinite recursive faults during
patching when this patch is present.  If I boot with
noreplace-paravirt it works OK, and it works as expected if I back
this patch out.  I haven't tracked down the exact failure mode; its a
little hard to debug because it overwrites all kernel memory with
recursive fault stackframes and then finally traps out to Xen when it
hits the bottom of memory.

I think we should back this one out before .23.

J
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-17 Thread Chris Wright
* Jeremy Fitzhardinge ([EMAIL PROTECTED]) wrote:
 This patch breaks Xen booting.  I get infinite recursive faults during
 patching when this patch is present.  If I boot with
 noreplace-paravirt it works OK, and it works as expected if I back
 this patch out.  I haven't tracked down the exact failure mode; its a
 little hard to debug because it overwrites all kernel memory with
 recursive fault stackframes and then finally traps out to Xen when it
 hits the bottom of memory.
 
 I think we should back this one out before .23.

I agree (second time this has broken during .23 devel).

thanks,
-chris
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-09 Thread Andi Kleen

Commit 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 "x86: Fix alternatives
and kprobes to remap write-protected kernel text" uses code which is
being patched for patching.

In particular, paravirt_ops does patching in two stages: first it
calls paravirt_ops.patch, then it fills any remaining instructions
with nop_out().  nop_out calls text_poke() which calls
lookup_address() which calls pgd_val() (aka paravirt_ops.pgd_val):
that call site is one of the places we patch.

If we always do patching as one single call to text_poke(), we only
need make sure we're not patching the memcpy in text_poke itself.
This means the prototype to paravirt_ops.patch needs to change, to
marshal the new code into a buffer rather than patching in place as it
does now.  It also means all patching goes through text_poke(), which
is known to be safe (apply_alternatives is also changed to make a
single patch).

AK: fix compilation on x86-64 (bad rusty!)
AK: fix boot on x86-64 (sigh)
AK: merged with other patches

Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>

---
 arch/i386/kernel/alternative.c |   33 --
 arch/i386/kernel/paravirt.c|   52 -
 arch/i386/kernel/vmi.c |   35 ---
 arch/i386/xen/enlighten.c  |   12 +
 drivers/lguest/lguest.c|9 +++
 include/asm-i386/paravirt.h|   16 +++-
 6 files changed, 90 insertions(+), 67 deletions(-)

Index: linux/arch/i386/kernel/alternative.c
===
--- linux.orig/arch/i386/kernel/alternative.c
+++ linux/arch/i386/kernel/alternative.c
@@ -11,6 +11,8 @@
 #include 
 #include 
 
+#define MAX_PATCH_LEN (255-1)
+
 #ifdef CONFIG_HOTPLUG_CPU
 static int smp_alt_once;
 
@@ -148,7 +150,8 @@ static unsigned char** find_nop_table(vo
 
 #endif /* CONFIG_X86_64 */
 
-static void nop_out(void *insns, unsigned int len)
+/* Use this to add nops to a buffer, then text_poke the whole buffer. */
+static void add_nops(void *insns, unsigned int len)
 {
unsigned char **noptable = find_nop_table();
 
@@ -156,7 +159,7 @@ static void nop_out(void *insns, unsigne
unsigned int noplen = len;
if (noplen > ASM_NOP_MAX)
noplen = ASM_NOP_MAX;
-   text_poke(insns, noptable[noplen], noplen);
+   memcpy(insns, noptable[noplen], noplen);
insns += noplen;
len -= noplen;
}
@@ -174,15 +177,15 @@ extern u8 *__smp_locks[], *__smp_locks_e
 void apply_alternatives(struct alt_instr *start, struct alt_instr *end)
 {
struct alt_instr *a;
-   u8 *instr;
-   int diff;
+   char insnbuf[MAX_PATCH_LEN];
 
DPRINTK("%s: alt table %p -> %p\n", __FUNCTION__, start, end);
for (a = start; a < end; a++) {
+   u8 *instr = a->instr;
BUG_ON(a->replacementlen > a->instrlen);
+   BUG_ON(a->instrlen > sizeof(insnbuf));
if (!boot_cpu_has(a->cpuid))
continue;
-   instr = a->instr;
 #ifdef CONFIG_X86_64
/* vsyscall code is not mapped yet. resolve it manually. */
if (instr >= (u8 *)VSYSCALL_START && instr < (u8*)VSYSCALL_END) 
{
@@ -191,9 +194,10 @@ void apply_alternatives(struct alt_instr
__FUNCTION__, a->instr, instr);
}
 #endif
-   memcpy(instr, a->replacement, a->replacementlen);
-   diff = a->instrlen - a->replacementlen;
-   nop_out(instr + a->replacementlen, diff);
+   memcpy(insnbuf, a->replacement, a->replacementlen);
+   add_nops(insnbuf + a->replacementlen,
+a->instrlen - a->replacementlen);
+   text_poke(instr, insnbuf, a->instrlen);
}
 }
 
@@ -215,16 +219,18 @@ static void alternatives_smp_lock(u8 **s
 static void alternatives_smp_unlock(u8 **start, u8 **end, u8 *text, u8 
*text_end)
 {
u8 **ptr;
+   char insn[1];
 
if (noreplace_smp)
return;
 
+   add_nops(insn, 1);
for (ptr = start; ptr < end; ptr++) {
if (*ptr < text)
continue;
if (*ptr > text_end)
continue;
-   nop_out(*ptr, 1);
+   text_poke(*ptr, insn, 1);
};
 }
 
@@ -351,6 +357,7 @@ void apply_paravirt(struct paravirt_patc
struct paravirt_patch_site *end)
 {
struct paravirt_patch_site *p;
+   char insnbuf[MAX_PATCH_LEN];
 
if (noreplace_paravirt)
return;
@@ -358,13 +365,15 @@ void apply_paravirt(struct paravirt_patc
for (p = start; p < end; p++) {
unsigned int used;
 
-   used = paravirt_ops.patch(p->instrtype, p->clobbers, p->instr,
-

[PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

2007-08-09 Thread Andi Kleen

Commit 19d36ccdc34f5ed444f8a6af0cbfdb6790eb1177 x86: Fix alternatives
and kprobes to remap write-protected kernel text uses code which is
being patched for patching.

In particular, paravirt_ops does patching in two stages: first it
calls paravirt_ops.patch, then it fills any remaining instructions
with nop_out().  nop_out calls text_poke() which calls
lookup_address() which calls pgd_val() (aka paravirt_ops.pgd_val):
that call site is one of the places we patch.

If we always do patching as one single call to text_poke(), we only
need make sure we're not patching the memcpy in text_poke itself.
This means the prototype to paravirt_ops.patch needs to change, to
marshal the new code into a buffer rather than patching in place as it
does now.  It also means all patching goes through text_poke(), which
is known to be safe (apply_alternatives is also changed to make a
single patch).

AK: fix compilation on x86-64 (bad rusty!)
AK: fix boot on x86-64 (sigh)
AK: merged with other patches

Signed-off-by: Rusty Russell [EMAIL PROTECTED]
Signed-off-by: Andi Kleen [EMAIL PROTECTED]

---
 arch/i386/kernel/alternative.c |   33 --
 arch/i386/kernel/paravirt.c|   52 -
 arch/i386/kernel/vmi.c |   35 ---
 arch/i386/xen/enlighten.c  |   12 +
 drivers/lguest/lguest.c|9 +++
 include/asm-i386/paravirt.h|   16 +++-
 6 files changed, 90 insertions(+), 67 deletions(-)

Index: linux/arch/i386/kernel/alternative.c
===
--- linux.orig/arch/i386/kernel/alternative.c
+++ linux/arch/i386/kernel/alternative.c
@@ -11,6 +11,8 @@
 #include asm/mce.h
 #include asm/nmi.h
 
+#define MAX_PATCH_LEN (255-1)
+
 #ifdef CONFIG_HOTPLUG_CPU
 static int smp_alt_once;
 
@@ -148,7 +150,8 @@ static unsigned char** find_nop_table(vo
 
 #endif /* CONFIG_X86_64 */
 
-static void nop_out(void *insns, unsigned int len)
+/* Use this to add nops to a buffer, then text_poke the whole buffer. */
+static void add_nops(void *insns, unsigned int len)
 {
unsigned char **noptable = find_nop_table();
 
@@ -156,7 +159,7 @@ static void nop_out(void *insns, unsigne
unsigned int noplen = len;
if (noplen  ASM_NOP_MAX)
noplen = ASM_NOP_MAX;
-   text_poke(insns, noptable[noplen], noplen);
+   memcpy(insns, noptable[noplen], noplen);
insns += noplen;
len -= noplen;
}
@@ -174,15 +177,15 @@ extern u8 *__smp_locks[], *__smp_locks_e
 void apply_alternatives(struct alt_instr *start, struct alt_instr *end)
 {
struct alt_instr *a;
-   u8 *instr;
-   int diff;
+   char insnbuf[MAX_PATCH_LEN];
 
DPRINTK(%s: alt table %p - %p\n, __FUNCTION__, start, end);
for (a = start; a  end; a++) {
+   u8 *instr = a-instr;
BUG_ON(a-replacementlen  a-instrlen);
+   BUG_ON(a-instrlen  sizeof(insnbuf));
if (!boot_cpu_has(a-cpuid))
continue;
-   instr = a-instr;
 #ifdef CONFIG_X86_64
/* vsyscall code is not mapped yet. resolve it manually. */
if (instr = (u8 *)VSYSCALL_START  instr  (u8*)VSYSCALL_END) 
{
@@ -191,9 +194,10 @@ void apply_alternatives(struct alt_instr
__FUNCTION__, a-instr, instr);
}
 #endif
-   memcpy(instr, a-replacement, a-replacementlen);
-   diff = a-instrlen - a-replacementlen;
-   nop_out(instr + a-replacementlen, diff);
+   memcpy(insnbuf, a-replacement, a-replacementlen);
+   add_nops(insnbuf + a-replacementlen,
+a-instrlen - a-replacementlen);
+   text_poke(instr, insnbuf, a-instrlen);
}
 }
 
@@ -215,16 +219,18 @@ static void alternatives_smp_lock(u8 **s
 static void alternatives_smp_unlock(u8 **start, u8 **end, u8 *text, u8 
*text_end)
 {
u8 **ptr;
+   char insn[1];
 
if (noreplace_smp)
return;
 
+   add_nops(insn, 1);
for (ptr = start; ptr  end; ptr++) {
if (*ptr  text)
continue;
if (*ptr  text_end)
continue;
-   nop_out(*ptr, 1);
+   text_poke(*ptr, insn, 1);
};
 }
 
@@ -351,6 +357,7 @@ void apply_paravirt(struct paravirt_patc
struct paravirt_patch_site *end)
 {
struct paravirt_patch_site *p;
+   char insnbuf[MAX_PATCH_LEN];
 
if (noreplace_paravirt)
return;
@@ -358,13 +365,15 @@ void apply_paravirt(struct paravirt_patc
for (p = start; p  end; p++) {
unsigned int used;
 
-   used = paravirt_ops.patch(p-instrtype, p-clobbers, p-instr,
- p-len);
+