2022-11-24 12:06:57

by Hou Wenlong

[permalink] [raw]
Subject: [PATCH v2] x86/paravirt: Use relative reference for original instruction

Similar to the alternative patching, use relative reference for original
instruction rather than absolute one, which saves 8 bytes for one entry
on x86_64. And it could generate R_X86_64_PC32 relocation instead of
R_X86_64_64 relocation, which also reduces relocation metadata on
relocatable builds. And the alignment could be hard coded to be 4 now.

Signed-off-by: Hou Wenlong <[email protected]>
---
arch/x86/include/asm/paravirt.h | 10 +++++-----
arch/x86/include/asm/paravirt_types.h | 8 ++++----
arch/x86/kernel/alternative.c | 8 +++++---
3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 2851bc2339d5..e56065ea73f2 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -735,16 +735,16 @@ extern void default_banner(void);

#else /* __ASSEMBLY__ */

-#define _PVSITE(ptype, ops, word, algn) \
+#define _PVSITE(ptype, ops) \
771:; \
ops; \
772:; \
.pushsection .parainstructions,"a"; \
- .align algn; \
- word 771b; \
+ .align 4; \
+ .long 771b-.; \
.byte ptype; \
.byte 772b-771b; \
- _ASM_ALIGN; \
+ .align 4; \
.popsection


@@ -752,7 +752,7 @@ extern void default_banner(void);
#ifdef CONFIG_PARAVIRT_XXL

#define PARA_PATCH(off) ((off) / 8)
-#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops, .quad, 8)
+#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops)
#define PARA_INDIRECT(addr) *addr(%rip)

#ifdef CONFIG_DEBUG_ENTRY
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 8c1da419260f..68952ae07a3f 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -5,7 +5,7 @@
#ifndef __ASSEMBLY__
/* These all sit in the .parainstructions section to tell us what to patch. */
struct paravirt_patch_site {
- u8 *instr; /* original instructions */
+ s32 instr_offset; /* original instructions */
u8 type; /* type of this instruction */
u8 len; /* length of original instruction */
};
@@ -273,11 +273,11 @@ extern struct paravirt_patch_template pv_ops;
#define _paravirt_alt(insn_string, type) \
"771:\n\t" insn_string "\n" "772:\n" \
".pushsection .parainstructions,\"a\"\n" \
- _ASM_ALIGN "\n" \
- _ASM_PTR " 771b\n" \
+ " .align 4\n" \
+ " .long 771b-.\n" \
" .byte " type "\n" \
" .byte 772b-771b\n" \
- _ASM_ALIGN "\n" \
+ " .align 4\n" \
".popsection\n"

/* Generate patchable code, with the default asm parameters. */
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 111b809f0ac2..6eea563a098d 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1232,20 +1232,22 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
{
struct paravirt_patch_site *p;
char insn_buff[MAX_PATCH_LEN];
+ u8 *instr;

for (p = start; p < end; p++) {
unsigned int used;

+ instr = (u8 *)&p->instr_offset + p->instr_offset;
BUG_ON(p->len > MAX_PATCH_LEN);
/* prep the buffer with the original instructions */
- memcpy(insn_buff, p->instr, p->len);
- used = paravirt_patch(p->type, insn_buff, (unsigned long)p->instr, p->len);
+ memcpy(insn_buff, instr, p->len);
+ used = paravirt_patch(p->type, insn_buff, (unsigned long)instr, p->len);

BUG_ON(used > p->len);

/* Pad the rest with nops */
add_nops(insn_buff + used, p->len - used);
- text_poke_early(p->instr, insn_buff, p->len);
+ text_poke_early(instr, insn_buff, p->len);
}
}
extern struct paravirt_patch_site __start_parainstructions[],
--
2.31.1


2022-11-24 12:20:55

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v2] x86/paravirt: Use relative reference for original instruction

On 24.11.22 12:51, Hou Wenlong wrote:
> Similar to the alternative patching, use relative reference for original
> instruction rather than absolute one, which saves 8 bytes for one entry
> on x86_64. And it could generate R_X86_64_PC32 relocation instead of
> R_X86_64_64 relocation, which also reduces relocation metadata on
> relocatable builds. And the alignment could be hard coded to be 4 now.
>
> Signed-off-by: Hou Wenlong <[email protected]>

Reviewed-by: Juergen Gross <[email protected]>


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.08 kB)
OpenPGP public key
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments

2022-11-27 17:35:49

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2] x86/paravirt: Use relative reference for original instruction

On November 24, 2022 3:51:53 AM PST, Hou Wenlong <[email protected]> wrote:
>Similar to the alternative patching, use relative reference for original
>instruction rather than absolute one, which saves 8 bytes for one entry
>on x86_64. And it could generate R_X86_64_PC32 relocation instead of
>R_X86_64_64 relocation, which also reduces relocation metadata on
>relocatable builds. And the alignment could be hard coded to be 4 now.
>
>Signed-off-by: Hou Wenlong <[email protected]>
>---
> arch/x86/include/asm/paravirt.h | 10 +++++-----
> arch/x86/include/asm/paravirt_types.h | 8 ++++----
> arch/x86/kernel/alternative.c | 8 +++++---
> 3 files changed, 14 insertions(+), 12 deletions(-)
>
>diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
>index 2851bc2339d5..e56065ea73f2 100644
>--- a/arch/x86/include/asm/paravirt.h
>+++ b/arch/x86/include/asm/paravirt.h
>@@ -735,16 +735,16 @@ extern void default_banner(void);
>
> #else /* __ASSEMBLY__ */
>
>-#define _PVSITE(ptype, ops, word, algn) \
>+#define _PVSITE(ptype, ops) \
> 771:; \
> ops; \
> 772:; \
> .pushsection .parainstructions,"a"; \
>- .align algn; \
>- word 771b; \
>+ .align 4; \
>+ .long 771b-.; \
> .byte ptype; \
> .byte 772b-771b; \
>- _ASM_ALIGN; \
>+ .align 4; \
> .popsection
>
>
>@@ -752,7 +752,7 @@ extern void default_banner(void);
> #ifdef CONFIG_PARAVIRT_XXL
>
> #define PARA_PATCH(off) ((off) / 8)
>-#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops, .quad, 8)
>+#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops)
> #define PARA_INDIRECT(addr) *addr(%rip)
>
> #ifdef CONFIG_DEBUG_ENTRY
>diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>index 8c1da419260f..68952ae07a3f 100644
>--- a/arch/x86/include/asm/paravirt_types.h
>+++ b/arch/x86/include/asm/paravirt_types.h
>@@ -5,7 +5,7 @@
> #ifndef __ASSEMBLY__
> /* These all sit in the .parainstructions section to tell us what to patch. */
> struct paravirt_patch_site {
>- u8 *instr; /* original instructions */
>+ s32 instr_offset; /* original instructions */
> u8 type; /* type of this instruction */
> u8 len; /* length of original instruction */
> };
>@@ -273,11 +273,11 @@ extern struct paravirt_patch_template pv_ops;
> #define _paravirt_alt(insn_string, type) \
> "771:\n\t" insn_string "\n" "772:\n" \
> ".pushsection .parainstructions,\"a\"\n" \
>- _ASM_ALIGN "\n" \
>- _ASM_PTR " 771b\n" \
>+ " .align 4\n" \
>+ " .long 771b-.\n" \
> " .byte " type "\n" \
> " .byte 772b-771b\n" \
>- _ASM_ALIGN "\n" \
>+ " .align 4\n" \
> ".popsection\n"
>
> /* Generate patchable code, with the default asm parameters. */
>diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>index 111b809f0ac2..6eea563a098d 100644
>--- a/arch/x86/kernel/alternative.c
>+++ b/arch/x86/kernel/alternative.c
>@@ -1232,20 +1232,22 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
> {
> struct paravirt_patch_site *p;
> char insn_buff[MAX_PATCH_LEN];
>+ u8 *instr;
>
> for (p = start; p < end; p++) {
> unsigned int used;
>
>+ instr = (u8 *)&p->instr_offset + p->instr_offset;
> BUG_ON(p->len > MAX_PATCH_LEN);
> /* prep the buffer with the original instructions */
>- memcpy(insn_buff, p->instr, p->len);
>- used = paravirt_patch(p->type, insn_buff, (unsigned long)p->instr, p->len);
>+ memcpy(insn_buff, instr, p->len);
>+ used = paravirt_patch(p->type, insn_buff, (unsigned long)instr, p->len);
>
> BUG_ON(used > p->len);
>
> /* Pad the rest with nops */
> add_nops(insn_buff + used, p->len - used);
>- text_poke_early(p->instr, insn_buff, p->len);
>+ text_poke_early(instr, insn_buff, p->len);
> }
> }
> extern struct paravirt_patch_site __start_parainstructions[],

Any reason that you couldn't use the same patching code?

2022-11-28 04:05:33

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2] x86/paravirt: Use relative reference for original instruction

On November 27, 2022 7:03:20 PM PST, Hou Wenlong <[email protected]> wrote:
>On Sun, Nov 27, 2022 at 09:24:34AM -0800, H. Peter Anvin wrote:
>> On November 24, 2022 3:51:53 AM PST, Hou Wenlong <[email protected]> wrote:
>> >Similar to the alternative patching, use relative reference for original
>> >instruction rather than absolute one, which saves 8 bytes for one entry
>> >on x86_64. And it could generate R_X86_64_PC32 relocation instead of
>> >R_X86_64_64 relocation, which also reduces relocation metadata on
>> >relocatable builds. And the alignment could be hard coded to be 4 now.
>> >
>> >Signed-off-by: Hou Wenlong <[email protected]>
>> >---
>> > arch/x86/include/asm/paravirt.h | 10 +++++-----
>> > arch/x86/include/asm/paravirt_types.h | 8 ++++----
>> > arch/x86/kernel/alternative.c | 8 +++++---
>> > 3 files changed, 14 insertions(+), 12 deletions(-)
>> >
>> >diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
>> >index 2851bc2339d5..e56065ea73f2 100644
>> >--- a/arch/x86/include/asm/paravirt.h
>> >+++ b/arch/x86/include/asm/paravirt.h
>> >@@ -735,16 +735,16 @@ extern void default_banner(void);
>> >
>> > #else /* __ASSEMBLY__ */
>> >
>> >-#define _PVSITE(ptype, ops, word, algn) \
>> >+#define _PVSITE(ptype, ops) \
>> > 771:; \
>> > ops; \
>> > 772:; \
>> > .pushsection .parainstructions,"a"; \
>> >- .align algn; \
>> >- word 771b; \
>> >+ .align 4; \
>> >+ .long 771b-.; \
>> > .byte ptype; \
>> > .byte 772b-771b; \
>> >- _ASM_ALIGN; \
>> >+ .align 4; \
>> > .popsection
>> >
>> >
>> >@@ -752,7 +752,7 @@ extern void default_banner(void);
>> > #ifdef CONFIG_PARAVIRT_XXL
>> >
>> > #define PARA_PATCH(off) ((off) / 8)
>> >-#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops, .quad, 8)
>> >+#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops)
>> > #define PARA_INDIRECT(addr) *addr(%rip)
>> >
>> > #ifdef CONFIG_DEBUG_ENTRY
>> >diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>> >index 8c1da419260f..68952ae07a3f 100644
>> >--- a/arch/x86/include/asm/paravirt_types.h
>> >+++ b/arch/x86/include/asm/paravirt_types.h
>> >@@ -5,7 +5,7 @@
>> > #ifndef __ASSEMBLY__
>> > /* These all sit in the .parainstructions section to tell us what to patch. */
>> > struct paravirt_patch_site {
>> >- u8 *instr; /* original instructions */
>> >+ s32 instr_offset; /* original instructions */
>> > u8 type; /* type of this instruction */
>> > u8 len; /* length of original instruction */
>> > };
>> >@@ -273,11 +273,11 @@ extern struct paravirt_patch_template pv_ops;
>> > #define _paravirt_alt(insn_string, type) \
>> > "771:\n\t" insn_string "\n" "772:\n" \
>> > ".pushsection .parainstructions,\"a\"\n" \
>> >- _ASM_ALIGN "\n" \
>> >- _ASM_PTR " 771b\n" \
>> >+ " .align 4\n" \
>> >+ " .long 771b-.\n" \
>> > " .byte " type "\n" \
>> > " .byte 772b-771b\n" \
>> >- _ASM_ALIGN "\n" \
>> >+ " .align 4\n" \
>> > ".popsection\n"
>> >
>> > /* Generate patchable code, with the default asm parameters. */
>> >diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>> >index 111b809f0ac2..6eea563a098d 100644
>> >--- a/arch/x86/kernel/alternative.c
>> >+++ b/arch/x86/kernel/alternative.c
>> >@@ -1232,20 +1232,22 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
>> > {
>> > struct paravirt_patch_site *p;
>> > char insn_buff[MAX_PATCH_LEN];
>> >+ u8 *instr;
>> >
>> > for (p = start; p < end; p++) {
>> > unsigned int used;
>> >
>> >+ instr = (u8 *)&p->instr_offset + p->instr_offset;
>> > BUG_ON(p->len > MAX_PATCH_LEN);
>> > /* prep the buffer with the original instructions */
>> >- memcpy(insn_buff, p->instr, p->len);
>> >- used = paravirt_patch(p->type, insn_buff, (unsigned long)p->instr, p->len);
>> >+ memcpy(insn_buff, instr, p->len);
>> >+ used = paravirt_patch(p->type, insn_buff, (unsigned long)instr, p->len);
>> >
>> > BUG_ON(used > p->len);
>> >
>> > /* Pad the rest with nops */
>> > add_nops(insn_buff + used, p->len - used);
>> >- text_poke_early(p->instr, insn_buff, p->len);
>> >+ text_poke_early(instr, insn_buff, p->len);
>> > }
>> > }
>> > extern struct paravirt_patch_site __start_parainstructions[],
>>
>> Any reason that you couldn't use the same patching code?
>
>Sorry, what do you mean using the same patching code? Do you
>mean that share some code between apply_alternatives() and
>apply_paravirt()?

Yes. Abstract the facility rather than duplicate.

2022-11-28 04:22:59

by Hou Wenlong

[permalink] [raw]
Subject: Re: [PATCH v2] x86/paravirt: Use relative reference for original instruction

On Sun, Nov 27, 2022 at 09:24:34AM -0800, H. Peter Anvin wrote:
> On November 24, 2022 3:51:53 AM PST, Hou Wenlong <[email protected]> wrote:
> >Similar to the alternative patching, use relative reference for original
> >instruction rather than absolute one, which saves 8 bytes for one entry
> >on x86_64. And it could generate R_X86_64_PC32 relocation instead of
> >R_X86_64_64 relocation, which also reduces relocation metadata on
> >relocatable builds. And the alignment could be hard coded to be 4 now.
> >
> >Signed-off-by: Hou Wenlong <[email protected]>
> >---
> > arch/x86/include/asm/paravirt.h | 10 +++++-----
> > arch/x86/include/asm/paravirt_types.h | 8 ++++----
> > arch/x86/kernel/alternative.c | 8 +++++---
> > 3 files changed, 14 insertions(+), 12 deletions(-)
> >
> >diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> >index 2851bc2339d5..e56065ea73f2 100644
> >--- a/arch/x86/include/asm/paravirt.h
> >+++ b/arch/x86/include/asm/paravirt.h
> >@@ -735,16 +735,16 @@ extern void default_banner(void);
> >
> > #else /* __ASSEMBLY__ */
> >
> >-#define _PVSITE(ptype, ops, word, algn) \
> >+#define _PVSITE(ptype, ops) \
> > 771:; \
> > ops; \
> > 772:; \
> > .pushsection .parainstructions,"a"; \
> >- .align algn; \
> >- word 771b; \
> >+ .align 4; \
> >+ .long 771b-.; \
> > .byte ptype; \
> > .byte 772b-771b; \
> >- _ASM_ALIGN; \
> >+ .align 4; \
> > .popsection
> >
> >
> >@@ -752,7 +752,7 @@ extern void default_banner(void);
> > #ifdef CONFIG_PARAVIRT_XXL
> >
> > #define PARA_PATCH(off) ((off) / 8)
> >-#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops, .quad, 8)
> >+#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops)
> > #define PARA_INDIRECT(addr) *addr(%rip)
> >
> > #ifdef CONFIG_DEBUG_ENTRY
> >diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> >index 8c1da419260f..68952ae07a3f 100644
> >--- a/arch/x86/include/asm/paravirt_types.h
> >+++ b/arch/x86/include/asm/paravirt_types.h
> >@@ -5,7 +5,7 @@
> > #ifndef __ASSEMBLY__
> > /* These all sit in the .parainstructions section to tell us what to patch. */
> > struct paravirt_patch_site {
> >- u8 *instr; /* original instructions */
> >+ s32 instr_offset; /* original instructions */
> > u8 type; /* type of this instruction */
> > u8 len; /* length of original instruction */
> > };
> >@@ -273,11 +273,11 @@ extern struct paravirt_patch_template pv_ops;
> > #define _paravirt_alt(insn_string, type) \
> > "771:\n\t" insn_string "\n" "772:\n" \
> > ".pushsection .parainstructions,\"a\"\n" \
> >- _ASM_ALIGN "\n" \
> >- _ASM_PTR " 771b\n" \
> >+ " .align 4\n" \
> >+ " .long 771b-.\n" \
> > " .byte " type "\n" \
> > " .byte 772b-771b\n" \
> >- _ASM_ALIGN "\n" \
> >+ " .align 4\n" \
> > ".popsection\n"
> >
> > /* Generate patchable code, with the default asm parameters. */
> >diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> >index 111b809f0ac2..6eea563a098d 100644
> >--- a/arch/x86/kernel/alternative.c
> >+++ b/arch/x86/kernel/alternative.c
> >@@ -1232,20 +1232,22 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
> > {
> > struct paravirt_patch_site *p;
> > char insn_buff[MAX_PATCH_LEN];
> >+ u8 *instr;
> >
> > for (p = start; p < end; p++) {
> > unsigned int used;
> >
> >+ instr = (u8 *)&p->instr_offset + p->instr_offset;
> > BUG_ON(p->len > MAX_PATCH_LEN);
> > /* prep the buffer with the original instructions */
> >- memcpy(insn_buff, p->instr, p->len);
> >- used = paravirt_patch(p->type, insn_buff, (unsigned long)p->instr, p->len);
> >+ memcpy(insn_buff, instr, p->len);
> >+ used = paravirt_patch(p->type, insn_buff, (unsigned long)instr, p->len);
> >
> > BUG_ON(used > p->len);
> >
> > /* Pad the rest with nops */
> > add_nops(insn_buff + used, p->len - used);
> >- text_poke_early(p->instr, insn_buff, p->len);
> >+ text_poke_early(instr, insn_buff, p->len);
> > }
> > }
> > extern struct paravirt_patch_site __start_parainstructions[],
>
> Any reason that you couldn't use the same patching code?

Sorry, what do you mean using the same patching code? Do you
mean that share some code between apply_alternatives() and
apply_paravirt()?

2022-11-28 04:45:37

by Hou Wenlong

[permalink] [raw]
Subject: Re: [PATCH v2] x86/paravirt: Use relative reference for original instruction

On Sun, Nov 27, 2022 at 07:34:39PM -0800, H. Peter Anvin wrote:
> On November 27, 2022 7:03:20 PM PST, Hou Wenlong <[email protected]> wrote:
> >On Sun, Nov 27, 2022 at 09:24:34AM -0800, H. Peter Anvin wrote:
> >> On November 24, 2022 3:51:53 AM PST, Hou Wenlong <[email protected]> wrote:
> >> >Similar to the alternative patching, use relative reference for original
> >> >instruction rather than absolute one, which saves 8 bytes for one entry
> >> >on x86_64. And it could generate R_X86_64_PC32 relocation instead of
> >> >R_X86_64_64 relocation, which also reduces relocation metadata on
> >> >relocatable builds. And the alignment could be hard coded to be 4 now.
> >> >
> >> >Signed-off-by: Hou Wenlong <[email protected]>
> >> >---
> >> > arch/x86/include/asm/paravirt.h | 10 +++++-----
> >> > arch/x86/include/asm/paravirt_types.h | 8 ++++----
> >> > arch/x86/kernel/alternative.c | 8 +++++---
> >> > 3 files changed, 14 insertions(+), 12 deletions(-)
> >> >
> >> >diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> >> >index 2851bc2339d5..e56065ea73f2 100644
> >> >--- a/arch/x86/include/asm/paravirt.h
> >> >+++ b/arch/x86/include/asm/paravirt.h
> >> >@@ -735,16 +735,16 @@ extern void default_banner(void);
> >> >
> >> > #else /* __ASSEMBLY__ */
> >> >
> >> >-#define _PVSITE(ptype, ops, word, algn) \
> >> >+#define _PVSITE(ptype, ops) \
> >> > 771:; \
> >> > ops; \
> >> > 772:; \
> >> > .pushsection .parainstructions,"a"; \
> >> >- .align algn; \
> >> >- word 771b; \
> >> >+ .align 4; \
> >> >+ .long 771b-.; \
> >> > .byte ptype; \
> >> > .byte 772b-771b; \
> >> >- _ASM_ALIGN; \
> >> >+ .align 4; \
> >> > .popsection
> >> >
> >> >
> >> >@@ -752,7 +752,7 @@ extern void default_banner(void);
> >> > #ifdef CONFIG_PARAVIRT_XXL
> >> >
> >> > #define PARA_PATCH(off) ((off) / 8)
> >> >-#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops, .quad, 8)
> >> >+#define PARA_SITE(ptype, ops) _PVSITE(ptype, ops)
> >> > #define PARA_INDIRECT(addr) *addr(%rip)
> >> >
> >> > #ifdef CONFIG_DEBUG_ENTRY
> >> >diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> >> >index 8c1da419260f..68952ae07a3f 100644
> >> >--- a/arch/x86/include/asm/paravirt_types.h
> >> >+++ b/arch/x86/include/asm/paravirt_types.h
> >> >@@ -5,7 +5,7 @@
> >> > #ifndef __ASSEMBLY__
> >> > /* These all sit in the .parainstructions section to tell us what to patch. */
> >> > struct paravirt_patch_site {
> >> >- u8 *instr; /* original instructions */
> >> >+ s32 instr_offset; /* original instructions */
> >> > u8 type; /* type of this instruction */
> >> > u8 len; /* length of original instruction */
> >> > };
> >> >@@ -273,11 +273,11 @@ extern struct paravirt_patch_template pv_ops;
> >> > #define _paravirt_alt(insn_string, type) \
> >> > "771:\n\t" insn_string "\n" "772:\n" \
> >> > ".pushsection .parainstructions,\"a\"\n" \
> >> >- _ASM_ALIGN "\n" \
> >> >- _ASM_PTR " 771b\n" \
> >> >+ " .align 4\n" \
> >> >+ " .long 771b-.\n" \
> >> > " .byte " type "\n" \
> >> > " .byte 772b-771b\n" \
> >> >- _ASM_ALIGN "\n" \
> >> >+ " .align 4\n" \
> >> > ".popsection\n"
> >> >
> >> > /* Generate patchable code, with the default asm parameters. */
> >> >diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> >> >index 111b809f0ac2..6eea563a098d 100644
> >> >--- a/arch/x86/kernel/alternative.c
> >> >+++ b/arch/x86/kernel/alternative.c
> >> >@@ -1232,20 +1232,22 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
> >> > {
> >> > struct paravirt_patch_site *p;
> >> > char insn_buff[MAX_PATCH_LEN];
> >> >+ u8 *instr;
> >> >
> >> > for (p = start; p < end; p++) {
> >> > unsigned int used;
> >> >
> >> >+ instr = (u8 *)&p->instr_offset + p->instr_offset;
> >> > BUG_ON(p->len > MAX_PATCH_LEN);
> >> > /* prep the buffer with the original instructions */
> >> >- memcpy(insn_buff, p->instr, p->len);
> >> >- used = paravirt_patch(p->type, insn_buff, (unsigned long)p->instr, p->len);
> >> >+ memcpy(insn_buff, instr, p->len);
> >> >+ used = paravirt_patch(p->type, insn_buff, (unsigned long)instr, p->len);
> >> >
> >> > BUG_ON(used > p->len);
> >> >
> >> > /* Pad the rest with nops */
> >> > add_nops(insn_buff + used, p->len - used);
> >> >- text_poke_early(p->instr, insn_buff, p->len);
> >> >+ text_poke_early(instr, insn_buff, p->len);
> >> > }
> >> > }
> >> > extern struct paravirt_patch_site __start_parainstructions[],
> >>
> >> Any reason that you couldn't use the same patching code?
> >
> >Sorry, what do you mean using the same patching code? Do you
> >mean that share some code between apply_alternatives() and
> >apply_paravirt()?
>
> Yes. Abstract the facility rather than duplicate.

The structure of patching entry is different between paravirt
patching and alternative patching. The only same logic of those
two patching functions is iteration in the section. The patching
way is really diffferent. I can abstract the facility like this:

#define for_each_patch_entry(p, start, end, patch_func) \
for (p = start; p < end; p++) { \
u8 *instr; \
char insn_buff[MAX_PATCH_LEN]; \
instr = p->instr_offset + p->instr_offset; \
BUG_ON(p->len > MAX_PATCH_LEN); \
used = patch_func(p, instr, insn_buff); \
add_nops(insn_buff + used, p->len - used); \
text_poke_early(instr, insn_buff, p->len); \
}

But I think it is ugly :(. Do you have any better ideas?