[PATCH v4 0/8] x86: Meltdown band-aid overhead reduction

[PATCH v4 0/8] x86: Meltdown band-aid overhead reduction

[Jan Beulich]

1: NOP out most XPTI entry/exit code when it's not in use
2: disable XPTI when RDCL_NO
3: x86: log XPTI enabled status
4: use %r12 to write zero into xen_cr3
5: reduce .text.entry
6: enable interrupts earlier with XPTI disabled
7: also NOP out xen_cr3 restores of XPTI
8: avoid double CR3 reload when switching to guest user mode

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Main change is the split of patch 1.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 1/8] x86: NOP out XPTI entry/exit code when it's not in use

[Jan Beulich]

Introduce a synthetic feature flag to use alternative instruction
patching to NOP out all code on entry/exit paths. Having NOPs here is
generally better than using conditional branches.

Also change the limit on the number of bytes we can patch in one go to
that resulting from the encoding in struct alt_instr - there's no point
reducing it below that limit, and without a check being in place that
the limit isn't actually exceeded, such an artificial boundary is a
latent risk.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Re-base. Split off controversial parts.
v3: Also patch NMI/#MC paths. Re-base.
v2: Introduce and use ALTERNATIVE_NOP. Re-base.

--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -26,7 +26,7 @@
 #include <asm/nmi.h>
 #include <xen/livepatch.h>
 
-#define MAX_PATCH_LEN (255-1)
+#define MAX_PATCH_LEN 255
 
 extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3703,7 +3703,7 @@ long do_mmu_update(
                      * to the page lock we hold, its pinned status, and uses on
                      * this (v)CPU.
                      */
-                    if ( !rc && this_cpu(root_pgt) &&
+                    if ( !rc && !cpu_has_no_xpti &&
                          ((page->u.inuse.type_info & PGT_count_mask) >
                           (1 + !!(page->u.inuse.type_info & PGT_pinned) +
                            (pagetable_get_pfn(curr->arch.guest_table) == mfn) +
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -169,6 +169,9 @@ static int __init parse_smap_param(const
 }
 custom_param("smap", parse_smap_param);
 
+static int8_t __initdata opt_xpti = -1;
+boolean_param("xpti", opt_xpti);
+
 bool __read_mostly acpi_disabled;
 bool __initdata acpi_force;
 static char __initdata acpi_param[10] = "";
@@ -1543,6 +1546,13 @@ void __init noreturn __start_xen(unsigne
 
     cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS;
 
+    if ( opt_xpti < 0 )
+        opt_xpti = boot_cpu_data.x86_vendor != X86_VENDOR_AMD;
+    if ( opt_xpti )
+        setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
+    else
+        setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
+
     if ( cpu_has_fsgsbase )
         set_in_cr4(X86_CR4_FSGSBASE);
 
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -752,8 +752,6 @@ static int clone_mapping(const void *ptr
     return 0;
 }
 
-static __read_mostly int8_t opt_xpti = -1;
-boolean_param("xpti", opt_xpti);
 DEFINE_PER_CPU(root_pgentry_t *, root_pgt);
 
 static root_pgentry_t common_pgt;
@@ -766,7 +764,7 @@ static int setup_cpu_root_pgt(unsigned i
     unsigned int off;
     int rc;
 
-    if ( !opt_xpti )
+    if ( cpu_has_no_xpti )
         return 0;
 
     rpt = alloc_xen_pagetable();
@@ -1047,9 +1045,6 @@ void __init smp_prepare_cpus(unsigned in
 
     stack_base[0] = stack_start;
 
-    if ( opt_xpti < 0 )
-        opt_xpti = boot_cpu_data.x86_vendor != X86_VENDOR_AMD;
-
     rc = setup_cpu_root_pgt(0);
     if ( rc )
         panic("Error %d setting up PV root page table\n", rc);
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -209,6 +209,7 @@ ENTRY(cstar_enter)
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
         GET_STACK_END(bx)
+.Lcstar_cr3_start:
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx
         neg   %rcx
         jz    .Lcstar_cr3_okay
@@ -217,6 +218,7 @@ ENTRY(cstar_enter)
         mov   %rcx, %cr3
         movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lcstar_cr3_okay:
+        ALTERNATIVE_NOP .Lcstar_cr3_start, .Lcstar_cr3_okay, X86_FEATURE_NO_XPTI
         sti
 
         movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -45,6 +45,7 @@ restore_all_guest:
         mov VCPUMSR_spec_ctrl_raw(%rdx), %r15d
 
         /* Copy guest mappings and switch to per-CPU root page table. */
+.Lrag_cr3_start:
         mov   VCPU_cr3(%rbx), %r9
         GET_STACK_END(dx)
         mov   STACK_CPUINFO_FIELD(pv_cr3)(%rdx), %rdi
@@ -52,7 +53,6 @@ restore_all_guest:
         movabs $DIRECTMAP_VIRT_START, %rcx
         mov   %rdi, %rax
         and   %rsi, %rdi
-        jz    .Lrag_keep_cr3
         and   %r9, %rsi
         add   %rcx, %rdi
         add   %rcx, %rsi
@@ -74,7 +74,8 @@ restore_all_guest:
         mov   %rdi, %cr4
         mov   %rax, %cr3
         mov   %rsi, %cr4
-.Lrag_keep_cr3:
+.Lrag_cr3_end:
+        ALTERNATIVE_NOP .Lrag_cr3_start, .Lrag_cr3_end, X86_FEATURE_NO_XPTI
 
         /* Restore stashed SPEC_CTRL value. */
         mov   %r15d, %eax
@@ -121,6 +122,7 @@ restore_all_xen:
          * case we return to late PV exit code (from an NMI or #MC).
          */
         GET_STACK_END(bx)
+.Lrax_cr3_start:
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rdx
         mov   STACK_CPUINFO_FIELD(pv_cr3)(%rbx), %rax
         test  %rdx, %rdx
@@ -136,6 +138,8 @@ UNLIKELY_START(g, exit_cr3)
         mov   %rax, %cr3
         mov   %rsi, %cr4
 UNLIKELY_END(exit_cr3)
+.Lrax_cr3_end:
+        ALTERNATIVE_NOP .Lrax_cr3_start, .Lrax_cr3_end, X86_FEATURE_NO_XPTI
 
         /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
         SPEC_CTRL_EXIT_TO_XEN_IST /* Req: %rbx=end, Clob: acd */
@@ -174,6 +178,7 @@ ENTRY(lstar_enter)
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
         GET_STACK_END(bx)
+.Llstar_cr3_start:
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx
         neg   %rcx
         jz    .Llstar_cr3_okay
@@ -182,6 +187,7 @@ ENTRY(lstar_enter)
         mov   %rcx, %cr3
         movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Llstar_cr3_okay:
+        ALTERNATIVE_NOP .Llstar_cr3_start, .Llstar_cr3_okay, X86_FEATURE_NO_XPTI
         sti
 
         movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
@@ -281,6 +287,7 @@ GLOBAL(sysenter_eflags_saved)
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
         GET_STACK_END(bx)
+.Lsyse_cr3_start:
         /* PUSHF above has saved EFLAGS.IF clear (the caller had it set). */
         orl   $X86_EFLAGS_IF, UREGS_eflags(%rsp)
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx
@@ -291,6 +298,7 @@ GLOBAL(sysenter_eflags_saved)
         mov   %rcx, %cr3
         movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lsyse_cr3_okay:
+        ALTERNATIVE_NOP .Lsyse_cr3_start, .Lsyse_cr3_okay, X86_FEATURE_NO_XPTI
         sti
 
         movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
@@ -333,6 +341,7 @@ ENTRY(int80_direct_trap)
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
         GET_STACK_END(bx)
+.Lint80_cr3_start:
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx
         neg   %rcx
         jz    .Lint80_cr3_okay
@@ -341,6 +350,7 @@ ENTRY(int80_direct_trap)
         mov   %rcx, %cr3
         movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lint80_cr3_okay:
+        ALTERNATIVE_NOP .Lint80_cr3_start, .Lint80_cr3_okay, X86_FEATURE_NO_XPTI
         sti
 
         cmpb  $0,untrusted_msi(%rip)
@@ -541,6 +551,7 @@ ENTRY(common_interrupt)
         SPEC_CTRL_ENTRY_FROM_INTR /* Req: %rsp=regs, %r14=end, Clob: acd */
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
+.Lintr_cr3_start:
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%r14), %rcx
         mov   %rcx, %r15
         neg   %rcx
@@ -555,6 +566,7 @@ ENTRY(common_interrupt)
         testb $3, UREGS_cs(%rsp)
         cmovnz %rcx, %r15
 .Lintr_cr3_okay:
+        ALTERNATIVE_NOP .Lintr_cr3_start, .Lintr_cr3_okay, X86_FEATURE_NO_XPTI
 
         CR4_PV32_RESTORE
         movq %rsp,%rdi
@@ -583,6 +595,7 @@ GLOBAL(handle_exception)
         SPEC_CTRL_ENTRY_FROM_INTR /* Req: %rsp=regs, %r14=end, Clob: acd */
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
+.Lxcpt_cr3_start:
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%r14), %rcx
         mov   %rcx, %r15
         neg   %rcx
@@ -597,6 +610,7 @@ GLOBAL(handle_exception)
         testb $3, UREGS_cs(%rsp)
         cmovnz %rcx, %r15
 .Lxcpt_cr3_okay:
+        ALTERNATIVE_NOP .Lxcpt_cr3_start, .Lxcpt_cr3_okay, X86_FEATURE_NO_XPTI
 
 handle_exception_saved:
         GET_CURRENT(bx)
@@ -800,6 +814,7 @@ handle_ist_exception:
         SPEC_CTRL_ENTRY_FROM_INTR_IST /* Req: %rsp=regs, %r14=end, Clob: acd */
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
+.List_cr3_start:
         mov   STACK_CPUINFO_FIELD(xen_cr3)(%r14), %rcx
         mov   %rcx, %r15
         neg   %rcx
@@ -811,6 +826,7 @@ handle_ist_exception:
         mov   %rcx, %cr3
         movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
 .List_cr3_okay:
+        ALTERNATIVE_NOP .List_cr3_start, .List_cr3_okay, X86_FEATURE_NO_XPTI
 
         CR4_PV32_RESTORE
         testb $3,UREGS_cs(%rsp)
--- a/xen/include/asm-x86/alternative-asm.h
+++ b/xen/include/asm-x86/alternative-asm.h
@@ -101,6 +101,13 @@
 #undef decl_orig
 #undef as_true
 
+/* Macro to replace an entire range by suitable NOPs. */
+.macro ALTERNATIVE_NOP start, end, feature
+    .pushsection .altinstructions, "a", @progbits
+    altinstruction_entry \start, \start, \feature, "\end - \start", 0, 0
+    .popsection
+.endm
+
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_X86_ALTERNATIVE_ASM_H_ */
 
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -112,6 +112,7 @@
 #define cpu_has_cpuid_faulting  boot_cpu_has(X86_FEATURE_CPUID_FAULTING)
 #define cpu_has_aperfmperf      boot_cpu_has(X86_FEATURE_APERFMPERF)
 #define cpu_has_lfence_dispatch boot_cpu_has(X86_FEATURE_LFENCE_DISPATCH)
+#define cpu_has_no_xpti         boot_cpu_has(X86_FEATURE_NO_XPTI)
 
 enum _cache_type {
     CACHE_TYPE_NULL = 0,
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -29,4 +29,5 @@ XEN_CPUFEATURE(XEN_IBPB,        (FSCAPIN
 XEN_CPUFEATURE(XEN_IBRS_SET,    (FSCAPINTS+0)*32+16) /* IBRSB && IRBS set in Xen */
 XEN_CPUFEATURE(XEN_IBRS_CLEAR,  (FSCAPINTS+0)*32+17) /* IBRSB && IBRS clear in Xen */
 XEN_CPUFEATURE(RSB_NATIVE,      (FSCAPINTS+0)*32+18) /* RSB overwrite needed for native */
-XEN_CPUFEATURE(RSB_VMEXIT,      (FSCAPINTS+0)*32+20) /* RSB overwrite needed for vmexit */
+XEN_CPUFEATURE(RSB_VMEXIT,      (FSCAPINTS+0)*32+19) /* RSB overwrite needed for vmexit */
+XEN_CPUFEATURE(NO_XPTI,         (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 2/8] x86: disable XPTI when RDCL_NO

[Jan Beulich]

Use the respective ARCH_CAPABILITIES MSR bit, but don't expose the MSR
to guests yet.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v3: Re-base.
v2: New.

--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -204,6 +204,7 @@ int libxl_cpuid_parse_config(libxl_cpuid
         {"avx512-4fmaps",0x00000007,  0, CPUID_REG_EDX,  3,  1},
         {"ibrsb",        0x00000007,  0, CPUID_REG_EDX, 26,  1},
         {"stibp",        0x00000007,  0, CPUID_REG_EDX, 27,  1},
+        {"arch-caps",    0x00000007,  0, CPUID_REG_EDX, 29,  1},
 
         {"lahfsahf",     0x80000001, NA, CPUID_REG_ECX,  0,  1},
         {"cmplegacy",    0x80000001, NA, CPUID_REG_ECX,  1,  1},
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -143,6 +143,7 @@ static const char *str_7d0[32] =
     [ 2] = "avx512_4vnniw", [ 3] = "avx512_4fmaps",
 
     [26] = "ibrsb",         [27] = "stibp",
+    /* 28 */                [29] = "arch_caps",
 };
 
 static struct {
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1547,7 +1547,16 @@ void __init noreturn __start_xen(unsigne
     cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS;
 
     if ( opt_xpti < 0 )
-        opt_xpti = boot_cpu_data.x86_vendor != X86_VENDOR_AMD;
+    {
+        uint64_t caps = 0;
+
+        if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+            caps = ARCH_CAPABILITIES_RDCL_NO;
+        else if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
+            rdmsrl(MSR_ARCH_CAPABILITIES, caps);
+
+        opt_xpti = !(caps & ARCH_CAPABILITIES_RDCL_NO);
+    }
     if ( opt_xpti )
         setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
     else
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -40,6 +40,8 @@
 #define PRED_CMD_IBPB			(_AC(1, ULL) << 0)
 
 #define MSR_ARCH_CAPABILITIES		0x0000010a
+#define ARCH_CAPABILITIES_RDCL_NO	(_AC(1, ULL) << 0)
+#define ARCH_CAPABILITIES_IBRS_ALL	(_AC(1, ULL) << 1)
 
 /* Intel MSRs. Some also available on other CPUs */
 #define MSR_IA32_PERFCTR0		0x000000c1
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -244,6 +244,7 @@ XEN_CPUFEATURE(AVX512_4VNNIW, 9*32+ 2) /
 XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A  AVX512 Multiply Accumulation Single Precision */
 XEN_CPUFEATURE(IBRSB,         9*32+26) /*A  IBRS and IBPB support (used by Intel) */
 XEN_CPUFEATURE(STIBP,         9*32+27) /*A! STIBP */
+XEN_CPUFEATURE(ARCH_CAPS,     9*32+29) /*   IA32_ARCH_CAPABILITIES MSR */
 
 #endif /* XEN_CPUFEATURE */
 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 3/8] x86: log XPTI enabled status

[Jan Beulich]

At the same time also report the state of the two defined
ARCH_CAPABILITIES MSR bits. To avoid further complicating the
conditional around that printk(), drop it (it's a debug level one only
anyway).

Issue the main message without any XENLOG_*, and also drop XENLOG_INFO
from the respective BTI message, to make sure they're visible at default
log level also in release builds.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v4: Drop XENLOG_INFO (also from respective BTI message).
v2: Re-base over split off earlier patch. Drop MSR_ from
    MSR_ARCH_CAPABILITIES_*. Drop conditional around debug printk().

--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -21,7 +21,7 @@
 #include <xen/lib.h>
 
 #include <asm/microcode.h>
-#include <asm/msr-index.h>
+#include <asm/msr.h>
 #include <asm/processor.h>
 #include <asm/spec_ctrl.h>
 #include <asm/spec_ctrl_asm.h>
@@ -100,30 +100,31 @@ custom_param("bti", parse_bti);
 static void __init print_details(enum ind_thunk thunk)
 {
     unsigned int _7d0 = 0, e8b = 0, tmp;
+    uint64_t caps = 0;
 
     /* Collect diagnostics about available mitigations. */
     if ( boot_cpu_data.cpuid_level >= 7 )
         cpuid_count(7, 0, &tmp, &tmp, &tmp, &_7d0);
     if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
         cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
+    if ( _7d0 & cpufeat_mask(X86_FEATURE_ARCH_CAPS) )
+        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
 
     printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
 
     /* Hardware features which pertain to speculative mitigations. */
-    if ( (_7d0 & (cpufeat_mask(X86_FEATURE_IBRSB) |
-                  cpufeat_mask(X86_FEATURE_STIBP))) ||
-         (e8b & cpufeat_mask(X86_FEATURE_IBPB)) )
-        printk(XENLOG_DEBUG "  Hardware features:%s%s%s\n",
-               (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
-               (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP"     : "",
-               (e8b  & cpufeat_mask(X86_FEATURE_IBPB))  ? " IBPB"      : "");
+    printk(XENLOG_DEBUG "  Hardware features:%s%s%s%s%s\n",
+           (_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
+           (_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP"     : "",
+           (e8b  & cpufeat_mask(X86_FEATURE_IBPB))  ? " IBPB"      : "",
+           (caps & ARCH_CAPABILITIES_IBRS_ALL)      ? " IBRS_ALL"  : "",
+           (caps & ARCH_CAPABILITIES_RDCL_NO)       ? " RDCL_NO"   : "");
 
     /* Compiled-in support which pertains to BTI mitigations. */
     if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
         printk(XENLOG_DEBUG "  Compiled-in support: INDIRECT_THUNK\n");
 
-    printk(XENLOG_INFO
-           "BTI mitigations: Thunk %s, Others:%s%s%s%s\n",
+    printk("BTI mitigations: Thunk %s, Others:%s%s%s%s\n",
            thunk == THUNK_NONE      ? "N/A" :
            thunk == THUNK_RETPOLINE ? "RETPOLINE" :
            thunk == THUNK_LFENCE    ? "LFENCE" :
@@ -133,6 +134,9 @@ static void __init print_details(enum in
            opt_ibpb                                  ? " IBPB"       : "",
            boot_cpu_has(X86_FEATURE_RSB_NATIVE)      ? " RSB_NATIVE" : "",
            boot_cpu_has(X86_FEATURE_RSB_VMEXIT)      ? " RSB_VMEXIT" : "");
+
+    printk("XPTI: %s\n",
+           boot_cpu_has(X86_FEATURE_NO_XPTI) ? "disabled" : "enabled");
 }
 
 /* Calculate whether Retpoline is known-safe on this CPU. */




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 4/8] x86/XPTI: use %r12 to write zero into xen_cr3

[Jan Beulich]

Now that we zero all registers early on all entry paths, use that to
avoid a couple of immediates here.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v4: Add comments about the %r12 being zero
---
We may want to consider eliminating a few more $0 this way. But
especially for byte ones I'm not sure it's worth it, due to the REX
prefix the use of %r12 would incur.

--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -216,7 +216,8 @@ ENTRY(cstar_enter)
         mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
         neg   %rcx
         mov   %rcx, %cr3
-        movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
+        /* %r12 is still zero at this point. */
+        mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lcstar_cr3_okay:
         ALTERNATIVE_NOP .Lcstar_cr3_start, .Lcstar_cr3_okay, X86_FEATURE_NO_XPTI
         sti
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -185,7 +185,8 @@ ENTRY(lstar_enter)
         mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
         neg   %rcx
         mov   %rcx, %cr3
-        movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
+        /* %r12 is still zero at this point. */
+        mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Llstar_cr3_okay:
         ALTERNATIVE_NOP .Llstar_cr3_start, .Llstar_cr3_okay, X86_FEATURE_NO_XPTI
         sti
@@ -296,7 +297,8 @@ GLOBAL(sysenter_eflags_saved)
         mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
         neg   %rcx
         mov   %rcx, %cr3
-        movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
+        /* %r12 is still zero at this point. */
+        mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lsyse_cr3_okay:
         ALTERNATIVE_NOP .Lsyse_cr3_start, .Lsyse_cr3_okay, X86_FEATURE_NO_XPTI
         sti
@@ -348,7 +350,8 @@ ENTRY(int80_direct_trap)
         mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
         neg   %rcx
         mov   %rcx, %cr3
-        movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
+        /* %r12 is still zero at this point. */
+        mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lint80_cr3_okay:
         ALTERNATIVE_NOP .Lint80_cr3_start, .Lint80_cr3_okay, X86_FEATURE_NO_XPTI
         sti
@@ -561,10 +564,10 @@ ENTRY(common_interrupt)
         neg   %rcx
 .Lintr_cr3_load:
         mov   %rcx, %cr3
-        xor   %ecx, %ecx
-        mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+        /* %r12 is still zero at this point. */
+        mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
         testb $3, UREGS_cs(%rsp)
-        cmovnz %rcx, %r15
+        cmovnz %r12, %r15
 .Lintr_cr3_okay:
         ALTERNATIVE_NOP .Lintr_cr3_start, .Lintr_cr3_okay, X86_FEATURE_NO_XPTI
 
@@ -605,10 +608,10 @@ GLOBAL(handle_exception)
         neg   %rcx
 .Lxcpt_cr3_load:
         mov   %rcx, %cr3
-        xor   %ecx, %ecx
-        mov   %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+        /* %r12 is still zero at this point. */
+        mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
         testb $3, UREGS_cs(%rsp)
-        cmovnz %rcx, %r15
+        cmovnz %r12, %r15
 .Lxcpt_cr3_okay:
         ALTERNATIVE_NOP .Lxcpt_cr3_start, .Lxcpt_cr3_okay, X86_FEATURE_NO_XPTI
 
@@ -824,7 +827,8 @@ handle_ist_exception:
         neg   %rcx
 .List_cr3_load:
         mov   %rcx, %cr3
-        movq  $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+        /* %r12 is still zero at this point. */
+        mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
 .List_cr3_okay:
         ALTERNATIVE_NOP .List_cr3_start, .List_cr3_okay, X86_FEATURE_NO_XPTI
 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 5/8] x86/XPTI: reduce .text.entry

[Jan Beulich]

This exposes less code pieces and at the same time reduces the range
covered from slightly above 3 pages to a little below 2 of them.

The code being moved is unchanged, except for the removal of trailing
blanks, insertion of blanks between operands, and a pointless q suffix
from "retq".

A few more small pieces could be moved, but it seems better to me to
leave them where they are to not make it overly hard to follow code
paths.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Re-base.

--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -13,8 +13,6 @@
 #include <public/xen.h>
 #include <irq_vectors.h>
 
-        .section .text.entry, "ax", @progbits
-
 ENTRY(entry_int82)
         ASM_CLAC
         pushq $0
@@ -192,6 +190,8 @@ ENTRY(compat_post_handle_exception)
         movb  $0,TRAPBOUNCE_flags(%rdx)
         jmp   compat_test_all_events
 
+        .section .text.entry, "ax", @progbits
+
 /* See lstar_enter for entry register state. */
 ENTRY(cstar_enter)
         /* sti could live here when we don't switch page tables below. */
@@ -249,6 +249,8 @@ UNLIKELY_END(compat_syscall_gpf)
         movb  %cl,TRAPBOUNCE_flags(%rdx)
         jmp   .Lcompat_bounce_exception
 
+        .text
+
 ENTRY(compat_sysenter)
         CR4_PV32_RESTORE
         movq  VCPU_trap_ctxt(%rbx),%rcx
@@ -268,9 +270,6 @@ ENTRY(compat_int80_direct_trap)
         call  compat_create_bounce_frame
         jmp   compat_test_all_events
 
-        /* compat_create_bounce_frame & helpers don't need to be in .text.entry */
-        .text
-
 /* CREATE A BASIC EXCEPTION FRAME ON GUEST OS (RING-1) STACK:            */
 /*   {[ERRCODE,] EIP, CS, EFLAGS, [ESP, SS]}                             */
 /* %rdx: trap_bounce, %rbx: struct vcpu                                  */
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -14,8 +14,6 @@
 #include <public/xen.h>
 #include <irq_vectors.h>
 
-        .section .text.entry, "ax", @progbits
-
 /* %rbx: struct vcpu */
 ENTRY(switch_to_kernel)
         leaq  VCPU_trap_bounce(%rbx),%rdx
@@ -34,8 +32,91 @@ ENTRY(switch_to_kernel)
         movb  %cl,TRAPBOUNCE_flags(%rdx)
         call  create_bounce_frame
         andl  $~X86_EFLAGS_DF,UREGS_eflags(%rsp)
+/* %rbx: struct vcpu */
+test_all_events:
+        ASSERT_NOT_IN_ATOMIC
+        cli                             # tests must not race interrupts
+/*test_softirqs:*/
+        movl  VCPU_processor(%rbx), %eax
+        shll  $IRQSTAT_shift, %eax
+        leaq  irq_stat+IRQSTAT_softirq_pending(%rip), %rcx
+        cmpl  $0, (%rcx, %rax, 1)
+        jne   process_softirqs
+        cmpb  $0, VCPU_mce_pending(%rbx)
+        jne   process_mce
+.Ltest_guest_nmi:
+        cmpb  $0, VCPU_nmi_pending(%rbx)
+        jne   process_nmi
+test_guest_events:
+        movq  VCPU_vcpu_info(%rbx), %rax
+        movzwl VCPUINFO_upcall_pending(%rax), %eax
+        decl  %eax
+        cmpl  $0xfe, %eax
+        ja    restore_all_guest
+/*process_guest_events:*/
+        sti
+        leaq  VCPU_trap_bounce(%rbx), %rdx
+        movq  VCPU_event_addr(%rbx), %rax
+        movq  %rax, TRAPBOUNCE_eip(%rdx)
+        movb  $TBF_INTERRUPT, TRAPBOUNCE_flags(%rdx)
+        call  create_bounce_frame
         jmp   test_all_events
 
+        ALIGN
+/* %rbx: struct vcpu */
+process_softirqs:
+        sti
+        call do_softirq
+        jmp  test_all_events
+
+        ALIGN
+/* %rbx: struct vcpu */
+process_mce:
+        testb $1 << VCPU_TRAP_MCE, VCPU_async_exception_mask(%rbx)
+        jnz  .Ltest_guest_nmi
+        sti
+        movb $0, VCPU_mce_pending(%rbx)
+        call set_guest_machinecheck_trapbounce
+        test %eax, %eax
+        jz   test_all_events
+        movzbl VCPU_async_exception_mask(%rbx), %edx # save mask for the
+        movb %dl, VCPU_mce_old_mask(%rbx)            # iret hypercall
+        orl  $1 << VCPU_TRAP_MCE, %edx
+        movb %dl, VCPU_async_exception_mask(%rbx)
+        jmp  process_trap
+
+        ALIGN
+/* %rbx: struct vcpu */
+process_nmi:
+        testb $1 << VCPU_TRAP_NMI, VCPU_async_exception_mask(%rbx)
+        jnz  test_guest_events
+        sti
+        movb $0, VCPU_nmi_pending(%rbx)
+        call set_guest_nmi_trapbounce
+        test %eax, %eax
+        jz   test_all_events
+        movzbl VCPU_async_exception_mask(%rbx), %edx # save mask for the
+        movb %dl, VCPU_nmi_old_mask(%rbx)            # iret hypercall
+        orl  $1 << VCPU_TRAP_NMI, %edx
+        movb %dl, VCPU_async_exception_mask(%rbx)
+        /* FALLTHROUGH */
+process_trap:
+        leaq VCPU_trap_bounce(%rbx), %rdx
+        call create_bounce_frame
+        jmp  test_all_events
+
+/* No special register assumptions. */
+ENTRY(ret_from_intr)
+        GET_CURRENT(bx)
+        testb $3, UREGS_cs(%rsp)
+        jz    restore_all_xen
+        movq  VCPU_domain(%rbx), %rax
+        cmpb  $0, DOMAIN_is_32bit_pv(%rax)
+        je    test_all_events
+        jmp   compat_test_all_events
+
+        .section .text.entry, "ax", @progbits
+
 /* %rbx: struct vcpu, interrupts disabled */
 restore_all_guest:
         ASSERT_INTERRUPTS_DISABLED
@@ -197,80 +278,8 @@ ENTRY(lstar_enter)
 
         mov   %rsp, %rdi
         call  pv_hypercall
-
-/* %rbx: struct vcpu */
-test_all_events:
-        ASSERT_NOT_IN_ATOMIC
-        cli                             # tests must not race interrupts
-/*test_softirqs:*/  
-        movl  VCPU_processor(%rbx),%eax
-        shll  $IRQSTAT_shift,%eax
-        leaq  irq_stat+IRQSTAT_softirq_pending(%rip),%rcx
-        cmpl  $0,(%rcx,%rax,1)
-        jne   process_softirqs
-        cmpb  $0, VCPU_mce_pending(%rbx)
-        jne   process_mce
-.Ltest_guest_nmi:
-        cmpb  $0, VCPU_nmi_pending(%rbx)
-        jne   process_nmi
-test_guest_events:
-        movq  VCPU_vcpu_info(%rbx),%rax
-        movzwl VCPUINFO_upcall_pending(%rax),%eax
-        decl  %eax
-        cmpl  $0xfe,%eax
-        ja    restore_all_guest
-/*process_guest_events:*/
-        sti
-        leaq  VCPU_trap_bounce(%rbx),%rdx
-        movq  VCPU_event_addr(%rbx),%rax
-        movq  %rax,TRAPBOUNCE_eip(%rdx)
-        movb  $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
-        call  create_bounce_frame
         jmp   test_all_events
 
-        ALIGN
-/* %rbx: struct vcpu */
-process_softirqs:
-        sti       
-        call do_softirq
-        jmp  test_all_events
-
-        ALIGN
-/* %rbx: struct vcpu */
-process_mce:
-        testb $1 << VCPU_TRAP_MCE,VCPU_async_exception_mask(%rbx)
-        jnz  .Ltest_guest_nmi
-        sti
-        movb $0,VCPU_mce_pending(%rbx)
-        call set_guest_machinecheck_trapbounce
-        test %eax,%eax
-        jz   test_all_events
-        movzbl VCPU_async_exception_mask(%rbx),%edx # save mask for the
-        movb %dl,VCPU_mce_old_mask(%rbx)            # iret hypercall
-        orl  $1 << VCPU_TRAP_MCE,%edx
-        movb %dl,VCPU_async_exception_mask(%rbx)
-        jmp  process_trap
-
-        ALIGN
-/* %rbx: struct vcpu */
-process_nmi:
-        testb $1 << VCPU_TRAP_NMI,VCPU_async_exception_mask(%rbx)
-        jnz  test_guest_events
-        sti
-        movb $0,VCPU_nmi_pending(%rbx)
-        call set_guest_nmi_trapbounce
-        test %eax,%eax
-        jz   test_all_events
-        movzbl VCPU_async_exception_mask(%rbx),%edx # save mask for the
-        movb %dl,VCPU_nmi_old_mask(%rbx)            # iret hypercall
-        orl  $1 << VCPU_TRAP_NMI,%edx
-        movb %dl,VCPU_async_exception_mask(%rbx)
-        /* FALLTHROUGH */
-process_trap:
-        leaq VCPU_trap_bounce(%rbx),%rdx
-        call create_bounce_frame
-        jmp  test_all_events
-
 ENTRY(sysenter_entry)
         /* sti could live here when we don't switch page tables below. */
         pushq $FLAT_USER_SS
@@ -577,16 +586,6 @@ ENTRY(common_interrupt)
         mov   %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
         jmp ret_from_intr
 
-/* No special register assumptions. */
-ENTRY(ret_from_intr)
-        GET_CURRENT(bx)
-        testb $3,UREGS_cs(%rsp)
-        jz    restore_all_xen
-        movq  VCPU_domain(%rbx),%rax
-        cmpb  $0, DOMAIN_is_32bit_pv(%rax)
-        je    test_all_events
-        jmp   compat_test_all_events
-
 ENTRY(page_fault)
         movl  $TRAP_page_fault,4(%rsp)
 /* No special register assumptions. */



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 6/8] x86: enable interrupts earlier with XPTI disabled

[Jan Beulich]

The STI instances were moved (or added in the INT80 case) to meet TLB
flush requirements. When XPTI is disabled, they can be put back where
they were (or omitted in the INT80 case).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Split off from earlier patch.
---
TBD: It is questionable whether having two back-to-back alternatives
     keyed to the same feature is a good idea. The original patch had
     them as a single instance each, but the variant here requires fewer
     (explicit) labels and hence results in overall more readable code.

--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -194,7 +194,7 @@ ENTRY(compat_post_handle_exception)
 
 /* See lstar_enter for entry register state. */
 ENTRY(cstar_enter)
-        /* sti could live here when we don't switch page tables below. */
+        ALTERNATIVE nop, sti, X86_FEATURE_NO_XPTI
         CR4_PV32_RESTORE
         movq  8(%rsp),%rax /* Restore %rax. */
         movq  $FLAT_KERNEL_SS,8(%rsp)
@@ -220,7 +220,7 @@ ENTRY(cstar_enter)
         mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lcstar_cr3_okay:
         ALTERNATIVE_NOP .Lcstar_cr3_start, .Lcstar_cr3_okay, X86_FEATURE_NO_XPTI
-        sti
+        ALTERNATIVE sti, "", X86_FEATURE_NO_XPTI
 
         movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
         movq  VCPU_domain(%rbx),%rcx
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -245,7 +245,7 @@ UNLIKELY_END(exit_cr3)
  * %ss must be saved into the space left by the trampoline.
  */
 ENTRY(lstar_enter)
-        /* sti could live here when we don't switch page tables below. */
+        ALTERNATIVE nop, sti, X86_FEATURE_NO_XPTI
         movq  8(%rsp),%rax /* Restore %rax. */
         movq  $FLAT_KERNEL_SS,8(%rsp)
         pushq %r11
@@ -270,7 +270,7 @@ ENTRY(lstar_enter)
         mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Llstar_cr3_okay:
         ALTERNATIVE_NOP .Llstar_cr3_start, .Llstar_cr3_okay, X86_FEATURE_NO_XPTI
-        sti
+        ALTERNATIVE sti, "", X86_FEATURE_NO_XPTI
 
         movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
         testb $TF_kernel_mode,VCPU_thread_flags(%rbx)
@@ -281,7 +281,7 @@ ENTRY(lstar_enter)
         jmp   test_all_events
 
 ENTRY(sysenter_entry)
-        /* sti could live here when we don't switch page tables below. */
+        ALTERNATIVE nop, sti, X86_FEATURE_NO_XPTI
         pushq $FLAT_USER_SS
         pushq $0
         pushfq
@@ -310,7 +310,7 @@ GLOBAL(sysenter_eflags_saved)
         mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lsyse_cr3_okay:
         ALTERNATIVE_NOP .Lsyse_cr3_start, .Lsyse_cr3_okay, X86_FEATURE_NO_XPTI
-        sti
+        ALTERNATIVE sti, "", X86_FEATURE_NO_XPTI
 
         movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
         cmpb  $0,VCPU_sysenter_disables_events(%rbx)
@@ -363,7 +363,7 @@ ENTRY(int80_direct_trap)
         mov   %r12, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
 .Lint80_cr3_okay:
         ALTERNATIVE_NOP .Lint80_cr3_start, .Lint80_cr3_okay, X86_FEATURE_NO_XPTI
-        sti
+        ALTERNATIVE sti, "", X86_FEATURE_NO_XPTI
 
         cmpb  $0,untrusted_msi(%rip)
 UNLIKELY_START(ne, msi_check)




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 7/8] x86: also NOP out xen_cr3 restores of XPTI

[Jan Beulich]

... despite quite likely the gain being rather limited.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Split off from earlier patch.

--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -583,7 +583,8 @@ ENTRY(common_interrupt)
         CR4_PV32_RESTORE
         movq %rsp,%rdi
         callq do_IRQ
-        mov   %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+        ALTERNATIVE __stringify(mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)), \
+                    "", X86_FEATURE_NO_XPTI
         jmp ret_from_intr
 
 ENTRY(page_fault)
@@ -665,7 +666,8 @@ handle_exception_saved:
         PERFC_INCR(exceptions, %rax, %rbx)
         mov   (%rdx, %rax, 8), %rdx
         INDIRECT_CALL %rdx
-        mov   %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+        ALTERNATIVE __stringify(mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)), \
+                    "", X86_FEATURE_NO_XPTI
         testb $3,UREGS_cs(%rsp)
         jz    restore_all_xen
         leaq  VCPU_trap_bounce(%rbx),%rdx
@@ -698,7 +700,8 @@ exception_with_ints_disabled:
         rep;  movsq                     # make room for ec/ev
 1:      movq  UREGS_error_code(%rsp),%rax # ec/ev
         movq  %rax,UREGS_kernel_sizeof(%rsp)
-        mov   %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+        ALTERNATIVE __stringify(mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)), \
+                    "", X86_FEATURE_NO_XPTI
         jmp   restore_all_xen           # return to fixup code
 
 /* No special register assumptions. */
@@ -849,7 +852,8 @@ handle_ist_exception:
         leaq  exception_table(%rip),%rdx
         mov   (%rdx, %rax, 8), %rdx
         INDIRECT_CALL %rdx
-        mov   %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+        ALTERNATIVE __stringify(mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)), \
+                    "", X86_FEATURE_NO_XPTI
         cmpb  $TRAP_nmi,UREGS_entry_vector(%rsp)
         jne   ret_from_intr
 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v4 8/8] x86: avoid double CR3 reload when switching to guest user mode

[Jan Beulich]

When XPTI is active, the CR3 load in restore_all_guest is sufficient
when switching to user mode, improving in particular system call and
page fault exit paths for the guest.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
v2: Add ASSERT(!in_irq()).

--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -219,10 +219,22 @@ int pv_domain_initialise(struct domain *
     return rc;
 }
 
-static void _toggle_guest_pt(struct vcpu *v)
+static void _toggle_guest_pt(struct vcpu *v, bool force_cr3)
 {
+    ASSERT(!in_irq());
+
     v->arch.flags ^= TF_kernel_mode;
     update_cr3(v);
+
+    /*
+     * There's no need to load CR3 here when it is going to be loaded on the
+     * way out to guest mode again anyway, and when the page tables we're
+     * currently on are the kernel ones (whereas when switching to kernel
+     * mode we need to be able to write a bounce frame onto the kernel stack).
+     */
+    if ( !force_cr3 && !(v->arch.flags & TF_kernel_mode) )
+        return;
+
     /* Don't flush user global mappings from the TLB. Don't tick TLB clock. */
     asm volatile ( "mov %0, %%cr3" : : "r" (v->arch.cr3) : "memory" );
 
@@ -252,13 +264,13 @@ void toggle_guest_mode(struct vcpu *v)
     }
     asm volatile ( "swapgs" );
 
-    _toggle_guest_pt(v);
+    _toggle_guest_pt(v, cpu_has_no_xpti);
 }
 
 void toggle_guest_pt(struct vcpu *v)
 {
     if ( !is_pv_32bit_vcpu(v) )
-        _toggle_guest_pt(v);
+        _toggle_guest_pt(v, true);
 }
 
 /*




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 2/8] x86: disable XPTI when RDCL_NO

[Jan Beulich]

>>> On 19.03.18 at 14:38, <JBeulich@suse.com> wrote:
> Use the respective ARCH_CAPABILITIES MSR bit, but don't expose the MSR
> to guests yet.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> v3: Re-base.
> v2: New.

And I realize I've once again forgot to Cc the two of you for the
smallish tools side changes.

Jan

> --- a/tools/libxl/libxl_cpuid.c
> +++ b/tools/libxl/libxl_cpuid.c
> @@ -204,6 +204,7 @@ int libxl_cpuid_parse_config(libxl_cpuid
>          {"avx512-4fmaps",0x00000007,  0, CPUID_REG_EDX,  3,  1},
>          {"ibrsb",        0x00000007,  0, CPUID_REG_EDX, 26,  1},
>          {"stibp",        0x00000007,  0, CPUID_REG_EDX, 27,  1},
> +        {"arch-caps",    0x00000007,  0, CPUID_REG_EDX, 29,  1},
>  
>          {"lahfsahf",     0x80000001, NA, CPUID_REG_ECX,  0,  1},
>          {"cmplegacy",    0x80000001, NA, CPUID_REG_ECX,  1,  1},
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -143,6 +143,7 @@ static const char *str_7d0[32] =
>      [ 2] = "avx512_4vnniw", [ 3] = "avx512_4fmaps",
>  
>      [26] = "ibrsb",         [27] = "stibp",
> +    /* 28 */                [29] = "arch_caps",
>  };
>  
>  static struct {
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1547,7 +1547,16 @@ void __init noreturn __start_xen(unsigne
>      cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS;
>  
>      if ( opt_xpti < 0 )
> -        opt_xpti = boot_cpu_data.x86_vendor != X86_VENDOR_AMD;
> +    {
> +        uint64_t caps = 0;
> +
> +        if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
> +            caps = ARCH_CAPABILITIES_RDCL_NO;
> +        else if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
> +            rdmsrl(MSR_ARCH_CAPABILITIES, caps);
> +
> +        opt_xpti = !(caps & ARCH_CAPABILITIES_RDCL_NO);
> +    }
>      if ( opt_xpti )
>          setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
>      else
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -40,6 +40,8 @@
>  #define PRED_CMD_IBPB			(_AC(1, ULL) << 0)
>  
>  #define MSR_ARCH_CAPABILITIES		0x0000010a
> +#define ARCH_CAPABILITIES_RDCL_NO	(_AC(1, ULL) << 0)
> +#define ARCH_CAPABILITIES_IBRS_ALL	(_AC(1, ULL) << 1)
>  
>  /* Intel MSRs. Some also available on other CPUs */
>  #define MSR_IA32_PERFCTR0		0x000000c1
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -244,6 +244,7 @@ XEN_CPUFEATURE(AVX512_4VNNIW, 9*32+ 2) /
>  XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A  AVX512 Multiply Accumulation 
> Single Precision */
>  XEN_CPUFEATURE(IBRSB,         9*32+26) /*A  IBRS and IBPB support (used by 
> Intel) */
>  XEN_CPUFEATURE(STIBP,         9*32+27) /*A! STIBP */
> +XEN_CPUFEATURE(ARCH_CAPS,     9*32+29) /*   IA32_ARCH_CAPABILITIES MSR */
>  
>  #endif /* XEN_CPUFEATURE */
>  
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org 
> https://lists.xenproject.org/mailman/listinfo/xen-devel 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 1/8] x86: NOP out XPTI entry/exit code when it's not in use

[Juergen Gross]

On 19/03/18 14:37, Jan Beulich wrote:
> Introduce a synthetic feature flag to use alternative instruction
> patching to NOP out all code on entry/exit paths. Having NOPs here is
> generally better than using conditional branches.
> 
> Also change the limit on the number of bytes we can patch in one go to
> that resulting from the encoding in struct alt_instr - there's no point
> reducing it below that limit, and without a check being in place that
> the limit isn't actually exceeded, such an artificial boundary is a
> latent risk.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 4/8] x86/XPTI: use %r12 to write zero into xen_cr3

[Juergen Gross]

On 19/03/18 14:39, Jan Beulich wrote:
> Now that we zero all registers early on all entry paths, use that to
> avoid a couple of immediates here.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 5/8] x86/XPTI: reduce .text.entry

[Juergen Gross]

On 19/03/18 14:40, Jan Beulich wrote:
> This exposes less code pieces and at the same time reduces the range
> covered from slightly above 3 pages to a little below 2 of them.
> 
> The code being moved is unchanged, except for the removal of trailing
> blanks, insertion of blanks between operands, and a pointless q suffix
> from "retq".
> 
> A few more small pieces could be moved, but it seems better to me to
> leave them where they are to not make it overly hard to follow code
> paths.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 6/8] x86: enable interrupts earlier with XPTI disabled

[Juergen Gross]

On 19/03/18 14:40, Jan Beulich wrote:
> The STI instances were moved (or added in the INT80 case) to meet TLB
> flush requirements. When XPTI is disabled, they can be put back where
> they were (or omitted in the INT80 case).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 7/8] x86: also NOP out xen_cr3 restores of XPTI

[Juergen Gross]

On 19/03/18 14:41, Jan Beulich wrote:
> ... despite quite likely the gain being rather limited.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 2/8] x86: disable XPTI when RDCL_NO

[Wei Liu]

On Mon, Mar 19, 2018 at 07:43:09AM -0600, Jan Beulich wrote:
> >>> On 19.03.18 at 14:38, <JBeulich@suse.com> wrote:
> > Use the respective ARCH_CAPABILITIES MSR bit, but don't expose the MSR
> > to guests yet.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > Tested-by: Juergen Gross <jgross@suse.com>
> > Reviewed-by: Juergen Gross <jgross@suse.com>
> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 4/8] x86/XPTI: use %r12 to write zero into xen_cr3

[Wei Liu]

On Mon, Mar 19, 2018 at 07:39:34AM -0600, Jan Beulich wrote:
> Now that we zero all registers early on all entry paths, use that to
> avoid a couple of immediates here.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 1/8] x86: NOP out XPTI entry/exit code when it's not in use

[Wei Liu]

On Mon, Mar 19, 2018 at 07:37:54AM -0600, Jan Beulich wrote:
> Introduce a synthetic feature flag to use alternative instruction
> patching to NOP out all code on entry/exit paths. Having NOPs here is
> generally better than using conditional branches.
> 
> Also change the limit on the number of bytes we can patch in one go to
> that resulting from the encoding in struct alt_instr - there's no point
> reducing it below that limit, and without a check being in place that
> the limit isn't actually exceeded, such an artificial boundary is a
> latent risk.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 3/8] x86: log XPTI enabled status

[Wei Liu]

On Mon, Mar 19, 2018 at 07:39:04AM -0600, Jan Beulich wrote:
> At the same time also report the state of the two defined
> ARCH_CAPABILITIES MSR bits. To avoid further complicating the
> conditional around that printk(), drop it (it's a debug level one only
> anyway).
> 
> Issue the main message without any XENLOG_*, and also drop XENLOG_INFO
> from the respective BTI message, to make sure they're visible at default
> log level also in release builds.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 5/8] x86/XPTI: reduce .text.entry

[Wei Liu]

On Mon, Mar 19, 2018 at 07:40:12AM -0600, Jan Beulich wrote:
> This exposes less code pieces and at the same time reduces the range
> covered from slightly above 3 pages to a little below 2 of them.
> 
> The code being moved is unchanged, except for the removal of trailing
> blanks, insertion of blanks between operands, and a pointless q suffix
> from "retq".
> 
> A few more small pieces could be moved, but it seems better to me to
> leave them where they are to not make it overly hard to follow code
> paths.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 6/8] x86: enable interrupts earlier with XPTI disabled

[Wei Liu]

On Mon, Mar 19, 2018 at 07:40:50AM -0600, Jan Beulich wrote:
> The STI instances were moved (or added in the INT80 case) to meet TLB
> flush requirements. When XPTI is disabled, they can be put back where
> they were (or omitted in the INT80 case).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 7/8] x86: also NOP out xen_cr3 restores of XPTI

[Wei Liu]

On Mon, Mar 19, 2018 at 07:41:11AM -0600, Jan Beulich wrote:
> ... despite quite likely the gain being rather limited.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 8/8] x86: avoid double CR3 reload when switching to guest user mode

[Wei Liu]

On Mon, Mar 19, 2018 at 07:41:42AM -0600, Jan Beulich wrote:
> When XPTI is active, the CR3 load in restore_all_guest is sufficient
> when switching to user mode, improving in particular system call and
> page fault exit paths for the guest.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>
> ---
> v2: Add ASSERT(!in_irq()).
> 
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -219,10 +219,22 @@ int pv_domain_initialise(struct domain *
>      return rc;
>  }
>  
> -static void _toggle_guest_pt(struct vcpu *v)
> +static void _toggle_guest_pt(struct vcpu *v, bool force_cr3)
>  {
> +    ASSERT(!in_irq());
> +
>      v->arch.flags ^= TF_kernel_mode;
>      update_cr3(v);
> +
> +    /*
> +     * There's no need to load CR3 here when it is going to be loaded on the
> +     * way out to guest mode again anyway, and when the page tables we're
> +     * currently on are the kernel ones (whereas when switching to kernel
> +     * mode we need to be able to write a bounce frame onto the kernel stack).
> +     */

Not sure I follow the comment. If you're talking about
create_bounce_frame, it wouldn't call this function in the first place,
right?

> +    if ( !force_cr3 && !(v->arch.flags & TF_kernel_mode) )

Also, it takes a bit of mental power to see !(v->arch.flags &
TF_kernel_mode) means the mode Xen is using. Can you maybe just use a
variable at the beginning like

   bool kernel_mode = v->arch.flags & TF_kernel_mode;

and then use it here?

> +        return;
> +
>      /* Don't flush user global mappings from the TLB. Don't tick TLB clock. */
>      asm volatile ( "mov %0, %%cr3" : : "r" (v->arch.cr3) : "memory" );
>  
> @@ -252,13 +264,13 @@ void toggle_guest_mode(struct vcpu *v)
>      }
>      asm volatile ( "swapgs" );
>  
> -    _toggle_guest_pt(v);
> +    _toggle_guest_pt(v, cpu_has_no_xpti);
>  }
>  
>  void toggle_guest_pt(struct vcpu *v)
>  {
>      if ( !is_pv_32bit_vcpu(v) )
> -        _toggle_guest_pt(v);
> +        _toggle_guest_pt(v, true);
>  }
>  
>  /*
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 8/8] x86: avoid double CR3 reload when switching to guest user mode

[Jan Beulich]

>>> On 22.03.18 at 14:20, <wei.liu2@citrix.com> wrote:
> On Mon, Mar 19, 2018 at 07:41:42AM -0600, Jan Beulich wrote:
>> --- a/xen/arch/x86/pv/domain.c
>> +++ b/xen/arch/x86/pv/domain.c
>> @@ -219,10 +219,22 @@ int pv_domain_initialise(struct domain *
>>      return rc;
>>  }
>>  
>> -static void _toggle_guest_pt(struct vcpu *v)
>> +static void _toggle_guest_pt(struct vcpu *v, bool force_cr3)
>>  {
>> +    ASSERT(!in_irq());
>> +
>>      v->arch.flags ^= TF_kernel_mode;
>>      update_cr3(v);
>> +
>> +    /*
>> +     * There's no need to load CR3 here when it is going to be loaded on the
>> +     * way out to guest mode again anyway, and when the page tables we're
>> +     * currently on are the kernel ones (whereas when switching to kernel
>> +     * mode we need to be able to write a bounce frame onto the kernel stack).
>> +     */
> 
> Not sure I follow the comment. If you're talking about
> create_bounce_frame, it wouldn't call this function in the first place,
> right?

Right. The comment is talking about what may happen after we
return from here.

>> +    if ( !force_cr3 && !(v->arch.flags & TF_kernel_mode) )
> 
> Also, it takes a bit of mental power to see !(v->arch.flags &
> TF_kernel_mode) means the mode Xen is using. Can you maybe just use a
> variable at the beginning like
> 
>    bool kernel_mode = v->arch.flags & TF_kernel_mode;
> 
> and then use it here?

Except for the (how I would say) clutter by the extra local variable
I don't see much of a difference.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 1/8] x86: NOP out XPTI entry/exit code when it's not in use

[Juergen Gross]

On 19/03/18 14:37, Jan Beulich wrote:
> Introduce a synthetic feature flag to use alternative instruction
> patching to NOP out all code on entry/exit paths. Having NOPs here is
> generally better than using conditional branches.
> 
> Also change the limit on the number of bytes we can patch in one go to
> that resulting from the encoding in struct alt_instr - there's no point
> reducing it below that limit, and without a check being in place that
> the limit isn't actually exceeded, such an artificial boundary is a
> latent risk.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Just did a parallel make of the hypervisor with and without the patch,
with xpti=true and with xpti=false (values in braces are stddev).

                       elapsed         system          user
unpatched, xpti=false: 118.69 ( 1.40)  168.38 (12.64)  186.11 ( 5.49)
unpatched, xpti=true : 128.02 ( 5.97)  219.66 (23.06)  197.84 ( 4.53)
patched,   xpti=false:  90.65 ( 6.63)   99.50 (14.79)  180.35 ( 5.97)
patched,   xpti=true : 111.69 ( 9.93)  163.63 (13.05)  181.22 ( 3.71)


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 1/8] x86: NOP out XPTI entry/exit code when it's not in use

[Juergen Gross]

On 03/04/18 19:48, Juergen Gross wrote:
> On 19/03/18 14:37, Jan Beulich wrote:
>> Introduce a synthetic feature flag to use alternative instruction
>> patching to NOP out all code on entry/exit paths. Having NOPs here is
>> generally better than using conditional branches.
>>
>> Also change the limit on the number of bytes we can patch in one go to
>> that resulting from the encoding in struct alt_instr - there's no point
>> reducing it below that limit, and without a check being in place that
>> the limit isn't actually exceeded, such an artificial boundary is a
>> latent risk.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Just did a parallel make of the hypervisor with and without the patch,
> with xpti=true and with xpti=false (values in braces are stddev).

The unpatched version was configured differently than the patched one.
So the real numbers are:

                       elapsed         system          user
unpatched, xpti=false:  89.96 ( 8.07)   97.05 ( 5.69)  178.64 ( 2.39)
unpatched, xpti=true : 113.42 ( 9.80)  165.99 (15.10)  180.99 ( 2.66)
patched,   xpti=false:  90.65 ( 6.63)   99.50 (14.79)  180.35 ( 5.97)
patched,   xpti=true : 111.69 ( 9.93)  163.63 (13.05)  181.22 ( 3.71)

So the XPTI case is a little bit faster with the patch, while the
non-XPTI case is a little bit slower.

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 8/8] x86: avoid double CR3 reload when switching to guest user mode

[Juergen Gross]

On 19/03/18 14:41, Jan Beulich wrote:
> When XPTI is active, the CR3 load in restore_all_guest is sufficient
> when switching to user mode, improving in particular system call and
> page fault exit paths for the guest.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>

I've done some more simple performance tests with some of the patches
with xpti=false and xpti=true. Data is always elapsed time, system time
and user time in seconds for a make -j 4 in dom0 with 4 vcpus, stddev
in braces, based on 5 runs each (I tried 30 runs, but the result didn't
really change):

xpti=false
no patch:  89.96 ( 2.84)   97.05 ( 2.39)  178.64 ( 1.55)
Jan p1:    90.65 ( 2.57)   99.50 ( 3.85)  180.35 ( 2.44)
Jan p5:    91.33 ( 0.89)   99.72 ( 2.56)  180.97 ( 1.71)
Jan p6:    90.86 ( 2.59)   97.09 ( 2.59)  177.85 ( 2.35)
Jan p7:    90.72 ( 2.84)  100.10 ( 4.60)  179.85 ( 2.61)
Jan p8:    88.59 ( 0.71)   96.31 ( 2.14)  178.47 ( 0.86)

xpti=true
no patch: 113.42 ( 3.13)  165.99 ( 3.89)  180.99 ( 1.63)
Jan p1:   111.69 ( 3.15)  163.63 ( 3.61)  181.22 ( 1.93)
Jan p5:   114.76 ( 2.28)  167.15 ( 4.67)  181.13 ( 1.75)
Jan p6:   116.85 ( 2.35)  168.73 ( 3.68)  181.27 ( 1.98)
Jan p7:   115.37 ( 2.71)  166.96 ( 4.41)  180.82 ( 1.98)
Jan p8:   114.85 ( 2.83)  167.08 ( 5.00)  181.27 ( 1.85)

Summing it up: performance isn't really changing for any of the patches.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 1/8] x86: NOP out XPTI entry/exit code when it's not in use

[Jan Beulich]

>>> On 04.04.18 at 08:07, <jgross@suse.com> wrote:
> On 03/04/18 19:48, Juergen Gross wrote:
>> On 19/03/18 14:37, Jan Beulich wrote:
>>> Introduce a synthetic feature flag to use alternative instruction
>>> patching to NOP out all code on entry/exit paths. Having NOPs here is
>>> generally better than using conditional branches.
>>>
>>> Also change the limit on the number of bytes we can patch in one go to
>>> that resulting from the encoding in struct alt_instr - there's no point
>>> reducing it below that limit, and without a check being in place that
>>> the limit isn't actually exceeded, such an artificial boundary is a
>>> latent risk.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> Just did a parallel make of the hypervisor with and without the patch,
>> with xpti=true and with xpti=false (values in braces are stddev).
> 
> The unpatched version was configured differently than the patched one.
> So the real numbers are:
> 
>                        elapsed         system          user
> unpatched, xpti=false:  89.96 ( 8.07)   97.05 ( 5.69)  178.64 ( 2.39)
> unpatched, xpti=true : 113.42 ( 9.80)  165.99 (15.10)  180.99 ( 2.66)
> patched,   xpti=false:  90.65 ( 6.63)   99.50 (14.79)  180.35 ( 5.97)
> patched,   xpti=true : 111.69 ( 9.93)  163.63 (13.05)  181.22 ( 3.71)
> 
> So the XPTI case is a little bit faster with the patch, while the
> non-XPTI case is a little bit slower.

That's rather counterintuitive a result.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v4 1/8] x86: NOP out XPTI entry/exit code when it's not in use

[Juergen Gross]

On 09/04/18 10:28, Jan Beulich wrote:
>>>> On 04.04.18 at 08:07, <jgross@suse.com> wrote:
>> On 03/04/18 19:48, Juergen Gross wrote:
>>> On 19/03/18 14:37, Jan Beulich wrote:
>>>> Introduce a synthetic feature flag to use alternative instruction
>>>> patching to NOP out all code on entry/exit paths. Having NOPs here is
>>>> generally better than using conditional branches.
>>>>
>>>> Also change the limit on the number of bytes we can patch in one go to
>>>> that resulting from the encoding in struct alt_instr - there's no point
>>>> reducing it below that limit, and without a check being in place that
>>>> the limit isn't actually exceeded, such an artificial boundary is a
>>>> latent risk.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Just did a parallel make of the hypervisor with and without the patch,
>>> with xpti=true and with xpti=false (values in braces are stddev).
>>
>> The unpatched version was configured differently than the patched one.
>> So the real numbers are:
>>
>>                        elapsed         system          user
>> unpatched, xpti=false:  89.96 ( 8.07)   97.05 ( 5.69)  178.64 ( 2.39)
>> unpatched, xpti=true : 113.42 ( 9.80)  165.99 (15.10)  180.99 ( 2.66)
>> patched,   xpti=false:  90.65 ( 6.63)   99.50 (14.79)  180.35 ( 5.97)
>> patched,   xpti=true : 111.69 ( 9.93)  163.63 (13.05)  181.22 ( 3.71)
>>
>> So the XPTI case is a little bit faster with the patch, while the
>> non-XPTI case is a little bit slower.
> 
> That's rather counterintuitive a result.

OTOH the standard deviation is not really small, so it might be the
performance difference is just below noise level.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel