| Message ID | 73c9ffac157087da78af9fca59cf7d8db7f17226.1669290510.git.houwenlong.hwl@antgroup.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp3354456wrr;
Thu, 24 Nov 2022 04:05:30 -0800 (PST)
X-Google-Smtp-Source:
AA0mqf6/vGflVKjaGx1+/yxShC7ANhc2VqmhkqawWm70kfGZeErP0h6RhtUN16cFspymsc6VDvhs
X-Received: by 2002:a17:907:7f9f:b0:7b2:a48e:302f with SMTP id
qk31-20020a1709077f9f00b007b2a48e302fmr25614697ejc.334.1669291530474;
Thu, 24 Nov 2022 04:05:30 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1669291530; cv=none;
d=google.com; s=arc-20160816;
b=VgDDDGZAnSxBYSD9lhdN563xuFtU4KQpMFlIdCq4hUn3QrtfH92wUmFb9lB3GfPgx3
7luVXc4tjSNLlWRGgVXAoiysmxFkr6v4abAqn5ZkA6k+g1o8JMWvB7gVw/gLHQYfQljq
G/k5LtMtpeEVtxgqq33JpwZvBNY8AQKeUQ15I09RxyaqQJUyL1eIKksXaoXaCSqM8S+u
+LlB9dZ8JNssZeYPKsdJy4//+QwIazbwCPvtvDaTSSLRJRzxGCvT8Q0GwbovKGSP28c2
SGp/M44tLqh/jIJPLfmj9RHfJWoj1p0q4oH8SZYJlQV9EYT/4VGNf40ySKUEPj5yvami
glkA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:content-transfer-encoding:mime-version
:message-id:date:subject:cc:to:from;
bh=Ln7xQMGd5lkxLqTRio2Pi+5KNULCqhCKcbEjd0gCn68=;
b=HW8qzYzM9aO/EOhmrgUVKT5vvlpHWT+8IF516ZD9EWfLUFh6ebmBP0moN22Qgh6Nnt
AoccaUmTOA0IlDGrUlSk5t0YqasOdHOmpfr3SUSAEOSYkFIlWTIu0lLfWOpQeFBWP72Y
Cr2tXzAoQB24NcH3zqGxF5sA7BRWkDVg+m1/8dxOU+pg6Yt7GF9K6KS+WLFmh37NpGxq
v9qU9B5Pm7ezvuZJa/+qlZSG93RTa5b4Hq/C5g6MMOgYdisAi46OM3jvLM0kmhXr1OsU
1m1IR7zWk6sjE4EpbEBUNxUgQ6+rUZe2jMdP9V4hDxrQOs3b1gOW+czOsmOnXJWORekA
6oNA==
ARC-Authentication-Results: i=1; mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE)
header.from=antgroup.com
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
he44-20020a1709073dac00b0078dee0fce23si241408ejc.123.2022.11.24.04.05.06;
Thu, 24 Nov 2022 04:05:30 -0800 (PST)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE)
header.from=antgroup.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S229552AbiKXLwL (ORCPT <rfc822;fengqi706@gmail.com> + 99 others);
Thu, 24 Nov 2022 06:52:11 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57892 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S229838AbiKXLwK (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Thu, 24 Nov 2022 06:52:10 -0500
Received: from out0-144.mail.aliyun.com (out0-144.mail.aliyun.com
[140.205.0.144])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9C16A9370B
for <linux-kernel@vger.kernel.org>;
Thu, 24 Nov 2022 03:52:08 -0800 (PST)
X-Alimail-AntiSpam:
AC=PASS;BC=-1|-1;BR=01201311R181e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018047206;MF=houwenlong.hwl@antgroup.com;NM=1;PH=DS;RN=18;SR=0;TI=SMTPD_---.QFud9ec_1669290723;
Received: from localhost(mailfrom:houwenlong.hwl@antgroup.com
fp:SMTPD_---.QFud9ec_1669290723)
by smtp.aliyun-inc.com;
Thu, 24 Nov 2022 19:52:03 +0800
From: "Hou Wenlong" <houwenlong.hwl@antgroup.com>
To: linux-kernel@vger.kernel.org
Cc: "Hou Wenlong" <houwenlong.hwl@antgroup.com>,
"Juergen Gross" <jgross@suse.com>, " =?utf-8?q?Srivatsa_S=2E_Bhat_=28VMware?=
=?utf-8?q?=29?= " <srivatsa@csail.mit.edu>,
"Alexey Makhalov" <amakhalov@vmware.com>,
"VMware PV-Drivers Reviewers" <pv-drivers@vmware.com>,
"Thomas Gleixner" <tglx@linutronix.de>, "Ingo Molnar" <mingo@redhat.com>,
"Borislav Petkov" <bp@alien8.de>,
"Dave Hansen" <dave.hansen@linux.intel.com>, <x86@kernel.org>,
"H. Peter Anvin" <hpa@zytor.com>, "Peter Zijlstra" <peterz@infradead.org>,
"Josh Poimboeuf" <jpoimboe@kernel.org>, "Kees Cook" <keescook@chromium.org>,
"Song Liu" <song@kernel.org>, "Nadav Amit" <namit@vmware.com>,
<virtualization@lists.linux-foundation.org>
Subject: [PATCH v2] x86/paravirt: Use relative reference for original
instruction
Date: Thu, 24 Nov 2022 19:51:53 +0800
Message-Id:
<73c9ffac157087da78af9fca59cf7d8db7f17226.1669290510.git.houwenlong.hwl@antgroup.com>
X-Mailer: git-send-email 2.31.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE,
SPF_PASS,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no
version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1750378919738161262?=
X-GMAIL-MSGID: =?utf-8?q?1750379035763018515?=
|
| Series |
[v2] x86/paravirt: Use relative reference for original instruction
|
|
Commit Message
Hou Wenlong
Nov. 24, 2022, 11:51 a.m. UTC
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@antgroup.com>
---
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(-)
Comments
On November 24, 2022 3:51:53 AM PST, Hou Wenlong <houwenlong.hwl@antgroup.com> 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@antgroup.com> >--- > 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?
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 <houwenlong.hwl@antgroup.com> 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@antgroup.com> > >--- > > 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()?
On November 27, 2022 7:03:20 PM PST, Hou Wenlong <houwenlong.hwl@antgroup.com> 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 <houwenlong.hwl@antgroup.com> 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@antgroup.com> >> >--- >> > 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.
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 <houwenlong.hwl@antgroup.com> 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 <houwenlong.hwl@antgroup.com> 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@antgroup.com> > >> >--- > >> > 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?
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[],