Re: [PATCH] x86: skip paravirt patching when appropriate
Chris Wright wrote: * Chris Wright ([EMAIL PROTECTED]) 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. Looks like it's already in -git. I tested with your fix and hit a different bug. Bisecting, will handle. Zach - 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] x86: skip paravirt patching when appropriate
Chris Wright wrote: * Chris Wright ([EMAIL PROTECTED]) 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. Looks like it's already in -git. I tested with your fix and hit a different bug. Bisecting, will handle. Zach - 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] x86: skip paravirt patching when appropriate
Chris Wright wrote: * Chris Wright ([EMAIL PROTECTED]) 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. I'm back. I'll give this one a spin after dinner tonight. Zach - 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] x86: skip paravirt patching when appropriate
* Chris Wright ([EMAIL PROTECTED]) 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 <[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 =
[PATCH] x86: skip paravirt patching when appropriate
* Chris Wright ([EMAIL PROTECTED]) 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 [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
Re: [PATCH] x86: skip paravirt patching when appropriate
Chris Wright wrote: * Chris Wright ([EMAIL PROTECTED]) 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. I'm back. I'll give this one a spin after dinner tonight. Zach - 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/
