diff mbox

[1/3] paravirt,xen: remove xen_patch()

Message ID 20170810162453.umt72mwdvvzitene@hirez.programming.kicks-ass.net (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Zijlstra Aug. 10, 2017, 4:24 p.m. UTC
On Thu, Aug 10, 2017 at 02:52:52PM +0200, Juergen Gross wrote:
> Xen's paravirt patch function xen_patch() does some special casing for
> irq_ops functions to apply relocations when those functions can be
> patched inline instead of calls.
> 
> Unfortunately none of the special case function replacements is small
> enough to be patches inline, so the special case never applies.
> 
> As xen_patch() will call paravirt_patch_default() in all cases it can
> be just dropped.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/xen/enlighten_pv.c | 59 +--------------------------------------------
>  1 file changed, 1 insertion(+), 58 deletions(-)

> -		SITE(pv_irq_ops, irq_enable);
> -		SITE(pv_irq_ops, irq_disable);
> -		SITE(pv_irq_ops, save_fl);
> -		SITE(pv_irq_ops, restore_fl);

You forgot to remove the actual ASM that's then never used either.


 arch/x86/xen/Makefile  |   2 +-
 arch/x86/xen/xen-asm.S | 150 -------------------------------------------------
 arch/x86/xen/xen-ops.h |  12 ----
 3 files changed, 1 insertion(+), 163 deletions(-)

Comments

Peter Zijlstra Aug. 10, 2017, 4:29 p.m. UTC | #1
On Thu, Aug 10, 2017 at 06:24:53PM +0200, Peter Zijlstra wrote:
> -ENTRY(xen_irq_enable_direct)
> -	FRAME_BEGIN
> -	/* Unmask events */
> -	movb $0, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_mask
> -
> -	/*
> -	 * Preempt here doesn't matter because that will deal with any
> -	 * pending interrupts.  The pending check may end up being run
> -	 * on the wrong CPU, but that doesn't hurt.
> -	 */
> -
> -	/* Test for pending */
> -	testb $0xff, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_pending
> -	jz 1f
> -
> -2:	call check_events
> -1:
> -ENDPATCH(xen_irq_enable_direct)
> -	FRAME_END
> -	ret
> -	ENDPROC(xen_irq_enable_direct)
> -	RELOC(xen_irq_enable_direct, 2b+1)

Oh my bad, part of that is still used.

arch/x86/xen/enlighten_pv.c:            pv_irq_ops.irq_enable = __PV_IS_CALLEE_SAVE(xen_irq_enable_direct);

It just needs cleanups for the ENDPATCH and such.
Jürgen Groß Aug. 10, 2017, 5:30 p.m. UTC | #2
On 10/08/17 18:29, Peter Zijlstra wrote:
> On Thu, Aug 10, 2017 at 06:24:53PM +0200, Peter Zijlstra wrote:
>> -ENTRY(xen_irq_enable_direct)
>> -	FRAME_BEGIN
>> -	/* Unmask events */
>> -	movb $0, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_mask
>> -
>> -	/*
>> -	 * Preempt here doesn't matter because that will deal with any
>> -	 * pending interrupts.  The pending check may end up being run
>> -	 * on the wrong CPU, but that doesn't hurt.
>> -	 */
>> -
>> -	/* Test for pending */
>> -	testb $0xff, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_pending
>> -	jz 1f
>> -
>> -2:	call check_events
>> -1:
>> -ENDPATCH(xen_irq_enable_direct)
>> -	FRAME_END
>> -	ret
>> -	ENDPROC(xen_irq_enable_direct)
>> -	RELOC(xen_irq_enable_direct, 2b+1)
> 
> Oh my bad, part of that is still used.
> 
> arch/x86/xen/enlighten_pv.c:            pv_irq_ops.irq_enable = __PV_IS_CALLEE_SAVE(xen_irq_enable_direct);
> 
> It just needs cleanups for the ENDPATCH and such.

Ah yes, of course.


Juergen
diff mbox

Patch

diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index bced7a369a11..551e12ba2a7b 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -14,7 +14,7 @@  CFLAGS_enlighten_pv.o		:= $(nostackp)
 CFLAGS_mmu_pv.o		:= $(nostackp)
 
 obj-y		:= enlighten.o multicalls.o mmu.o irq.o \
-			time.o xen-asm.o xen-asm_$(BITS).o \
+			time.o xen-asm_$(BITS).o \
 			grant-table.o suspend.o platform-pci-unplug.o
 
 obj-$(CONFIG_XEN_PVHVM)		+= enlighten_hvm.o mmu_hvm.o suspend_hvm.o
diff --git a/arch/x86/xen/xen-asm.S b/arch/x86/xen/xen-asm.S
deleted file mode 100644
index eff224df813f..000000000000
--- a/arch/x86/xen/xen-asm.S
+++ /dev/null
@@ -1,150 +0,0 @@ 
-/*
- * Asm versions of Xen pv-ops, suitable for either direct use or
- * inlining.  The inline versions are the same as the direct-use
- * versions, with the pre- and post-amble chopped off.
- *
- * This code is encoded for size rather than absolute efficiency, with
- * a view to being able to inline as much as possible.
- *
- * We only bother with direct forms (ie, vcpu in percpu data) of the
- * operations here; the indirect forms are better handled in C, since
- * they're generally too large to inline anyway.
- */
-
-#include <asm/asm-offsets.h>
-#include <asm/percpu.h>
-#include <asm/processor-flags.h>
-#include <asm/frame.h>
-
-#include "xen-asm.h"
-
-/*
- * Enable events.  This clears the event mask and tests the pending
- * event status with one and operation.  If there are pending events,
- * then enter the hypervisor to get them handled.
- */
-ENTRY(xen_irq_enable_direct)
-	FRAME_BEGIN
-	/* Unmask events */
-	movb $0, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_mask
-
-	/*
-	 * Preempt here doesn't matter because that will deal with any
-	 * pending interrupts.  The pending check may end up being run
-	 * on the wrong CPU, but that doesn't hurt.
-	 */
-
-	/* Test for pending */
-	testb $0xff, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_pending
-	jz 1f
-
-2:	call check_events
-1:
-ENDPATCH(xen_irq_enable_direct)
-	FRAME_END
-	ret
-	ENDPROC(xen_irq_enable_direct)
-	RELOC(xen_irq_enable_direct, 2b+1)
-
-
-/*
- * Disabling events is simply a matter of making the event mask
- * non-zero.
- */
-ENTRY(xen_irq_disable_direct)
-	movb $1, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_mask
-ENDPATCH(xen_irq_disable_direct)
-	ret
-	ENDPROC(xen_irq_disable_direct)
-	RELOC(xen_irq_disable_direct, 0)
-
-/*
- * (xen_)save_fl is used to get the current interrupt enable status.
- * Callers expect the status to be in X86_EFLAGS_IF, and other bits
- * may be set in the return value.  We take advantage of this by
- * making sure that X86_EFLAGS_IF has the right value (and other bits
- * in that byte are 0), but other bits in the return value are
- * undefined.  We need to toggle the state of the bit, because Xen and
- * x86 use opposite senses (mask vs enable).
- */
-ENTRY(xen_save_fl_direct)
-	testb $0xff, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_mask
-	setz %ah
-	addb %ah, %ah
-ENDPATCH(xen_save_fl_direct)
-	ret
-	ENDPROC(xen_save_fl_direct)
-	RELOC(xen_save_fl_direct, 0)
-
-
-/*
- * In principle the caller should be passing us a value return from
- * xen_save_fl_direct, but for robustness sake we test only the
- * X86_EFLAGS_IF flag rather than the whole byte. After setting the
- * interrupt mask state, it checks for unmasked pending events and
- * enters the hypervisor to get them delivered if so.
- */
-ENTRY(xen_restore_fl_direct)
-	FRAME_BEGIN
-#ifdef CONFIG_X86_64
-	testw $X86_EFLAGS_IF, %di
-#else
-	testb $X86_EFLAGS_IF>>8, %ah
-#endif
-	setz PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_mask
-	/*
-	 * Preempt here doesn't matter because that will deal with any
-	 * pending interrupts.  The pending check may end up being run
-	 * on the wrong CPU, but that doesn't hurt.
-	 */
-
-	/* check for unmasked and pending */
-	cmpw $0x0001, PER_CPU_VAR(xen_vcpu_info) + XEN_vcpu_info_pending
-	jnz 1f
-2:	call check_events
-1:
-ENDPATCH(xen_restore_fl_direct)
-	FRAME_END
-	ret
-	ENDPROC(xen_restore_fl_direct)
-	RELOC(xen_restore_fl_direct, 2b+1)
-
-
-/*
- * Force an event check by making a hypercall, but preserve regs
- * before making the call.
- */
-ENTRY(check_events)
-	FRAME_BEGIN
-#ifdef CONFIG_X86_32
-	push %eax
-	push %ecx
-	push %edx
-	call xen_force_evtchn_callback
-	pop %edx
-	pop %ecx
-	pop %eax
-#else
-	push %rax
-	push %rcx
-	push %rdx
-	push %rsi
-	push %rdi
-	push %r8
-	push %r9
-	push %r10
-	push %r11
-	call xen_force_evtchn_callback
-	pop %r11
-	pop %r10
-	pop %r9
-	pop %r8
-	pop %rdi
-	pop %rsi
-	pop %rdx
-	pop %rcx
-	pop %rax
-#endif
-	FRAME_END
-	ret
-ENDPROC(check_events)
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 0d5004477db6..7b54fd7e9915 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -129,18 +129,6 @@  static inline void __init xen_efi_init(void)
 }
 #endif
 
-/* Declare an asm function, along with symbols needed to make it
-   inlineable */
-#define DECL_ASM(ret, name, ...)		\
-	__visible ret name(__VA_ARGS__);	\
-	extern char name##_end[] __visible;	\
-	extern char name##_reloc[] __visible
-
-DECL_ASM(void, xen_irq_enable_direct, void);
-DECL_ASM(void, xen_irq_disable_direct, void);
-DECL_ASM(unsigned long, xen_save_fl_direct, void);
-DECL_ASM(void, xen_restore_fl_direct, unsigned long);
-
 /* These are not functions, and cannot be called normally */
 __visible void xen_iret(void);
 __visible void xen_sysret32(void);