diff mbox series

[v3,08/15] x86: add new features for paravirt patching

Message ID 20201217093133.1507-9-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series x86: major paravirt cleanup | expand

Commit Message

Jürgen Groß Dec. 17, 2020, 9:31 a.m. UTC 
For being able to switch paravirt patching from special cased custom
code sequences to ALTERNATIVE handling some X86_FEATURE_* are needed
as new features. This enables to have the standard indirect pv call
as the default code and to patch that with the non-Xen custom code
sequence via ALTERNATIVE patching later.

Make sure paravirt patching is performed before alternative patching.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- add comment (Boris Petkov)
- no negative features (Boris Petkov)
---
 arch/x86/include/asm/cpufeatures.h |  2 ++
 arch/x86/kernel/alternative.c      | 40 ++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

Comments

kernel test robot Dec. 17, 2020, 1:12 p.m. UTC | #1 
Hi Juergen,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.10]
[cannot apply to xen-tip/linux-next tip/x86/core tip/x86/asm next-20201217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Juergen-Gross/x86-major-paravirt-cleanup/20201217-173646
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git accefff5b547a9a1d959c7e76ad539bf2480e78b
config: i386-randconfig-r021-20201217 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/032ee351da7a8adab17b0306cf5908b02f5728d2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Juergen-Gross/x86-major-paravirt-cleanup/20201217-173646
        git checkout 032ee351da7a8adab17b0306cf5908b02f5728d2
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: arch/x86/kernel/alternative.o: in function `paravirt_set_cap':
>> arch/x86/kernel/alternative.c:605: undefined reference to `pv_is_native_spin_unlock'
>> ld: arch/x86/kernel/alternative.c:608: undefined reference to `pv_is_native_vcpu_is_preempted'


vim +605 arch/x86/kernel/alternative.c

   601	
   602	#ifdef CONFIG_PARAVIRT
   603	static void __init paravirt_set_cap(void)
   604	{
 > 605		if (!pv_is_native_spin_unlock())
   606			setup_force_cpu_cap(X86_FEATURE_PVUNLOCK);
   607	
 > 608		if (!pv_is_native_vcpu_is_preempted())
   609			setup_force_cpu_cap(X86_FEATURE_VCPUPREEMPT);
   610	}
   611	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Borislav Petkov Jan. 14, 2021, 6:30 p.m. UTC | #2 
On Thu, Dec 17, 2020 at 09:12:57PM +0800, kernel test robot wrote:
>    ld: arch/x86/kernel/alternative.o: in function `paravirt_set_cap':
> >> arch/x86/kernel/alternative.c:605: undefined reference to `pv_is_native_spin_unlock'
> >> ld: arch/x86/kernel/alternative.c:608: undefined reference to `pv_is_native_vcpu_is_preempted'

Looks like alternative.c needs #include <asm/paravirt.h>.
Borislav Petkov Jan. 14, 2021, 6:40 p.m. UTC | #3 
On Thu, Jan 14, 2021 at 07:30:24PM +0100, Borislav Petkov wrote:
> On Thu, Dec 17, 2020 at 09:12:57PM +0800, kernel test robot wrote:
> >    ld: arch/x86/kernel/alternative.o: in function `paravirt_set_cap':
> > >> arch/x86/kernel/alternative.c:605: undefined reference to `pv_is_native_spin_unlock'
> > >> ld: arch/x86/kernel/alternative.c:608: undefined reference to `pv_is_native_vcpu_is_preempted'
> 
> Looks like alternative.c needs #include <asm/paravirt.h>.

Nah, that needs paravirt_set_cap() to be inside
CONFIG_PARAVIRT_SPINLOCKS-enabled ifdeffery.

Jürgen, why don't you move it to arch/x86/kernel/paravirt.c? That
should keep the ifdeffery low.

Thx.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index f5ef2d5b9231..1077b675a008 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -237,6 +237,8 @@ 
 #define X86_FEATURE_VMCALL		( 8*32+18) /* "" Hypervisor supports the VMCALL instruction */
 #define X86_FEATURE_VMW_VMMCALL		( 8*32+19) /* "" VMware prefers VMMCALL hypercall instruction */
 #define X86_FEATURE_SEV_ES		( 8*32+20) /* AMD Secure Encrypted Virtualization - Encrypted State */
+#define X86_FEATURE_PVUNLOCK		( 8*32+21) /* "" PV unlock function */
+#define X86_FEATURE_VCPUPREEMPT		( 8*32+22) /* "" PV vcpu_is_preempted function */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (EBX), word 9 */
 #define X86_FEATURE_FSGSBASE		( 9*32+ 0) /* RDFSBASE, WRFSBASE, RDGSBASE, WRGSBASE instructions*/
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 0a904fb2678b..abb481808811 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -600,6 +600,15 @@  int alternatives_text_reserved(void *start, void *end)
 #endif /* CONFIG_SMP */
 
 #ifdef CONFIG_PARAVIRT
+static void __init paravirt_set_cap(void)
+{
+	if (!pv_is_native_spin_unlock())
+		setup_force_cpu_cap(X86_FEATURE_PVUNLOCK);
+
+	if (!pv_is_native_vcpu_is_preempted())
+		setup_force_cpu_cap(X86_FEATURE_VCPUPREEMPT);
+}
+
 void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
 				     struct paravirt_patch_site *end)
 {
@@ -623,6 +632,8 @@  void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
 }
 extern struct paravirt_patch_site __start_parainstructions[],
 	__stop_parainstructions[];
+#else
+static void __init paravirt_set_cap(void) { }
 #endif	/* CONFIG_PARAVIRT */
 
 /*
@@ -730,6 +741,33 @@  void __init alternative_instructions(void)
 	 * patching.
 	 */
 
+	/*
+	 * Paravirt patching and alternative patching can be combined to
+	 * replace a function call with a short direct code sequence (e.g.
+	 * by setting a constant return value instead of doing that in an
+	 * external function).
+	 * In order to make this work the following sequence is required:
+	 * 1. set (artificial) features depending on used paravirt
+	 *    functions which can later influence alternative patching
+	 * 2. apply paravirt patching (generally replacing an indirect
+	 *    function call with a direct one)
+	 * 3. apply alternative patching (e.g. replacing a direct function
+	 *    call with a custom code sequence)
+	 * Doing paravirt patching after alternative patching would clobber
+	 * the optimization of the custom code with a function call again.
+	 */
+	paravirt_set_cap();
+
+	/*
+	 * First patch paravirt functions, such that we overwrite the indirect
+	 * call with the direct call.
+	 */
+	apply_paravirt(__parainstructions, __parainstructions_end);
+
+	/*
+	 * Then patch alternatives, such that those paravirt calls that are in
+	 * alternatives can be overwritten by their immediate fragments.
+	 */
 	apply_alternatives(__alt_instructions, __alt_instructions_end);
 
 #ifdef CONFIG_SMP
@@ -748,8 +786,6 @@  void __init alternative_instructions(void)
 	}
 #endif
 
-	apply_paravirt(__parainstructions, __parainstructions_end);
-
 	restart_nmi();
 	alternatives_patched = 1;
 }