[PATCH] x86: skip paravirt patching when appropriate

From: Chris Wright
Date: Sat Aug 18 2007 - 18:29:22 EST


* Chris Wright (chrisw@xxxxxxxxxxxx) wrote:
> 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.

This one avoids the patching when necessary, but needs some validation on
VMI (and Zach's not avail today). I'll resend when I know it's working
for all paravirt patchers.

thanks,
-chris
--

Subject: [PATCH] x86: skip paravirt patching when appropriate
From: Chris Wright <chrisw@xxxxxxxxxxxx>

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 <jeremy@xxxxxxxx>
Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
Cc: Zach Amsden <zach@xxxxxxxxxx>
Signed-off-by: Chris Wright <chrisw@xxxxxxxxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/