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

[Peter Zijlstra <peterz@xxxxxxxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel