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.
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to