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

From: Peter Zijlstra
Date: Thu Aug 10 2017 - 12:25:12 EST


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@xxxxxxxx>
> ---
> 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(-)

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);