* [PATCH v9 0/9] xen/x86: various XPTI speedups
@ 2018-04-26 11:33 Juergen Gross
2018-04-26 11:33 ` [PATCH v9 1/9] x86/xpti: avoid copying L4 page table contents when possible Juergen Gross
` (11 more replies)
0 siblings, 12 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
This patch series aims at reducing the overhead of the XPTI Meltdown
mitigation.
Patch 1 had been posted before, the main changes in this patch are due
to addressing Jan's comments on my first version. The main objective of
that patch is to avoid copying the L4 page table each time the guest is
being activated, as often the contents didn't change while the
hypervisor was active.
Patch 2 adds a new helper for writing cr3 instead of open coding the
inline assembly in multiple places.
Patch 3 sets the stage for being able to activate XPTI per domain. As a
first step it is now possible to switch XPTI off for dom0 via the xpti
boot parameter.
Patch 4 adds support for using the INVPCID instruction for flushing
the TLB.
Patch 5 reduces the costs of TLB flushes even further: as we don't make
any use of global TLB entries with XPTI being active we can avoid
removing all global TLB entries on TLB flushes by simply deactivating
the global pages in CR4.
Patch 6 prepares using PCIDs in patch 6.
For that purpose it was necessary to allow CR3 values with bit 63 set
in order to avoid flushing TLB entries when writing CR3. This requires
a modification of Jan's rather clever state machine with positive and
negative CR3 values for the hypervisor by using a dedicated flag byte
instead.
Patch 7 converts pv_guest_cr4_to_real_cr4() from a macro to a function
as it was becoming more and more complex.
Patch 8 adds some PCID helper functions for accessing the different
parts of cr3 (address and pcid part).
Patch 9 is the main performance contributor: by making use of the PCID
feature (if available) TLB entries can survive CR3 switches. The TLB
needs to be flushed on context switches only and not when switching
between guest and hypervisor or guest kernel and user mode.
On my machine (Intel i7-4600M) using the PCID feature in the non-XPTI
case showed a slightly worse performance than using global pages
instead (using PCID and global pages is a bad idea as invalidating
global pages in this case would need a complete TLB flush). For this
reason I've decided to use PCID for XPTI only as the default. That
can easily be changed by using the command line parameter "pcid=true".
The complete series has been verified to still mitigate against
Meltdown attacks. A simple performance test (make -j 4 in the Xen
hypervisor directory) showed significant improvements compared to the
state without this series.
Numbers are seconds, stddev in braces.
xpti=false elapsed system user
unpatched: 88.42 ( 2.01) 94.49 ( 1.38) 180.40 ( 1.41)
patched : 89.45 ( 3.10) 96.47 ( 3.22) 181.34 ( 1.98)
xpti=true elapsed system user
unpatched: 113.43 ( 3.68) 165.44 ( 4.41) 183.30 ( 1.72)
patched : 92.76 ( 2.11) 103.39 ( 1.13) 184.86 ( 0.12)
There have been reports of hanging old (kernel 2.6.18 based) 64-bit pv
guests on old hardware (no full PCID support). I have not been able to
reproduce those hangs. I suggest to wait applying the last patch until
this issue has been sorted out.
Changes since last version:
- Patch 1: use flush_mask() to do root page table flushing (Jan Beulich)
- Patch 1: move root page table flushing out of shadow code (Tim Deegan)
- Patch 3: adjust boot message (Sergey Dyasli)
- Patch 3: adjust boot parameter documentation (Sergey Dyasli)
Juergen Gross (9):
x86/xpti: avoid copying L4 page table contents when possible
xen/x86: add a function for modifying cr3
xen/x86: support per-domain flag for xpti
xen/x86: use invpcid for flushing the TLB
xen/x86: disable global pages for domains with XPTI active
xen/x86: use flag byte for decision whether xen_cr3 is valid
xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
xen/x86: add some cr3 helpers
xen/x86: use PCID feature
docs/misc/xen-command-line.markdown | 37 +++++++++++++-
xen/arch/x86/cpu/mtrr/generic.c | 37 +++++++++-----
xen/arch/x86/debug.c | 2 +-
xen/arch/x86/domain.c | 6 +--
xen/arch/x86/domain_page.c | 2 +-
xen/arch/x86/flushtlb.c | 99 ++++++++++++++++++++++++++++++-------
xen/arch/x86/mm.c | 86 ++++++++++++++++++++++++++------
xen/arch/x86/mm/shadow/multi.c | 3 ++
xen/arch/x86/pv/dom0_build.c | 8 +--
xen/arch/x86/pv/domain.c | 89 ++++++++++++++++++++++++++++++++-
xen/arch/x86/setup.c | 27 +++-------
xen/arch/x86/smp.c | 2 +-
xen/arch/x86/smpboot.c | 6 ++-
xen/arch/x86/spec_ctrl.c | 75 +++++++++++++++++++++++++++-
xen/arch/x86/x86_64/asm-offsets.c | 2 +
xen/arch/x86/x86_64/compat/entry.S | 5 +-
xen/arch/x86/x86_64/entry.S | 82 +++++++++++++-----------------
xen/common/efi/runtime.c | 4 +-
xen/include/asm-x86/current.h | 23 +++++++--
xen/include/asm-x86/domain.h | 17 +++----
xen/include/asm-x86/flushtlb.h | 10 +++-
xen/include/asm-x86/invpcid.h | 2 +
xen/include/asm-x86/processor.h | 18 +++++++
xen/include/asm-x86/pv/domain.h | 31 ++++++++++++
xen/include/asm-x86/spec_ctrl.h | 4 ++
xen/include/asm-x86/x86-defns.h | 4 +-
26 files changed, 532 insertions(+), 149 deletions(-)
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v9 1/9] x86/xpti: avoid copying L4 page table contents when possible
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-26 14:01 ` Tim Deegan
2018-04-26 11:33 ` [PATCH v9 2/9] xen/x86: add a function for modifying cr3 Juergen Gross
` (10 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
For mitigation of Meltdown the current L4 page table is copied to the
cpu local root page table each time a 64 bit pv guest is entered.
Copying can be avoided in cases where the guest L4 page table hasn't
been modified while running the hypervisor, e.g. when handling
interrupts or any hypercall not modifying the L4 page table or %cr3.
So add a per-cpu flag indicating whether the copying should be
performed and set that flag only when loading a new %cr3 or modifying
the L4 page table. This includes synchronization of the cpu local
root page table with other cpus, so add a special synchronization flag
for that case.
A simple performance check (compiling the hypervisor via "make -j 4")
in dom0 with 4 vcpus shows a significant improvement:
- real time drops from 112 seconds to 103 seconds
- system time drops from 142 seconds to 131 seconds
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V9:
- use flush_mask() to do root page table flushing (Jan Beulich)
- move root page table flushing out of shadow code (Tim Deegan)
V8:
- more shadow code: set flag on all affected cpus (Tim Deegan)
V7:
- add missing flag setting in shadow code
V6:
- correct an error from rebasing to staging in assembly part
V4:
- move setting of root_pgt_changed flag in flush_area_local() out of
irq disabled section (Jan Beulich)
- move setting of root_pgt_changed in make_cr3() to _toggle_guest_pt()
(Jan Beulich)
- remove most conditionals in write_ptbase() (Jan Beulich)
- don't set root_pgt_changed in do_mmu_update() for modification of
the user page table (Jan Beulich)
V3:
- set flag locally only if affected L4 is active (Jan Beulich)
- add setting flag to flush_area_mask() (Jan Beulich)
- set flag in make_cr3() only if called for current active vcpu
To be applied on top of Jan's "Meltdown band-aid overhead reduction"
series
---
xen/arch/x86/flushtlb.c | 4 ++++
xen/arch/x86/mm.c | 36 +++++++++++++++++++++++-------------
xen/arch/x86/mm/shadow/multi.c | 3 +++
xen/arch/x86/pv/domain.c | 2 ++
xen/arch/x86/smp.c | 2 +-
xen/arch/x86/x86_64/asm-offsets.c | 1 +
xen/arch/x86/x86_64/entry.S | 9 +++++++--
xen/include/asm-x86/current.h | 8 ++++++++
xen/include/asm-x86/flushtlb.h | 8 ++++++++
9 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 8a7a76b8ff..2729ba42e7 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -8,6 +8,7 @@
*/
#include <xen/sched.h>
+#include <xen/smp.h>
#include <xen/softirq.h>
#include <asm/flushtlb.h>
#include <asm/page.h>
@@ -160,5 +161,8 @@ unsigned int flush_area_local(const void *va, unsigned int flags)
local_irq_restore(irqfl);
+ if ( flags & FLUSH_ROOT_PGTBL )
+ get_cpu_info()->root_pgt_changed = true;
+
return flags;
}
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 53b36d8889..5373879359 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -502,6 +502,7 @@ void make_cr3(struct vcpu *v, mfn_t mfn)
void write_ptbase(struct vcpu *v)
{
+ get_cpu_info()->root_pgt_changed = true;
write_cr3(v->arch.cr3);
}
@@ -3699,18 +3700,27 @@ long do_mmu_update(
break;
rc = mod_l4_entry(va, l4e_from_intpte(req.val), mfn,
cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
- /*
- * No need to sync if all uses of the page can be accounted
- * to the page lock we hold, its pinned status, and uses on
- * this (v)CPU.
- */
- 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) +
- (pagetable_get_pfn(curr->arch.guest_table_user) ==
- mfn))) )
- sync_guest = true;
+ if ( !rc && !cpu_has_no_xpti )
+ {
+ bool local_in_use = false;
+
+ if ( pagetable_get_pfn(curr->arch.guest_table) == mfn )
+ {
+ local_in_use = true;
+ get_cpu_info()->root_pgt_changed = true;
+ }
+
+ /*
+ * No need to sync if all uses of the page can be
+ * accounted to the page lock we hold, its pinned
+ * status, and uses on this (v)CPU.
+ */
+ if ( (page->u.inuse.type_info & PGT_count_mask) >
+ (1 + !!(page->u.inuse.type_info & PGT_pinned) +
+ (pagetable_get_pfn(curr->arch.guest_table_user) ==
+ mfn) + local_in_use) )
+ sync_guest = true;
+ }
break;
case PGT_writable_page:
@@ -3825,7 +3835,7 @@ long do_mmu_update(
cpumask_andnot(mask, pt_owner->dirty_cpumask, cpumask_of(cpu));
if ( !cpumask_empty(mask) )
- flush_mask(mask, FLUSH_TLB_GLOBAL);
+ flush_mask(mask, FLUSH_TLB_GLOBAL | FLUSH_ROOT_PGTBL);
}
perfc_add(num_page_updates, i);
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 9c3af330ec..da586c21c7 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -952,6 +952,8 @@ static int shadow_set_l4e(struct domain *d,
/* Write the new entry */
shadow_write_entries(sl4e, &new_sl4e, 1, sl4mfn);
+ flush_root_pgtbl_domain(d);
+
flags |= SHADOW_SET_CHANGED;
if ( shadow_l4e_get_flags(old_sl4e) & _PAGE_PRESENT )
@@ -966,6 +968,7 @@ static int shadow_set_l4e(struct domain *d,
}
sh_put_ref(d, osl3mfn, paddr);
}
+
return flags;
}
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index ac65ba4609..b1c40373fa 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -217,6 +217,8 @@ static void _toggle_guest_pt(struct vcpu *v)
{
v->arch.flags ^= TF_kernel_mode;
update_cr3(v);
+ get_cpu_info()->root_pgt_changed = true;
+
/* Don't flush user global mappings from the TLB. Don't tick TLB clock. */
asm volatile ( "mov %0, %%cr3" : : "r" (v->arch.cr3) : "memory" );
diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
index 033dd05958..63e819ca38 100644
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -208,7 +208,7 @@ void invalidate_interrupt(struct cpu_user_regs *regs)
ack_APIC_irq();
perfc_incr(ipis);
if ( (flags & FLUSH_VCPU_STATE) && __sync_local_execstate() )
- flags &= ~(FLUSH_TLB | FLUSH_TLB_GLOBAL);
+ flags &= ~(FLUSH_TLB | FLUSH_TLB_GLOBAL | FLUSH_ROOT_PGTBL);
if ( flags & ~(FLUSH_VCPU_STATE | FLUSH_ORDER_MASK) )
flush_area_local(flush_va, flags);
cpumask_clear_cpu(smp_processor_id(), &flush_cpumask);
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index a2fea94f4c..9e2aefb00f 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -143,6 +143,7 @@ void __dummy__(void)
OFFSET(CPUINFO_shadow_spec_ctrl, struct cpu_info, shadow_spec_ctrl);
OFFSET(CPUINFO_use_shadow_spec_ctrl, struct cpu_info, use_shadow_spec_ctrl);
OFFSET(CPUINFO_bti_ist_info, struct cpu_info, bti_ist_info);
+ OFFSET(CPUINFO_root_pgt_changed, struct cpu_info, root_pgt_changed);
DEFINE(CPUINFO_sizeof, sizeof(struct cpu_info));
BLANK();
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 9c45c00bb0..732f834a09 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -129,11 +129,15 @@ restore_all_guest:
mov VCPU_cr3(%rbx), %r9
GET_STACK_END(dx)
mov STACK_CPUINFO_FIELD(pv_cr3)(%rdx), %rdi
+ test %rdi, %rdi
+ jz .Lrag_keep_cr3
+ mov %rdi, %rax
+ cmpb $0, STACK_CPUINFO_FIELD(root_pgt_changed)(%rdx)
+ je .Lrag_copy_done
+ movb $0, STACK_CPUINFO_FIELD(root_pgt_changed)(%rdx)
movabs $PADDR_MASK & PAGE_MASK, %rsi
movabs $DIRECTMAP_VIRT_START, %rcx
- mov %rdi, %rax
and %rsi, %rdi
- jz .Lrag_keep_cr3
and %r9, %rsi
add %rcx, %rdi
add %rcx, %rsi
@@ -148,6 +152,7 @@ restore_all_guest:
sub $(ROOT_PAGETABLE_FIRST_XEN_SLOT - \
ROOT_PAGETABLE_LAST_XEN_SLOT - 1) * 8, %rdi
rep movsq
+.Lrag_copy_done:
mov STACK_CPUINFO_FIELD(cr4)(%rdx), %rdi
mov %r9, STACK_CPUINFO_FIELD(xen_cr3)(%rdx)
mov %rdi, %rsi
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 3a0e1eef36..f2491b4423 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -59,6 +59,14 @@ struct cpu_info {
bool use_shadow_spec_ctrl;
uint8_t bti_ist_info;
+ /*
+ * The following field controls copying of the L4 page table of 64-bit
+ * PV guests to the per-cpu root page table on entering the guest context.
+ * If set the L4 page table is being copied to the root page table and
+ * the field will be reset.
+ */
+ bool root_pgt_changed;
+
unsigned long __pad;
/* get_stack_bottom() must be 16-byte aligned */
};
diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm-x86/flushtlb.h
index 2cade9cbfb..9b2336c07a 100644
--- a/xen/include/asm-x86/flushtlb.h
+++ b/xen/include/asm-x86/flushtlb.h
@@ -103,6 +103,8 @@ void write_cr3(unsigned long cr3);
#define FLUSH_VA_VALID 0x800
/* Flush CPU state */
#define FLUSH_VCPU_STATE 0x1000
+ /* Flush the per-cpu root page table */
+#define FLUSH_ROOT_PGTBL 0x2000
/* Flush local TLBs/caches. */
unsigned int flush_area_local(const void *va, unsigned int flags);
@@ -134,6 +136,12 @@ void flush_area_mask(const cpumask_t *, const void *va, unsigned int flags);
#define flush_tlb_one_all(v) \
flush_tlb_one_mask(&cpu_online_map, v)
+#define flush_root_pgtbl_domain(d) \
+{ \
+ if ( !cpu_has_no_xpti && is_pv_domain(d) && !is_pv_32bit_domain(d) ) \
+ flush_mask((d)->dirty_cpumask, FLUSH_ROOT_PGTBL); \
+}
+
static inline void flush_page_to_ram(unsigned long mfn, bool sync_icache) {}
static inline int invalidate_dcache_va_range(const void *p,
unsigned long size)
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 2/9] xen/x86: add a function for modifying cr3
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
2018-04-26 11:33 ` [PATCH v9 1/9] x86/xpti: avoid copying L4 page table contents when possible Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-26 11:33 ` [PATCH v9 3/9] xen/x86: support per-domain flag for xpti Juergen Gross
` (9 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
Instead of having multiple places with more or less identical asm
statements just have one function doing a write to cr3.
As this function should be named write_cr3() rename the current
write_cr3() function to switch_cr3().
Suggested-by: Andrew Copper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V6:
- new patch
---
xen/arch/x86/flushtlb.c | 4 ++--
xen/arch/x86/mm.c | 2 +-
xen/arch/x86/pv/domain.c | 2 +-
xen/common/efi/runtime.c | 4 ++--
xen/include/asm-x86/flushtlb.h | 2 +-
xen/include/asm-x86/processor.h | 5 +++++
6 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 2729ba42e7..1af9221607 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -72,7 +72,7 @@ static void post_flush(u32 t)
this_cpu(tlbflush_time) = t;
}
-void write_cr3(unsigned long cr3)
+void switch_cr3(unsigned long cr3)
{
unsigned long flags, cr4;
u32 t;
@@ -84,7 +84,7 @@ void write_cr3(unsigned long cr3)
cr4 = read_cr4();
write_cr4(cr4 & ~X86_CR4_PGE);
- asm volatile ( "mov %0, %%cr3" : : "r" (cr3) : "memory" );
+ write_cr3(cr3);
write_cr4(cr4);
post_flush(t);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5373879359..7e2cc924fd 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -503,7 +503,7 @@ void make_cr3(struct vcpu *v, mfn_t mfn)
void write_ptbase(struct vcpu *v)
{
get_cpu_info()->root_pgt_changed = true;
- write_cr3(v->arch.cr3);
+ switch_cr3(v->arch.cr3);
}
/*
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index b1c40373fa..be40843b05 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -220,7 +220,7 @@ static void _toggle_guest_pt(struct vcpu *v)
get_cpu_info()->root_pgt_changed = true;
/* Don't flush user global mappings from the TLB. Don't tick TLB clock. */
- asm volatile ( "mov %0, %%cr3" : : "r" (v->arch.cr3) : "memory" );
+ write_cr3(v->arch.cr3);
if ( !(v->arch.flags & TF_kernel_mode) )
return;
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 3dbc2e8ee5..4e5ddfef4f 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -111,7 +111,7 @@ struct efi_rs_state efi_rs_enter(void)
lgdt(&gdt_desc);
}
- write_cr3(virt_to_maddr(efi_l4_pgtable));
+ switch_cr3(virt_to_maddr(efi_l4_pgtable));
return state;
}
@@ -120,7 +120,7 @@ void efi_rs_leave(struct efi_rs_state *state)
{
if ( !state->cr3 )
return;
- write_cr3(state->cr3);
+ switch_cr3(state->cr3);
if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
{
struct desc_ptr gdt_desc = {
diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm-x86/flushtlb.h
index 9b2336c07a..d27906c0ae 100644
--- a/xen/include/asm-x86/flushtlb.h
+++ b/xen/include/asm-x86/flushtlb.h
@@ -84,7 +84,7 @@ static inline unsigned long read_cr3(void)
}
/* Write pagetable base and implicitly tick the tlbflush clock. */
-void write_cr3(unsigned long cr3);
+void switch_cr3(unsigned long cr3);
/* flush_* flag fields: */
/*
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index db9988ab33..71d32c0333 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -283,6 +283,11 @@ static inline unsigned long read_cr2(void)
return cr2;
}
+static inline void write_cr3(unsigned long val)
+{
+ asm volatile ( "mov %0, %%cr3" : : "r" (val) : "memory" );
+}
+
static inline unsigned long read_cr4(void)
{
return get_cpu_info()->cr4;
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 3/9] xen/x86: support per-domain flag for xpti
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
2018-04-26 11:33 ` [PATCH v9 1/9] x86/xpti: avoid copying L4 page table contents when possible Juergen Gross
2018-04-26 11:33 ` [PATCH v9 2/9] xen/x86: add a function for modifying cr3 Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-27 7:55 ` Sergey Dyasli
2018-04-26 11:33 ` [PATCH v9 4/9] xen/x86: use invpcid for flushing the TLB Juergen Gross
` (8 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
Instead of switching XPTI globally on or off add a per-domain flag for
that purpose. This allows to modify the xpti boot parameter to support
running dom0 without Meltdown mitigations. Using "xpti=no-dom0" as boot
parameter will achieve that.
Move the xpti boot parameter handling to xen/arch/x86/pv/domain.c as
it is pv-domain specific.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V9:
- adjust boot message (Sergey Dyasli)
- adjust boot parameter documentation (Sergey Dyasli)
V6.1:
- address some minor comments (Jan Beulich)
V6:
- modify xpti boot parameter options (Andrew Cooper)
- move xpti_init() code to spec_ctrl.c (Andrew Cooper)
- irework init of per-domain xpti flag (Andrew Cooper)
V3:
- latch get_cpu_info() return value in variable (Jan Beulich)
- call always xpti_domain_init() for pv dom0 (Jan Beulich)
- add __init annotations (Jan Beulich)
- drop per domain XPTI message (Jan Beulich)
- document xpti=default support (Jan Beulich)
- move domain xpti flag into a padding hole (Jan Beulich)
---
docs/misc/xen-command-line.markdown | 14 ++++++-
xen/arch/x86/mm.c | 17 ++++++++-
xen/arch/x86/pv/dom0_build.c | 1 +
xen/arch/x86/pv/domain.c | 6 +++
xen/arch/x86/setup.c | 19 ----------
xen/arch/x86/smpboot.c | 4 +-
xen/arch/x86/spec_ctrl.c | 75 ++++++++++++++++++++++++++++++++++++-
xen/include/asm-x86/current.h | 3 +-
xen/include/asm-x86/domain.h | 3 ++
xen/include/asm-x86/flushtlb.h | 2 +-
xen/include/asm-x86/spec_ctrl.h | 4 ++
11 files changed, 119 insertions(+), 29 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index b353352adf..220d1ba020 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1955,14 +1955,24 @@ clustered mode. The default, given no hint from the **FADT**, is cluster
mode.
### xpti
-> `= <boolean>`
+> `= List of [ default | <boolean> | dom0=<bool> | domu=<bool> ]`
-> Default: `false` on AMD hardware
+> Default: `false` on hardware not to be vulnerable to Meltdown (e.g. AMD)
> Default: `true` everywhere else
Override default selection of whether to isolate 64-bit PV guest page
tables.
+`true` activates page table isolation even on hardware not vulnerable by
+Meltdown for all domains.
+
+`false` deactivates page table isolation on all systems for all domains.
+
+`default` sets the default behaviour.
+
+With `dom0` and `domu` it is possible to control page table isolation
+for dom0 or guest domains only.
+
### xsave
> `= <boolean>`
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7e2cc924fd..b1e8133d92 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -502,8 +502,21 @@ void make_cr3(struct vcpu *v, mfn_t mfn)
void write_ptbase(struct vcpu *v)
{
- get_cpu_info()->root_pgt_changed = true;
- switch_cr3(v->arch.cr3);
+ struct cpu_info *cpu_info = get_cpu_info();
+
+ if ( is_pv_vcpu(v) && v->domain->arch.pv_domain.xpti )
+ {
+ cpu_info->root_pgt_changed = true;
+ cpu_info->pv_cr3 = __pa(this_cpu(root_pgt));
+ switch_cr3(v->arch.cr3);
+ }
+ else
+ {
+ /* Make sure to clear xen_cr3 before pv_cr3; switch_cr3() serializes. */
+ cpu_info->xen_cr3 = 0;
+ switch_cr3(v->arch.cr3);
+ cpu_info->pv_cr3 = 0;
+ }
}
/*
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 5b4325b87f..d148395919 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -387,6 +387,7 @@ int __init dom0_construct_pv(struct domain *d,
if ( compat32 )
{
d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
+ d->arch.pv_domain.xpti = false;
v->vcpu_info = (void *)&d->shared_info->compat.vcpu_info[0];
if ( setup_compat_arg_xlat(v) != 0 )
BUG();
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index be40843b05..ce1a1a9d35 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -9,6 +9,7 @@
#include <xen/lib.h>
#include <xen/sched.h>
+#include <asm/spec_ctrl.h>
#include <asm/pv/domain.h>
static void noreturn continue_nonidle_domain(struct vcpu *v)
@@ -75,6 +76,8 @@ int switch_compat(struct domain *d)
d->arch.x87_fip_width = 4;
+ d->arch.pv_domain.xpti = false;
+
return 0;
undo_and_fail:
@@ -205,6 +208,9 @@ int pv_domain_initialise(struct domain *d)
/* 64-bit PV guest by default. */
d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
+ d->arch.pv_domain.xpti = opt_xpti & (is_hardware_domain(d)
+ ? OPT_XPTI_DOM0 : OPT_XPTI_DOMU);
+
return 0;
fail:
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b2baee3d2c..f803980b97 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -169,9 +169,6 @@ static int __init parse_smap_param(const char *s)
}
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] = "";
@@ -1546,22 +1543,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS;
- if ( opt_xpti < 0 )
- {
- 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
- setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
-
if ( cpu_has_fsgsbase )
set_in_cr4(X86_CR4_FSGSBASE);
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 0c38061896..48aecdd17f 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -337,7 +337,7 @@ void start_secondary(void *unused)
spin_debug_disable();
get_cpu_info()->xen_cr3 = 0;
- get_cpu_info()->pv_cr3 = this_cpu(root_pgt) ? __pa(this_cpu(root_pgt)) : 0;
+ get_cpu_info()->pv_cr3 = 0;
load_system_tables();
@@ -1056,7 +1056,7 @@ void __init smp_prepare_cpus(void)
panic("Error %d setting up PV root page table\n", rc);
if ( per_cpu(root_pgt, 0) )
{
- get_cpu_info()->pv_cr3 = __pa(per_cpu(root_pgt, 0));
+ get_cpu_info()->pv_cr3 = 0;
/*
* All entry points which may need to switch page tables have to start
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index fab3c1dc0e..037e84dc13 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -136,8 +136,9 @@ static void __init print_details(enum ind_thunk thunk)
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");
+ printk("XPTI (64-bit PV only): Dom0 %s, DomU %s\n",
+ opt_xpti & OPT_XPTI_DOM0 ? "enabled" : "disabled",
+ opt_xpti & OPT_XPTI_DOMU ? "enabled" : "disabled");
}
/* Calculate whether Retpoline is known-safe on this CPU. */
@@ -230,6 +231,70 @@ static bool __init retpoline_safe(void)
}
}
+#define OPT_XPTI_DEFAULT 0xff
+uint8_t __read_mostly opt_xpti = OPT_XPTI_DEFAULT;
+
+static __init void xpti_init_default(bool force)
+{
+ uint64_t caps = 0;
+
+ if ( !force && (opt_xpti != OPT_XPTI_DEFAULT) )
+ return;
+
+ 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);
+
+ if ( caps & ARCH_CAPABILITIES_RDCL_NO )
+ opt_xpti = 0;
+ else
+ opt_xpti = OPT_XPTI_DOM0 | OPT_XPTI_DOMU;
+}
+
+static __init int parse_xpti(const char *s)
+{
+ const char *ss;
+ int val, rc = 0;
+
+ xpti_init_default(false);
+
+ do {
+ ss = strchr(s, ',');
+ if ( !ss )
+ ss = strchr(s, '\0');
+
+ switch ( parse_bool(s, ss) )
+ {
+ case 0:
+ opt_xpti = 0;
+ break;
+
+ case 1:
+ opt_xpti = OPT_XPTI_DOM0 | OPT_XPTI_DOMU;
+ break;
+
+ default:
+ if ( !strcmp(s, "default") )
+ xpti_init_default(true);
+ else if ( (val = parse_boolean("dom0", s, ss)) >= 0 )
+ opt_xpti = (opt_xpti & ~OPT_XPTI_DOM0) |
+ (val ? OPT_XPTI_DOM0 : 0);
+ else if ( (val = parse_boolean("domu", s, ss)) >= 0 )
+ opt_xpti = (opt_xpti & ~OPT_XPTI_DOMU) |
+ (val ? OPT_XPTI_DOMU : 0);
+ else
+ rc = -EINVAL;
+ break;
+ }
+
+ s = ss + 1;
+ } while ( *ss );
+
+ return rc;
+}
+custom_param("xpti", parse_xpti);
+
void __init init_speculation_mitigations(void)
{
enum ind_thunk thunk = THUNK_DEFAULT;
@@ -347,6 +412,12 @@ void __init init_speculation_mitigations(void)
/* (Re)init BSP state now that default_bti_ist_info has been calculated. */
init_shadow_spec_ctrl_state();
+ xpti_init_default(false);
+ if ( opt_xpti == 0 )
+ setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
+ else
+ setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
+
print_details(thunk);
}
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index f2491b4423..b2475783f8 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -44,7 +44,8 @@ struct cpu_info {
/*
* Of the two following fields the latter is being set to the CR3 value
* to be used on the given pCPU for loading whenever 64-bit PV guest
- * context is being entered. The value never changes once set.
+ * context is being entered. A value of zero indicates no setting of CR3
+ * is to be performed.
* The former is the value to restore when re-entering Xen, if any. IOW
* its value being zero means there's nothing to restore. However, its
* value can also be negative, indicating to the exit-to-Xen code that
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index a12ae47f1b..ed4199931a 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -253,6 +253,9 @@ struct pv_domain
atomic_t nr_l4_pages;
+ /* XPTI active? */
+ bool xpti;
+
/* map_domain_page() mapping cache. */
struct mapcache_domain mapcache;
diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm-x86/flushtlb.h
index d27906c0ae..fe48c89de7 100644
--- a/xen/include/asm-x86/flushtlb.h
+++ b/xen/include/asm-x86/flushtlb.h
@@ -138,7 +138,7 @@ void flush_area_mask(const cpumask_t *, const void *va, unsigned int flags);
#define flush_root_pgtbl_domain(d) \
{ \
- if ( !cpu_has_no_xpti && is_pv_domain(d) && !is_pv_32bit_domain(d) ) \
+ if ( is_pv_domain(d) && (d)->arch.pv_domain.xpti ) \
flush_mask((d)->dirty_cpumask, FLUSH_ROOT_PGTBL); \
}
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 5ab4ff3f68..b4fa43269e 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -29,6 +29,10 @@ void init_speculation_mitigations(void);
extern bool opt_ibpb;
extern uint8_t default_bti_ist_info;
+extern uint8_t opt_xpti;
+#define OPT_XPTI_DOM0 0x01
+#define OPT_XPTI_DOMU 0x02
+
static inline void init_shadow_spec_ctrl_state(void)
{
struct cpu_info *info = get_cpu_info();
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 4/9] xen/x86: use invpcid for flushing the TLB
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (2 preceding siblings ...)
2018-04-26 11:33 ` [PATCH v9 3/9] xen/x86: support per-domain flag for xpti Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-26 11:33 ` [PATCH v9 5/9] xen/x86: disable global pages for domains with XPTI active Juergen Gross
` (7 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
If possible use the INVPCID instruction for flushing the TLB instead of
toggling cr4.pge for that purpose.
While at it remove the dependency on cr4.pge being required for mtrr
loading, as this will be required later anyway.
Add a command line option "invpcid" for controlling the use of
INVPCID (default to true).
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V6:
- reword invpcid parameter description (Andrew Cooper)
- add __read_mostly to use_invpcid definition (Andrew Cooper)
V5:
- use pre_flush() as an initializer in do_tlb_flush() (Jan Beulich)
- introduce boolean use_invpcid instead of clearing X86_FEATURE_INVPCID
(Jan Beulich)
V4:
- option "invpcid" instead of "noinvpcid" (Jan Beulich)
V3:
- new patch
---
docs/misc/xen-command-line.markdown | 9 +++++++++
xen/arch/x86/cpu/mtrr/generic.c | 37 ++++++++++++++++++++++++++-----------
xen/arch/x86/flushtlb.c | 29 +++++++++++++++++++----------
xen/arch/x86/setup.c | 8 ++++++++
xen/include/asm-x86/invpcid.h | 2 ++
5 files changed, 64 insertions(+), 21 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 220d1ba020..f8264d8abb 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1380,6 +1380,15 @@ Because responsibility for APIC setup is shared between Xen and the
domain 0 kernel this option is automatically propagated to the domain
0 command line.
+### invpcid (x86)
+> `= <boolean>`
+
+> Default: `true`
+
+By default, Xen will use the INVPCID instruction for TLB management if
+it is available. This option can be used to cause Xen to fall back to
+older mechanisms, which are generally slower.
+
### noirqbalance
> `= <boolean>`
diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index e9c0e5e059..7ba0c3f0fe 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -5,6 +5,7 @@
#include <xen/mm.h>
#include <xen/stdbool.h>
#include <asm/flushtlb.h>
+#include <asm/invpcid.h>
#include <asm/io.h>
#include <asm/mtrr.h>
#include <asm/msr.h>
@@ -400,8 +401,10 @@ static DEFINE_SPINLOCK(set_atomicity_lock);
* has been called.
*/
-static void prepare_set(void)
+static bool prepare_set(void)
{
+ unsigned long cr4;
+
/* Note that this is not ideal, since the cache is only flushed/disabled
for this CPU while the MTRRs are changed, but changing this requires
more invasive changes to the way the kernel boots */
@@ -412,18 +415,24 @@ static void prepare_set(void)
write_cr0(read_cr0() | X86_CR0_CD);
wbinvd();
- /* TLB flushing here relies on Xen always using CR4.PGE. */
- BUILD_BUG_ON(!(XEN_MINIMAL_CR4 & X86_CR4_PGE));
- write_cr4(read_cr4() & ~X86_CR4_PGE);
+ cr4 = read_cr4();
+ if (cr4 & X86_CR4_PGE)
+ write_cr4(cr4 & ~X86_CR4_PGE);
+ else if (use_invpcid)
+ invpcid_flush_all();
+ else
+ write_cr3(read_cr3());
/* Save MTRR state */
rdmsrl(MSR_MTRRdefType, deftype);
/* Disable MTRRs, and set the default type to uncached */
mtrr_wrmsr(MSR_MTRRdefType, deftype & ~0xcff);
+
+ return cr4 & X86_CR4_PGE;
}
-static void post_set(void)
+static void post_set(bool pge)
{
/* Intel (P6) standard MTRRs */
mtrr_wrmsr(MSR_MTRRdefType, deftype);
@@ -432,7 +441,12 @@ static void post_set(void)
write_cr0(read_cr0() & ~X86_CR0_CD);
/* Reenable CR4.PGE (also flushes the TLB) */
- write_cr4(read_cr4() | X86_CR4_PGE);
+ if (pge)
+ write_cr4(read_cr4() | X86_CR4_PGE);
+ else if (use_invpcid)
+ invpcid_flush_all();
+ else
+ write_cr3(read_cr3());
spin_unlock(&set_atomicity_lock);
}
@@ -441,14 +455,15 @@ static void generic_set_all(void)
{
unsigned long mask, count;
unsigned long flags;
+ bool pge;
local_irq_save(flags);
- prepare_set();
+ pge = prepare_set();
/* Actually set the state */
mask = set_mtrr_state();
- post_set();
+ post_set(pge);
local_irq_restore(flags);
/* Use the atomic bitops to update the global mask */
@@ -457,7 +472,6 @@ static void generic_set_all(void)
set_bit(count, &smp_changes_mask);
mask >>= 1;
}
-
}
static void generic_set_mtrr(unsigned int reg, unsigned long base,
@@ -474,11 +488,12 @@ static void generic_set_mtrr(unsigned int reg, unsigned long base,
{
unsigned long flags;
struct mtrr_var_range *vr;
+ bool pge;
vr = &mtrr_state.var_ranges[reg];
local_irq_save(flags);
- prepare_set();
+ pge = prepare_set();
if (size == 0) {
/* The invalid bit is kept in the mask, so we simply clear the
@@ -499,7 +514,7 @@ static void generic_set_mtrr(unsigned int reg, unsigned long base,
mtrr_wrmsr(MSR_IA32_MTRR_PHYSMASK(reg), vr->mask);
}
- post_set();
+ post_set(pge);
local_irq_restore(flags);
}
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 1af9221607..b6817f1817 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -11,6 +11,7 @@
#include <xen/smp.h>
#include <xen/softirq.h>
#include <asm/flushtlb.h>
+#include <asm/invpcid.h>
#include <asm/page.h>
/* Debug builds: Wrap frequently to stress-test the wrap logic. */
@@ -72,6 +73,23 @@ static void post_flush(u32 t)
this_cpu(tlbflush_time) = t;
}
+static void do_tlb_flush(void)
+{
+ u32 t = pre_flush();
+
+ if ( use_invpcid )
+ invpcid_flush_all();
+ else
+ {
+ unsigned long cr4 = read_cr4();
+
+ write_cr4(cr4 ^ X86_CR4_PGE);
+ write_cr4(cr4);
+ }
+
+ post_flush(t);
+}
+
void switch_cr3(unsigned long cr3)
{
unsigned long flags, cr4;
@@ -119,16 +137,7 @@ unsigned int flush_area_local(const void *va, unsigned int flags)
: : "m" (*(const char *)(va)) : "memory" );
}
else
- {
- u32 t = pre_flush();
- unsigned long cr4 = read_cr4();
-
- write_cr4(cr4 & ~X86_CR4_PGE);
- barrier();
- write_cr4(cr4);
-
- post_flush(t);
- }
+ do_tlb_flush();
}
if ( flags & FLUSH_CACHE )
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index f803980b97..164c42cbf1 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -62,6 +62,11 @@ boolean_param("nosmp", opt_nosmp);
static unsigned int __initdata max_cpus;
integer_param("maxcpus", max_cpus);
+/* opt_invpcid: If false, don't use INVPCID instruction even if available. */
+static bool __initdata opt_invpcid = true;
+boolean_param("invpcid", opt_invpcid);
+bool __read_mostly use_invpcid;
+
unsigned long __read_mostly cr4_pv32_mask;
/* **** Linux config option: propagated to domain0. */
@@ -1546,6 +1551,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
if ( cpu_has_fsgsbase )
set_in_cr4(X86_CR4_FSGSBASE);
+ if ( opt_invpcid && cpu_has_invpcid )
+ use_invpcid = true;
+
init_speculation_mitigations();
init_idle_domain();
diff --git a/xen/include/asm-x86/invpcid.h b/xen/include/asm-x86/invpcid.h
index b46624a865..edd8b68706 100644
--- a/xen/include/asm-x86/invpcid.h
+++ b/xen/include/asm-x86/invpcid.h
@@ -3,6 +3,8 @@
#include <xen/types.h>
+extern bool use_invpcid;
+
#define INVPCID_TYPE_INDIV_ADDR 0
#define INVPCID_TYPE_SINGLE_CTXT 1
#define INVPCID_TYPE_ALL_INCL_GLOBAL 2
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 5/9] xen/x86: disable global pages for domains with XPTI active
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (3 preceding siblings ...)
2018-04-26 11:33 ` [PATCH v9 4/9] xen/x86: use invpcid for flushing the TLB Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-26 11:33 ` [PATCH v9 6/9] xen/x86: use flag byte for decision whether xen_cr3 is valid Juergen Gross
` (6 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
Instead of flushing the TLB from global pages when switching address
spaces with XPTI being active just disable global pages via %cr4
completely when a domain subject to XPTI is active. This avoids the
need for extra TLB flushes as loading %cr3 will remove all TLB
entries.
In order to avoid states with cr3/cr4 having inconsistent values
(e.g. global pages being activated while cr3 already specifies a XPTI
address space) move loading of the new cr4 value to write_ptbase()
(actually to switch_cr3_cr4() called by write_ptbase()).
This requires to use switch_cr3_cr4() instead of write_ptbase() when
building dom0 in order to avoid setting cr4 with cr4.smap set.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V7:
- use switch_cr3_cr4() in dom0_build.c
V6:
- don't call read_cr4() multiple times in switch_cr3_cr4()
(Andrew Cooper)
V4:
- don't use mmu_cr4_features for setting new cr4 value (Jan Beulich)
- use simpler scheme for setting X86_CR4_PGE in
pv_guest_cr4_to_real_cr4() (Jan Beulich)
V3:
- move cr4 loading for all domains from *_ctxt_switch_to() to
write_cr3_cr4() called by write_ptbase() (Jan Beulich)
- rebase
---
xen/arch/x86/domain.c | 5 -----
xen/arch/x86/flushtlb.c | 17 ++++++++++++-----
xen/arch/x86/mm.c | 14 +++++++++++---
xen/arch/x86/pv/dom0_build.c | 6 +++---
xen/arch/x86/x86_64/entry.S | 10 ----------
xen/common/efi/runtime.c | 4 ++--
xen/include/asm-x86/domain.h | 3 ++-
xen/include/asm-x86/flushtlb.h | 2 +-
8 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 3d9c19d055..9b001a03ec 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1523,17 +1523,12 @@ void paravirt_ctxt_switch_from(struct vcpu *v)
void paravirt_ctxt_switch_to(struct vcpu *v)
{
root_pgentry_t *root_pgt = this_cpu(root_pgt);
- unsigned long cr4;
if ( root_pgt )
root_pgt[root_table_offset(PERDOMAIN_VIRT_START)] =
l4e_from_page(v->domain->arch.perdomain_l3_pg,
__PAGE_HYPERVISOR_RW);
- cr4 = pv_guest_cr4_to_real_cr4(v);
- if ( unlikely(cr4 != read_cr4()) )
- write_cr4(cr4);
-
if ( unlikely(v->arch.debugreg[7] & DR7_ACTIVE_MASK) )
activate_debugregs(v);
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index b6817f1817..59884e7989 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -90,20 +90,27 @@ static void do_tlb_flush(void)
post_flush(t);
}
-void switch_cr3(unsigned long cr3)
+void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
{
- unsigned long flags, cr4;
+ unsigned long flags, old_cr4;
u32 t;
/* This non-reentrant function is sometimes called in interrupt context. */
local_irq_save(flags);
t = pre_flush();
- cr4 = read_cr4();
- write_cr4(cr4 & ~X86_CR4_PGE);
+ old_cr4 = read_cr4();
+ if ( old_cr4 & X86_CR4_PGE )
+ {
+ old_cr4 = cr4 & ~X86_CR4_PGE;
+ write_cr4(old_cr4);
+ }
+
write_cr3(cr3);
- write_cr4(cr4);
+
+ if ( old_cr4 != cr4 )
+ write_cr4(cr4);
post_flush(t);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b1e8133d92..13e13dfde7 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -503,20 +503,28 @@ void make_cr3(struct vcpu *v, mfn_t mfn)
void write_ptbase(struct vcpu *v)
{
struct cpu_info *cpu_info = get_cpu_info();
+ unsigned long new_cr4;
+
+ new_cr4 = (is_pv_vcpu(v) && !is_idle_vcpu(v))
+ ? pv_guest_cr4_to_real_cr4(v)
+ : ((read_cr4() & ~X86_CR4_TSD) | X86_CR4_PGE);
if ( is_pv_vcpu(v) && v->domain->arch.pv_domain.xpti )
{
cpu_info->root_pgt_changed = true;
cpu_info->pv_cr3 = __pa(this_cpu(root_pgt));
- switch_cr3(v->arch.cr3);
+ switch_cr3_cr4(v->arch.cr3, new_cr4);
}
else
{
- /* Make sure to clear xen_cr3 before pv_cr3; switch_cr3() serializes. */
+ /* Make sure to clear xen_cr3 before pv_cr3. */
cpu_info->xen_cr3 = 0;
- switch_cr3(v->arch.cr3);
+ /* switch_cr3_cr4() serializes. */
+ switch_cr3_cr4(v->arch.cr3, new_cr4);
cpu_info->pv_cr3 = 0;
}
+
+ ASSERT(is_pv_vcpu(v) || read_cr4() == mmu_cr4_features);
}
/*
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index d148395919..4465a059a8 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -717,7 +717,7 @@ int __init dom0_construct_pv(struct domain *d,
update_cr3(v);
/* We run on dom0's page tables for the final part of the build process. */
- write_ptbase(v);
+ switch_cr3_cr4(v->arch.cr3, read_cr4());
mapcache_override_current(v);
/* Copy the OS image and free temporary buffer. */
@@ -738,7 +738,7 @@ int __init dom0_construct_pv(struct domain *d,
(parms.virt_hypercall >= v_end) )
{
mapcache_override_current(NULL);
- write_ptbase(current);
+ switch_cr3_cr4(current->arch.cr3, read_cr4());
printk("Invalid HYPERCALL_PAGE field in ELF notes.\n");
rc = -1;
goto out;
@@ -871,7 +871,7 @@ int __init dom0_construct_pv(struct domain *d,
/* Return to idle domain's page tables. */
mapcache_override_current(NULL);
- write_ptbase(current);
+ switch_cr3_cr4(current->arch.cr3, read_cr4());
update_domain_wallclock_time(d);
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 732f834a09..197dc6e4de 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -153,13 +153,8 @@ restore_all_guest:
ROOT_PAGETABLE_LAST_XEN_SLOT - 1) * 8, %rdi
rep movsq
.Lrag_copy_done:
- mov STACK_CPUINFO_FIELD(cr4)(%rdx), %rdi
mov %r9, STACK_CPUINFO_FIELD(xen_cr3)(%rdx)
- mov %rdi, %rsi
- and $~X86_CR4_PGE, %rdi
- mov %rdi, %cr4
mov %rax, %cr3
- mov %rsi, %cr4
.Lrag_keep_cr3:
/* Restore stashed SPEC_CTRL value. */
@@ -215,12 +210,7 @@ restore_all_xen:
* so "g" will have to do.
*/
UNLIKELY_START(g, exit_cr3)
- mov %cr4, %rdi
- mov %rdi, %rsi
- and $~X86_CR4_PGE, %rdi
- mov %rdi, %cr4
mov %rax, %cr3
- mov %rsi, %cr4
UNLIKELY_END(exit_cr3)
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 4e5ddfef4f..070a70d784 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -111,7 +111,7 @@ struct efi_rs_state efi_rs_enter(void)
lgdt(&gdt_desc);
}
- switch_cr3(virt_to_maddr(efi_l4_pgtable));
+ switch_cr3_cr4(virt_to_maddr(efi_l4_pgtable), read_cr4());
return state;
}
@@ -120,7 +120,7 @@ void efi_rs_leave(struct efi_rs_state *state)
{
if ( !state->cr3 )
return;
- switch_cr3(state->cr3);
+ switch_cr3_cr4(state->cr3, read_cr4());
if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
{
struct desc_ptr gdt_desc = {
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index ed4199931a..b7894dc8c8 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -618,9 +618,10 @@ unsigned long pv_guest_cr4_fixup(const struct vcpu *, unsigned long guest_cr4);
#define pv_guest_cr4_to_real_cr4(v) \
(((v)->arch.pv_vcpu.ctrlreg[4] \
| (mmu_cr4_features \
- & (X86_CR4_PGE | X86_CR4_PSE | X86_CR4_SMEP | \
+ & (X86_CR4_PSE | X86_CR4_SMEP | \
X86_CR4_SMAP | X86_CR4_OSXSAVE | \
X86_CR4_FSGSBASE)) \
+ | ((v)->domain->arch.pv_domain.xpti ? 0 : X86_CR4_PGE) \
| ((v)->domain->arch.vtsc ? X86_CR4_TSD : 0)) \
& ~X86_CR4_DE)
#define real_cr4_to_pv_guest_cr4(c) \
diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm-x86/flushtlb.h
index fe48c89de7..ed5f45e806 100644
--- a/xen/include/asm-x86/flushtlb.h
+++ b/xen/include/asm-x86/flushtlb.h
@@ -84,7 +84,7 @@ static inline unsigned long read_cr3(void)
}
/* Write pagetable base and implicitly tick the tlbflush clock. */
-void switch_cr3(unsigned long cr3);
+void switch_cr3_cr4(unsigned long cr3, unsigned long cr4);
/* flush_* flag fields: */
/*
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 6/9] xen/x86: use flag byte for decision whether xen_cr3 is valid
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (4 preceding siblings ...)
2018-04-26 11:33 ` [PATCH v9 5/9] xen/x86: disable global pages for domains with XPTI active Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-26 11:33 ` [PATCH v9 7/9] xen/x86: convert pv_guest_cr4_to_real_cr4() to a function Juergen Gross
` (5 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
Today cpu_info->xen_cr3 is either 0 to indicate %cr3 doesn't need to
be switched on entry to Xen, or negative for keeping the value while
indicating not to restore %cr3, or positive in case %cr3 is to be
restored.
Switch to use a flag byte instead of a negative xen_cr3 value in order
to allow %cr3 values with the high bit set in case we want to keep TLB
entries when using the PCID feature.
This reduces the number of branches in interrupt handling and results
in better performance (e.g. parallel make of the Xen hypervisor on my
system was using about 3% less system time).
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V3:
- renamed use_xen_cr3 to better fitting use_pv_cr3
- corrected comment regarding semantics of use_pv_cr3 (Jan Beulich)
- prefer 32-bit operations over 8- or 16-bit ones (Jan Beulich)
---
xen/arch/x86/domain.c | 1 +
xen/arch/x86/mm.c | 3 +-
xen/arch/x86/smpboot.c | 2 ++
xen/arch/x86/x86_64/asm-offsets.c | 1 +
xen/arch/x86/x86_64/compat/entry.S | 5 ++-
xen/arch/x86/x86_64/entry.S | 63 ++++++++++++++++----------------------
xen/include/asm-x86/current.h | 12 +++++---
7 files changed, 43 insertions(+), 44 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 9b001a03ec..801ac33810 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1696,6 +1696,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
ASSERT(local_irq_is_enabled());
+ get_cpu_info()->use_pv_cr3 = false;
get_cpu_info()->xen_cr3 = 0;
if ( unlikely(dirty_cpu != cpu) && dirty_cpu != VCPU_CPU_CLEAN )
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 13e13dfde7..e93f686622 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -517,7 +517,8 @@ void write_ptbase(struct vcpu *v)
}
else
{
- /* Make sure to clear xen_cr3 before pv_cr3. */
+ /* Make sure to clear use_pv_cr3 and xen_cr3 before pv_cr3. */
+ cpu_info->use_pv_cr3 = false;
cpu_info->xen_cr3 = 0;
/* switch_cr3_cr4() serializes. */
switch_cr3_cr4(v->arch.cr3, new_cr4);
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 48aecdd17f..86fa4105be 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -336,6 +336,7 @@ void start_secondary(void *unused)
*/
spin_debug_disable();
+ get_cpu_info()->use_pv_cr3 = false;
get_cpu_info()->xen_cr3 = 0;
get_cpu_info()->pv_cr3 = 0;
@@ -1135,6 +1136,7 @@ void __init smp_prepare_boot_cpu(void)
per_cpu(scratch_cpumask, cpu) = &scratch_cpu0mask;
#endif
+ get_cpu_info()->use_pv_cr3 = false;
get_cpu_info()->xen_cr3 = 0;
get_cpu_info()->pv_cr3 = 0;
}
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index 9e2aefb00f..7ad024cf37 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -144,6 +144,7 @@ void __dummy__(void)
OFFSET(CPUINFO_use_shadow_spec_ctrl, struct cpu_info, use_shadow_spec_ctrl);
OFFSET(CPUINFO_bti_ist_info, struct cpu_info, bti_ist_info);
OFFSET(CPUINFO_root_pgt_changed, struct cpu_info, root_pgt_changed);
+ OFFSET(CPUINFO_use_pv_cr3, struct cpu_info, use_pv_cr3);
DEFINE(CPUINFO_sizeof, sizeof(struct cpu_info));
BLANK();
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index ae2bb4bf1e..b909977e33 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -210,10 +210,9 @@ ENTRY(cstar_enter)
GET_STACK_END(bx)
mov STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx
- neg %rcx
+ test %rcx, %rcx
jz .Lcstar_cr3_okay
- mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
- neg %rcx
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx)
mov %rcx, %cr3
movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
.Lcstar_cr3_okay:
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 197dc6e4de..7bcc544bce 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -154,6 +154,7 @@ restore_all_guest:
rep movsq
.Lrag_copy_done:
mov %r9, STACK_CPUINFO_FIELD(xen_cr3)(%rdx)
+ movb $1, STACK_CPUINFO_FIELD(use_pv_cr3)(%rdx)
mov %rax, %cr3
.Lrag_keep_cr3:
@@ -202,14 +203,9 @@ restore_all_xen:
* case we return to late PV exit code (from an NMI or #MC).
*/
GET_STACK_END(bx)
- mov STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rdx
+ cmpb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx)
+UNLIKELY_START(ne, exit_cr3)
mov STACK_CPUINFO_FIELD(pv_cr3)(%rbx), %rax
- test %rdx, %rdx
- /*
- * Ideally the condition would be "nsz", but such doesn't exist,
- * so "g" will have to do.
- */
-UNLIKELY_START(g, exit_cr3)
mov %rax, %cr3
UNLIKELY_END(exit_cr3)
@@ -251,10 +247,9 @@ ENTRY(lstar_enter)
GET_STACK_END(bx)
mov STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx
- neg %rcx
+ test %rcx, %rcx
jz .Llstar_cr3_okay
- mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
- neg %rcx
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx)
mov %rcx, %cr3
movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
.Llstar_cr3_okay:
@@ -288,10 +283,9 @@ GLOBAL(sysenter_eflags_saved)
/* 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
- neg %rcx
+ test %rcx, %rcx
jz .Lsyse_cr3_okay
- mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
- neg %rcx
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx)
mov %rcx, %cr3
movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
.Lsyse_cr3_okay:
@@ -338,10 +332,9 @@ ENTRY(int80_direct_trap)
GET_STACK_END(bx)
mov STACK_CPUINFO_FIELD(xen_cr3)(%rbx), %rcx
- neg %rcx
+ test %rcx, %rcx
jz .Lint80_cr3_okay
- mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
- neg %rcx
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%rbx)
mov %rcx, %cr3
movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%rbx)
.Lint80_cr3_okay:
@@ -552,24 +545,24 @@ ENTRY(common_interrupt)
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
mov STACK_CPUINFO_FIELD(xen_cr3)(%r14), %rcx
+ mov STACK_CPUINFO_FIELD(use_pv_cr3)(%r14), %bl
mov %rcx, %r15
- neg %rcx
+ test %rcx, %rcx
jz .Lintr_cr3_okay
- jns .Lintr_cr3_load
- mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
- neg %rcx
-.Lintr_cr3_load:
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
mov %rcx, %cr3
xor %ecx, %ecx
mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
testb $3, UREGS_cs(%rsp)
cmovnz %rcx, %r15
+ cmovnz %rcx, %rbx
.Lintr_cr3_okay:
CR4_PV32_RESTORE
movq %rsp,%rdi
callq do_IRQ
mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+ mov %bl, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
jmp ret_from_intr
ENTRY(page_fault)
@@ -584,18 +577,17 @@ GLOBAL(handle_exception)
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
mov STACK_CPUINFO_FIELD(xen_cr3)(%r14), %rcx
+ mov STACK_CPUINFO_FIELD(use_pv_cr3)(%r14), %r13b
mov %rcx, %r15
- neg %rcx
+ test %rcx, %rcx
jz .Lxcpt_cr3_okay
- jns .Lxcpt_cr3_load
- mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
- neg %rcx
-.Lxcpt_cr3_load:
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
mov %rcx, %cr3
xor %ecx, %ecx
mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
testb $3, UREGS_cs(%rsp)
cmovnz %rcx, %r15
+ cmovnz %rcx, %r13
.Lxcpt_cr3_okay:
handle_exception_saved:
@@ -650,6 +642,7 @@ handle_exception_saved:
mov (%rdx, %rax, 8), %rdx
INDIRECT_CALL %rdx
mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+ mov %r13b, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
testb $3,UREGS_cs(%rsp)
jz restore_all_xen
leaq VCPU_trap_bounce(%rbx),%rdx
@@ -683,6 +676,7 @@ exception_with_ints_disabled:
1: movq UREGS_error_code(%rsp),%rax # ec/ev
movq %rax,UREGS_kernel_sizeof(%rsp)
mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+ mov %r13b, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
jmp restore_all_xen # return to fixup code
/* No special register assumptions. */
@@ -768,12 +762,9 @@ ENTRY(double_fault)
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
mov STACK_CPUINFO_FIELD(xen_cr3)(%r14), %rbx
- neg %rbx
+ test %rbx, %rbx
jz .Ldblf_cr3_okay
- jns .Ldblf_cr3_load
- mov %rbx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
- neg %rbx
-.Ldblf_cr3_load:
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
mov %rbx, %cr3
movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
.Ldblf_cr3_okay:
@@ -803,13 +794,11 @@ handle_ist_exception:
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
mov STACK_CPUINFO_FIELD(xen_cr3)(%r14), %rcx
+ mov STACK_CPUINFO_FIELD(use_pv_cr3)(%r14), %bl
mov %rcx, %r15
- neg %rcx
+ test %rcx, %rcx
jz .List_cr3_okay
- jns .List_cr3_load
- mov %rcx, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
- neg %rcx
-.List_cr3_load:
+ movb $0, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
mov %rcx, %cr3
movq $0, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
.List_cr3_okay:
@@ -822,6 +811,7 @@ handle_ist_exception:
* and copy the context to stack bottom.
*/
xor %r15, %r15
+ xor %ebx, %ebx
GET_CPUINFO_FIELD(guest_cpu_user_regs,di)
movq %rsp,%rsi
movl $UREGS_kernel_sizeof/8,%ecx
@@ -833,6 +823,7 @@ handle_ist_exception:
mov (%rdx, %rax, 8), %rdx
INDIRECT_CALL %rdx
mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
+ mov %bl, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
cmpb $TRAP_nmi,UREGS_entry_vector(%rsp)
jne ret_from_intr
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index b2475783f8..43bdec1f49 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -47,10 +47,7 @@ struct cpu_info {
* context is being entered. A value of zero indicates no setting of CR3
* is to be performed.
* The former is the value to restore when re-entering Xen, if any. IOW
- * its value being zero means there's nothing to restore. However, its
- * value can also be negative, indicating to the exit-to-Xen code that
- * restoring is not necessary, but allowing any nested entry code paths
- * to still know the value to put back into CR3.
+ * its value being zero means there's nothing to restore.
*/
unsigned long xen_cr3;
unsigned long pv_cr3;
@@ -68,6 +65,13 @@ struct cpu_info {
*/
bool root_pgt_changed;
+ /*
+ * use_pv_cr3 is set in case the value of pv_cr3 is to be written into
+ * CR3 when returning from an interrupt. The main use is when returning
+ * from a NMI or MCE to hypervisor code where pv_cr3 was active.
+ */
+ bool use_pv_cr3;
+
unsigned long __pad;
/* get_stack_bottom() must be 16-byte aligned */
};
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 7/9] xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (5 preceding siblings ...)
2018-04-26 11:33 ` [PATCH v9 6/9] xen/x86: use flag byte for decision whether xen_cr3 is valid Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-26 11:33 ` [PATCH v9 8/9] xen/x86: add some cr3 helpers Juergen Gross
` (4 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
pv_guest_cr4_to_real_cr4() is becoming more and more complex. Convert
it from a macro to an ordinary function.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V6:
- new patch, split off from (old) patch 7 (Andrew Cooper)
---
xen/arch/x86/mm.c | 14 ++++++++++++++
xen/include/asm-x86/domain.h | 11 ++---------
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index e93f686622..4f878c8dd1 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -500,6 +500,20 @@ void make_cr3(struct vcpu *v, mfn_t mfn)
v->arch.cr3 = mfn_x(mfn) << PAGE_SHIFT;
}
+unsigned long pv_guest_cr4_to_real_cr4(const struct vcpu *v)
+{
+ const struct domain *d = v->domain;
+ unsigned long cr4;
+
+ cr4 = v->arch.pv_vcpu.ctrlreg[4] & ~X86_CR4_DE;
+ cr4 |= mmu_cr4_features & (X86_CR4_PSE | X86_CR4_SMEP | X86_CR4_SMAP |
+ X86_CR4_OSXSAVE | X86_CR4_FSGSBASE);
+ cr4 |= d->arch.pv_domain.xpti ? 0 : X86_CR4_PGE;
+ cr4 |= d->arch.vtsc ? X86_CR4_TSD : 0;
+
+ return cr4;
+}
+
void write_ptbase(struct vcpu *v)
{
struct cpu_info *cpu_info = get_cpu_info();
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index b7894dc8c8..9627058cd0 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -615,15 +615,8 @@ void vcpu_show_registers(const struct vcpu *);
unsigned long pv_guest_cr4_fixup(const struct vcpu *, unsigned long guest_cr4);
/* Convert between guest-visible and real CR4 values. */
-#define pv_guest_cr4_to_real_cr4(v) \
- (((v)->arch.pv_vcpu.ctrlreg[4] \
- | (mmu_cr4_features \
- & (X86_CR4_PSE | X86_CR4_SMEP | \
- X86_CR4_SMAP | X86_CR4_OSXSAVE | \
- X86_CR4_FSGSBASE)) \
- | ((v)->domain->arch.pv_domain.xpti ? 0 : X86_CR4_PGE) \
- | ((v)->domain->arch.vtsc ? X86_CR4_TSD : 0)) \
- & ~X86_CR4_DE)
+unsigned long pv_guest_cr4_to_real_cr4(const struct vcpu *v);
+
#define real_cr4_to_pv_guest_cr4(c) \
((c) & ~(X86_CR4_PGE | X86_CR4_PSE | X86_CR4_TSD | \
X86_CR4_OSXSAVE | X86_CR4_SMEP | \
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 8/9] xen/x86: add some cr3 helpers
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (6 preceding siblings ...)
2018-04-26 11:33 ` [PATCH v9 7/9] xen/x86: convert pv_guest_cr4_to_real_cr4() to a function Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-04-26 11:33 ` [PATCH v9 9/9] xen/x86: use PCID feature Juergen Gross
` (3 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
Add some helper macros to access the address and pcid parts of cr3.
Use those helpers where appropriate.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V6:
- new patch (Andrew Cooper)
---
xen/arch/x86/debug.c | 2 +-
xen/arch/x86/domain_page.c | 2 +-
xen/include/asm-x86/processor.h | 10 ++++++++++
xen/include/asm-x86/x86-defns.h | 4 +++-
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index 9159f32db4..a500df01ac 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -98,7 +98,7 @@ dbg_pv_va2mfn(dbgva_t vaddr, struct domain *dp, uint64_t pgd3val)
l2_pgentry_t l2e, *l2t;
l1_pgentry_t l1e, *l1t;
unsigned long cr3 = (pgd3val ? pgd3val : dp->vcpu[0]->arch.cr3);
- mfn_t mfn = maddr_to_mfn(cr3);
+ mfn_t mfn = maddr_to_mfn(cr3_pa(cr3));
DBGP2("vaddr:%lx domid:%d cr3:%lx pgd3:%lx\n", vaddr, dp->domain_id,
cr3, pgd3val);
diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
index 11b6a5421a..0c24530ed9 100644
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -51,7 +51,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
if ( (v = idle_vcpu[smp_processor_id()]) == current )
sync_local_execstate();
/* We must now be running on the idle page table. */
- ASSERT(read_cr3() == __pa(idle_pg_table));
+ ASSERT(cr3_pa(read_cr3()) == __pa(idle_pg_table));
}
return v;
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 71d32c0333..36628459dc 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -288,6 +288,16 @@ static inline void write_cr3(unsigned long val)
asm volatile ( "mov %0, %%cr3" : : "r" (val) : "memory" );
}
+static inline unsigned long cr3_pa(unsigned long cr3)
+{
+ return cr3 & X86_CR3_ADDR_MASK;
+}
+
+static inline unsigned long cr3_pcid(unsigned long cr3)
+{
+ return cr3 & X86_CR3_PCID_MASK;
+}
+
static inline unsigned long read_cr4(void)
{
return get_cpu_info()->cr4;
diff --git a/xen/include/asm-x86/x86-defns.h b/xen/include/asm-x86/x86-defns.h
index ff8d66be3c..904041e1ab 100644
--- a/xen/include/asm-x86/x86-defns.h
+++ b/xen/include/asm-x86/x86-defns.h
@@ -45,7 +45,9 @@
/*
* Intel CPU flags in CR3
*/
-#define X86_CR3_NOFLUSH (_AC(1, ULL) << 63)
+#define X86_CR3_NOFLUSH (_AC(1, ULL) << 63)
+#define X86_CR3_ADDR_MASK (PAGE_MASK & PADDR_MASK)
+#define X86_CR3_PCID_MASK _AC(0x0fff, ULL) /* Mask for PCID */
/*
* Intel CPU features in CR4
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v9 9/9] xen/x86: use PCID feature
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (7 preceding siblings ...)
2018-04-26 11:33 ` [PATCH v9 8/9] xen/x86: add some cr3 helpers Juergen Gross
@ 2018-04-26 11:33 ` Juergen Gross
2018-05-01 9:28 ` [PATCH v9 0/9] xen/x86: various XPTI speedups Andrew Cooper
` (2 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-04-26 11:33 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, andrew.cooper3, tim, jbeulich
Avoid flushing the complete TLB when switching %cr3 for mitigation of
Meltdown by using the PCID feature if available.
We are using 4 PCID values for a 64 bit pv domain subject to XPTI and
2 values for the non-XPTI case:
- guest active and in kernel mode
- guest active and in user mode
- hypervisor active and guest in user mode (XPTI only)
- hypervisor active and guest in kernel mode (XPTI only)
We use PCID only if PCID _and_ INVPCID are supported. With PCID in use
we disable global pages in cr4. A command line parameter controls in
which cases PCID is being used.
As the non-XPTI case has shown not to perform better with PCID at least
on some machines the default is to use PCID only for domains subject to
XPTI.
With PCID enabled we always disable global pages. This avoids having to
either flush the complete TLB or do a cycle through all PCID values
when invalidating a single global page.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V6.1:
- address some minor comments (Jan Beulich)
V6:
- split off pv_guest_cr4_to_real_cr4() conversion to function into new
patch (Andrew Cooper)
- changed some comments (Jan Beulich, Andrew Cooper)
V5:
- use X86_CR3_ADDR_MASK instead of ~X86_CR3_PCID_MASK (Jan Beulich)
- add some const qualifiers (Jan Beulich)
- mask X86_CR3_ADDR_MASK with PADDR_MASK (Jan Beulich)
- add flushing the TLB from old PCID related entries in write_cr3_cr4()
(Jan Beulich)
V4:
- add cr3 mask for page table address and use that in dbg_pv_va2mfn()
(Jan Beulich)
- use invpcid_flush_all_nonglobals() instead of invpcid_flush_all()
(Jan Beulich)
- use PCIDs 0/1 when running in Xen or without XPTI, 2/3 with XPTI in
guest (Jan Beulich)
- ASSERT cr4.pge and cr4.pcide are never active at the same time
(Jan Beulich)
- make pv_guest_cr4_to_real_cr4() a real function
V3:
- support PCID for non-XPTI case, too
- add command line parameter for controlling usage of PCID
- check PCID active by using cr4.pcide (Jan Beulich)
---
docs/misc/xen-command-line.markdown | 14 +++++++
xen/arch/x86/flushtlb.c | 47 ++++++++++++++++++++-
xen/arch/x86/mm.c | 16 +++++++-
xen/arch/x86/pv/dom0_build.c | 3 +-
xen/arch/x86/pv/domain.c | 81 ++++++++++++++++++++++++++++++++++++-
xen/include/asm-x86/domain.h | 4 +-
xen/include/asm-x86/processor.h | 3 ++
xen/include/asm-x86/pv/domain.h | 31 ++++++++++++++
8 files changed, 192 insertions(+), 7 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index f8264d8abb..e38230d09f 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1451,6 +1451,20 @@ All numbers specified must be hexadecimal ones.
This option can be specified more than once (up to 8 times at present).
+### pcid (x86)
+> `= <boolean> | xpti=<bool>`
+
+> Default: `xpti`
+
+> Can be modified at runtime (change takes effect only for domains created
+ afterwards)
+
+If available, control usage of the PCID feature of the processor for
+64-bit pv-domains. PCID can be used either for no domain at all (`false`),
+for all of them (`true`), only for those subject to XPTI (`xpti`) or for
+those not subject to XPTI (`no-xpti`). The feature is used only in case
+INVPCID is supported and not disabled via `invpcid=false`.
+
### ple\_gap
> `= <integer>`
diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 59884e7989..797c5d52cc 100644
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -13,6 +13,7 @@
#include <asm/flushtlb.h>
#include <asm/invpcid.h>
#include <asm/page.h>
+#include <asm/pv/domain.h>
/* Debug builds: Wrap frequently to stress-test the wrap logic. */
#ifdef NDEBUG
@@ -94,6 +95,7 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
{
unsigned long flags, old_cr4;
u32 t;
+ unsigned long old_pcid = cr3_pcid(read_cr3());
/* This non-reentrant function is sometimes called in interrupt context. */
local_irq_save(flags);
@@ -103,14 +105,34 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
old_cr4 = read_cr4();
if ( old_cr4 & X86_CR4_PGE )
{
+ /*
+ * X86_CR4_PGE set means PCID is inactive.
+ * We have to purge the TLB via flipping cr4.pge.
+ */
old_cr4 = cr4 & ~X86_CR4_PGE;
write_cr4(old_cr4);
}
+ else if ( use_invpcid )
+ /*
+ * Flushing the TLB via INVPCID is necessary only in case PCIDs are
+ * in use, which is true only with INVPCID being available.
+ * Without PCID usage the following write_cr3() will purge the TLB
+ * (we are in the cr4.pge off path) of all entries.
+ * Using invpcid_flush_all_nonglobals() seems to be faster than
+ * invpcid_flush_all(), so use that.
+ */
+ invpcid_flush_all_nonglobals();
write_cr3(cr3);
if ( old_cr4 != cr4 )
write_cr4(cr4);
+ else if ( old_pcid != cr3_pcid(cr3) )
+ /*
+ * Make sure no TLB entries related to the old PCID created between
+ * flushing the TLB and writing the new %cr3 value remain in the TLB.
+ */
+ invpcid_flush_single_context(old_pcid);
post_flush(t);
@@ -140,8 +162,29 @@ unsigned int flush_area_local(const void *va, unsigned int flags)
* are various errata surrounding INVLPG usage on superpages, and
* a full flush is in any case not *that* expensive.
*/
- asm volatile ( "invlpg %0"
- : : "m" (*(const char *)(va)) : "memory" );
+ if ( read_cr4() & X86_CR4_PCIDE )
+ {
+ unsigned long addr = (unsigned long)va;
+
+ /*
+ * Flush the addresses for all potential address spaces.
+ * We can't check the current domain for being subject to
+ * XPTI as current might be the idle vcpu while we still have
+ * some XPTI domain TLB entries.
+ * Using invpcid is okay here, as with PCID enabled we always
+ * have global pages disabled.
+ */
+ invpcid_flush_one(PCID_PV_PRIV, addr);
+ invpcid_flush_one(PCID_PV_USER, addr);
+ if ( !cpu_has_no_xpti )
+ {
+ invpcid_flush_one(PCID_PV_PRIV | PCID_PV_XPTI, addr);
+ invpcid_flush_one(PCID_PV_USER | PCID_PV_XPTI, addr);
+ }
+ }
+ else
+ asm volatile ( "invlpg %0"
+ : : "m" (*(const char *)(va)) : "memory" );
}
else
do_tlb_flush();
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 4f878c8dd1..f73f43edc9 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -126,6 +126,7 @@
#include <asm/hvm/ioreq.h>
#include <asm/hvm/grant_table.h>
+#include <asm/pv/domain.h>
#include <asm/pv/grant_table.h>
#include <asm/pv/mm.h>
@@ -497,7 +498,11 @@ void free_shared_domheap_page(struct page_info *page)
void make_cr3(struct vcpu *v, mfn_t mfn)
{
+ struct domain *d = v->domain;
+
v->arch.cr3 = mfn_x(mfn) << PAGE_SHIFT;
+ if ( is_pv_domain(d) && d->arch.pv_domain.pcid )
+ v->arch.cr3 |= get_pcid_bits(v, false);
}
unsigned long pv_guest_cr4_to_real_cr4(const struct vcpu *v)
@@ -508,7 +513,12 @@ unsigned long pv_guest_cr4_to_real_cr4(const struct vcpu *v)
cr4 = v->arch.pv_vcpu.ctrlreg[4] & ~X86_CR4_DE;
cr4 |= mmu_cr4_features & (X86_CR4_PSE | X86_CR4_SMEP | X86_CR4_SMAP |
X86_CR4_OSXSAVE | X86_CR4_FSGSBASE);
- cr4 |= d->arch.pv_domain.xpti ? 0 : X86_CR4_PGE;
+
+ if ( d->arch.pv_domain.pcid )
+ cr4 |= X86_CR4_PCIDE;
+ else if ( !d->arch.pv_domain.xpti )
+ cr4 |= X86_CR4_PGE;
+
cr4 |= d->arch.vtsc ? X86_CR4_TSD : 0;
return cr4;
@@ -521,12 +531,14 @@ void write_ptbase(struct vcpu *v)
new_cr4 = (is_pv_vcpu(v) && !is_idle_vcpu(v))
? pv_guest_cr4_to_real_cr4(v)
- : ((read_cr4() & ~X86_CR4_TSD) | X86_CR4_PGE);
+ : ((read_cr4() & ~(X86_CR4_PCIDE | X86_CR4_TSD)) | X86_CR4_PGE);
if ( is_pv_vcpu(v) && v->domain->arch.pv_domain.xpti )
{
cpu_info->root_pgt_changed = true;
cpu_info->pv_cr3 = __pa(this_cpu(root_pgt));
+ if ( new_cr4 & X86_CR4_PCIDE )
+ cpu_info->pv_cr3 |= get_pcid_bits(v, true);
switch_cr3_cr4(v->arch.cr3, new_cr4);
}
else
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 4465a059a8..34c77bcbe4 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -388,6 +388,7 @@ int __init dom0_construct_pv(struct domain *d,
{
d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
d->arch.pv_domain.xpti = false;
+ d->arch.pv_domain.pcid = false;
v->vcpu_info = (void *)&d->shared_info->compat.vcpu_info[0];
if ( setup_compat_arg_xlat(v) != 0 )
BUG();
@@ -717,7 +718,7 @@ int __init dom0_construct_pv(struct domain *d,
update_cr3(v);
/* We run on dom0's page tables for the final part of the build process. */
- switch_cr3_cr4(v->arch.cr3, read_cr4());
+ switch_cr3_cr4(cr3_pa(v->arch.cr3), read_cr4());
mapcache_override_current(v);
/* Copy the OS image and free temporary buffer. */
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index ce1a1a9d35..a4f0bd239d 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -9,9 +9,54 @@
#include <xen/lib.h>
#include <xen/sched.h>
+#include <asm/cpufeature.h>
+#include <asm/invpcid.h>
#include <asm/spec_ctrl.h>
#include <asm/pv/domain.h>
+static __read_mostly enum {
+ PCID_OFF,
+ PCID_ALL,
+ PCID_XPTI,
+ PCID_NOXPTI
+} opt_pcid = PCID_XPTI;
+
+static __init int parse_pcid(const char *s)
+{
+ int rc = 0;
+
+ switch ( parse_bool(s, NULL) )
+ {
+ case 0:
+ opt_pcid = PCID_OFF;
+ break;
+
+ case 1:
+ opt_pcid = PCID_ALL;
+ break;
+
+ default:
+ switch ( parse_boolean("xpti", s, NULL) )
+ {
+ case 0:
+ opt_pcid = PCID_NOXPTI;
+ break;
+
+ case 1:
+ opt_pcid = PCID_XPTI;
+ break;
+
+ default:
+ rc = -EINVAL;
+ break;
+ }
+ break;
+ }
+
+ return rc;
+}
+custom_runtime_param("pcid", parse_pcid);
+
static void noreturn continue_nonidle_domain(struct vcpu *v)
{
check_wakeup_from_wait();
@@ -77,6 +122,7 @@ int switch_compat(struct domain *d)
d->arch.x87_fip_width = 4;
d->arch.pv_domain.xpti = false;
+ d->arch.pv_domain.pcid = false;
return 0;
@@ -211,6 +257,29 @@ int pv_domain_initialise(struct domain *d)
d->arch.pv_domain.xpti = opt_xpti & (is_hardware_domain(d)
? OPT_XPTI_DOM0 : OPT_XPTI_DOMU);
+ if ( !is_pv_32bit_domain(d) && use_invpcid && cpu_has_pcid )
+ switch ( opt_pcid )
+ {
+ case PCID_OFF:
+ break;
+
+ case PCID_ALL:
+ d->arch.pv_domain.pcid = true;
+ break;
+
+ case PCID_XPTI:
+ d->arch.pv_domain.pcid = d->arch.pv_domain.xpti;
+ break;
+
+ case PCID_NOXPTI:
+ d->arch.pv_domain.pcid = !d->arch.pv_domain.xpti;
+ break;
+
+ default:
+ ASSERT_UNREACHABLE();
+ break;
+ }
+
return 0;
fail:
@@ -221,9 +290,19 @@ int pv_domain_initialise(struct domain *d)
static void _toggle_guest_pt(struct vcpu *v)
{
+ const struct domain *d = v->domain;
+
v->arch.flags ^= TF_kernel_mode;
update_cr3(v);
- get_cpu_info()->root_pgt_changed = true;
+ if ( d->arch.pv_domain.xpti )
+ {
+ struct cpu_info *cpu_info = get_cpu_info();
+
+ cpu_info->root_pgt_changed = true;
+ cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) |
+ (d->arch.pv_domain.pcid
+ ? get_pcid_bits(v, true) : 0);
+ }
/* Don't flush user global mappings from the TLB. Don't tick TLB clock. */
write_cr3(v->arch.cr3);
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 9627058cd0..8b66096e7f 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -255,6 +255,8 @@ struct pv_domain
/* XPTI active? */
bool xpti;
+ /* Use PCID feature? */
+ bool pcid;
/* map_domain_page() mapping cache. */
struct mapcache_domain mapcache;
@@ -620,7 +622,7 @@ unsigned long pv_guest_cr4_to_real_cr4(const struct vcpu *v);
#define real_cr4_to_pv_guest_cr4(c) \
((c) & ~(X86_CR4_PGE | X86_CR4_PSE | X86_CR4_TSD | \
X86_CR4_OSXSAVE | X86_CR4_SMEP | \
- X86_CR4_FSGSBASE | X86_CR4_SMAP))
+ X86_CR4_FSGSBASE | X86_CR4_SMAP | X86_CR4_PCIDE))
#define domain_max_vcpus(d) (is_hvm_domain(d) ? HVM_MAX_VCPUS : MAX_VIRT_CPUS)
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 36628459dc..c4aa385a6f 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -305,6 +305,9 @@ static inline unsigned long read_cr4(void)
static inline void write_cr4(unsigned long val)
{
+ /* No global pages in case of PCIDs enabled! */
+ ASSERT(!(val & X86_CR4_PGE) || !(val & X86_CR4_PCIDE));
+
get_cpu_info()->cr4 = val;
asm volatile ( "mov %0,%%cr4" : : "r" (val) );
}
diff --git a/xen/include/asm-x86/pv/domain.h b/xen/include/asm-x86/pv/domain.h
index 5e34176939..4fea76444a 100644
--- a/xen/include/asm-x86/pv/domain.h
+++ b/xen/include/asm-x86/pv/domain.h
@@ -21,6 +21,37 @@
#ifndef __X86_PV_DOMAIN_H__
#define __X86_PV_DOMAIN_H__
+/*
+ * PCID values for the address spaces of 64-bit pv domains:
+ *
+ * We are using 4 PCID values for a 64 bit pv domain subject to XPTI:
+ * - hypervisor active and guest in kernel mode PCID 0
+ * - hypervisor active and guest in user mode PCID 1
+ * - guest active and in kernel mode PCID 2
+ * - guest active and in user mode PCID 3
+ *
+ * Without XPTI only 2 values are used:
+ * - guest in kernel mode PCID 0
+ * - guest in user mode PCID 1
+ */
+
+#define PCID_PV_PRIV 0x0000 /* Used for other domains, too. */
+#define PCID_PV_USER 0x0001
+#define PCID_PV_XPTI 0x0002 /* To be ORed to above values. */
+
+/*
+ * Return additional PCID specific cr3 bits.
+ *
+ * Note that X86_CR3_NOFLUSH will not be readable in cr3. Anyone consuming
+ * v->arch.cr3 should mask away X86_CR3_NOFLUSH and X86_CR3_PCIDMASK in case
+ * the value is used to address the root page table.
+ */
+static inline unsigned long get_pcid_bits(const struct vcpu *v, bool is_xpti)
+{
+ return X86_CR3_NOFLUSH | (is_xpti ? PCID_PV_XPTI : 0) |
+ ((v->arch.flags & TF_kernel_mode) ? PCID_PV_PRIV : PCID_PV_USER);
+}
+
#ifdef CONFIG_PV
void pv_vcpu_destroy(struct vcpu *v);
--
2.13.6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v9 1/9] x86/xpti: avoid copying L4 page table contents when possible
2018-04-26 11:33 ` [PATCH v9 1/9] x86/xpti: avoid copying L4 page table contents when possible Juergen Gross
@ 2018-04-26 14:01 ` Tim Deegan
0 siblings, 0 replies; 26+ messages in thread
From: Tim Deegan @ 2018-04-26 14:01 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, jbeulich, andrew.cooper3
Hi,
At 13:33 +0200 on 26 Apr (1524749590), Juergen Gross wrote:
> For mitigation of Meltdown the current L4 page table is copied to the
> cpu local root page table each time a 64 bit pv guest is entered.
>
> Copying can be avoided in cases where the guest L4 page table hasn't
> been modified while running the hypervisor, e.g. when handling
> interrupts or any hypercall not modifying the L4 page table or %cr3.
>
> So add a per-cpu flag indicating whether the copying should be
> performed and set that flag only when loading a new %cr3 or modifying
> the L4 page table. This includes synchronization of the cpu local
> root page table with other cpus, so add a special synchronization flag
> for that case.
>
> A simple performance check (compiling the hypervisor via "make -j 4")
> in dom0 with 4 vcpus shows a significant improvement:
>
> - real time drops from 112 seconds to 103 seconds
> - system time drops from 142 seconds to 131 seconds
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Tim Deegan <tim@xen.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 3/9] xen/x86: support per-domain flag for xpti
2018-04-26 11:33 ` [PATCH v9 3/9] xen/x86: support per-domain flag for xpti Juergen Gross
@ 2018-04-27 7:55 ` Sergey Dyasli
2018-04-27 7:59 ` Juergen Gross
0 siblings, 1 reply; 26+ messages in thread
From: Sergey Dyasli @ 2018-04-27 7:55 UTC (permalink / raw)
To: jgross, xen-devel; +Cc: Sergey Dyasli, Tim (Xen.org), jbeulich, Andrew Cooper
On Thu, 2018-04-26 at 13:33 +0200, Juergen Gross wrote:
> Instead of switching XPTI globally on or off add a per-domain flag for
> that purpose. This allows to modify the xpti boot parameter to support
> running dom0 without Meltdown mitigations. Using "xpti=no-dom0" as boot
"xpti=dom0=0"
> parameter will achieve that.
>
> Move the xpti boot parameter handling to xen/arch/x86/pv/domain.c as
> it is pv-domain specific.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---
> V9:
> - adjust boot message (Sergey Dyasli)
> - adjust boot parameter documentation (Sergey Dyasli)
>
> V6.1:
> - address some minor comments (Jan Beulich)
>
> V6:
> - modify xpti boot parameter options (Andrew Cooper)
> - move xpti_init() code to spec_ctrl.c (Andrew Cooper)
> - irework init of per-domain xpti flag (Andrew Cooper)
>
> V3:
> - latch get_cpu_info() return value in variable (Jan Beulich)
> - call always xpti_domain_init() for pv dom0 (Jan Beulich)
> - add __init annotations (Jan Beulich)
> - drop per domain XPTI message (Jan Beulich)
> - document xpti=default support (Jan Beulich)
> - move domain xpti flag into a padding hole (Jan Beulich)
> ---
> docs/misc/xen-command-line.markdown | 14 ++++++-
> xen/arch/x86/mm.c | 17 ++++++++-
> xen/arch/x86/pv/dom0_build.c | 1 +
> xen/arch/x86/pv/domain.c | 6 +++
> xen/arch/x86/setup.c | 19 ----------
> xen/arch/x86/smpboot.c | 4 +-
> xen/arch/x86/spec_ctrl.c | 75 ++++++++++++++++++++++++++++++++++++-
> xen/include/asm-x86/current.h | 3 +-
> xen/include/asm-x86/domain.h | 3 ++
> xen/include/asm-x86/flushtlb.h | 2 +-
> xen/include/asm-x86/spec_ctrl.h | 4 ++
> 11 files changed, 119 insertions(+), 29 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index b353352adf..220d1ba020 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1955,14 +1955,24 @@ clustered mode. The default, given no hint from the **FADT**, is cluster
> mode.
>
> ### xpti
> -> `= <boolean>`
> +> `= List of [ default | <boolean> | dom0=<bool> | domu=<bool> ]`
>
> -> Default: `false` on AMD hardware
> +> Default: `false` on hardware not to be vulnerable to Meltdown (e.g. AMD)
^
known
> > Default: `true` everywhere else
>
> Override default selection of whether to isolate 64-bit PV guest page
> tables.
>
> +`true` activates page table isolation even on hardware not vulnerable by
> +Meltdown for all domains.
> +
> +`false` deactivates page table isolation on all systems for all domains.
> +
> +`default` sets the default behaviour.
> +
> +With `dom0` and `domu` it is possible to control page table isolation
> +for dom0 or guest domains only.
> +
> ### xsave
> > `= <boolean>`
--
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 3/9] xen/x86: support per-domain flag for xpti
2018-04-27 7:55 ` Sergey Dyasli
@ 2018-04-27 7:59 ` Juergen Gross
2018-04-27 8:15 ` Jan Beulich
0 siblings, 1 reply; 26+ messages in thread
From: Juergen Gross @ 2018-04-27 7:59 UTC (permalink / raw)
To: Sergey Dyasli, xen-devel; +Cc: Andrew Cooper, Tim (Xen.org), jbeulich
On 27/04/18 09:55, Sergey Dyasli wrote:
> On Thu, 2018-04-26 at 13:33 +0200, Juergen Gross wrote:
>> Instead of switching XPTI globally on or off add a per-domain flag for
>> that purpose. This allows to modify the xpti boot parameter to support
>> running dom0 without Meltdown mitigations. Using "xpti=no-dom0" as boot
>
> "xpti=dom0=0"
Both variants are valid.
>
>> parameter will achieve that.
>>
>> Move the xpti boot parameter handling to xen/arch/x86/pv/domain.c as
>> it is pv-domain specific.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> V9:
>> - adjust boot message (Sergey Dyasli)
>> - adjust boot parameter documentation (Sergey Dyasli)
>>
>> V6.1:
>> - address some minor comments (Jan Beulich)
>>
>> V6:
>> - modify xpti boot parameter options (Andrew Cooper)
>> - move xpti_init() code to spec_ctrl.c (Andrew Cooper)
>> - irework init of per-domain xpti flag (Andrew Cooper)
>>
>> V3:
>> - latch get_cpu_info() return value in variable (Jan Beulich)
>> - call always xpti_domain_init() for pv dom0 (Jan Beulich)
>> - add __init annotations (Jan Beulich)
>> - drop per domain XPTI message (Jan Beulich)
>> - document xpti=default support (Jan Beulich)
>> - move domain xpti flag into a padding hole (Jan Beulich)
>> ---
>> docs/misc/xen-command-line.markdown | 14 ++++++-
>> xen/arch/x86/mm.c | 17 ++++++++-
>> xen/arch/x86/pv/dom0_build.c | 1 +
>> xen/arch/x86/pv/domain.c | 6 +++
>> xen/arch/x86/setup.c | 19 ----------
>> xen/arch/x86/smpboot.c | 4 +-
>> xen/arch/x86/spec_ctrl.c | 75 ++++++++++++++++++++++++++++++++++++-
>> xen/include/asm-x86/current.h | 3 +-
>> xen/include/asm-x86/domain.h | 3 ++
>> xen/include/asm-x86/flushtlb.h | 2 +-
>> xen/include/asm-x86/spec_ctrl.h | 4 ++
>> 11 files changed, 119 insertions(+), 29 deletions(-)
>>
>> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
>> index b353352adf..220d1ba020 100644
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -1955,14 +1955,24 @@ clustered mode. The default, given no hint from the **FADT**, is cluster
>> mode.
>>
>> ### xpti
>> -> `= <boolean>`
>> +> `= List of [ default | <boolean> | dom0=<bool> | domu=<bool> ]`
>>
>> -> Default: `false` on AMD hardware
>> +> Default: `false` on hardware not to be vulnerable to Meltdown (e.g. AMD)
> ^
> known
Yes, indeed.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 3/9] xen/x86: support per-domain flag for xpti
2018-04-27 7:59 ` Juergen Gross
@ 2018-04-27 8:15 ` Jan Beulich
2018-05-04 15:06 ` Wei Liu
0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2018-04-27 8:15 UTC (permalink / raw)
To: Sergey Dyasli, Juergen Gross; +Cc: Andrew Cooper, Tim Deegan, xen-devel
>>> On 27.04.18 at 09:59, <jgross@suse.com> wrote:
> On 27/04/18 09:55, Sergey Dyasli wrote:
>> On Thu, 2018-04-26 at 13:33 +0200, Juergen Gross wrote:
>>> index b353352adf..220d1ba020 100644
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -1955,14 +1955,24 @@ clustered mode. The default, given no hint from the **FADT**, is cluster
>>> mode.
>>>
>>> ### xpti
>>> -> `= <boolean>`
>>> +> `= List of [ default | <boolean> | dom0=<bool> | domu=<bool> ]`
>>>
>>> -> Default: `false` on AMD hardware
>>> +> Default: `false` on hardware not to be vulnerable to Meltdown (e.g. AMD)
>> ^
>> known
>
> Yes, indeed.
Recorded for eventual application of the series; no need to resend just for this.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 0/9] xen/x86: various XPTI speedups
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (8 preceding siblings ...)
2018-04-26 11:33 ` [PATCH v9 9/9] xen/x86: use PCID feature Juergen Gross
@ 2018-05-01 9:28 ` Andrew Cooper
2018-05-02 10:38 ` Juergen Gross
2018-05-04 14:59 ` Wei Liu
2018-05-16 9:06 ` backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups) Jan Beulich
11 siblings, 1 reply; 26+ messages in thread
From: Andrew Cooper @ 2018-05-01 9:28 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: tim, jbeulich
On 26/04/18 12:33, Juergen Gross wrote:
> This patch series aims at reducing the overhead of the XPTI Meltdown
> mitigation.
With just the first 3 patches of this series (in a bisection attempt),
on a XenServer build based off staging, XenRT finds the following:
(XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
(XEN) ----[ Xen-4.11.0-6.0.0-d x86_64 debug=y Not tainted ]----
(XEN) CPU: 0
(XEN) RIP: e008:[<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
(XEN) RFLAGS: 0000000000010286 CONTEXT: hypervisor (d33v0)
(XEN) rax: ffff82e01307ade8 rbx: 000000000007ffff rcx: 8180000000000000
(XEN) rdx: 0000000000000000 rsi: 00000000000001b5 rdi: 0000000000000000
(XEN) rbp: ffff8300952b7ba8 rsp: ffff8300952b7b18 r8: 8000000000000000
(XEN) r9: ffff82e01307ade8 r10: 0180000000000000 r11: 7fffffffffffffff
(XEN) r12: 0000000000000000 r13: 00000000024c2e83 r14: 0000000000000000
(XEN) r15: ffff82e01307add8 cr0: 0000000080050033 cr4: 00000000001526e0
(XEN) cr3: 0000000799c41000 cr2: 00007fdaf5539000
(XEN) fsb: 0000000000000000 gsb: 0000000000000000 gss: 0000000000000000
(XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008
(XEN) Xen code around <ffff82d080229914> (page_alloc.c#alloc_heap_pages+0x371/0x6f2):
(XEN) ff 0f 0b 48 85 c9 79 31 <0f> 0b 48 c7 42 08 00 00 00 00 c7 42 10 00 00 00
(XEN) Xen stack trace from rsp=ffff8300952b7b18:
(XEN) 0000000000000001 ffff830799cdd000 0000000000000000 00000000003037e9
(XEN) 0000000100000004 ffff8300952b7b68 0000000100000000 ffff830095738000
(XEN) ffff8300952b7be8 000000008033bfe8 ffff82e01295e540 0000000000001adc
(XEN) ffff830756971770 0000000000000028 0000000000000000 ffff830799cdd000
(XEN) 0000000000000000 ffff830799cdd000 ffff8300952b7be8 ffff82d080229d4c
(XEN) 0000000000000000 ffff8300952b7d40 0000000000000000 0000000000000000
(XEN) 00000000000000a8 ffff830799cdd000 ffff8300952b7c98 ffff82d080221d90
(XEN) 0000000100000000 ffff830799cdd000 0000000000000000 0000000099cdd000
(XEN) ffff82e009cd0fd8 00000000000e7b1f ffff8300952b7c88 0000000000000020
(XEN) ffff8800e7b1fdd8 0000000000000002 0000000000000006 ffff830799cdd000
(XEN) ffff8300952b7c78 000000000039f480 0000000000000000 000000000000008d
(XEN) ffff8800e7b1fdd8 ffff830799cdd000 0000000000000006 ffff830799cdd000
(XEN) ffff8300952b7db8 ffff82d080223ad7 0000000000000046 ffff830088ff9000
(XEN) ffff8300952b7d18 ffff82d08023cfaf ffff82c000230118 ffff830842ceeb8c
(XEN) ffff82e009f54db8 00000000003bc78b ffff830842cd2770 ffff830088ff9000
(XEN) 0000000000000000 0000000000000000 ffff83085d6b9350 0000000000000000
(XEN) ffff8300952b7d28 ffff82d08023d766 ffff8300952b7d58 ffff82d08020c9a2
(XEN) ffff830842cee000 ffff830799cdd000 ffffffff81adbec0 0000000000000200
(XEN) 0000008d00000000 ffff82d000000000 ffffffff81adbec0 0000000000000200
(XEN) 0000000000000000 0000000000007ff0 ffff83085d6b9350 0000000000000006
(XEN) Xen call trace:
(XEN) [<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
(XEN) [<ffff82d080229d4c>] alloc_domheap_pages+0xb7/0x157
(XEN) [<ffff82d080221d90>] memory.c#populate_physmap+0x27e/0x4c9
(XEN) [<ffff82d080223ad7>] do_memory_op+0x2e2/0x2695
(XEN) [<ffff82d080308be9>] hypercall.c#hvm_memory_op+0x36/0x60
(XEN) [<ffff82d0803091c2>] hvm_hypercall+0x5af/0x681
(XEN) [<ffff82d08032fee6>] vmx_vmexit_handler+0x1040/0x1e14
(XEN) [<ffff82d080335f88>] vmx_asm_vmexit_handler+0xe8/0x250
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
(XEN) ****************************************
Running repeated tests on adjacent builds, we never see the assertion
failure without the patches (6 runs), and have so far seen for 3 of 4
runs (2 still pending) with the patches.
What is rather strange is that there is a lot of migration and
ballooning going on, but only for HVM (Debian Jessie, not that this
should matter) VMs. dom0 will be the only PV domain in the system, and
is 64bit.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 0/9] xen/x86: various XPTI speedups
2018-05-01 9:28 ` [PATCH v9 0/9] xen/x86: various XPTI speedups Andrew Cooper
@ 2018-05-02 10:38 ` Juergen Gross
2018-05-03 17:41 ` Andrew Cooper
0 siblings, 1 reply; 26+ messages in thread
From: Juergen Gross @ 2018-05-02 10:38 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: tim, jbeulich
On 01/05/18 11:28, Andrew Cooper wrote:
> On 26/04/18 12:33, Juergen Gross wrote:
>> This patch series aims at reducing the overhead of the XPTI Meltdown
>> mitigation.
>
> With just the first 3 patches of this series (in a bisection attempt),
> on a XenServer build based off staging, XenRT finds the following:
>
> (XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
> (XEN) ----[ Xen-4.11.0-6.0.0-d x86_64 debug=y Not tainted ]----
> (XEN) CPU: 0
> (XEN) RIP: e008:[<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
> (XEN) RFLAGS: 0000000000010286 CONTEXT: hypervisor (d33v0)
> (XEN) rax: ffff82e01307ade8 rbx: 000000000007ffff rcx: 8180000000000000
> (XEN) rdx: 0000000000000000 rsi: 00000000000001b5 rdi: 0000000000000000
> (XEN) rbp: ffff8300952b7ba8 rsp: ffff8300952b7b18 r8: 8000000000000000
> (XEN) r9: ffff82e01307ade8 r10: 0180000000000000 r11: 7fffffffffffffff
> (XEN) r12: 0000000000000000 r13: 00000000024c2e83 r14: 0000000000000000
> (XEN) r15: ffff82e01307add8 cr0: 0000000080050033 cr4: 00000000001526e0
> (XEN) cr3: 0000000799c41000 cr2: 00007fdaf5539000
> (XEN) fsb: 0000000000000000 gsb: 0000000000000000 gss: 0000000000000000
> (XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008
> (XEN) Xen code around <ffff82d080229914> (page_alloc.c#alloc_heap_pages+0x371/0x6f2):
> (XEN) ff 0f 0b 48 85 c9 79 31 <0f> 0b 48 c7 42 08 00 00 00 00 c7 42 10 00 00 00
> (XEN) Xen stack trace from rsp=ffff8300952b7b18:
> (XEN) 0000000000000001 ffff830799cdd000 0000000000000000 00000000003037e9
> (XEN) 0000000100000004 ffff8300952b7b68 0000000100000000 ffff830095738000
> (XEN) ffff8300952b7be8 000000008033bfe8 ffff82e01295e540 0000000000001adc
> (XEN) ffff830756971770 0000000000000028 0000000000000000 ffff830799cdd000
> (XEN) 0000000000000000 ffff830799cdd000 ffff8300952b7be8 ffff82d080229d4c
> (XEN) 0000000000000000 ffff8300952b7d40 0000000000000000 0000000000000000
> (XEN) 00000000000000a8 ffff830799cdd000 ffff8300952b7c98 ffff82d080221d90
> (XEN) 0000000100000000 ffff830799cdd000 0000000000000000 0000000099cdd000
> (XEN) ffff82e009cd0fd8 00000000000e7b1f ffff8300952b7c88 0000000000000020
> (XEN) ffff8800e7b1fdd8 0000000000000002 0000000000000006 ffff830799cdd000
> (XEN) ffff8300952b7c78 000000000039f480 0000000000000000 000000000000008d
> (XEN) ffff8800e7b1fdd8 ffff830799cdd000 0000000000000006 ffff830799cdd000
> (XEN) ffff8300952b7db8 ffff82d080223ad7 0000000000000046 ffff830088ff9000
> (XEN) ffff8300952b7d18 ffff82d08023cfaf ffff82c000230118 ffff830842ceeb8c
> (XEN) ffff82e009f54db8 00000000003bc78b ffff830842cd2770 ffff830088ff9000
> (XEN) 0000000000000000 0000000000000000 ffff83085d6b9350 0000000000000000
> (XEN) ffff8300952b7d28 ffff82d08023d766 ffff8300952b7d58 ffff82d08020c9a2
> (XEN) ffff830842cee000 ffff830799cdd000 ffffffff81adbec0 0000000000000200
> (XEN) 0000008d00000000 ffff82d000000000 ffffffff81adbec0 0000000000000200
> (XEN) 0000000000000000 0000000000007ff0 ffff83085d6b9350 0000000000000006
> (XEN) Xen call trace:
> (XEN) [<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
> (XEN) [<ffff82d080229d4c>] alloc_domheap_pages+0xb7/0x157
> (XEN) [<ffff82d080221d90>] memory.c#populate_physmap+0x27e/0x4c9
> (XEN) [<ffff82d080223ad7>] do_memory_op+0x2e2/0x2695
> (XEN) [<ffff82d080308be9>] hypercall.c#hvm_memory_op+0x36/0x60
> (XEN) [<ffff82d0803091c2>] hvm_hypercall+0x5af/0x681
> (XEN) [<ffff82d08032fee6>] vmx_vmexit_handler+0x1040/0x1e14
> (XEN) [<ffff82d080335f88>] vmx_asm_vmexit_handler+0xe8/0x250
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
> (XEN) ****************************************
>
> Running repeated tests on adjacent builds, we never see the assertion
> failure without the patches (6 runs), and have so far seen for 3 of 4
> runs (2 still pending) with the patches.
>
> What is rather strange is that there is a lot of migration and
> ballooning going on, but only for HVM (Debian Jessie, not that this
> should matter) VMs. dom0 will be the only PV domain in the system, and
> is 64bit.
Are you sure you have no other patches compared to staging in your
hypervisor? I can't imagine how one of the 3 patches could cause that
behavior.
I've tried to do similar testing on my machine: 2 HVM domains + 64-bit
Pv dom0. dom0 and one HVM domain are ballooned up and down all the time
while the other HVM domain is being migrated (localhost) in a loop.
Migration count is at 600 already...
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 0/9] xen/x86: various XPTI speedups
2018-05-02 10:38 ` Juergen Gross
@ 2018-05-03 17:41 ` Andrew Cooper
2018-05-03 18:41 ` Juergen Gross
0 siblings, 1 reply; 26+ messages in thread
From: Andrew Cooper @ 2018-05-03 17:41 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: tim, jbeulich
On 02/05/18 11:38, Juergen Gross wrote:
> On 01/05/18 11:28, Andrew Cooper wrote:
>> On 26/04/18 12:33, Juergen Gross wrote:
>>> This patch series aims at reducing the overhead of the XPTI Meltdown
>>> mitigation.
>> With just the first 3 patches of this series (in a bisection attempt),
>> on a XenServer build based off staging, XenRT finds the following:
>>
>> (XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
>> (XEN) ----[ Xen-4.11.0-6.0.0-d x86_64 debug=y Not tainted ]----
>> (XEN) CPU: 0
>> (XEN) RIP: e008:[<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
>> (XEN) RFLAGS: 0000000000010286 CONTEXT: hypervisor (d33v0)
>> (XEN) rax: ffff82e01307ade8 rbx: 000000000007ffff rcx: 8180000000000000
>> (XEN) rdx: 0000000000000000 rsi: 00000000000001b5 rdi: 0000000000000000
>> (XEN) rbp: ffff8300952b7ba8 rsp: ffff8300952b7b18 r8: 8000000000000000
>> (XEN) r9: ffff82e01307ade8 r10: 0180000000000000 r11: 7fffffffffffffff
>> (XEN) r12: 0000000000000000 r13: 00000000024c2e83 r14: 0000000000000000
>> (XEN) r15: ffff82e01307add8 cr0: 0000000080050033 cr4: 00000000001526e0
>> (XEN) cr3: 0000000799c41000 cr2: 00007fdaf5539000
>> (XEN) fsb: 0000000000000000 gsb: 0000000000000000 gss: 0000000000000000
>> (XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008
>> (XEN) Xen code around <ffff82d080229914> (page_alloc.c#alloc_heap_pages+0x371/0x6f2):
>> (XEN) ff 0f 0b 48 85 c9 79 31 <0f> 0b 48 c7 42 08 00 00 00 00 c7 42 10 00 00 00
>> (XEN) Xen stack trace from rsp=ffff8300952b7b18:
>> (XEN) 0000000000000001 ffff830799cdd000 0000000000000000 00000000003037e9
>> (XEN) 0000000100000004 ffff8300952b7b68 0000000100000000 ffff830095738000
>> (XEN) ffff8300952b7be8 000000008033bfe8 ffff82e01295e540 0000000000001adc
>> (XEN) ffff830756971770 0000000000000028 0000000000000000 ffff830799cdd000
>> (XEN) 0000000000000000 ffff830799cdd000 ffff8300952b7be8 ffff82d080229d4c
>> (XEN) 0000000000000000 ffff8300952b7d40 0000000000000000 0000000000000000
>> (XEN) 00000000000000a8 ffff830799cdd000 ffff8300952b7c98 ffff82d080221d90
>> (XEN) 0000000100000000 ffff830799cdd000 0000000000000000 0000000099cdd000
>> (XEN) ffff82e009cd0fd8 00000000000e7b1f ffff8300952b7c88 0000000000000020
>> (XEN) ffff8800e7b1fdd8 0000000000000002 0000000000000006 ffff830799cdd000
>> (XEN) ffff8300952b7c78 000000000039f480 0000000000000000 000000000000008d
>> (XEN) ffff8800e7b1fdd8 ffff830799cdd000 0000000000000006 ffff830799cdd000
>> (XEN) ffff8300952b7db8 ffff82d080223ad7 0000000000000046 ffff830088ff9000
>> (XEN) ffff8300952b7d18 ffff82d08023cfaf ffff82c000230118 ffff830842ceeb8c
>> (XEN) ffff82e009f54db8 00000000003bc78b ffff830842cd2770 ffff830088ff9000
>> (XEN) 0000000000000000 0000000000000000 ffff83085d6b9350 0000000000000000
>> (XEN) ffff8300952b7d28 ffff82d08023d766 ffff8300952b7d58 ffff82d08020c9a2
>> (XEN) ffff830842cee000 ffff830799cdd000 ffffffff81adbec0 0000000000000200
>> (XEN) 0000008d00000000 ffff82d000000000 ffffffff81adbec0 0000000000000200
>> (XEN) 0000000000000000 0000000000007ff0 ffff83085d6b9350 0000000000000006
>> (XEN) Xen call trace:
>> (XEN) [<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
>> (XEN) [<ffff82d080229d4c>] alloc_domheap_pages+0xb7/0x157
>> (XEN) [<ffff82d080221d90>] memory.c#populate_physmap+0x27e/0x4c9
>> (XEN) [<ffff82d080223ad7>] do_memory_op+0x2e2/0x2695
>> (XEN) [<ffff82d080308be9>] hypercall.c#hvm_memory_op+0x36/0x60
>> (XEN) [<ffff82d0803091c2>] hvm_hypercall+0x5af/0x681
>> (XEN) [<ffff82d08032fee6>] vmx_vmexit_handler+0x1040/0x1e14
>> (XEN) [<ffff82d080335f88>] vmx_asm_vmexit_handler+0xe8/0x250
>> (XEN)
>> (XEN)
>> (XEN) ****************************************
>> (XEN) Panic on CPU 0:
>> (XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
>> (XEN) ****************************************
>>
>> Running repeated tests on adjacent builds, we never see the assertion
>> failure without the patches (6 runs), and have so far seen for 3 of 4
>> runs (2 still pending) with the patches.
>>
>> What is rather strange is that there is a lot of migration and
>> ballooning going on, but only for HVM (Debian Jessie, not that this
>> should matter) VMs. dom0 will be the only PV domain in the system, and
>> is 64bit.
> Are you sure you have no other patches compared to staging in your
> hypervisor? I can't imagine how one of the 3 patches could cause that
> behavior.
>
> I've tried to do similar testing on my machine: 2 HVM domains + 64-bit
> Pv dom0. dom0 and one HVM domain are ballooned up and down all the time
> while the other HVM domain is being migrated (localhost) in a loop.
>
> Migration count is at 600 already...
So it turns out that I've now reproduce this ASSERT() once without any
patches from this series applied.
Therefore, it is a latent bug in either XenServer or Xen, but shouldn't
block this series (Especially as this series makes it easier to reproduce).
At this point, as we're planning to take the series for 4.11, it might
be better to throw the whole series in and get some wider testing that way.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 0/9] xen/x86: various XPTI speedups
2018-05-03 17:41 ` Andrew Cooper
@ 2018-05-03 18:41 ` Juergen Gross
0 siblings, 0 replies; 26+ messages in thread
From: Juergen Gross @ 2018-05-03 18:41 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: tim, jbeulich
On 03/05/18 19:41, Andrew Cooper wrote:
> On 02/05/18 11:38, Juergen Gross wrote:
>> On 01/05/18 11:28, Andrew Cooper wrote:
>>> On 26/04/18 12:33, Juergen Gross wrote:
>>>> This patch series aims at reducing the overhead of the XPTI Meltdown
>>>> mitigation.
>>> With just the first 3 patches of this series (in a bisection attempt),
>>> on a XenServer build based off staging, XenRT finds the following:
>>>
>>> (XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
>>> (XEN) ----[ Xen-4.11.0-6.0.0-d x86_64 debug=y Not tainted ]----
>>> (XEN) CPU: 0
>>> (XEN) RIP: e008:[<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
>>> (XEN) RFLAGS: 0000000000010286 CONTEXT: hypervisor (d33v0)
>>> (XEN) rax: ffff82e01307ade8 rbx: 000000000007ffff rcx: 8180000000000000
>>> (XEN) rdx: 0000000000000000 rsi: 00000000000001b5 rdi: 0000000000000000
>>> (XEN) rbp: ffff8300952b7ba8 rsp: ffff8300952b7b18 r8: 8000000000000000
>>> (XEN) r9: ffff82e01307ade8 r10: 0180000000000000 r11: 7fffffffffffffff
>>> (XEN) r12: 0000000000000000 r13: 00000000024c2e83 r14: 0000000000000000
>>> (XEN) r15: ffff82e01307add8 cr0: 0000000080050033 cr4: 00000000001526e0
>>> (XEN) cr3: 0000000799c41000 cr2: 00007fdaf5539000
>>> (XEN) fsb: 0000000000000000 gsb: 0000000000000000 gss: 0000000000000000
>>> (XEN) ds: 0000 es: 0000 fs: 0000 gs: 0000 ss: 0000 cs: e008
>>> (XEN) Xen code around <ffff82d080229914> (page_alloc.c#alloc_heap_pages+0x371/0x6f2):
>>> (XEN) ff 0f 0b 48 85 c9 79 31 <0f> 0b 48 c7 42 08 00 00 00 00 c7 42 10 00 00 00
>>> (XEN) Xen stack trace from rsp=ffff8300952b7b18:
>>> (XEN) 0000000000000001 ffff830799cdd000 0000000000000000 00000000003037e9
>>> (XEN) 0000000100000004 ffff8300952b7b68 0000000100000000 ffff830095738000
>>> (XEN) ffff8300952b7be8 000000008033bfe8 ffff82e01295e540 0000000000001adc
>>> (XEN) ffff830756971770 0000000000000028 0000000000000000 ffff830799cdd000
>>> (XEN) 0000000000000000 ffff830799cdd000 ffff8300952b7be8 ffff82d080229d4c
>>> (XEN) 0000000000000000 ffff8300952b7d40 0000000000000000 0000000000000000
>>> (XEN) 00000000000000a8 ffff830799cdd000 ffff8300952b7c98 ffff82d080221d90
>>> (XEN) 0000000100000000 ffff830799cdd000 0000000000000000 0000000099cdd000
>>> (XEN) ffff82e009cd0fd8 00000000000e7b1f ffff8300952b7c88 0000000000000020
>>> (XEN) ffff8800e7b1fdd8 0000000000000002 0000000000000006 ffff830799cdd000
>>> (XEN) ffff8300952b7c78 000000000039f480 0000000000000000 000000000000008d
>>> (XEN) ffff8800e7b1fdd8 ffff830799cdd000 0000000000000006 ffff830799cdd000
>>> (XEN) ffff8300952b7db8 ffff82d080223ad7 0000000000000046 ffff830088ff9000
>>> (XEN) ffff8300952b7d18 ffff82d08023cfaf ffff82c000230118 ffff830842ceeb8c
>>> (XEN) ffff82e009f54db8 00000000003bc78b ffff830842cd2770 ffff830088ff9000
>>> (XEN) 0000000000000000 0000000000000000 ffff83085d6b9350 0000000000000000
>>> (XEN) ffff8300952b7d28 ffff82d08023d766 ffff8300952b7d58 ffff82d08020c9a2
>>> (XEN) ffff830842cee000 ffff830799cdd000 ffffffff81adbec0 0000000000000200
>>> (XEN) 0000008d00000000 ffff82d000000000 ffffffff81adbec0 0000000000000200
>>> (XEN) 0000000000000000 0000000000007ff0 ffff83085d6b9350 0000000000000006
>>> (XEN) Xen call trace:
>>> (XEN) [<ffff82d080229914>] page_alloc.c#alloc_heap_pages+0x371/0x6f2
>>> (XEN) [<ffff82d080229d4c>] alloc_domheap_pages+0xb7/0x157
>>> (XEN) [<ffff82d080221d90>] memory.c#populate_physmap+0x27e/0x4c9
>>> (XEN) [<ffff82d080223ad7>] do_memory_op+0x2e2/0x2695
>>> (XEN) [<ffff82d080308be9>] hypercall.c#hvm_memory_op+0x36/0x60
>>> (XEN) [<ffff82d0803091c2>] hvm_hypercall+0x5af/0x681
>>> (XEN) [<ffff82d08032fee6>] vmx_vmexit_handler+0x1040/0x1e14
>>> (XEN) [<ffff82d080335f88>] vmx_asm_vmexit_handler+0xe8/0x250
>>> (XEN)
>>> (XEN)
>>> (XEN) ****************************************
>>> (XEN) Panic on CPU 0:
>>> (XEN) Assertion 'first_dirty != INVALID_DIRTY_IDX || !(pg[i].count_info & PGC_need_scrub)' failed at page_alloc.c:979
>>> (XEN) ****************************************
>>>
>>> Running repeated tests on adjacent builds, we never see the assertion
>>> failure without the patches (6 runs), and have so far seen for 3 of 4
>>> runs (2 still pending) with the patches.
>>>
>>> What is rather strange is that there is a lot of migration and
>>> ballooning going on, but only for HVM (Debian Jessie, not that this
>>> should matter) VMs. dom0 will be the only PV domain in the system, and
>>> is 64bit.
>> Are you sure you have no other patches compared to staging in your
>> hypervisor? I can't imagine how one of the 3 patches could cause that
>> behavior.
>>
>> I've tried to do similar testing on my machine: 2 HVM domains + 64-bit
>> Pv dom0. dom0 and one HVM domain are ballooned up and down all the time
>> while the other HVM domain is being migrated (localhost) in a loop.
>>
>> Migration count is at 600 already...
>
> So it turns out that I've now reproduce this ASSERT() once without any
> patches from this series applied.
>
> Therefore, it is a latent bug in either XenServer or Xen, but shouldn't
> block this series (Especially as this series makes it easier to reproduce).
>
> At this point, as we're planning to take the series for 4.11, it might
> be better to throw the whole series in and get some wider testing that way.
I believe taking this for RC3 tomorrow isn't the best idea, so lets wait
until Monday. This way we can let OSStest take a try with the series.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 0/9] xen/x86: various XPTI speedups
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (9 preceding siblings ...)
2018-05-01 9:28 ` [PATCH v9 0/9] xen/x86: various XPTI speedups Andrew Cooper
@ 2018-05-04 14:59 ` Wei Liu
2018-05-16 9:06 ` backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups) Jan Beulich
11 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2018-05-04 14:59 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, tim, Wei Liu, jbeulich, andrew.cooper3
On Thu, Apr 26, 2018 at 01:33:09PM +0200, Juergen Gross wrote:
> Juergen Gross (9):
> x86/xpti: avoid copying L4 page table contents when possible
> xen/x86: add a function for modifying cr3
> xen/x86: support per-domain flag for xpti
> xen/x86: use invpcid for flushing the TLB
> xen/x86: disable global pages for domains with XPTI active
> xen/x86: use flag byte for decision whether xen_cr3 is valid
> xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
> xen/x86: add some cr3 helpers
> xen/x86: use PCID feature
There are conflicts in xen-command-line. I fixed them up and pushed this
series to staging. Please check if the result is correct.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v9 3/9] xen/x86: support per-domain flag for xpti
2018-04-27 8:15 ` Jan Beulich
@ 2018-05-04 15:06 ` Wei Liu
0 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2018-05-04 15:06 UTC (permalink / raw)
To: Jan Beulich
Cc: Juergen Gross, Sergey Dyasli, Wei Liu, Andrew Cooper, Tim Deegan,
xen-devel
On Fri, Apr 27, 2018 at 02:15:25AM -0600, Jan Beulich wrote:
> >>> On 27.04.18 at 09:59, <jgross@suse.com> wrote:
> > On 27/04/18 09:55, Sergey Dyasli wrote:
> >> On Thu, 2018-04-26 at 13:33 +0200, Juergen Gross wrote:
> >>> index b353352adf..220d1ba020 100644
> >>> --- a/docs/misc/xen-command-line.markdown
> >>> +++ b/docs/misc/xen-command-line.markdown
> >>> @@ -1955,14 +1955,24 @@ clustered mode. The default, given no hint from the **FADT**, is cluster
> >>> mode.
> >>>
> >>> ### xpti
> >>> -> `= <boolean>`
> >>> +> `= List of [ default | <boolean> | dom0=<bool> | domu=<bool> ]`
> >>>
> >>> -> Default: `false` on AMD hardware
> >>> +> Default: `false` on hardware not to be vulnerable to Meltdown (e.g. AMD)
> >> ^
> >> known
> >
> > Yes, indeed.
>
> Recorded for eventual application of the series; no need to resend just for this.
Oops, I missed this while fixing up the conflicts.
I will submit a patch to fix this.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
` (10 preceding siblings ...)
2018-05-04 14:59 ` Wei Liu
@ 2018-05-16 9:06 ` Jan Beulich
2018-05-16 13:18 ` George Dunlap
11 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2018-05-16 9:06 UTC (permalink / raw)
To: xen-devel
>>> On 26.04.18 at 13:33, <jgross@suse.com> wrote:
> Juergen Gross (9):
> x86/xpti: avoid copying L4 page table contents when possible
> xen/x86: add a function for modifying cr3
> xen/x86: support per-domain flag for xpti
> xen/x86: use invpcid for flushing the TLB
> xen/x86: disable global pages for domains with XPTI active
> xen/x86: use flag byte for decision whether xen_cr3 is valid
> xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
> xen/x86: add some cr3 helpers
> xen/x86: use PCID feature
This being a performance improvement rather than a plain bug fix series,
I'm not entirely certain about backporting here. My current thinking is to
put this into 4.10 (Jürgen was kind enough to do the backporting work
already), but not into any older trees. Otoh at SUSE we already have
this in our 4.9-based branch as well, and if other consumers of that or
the 4.8 branch would mostly agree it should go there, I could certainly
be convinced. It is imo out of question of putting it into 4.7 or older.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)
2018-05-16 9:06 ` backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups) Jan Beulich
@ 2018-05-16 13:18 ` George Dunlap
2018-05-16 14:01 ` Jan Beulich
0 siblings, 1 reply; 26+ messages in thread
From: George Dunlap @ 2018-05-16 13:18 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Wed, May 16, 2018 at 10:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 26.04.18 at 13:33, <jgross@suse.com> wrote:
>> Juergen Gross (9):
>> x86/xpti: avoid copying L4 page table contents when possible
>> xen/x86: add a function for modifying cr3
>> xen/x86: support per-domain flag for xpti
>> xen/x86: use invpcid for flushing the TLB
>> xen/x86: disable global pages for domains with XPTI active
>> xen/x86: use flag byte for decision whether xen_cr3 is valid
>> xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
>> xen/x86: add some cr3 helpers
>> xen/x86: use PCID feature
>
> This being a performance improvement rather than a plain bug fix series,
> I'm not entirely certain about backporting here. My current thinking is to
> put this into 4.10 (Jürgen was kind enough to do the backporting work
> already), but not into any older trees. Otoh at SUSE we already have
> this in our 4.9-based branch as well, and if other consumers of that or
> the 4.8 branch would mostly agree it should go there, I could certainly
> be convinced.
Turning on XPTI causes a pretty large regression in performance; and
these reduce that regression significantly. I'm pretty sure that most
downstreams will end up backporting these anyway (I'm sure CentOS
will); it's much better to do it officially once, rather than have
individual downstreams (most of whom do not have hypervisor developers
maintaining packages) all do it separately.
> It is imo out of question of putting it into 4.7 or older.
Is that because of the complexity? Or because 4.7 is in "security-only" mode?
If the latter, I think the same argument applies: turning on XPTI is a
requirement for many people, and thus represents a pretty hefty
performance regression. While we don't need to backport normal fixes
to security-only releases, we should certainly try to avoid
regressions.
(Note that this doesn't mean you personally have to do the work if you
think it's not worth your time.)
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)
2018-05-16 13:18 ` George Dunlap
@ 2018-05-16 14:01 ` Jan Beulich
2018-05-16 14:53 ` George Dunlap
0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2018-05-16 14:01 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
>>> On 16.05.18 at 15:18, <dunlapg@umich.edu> wrote:
> On Wed, May 16, 2018 at 10:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 26.04.18 at 13:33, <jgross@suse.com> wrote:
>>> Juergen Gross (9):
>>> x86/xpti: avoid copying L4 page table contents when possible
>>> xen/x86: add a function for modifying cr3
>>> xen/x86: support per-domain flag for xpti
>>> xen/x86: use invpcid for flushing the TLB
>>> xen/x86: disable global pages for domains with XPTI active
>>> xen/x86: use flag byte for decision whether xen_cr3 is valid
>>> xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
>>> xen/x86: add some cr3 helpers
>>> xen/x86: use PCID feature
>>
>> This being a performance improvement rather than a plain bug fix series,
>> I'm not entirely certain about backporting here. My current thinking is to
>> put this into 4.10 (Jürgen was kind enough to do the backporting work
>> already), but not into any older trees. Otoh at SUSE we already have
>> this in our 4.9-based branch as well, and if other consumers of that or
>> the 4.8 branch would mostly agree it should go there, I could certainly
>> be convinced.
>
> Turning on XPTI causes a pretty large regression in performance; and
> these reduce that regression significantly. I'm pretty sure that most
> downstreams will end up backporting these anyway (I'm sure CentOS
> will); it's much better to do it officially once, rather than have
> individual downstreams (most of whom do not have hypervisor developers
> maintaining packages) all do it separately.
Thanks, recorded as a data point. I don't, however, consider "do not
have hypervisor developers" as a valid excuse. Package maintainers
ought to be understanding their packages well enough to be capable of
doing such backports if they really think they need them. Basically this
goes back to there being (too many?) downstreams who don't care at
all to contribute anything back.
>> It is imo out of question of putting it into 4.7 or older.
>
> Is that because of the complexity? Or because 4.7 is in "security-only"
> mode?
The latter.
> If the latter, I think the same argument applies: turning on XPTI is a
> requirement for many people, and thus represents a pretty hefty
> performance regression. While we don't need to backport normal fixes
> to security-only releases, we should certainly try to avoid
> regressions.
I don't think we would have addressed non-security fallout (or other
than really severe regressions) from other security patches in the
past on security only branches. People caring about performance
should upgrade.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)
2018-05-16 14:01 ` Jan Beulich
@ 2018-05-16 14:53 ` George Dunlap
2018-05-16 16:01 ` Jan Beulich
0 siblings, 1 reply; 26+ messages in thread
From: George Dunlap @ 2018-05-16 14:53 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Wed, May 16, 2018 at 3:01 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 16.05.18 at 15:18, <dunlapg@umich.edu> wrote:
>> On Wed, May 16, 2018 at 10:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 26.04.18 at 13:33, <jgross@suse.com> wrote:
>>>> Juergen Gross (9):
>>>> x86/xpti: avoid copying L4 page table contents when possible
>>>> xen/x86: add a function for modifying cr3
>>>> xen/x86: support per-domain flag for xpti
>>>> xen/x86: use invpcid for flushing the TLB
>>>> xen/x86: disable global pages for domains with XPTI active
>>>> xen/x86: use flag byte for decision whether xen_cr3 is valid
>>>> xen/x86: convert pv_guest_cr4_to_real_cr4() to a function
>>>> xen/x86: add some cr3 helpers
>>>> xen/x86: use PCID feature
>>>
>>> This being a performance improvement rather than a plain bug fix series,
>>> I'm not entirely certain about backporting here. My current thinking is to
>>> put this into 4.10 (Jürgen was kind enough to do the backporting work
>>> already), but not into any older trees. Otoh at SUSE we already have
>>> this in our 4.9-based branch as well, and if other consumers of that or
>>> the 4.8 branch would mostly agree it should go there, I could certainly
>>> be convinced.
>>
>> Turning on XPTI causes a pretty large regression in performance; and
>> these reduce that regression significantly. I'm pretty sure that most
>> downstreams will end up backporting these anyway (I'm sure CentOS
>> will); it's much better to do it officially once, rather than have
>> individual downstreams (most of whom do not have hypervisor developers
>> maintaining packages) all do it separately.
>
> Thanks, recorded as a data point. I don't, however, consider "do not
> have hypervisor developers" as a valid excuse. Package maintainers
> ought to be understanding their packages well enough to be capable of
> doing such backports if they really think they need them. Basically this
> goes back to there being (too many?) downstreams who don't care at
> all to contribute anything back.
I'm not sure exactly who you have in mind here; maybe you mean
downstreams like SuSE and XenServer that sell a product with Xen
inside. Such companies certainly should have engineers whose job it
is to know the code to one of their key components.
I had in mind downstreams like Fedora, Debian, CentOS, ArchLinux,
XenMadeEasy, and so on; and/or end users who recompile their own
packages based on those; even small projects like QubesOS. Most of
those people know C and can make a good stab at fixing a patch which
doesn't apply. But it's not reasonable to expect volunteers
maintaining packages for free, or end users, to be experts in the
hypervisor -- each such port introduces a risk that there will be a
mistake. And anyway, it seems like a waste of time to make each
downstream using (say) 4.8 duplicate their effort, instead of doing it
once and letting them all share the results.
Having xen packages available for end-users to use *does* contribute
back to the Xen project; and many of the downstreams do contribute
back in many ways, including actual code -- just typically not code in
the hypervisor.
>> If the latter, I think the same argument applies: turning on XPTI is a
>> requirement for many people, and thus represents a pretty hefty
>> performance regression. While we don't need to backport normal fixes
>> to security-only releases, we should certainly try to avoid
>> regressions.
>
> I don't think we would have addressed non-security fallout (or other
> than really severe regressions) from other security patches in the
> past on security only branches. People caring about performance
> should upgrade.
If a security patch, when backported to 4.6, broke some fairly
critical bit of functionality (say, openvswitch support), you would
oppose a subsequent patch which would fix that regression?
That doesn't seem very reasonable to me. Users shouldn't have to
choose between being vulnerable to a security issue and losing
functionality which was working at the last release. Otherwise,
what's the point of having "security supported" releases?
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)
2018-05-16 14:53 ` George Dunlap
@ 2018-05-16 16:01 ` Jan Beulich
2018-05-16 16:42 ` George Dunlap
0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2018-05-16 16:01 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
>>> On 16.05.18 at 16:53, <dunlapg@umich.edu> wrote:
> On Wed, May 16, 2018 at 3:01 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 16.05.18 at 15:18, <dunlapg@umich.edu> wrote:
>>> If the latter, I think the same argument applies: turning on XPTI is a
>>> requirement for many people, and thus represents a pretty hefty
>>> performance regression. While we don't need to backport normal fixes
>>> to security-only releases, we should certainly try to avoid
>>> regressions.
>>
>> I don't think we would have addressed non-security fallout (or other
>> than really severe regressions) from other security patches in the
>> past on security only branches. People caring about performance
>> should upgrade.
>
> If a security patch, when backported to 4.6, broke some fairly
> critical bit of functionality (say, openvswitch support), you would
> oppose a subsequent patch which would fix that regression?
>
> That doesn't seem very reasonable to me. Users shouldn't have to
> choose between being vulnerable to a security issue and losing
> functionality which was working at the last release. Otherwise,
> what's the point of having "security supported" releases?
Note how I did say "or other than really severe regressions". I think
your "fairly critical bit of functionality" falls into exactly that area.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups)
2018-05-16 16:01 ` Jan Beulich
@ 2018-05-16 16:42 ` George Dunlap
0 siblings, 0 replies; 26+ messages in thread
From: George Dunlap @ 2018-05-16 16:42 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Wed, May 16, 2018 at 5:01 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 16.05.18 at 16:53, <dunlapg@umich.edu> wrote:
>> On Wed, May 16, 2018 at 3:01 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 16.05.18 at 15:18, <dunlapg@umich.edu> wrote:
>>>> If the latter, I think the same argument applies: turning on XPTI is a
>>>> requirement for many people, and thus represents a pretty hefty
>>>> performance regression. While we don't need to backport normal fixes
>>>> to security-only releases, we should certainly try to avoid
>>>> regressions.
>>>
>>> I don't think we would have addressed non-security fallout (or other
>>> than really severe regressions) from other security patches in the
>>> past on security only branches. People caring about performance
>>> should upgrade.
>>
>> If a security patch, when backported to 4.6, broke some fairly
>> critical bit of functionality (say, openvswitch support), you would
>> oppose a subsequent patch which would fix that regression?
>>
>> That doesn't seem very reasonable to me. Users shouldn't have to
>> choose between being vulnerable to a security issue and losing
>> functionality which was working at the last release. Otherwise,
>> what's the point of having "security supported" releases?
>
> Note how I did say "or other than really severe regressions". I think
> your "fairly critical bit of functionality" falls into exactly that area.
Right, so we agree on the basic principles, but disagree about whether
XPTI's performance hit counts as a "really severe regression". At
least one of my CentOS users was seriously considering applying
juergen's XPTI improvement patch before it even hit staging, because,
"We're struggling with slowdowns from xpti within the dom0." (I
advised her not to at that point, because XenRT had flagged up some
potential issues.) If a user describes herself as "struggling", I
think that counts as a regression. This was for Xen 4.8, but I think
the basic principle applies. I'll definitely be backporting those to
the 4.8 CentOS packages now that they're in staging; and I'll probably
try backporting them to the 4.6 packages too (since they're currently
the default, although hopefully not for long).
Like I said, I don't expect you personally to do something you don't
think is worth your time. But I think it would in general be good if
the XenProject fixed this performance regression for releases which
are still under security support.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-05-16 16:42 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 11:33 [PATCH v9 0/9] xen/x86: various XPTI speedups Juergen Gross
2018-04-26 11:33 ` [PATCH v9 1/9] x86/xpti: avoid copying L4 page table contents when possible Juergen Gross
2018-04-26 14:01 ` Tim Deegan
2018-04-26 11:33 ` [PATCH v9 2/9] xen/x86: add a function for modifying cr3 Juergen Gross
2018-04-26 11:33 ` [PATCH v9 3/9] xen/x86: support per-domain flag for xpti Juergen Gross
2018-04-27 7:55 ` Sergey Dyasli
2018-04-27 7:59 ` Juergen Gross
2018-04-27 8:15 ` Jan Beulich
2018-05-04 15:06 ` Wei Liu
2018-04-26 11:33 ` [PATCH v9 4/9] xen/x86: use invpcid for flushing the TLB Juergen Gross
2018-04-26 11:33 ` [PATCH v9 5/9] xen/x86: disable global pages for domains with XPTI active Juergen Gross
2018-04-26 11:33 ` [PATCH v9 6/9] xen/x86: use flag byte for decision whether xen_cr3 is valid Juergen Gross
2018-04-26 11:33 ` [PATCH v9 7/9] xen/x86: convert pv_guest_cr4_to_real_cr4() to a function Juergen Gross
2018-04-26 11:33 ` [PATCH v9 8/9] xen/x86: add some cr3 helpers Juergen Gross
2018-04-26 11:33 ` [PATCH v9 9/9] xen/x86: use PCID feature Juergen Gross
2018-05-01 9:28 ` [PATCH v9 0/9] xen/x86: various XPTI speedups Andrew Cooper
2018-05-02 10:38 ` Juergen Gross
2018-05-03 17:41 ` Andrew Cooper
2018-05-03 18:41 ` Juergen Gross
2018-05-04 14:59 ` Wei Liu
2018-05-16 9:06 ` backporting considerations (Re: [PATCH v9 0/9] xen/x86: various XPTI speedups) Jan Beulich
2018-05-16 13:18 ` George Dunlap
2018-05-16 14:01 ` Jan Beulich
2018-05-16 14:53 ` George Dunlap
2018-05-16 16:01 ` Jan Beulich
2018-05-16 16:42 ` George Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.