Re: [PATCH v2] x86/paravirt: Use relative reference for original instruction
From: H. Peter Anvin
Date: Sun Nov 27 2022 - 12:25:42 EST
On November 24, 2022 3:51:53 AM PST, Hou Wenlong <houwenlong.hwl@xxxxxxxxxxxx> 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 <houwenlong.hwl@xxxxxxxxxxxx>
>---
> 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?