Compare commits

...
Sign in to create a new pull request.

12 commits

Author SHA1 Message Date
Michael Young
12be10b823 - xen: various flaws (#1685577)
grant table transfer issues on large hosts [XSA-284]
  race with pass-through device hotplug [XSA-285]
  x86: steal_page violates page_struct access discipline [XSA-287]
  x86: Inconsistent PV IOMMU discipline [XSA-288]
  missing preemption in x86 PV page table unvalidation [XSA-290]
  x86/PV: page type reference counting issue with failed IOMMU update [XSA-291]
  x86: insufficient TLB flushing when using PCID [XSA-292]
  x86: PV kernel context switch corruption [XSA-293]
  x86 shadow: Insufficient TLB flushing when using PCID [XSA-294]
2019-03-05 22:40:55 +00:00
Michael Young
f31a843850 clean up patches superceded by xen-4.10.3 2019-03-05 22:27:30 +00:00
Michael Young
bf40b8fe93 update to xen-4.10.3 2019-02-27 00:06:55 +00:00
Michael Young
6eed3dc249 add CVE reference 2019-01-08 22:47:47 +00:00
Michael Young
16ab24a4fb add CVE references 2018-12-12 12:40:17 +00:00
Michael Young
868acb702a add bug references 2018-11-21 18:36:58 +00:00
Michael Young
014c4d12c8 insufficient TLB flushing / improper large page mappings with AMD IOMMUs
[XSA-275] (#1651665)
x86: DoS from attempting to use INVPCID with a non-canonical addresses
        [XSA-279]
Fix for XSA-240 conflicts with shadow paging [XSA-280]
2018-11-20 16:34:56 +00:00
Michael Young
10b39d4b31 guest use of HLE constructs may lock up host [XSA-282] 2018-11-06 21:46:50 +00:00
Michael Young
8f6c23d9f6 x86: Nested VT-x usable even when disabled [XSA-278] (#1643118) 2018-10-25 20:18:08 +01:00
Michael Young
98784ffc48 update to xen-4.10.2 2018-09-25 20:09:06 +01:00
Michael Young
1836b9b328 add CVE references 2018-09-12 19:47:45 +01:00
Michael Young
37e3a4f8c2 L1 Terminal Fault speculative side channel patch bundle [XSA-273,
CVE-2018-3620, CVE-2018-3646]
drop patches also in the bundle, which also includes
Use of v2 grant tables may cause crash on ARM [XSA-268] (#1616081)
x86: Incorrect MSR_DEBUGCTL handling lets guests enable BTS [XSA-269]
	(#1616077)
oxenstored does not apply quota-maxentity [XSA-272] (#1616080)
2018-08-15 21:32:30 +01:00
45 changed files with 1869 additions and 4031 deletions

2
.gitignore vendored
View file

@ -6,4 +6,4 @@ lwip-1.3.0.tar.gz
pciutils-2.2.9.tar.bz2
zlib-1.2.3.tar.gz
polarssl-1.1.4-gpl.tgz
/xen-4.10.1.tar.gz
/xen-4.10.3.tar.gz

View file

@ -4,4 +4,4 @@ SHA512 (newlib-1.16.0.tar.gz) = 40eb96bbc6736a16b6399e0cdb73e853d0d90b685c967e77
SHA512 (zlib-1.2.3.tar.gz) = 021b958fcd0d346c4ba761bcf0cc40f3522de6186cf5a0a6ea34a70504ce9622b1c2626fce40675bc8282cf5f5ade18473656abc38050f72f5d6480507a2106e
SHA512 (polarssl-1.1.4-gpl.tgz) = 88da614e4d3f4409c4fd3bb3e44c7587ba051e3fed4e33d526069a67e8180212e1ea22da984656f50e290049f60ddca65383e5983c0f8884f648d71f698303ad
SHA512 (pciutils-2.2.9.tar.bz2) = 2b3d98d027e46d8c08037366dde6f0781ca03c610ef2b380984639e4ef39899ed8d8b8e4cd9c9dc54df101279b95879bd66bfd4d04ad07fef41e847ea7ae32b5
SHA512 (xen-4.10.1.tar.gz) = 236c02bee69e33644703ed26d323d4c491a91fc05bd0ee0990a7368579f7c82f5bb4510845bf80348fd923024d7d60d521f593dfd0365d971dc592f8ef10fbea
SHA512 (xen-4.10.3.tar.gz) = 030d23109ce4c0ebf50ade6841d5525a39032bcf8fd0599ccaa9502ff301c1f61f9c5f49233b41a5934a71228958febc1e529a9fc6822ba401a0c18c490ad412

View file

@ -1,50 +1,3 @@
--- xen-4.8.0/xen/arch/x86/Makefile.orig 2016-12-05 12:03:27.000000000 +0000
+++ xen-4.8.0/xen/arch/x86/Makefile 2017-02-28 00:02:27.989589825 +0000
@@ -73,6 +73,7 @@
efi-y := $(shell if [ ! -r $(BASEDIR)/include/xen/compile.h -o \
-O $(BASEDIR)/include/xen/compile.h ]; then \
echo '$(TARGET).efi'; fi)
+LD_EFI ?= $(LD)
ifneq ($(build_id_linker),)
notes_phdrs = --notes
@@ -173,20 +174,20 @@
$(TARGET).efi: prelink-efi.o $(note_file) efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbols-dummy.o efi/mkreloc
$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
- $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \
+ $(guard) $(LD_EFI) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \
$(BASEDIR)/common/symbols-dummy.o $(note_file) -o $(@D)/.$(@F).$(base).0 &&) :
$(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S
$(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \
| $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S
$(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o
$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
- $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \
+ $(guard) $(LD_EFI) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \
$(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(note_file) -o $(@D)/.$(@F).$(base).1 &&) :
$(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S
$(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \
| $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S
$(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o
- $(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
+ $(guard) $(LD_EFI) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
$(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(note_file) -o $@
if $(guard) false; then rm -f $@; echo 'EFI support disabled'; \
else $(NM) -pa --format=sysv $(@D)/$(@F) \
--- xen-4.9.0-rc1.2/xen/arch/x86/efi/Makefile.orig 2017-04-12 16:18:57.000000000 +0100
+++ xen-4.9.0-rc1.2/xen/arch/x86/efi/Makefile 2017-04-13 21:05:54.170387130 +0100
@@ -1,8 +1,9 @@
CFLAGS += -fshort-wchar
+LD_EFI ?= $(LD)
efi := y$(shell rm -f disabled)
efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c check.c 2>disabled && echo y))
-efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y))
+efi := $(if $(efi),$(shell $(LD_EFI) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y))
efi := $(if $(efi),$(shell rm disabled)y)
%.o: %.ihex
--- xen-4.8.0/xen/Makefile.orig 2016-12-05 12:03:27.000000000 +0000
+++ xen-4.8.0/xen/Makefile 2017-02-28 00:02:54.080529810 +0000
@@ -20,6 +20,7 @@

View file

@ -1,63 +0,0 @@
From 6b8d820bbe5c4aeba8601b31a650f6e6cd12843e Mon Sep 17 00:00:00 2001
From: Jan Beulich <jbeulich@suse.com>
Date: Fri, 18 May 2018 11:39:07 +0200
Subject: [PATCH] x86: correct ordering of operations during S3 resume
Microcode loading needs to happen before re-enabling interrupts, in case
only updated microcode allows the use of e.g. the SPEC_{CTRL,CMD} MSRs.
Otoh it doesn't need to happen at all when we didn't suspend in the
first place. It needs to happen before spin_debug_enable() though, as it
acquires a lock and hence would otherwise make
common/spinlock.c:check_lock() unhappy. As micrcode loading can be
pretty verbose, also make sure it only runs after console_end_sync().
cpufreq_add_cpu() doesn't need calling on the only "goto enable_cpu"
path, which sits ahead of cpufreq_del_cpu().
Reported-by: Simon Gaiser <simon@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: cb2a4a449dfd50af309a333aa805835015fbc8c8
master date: 2018-04-16 14:08:30 +0200
---
xen/arch/x86/acpi/power.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index 1e4e5680a7..cb06f842cd 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -203,6 +203,7 @@ static int enter_state(u32 state)
printk(XENLOG_ERR "Some devices failed to power down.");
system_state = SYS_STATE_resume;
device_power_up(error);
+ console_end_sync();
error = -EIO;
goto done;
}
@@ -243,17 +244,19 @@ static int enter_state(u32 state)
if ( (state == ACPI_STATE_S3) && error )
tboot_s3_error(error);
+ console_end_sync();
+
+ microcode_resume_cpu(0);
+
done:
spin_debug_enable();
local_irq_restore(flags);
- console_end_sync();
acpi_sleep_post(state);
if ( hvm_cpu_up() )
BUG();
+ cpufreq_add_cpu(0);
enable_cpu:
- cpufreq_add_cpu(0);
- microcode_resume_cpu(0);
rcu_barrier();
mtrr_aps_sync_begin();
enable_nonboot_cpus();
--
2.11.0

View file

@ -1,148 +0,0 @@
From a7f8880adc1604335e42920340c127ba7c51b0a5 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Fri, 18 May 2018 11:41:53 +0200
Subject: [PATCH] x86/spec_ctrl: Updates to retpoline-safety decision making
All of this is as recommended by the Intel whitepaper:
https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf
The 'RSB Alternative' bit in MSR_ARCH_CAPABILITIES may be set by a hypervisor
to indicate that the virtual machine may migrate to a processor which isn't
retpoline-safe. Introduce a shortened name (to reduce code volume), treat it
as authorative in retpoline_safe(), and print its value along with the other
ARCH_CAPS bits.
The exact processor models which do have RSB semantics which fall back to BTB
predictions are enumerated, and include Kabylake and Coffeelake. Leave a
printk() in the default case to help identify cases which aren't covered.
The exact microcode versions from Broadwell RSB-safety are taken from the
referenced microcode update file (adjusting for the known-bad microcode
versions). Despite the exact wording of the text, it is only Broadwell
processors which need a microcode check.
In practice, this means that all Broadwell hardware with up-to-date microcode
will use retpoline in preference to IBRS, which will be a performance
improvement for desktop and server systems which would previously always opt
for IBRS over retpoline.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
x86/spec_ctrl: Fix typo in ARCH_CAPS decode
Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 1232378bd2fef45f613db049b33852fdf84d7ddf
master date: 2018-04-19 17:28:23 +0100
master commit: 27170adb54a558e11defcd51989326a9beb95afe
master date: 2018-04-24 13:34:12 +0100
---
xen/arch/x86/spec_ctrl.c | 51 +++++++++++++++++++++++++++++++++++------
xen/include/asm-x86/msr-index.h | 1 +
2 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 3c7447bfe6..fa67a0ffbd 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -97,12 +97,13 @@ static void __init print_details(enum ind_thunk thunk)
printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
/* Hardware features which pertain to speculative mitigations. */
- printk(XENLOG_DEBUG " Hardware features:%s%s%s%s%s\n",
+ printk(XENLOG_DEBUG " Hardware features:%s%s%s%s%s%s\n",
(_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
(_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP" : "",
(e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "",
(caps & ARCH_CAPABILITIES_IBRS_ALL) ? " IBRS_ALL" : "",
- (caps & ARCH_CAPABILITIES_RDCL_NO) ? " RDCL_NO" : "");
+ (caps & ARCH_CAPABILITIES_RDCL_NO) ? " RDCL_NO" : "",
+ (caps & ARCH_CAPS_RSBA) ? " RSBA" : "");
/* Compiled-in support which pertains to BTI mitigations. */
if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
@@ -135,6 +136,20 @@ static bool __init retpoline_safe(void)
boot_cpu_data.x86 != 6 )
return false;
+ if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
+ {
+ uint64_t caps;
+
+ rdmsrl(MSR_ARCH_CAPABILITIES, caps);
+
+ /*
+ * RBSA may be set by a hypervisor to indicate that we may move to a
+ * processor which isn't retpoline-safe.
+ */
+ if ( caps & ARCH_CAPS_RSBA )
+ return false;
+ }
+
switch ( boot_cpu_data.x86_model )
{
case 0x17: /* Penryn */
@@ -161,18 +176,40 @@ static bool __init retpoline_safe(void)
* versions.
*/
case 0x3d: /* Broadwell */
- return ucode_rev >= 0x28;
+ return ucode_rev >= 0x2a;
case 0x47: /* Broadwell H */
- return ucode_rev >= 0x1b;
+ return ucode_rev >= 0x1d;
case 0x4f: /* Broadwell EP/EX */
- return ucode_rev >= 0xb000025;
+ return ucode_rev >= 0xb000021;
case 0x56: /* Broadwell D */
- return false; /* TBD. */
+ switch ( boot_cpu_data.x86_mask )
+ {
+ case 2: return ucode_rev >= 0x15;
+ case 3: return ucode_rev >= 0x7000012;
+ case 4: return ucode_rev >= 0xf000011;
+ case 5: return ucode_rev >= 0xe000009;
+ default:
+ printk("Unrecognised CPU stepping %#x - assuming not reptpoline safe\n",
+ boot_cpu_data.x86_mask);
+ return false;
+ }
+ break;
/*
- * Skylake and later processors are not retpoline-safe.
+ * Skylake, Kabylake and Cannonlake processors are not retpoline-safe.
*/
+ case 0x4e:
+ case 0x55:
+ case 0x5e:
+ case 0x66:
+ case 0x67:
+ case 0x8e:
+ case 0x9e:
+ return false;
+
default:
+ printk("Unrecognised CPU model %#x - assuming not reptpoline safe\n",
+ boot_cpu_data.x86_model);
return false;
}
}
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index a8ceecf3e2..bb6295790c 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -42,6 +42,7 @@
#define MSR_ARCH_CAPABILITIES 0x0000010a
#define ARCH_CAPABILITIES_RDCL_NO (_AC(1, ULL) << 0)
#define ARCH_CAPABILITIES_IBRS_ALL (_AC(1, ULL) << 1)
+#define ARCH_CAPS_RSBA (_AC(1, ULL) << 2)
/* Intel MSRs. Some also available on other CPUs */
#define MSR_IA32_PERFCTR0 0x000000c1
--
2.11.0

View file

@ -1,77 +0,0 @@
From d93ae631a49289992dabb7bcd08358afc5f22b56 Mon Sep 17 00:00:00 2001
From: Jan Beulich <jbeulich@suse.com>
Date: Fri, 18 May 2018 11:39:38 +0200
Subject: [PATCH] x86: suppress BTI mitigations around S3 suspend/resume
NMI and #MC can occur at any time after S3 resume, yet the MSR_SPEC_CTRL
may become available only once we're reloaded microcode. Make
SPEC_CTRL_ENTRY_FROM_INTR_IST and DO_SPEC_CTRL_EXIT_TO_XEN no-ops for
the critical period of time.
Also set the MSR back to its intended value.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
x86: Use spec_ctrl_{enter,exit}_idle() in the S3/S5 path
The main purpose of this patch is to avoid opencoding the recovery logic at
the end, but also has the positive side effect of relaxing the SPEC_CTRL
mitigations when working to shut the final CPU down.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 710a8ebf2bc111a34bba04d1c85b6d07ed3d9389
master date: 2018-04-16 14:09:55 +0200
master commit: ef3ab46493f650b7e5cca2b2578a99ca0cbff195
master date: 2018-04-19 10:55:59 +0100
---
xen/arch/x86/acpi/power.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index cb06f842cd..f7085d3c7b 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -28,6 +28,7 @@
#include <asm/tboot.h>
#include <asm/apic.h>
#include <asm/io_apic.h>
+#include <asm/spec_ctrl.h>
#include <acpi/cpufreq/cpufreq.h>
uint32_t system_reset_counter = 1;
@@ -163,6 +164,7 @@ static int enter_state(u32 state)
{
unsigned long flags;
int error;
+ struct cpu_info *ci;
unsigned long cr4;
if ( (state <= ACPI_STATE_S0) || (state > ACPI_S_STATES_MAX) )
@@ -210,6 +212,11 @@ static int enter_state(u32 state)
else
error = 0;
+ ci = get_cpu_info();
+ spec_ctrl_enter_idle(ci);
+ /* Avoid NMI/#MC using MSR_SPEC_CTRL until we've reloaded microcode. */
+ ci->bti_ist_info = 0;
+
ACPI_FLUSH_CPU_CACHE();
switch ( state )
@@ -248,6 +255,10 @@ static int enter_state(u32 state)
microcode_resume_cpu(0);
+ /* Re-enabled default NMI/#MC use of MSR_SPEC_CTRL. */
+ ci->bti_ist_info = default_bti_ist_info;
+ spec_ctrl_exit_idle(ci);
+
done:
spin_debug_enable();
local_irq_restore(flags);
--
2.11.0

View file

@ -1,20 +0,0 @@
--- xen-4.10.1/tools/libacpi/Makefile.orig 2018-06-15 22:05:18.875735520 +0100
+++ xen-4.10.1/tools/libacpi/Makefile 2018-06-15 22:07:16.087982920 +0100
@@ -43,7 +43,7 @@
$(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
iasl -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $<
- sed -e 's/AmlCode/$*/g' $(ACPI_BUILD_DIR)/$*.hex >$@
+ sed -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@
rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex)
$(MK_DSDT): mk_dsdt.c
@@ -76,7 +76,7 @@
$(C_SRC): $(ACPI_BUILD_DIR)/%.c: iasl $(ACPI_BUILD_DIR)/%.asl
iasl -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $(ACPI_BUILD_DIR)/$*.asl
- sed -e 's/AmlCode/$*/g' $(ACPI_BUILD_DIR)/$*.hex > $@.$(TMP_SUFFIX)
+ sed -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex > $@.$(TMP_SUFFIX)
echo "int $*_len=sizeof($*);" >> $@.$(TMP_SUFFIX)
mv -f $@.$(TMP_SUFFIX) $@
rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex)

117
xen.spec
View file

@ -36,7 +36,8 @@
# --without efi
%define build_efi %{?_without_efi: 0} %{?!_without_efi: 1}
# xen only supports efi boot images on x86_64 or aarch64
%ifnarch x86_64 aarch64
# i686 builds a x86_64 hypervisor so add that as well
%ifnarch x86_64 aarch64 %{ix86}
%define build_efi 0
%endif
%if %build_efi && "%dist" < ".fc26"
@ -59,8 +60,8 @@
Summary: Xen is a virtual machine monitor
Name: xen
Version: 4.10.1
Release: 5%{?dist}
Version: 4.10.3
Release: 2%{?dist}
Group: Development/Libraries
License: GPLv2+ and LGPLv2+ and BSD
URL: http://xen.org/
@ -118,35 +119,17 @@ Patch39: qemu.trad.CVE-2017-9330.patch
Patch40: xen.ocaml.safe-strings.patch
Patch45: xen.gcc8.fix.patch
Patch51: xen.gcc8.temp.fix.patch
Patch52: xsa260-4.10-1.patch
Patch53: xsa260-4.10-2.patch
Patch54: xsa260-4.10-3.patch
Patch55: xsa260-4.10-4.patch
Patch56: xsa261.patch
Patch57: xsa262-4.10.patch
Patch58: xen.git-a7f8880adc1604335e42920340c127ba7c51b0a5.patch
Patch59: xen.git-6b8d820bbe5c4aeba8601b31a650f6e6cd12843e.patch
Patch60: xen.git-d93ae631a49289992dabb7bcd08358afc5f22b56.patch
Patch61: xsa263-4.10-0001-x86-spec_ctrl-Read-MSR_ARCH_CAPABILITIES-only-once.patch
Patch62: xsa263-4.10-0002-x86-spec_ctrl-Express-Xen-s-choice-of-MSR_SPEC_CTRL-.patch
Patch63: xsa263-4.10-0003-x86-spec_ctrl-Merge-bti_ist_info-and-use_shadow_spec.patch
Patch64: xsa263-4.10-0004-x86-spec_ctrl-Fold-the-XEN_IBRS_-SET-CLEAR-ALTERNATI.patch
Patch65: xsa263-4.10-0005-x86-spec_ctrl-Rename-bits-of-infrastructure-to-avoid.patch
Patch66: xsa263-4.10-0006-x86-spec_ctrl-Elide-MSR_SPEC_CTRL-handling-in-idle-c.patch
Patch67: xsa263-4.10-0007-x86-spec_ctrl-Split-X86_FEATURE_SC_MSR-into-PV-and-H.patch
Patch68: xsa263-4.10-0008-x86-spec_ctrl-Explicitly-set-Xen-s-default-MSR_SPEC_.patch
Patch69: xsa263-4.10-0009-x86-cpuid-Improvements-to-guest-policies-for-specula.patch
Patch70: xsa263-4.10-0010-x86-spec_ctrl-Introduce-a-new-spec-ctrl-command-line.patch
Patch71: xsa263-4.10-0011-x86-AMD-Mitigations-for-GPZ-SP4-Speculative-Store-By.patch
Patch72: xsa263-4.10-0012-x86-Intel-Mitigations-for-GPZ-SP4-Speculative-Store-.patch
Patch73: xsa263-4.10-0013-x86-msr-Virtualise-MSR_SPEC_CTRL.SSBD-for-guests-to-.patch
Patch74: xsa267-4.10-1.patch
Patch75: xsa267-4.10-2.patch
Patch76: xen.iasl.fix.patch
Patch77: xsa264-4.10.patch
Patch78: xsa265.patch
Patch79: xsa266-4.10-0001-libxl-qemu_disk_scsi_drive_string-Break-out-common-p.patch
Patch80: xsa266-4.10-0002-libxl-restore-passing-readonly-to-qemu-for-SCSI-disk.patch
Patch52: xsa284.patch
Patch53: xsa285-4.11.patch
Patch54: xsa287-4.10.patch
Patch55: xsa288-4.11.patch
Patch56: xsa290-4.10-1.patch
Patch57: xsa290-4.10-2.patch
Patch58: xsa291-4.11.patch
Patch59: xsa292.patch
Patch60: xsa293-4.10-1.patch
Patch61: xsa293-4.10-2.patch
Patch62: xsa294-4.10.patch
%if %build_qemutrad
@ -367,24 +350,6 @@ manage Xen virtual machines.
%patch60 -p1
%patch61 -p1
%patch62 -p1
%patch63 -p1
%patch64 -p1
%patch65 -p1
%patch66 -p1
%patch67 -p1
%patch68 -p1
%patch69 -p1
%patch70 -p1
%patch71 -p1
%patch72 -p1
%patch73 -p1
%patch74 -p1
%patch75 -p1
%patch76 -p1
%patch77 -p1
%patch78 -p1
%patch79 -p1
%patch80 -p1
# qemu-xen-traditional patches
pushd tools/qemu-xen-traditional
@ -540,6 +505,9 @@ rm -rf %{buildroot}/%{_libdir}/*.a
%if %build_efi
# clean up extra efi files
rm -rf %{buildroot}/%{_libdir}/efi
%ifarch %{ix86}
rm -rf %{buildroot}/usr/lib64/efi
%endif
%endif
%if ! %build_ocaml
@ -922,6 +890,55 @@ fi
%endif
%changelog
* Tue Mar 05 2019 Michael Young <m.a.young@durham.ac.uk> - 4.10.3-2
- xen: various flaws (#1685577)
grant table transfer issues on large hosts [XSA-284]
race with pass-through device hotplug [XSA-285]
x86: steal_page violates page_struct access discipline [XSA-287]
x86: Inconsistent PV IOMMU discipline [XSA-288]
missing preemption in x86 PV page table unvalidation [XSA-290]
x86/PV: page type reference counting issue with failed IOMMU update [XSA-291]
x86: insufficient TLB flushing when using PCID [XSA-292]
x86: PV kernel context switch corruption [XSA-293]
x86 shadow: Insufficient TLB flushing when using PCID [XSA-294]
* Tue Feb 26 2019 Michael Young <m.a.young@durham.ac.uk> - 4.10.3-1
- update to xen-4.10.3
adjust xen.use.fedora.ipxe.patch
drop most of xen.fedora.efi.build.patch and let efi build on i686
remove patches for issues now fixed upstream
* Tue Nov 20 2018 Michael Young <m.a.young@durham.ac.uk> - 4.10.2-4
- insufficient TLB flushing / improper large page mappings with AMD IOMMUs
[XSA-275, CVE-2018-19961, CVE-2018-19962] (#1651665)
- x86: DoS from attempting to use INVPCID with a non-canonical addresses
[XSA-279, CVE-2018-19965] (#1651970)
- Fix for XSA-240 conflicts with shadow paging [XSA-280, CVE-2018-19966]
(#1652251)
* Tue Nov 06 2018 Michael Young <m.a.young@durham.ac.uk> - 4.10.2-3
- guest use of HLE constructs may lock up host [XSA-282, CVE-2018-19967]
* Thu Oct 25 2018 Michael Young <m.a.young@durham.ac.uk> - 4.10.2-2
- x86: Nested VT-x usable even when disabled [XSA-278, CVE-2018-18883]
(#1643118)
* Tue Sep 25 2018 Michael Young <m.a.young@durham.ac.uk> - 4.10.2-1
- update to xen-4.10.2
adjust xen.use.fedora.ipxe.patch
remove patches for issues now fixed upstream
* Wed Aug 15 2018 Michael Young <m.a.young@durham.ac.uk> - 4.10.1-6
- L1 Terminal Fault speculative side channel patch bundle [XSA-273,
CVE-2018-3620, CVE-2018-3646]
drop patches also in the bundle, which also includes
Use of v2 grant tables may cause crash on ARM [XSA-268, CVE-2018-15469]
(#1616081)
x86: Incorrect MSR_DEBUGCTL handling lets guests enable BTS [XSA-269,
CVE-2018-15468] (#1616077)
oxenstored does not apply quota-maxentity [XSA-272, CVE-2018-15470]
(#1616080)
* Wed Jun 27 2018 Michael Young <m.a.young@durham.ac.uk> - 4.10.1-5
- preemption checks bypassed in x86 PV MM handling [XSA-264, CVE-2018-12891]
(#1595959)

View file

@ -19,7 +19,7 @@
+ETHERBOOT_NICS ?= 10ec8139 8086100e
QEMU_TRADITIONAL_REVISION ?= xen-4.10.1
QEMU_TRADITIONAL_REVISION ?= xen-4.10.3
--- xen-4.2.0/tools/firmware/Makefile.orig 2012-05-27 21:57:04.480812871 +0100
+++ xen-4.2.0/tools/firmware/Makefile 2012-06-02 19:03:52.254691484 +0100
@@ -10,7 +10,7 @@

View file

@ -1,72 +0,0 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/traps: Fix %dr6 handing in #DB handler
Most bits in %dr6 accumulate, rather than being set directly based on the
current source of #DB. Have the handler follow the manuals guidance, which
avoids leaking hypervisor debugging activities into guest context.
This is part of XSA-260 / CVE-2018-8897.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/traps.c 2018-04-13 15:29:36.006747135 +0200
+++ b/xen/arch/x86/traps.c 2018-04-13 15:44:57.015516185 +0200
@@ -1761,11 +1761,36 @@ static void ler_enable(void)
void do_debug(struct cpu_user_regs *regs)
{
+ unsigned long dr6;
struct vcpu *v = current;
+ /* Stash dr6 as early as possible. */
+ dr6 = read_debugreg(6);
+
if ( debugger_trap_entry(TRAP_debug, regs) )
return;
+ /*
+ * At the time of writing (March 2018), on the subject of %dr6:
+ *
+ * The Intel manual says:
+ * Certain debug exceptions may clear bits 0-3. The remaining contents
+ * of the DR6 register are never cleared by the processor. To avoid
+ * confusion in identifying debug exceptions, debug handlers should
+ * clear the register (except bit 16, which they should set) before
+ * returning to the interrupted task.
+ *
+ * The AMD manual says:
+ * Bits 15:13 of the DR6 register are not cleared by the processor and
+ * must be cleared by software after the contents have been read.
+ *
+ * Some bits are reserved set, some are reserved clear, and some bits
+ * which were previously reserved set are reused and cleared by hardware.
+ * For future compatibility, reset to the default value, which will allow
+ * us to spot any bit being changed by hardware to its non-default value.
+ */
+ write_debugreg(6, X86_DR6_DEFAULT);
+
if ( !guest_mode(regs) )
{
if ( regs->eflags & X86_EFLAGS_TF )
@@ -1798,7 +1823,8 @@ void do_debug(struct cpu_user_regs *regs
}
/* Save debug status register where guest OS can peek at it */
- v->arch.debugreg[6] = read_debugreg(6);
+ v->arch.debugreg[6] |= (dr6 & ~X86_DR6_DEFAULT);
+ v->arch.debugreg[6] &= (dr6 | ~X86_DR6_DEFAULT);
ler_enable();
pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
--- a/xen/include/asm-x86/debugreg.h 2015-02-11 09:36:29.000000000 +0100
+++ b/xen/include/asm-x86/debugreg.h 2018-04-13 15:44:57.015516185 +0200
@@ -24,6 +24,8 @@
#define DR_STATUS_RESERVED_ZERO (~0xffffeffful) /* Reserved, read as zero */
#define DR_STATUS_RESERVED_ONE 0xffff0ff0ul /* Reserved, read as one */
+#define X86_DR6_DEFAULT 0xffff0ff0ul /* Default %dr6 value. */
+
/* Now define a bunch of things for manipulating the control register.
The top two bytes of the control register consist of 4 fields of 4
bits - each field corresponds to one of the four debug registers,

View file

@ -1,110 +0,0 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/pv: Move exception injection into {,compat_}test_all_events()
This allows paths to jump straight to {,compat_}test_all_events() and have
injection of pending exceptions happen automatically, rather than requiring
all calling paths to handle exceptions themselves.
The normal exception path is simplified as a result, and
compat_post_handle_exception() is removed entirely.
This is part of XSA-260 / CVE-2018-8897.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -39,6 +39,12 @@ ENTRY(compat_test_all_events)
leaq irq_stat+IRQSTAT_softirq_pending(%rip),%rcx
cmpl $0,(%rcx,%rax,1)
jne compat_process_softirqs
+
+ /* Inject exception if pending. */
+ lea VCPU_trap_bounce(%rbx), %rdx
+ testb $TBF_EXCEPTION, TRAPBOUNCE_flags(%rdx)
+ jnz .Lcompat_process_trapbounce
+
testb $1,VCPU_mce_pending(%rbx)
jnz compat_process_mce
.Lcompat_test_guest_nmi:
@@ -68,6 +74,15 @@ compat_process_softirqs:
call do_softirq
jmp compat_test_all_events
+ ALIGN
+/* %rbx: struct vcpu, %rdx: struct trap_bounce */
+.Lcompat_process_trapbounce:
+ sti
+.Lcompat_bounce_exception:
+ call compat_create_bounce_frame
+ movb $0, TRAPBOUNCE_flags(%rdx)
+ jmp compat_test_all_events
+
ALIGN
/* %rbx: struct vcpu */
compat_process_mce:
@@ -189,15 +204,6 @@ ENTRY(cr4_pv32_restore)
xor %eax, %eax
ret
-/* %rdx: trap_bounce, %rbx: struct vcpu */
-ENTRY(compat_post_handle_exception)
- testb $TBF_EXCEPTION,TRAPBOUNCE_flags(%rdx)
- jz compat_test_all_events
-.Lcompat_bounce_exception:
- call compat_create_bounce_frame
- movb $0,TRAPBOUNCE_flags(%rdx)
- jmp compat_test_all_events
-
.section .text.entry, "ax", @progbits
/* See lstar_enter for entry register state. */
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -42,6 +42,12 @@ test_all_events:
leaq irq_stat+IRQSTAT_softirq_pending(%rip), %rcx
cmpl $0, (%rcx, %rax, 1)
jne process_softirqs
+
+ /* Inject exception if pending. */
+ lea VCPU_trap_bounce(%rbx), %rdx
+ testb $TBF_EXCEPTION, TRAPBOUNCE_flags(%rdx)
+ jnz .Lprocess_trapbounce
+
cmpb $0, VCPU_mce_pending(%rbx)
jne process_mce
.Ltest_guest_nmi:
@@ -70,6 +76,15 @@ process_softirqs:
jmp test_all_events
ALIGN
+/* %rbx: struct vcpu, %rdx struct trap_bounce */
+.Lprocess_trapbounce:
+ sti
+.Lbounce_exception:
+ call create_bounce_frame
+ movb $0, TRAPBOUNCE_flags(%rdx)
+ jmp test_all_events
+
+ ALIGN
/* %rbx: struct vcpu */
process_mce:
testb $1 << VCPU_TRAP_MCE, VCPU_async_exception_mask(%rbx)
@@ -667,15 +682,9 @@ handle_exception_saved:
mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14)
testb $3,UREGS_cs(%rsp)
jz restore_all_xen
- leaq VCPU_trap_bounce(%rbx),%rdx
movq VCPU_domain(%rbx),%rax
testb $1,DOMAIN_is_32bit_pv(%rax)
- jnz compat_post_handle_exception
- testb $TBF_EXCEPTION,TRAPBOUNCE_flags(%rdx)
- jz test_all_events
-.Lbounce_exception:
- call create_bounce_frame
- movb $0,TRAPBOUNCE_flags(%rdx)
+ jnz compat_test_all_events
jmp test_all_events
/* No special register assumptions. */

View file

@ -1,138 +0,0 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/traps: Use an Interrupt Stack Table for #DB
PV guests can use architectural corner cases to cause #DB to be raised after
transitioning into supervisor mode.
Use an interrupt stack table for #DB to prevent the exception being taken with
a guest controlled stack pointer.
This is part of XSA-260 / CVE-2018-8897.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -679,6 +679,7 @@ void load_system_tables(void)
[IST_MCE - 1] = stack_top + IST_MCE * PAGE_SIZE,
[IST_DF - 1] = stack_top + IST_DF * PAGE_SIZE,
[IST_NMI - 1] = stack_top + IST_NMI * PAGE_SIZE,
+ [IST_DB - 1] = stack_top + IST_DB * PAGE_SIZE,
[IST_MAX ... ARRAY_SIZE(tss->ist) - 1] =
0x8600111111111111ul,
@@ -706,6 +707,7 @@ void load_system_tables(void)
set_ist(&idt_tables[cpu][TRAP_double_fault], IST_DF);
set_ist(&idt_tables[cpu][TRAP_nmi], IST_NMI);
set_ist(&idt_tables[cpu][TRAP_machine_check], IST_MCE);
+ set_ist(&idt_tables[cpu][TRAP_debug], IST_DB);
/*
* Bottom-of-stack must be 16-byte aligned!
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1046,6 +1046,7 @@ static void svm_ctxt_switch_from(struct
set_ist(&idt_tables[cpu][TRAP_double_fault], IST_DF);
set_ist(&idt_tables[cpu][TRAP_nmi], IST_NMI);
set_ist(&idt_tables[cpu][TRAP_machine_check], IST_MCE);
+ set_ist(&idt_tables[cpu][TRAP_debug], IST_DB);
}
static void svm_ctxt_switch_to(struct vcpu *v)
@@ -1067,6 +1068,7 @@ static void svm_ctxt_switch_to(struct vc
set_ist(&idt_tables[cpu][TRAP_double_fault], IST_NONE);
set_ist(&idt_tables[cpu][TRAP_nmi], IST_NONE);
set_ist(&idt_tables[cpu][TRAP_machine_check], IST_NONE);
+ set_ist(&idt_tables[cpu][TRAP_debug], IST_NONE);
svm_restore_dr(v);
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -964,6 +964,7 @@ static int cpu_smpboot_alloc(unsigned in
set_ist(&idt_tables[cpu][TRAP_double_fault], IST_NONE);
set_ist(&idt_tables[cpu][TRAP_nmi], IST_NONE);
set_ist(&idt_tables[cpu][TRAP_machine_check], IST_NONE);
+ set_ist(&idt_tables[cpu][TRAP_debug], IST_NONE);
for ( stub_page = 0, i = cpu & ~(STUBS_PER_PAGE - 1);
i < nr_cpu_ids && i <= (cpu | (STUBS_PER_PAGE - 1)); ++i )
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -325,13 +325,13 @@ static void show_guest_stack(struct vcpu
/*
* Notes for get_stack_trace_bottom() and get_stack_dump_bottom()
*
- * Stack pages 0, 1 and 2:
+ * Stack pages 0 - 3:
* These are all 1-page IST stacks. Each of these stacks have an exception
* frame and saved register state at the top. The interesting bound for a
* trace is the word adjacent to this, while the bound for a dump is the
* very top, including the exception frame.
*
- * Stack pages 3, 4 and 5:
+ * Stack pages 4 and 5:
* None of these are particularly interesting. With MEMORY_GUARD, page 5 is
* explicitly not present, so attempting to dump or trace it is
* counterproductive. Without MEMORY_GUARD, it is possible for a call chain
@@ -352,12 +352,12 @@ unsigned long get_stack_trace_bottom(uns
{
switch ( get_stack_page(sp) )
{
- case 0 ... 2:
+ case 0 ... 3:
return ROUNDUP(sp, PAGE_SIZE) -
offsetof(struct cpu_user_regs, es) - sizeof(unsigned long);
#ifndef MEMORY_GUARD
- case 3 ... 5:
+ case 4 ... 5:
#endif
case 6 ... 7:
return ROUNDUP(sp, STACK_SIZE) -
@@ -372,11 +372,11 @@ unsigned long get_stack_dump_bottom(unsi
{
switch ( get_stack_page(sp) )
{
- case 0 ... 2:
+ case 0 ... 3:
return ROUNDUP(sp, PAGE_SIZE) - sizeof(unsigned long);
#ifndef MEMORY_GUARD
- case 3 ... 5:
+ case 4 ... 5:
#endif
case 6 ... 7:
return ROUNDUP(sp, STACK_SIZE) - sizeof(unsigned long);
@@ -1943,6 +1943,7 @@ void __init init_idt_traps(void)
set_ist(&idt_table[TRAP_double_fault], IST_DF);
set_ist(&idt_table[TRAP_nmi], IST_NMI);
set_ist(&idt_table[TRAP_machine_check], IST_MCE);
+ set_ist(&idt_table[TRAP_debug], IST_DB);
/* CPU0 uses the master IDT. */
idt_tables[0] = idt_table;
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -739,7 +739,7 @@ ENTRY(device_not_available)
ENTRY(debug)
pushq $0
movl $TRAP_debug,4(%rsp)
- jmp handle_exception
+ jmp handle_ist_exception
ENTRY(int3)
pushq $0
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -443,7 +443,8 @@ struct __packed __cacheline_aligned tss_
#define IST_DF 1UL
#define IST_NMI 2UL
#define IST_MCE 3UL
-#define IST_MAX 3UL
+#define IST_DB 4UL
+#define IST_MAX 4UL
/* Set the interrupt stack table used by a particular interrupt
* descriptor table entry. */

View file

@ -1,72 +0,0 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/traps: Fix handling of #DB exceptions in hypervisor context
The WARN_ON() can be triggered by guest activities, and emits a full stack
trace without rate limiting. Swap it out for a ratelimited printk with just
enough information to work out what is going on.
Not all #DB exceptions are traps, so blindly continuing is not a safe action
to take. We don't let PV guests select these settings in the real %dr7 to
begin with, but for added safety against unexpected situations, detect the
fault cases and crash in an obvious manner.
This is part of XSA-260 / CVE-2018-8897.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1809,16 +1809,44 @@ void do_debug(struct cpu_user_regs *regs
regs->eflags &= ~X86_EFLAGS_TF;
}
}
- else
+
+ /*
+ * Check for fault conditions. General Detect, and instruction
+ * breakpoints are faults rather than traps, at which point attempting
+ * to ignore and continue will result in a livelock.
+ */
+ if ( dr6 & DR_GENERAL_DETECT )
+ {
+ printk(XENLOG_ERR "Hit General Detect in Xen context\n");
+ fatal_trap(regs, 0);
+ }
+
+ if ( dr6 & (DR_TRAP3 | DR_TRAP2 | DR_TRAP1 | DR_TRAP0) )
{
- /*
- * We ignore watchpoints when they trigger within Xen. This may
- * happen when a buffer is passed to us which previously had a
- * watchpoint set on it. No need to bump EIP; the only faulting
- * trap is an instruction breakpoint, which can't happen to us.
- */
- WARN_ON(!search_exception_table(regs));
+ unsigned int bp, dr7 = read_debugreg(7) >> DR_CONTROL_SHIFT;
+
+ for ( bp = 0; bp < 4; ++bp )
+ {
+ if ( (dr6 & (1u << bp)) && /* Breakpoint triggered? */
+ ((dr7 & (3u << (bp * DR_CONTROL_SIZE))) == 0) /* Insn? */ )
+ {
+ printk(XENLOG_ERR
+ "Hit instruction breakpoint in Xen context\n");
+ fatal_trap(regs, 0);
+ }
+ }
}
+
+ /*
+ * Whatever caused this #DB should be a trap. Note it and continue.
+ * Guests can trigger this in certain corner cases, so ensure the
+ * message is ratelimited.
+ */
+ gprintk(XENLOG_WARNING,
+ "Hit #DB in Xen context: %04x:%p [%ps], stk %04x:%p, dr6 %lx\n",
+ regs->cs, _p(regs->rip), _p(regs->rip),
+ regs->ss, _p(regs->rsp), dr6);
+
goto out;
}

View file

@ -1,279 +0,0 @@
From: Xen Project Security Team <security@xenproject.org>
Subject: x86/vpt: add support for IO-APIC routed interrupts
And modify the HPET code to make use of it. Currently HPET interrupts
are always treated as ISA and thus injected through the vPIC. This is
wrong because HPET interrupts when not in legacy mode should be
injected from the IO-APIC.
To make things worse, the supported interrupt routing values are set
to [20..23], which clearly falls outside of the ISA range, thus
leading to an ASSERT in debug builds or memory corruption in non-debug
builds because the interrupt injection code will write out of the
bounds of the arch.hvm_domain.vpic array.
Since the HPET interrupt source can change between ISA and IO-APIC
always destroy the timer before changing the mode, or else Xen risks
changing it while the timer is active.
Note that vpt interrupt injection is racy in the sense that the
vIO-APIC RTE entry can be written by the guest in between the call to
pt_irq_masked and hvm_ioapic_assert, or the call to pt_update_irq and
pt_intr_post. Those are not deemed to be security issues, but rather
quirks of the current implementation. In the worse case the guest
might lose interrupts or get multiple interrupt vectors injected for
the same timer source.
This is part of XSA-261.
Address actual and potential compiler warnings. Fix formatting.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v2:
- Move fallthrough comment to be just above the case label.
- Fix now stale comment in pt_update_irq.
- Use NR_ISAIRQS instead of 16.
- Expand commit message to mention the quirkiness of vpt interrupt
injection.
Changes since v1:
- Simply usage of gsi in pt_irq_masked.
- Introduce hvm_ioapic_assert.
- Fix pt->source == PTSRC_isa in create_periodic_time.
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -264,13 +264,20 @@ static void hpet_set_timer(HPETState *h,
diff = (timer_is_32bit(h, tn) && (-diff > HPET_TINY_TIME_SPAN))
? (uint32_t)diff : 0;
+ destroy_periodic_time(&h->pt[tn]);
if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
+ {
/* if LegacyReplacementRoute bit is set, HPET specification requires
timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC. */
irq = (tn == 0) ? 0 : 8;
+ h->pt[tn].source = PTSRC_isa;
+ }
else
+ {
irq = timer_int_route(h, tn);
+ h->pt[tn].source = PTSRC_ioapic;
+ }
/*
* diff is the time from now when the timer should fire, for a periodic
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -41,6 +41,26 @@ static void assert_gsi(struct domain *d,
vioapic_irq_positive_edge(d, ioapic_gsi);
}
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level)
+{
+ struct hvm_irq *hvm_irq = hvm_domain_irq(d);
+ int vector;
+
+ if ( gsi >= hvm_irq->nr_gsis )
+ {
+ ASSERT_UNREACHABLE();
+ return -1;
+ }
+
+ spin_lock(&d->arch.hvm_domain.irq_lock);
+ if ( !level || hvm_irq->gsi_assert_count[gsi]++ == 0 )
+ assert_gsi(d, gsi);
+ vector = vioapic_get_vector(d, gsi);
+ spin_unlock(&d->arch.hvm_domain.irq_lock);
+
+ return vector;
+}
+
static void assert_irq(struct domain *d, unsigned ioapic_gsi, unsigned pic_irq)
{
assert_gsi(d, ioapic_gsi);
--- a/xen/arch/x86/hvm/vpt.c
+++ b/xen/arch/x86/hvm/vpt.c
@@ -107,31 +107,49 @@ static int pt_irq_vector(struct periodic
static int pt_irq_masked(struct periodic_time *pt)
{
struct vcpu *v = pt->vcpu;
- unsigned int gsi, isa_irq;
- int mask;
- uint8_t pic_imr;
+ unsigned int gsi = pt->irq;
- if ( pt->source == PTSRC_lapic )
+ switch ( pt->source )
+ {
+ case PTSRC_lapic:
{
struct vlapic *vlapic = vcpu_vlapic(v);
+
return (!vlapic_enabled(vlapic) ||
(vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_MASKED));
}
- isa_irq = pt->irq;
- gsi = hvm_isa_irq_to_gsi(isa_irq);
- pic_imr = v->domain->arch.hvm_domain.vpic[isa_irq >> 3].imr;
- mask = vioapic_get_mask(v->domain, gsi);
- if ( mask < 0 )
- {
- dprintk(XENLOG_WARNING, "d%u: invalid GSI (%u) for platform timer\n",
- v->domain->domain_id, gsi);
- domain_crash(v->domain);
- return -1;
+ case PTSRC_isa:
+ {
+ uint8_t pic_imr = v->domain->arch.hvm_domain.vpic[pt->irq >> 3].imr;
+
+ /* Check if the interrupt is unmasked in the PIC. */
+ if ( !(pic_imr & (1 << (pt->irq & 7))) && vlapic_accept_pic_intr(v) )
+ return 0;
+
+ gsi = hvm_isa_irq_to_gsi(pt->irq);
+ }
+
+ /* Fallthrough to check if the interrupt is masked on the IO APIC. */
+ case PTSRC_ioapic:
+ {
+ int mask = vioapic_get_mask(v->domain, gsi);
+
+ if ( mask < 0 )
+ {
+ dprintk(XENLOG_WARNING,
+ "d%d: invalid GSI (%u) for platform timer\n",
+ v->domain->domain_id, gsi);
+ domain_crash(v->domain);
+ return -1;
+ }
+
+ return mask;
+ }
}
- return (((pic_imr & (1 << (isa_irq & 7))) || !vlapic_accept_pic_intr(v)) &&
- mask);
+ ASSERT_UNREACHABLE();
+ return 1;
}
static void pt_lock(struct periodic_time *pt)
@@ -252,7 +270,7 @@ int pt_update_irq(struct vcpu *v)
struct list_head *head = &v->arch.hvm_vcpu.tm_list;
struct periodic_time *pt, *temp, *earliest_pt;
uint64_t max_lag;
- int irq, is_lapic, pt_vector;
+ int irq, pt_vector = -1;
spin_lock(&v->arch.hvm_vcpu.tm_lock);
@@ -288,29 +306,26 @@ int pt_update_irq(struct vcpu *v)
earliest_pt->irq_issued = 1;
irq = earliest_pt->irq;
- is_lapic = (earliest_pt->source == PTSRC_lapic);
spin_unlock(&v->arch.hvm_vcpu.tm_lock);
- /*
- * If periodic timer interrut is handled by lapic, its vector in
- * IRR is returned and used to set eoi_exit_bitmap for virtual
- * interrupt delivery case. Otherwise return -1 to do nothing.
- */
- if ( is_lapic )
+ switch ( earliest_pt->source )
{
+ case PTSRC_lapic:
+ /*
+ * If periodic timer interrupt is handled by lapic, its vector in
+ * IRR is returned and used to set eoi_exit_bitmap for virtual
+ * interrupt delivery case. Otherwise return -1 to do nothing.
+ */
vlapic_set_irq(vcpu_vlapic(v), irq, 0);
pt_vector = irq;
- }
- else
- {
+ break;
+
+ case PTSRC_isa:
hvm_isa_irq_deassert(v->domain, irq);
if ( platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
v->domain->arch.hvm_domain.vpic[irq >> 3].int_output )
- {
hvm_isa_irq_assert(v->domain, irq, NULL);
- pt_vector = -1;
- }
else
{
pt_vector = hvm_isa_irq_assert(v->domain, irq, vioapic_get_vector);
@@ -321,6 +336,17 @@ int pt_update_irq(struct vcpu *v)
if ( pt_vector < 0 || !vlapic_test_irq(vcpu_vlapic(v), pt_vector) )
pt_vector = -1;
}
+ break;
+
+ case PTSRC_ioapic:
+ /*
+ * NB: At the moment IO-APIC routed interrupts generated by vpt devices
+ * (HPET) are edge-triggered.
+ */
+ pt_vector = hvm_ioapic_assert(v->domain, irq, false);
+ if ( pt_vector < 0 || !vlapic_test_irq(vcpu_vlapic(v), pt_vector) )
+ pt_vector = -1;
+ break;
}
return pt_vector;
@@ -418,7 +444,14 @@ void create_periodic_time(
struct vcpu *v, struct periodic_time *pt, uint64_t delta,
uint64_t period, uint8_t irq, time_cb *cb, void *data)
{
- ASSERT(pt->source != 0);
+ if ( !pt->source ||
+ (pt->irq >= NR_ISAIRQS && pt->source == PTSRC_isa) ||
+ (pt->irq >= hvm_domain_irq(v->domain)->nr_gsis &&
+ pt->source == PTSRC_ioapic) )
+ {
+ ASSERT_UNREACHABLE();
+ return;
+ }
destroy_periodic_time(pt);
@@ -498,7 +531,7 @@ static void pt_adjust_vcpu(struct period
{
int on_list;
- ASSERT(pt->source == PTSRC_isa);
+ ASSERT(pt->source == PTSRC_isa || pt->source == PTSRC_ioapic);
if ( pt->vcpu == NULL )
return;
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -207,6 +207,9 @@ int hvm_set_pci_link_route(struct domain
int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data);
+/* Assert an IO APIC pin. */
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level);
+
void hvm_maybe_deassert_evtchn_irq(void);
void hvm_assert_evtchn_irq(struct vcpu *v);
void hvm_set_callback_via(struct domain *d, uint64_t via);
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -44,6 +44,7 @@ struct periodic_time {
bool_t warned_timeout_too_short;
#define PTSRC_isa 1 /* ISA time source */
#define PTSRC_lapic 2 /* LAPIC time source */
+#define PTSRC_ioapic 3 /* IOAPIC time source */
u8 source; /* PTSRC_ */
u8 irq;
struct vcpu *vcpu; /* vcpu timer interrupt delivers to */

View file

@ -1,76 +0,0 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/HVM: guard against emulator driving ioreq state in weird ways
In the case where hvm_wait_for_io() calls wait_on_xen_event_channel(),
p->state ends up being read twice in succession: once to determine that
state != p->state, and then again at the top of the loop. This gives a
compromised emulator a chance to change the state back between the two
reads, potentially keeping Xen in a loop indefinitely.
Instead:
* Read p->state once in each of the wait_on_xen_event_channel() tests,
* re-use that value the next time around,
* and insist that the states continue to transition "forward" (with the
exception of the transition to STATE_IOREQ_NONE).
This is XSA-262.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -87,14 +87,17 @@ static void hvm_io_assist(struct hvm_ior
static bool hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p)
{
+ unsigned int prev_state = STATE_IOREQ_NONE;
+
while ( sv->pending )
{
unsigned int state = p->state;
- rmb();
- switch ( state )
+ smp_rmb();
+
+ recheck:
+ if ( unlikely(state == STATE_IOREQ_NONE) )
{
- case STATE_IOREQ_NONE:
/*
* The only reason we should see this case is when an
* emulator is dying and it races with an I/O being
@@ -102,14 +105,30 @@ static bool hvm_wait_for_io(struct hvm_i
*/
hvm_io_assist(sv, ~0ul);
break;
+ }
+
+ if ( unlikely(state < prev_state) )
+ {
+ gdprintk(XENLOG_ERR, "Weird HVM ioreq state transition %u -> %u\n",
+ prev_state, state);
+ sv->pending = false;
+ domain_crash(sv->vcpu->domain);
+ return false; /* bail */
+ }
+
+ switch ( prev_state = state )
+ {
case STATE_IORESP_READY: /* IORESP_READY -> NONE */
p->state = STATE_IOREQ_NONE;
hvm_io_assist(sv, p->data);
break;
case STATE_IOREQ_READY: /* IOREQ_{READY,INPROCESS} -> IORESP_READY */
case STATE_IOREQ_INPROCESS:
- wait_on_xen_event_channel(sv->ioreq_evtchn, p->state != state);
- break;
+ wait_on_xen_event_channel(sv->ioreq_evtchn,
+ ({ state = p->state;
+ smp_rmb();
+ state != prev_state; }));
+ goto recheck;
default:
gdprintk(XENLOG_ERR, "Weird HVM iorequest state %u\n", state);
sv->pending = false;

View file

@ -1,110 +0,0 @@
From 13fafdf5c97d3bc2a8851c4d1796feac0f82d498 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Thu, 26 Apr 2018 12:21:00 +0100
Subject: [PATCH] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once
Make it available from the beginning of init_speculation_mitigations(), and
pass it into appropriate functions. Fix an RSBA typo while moving the
affected comment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit d6c65187252a6c1810fd24c4d46f812840de8d3c)
---
xen/arch/x86/spec_ctrl.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index fa67a0f..dc90743 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -81,18 +81,15 @@ static int __init parse_bti(const char *s)
}
custom_param("bti", parse_bti);
-static void __init print_details(enum ind_thunk thunk)
+static void __init print_details(enum ind_thunk thunk, uint64_t caps)
{
unsigned int _7d0 = 0, e8b = 0, tmp;
- uint64_t caps = 0;
/* Collect diagnostics about available mitigations. */
if ( boot_cpu_data.cpuid_level >= 7 )
cpuid_count(7, 0, &tmp, &tmp, &tmp, &_7d0);
if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
- if ( _7d0 & cpufeat_mask(X86_FEATURE_ARCH_CAPS) )
- rdmsrl(MSR_ARCH_CAPABILITIES, caps);
printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
@@ -125,7 +122,7 @@ static void __init print_details(enum ind_thunk thunk)
}
/* Calculate whether Retpoline is known-safe on this CPU. */
-static bool __init retpoline_safe(void)
+static bool __init retpoline_safe(uint64_t caps)
{
unsigned int ucode_rev = this_cpu(ucode_cpu_info).cpu_sig.rev;
@@ -136,19 +133,12 @@ static bool __init retpoline_safe(void)
boot_cpu_data.x86 != 6 )
return false;
- if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
- {
- uint64_t caps;
-
- rdmsrl(MSR_ARCH_CAPABILITIES, caps);
-
- /*
- * RBSA may be set by a hypervisor to indicate that we may move to a
- * processor which isn't retpoline-safe.
- */
- if ( caps & ARCH_CAPS_RSBA )
- return false;
- }
+ /*
+ * RSBA may be set by a hypervisor to indicate that we may move to a
+ * processor which isn't retpoline-safe.
+ */
+ if ( caps & ARCH_CAPS_RSBA )
+ return false;
switch ( boot_cpu_data.x86_model )
{
@@ -218,6 +208,10 @@ void __init init_speculation_mitigations(void)
{
enum ind_thunk thunk = THUNK_DEFAULT;
bool ibrs = false;
+ uint64_t caps = 0;
+
+ if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
+ rdmsrl(MSR_ARCH_CAPABILITIES, caps);
/*
* Has the user specified any custom BTI mitigations? If so, follow their
@@ -246,7 +240,7 @@ void __init init_speculation_mitigations(void)
* On Intel hardware, we'd like to use retpoline in preference to
* IBRS, but only if it is safe on this hardware.
*/
- else if ( retpoline_safe() )
+ else if ( retpoline_safe(caps) )
thunk = THUNK_RETPOLINE;
else if ( boot_cpu_has(X86_FEATURE_IBRSB) )
ibrs = true;
@@ -331,7 +325,7 @@ void __init init_speculation_mitigations(void)
/* (Re)init BSP state now that default_bti_ist_info has been calculated. */
init_shadow_spec_ctrl_state();
- print_details(thunk);
+ print_details(thunk, caps);
}
static void __init __maybe_unused build_assertions(void)
--
2.1.4

View file

@ -1,138 +0,0 @@
From d7b345e4ca136a995bfaaf2ee20901ee20e63570 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 17 Apr 2018 14:15:04 +0100
Subject: [PATCH] x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as
a variable
At the moment, we have two different encodings of Xen's MSR_SPEC_CTRL value,
which is a side effect of how the Spectre series developed. One encoding is
via an alias with the bottom bit of bti_ist_info, and can encode IBRS or not,
but not other configurations such as STIBP.
Break Xen's value out into a separate variable (in the top of stack block for
XPTI reasons) and use this instead of bti_ist_info in the IST path.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit 66dfae0f32bfbc899c2f3446d5ee57068cb7f957)
---
xen/arch/x86/spec_ctrl.c | 8 +++++---
xen/arch/x86/x86_64/asm-offsets.c | 1 +
xen/include/asm-x86/current.h | 1 +
xen/include/asm-x86/spec_ctrl.h | 2 ++
xen/include/asm-x86/spec_ctrl_asm.h | 8 ++------
5 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index dc90743..1143521 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -38,6 +38,7 @@ static int8_t __initdata opt_ibrs = -1;
static bool __initdata opt_rsb_native = true;
static bool __initdata opt_rsb_vmexit = true;
bool __read_mostly opt_ibpb = true;
+uint8_t __read_mostly default_xen_spec_ctrl;
uint8_t __read_mostly default_bti_ist_info;
static int __init parse_bti(const char *s)
@@ -285,11 +286,14 @@ void __init init_speculation_mitigations(void)
* guests.
*/
if ( ibrs )
+ {
+ default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_SET);
+ }
else
setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_CLEAR);
- default_bti_ist_info |= BTI_IST_WRMSR | ibrs;
+ default_bti_ist_info |= BTI_IST_WRMSR;
}
/*
@@ -330,8 +334,6 @@ void __init init_speculation_mitigations(void)
static void __init __maybe_unused build_assertions(void)
{
- /* The optimised assembly relies on this alias. */
- BUILD_BUG_ON(BTI_IST_IBRS != SPEC_CTRL_IBRS);
}
/*
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index 13478d4..0726147 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -142,6 +142,7 @@ void __dummy__(void)
OFFSET(CPUINFO_xen_cr3, struct cpu_info, xen_cr3);
OFFSET(CPUINFO_pv_cr3, struct cpu_info, pv_cr3);
OFFSET(CPUINFO_shadow_spec_ctrl, struct cpu_info, shadow_spec_ctrl);
+ OFFSET(CPUINFO_xen_spec_ctrl, struct cpu_info, xen_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);
DEFINE(CPUINFO_sizeof, sizeof(struct cpu_info));
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 4678a0f..d10b13c 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -56,6 +56,7 @@ struct cpu_info {
/* See asm-x86/spec_ctrl_asm.h for usage. */
unsigned int shadow_spec_ctrl;
+ uint8_t xen_spec_ctrl;
bool use_shadow_spec_ctrl;
uint8_t bti_ist_info;
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 5ab4ff3..5e4fc84 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -27,6 +27,7 @@
void init_speculation_mitigations(void);
extern bool opt_ibpb;
+extern uint8_t default_xen_spec_ctrl;
extern uint8_t default_bti_ist_info;
static inline void init_shadow_spec_ctrl_state(void)
@@ -34,6 +35,7 @@ static inline void init_shadow_spec_ctrl_state(void)
struct cpu_info *info = get_cpu_info();
info->shadow_spec_ctrl = info->use_shadow_spec_ctrl = 0;
+ info->xen_spec_ctrl = default_xen_spec_ctrl;
info->bti_ist_info = default_bti_ist_info;
}
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 1f2b6f3..697da13 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -21,7 +21,6 @@
#define __X86_SPEC_CTRL_ASM_H__
/* Encoding of the bottom bits in cpuinfo.bti_ist_info */
-#define BTI_IST_IBRS (1 << 0)
#define BTI_IST_WRMSR (1 << 1)
#define BTI_IST_RSB (1 << 2)
@@ -286,12 +285,9 @@
setz %dl
and %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
- /*
- * Load Xen's intended value. SPEC_CTRL_IBRS vs 0 is encoded in the
- * bottom bit of bti_ist_info, via a deliberate alias with BTI_IST_IBRS.
- */
+ /* Load Xen's intended value. */
mov $MSR_SPEC_CTRL, %ecx
- and $BTI_IST_IBRS, %eax
+ movzbl STACK_CPUINFO_FIELD(xen_spec_ctrl)(%r14), %eax
xor %edx, %edx
wrmsr
--
2.1.4

View file

@ -1,340 +0,0 @@
From a0c2f734b4c683cb407e10ff943671c413480287 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 17 Apr 2018 14:15:04 +0100
Subject: [PATCH] x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl
into spec_ctrl_flags
All 3 bits of information here are control flags for the entry/exit code
behaviour. Treat them as such, rather than having two different variables.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit 5262ba2e7799001402dfe139ff944e035dfff928)
---
xen/arch/x86/acpi/power.c | 4 +--
xen/arch/x86/spec_ctrl.c | 10 ++++---
xen/arch/x86/x86_64/asm-offsets.c | 3 +--
xen/include/asm-x86/current.h | 3 +--
xen/include/asm-x86/nops.h | 5 ++--
xen/include/asm-x86/spec_ctrl.h | 10 +++----
xen/include/asm-x86/spec_ctrl_asm.h | 52 ++++++++++++++++++++-----------------
7 files changed, 45 insertions(+), 42 deletions(-)
diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index f7085d3..f3480aa 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -215,7 +215,7 @@ static int enter_state(u32 state)
ci = get_cpu_info();
spec_ctrl_enter_idle(ci);
/* Avoid NMI/#MC using MSR_SPEC_CTRL until we've reloaded microcode. */
- ci->bti_ist_info = 0;
+ ci->spec_ctrl_flags &= ~SCF_ist_wrmsr;
ACPI_FLUSH_CPU_CACHE();
@@ -256,7 +256,7 @@ static int enter_state(u32 state)
microcode_resume_cpu(0);
/* Re-enabled default NMI/#MC use of MSR_SPEC_CTRL. */
- ci->bti_ist_info = default_bti_ist_info;
+ ci->spec_ctrl_flags |= (default_spec_ctrl_flags & SCF_ist_wrmsr);
spec_ctrl_exit_idle(ci);
done:
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 1143521..2d69910 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -39,7 +39,7 @@ static bool __initdata opt_rsb_native = true;
static bool __initdata opt_rsb_vmexit = true;
bool __read_mostly opt_ibpb = true;
uint8_t __read_mostly default_xen_spec_ctrl;
-uint8_t __read_mostly default_bti_ist_info;
+uint8_t __read_mostly default_spec_ctrl_flags;
static int __init parse_bti(const char *s)
{
@@ -293,7 +293,7 @@ void __init init_speculation_mitigations(void)
else
setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_CLEAR);
- default_bti_ist_info |= BTI_IST_WRMSR;
+ default_spec_ctrl_flags |= SCF_ist_wrmsr;
}
/*
@@ -312,7 +312,7 @@ void __init init_speculation_mitigations(void)
if ( opt_rsb_native )
{
setup_force_cpu_cap(X86_FEATURE_RSB_NATIVE);
- default_bti_ist_info |= BTI_IST_RSB;
+ default_spec_ctrl_flags |= SCF_ist_rsb;
}
/*
@@ -326,7 +326,7 @@ void __init init_speculation_mitigations(void)
if ( !boot_cpu_has(X86_FEATURE_IBRSB) && !boot_cpu_has(X86_FEATURE_IBPB) )
opt_ibpb = false;
- /* (Re)init BSP state now that default_bti_ist_info has been calculated. */
+ /* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
init_shadow_spec_ctrl_state();
print_details(thunk, caps);
@@ -334,6 +334,8 @@ void __init init_speculation_mitigations(void)
static void __init __maybe_unused build_assertions(void)
{
+ /* The optimised assembly relies on this alias. */
+ BUILD_BUG_ON(SCF_use_shadow != 1);
}
/*
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index 0726147..97242e5 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -143,8 +143,7 @@ void __dummy__(void)
OFFSET(CPUINFO_pv_cr3, struct cpu_info, pv_cr3);
OFFSET(CPUINFO_shadow_spec_ctrl, struct cpu_info, shadow_spec_ctrl);
OFFSET(CPUINFO_xen_spec_ctrl, struct cpu_info, xen_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_spec_ctrl_flags, struct cpu_info, spec_ctrl_flags);
DEFINE(CPUINFO_sizeof, sizeof(struct cpu_info));
BLANK();
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index d10b13c..7afff0e 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -57,8 +57,7 @@ struct cpu_info {
/* See asm-x86/spec_ctrl_asm.h for usage. */
unsigned int shadow_spec_ctrl;
uint8_t xen_spec_ctrl;
- bool use_shadow_spec_ctrl;
- uint8_t bti_ist_info;
+ uint8_t spec_ctrl_flags;
unsigned long __pad;
/* get_stack_bottom() must be 16-byte aligned */
diff --git a/xen/include/asm-x86/nops.h b/xen/include/asm-x86/nops.h
index 37f9819..b744895 100644
--- a/xen/include/asm-x86/nops.h
+++ b/xen/include/asm-x86/nops.h
@@ -62,10 +62,9 @@
#define ASM_NOP8 _ASM_MK_NOP(K8_NOP8)
#define ASM_NOP17 ASM_NOP8; ASM_NOP7; ASM_NOP2
-#define ASM_NOP21 ASM_NOP8; ASM_NOP8; ASM_NOP5
+#define ASM_NOP22 ASM_NOP8; ASM_NOP8; ASM_NOP6
#define ASM_NOP24 ASM_NOP8; ASM_NOP8; ASM_NOP8
-#define ASM_NOP29 ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP5
-#define ASM_NOP32 ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP8
+#define ASM_NOP33 ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP7; ASM_NOP2
#define ASM_NOP40 ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP8
#define ASM_NOP_MAX 8
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 5e4fc84..059e291 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -28,15 +28,15 @@ void init_speculation_mitigations(void);
extern bool opt_ibpb;
extern uint8_t default_xen_spec_ctrl;
-extern uint8_t default_bti_ist_info;
+extern uint8_t default_spec_ctrl_flags;
static inline void init_shadow_spec_ctrl_state(void)
{
struct cpu_info *info = get_cpu_info();
- info->shadow_spec_ctrl = info->use_shadow_spec_ctrl = 0;
+ info->shadow_spec_ctrl = 0;
info->xen_spec_ctrl = default_xen_spec_ctrl;
- info->bti_ist_info = default_bti_ist_info;
+ info->spec_ctrl_flags = default_spec_ctrl_flags;
}
/* WARNING! `ret`, `call *`, `jmp *` not safe after this call. */
@@ -50,7 +50,7 @@ static always_inline void spec_ctrl_enter_idle(struct cpu_info *info)
*/
info->shadow_spec_ctrl = val;
barrier();
- info->use_shadow_spec_ctrl = true;
+ info->spec_ctrl_flags |= SCF_use_shadow;
barrier();
asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
@@ -65,7 +65,7 @@ static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
* Disable shadowing before updating the MSR. There are no SMP issues
* here; only local processor ordering concerns.
*/
- info->use_shadow_spec_ctrl = false;
+ info->spec_ctrl_flags &= ~SCF_use_shadow;
barrier();
asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 697da13..39fb4f8 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -20,9 +20,10 @@
#ifndef __X86_SPEC_CTRL_ASM_H__
#define __X86_SPEC_CTRL_ASM_H__
-/* Encoding of the bottom bits in cpuinfo.bti_ist_info */
-#define BTI_IST_WRMSR (1 << 1)
-#define BTI_IST_RSB (1 << 2)
+/* Encoding of cpuinfo.spec_ctrl_flags */
+#define SCF_use_shadow (1 << 0)
+#define SCF_ist_wrmsr (1 << 1)
+#define SCF_ist_rsb (1 << 2)
#ifdef __ASSEMBLY__
#include <asm/msr-index.h>
@@ -49,20 +50,20 @@
* after VMEXIT. The VMEXIT-specific code reads MSR_SPEC_CTRL and updates
* current before loading Xen's MSR_SPEC_CTRL setting.
*
- * Factor 2 is harder. We maintain a shadow_spec_ctrl value, and
- * use_shadow_spec_ctrl boolean per cpu. The synchronous use is:
+ * Factor 2 is harder. We maintain a shadow_spec_ctrl value, and a use_shadow
+ * boolean in the per cpu spec_ctrl_flags. The synchronous use is:
*
* 1) Store guest value in shadow_spec_ctrl
- * 2) Set use_shadow_spec_ctrl boolean
+ * 2) Set the use_shadow boolean
* 3) Load guest value into MSR_SPEC_CTRL
* 4) Exit to guest
* 5) Entry from guest
- * 6) Clear use_shadow_spec_ctrl boolean
+ * 6) Clear the use_shadow boolean
* 7) Load Xen's value into MSR_SPEC_CTRL
*
* The asynchronous use for interrupts/exceptions is:
* - Set/clear IBRS on entry to Xen
- * - On exit to Xen, check use_shadow_spec_ctrl
+ * - On exit to Xen, check use_shadow
* - If set, load shadow_spec_ctrl
*
* Therefore, an interrupt/exception which hits the synchronous path between
@@ -134,7 +135,7 @@
xor %edx, %edx
/* Clear SPEC_CTRL shadowing *before* loading Xen's value. */
- movb %dl, CPUINFO_use_shadow_spec_ctrl(%rsp)
+ andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
/* Load Xen's intended value. */
mov $\ibrs_val, %eax
@@ -160,12 +161,14 @@
* block so calculate the position directly.
*/
.if \maybexen
+ xor %eax, %eax
/* Branchless `if ( !xen ) clear_shadowing` */
testb $3, UREGS_cs(%rsp)
- setz %al
- and %al, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
+ setnz %al
+ not %eax
+ and %al, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14)
.else
- movb %dl, CPUINFO_use_shadow_spec_ctrl(%rsp)
+ andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
.endif
/* Load Xen's intended value. */
@@ -184,8 +187,8 @@
*/
xor %edx, %edx
- cmpb %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%rbx)
- je .L\@_skip
+ testb $SCF_use_shadow, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%rbx)
+ jz .L\@_skip
mov STACK_CPUINFO_FIELD(shadow_spec_ctrl)(%rbx), %eax
mov $MSR_SPEC_CTRL, %ecx
@@ -206,7 +209,7 @@
mov %eax, CPUINFO_shadow_spec_ctrl(%rsp)
/* Set SPEC_CTRL shadowing *before* loading the guest value. */
- movb $1, CPUINFO_use_shadow_spec_ctrl(%rsp)
+ orb $SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
mov $MSR_SPEC_CTRL, %ecx
xor %edx, %edx
@@ -217,7 +220,7 @@
#define SPEC_CTRL_ENTRY_FROM_VMEXIT \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_RSB_VMEXIT; \
- ALTERNATIVE_2 __stringify(ASM_NOP32), \
+ ALTERNATIVE_2 __stringify(ASM_NOP33), \
__stringify(DO_SPEC_CTRL_ENTRY_FROM_VMEXIT \
ibrs_val=SPEC_CTRL_IBRS), \
X86_FEATURE_XEN_IBRS_SET, \
@@ -229,7 +232,7 @@
#define SPEC_CTRL_ENTRY_FROM_PV \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
- ALTERNATIVE_2 __stringify(ASM_NOP21), \
+ ALTERNATIVE_2 __stringify(ASM_NOP22), \
__stringify(DO_SPEC_CTRL_ENTRY maybexen=0 \
ibrs_val=SPEC_CTRL_IBRS), \
X86_FEATURE_XEN_IBRS_SET, \
@@ -240,7 +243,7 @@
#define SPEC_CTRL_ENTRY_FROM_INTR \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
- ALTERNATIVE_2 __stringify(ASM_NOP29), \
+ ALTERNATIVE_2 __stringify(ASM_NOP33), \
__stringify(DO_SPEC_CTRL_ENTRY maybexen=1 \
ibrs_val=SPEC_CTRL_IBRS), \
X86_FEATURE_XEN_IBRS_SET, \
@@ -268,22 +271,23 @@
* This is logical merge of DO_OVERWRITE_RSB and DO_SPEC_CTRL_ENTRY
* maybexen=1, but with conditionals rather than alternatives.
*/
- movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %eax
+ movzbl STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14), %eax
- testb $BTI_IST_RSB, %al
+ test $SCF_ist_rsb, %al
jz .L\@_skip_rsb
DO_OVERWRITE_RSB tmp=rdx /* Clobbers %rcx/%rdx */
.L\@_skip_rsb:
- testb $BTI_IST_WRMSR, %al
+ test $SCF_ist_wrmsr, %al
jz .L\@_skip_wrmsr
xor %edx, %edx
testb $3, UREGS_cs(%rsp)
- setz %dl
- and %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
+ setnz %dl
+ not %edx
+ and %dl, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14)
/* Load Xen's intended value. */
mov $MSR_SPEC_CTRL, %ecx
@@ -310,7 +314,7 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise):
* Requires %rbx=stack_end
* Clobbers %rax, %rcx, %rdx
*/
- testb $BTI_IST_WRMSR, STACK_CPUINFO_FIELD(bti_ist_info)(%rbx)
+ testb $SCF_ist_wrmsr, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%rbx)
jz .L\@_skip
DO_SPEC_CTRL_EXIT_TO_XEN
--
2.1.4

View file

@ -1,221 +0,0 @@
From 0b1aded85866f48cdede20c54d30cf593f8a83f7 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 17 Apr 2018 14:15:04 +0100
Subject: [PATCH] x86/spec_ctrl: Fold the XEN_IBRS_{SET,CLEAR} ALTERNATIVES
together
Currently, the SPEC_CTRL_{ENTRY,EXIT}_* macros encode Xen's choice of
MSR_SPEC_CTRL as an immediate constant, and chooses between IBRS or not by
doubling up the entire alternative block.
There is now a variable holding Xen's choice of value, so use that and
simplify the alternatives.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit af949407eaba7af71067f23d5866cd0bf1f1144d)
---
xen/arch/x86/spec_ctrl.c | 12 +++++-----
xen/include/asm-x86/cpufeatures.h | 3 +--
xen/include/asm-x86/nops.h | 3 ++-
xen/include/asm-x86/spec_ctrl.h | 6 ++---
xen/include/asm-x86/spec_ctrl_asm.h | 45 +++++++++++++------------------------
5 files changed, 26 insertions(+), 43 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 2d69910..b62cfcc 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -112,8 +112,9 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE ? "LFENCE" :
thunk == THUNK_JMP ? "JMP" : "?",
- boot_cpu_has(X86_FEATURE_XEN_IBRS_SET) ? " IBRS+" :
- boot_cpu_has(X86_FEATURE_XEN_IBRS_CLEAR) ? " IBRS-" : "",
+ boot_cpu_has(X86_FEATURE_SC_MSR) ?
+ default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
+ " IBRS-" : "",
opt_ibpb ? " IBPB" : "",
boot_cpu_has(X86_FEATURE_RSB_NATIVE) ? " RSB_NATIVE" : "",
boot_cpu_has(X86_FEATURE_RSB_VMEXIT) ? " RSB_VMEXIT" : "");
@@ -285,13 +286,10 @@ void __init init_speculation_mitigations(void)
* need the IBRS entry/exit logic to virtualise IBRS support for
* guests.
*/
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR);
+
if ( ibrs )
- {
default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
- setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_SET);
- }
- else
- setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_CLEAR);
default_spec_ctrl_flags |= SCF_ist_wrmsr;
}
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index c9b1a48..ca58b0e 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -26,8 +26,7 @@ XEN_CPUFEATURE(LFENCE_DISPATCH, (FSCAPINTS+0)*32+12) /* lfence set as Dispatch S
XEN_CPUFEATURE(IND_THUNK_LFENCE,(FSCAPINTS+0)*32+13) /* Use IND_THUNK_LFENCE */
XEN_CPUFEATURE(IND_THUNK_JMP, (FSCAPINTS+0)*32+14) /* Use IND_THUNK_JMP */
XEN_CPUFEATURE(XEN_IBPB, (FSCAPINTS+0)*32+15) /* IBRSB || IBPB */
-XEN_CPUFEATURE(XEN_IBRS_SET, (FSCAPINTS+0)*32+16) /* IBRSB && IRBS set in Xen */
-XEN_CPUFEATURE(XEN_IBRS_CLEAR, (FSCAPINTS+0)*32+17) /* IBRSB && IBRS clear in Xen */
+XEN_CPUFEATURE(SC_MSR, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen */
XEN_CPUFEATURE(RSB_NATIVE, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for native */
XEN_CPUFEATURE(RSB_VMEXIT, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for vmexit */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
diff --git a/xen/include/asm-x86/nops.h b/xen/include/asm-x86/nops.h
index b744895..913e9f0 100644
--- a/xen/include/asm-x86/nops.h
+++ b/xen/include/asm-x86/nops.h
@@ -62,9 +62,10 @@
#define ASM_NOP8 _ASM_MK_NOP(K8_NOP8)
#define ASM_NOP17 ASM_NOP8; ASM_NOP7; ASM_NOP2
-#define ASM_NOP22 ASM_NOP8; ASM_NOP8; ASM_NOP6
#define ASM_NOP24 ASM_NOP8; ASM_NOP8; ASM_NOP8
+#define ASM_NOP25 ASM_NOP8; ASM_NOP8; ASM_NOP7; ASM_NOP2
#define ASM_NOP33 ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP7; ASM_NOP2
+#define ASM_NOP36 ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP4
#define ASM_NOP40 ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP8; ASM_NOP8
#define ASM_NOP_MAX 8
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 059e291..7d7c42e 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -52,14 +52,14 @@ static always_inline void spec_ctrl_enter_idle(struct cpu_info *info)
barrier();
info->spec_ctrl_flags |= SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
/* WARNING! `ret`, `call *`, `jmp *` not safe before this call. */
static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
{
- uint32_t val = SPEC_CTRL_IBRS;
+ uint32_t val = info->xen_spec_ctrl;
/*
* Disable shadowing before updating the MSR. There are no SMP issues
@@ -67,7 +67,7 @@ static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
*/
info->spec_ctrl_flags &= ~SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 39fb4f8..17dd2cc 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -117,7 +117,7 @@
mov %\tmp, %rsp /* Restore old %rsp */
.endm
-.macro DO_SPEC_CTRL_ENTRY_FROM_VMEXIT ibrs_val:req
+.macro DO_SPEC_CTRL_ENTRY_FROM_VMEXIT
/*
* Requires %rbx=current, %rsp=regs/cpuinfo
* Clobbers %rax, %rcx, %rdx
@@ -138,11 +138,11 @@
andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
/* Load Xen's intended value. */
- mov $\ibrs_val, %eax
+ movzbl CPUINFO_xen_spec_ctrl(%rsp), %eax
wrmsr
.endm
-.macro DO_SPEC_CTRL_ENTRY maybexen:req ibrs_val:req
+.macro DO_SPEC_CTRL_ENTRY maybexen:req
/*
* Requires %rsp=regs (also cpuinfo if !maybexen)
* Requires %r14=stack_end (if maybexen)
@@ -167,12 +167,12 @@
setnz %al
not %eax
and %al, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14)
+ movzbl STACK_CPUINFO_FIELD(xen_spec_ctrl)(%r14), %eax
.else
andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
+ movzbl CPUINFO_xen_spec_ctrl(%rsp), %eax
.endif
- /* Load Xen's intended value. */
- mov $\ibrs_val, %eax
wrmsr
.endm
@@ -220,47 +220,32 @@
#define SPEC_CTRL_ENTRY_FROM_VMEXIT \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_RSB_VMEXIT; \
- ALTERNATIVE_2 __stringify(ASM_NOP33), \
- __stringify(DO_SPEC_CTRL_ENTRY_FROM_VMEXIT \
- ibrs_val=SPEC_CTRL_IBRS), \
- X86_FEATURE_XEN_IBRS_SET, \
- __stringify(DO_SPEC_CTRL_ENTRY_FROM_VMEXIT \
- ibrs_val=0), \
- X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE __stringify(ASM_NOP36), \
+ DO_SPEC_CTRL_ENTRY_FROM_VMEXIT, X86_FEATURE_SC_MSR
/* Use after an entry from PV context (syscall/sysenter/int80/int82/etc). */
#define SPEC_CTRL_ENTRY_FROM_PV \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
- ALTERNATIVE_2 __stringify(ASM_NOP22), \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=0 \
- ibrs_val=SPEC_CTRL_IBRS), \
- X86_FEATURE_XEN_IBRS_SET, \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=0 ibrs_val=0), \
- X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE __stringify(ASM_NOP25), \
+ __stringify(DO_SPEC_CTRL_ENTRY maybexen=0), X86_FEATURE_SC_MSR
/* Use in interrupt/exception context. May interrupt Xen or PV context. */
#define SPEC_CTRL_ENTRY_FROM_INTR \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
- ALTERNATIVE_2 __stringify(ASM_NOP33), \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=1 \
- ibrs_val=SPEC_CTRL_IBRS), \
- X86_FEATURE_XEN_IBRS_SET, \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=1 ibrs_val=0), \
- X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE __stringify(ASM_NOP33), \
+ __stringify(DO_SPEC_CTRL_ENTRY maybexen=1), X86_FEATURE_SC_MSR
/* Use when exiting to Xen context. */
#define SPEC_CTRL_EXIT_TO_XEN \
- ALTERNATIVE_2 __stringify(ASM_NOP17), \
- DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_XEN_IBRS_SET, \
- DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE __stringify(ASM_NOP17), \
+ DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR
/* Use when exiting to guest context. */
#define SPEC_CTRL_EXIT_TO_GUEST \
- ALTERNATIVE_2 __stringify(ASM_NOP24), \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_SET, \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE __stringify(ASM_NOP24), \
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
/* TODO: Drop these when the alternatives infrastructure is NMI/#MC safe. */
.macro SPEC_CTRL_ENTRY_FROM_INTR_IST
--
2.1.4

View file

@ -1,273 +0,0 @@
From 5cc3611de7d09140e55caa2c2d120ad326fff937 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Mon, 30 Apr 2018 14:20:23 +0100
Subject: [PATCH] x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE
and VMEXIT
In hindsight, using NATIVE and VMEXIT as naming terminology was not clever.
A future change wants to split SPEC_CTRL_EXIT_TO_GUEST into PV and HVM
specific implementations, and using VMEXIT as a term is completely wrong.
Take the opportunity to fix some stale documentation in spec_ctrl_asm.h. The
IST helpers were missing from the large comment block, and since
SPEC_CTRL_ENTRY_FROM_INTR_IST was introduced, we've gained a new piece of
functionality which currently depends on the fine grain control, which exists
in lieu of livepatching. Note this in the comment.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit d9822b8a38114e96e4516dc998f4055249364d5d)
---
xen/arch/x86/hvm/svm/entry.S | 4 ++--
xen/arch/x86/hvm/vmx/entry.S | 4 ++--
xen/arch/x86/spec_ctrl.c | 20 ++++++++++----------
xen/arch/x86/x86_64/compat/entry.S | 2 +-
xen/arch/x86/x86_64/entry.S | 2 +-
xen/include/asm-x86/cpufeatures.h | 4 ++--
xen/include/asm-x86/spec_ctrl_asm.h | 36 +++++++++++++++++++++++++-----------
7 files changed, 43 insertions(+), 29 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index bf092fe..5e7c080 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -83,7 +83,7 @@ UNLIKELY_END(svm_trace)
mov VCPUMSR_spec_ctrl_raw(%rax), %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_HVM /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
pop %r15
pop %r14
@@ -108,7 +108,7 @@ UNLIKELY_END(svm_trace)
GET_CURRENT(bx)
- SPEC_CTRL_ENTRY_FROM_VMEXIT /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
+ SPEC_CTRL_ENTRY_FROM_HVM /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
mov VCPU_svm_vmcb(%rbx),%rcx
diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index e750544..aa2f103 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -38,7 +38,7 @@ ENTRY(vmx_asm_vmexit_handler)
movb $1,VCPU_vmx_launched(%rbx)
mov %rax,VCPU_hvm_guest_cr2(%rbx)
- SPEC_CTRL_ENTRY_FROM_VMEXIT /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
+ SPEC_CTRL_ENTRY_FROM_HVM /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
mov %rsp,%rdi
@@ -76,7 +76,7 @@ UNLIKELY_END(realmode)
mov VCPUMSR_spec_ctrl_raw(%rax), %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_HVM /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
mov VCPU_hvm_guest_cr2(%rbx),%rax
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index b62cfcc..015a9e2 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -35,8 +35,8 @@ static enum ind_thunk {
THUNK_JMP,
} opt_thunk __initdata = THUNK_DEFAULT;
static int8_t __initdata opt_ibrs = -1;
-static bool __initdata opt_rsb_native = true;
-static bool __initdata opt_rsb_vmexit = true;
+static bool __initdata opt_rsb_pv = true;
+static bool __initdata opt_rsb_hvm = true;
bool __read_mostly opt_ibpb = true;
uint8_t __read_mostly default_xen_spec_ctrl;
uint8_t __read_mostly default_spec_ctrl_flags;
@@ -69,9 +69,9 @@ static int __init parse_bti(const char *s)
else if ( (val = parse_boolean("ibpb", s, ss)) >= 0 )
opt_ibpb = val;
else if ( (val = parse_boolean("rsb_native", s, ss)) >= 0 )
- opt_rsb_native = val;
+ opt_rsb_pv = val;
else if ( (val = parse_boolean("rsb_vmexit", s, ss)) >= 0 )
- opt_rsb_vmexit = val;
+ opt_rsb_hvm = val;
else
rc = -EINVAL;
@@ -116,8 +116,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
" IBRS-" : "",
opt_ibpb ? " IBPB" : "",
- boot_cpu_has(X86_FEATURE_RSB_NATIVE) ? " RSB_NATIVE" : "",
- boot_cpu_has(X86_FEATURE_RSB_VMEXIT) ? " RSB_VMEXIT" : "");
+ boot_cpu_has(X86_FEATURE_SC_RSB_PV) ? " RSB_NATIVE" : "",
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB_VMEXIT" : "");
printk("XPTI: %s\n",
boot_cpu_has(X86_FEATURE_NO_XPTI) ? "disabled" : "enabled");
@@ -307,9 +307,9 @@ void __init init_speculation_mitigations(void)
* If a processors speculates to 32bit PV guest kernel mappings, it is
* speculating in 64bit supervisor mode, and can leak data.
*/
- if ( opt_rsb_native )
+ if ( opt_rsb_pv )
{
- setup_force_cpu_cap(X86_FEATURE_RSB_NATIVE);
+ setup_force_cpu_cap(X86_FEATURE_SC_RSB_PV);
default_spec_ctrl_flags |= SCF_ist_rsb;
}
@@ -317,8 +317,8 @@ void __init init_speculation_mitigations(void)
* HVM guests can always poison the RSB to point at Xen supervisor
* mappings.
*/
- if ( opt_rsb_vmexit )
- setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT);
+ if ( opt_rsb_hvm )
+ setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM);
/* Check we have hardware IBPB support before using it... */
if ( !boot_cpu_has(X86_FEATURE_IBRSB) && !boot_cpu_has(X86_FEATURE_IBPB) )
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index a47cb9d..6a27d98 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -166,7 +166,7 @@ ENTRY(compat_restore_all_guest)
mov VCPUMSR_spec_ctrl_raw(%rax), %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
RESTORE_ALL adj=8 compat=1
.Lft0: iretq
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 41d3ec2..0a0763a 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -196,7 +196,7 @@ restore_all_guest:
mov %r15d, %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
RESTORE_ALL
testw $TRAP_syscall,4(%rsp)
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index ca58b0e..f9aa5d7 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -27,6 +27,6 @@ XEN_CPUFEATURE(IND_THUNK_LFENCE,(FSCAPINTS+0)*32+13) /* Use IND_THUNK_LFENCE */
XEN_CPUFEATURE(IND_THUNK_JMP, (FSCAPINTS+0)*32+14) /* Use IND_THUNK_JMP */
XEN_CPUFEATURE(XEN_IBPB, (FSCAPINTS+0)*32+15) /* IBRSB || IBPB */
XEN_CPUFEATURE(SC_MSR, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen */
-XEN_CPUFEATURE(RSB_NATIVE, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for native */
-XEN_CPUFEATURE(RSB_VMEXIT, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for vmexit */
+XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
+XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 17dd2cc..3d156ed 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -72,11 +72,14 @@
*
* The following ASM fragments implement this algorithm. See their local
* comments for further details.
- * - SPEC_CTRL_ENTRY_FROM_VMEXIT
+ * - SPEC_CTRL_ENTRY_FROM_HVM
* - SPEC_CTRL_ENTRY_FROM_PV
* - SPEC_CTRL_ENTRY_FROM_INTR
+ * - SPEC_CTRL_ENTRY_FROM_INTR_IST
+ * - SPEC_CTRL_EXIT_TO_XEN_IST
* - SPEC_CTRL_EXIT_TO_XEN
- * - SPEC_CTRL_EXIT_TO_GUEST
+ * - SPEC_CTRL_EXIT_TO_PV
+ * - SPEC_CTRL_EXIT_TO_HVM
*/
.macro DO_OVERWRITE_RSB tmp=rax
@@ -117,7 +120,7 @@
mov %\tmp, %rsp /* Restore old %rsp */
.endm
-.macro DO_SPEC_CTRL_ENTRY_FROM_VMEXIT
+.macro DO_SPEC_CTRL_ENTRY_FROM_HVM
/*
* Requires %rbx=current, %rsp=regs/cpuinfo
* Clobbers %rax, %rcx, %rdx
@@ -217,23 +220,23 @@
.endm
/* Use after a VMEXIT from an HVM guest. */
-#define SPEC_CTRL_ENTRY_FROM_VMEXIT \
+#define SPEC_CTRL_ENTRY_FROM_HVM \
ALTERNATIVE __stringify(ASM_NOP40), \
- DO_OVERWRITE_RSB, X86_FEATURE_RSB_VMEXIT; \
+ DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_HVM; \
ALTERNATIVE __stringify(ASM_NOP36), \
- DO_SPEC_CTRL_ENTRY_FROM_VMEXIT, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_ENTRY_FROM_HVM, X86_FEATURE_SC_MSR
/* Use after an entry from PV context (syscall/sysenter/int80/int82/etc). */
#define SPEC_CTRL_ENTRY_FROM_PV \
ALTERNATIVE __stringify(ASM_NOP40), \
- DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
+ DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE __stringify(ASM_NOP25), \
__stringify(DO_SPEC_CTRL_ENTRY maybexen=0), X86_FEATURE_SC_MSR
/* Use in interrupt/exception context. May interrupt Xen or PV context. */
#define SPEC_CTRL_ENTRY_FROM_INTR \
ALTERNATIVE __stringify(ASM_NOP40), \
- DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
+ DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE __stringify(ASM_NOP33), \
__stringify(DO_SPEC_CTRL_ENTRY maybexen=1), X86_FEATURE_SC_MSR
@@ -242,12 +245,22 @@
ALTERNATIVE __stringify(ASM_NOP17), \
DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR
-/* Use when exiting to guest context. */
-#define SPEC_CTRL_EXIT_TO_GUEST \
+/* Use when exiting to PV guest context. */
+#define SPEC_CTRL_EXIT_TO_PV \
ALTERNATIVE __stringify(ASM_NOP24), \
DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
-/* TODO: Drop these when the alternatives infrastructure is NMI/#MC safe. */
+/* Use when exiting to HVM guest context. */
+#define SPEC_CTRL_EXIT_TO_HVM \
+ ALTERNATIVE __stringify(ASM_NOP24), \
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
+
+/*
+ * Use in IST interrupt/exception context. May interrupt Xen or PV context.
+ * Fine grain control of SCF_ist_wrmsr is needed for safety in the S3 resume
+ * path to avoid using MSR_SPEC_CTRL before the microcode introducing it has
+ * been reloaded.
+ */
.macro SPEC_CTRL_ENTRY_FROM_INTR_IST
/*
* Requires %rsp=regs, %r14=stack_end
@@ -294,6 +307,7 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise):
UNLIKELY_END(\@_serialise)
.endm
+/* Use when exiting to Xen in IST context. */
.macro SPEC_CTRL_EXIT_TO_XEN_IST
/*
* Requires %rbx=stack_end
--
2.1.4

View file

@ -1,71 +0,0 @@
From 811fcf5137abdcd5b9ea7e5212098adb5bedae0f Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Mon, 7 May 2018 14:06:16 +0100
Subject: [PATCH] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context
when possible
If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as its
own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to the
MSR.
Requested-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit 94df6e8588e35cc2028ccb3fd2921c6e6360605e)
---
xen/arch/x86/spec_ctrl.c | 4 ++++
xen/include/asm-x86/cpufeatures.h | 1 +
xen/include/asm-x86/spec_ctrl.h | 4 ++--
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 015a9e2..55ef79f 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -327,6 +327,10 @@ void __init init_speculation_mitigations(void)
/* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
init_shadow_spec_ctrl_state();
+ /* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. */
+ if ( default_xen_spec_ctrl )
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
+
print_details(thunk, caps);
}
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index f9aa5d7..32b7f04 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -30,3 +30,4 @@ XEN_CPUFEATURE(SC_MSR, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xe
XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
+XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* SC_MSR && default_xen_spec_ctrl */
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 7d7c42e..77f92ba 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -52,7 +52,7 @@ static always_inline void spec_ctrl_enter_idle(struct cpu_info *info)
barrier();
info->spec_ctrl_flags |= SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR_IDLE)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
@@ -67,7 +67,7 @@ static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
*/
info->spec_ctrl_flags &= ~SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR_IDLE)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
--
2.1.4

View file

@ -1,111 +0,0 @@
From 2acc4cba7eb2559bafdd4d8238466ad81322a35a Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 17 Apr 2018 14:15:04 +0100
Subject: [PATCH] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM
variants
In order to separately control whether MSR_SPEC_CTRL is virtualised for PV and
HVM guests, split the feature used to control runtime alternatives into two.
Xen will use MSR_SPEC_CTRL itself if either of these features are active.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit fa9eb09d446a1279f5e861e6b84fa8675dabf148)
---
xen/arch/x86/spec_ctrl.c | 6 ++++--
xen/include/asm-x86/cpufeatures.h | 5 +++--
xen/include/asm-x86/spec_ctrl_asm.h | 12 ++++++------
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 55ef79f..a940308 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -112,7 +112,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE ? "LFENCE" :
thunk == THUNK_JMP ? "JMP" : "?",
- boot_cpu_has(X86_FEATURE_SC_MSR) ?
+ (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
+ boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
" IBRS-" : "",
opt_ibpb ? " IBPB" : "",
@@ -286,7 +287,8 @@ void __init init_speculation_mitigations(void)
* need the IBRS entry/exit logic to virtualise IBRS support for
* guests.
*/
- setup_force_cpu_cap(X86_FEATURE_SC_MSR);
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
if ( ibrs )
default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index 32b7f04..b90aa2d 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -26,8 +26,9 @@ XEN_CPUFEATURE(LFENCE_DISPATCH, (FSCAPINTS+0)*32+12) /* lfence set as Dispatch S
XEN_CPUFEATURE(IND_THUNK_LFENCE,(FSCAPINTS+0)*32+13) /* Use IND_THUNK_LFENCE */
XEN_CPUFEATURE(IND_THUNK_JMP, (FSCAPINTS+0)*32+14) /* Use IND_THUNK_JMP */
XEN_CPUFEATURE(XEN_IBPB, (FSCAPINTS+0)*32+15) /* IBRSB || IBPB */
-XEN_CPUFEATURE(SC_MSR, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen */
+XEN_CPUFEATURE(SC_MSR_PV, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen for PV */
+XEN_CPUFEATURE(SC_MSR_HVM, (FSCAPINTS+0)*32+17) /* MSR_SPEC_CTRL used by Xen for HVM */
XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
-XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* SC_MSR && default_xen_spec_ctrl */
+XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || SC_MSR_HVM) && default_xen_spec_ctrl */
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 3d156ed..c659f3f 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -224,36 +224,36 @@
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_HVM; \
ALTERNATIVE __stringify(ASM_NOP36), \
- DO_SPEC_CTRL_ENTRY_FROM_HVM, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_ENTRY_FROM_HVM, X86_FEATURE_SC_MSR_HVM
/* Use after an entry from PV context (syscall/sysenter/int80/int82/etc). */
#define SPEC_CTRL_ENTRY_FROM_PV \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE __stringify(ASM_NOP25), \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=0), X86_FEATURE_SC_MSR
+ __stringify(DO_SPEC_CTRL_ENTRY maybexen=0), X86_FEATURE_SC_MSR_PV
/* Use in interrupt/exception context. May interrupt Xen or PV context. */
#define SPEC_CTRL_ENTRY_FROM_INTR \
ALTERNATIVE __stringify(ASM_NOP40), \
DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE __stringify(ASM_NOP33), \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=1), X86_FEATURE_SC_MSR
+ __stringify(DO_SPEC_CTRL_ENTRY maybexen=1), X86_FEATURE_SC_MSR_PV
/* Use when exiting to Xen context. */
#define SPEC_CTRL_EXIT_TO_XEN \
ALTERNATIVE __stringify(ASM_NOP17), \
- DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR_PV
/* Use when exiting to PV guest context. */
#define SPEC_CTRL_EXIT_TO_PV \
ALTERNATIVE __stringify(ASM_NOP24), \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR_PV
/* Use when exiting to HVM guest context. */
#define SPEC_CTRL_EXIT_TO_HVM \
ALTERNATIVE __stringify(ASM_NOP24), \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR_HVM
/*
* Use in IST interrupt/exception context. May interrupt Xen or PV context.
--
2.1.4

View file

@ -1,134 +0,0 @@
From 5b223f41d59887ea5d13e2406597ff472ba6f2fc Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Wed, 9 May 2018 13:59:56 +0100
Subject: [PATCH] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL
value
With the impending ability to disable MSR_SPEC_CTRL handling on a
per-guest-type basis, the first exit-from-guest may not have the side effect
of loading Xen's choice of value. Explicitly set Xen's default during the BSP
and AP boot paths.
For the BSP however, delay setting a non-zero MSR_SPEC_CTRL default until
after dom0 has been constructed when safe to do so. Oracle report that this
speeds up boots of some hardware by 50s.
"when safe to do so" is based on whether we are virtualised. A native boot
won't have any other code running in a position to mount an attack.
Reported-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit cb8c12020307b39a89273d7699e89000451987ab)
---
xen/arch/x86/setup.c | 7 +++++++
xen/arch/x86/smpboot.c | 8 ++++++++
xen/arch/x86/spec_ctrl.c | 32 ++++++++++++++++++++++++++++++++
xen/include/asm-x86/spec_ctrl.h | 2 ++
4 files changed, 49 insertions(+)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 482fe11..1995c4c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1746,6 +1746,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
setup_io_bitmap(dom0);
+ if ( bsp_delay_spec_ctrl )
+ {
+ get_cpu_info()->spec_ctrl_flags &= ~SCF_use_shadow;
+ barrier();
+ wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
+ }
+
/* Jump to the 1:1 virtual mappings of cpu0_stack. */
asm volatile ("mov %[stk], %%rsp; jmp %c[fn]" ::
[stk] "g" (__va(__pa(get_stack_bottom()))),
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index f81fc2c..ee8b183 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -351,6 +351,14 @@ void start_secondary(void *unused)
else
microcode_resume_cpu(cpu);
+ /*
+ * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
+ * any firmware settings. Note: MSR_SPEC_CTRL may only become available
+ * after loading microcode.
+ */
+ if ( boot_cpu_has(X86_FEATURE_IBRSB) )
+ wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
+
if ( xen_guest )
hypervisor_ap_setup();
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index a940308..3adec1a 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -38,6 +38,8 @@ static int8_t __initdata opt_ibrs = -1;
static bool __initdata opt_rsb_pv = true;
static bool __initdata opt_rsb_hvm = true;
bool __read_mostly opt_ibpb = true;
+
+bool __initdata bsp_delay_spec_ctrl;
uint8_t __read_mostly default_xen_spec_ctrl;
uint8_t __read_mostly default_spec_ctrl_flags;
@@ -334,6 +336,36 @@ void __init init_speculation_mitigations(void)
setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
print_details(thunk, caps);
+
+ /*
+ * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
+ * any firmware settings. For performance reasons, when safe to do so, we
+ * delay applying non-zero settings until after dom0 has been constructed.
+ *
+ * "when safe to do so" is based on whether we are virtualised. A native
+ * boot won't have any other code running in a position to mount an
+ * attack.
+ */
+ if ( boot_cpu_has(X86_FEATURE_IBRSB) )
+ {
+ bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
+
+ /*
+ * If delaying MSR_SPEC_CTRL setup, use the same mechanism as
+ * spec_ctrl_enter_idle(), by using a shadow value of zero.
+ */
+ if ( bsp_delay_spec_ctrl )
+ {
+ struct cpu_info *info = get_cpu_info();
+
+ info->shadow_spec_ctrl = 0;
+ barrier();
+ info->spec_ctrl_flags |= SCF_use_shadow;
+ barrier();
+ }
+
+ wrmsrl(MSR_SPEC_CTRL, bsp_delay_spec_ctrl ? 0 : default_xen_spec_ctrl);
+ }
}
static void __init __maybe_unused build_assertions(void)
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 77f92ba..c6a38f4 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -27,6 +27,8 @@
void init_speculation_mitigations(void);
extern bool opt_ibpb;
+
+extern bool bsp_delay_spec_ctrl;
extern uint8_t default_xen_spec_ctrl;
extern uint8_t default_spec_ctrl_flags;
--
2.1.4

View file

@ -1,132 +0,0 @@
From bce7a2145abc3c7e5bfd7e2168714d194124a3ab Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 1 May 2018 11:59:03 +0100
Subject: [PATCH] x86/cpuid: Improvements to guest policies for speculative
sidechannel features
If Xen isn't virtualising MSR_SPEC_CTRL for guests, IBRSB shouldn't be
advertised. It is not currently possible to express this via the existing
command line options, but such an ability will be introduced.
Another useful option in some usecases is to offer IBPB without IBRS. When a
guest kernel is known to be compatible (uses retpoline and knows about the AMD
IBPB feature bit), an administrator with pre-Skylake hardware may wish to hide
IBRS. This allows the VM to have full protection, without Xen or the VM
needing to touch MSR_SPEC_CTRL, which can reduce the overhead of Spectre
mitigations.
Break the logic common to both PV and HVM CPUID calculations into a common
helper, to avoid duplication.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit cb06b308ec71b23f37a44f5e2351fe2cae0306e9)
---
xen/arch/x86/cpuid.c | 60 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index b3c9ac6..b45b145 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -368,6 +368,28 @@ static void __init calculate_host_policy(void)
}
}
+static void __init guest_common_feature_adjustments(uint32_t *fs)
+{
+ /* Unconditionally claim to be able to set the hypervisor bit. */
+ __set_bit(X86_FEATURE_HYPERVISOR, fs);
+
+ /*
+ * If IBRS is offered to the guest, unconditionally offer STIBP. It is a
+ * nop on non-HT hardware, and has this behaviour to make heterogeneous
+ * setups easier to manage.
+ */
+ if ( test_bit(X86_FEATURE_IBRSB, fs) )
+ __set_bit(X86_FEATURE_STIBP, fs);
+
+ /*
+ * On hardware which supports IBRS/IBPB, we can offer IBPB independently
+ * of IBRS by using the AMD feature bit. An administrator may wish for
+ * performance reasons to offer IBPB without IBRS.
+ */
+ if ( host_cpuid_policy.feat.ibrsb )
+ __set_bit(X86_FEATURE_IBPB, fs);
+}
+
static void __init calculate_pv_max_policy(void)
{
struct cpuid_policy *p = &pv_max_cpuid_policy;
@@ -380,18 +402,14 @@ static void __init calculate_pv_max_policy(void)
for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
pv_featureset[i] &= pv_featuremask[i];
- /* Unconditionally claim to be able to set the hypervisor bit. */
- __set_bit(X86_FEATURE_HYPERVISOR, pv_featureset);
-
- /* On hardware with IBRS/IBPB support, there are further adjustments. */
- if ( test_bit(X86_FEATURE_IBRSB, pv_featureset) )
- {
- /* Offer STIBP unconditionally. It is a nop on non-HT hardware. */
- __set_bit(X86_FEATURE_STIBP, pv_featureset);
+ /*
+ * If Xen isn't virtualising MSR_SPEC_CTRL for PV guests because of
+ * administrator choice, hide the feature.
+ */
+ if ( !boot_cpu_has(X86_FEATURE_SC_MSR_PV) )
+ __clear_bit(X86_FEATURE_IBRSB, pv_featureset);
- /* AMD's IBPB is a subset of IBRS/IBPB. */
- __set_bit(X86_FEATURE_IBPB, pv_featureset);
- }
+ guest_common_feature_adjustments(pv_featureset);
sanitise_featureset(pv_featureset);
cpuid_featureset_to_policy(pv_featureset, p);
@@ -419,9 +437,6 @@ static void __init calculate_hvm_max_policy(void)
for ( i = 0; i < ARRAY_SIZE(hvm_featureset); ++i )
hvm_featureset[i] &= hvm_featuremask[i];
- /* Unconditionally claim to be able to set the hypervisor bit. */
- __set_bit(X86_FEATURE_HYPERVISOR, hvm_featureset);
-
/*
* Xen can provide an APIC emulation to HVM guests even if the host's APIC
* isn't enabled.
@@ -438,6 +453,13 @@ static void __init calculate_hvm_max_policy(void)
__set_bit(X86_FEATURE_SEP, hvm_featureset);
/*
+ * If Xen isn't virtualising MSR_SPEC_CTRL for HVM guests because of
+ * administrator choice, hide the feature.
+ */
+ if ( !boot_cpu_has(X86_FEATURE_SC_MSR_HVM) )
+ __clear_bit(X86_FEATURE_IBRSB, hvm_featureset);
+
+ /*
* With VT-x, some features are only supported by Xen if dedicated
* hardware support is also available.
*/
@@ -450,15 +472,7 @@ static void __init calculate_hvm_max_policy(void)
__clear_bit(X86_FEATURE_XSAVES, hvm_featureset);
}
- /* On hardware with IBRS/IBPB support, there are further adjustments. */
- if ( test_bit(X86_FEATURE_IBRSB, hvm_featureset) )
- {
- /* Offer STIBP unconditionally. It is a nop on non-HT hardware. */
- __set_bit(X86_FEATURE_STIBP, hvm_featureset);
-
- /* AMD's IBPB is a subset of IBRS/IBPB. */
- __set_bit(X86_FEATURE_IBPB, hvm_featureset);
- }
+ guest_common_feature_adjustments(hvm_featureset);
sanitise_featureset(hvm_featureset);
cpuid_featureset_to_policy(hvm_featureset, p);
--
2.1.4

View file

@ -1,344 +0,0 @@
From 952ff9f5590e37952d7dd3d89e16a47a238ab079 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Thu, 26 Apr 2018 10:52:55 +0100
Subject: [PATCH] x86/spec_ctrl: Introduce a new `spec-ctrl=` command line
argument to replace `bti=`
In hindsight, the options for `bti=` aren't as flexible or useful as expected
(including several options which don't appear to behave as intended).
Changing the behaviour of an existing option is problematic for compatibility,
so introduce a new `spec-ctrl=` in the hopes that we can do better.
One common way of deploying Xen is with a single PV dom0 and all domUs being
HVM domains. In such a setup, an administrator who has weighed up the risks
may wish to forgo protection against malicious PV domains, to reduce the
overall performance hit. To cater for this usecase, `spec-ctrl=no-pv` will
disable all speculative protection for PV domains, while leaving all
speculative protection for HVM domains intact.
For coding clarity as much as anything else, the suboptions are grouped by
logical area; those which affect the alternatives blocks, and those which
affect Xen's in-hypervisor settings. See the xen-command-line.markdown for
full details of the new options.
While changing the command line options, take the time to change how the data
is reported to the user. The three DEBUG printks are upgraded to unilateral,
as they are all relevant pieces of information, and the old "mitigations:"
line is split in the two logical areas described above.
Sample output from booting with `spec-ctrl=no-pv` looks like:
(XEN) Speculative mitigation facilities:
(XEN) Hardware features: IBRS/IBPB STIBP IBPB
(XEN) Compiled-in support: INDIRECT_THUNK
(XEN) Xen settings: BTI-Thunk RETPOLINE, SPEC_CTRL: IBRS-, Other: IBPB
(XEN) Support for VMs: PV: None, HVM: MSR_SPEC_CTRL RSB
(XEN) XPTI (64-bit PV only): Dom0 enabled, DomU enabled
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit 3352afc26c497d26ecb70527db3cb29daf7b1422)
---
docs/misc/xen-command-line.markdown | 49 +++++++++++
xen/arch/x86/spec_ctrl.c | 160 ++++++++++++++++++++++++++++++------
2 files changed, 186 insertions(+), 23 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 6c673ee..43a6ddb 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -248,6 +248,9 @@ the NMI watchdog is also enabled.
### bti (x86)
> `= List of [ thunk=retpoline|lfence|jmp, ibrs=<bool>, ibpb=<bool>, rsb_{vmexit,native}=<bool> ]`
+**WARNING: This command line option is deprecated, and superseded by
+_spec-ctrl=_ - using both options in combination is undefined.**
+
Branch Target Injection controls. By default, Xen will pick the most
appropriate BTI mitigations based on compiled in support, loaded microcode,
and hardware details.
@@ -1698,6 +1701,52 @@ enforces the maximum theoretically necessary timeout of 670ms. Any number
is being interpreted as a custom timeout in milliseconds. Zero or boolean
false disable the quirk workaround, which is also the default.
+### spec-ctrl (x86)
+> `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>,
+> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb}=<bool> ]`
+
+Controls for speculative execution sidechannel mitigations. By default, Xen
+will pick the most appropriate mitigations based on compiled in support,
+loaded microcode, and hardware details, and will virtualise appropriate
+mitigations for guests to use.
+
+**WARNING: Any use of this option may interfere with heuristics. Use with
+extreme care.**
+
+An overall boolean value, `spec-ctrl=no`, can be specified to turn off all
+mitigations, including pieces of infrastructure used to virtualise certain
+mitigation features for guests. Alternatively, a slightly more restricted
+`spec-ctrl=no-xen` can be used to turn off all of Xen's mitigations, while
+leaving the virtualisation support in place for guests to use. Use of a
+positive boolean value for either of these options is invalid.
+
+The booleans `pv=`, `hvm=`, `msr-sc=` and `rsb=` offer fine grained control
+over the alternative blocks used by Xen. These impact Xen's ability to
+protect itself, and Xen's ability to virtualise support for guests to use.
+
+* `pv=` and `hvm=` offer control over all suboptions for PV and HVM guests
+ respectively.
+* `msr-sc=` offers control over Xen's support for manipulating MSR\_SPEC\_CTRL
+ on entry and exit. These blocks are necessary to virtualise support for
+ guests and if disabled, guests will be unable to use IBRS/STIBP/etc.
+* `rsb=` offers control over whether to overwrite the Return Stack Buffer /
+ Return Address Stack on entry to Xen.
+
+If Xen was compiled with INDIRECT\_THUNK support, `bti-thunk=` can be used to
+select which of the thunks gets patched into the `__x86_indirect_thunk_%reg`
+locations. The default thunk is `retpoline` (generally preferred for Intel
+hardware), with the alternatives being `jmp` (a `jmp *%reg` gadget, minimal
+overhead), and `lfence` (an `lfence; jmp *%reg` gadget, preferred for AMD).
+
+On hardware supporting IBRS (Indirect Branch Restricted Speculation), the
+`ibrs=` option can be used to force or prevent Xen using the feature itself.
+If Xen is not using IBRS itself, functionality is still set up so IBRS can be
+virtualised for guests.
+
+On hardware supporting IBPB (Indirect Branch Prediction Barrier), the `ibpb=`
+option can be used to force (the default) or prevent Xen from issuing branch
+prediction barriers on vcpu context switches.
+
### sync\_console
> `= <boolean>`
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 3adec1a..4f9282f 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -26,6 +26,13 @@
#include <asm/spec_ctrl.h>
#include <asm/spec_ctrl_asm.h>
+/* Cmdline controls for Xen's alternative blocks. */
+static bool __initdata opt_msr_sc_pv = true;
+static bool __initdata opt_msr_sc_hvm = true;
+static bool __initdata opt_rsb_pv = true;
+static bool __initdata opt_rsb_hvm = true;
+
+/* Cmdline controls for Xen's speculative settings. */
static enum ind_thunk {
THUNK_DEFAULT, /* Decide which thunk to use at boot time. */
THUNK_NONE, /* Missing compiler support for thunks. */
@@ -35,8 +42,6 @@ static enum ind_thunk {
THUNK_JMP,
} opt_thunk __initdata = THUNK_DEFAULT;
static int8_t __initdata opt_ibrs = -1;
-static bool __initdata opt_rsb_pv = true;
-static bool __initdata opt_rsb_hvm = true;
bool __read_mostly opt_ibpb = true;
bool __initdata bsp_delay_spec_ctrl;
@@ -84,8 +89,95 @@ static int __init parse_bti(const char *s)
}
custom_param("bti", parse_bti);
+static int __init parse_spec_ctrl(const char *s)
+{
+ const char *ss;
+ int val, rc = 0;
+
+ do {
+ ss = strchr(s, ',');
+ if ( !ss )
+ ss = strchr(s, '\0');
+
+ /* Global and Xen-wide disable. */
+ val = parse_bool(s, ss);
+ if ( !val )
+ {
+ opt_msr_sc_pv = false;
+ opt_msr_sc_hvm = false;
+
+ disable_common:
+ opt_rsb_pv = false;
+ opt_rsb_hvm = false;
+
+ opt_thunk = THUNK_JMP;
+ opt_ibrs = 0;
+ opt_ibpb = false;
+ }
+ else if ( val > 0 )
+ rc = -EINVAL;
+ else if ( (val = parse_boolean("xen", s, ss)) >= 0 )
+ {
+ if ( !val )
+ goto disable_common;
+
+ rc = -EINVAL;
+ }
+
+ /* Xen's alternative blocks. */
+ else if ( (val = parse_boolean("pv", s, ss)) >= 0 )
+ {
+ opt_msr_sc_pv = val;
+ opt_rsb_pv = val;
+ }
+ else if ( (val = parse_boolean("hvm", s, ss)) >= 0 )
+ {
+ opt_msr_sc_hvm = val;
+ opt_rsb_hvm = val;
+ }
+ else if ( (val = parse_boolean("msr-sc", s, ss)) >= 0 )
+ {
+ opt_msr_sc_pv = val;
+ opt_msr_sc_hvm = val;
+ }
+ else if ( (val = parse_boolean("rsb", s, ss)) >= 0 )
+ {
+ opt_rsb_pv = val;
+ opt_rsb_hvm = val;
+ }
+
+ /* Xen's speculative sidechannel mitigation settings. */
+ else if ( !strncmp(s, "bti-thunk=", 10) )
+ {
+ s += 10;
+
+ if ( !strncmp(s, "retpoline", ss - s) )
+ opt_thunk = THUNK_RETPOLINE;
+ else if ( !strncmp(s, "lfence", ss - s) )
+ opt_thunk = THUNK_LFENCE;
+ else if ( !strncmp(s, "jmp", ss - s) )
+ opt_thunk = THUNK_JMP;
+ else
+ rc = -EINVAL;
+ }
+ else if ( (val = parse_boolean("ibrs", s, ss)) >= 0 )
+ opt_ibrs = val;
+ else if ( (val = parse_boolean("ibpb", s, ss)) >= 0 )
+ opt_ibpb = val;
+ else
+ rc = -EINVAL;
+
+ s = ss + 1;
+ } while ( *ss );
+
+ return rc;
+}
+custom_param("spec-ctrl", parse_spec_ctrl);
+
static void __init print_details(enum ind_thunk thunk, uint64_t caps)
{
+ bool use_spec_ctrl = (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
+ boot_cpu_has(X86_FEATURE_SC_MSR_HVM));
unsigned int _7d0 = 0, e8b = 0, tmp;
/* Collect diagnostics about available mitigations. */
@@ -94,10 +186,10 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
- printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
+ printk("Speculative mitigation facilities:\n");
/* Hardware features which pertain to speculative mitigations. */
- printk(XENLOG_DEBUG " Hardware features:%s%s%s%s%s%s\n",
+ printk(" Hardware features:%s%s%s%s%s%s\n",
(_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
(_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP" : "",
(e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "",
@@ -107,20 +199,31 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
/* Compiled-in support which pertains to BTI mitigations. */
if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
- printk(XENLOG_DEBUG " Compiled-in support: INDIRECT_THUNK\n");
+ printk(" Compiled-in support: INDIRECT_THUNK\n");
- printk("BTI mitigations: Thunk %s, Others:%s%s%s%s\n",
+ /* Settings for Xen's protection, irrespective of guests. */
+ printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s, Other:%s\n",
thunk == THUNK_NONE ? "N/A" :
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE ? "LFENCE" :
thunk == THUNK_JMP ? "JMP" : "?",
+ !use_spec_ctrl ? "No" :
+ (default_xen_spec_ctrl & SPEC_CTRL_IBRS) ? "IBRS+" : "IBRS-",
+ opt_ibpb ? " IBPB" : "");
+
+ /*
+ * Alternatives blocks for protecting against and/or virtualising
+ * mitigation support for guests.
+ */
+ printk(" Support for VMs: PV:%s%s%s, HVM:%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
- boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
- default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
- " IBRS-" : "",
- opt_ibpb ? " IBPB" : "",
- boot_cpu_has(X86_FEATURE_SC_RSB_PV) ? " RSB_NATIVE" : "",
- boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB_VMEXIT" : "");
+ boot_cpu_has(X86_FEATURE_SC_RSB_PV)) ? "" : " None",
+ boot_cpu_has(X86_FEATURE_SC_MSR_PV) ? " MSR_SPEC_CTRL" : "",
+ boot_cpu_has(X86_FEATURE_SC_RSB_PV) ? " RSB" : "",
+ (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM)) ? "" : " None",
+ boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ? " MSR_SPEC_CTRL" : "",
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB" : "");
printk("XPTI: %s\n",
boot_cpu_has(X86_FEATURE_NO_XPTI) ? "disabled" : "enabled");
@@ -212,7 +315,7 @@ static bool __init retpoline_safe(uint64_t caps)
void __init init_speculation_mitigations(void)
{
enum ind_thunk thunk = THUNK_DEFAULT;
- bool ibrs = false;
+ bool use_spec_ctrl = false, ibrs = false;
uint64_t caps = 0;
if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
@@ -282,20 +385,31 @@ void __init init_speculation_mitigations(void)
else if ( thunk == THUNK_JMP )
setup_force_cpu_cap(X86_FEATURE_IND_THUNK_JMP);
+ /*
+ * If we are on hardware supporting MSR_SPEC_CTRL, see about setting up
+ * the alternatives blocks so we can virtualise support for guests.
+ */
if ( boot_cpu_has(X86_FEATURE_IBRSB) )
{
- /*
- * Even if we've chosen to not have IBRS set in Xen context, we still
- * need the IBRS entry/exit logic to virtualise IBRS support for
- * guests.
- */
- setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
- setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
+ if ( opt_msr_sc_pv )
+ {
+ use_spec_ctrl = true;
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
+ }
- if ( ibrs )
- default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
+ if ( opt_msr_sc_hvm )
+ {
+ use_spec_ctrl = true;
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
+ }
+
+ if ( use_spec_ctrl )
+ {
+ if ( ibrs )
+ default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
- default_spec_ctrl_flags |= SCF_ist_wrmsr;
+ default_spec_ctrl_flags |= SCF_ist_wrmsr;
+ }
}
/*
--
2.1.4

View file

@ -1,123 +0,0 @@
From 918320daf34931cd5c1c0d9c439ce853f6575970 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Thu, 26 Apr 2018 10:56:28 +0100
Subject: [PATCH] x86/AMD: Mitigations for GPZ SP4 - Speculative Store Bypass
AMD processors will execute loads and stores with the same base register in
program order, which is typically how a compiler emits code.
Therefore, by default no mitigating actions are taken, despite there being
corner cases which are vulnerable to the issue.
For performance testing, or for users with particularly sensitive workloads,
the `spec-ctrl=ssbd` command line option is available to force Xen to disable
Memory Disambiguation on applicable hardware.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
docs/misc/xen-command-line.markdown | 7 ++++++-
xen/arch/x86/cpu/amd.c | 20 ++++++++++++++++++++
xen/arch/x86/spec_ctrl.c | 3 +++
xen/include/asm-x86/spec_ctrl.h | 1 +
4 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 43a6ddb..4e0e580 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1703,7 +1703,7 @@ false disable the quirk workaround, which is also the default.
### spec-ctrl (x86)
> `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>,
-> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb}=<bool> ]`
+> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd}=<bool> ]`
Controls for speculative execution sidechannel mitigations. By default, Xen
will pick the most appropriate mitigations based on compiled in support,
@@ -1747,6 +1747,11 @@ On hardware supporting IBPB (Indirect Branch Prediction Barrier), the `ibpb=`
option can be used to force (the default) or prevent Xen from issuing branch
prediction barriers on vcpu context switches.
+On hardware supporting SSBD (Speculative Store Bypass Disable), the `ssbd=`
+option can be used to force or prevent Xen using the feature itself. On AMD
+hardware, this is a global option applied at boot, and not virtualised for
+guest use.
+
### sync\_console
> `= <boolean>`
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index fc9677f..458a3fe 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -9,6 +9,7 @@
#include <asm/amd.h>
#include <asm/hvm/support.h>
#include <asm/setup.h> /* amd_init_cpu */
+#include <asm/spec_ctrl.h>
#include <asm/acpi.h>
#include <asm/apic.h>
@@ -594,6 +595,25 @@ static void init_amd(struct cpuinfo_x86 *c)
c->x86_capability);
}
+ /*
+ * If the user has explicitly chosen to disable Memory Disambiguation
+ * to mitigiate Speculative Store Bypass, poke the appropriate MSR.
+ */
+ if (opt_ssbd) {
+ int bit = -1;
+
+ switch (c->x86) {
+ case 0x15: bit = 54; break;
+ case 0x16: bit = 33; break;
+ case 0x17: bit = 10; break;
+ }
+
+ if (bit >= 0 && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) {
+ value |= 1ull << bit;
+ wrmsr_safe(MSR_AMD64_LS_CFG, value);
+ }
+ }
+
/* MFENCE stops RDTSC speculation */
if (!cpu_has_lfence_dispatch)
__set_bit(X86_FEATURE_MFENCE_RDTSC, c->x86_capability);
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 4f9282f..e326056 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -43,6 +43,7 @@ static enum ind_thunk {
} opt_thunk __initdata = THUNK_DEFAULT;
static int8_t __initdata opt_ibrs = -1;
bool __read_mostly opt_ibpb = true;
+bool __read_mostly opt_ssbd = false;
bool __initdata bsp_delay_spec_ctrl;
uint8_t __read_mostly default_xen_spec_ctrl;
@@ -164,6 +165,8 @@ static int __init parse_spec_ctrl(const char *s)
opt_ibrs = val;
else if ( (val = parse_boolean("ibpb", s, ss)) >= 0 )
opt_ibpb = val;
+ else if ( (val = parse_boolean("ssbd", s, ss)) >= 0 )
+ opt_ssbd = val;
else
rc = -EINVAL;
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index c6a38f4..4678a40 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -27,6 +27,7 @@
void init_speculation_mitigations(void);
extern bool opt_ibpb;
+extern bool opt_ssbd;
extern bool bsp_delay_spec_ctrl;
extern uint8_t default_xen_spec_ctrl;
--
2.1.4

View file

@ -1,224 +0,0 @@
From db6adc8e55dd43a1b4bb20e06a69475c503cb934 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Wed, 28 Mar 2018 15:21:39 +0100
Subject: [PATCH] x86/Intel: Mitigations for GPZ SP4 - Speculative Store Bypass
To combat GPZ SP4 "Speculative Store Bypass", Intel have extended their
speculative sidechannel mitigations specification as follows:
* A feature bit to indicate that Speculative Store Bypass Disable is
supported.
* A new bit in MSR_SPEC_CTRL which, when set, disables memory disambiguation
in the pipeline.
* A new bit in MSR_ARCH_CAPABILITIES, which will be set in future hardware,
indicating that the hardware is not susceptible to Speculative Store Bypass
sidechannels.
For contemporary processors, this interface will be implemented via a
microcode update.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
docs/misc/xen-command-line.markdown | 12 +++++++-----
tools/libxl/libxl_cpuid.c | 1 +
tools/misc/xen-cpuid.c | 3 +--
xen/arch/x86/cpuid.c | 5 +++++
xen/arch/x86/spec_ctrl.c | 15 ++++++++++++---
xen/include/asm-x86/msr-index.h | 2 ++
xen/include/public/arch-x86/cpufeatureset.h | 1 +
xen/tools/gen-cpuid.py | 17 +++++++++++++----
8 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 4e0e580..107889d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -496,9 +496,10 @@ accounting for hardware capabilities as enumerated via CPUID.
Currently accepted:
-The Speculation Control hardware features `ibrsb`, `stibp`, `ibpb` are used by
-default if avaiable. They can be ignored, e.g. `no-ibrsb`, at which point Xen
-won't use them itself, and won't offer them to guests.
+The Speculation Control hardware features `ibrsb`, `stibp`, `ibpb`, `ssbd` are
+used by default if available and applicable. They can be ignored,
+e.g. `no-ibrsb`, at which point Xen won't use them itself, and won't offer
+them to guests.
### cpuid\_mask\_cpu (AMD only)
> `= fam_0f_rev_c | fam_0f_rev_d | fam_0f_rev_e | fam_0f_rev_f | fam_0f_rev_g | fam_10_rev_b | fam_10_rev_c | fam_11_rev_b`
@@ -1728,7 +1729,7 @@ protect itself, and Xen's ability to virtualise support for guests to use.
respectively.
* `msr-sc=` offers control over Xen's support for manipulating MSR\_SPEC\_CTRL
on entry and exit. These blocks are necessary to virtualise support for
- guests and if disabled, guests will be unable to use IBRS/STIBP/etc.
+ guests and if disabled, guests will be unable to use IBRS/STIBP/SSBD/etc.
* `rsb=` offers control over whether to overwrite the Return Stack Buffer /
Return Address Stack on entry to Xen.
@@ -1750,7 +1751,8 @@ prediction barriers on vcpu context switches.
On hardware supporting SSBD (Speculative Store Bypass Disable), the `ssbd=`
option can be used to force or prevent Xen using the feature itself. On AMD
hardware, this is a global option applied at boot, and not virtualised for
-guest use.
+guest use. On Intel hardware, the feature is virtualised for guests,
+independently of Xen's choice of setting.
### sync\_console
> `= <boolean>`
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 3a21f4e..7b0f594 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -205,6 +205,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
{"ibrsb", 0x00000007, 0, CPUID_REG_EDX, 26, 1},
{"stibp", 0x00000007, 0, CPUID_REG_EDX, 27, 1},
{"arch-caps", 0x00000007, 0, CPUID_REG_EDX, 29, 1},
+ {"ssbd", 0x00000007, 0, CPUID_REG_EDX, 31, 1},
{"lahfsahf", 0x80000001, NA, CPUID_REG_ECX, 0, 1},
{"cmplegacy", 0x80000001, NA, CPUID_REG_ECX, 1, 1},
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index b1a46c6..2483a81 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -166,8 +166,7 @@ static const char *str_7d0[32] =
[26] = "ibrsb", [27] = "stibp",
[28] = "REZ", [29] = "arch_caps",
-
- [30 ... 31] = "REZ",
+ [30] = "REZ", [31] = "ssbd",
};
static struct {
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index b45b145..6a710b7 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -43,6 +43,11 @@ static int __init parse_xen_cpuid(const char *s)
if ( !val )
setup_clear_cpu_cap(X86_FEATURE_STIBP);
}
+ else if ( (val = parse_boolean("ssbd", s, ss)) >= 0 )
+ {
+ if ( !val )
+ setup_clear_cpu_cap(X86_FEATURE_SSBD);
+ }
else
rc = -EINVAL;
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index e326056..89e3825 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -192,26 +192,31 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
printk("Speculative mitigation facilities:\n");
/* Hardware features which pertain to speculative mitigations. */
- printk(" Hardware features:%s%s%s%s%s%s\n",
+ printk(" Hardware features:%s%s%s%s%s%s%s%s\n",
(_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
(_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP" : "",
+ (_7d0 & cpufeat_mask(X86_FEATURE_SSBD)) ? " SSBD" : "",
(e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "",
(caps & ARCH_CAPABILITIES_IBRS_ALL) ? " IBRS_ALL" : "",
(caps & ARCH_CAPABILITIES_RDCL_NO) ? " RDCL_NO" : "",
- (caps & ARCH_CAPS_RSBA) ? " RSBA" : "");
+ (caps & ARCH_CAPS_RSBA) ? " RSBA" : "",
+ (caps & ARCH_CAPS_SSB_NO) ? " SSB_NO" : "");
/* Compiled-in support which pertains to BTI mitigations. */
if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
printk(" Compiled-in support: INDIRECT_THUNK\n");
/* Settings for Xen's protection, irrespective of guests. */
- printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s, Other:%s\n",
+ printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s, Other:%s\n",
thunk == THUNK_NONE ? "N/A" :
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE ? "LFENCE" :
thunk == THUNK_JMP ? "JMP" : "?",
!use_spec_ctrl ? "No" :
(default_xen_spec_ctrl & SPEC_CTRL_IBRS) ? "IBRS+" : "IBRS-",
+ !use_spec_ctrl || !boot_cpu_has(X86_FEATURE_SSBD)
+ ? "" :
+ (default_xen_spec_ctrl & SPEC_CTRL_SSBD) ? " SSBD+" : " SSBD-",
opt_ibpb ? " IBPB" : "");
/*
@@ -415,6 +420,10 @@ void __init init_speculation_mitigations(void)
}
}
+ /* If we have SSBD available, see whether we should use it. */
+ if ( boot_cpu_has(X86_FEATURE_SSBD) && use_spec_ctrl && opt_ssbd )
+ default_xen_spec_ctrl |= SPEC_CTRL_SSBD;
+
/*
* PV guests can poison the RSB to any virtual address from which
* they can execute a call instruction. This is necessarily outside
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 68fae91..93d6f4e 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -38,6 +38,7 @@
#define MSR_SPEC_CTRL 0x00000048
#define SPEC_CTRL_IBRS (_AC(1, ULL) << 0)
#define SPEC_CTRL_STIBP (_AC(1, ULL) << 1)
+#define SPEC_CTRL_SSBD (_AC(1, ULL) << 2)
#define MSR_PRED_CMD 0x00000049
#define PRED_CMD_IBPB (_AC(1, ULL) << 0)
@@ -46,6 +47,7 @@
#define ARCH_CAPABILITIES_RDCL_NO (_AC(1, ULL) << 0)
#define ARCH_CAPABILITIES_IBRS_ALL (_AC(1, ULL) << 1)
#define ARCH_CAPS_RSBA (_AC(1, ULL) << 2)
+#define ARCH_CAPS_SSB_NO (_AC(1, ULL) << 4)
/* Intel MSRs. Some also available on other CPUs */
#define MSR_IA32_PERFCTR0 0x000000c1
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 8da5783..7acf822 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -245,6 +245,7 @@ XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A AVX512 Multiply Accumulation Single
XEN_CPUFEATURE(IBRSB, 9*32+26) /*A IBRS and IBPB support (used by Intel) */
XEN_CPUFEATURE(STIBP, 9*32+27) /*A! STIBP */
XEN_CPUFEATURE(ARCH_CAPS, 9*32+29) /* IA32_ARCH_CAPABILITIES MSR */
+XEN_CPUFEATURE(SSBD, 9*32+31) /* MSR_SPEC_CTRL.SSBD available */
#endif /* XEN_CPUFEATURE */
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 613b909..65526ff 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -257,10 +257,19 @@ def crunch_numbers(state):
AVX512BW, AVX512VL, AVX512VBMI, AVX512_4VNNIW,
AVX512_4FMAPS, AVX512_VPOPCNTDQ],
- # Single Thread Indirect Branch Predictors enumerates a new bit in the
- # MSR enumerated by Indirect Branch Restricted Speculation/Indirect
- # Branch Prediction Barrier enumeration.
- IBRSB: [STIBP],
+ # The features:
+ # * Single Thread Indirect Branch Predictors
+ # * Speculative Store Bypass Disable
+ #
+ # enumerate new bits in MSR_SPEC_CTRL, which is enumerated by Indirect
+ # Branch Restricted Speculation/Indirect Branch Prediction Barrier.
+ #
+ # In practice, these features also enumerate the presense of
+ # MSR_SPEC_CTRL. However, no real hardware will exist with SSBD but
+ # not IBRSB, and we pass this MSR directly to guests. Treating them
+ # as dependent features simplifies Xen's logic, and prevents the guest
+ # from seeing implausible configurations.
+ IBRSB: [STIBP, SSBD],
}
deep_features = tuple(sorted(deps.keys()))
--
2.1.4

View file

@ -1,70 +0,0 @@
From 02d0027a89dc49875a41e939498936874a32360f Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Fri, 13 Apr 2018 15:42:34 +0000
Subject: [PATCH] x86/msr: Virtualise MSR_SPEC_CTRL.SSBD for guests to use
Almost all infrastructure is already in place. Update the reserved bits
calculation in guest_wrmsr(), and offer SSBD to guests by default.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/msr.c | 8 ++++++--
xen/include/public/arch-x86/cpufeatureset.h | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 48d061d..21219c4 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -178,6 +178,8 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
switch ( msr )
{
+ uint64_t rsvd;
+
case MSR_INTEL_PLATFORM_INFO:
case MSR_ARCH_CAPABILITIES:
/* Read-only */
@@ -213,8 +215,10 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* Note: SPEC_CTRL_STIBP is specified as safe to use (i.e. ignored)
* when STIBP isn't enumerated in hardware.
*/
+ rsvd = ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP |
+ (cp->feat.ssbd ? SPEC_CTRL_SSBD : 0));
- if ( val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP) )
+ if ( val & rsvd )
goto gp_fault; /* Rsvd bit set? */
vp->spec_ctrl.raw = val;
@@ -233,12 +237,12 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
case MSR_INTEL_MISC_FEATURES_ENABLES:
{
- uint64_t rsvd = ~0ull;
bool old_cpuid_faulting = vp->misc_features_enables.cpuid_faulting;
if ( !vp->misc_features_enables.available )
goto gp_fault;
+ rsvd = ~0ull;
if ( dp->plaform_info.cpuid_faulting )
rsvd &= ~MSR_MISC_FEATURES_CPUID_FAULTING;
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 7acf822..c721c12 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -245,7 +245,7 @@ XEN_CPUFEATURE(AVX512_4FMAPS, 9*32+ 3) /*A AVX512 Multiply Accumulation Single
XEN_CPUFEATURE(IBRSB, 9*32+26) /*A IBRS and IBPB support (used by Intel) */
XEN_CPUFEATURE(STIBP, 9*32+27) /*A! STIBP */
XEN_CPUFEATURE(ARCH_CAPS, 9*32+29) /* IA32_ARCH_CAPABILITIES MSR */
-XEN_CPUFEATURE(SSBD, 9*32+31) /* MSR_SPEC_CTRL.SSBD available */
+XEN_CPUFEATURE(SSBD, 9*32+31) /*A MSR_SPEC_CTRL.SSBD available */
#endif /* XEN_CPUFEATURE */
--
2.1.4

View file

@ -1,52 +0,0 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/mm: don't bypass preemption checks
While unlikely, it is not impossible for a multi-vCPU guest to leverage
bypasses of preemption checks to drive Xen into an unbounded loop.
This is XSA-264.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2526,7 +2526,7 @@ static int _put_page_type(struct page_in
nx = x & ~(PGT_validated|PGT_partial);
if ( unlikely((y = cmpxchg(&page->u.inuse.type_info,
x, nx)) != x) )
- continue;
+ goto maybe_preempt;
/* We cleared the 'valid bit' so we do the clean up. */
rc = _put_final_page_type(page, x, preemptible, ptpg);
ptpg = NULL;
@@ -2558,12 +2558,13 @@ static int _put_page_type(struct page_in
*/
cpu_relax();
y = page->u.inuse.type_info;
- continue;
+ goto maybe_preempt;
}
if ( likely((y = cmpxchg(&page->u.inuse.type_info, x, nx)) == x) )
break;
+ maybe_preempt:
if ( preemptible && hypercall_preempt_check() )
return -EINTR;
}
@@ -2676,12 +2677,11 @@ static int __get_page_type(struct page_i
if ( !(x & PGT_partial) )
{
/* Someone else is updating validation of this page. Wait... */
- while ( (y = page->u.inuse.type_info) == x )
- {
+ do {
if ( preemptible && hypercall_preempt_check() )
return -EINTR;
cpu_relax();
- }
+ } while ( (y = page->u.inuse.type_info) == x );
continue;
}
/* Type ref count was left at 1 when PGT_partial got set. */

View file

@ -1,104 +0,0 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86: Refine checks in #DB handler for faulting conditions
One of the fix for XSA-260 (c/s 75d6828bc2 "x86/traps: Fix handling of #DB
exceptions in hypervisor context") added some safety checks to help avoid
livelocks of #DB faults.
While a General Detect #DB exception does have fault semantics, hardware
clears %dr7.gd on entry to the handler, meaning that it is actually safe to
return to. Furthermore, %dr6.gd is guest controlled and sticky (never cleared
by hardware). A malicious PV guest can therefore trigger the fatal_trap() and
crash Xen.
Instruction breakpoints are more tricky. The breakpoint match bits in %dr6
are not sticky, but the Intel manual warns that they may be set for
non-enabled breakpoints, so add a breakpoint enabled check.
Beyond that, because of the restriction on the linear addresses PV guests can
set, and the fault (rather than trap) nature of instruction breakpoints
(i.e. can't be deferred by a MovSS shadow), there should be no way to
encounter an instruction breakpoint in Xen context. However, for extra
robustness, deal with this situation by clearing the breakpoint configuration,
rather than crashing.
This is XSA-265
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e79ca88..3e05cf1 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1809,6 +1809,13 @@ void do_debug(struct cpu_user_regs *regs)
if ( !guest_mode(regs) )
{
+ /*
+ * !!! WARNING !!!
+ *
+ * %dr6 is mostly guest controlled at this point. Any decsions base
+ * on its value must be crosschecked with non-guest controlled state.
+ */
+
if ( regs->eflags & X86_EFLAGS_TF )
{
/* In SYSENTER entry path we can't zap TF until EFLAGS is saved. */
@@ -1830,33 +1837,44 @@ void do_debug(struct cpu_user_regs *regs)
* Check for fault conditions. General Detect, and instruction
* breakpoints are faults rather than traps, at which point attempting
* to ignore and continue will result in a livelock.
+ *
+ * However, on entering the #DB handler, hardware clears %dr7.gd for
+ * us (as confirmed by the earlier %dr6 accesses succeeding), meaning
+ * that a real General Detect exception is restartable.
+ *
+ * PV guests are not permitted to point %dr{0..3} at Xen linear
+ * addresses, and Instruction Breakpoints (being faults) don't get
+ * delayed by a MovSS shadow, so we should never encounter one in
+ * hypervisor context.
+ *
+ * If however we do, safety measures need to be enacted. Use a big
+ * hammer and clear all debug settings.
*/
- if ( dr6 & DR_GENERAL_DETECT )
- {
- printk(XENLOG_ERR "Hit General Detect in Xen context\n");
- fatal_trap(regs, 0);
- }
-
if ( dr6 & (DR_TRAP3 | DR_TRAP2 | DR_TRAP1 | DR_TRAP0) )
{
- unsigned int bp, dr7 = read_debugreg(7) >> DR_CONTROL_SHIFT;
+ unsigned int bp, dr7 = read_debugreg(7);
for ( bp = 0; bp < 4; ++bp )
{
if ( (dr6 & (1u << bp)) && /* Breakpoint triggered? */
- ((dr7 & (3u << (bp * DR_CONTROL_SIZE))) == 0) /* Insn? */ )
+ (dr7 & (3u << (bp * DR_ENABLE_SIZE))) && /* Enabled? */
+ ((dr7 & (3u << ((bp * DR_CONTROL_SIZE) + /* Insn? */
+ DR_CONTROL_SHIFT))) == DR_RW_EXECUTE) )
{
+ ASSERT_UNREACHABLE();
+
printk(XENLOG_ERR
"Hit instruction breakpoint in Xen context\n");
- fatal_trap(regs, 0);
+ write_debugreg(7, 0);
+ break;
}
}
}
/*
- * Whatever caused this #DB should be a trap. Note it and continue.
- * Guests can trigger this in certain corner cases, so ensure the
- * message is ratelimited.
+ * Whatever caused this #DB should be restartable by this point. Note
+ * it and continue. Guests can trigger this in certain corner cases,
+ * so ensure the message is ratelimited.
*/
gprintk(XENLOG_WARNING,
"Hit #DB in Xen context: %04x:%p [%ps], stk %04x:%p, dr6 %lx\n",

View file

@ -1,78 +0,0 @@
From 82f98f8484f47163a06e4d5610dba6e5fc459e78 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 13 Jun 2018 15:51:36 +0100
Subject: [PATCH 1/2] libxl: qemu_disk_scsi_drive_string: Break out common
parts of disk config
The generated configurations are identical apart from, in some cases,
reordering of the id=%s element. So, overall, no functional change.
This is part of XSA-266.
Reported-by: Andrew Reimers <andrew.reimers@orionvm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_dm.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b51178b..28bbeb6 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -798,6 +798,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path,
int colo_mode)
{
char *drive = NULL;
+ char *common = GCSPRINTF("cache=writeback");
const char *exportname = disk->colo_export;
const char *active_disk = disk->active_disk;
const char *hidden_disk = disk->hidden_disk;
@@ -805,8 +806,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path,
switch (colo_mode) {
case LIBXL__COLO_NONE:
drive = libxl__sprintf
- (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
- target_path, unit, format);
+ (gc, "%s,file=%s,if=scsi,bus=0,unit=%d,format=%s",
+ common, target_path, unit, format);
break;
case LIBXL__COLO_PRIMARY:
/*
@@ -819,13 +820,13 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path,
* vote-threshold=1
*/
drive = GCSPRINTF(
- "if=scsi,bus=0,unit=%d,cache=writeback,driver=quorum,"
+ "%s,if=scsi,bus=0,unit=%d,,driver=quorum,"
"id=%s,"
"children.0.file.filename=%s,"
"children.0.driver=%s,"
"read-pattern=fifo,"
"vote-threshold=1",
- unit, exportname, target_path, format);
+ common, unit, exportname, target_path, format);
break;
case LIBXL__COLO_SECONDARY:
/*
@@ -839,7 +840,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path,
* file.backing.backing=exportname,
*/
drive = GCSPRINTF(
- "if=scsi,id=top-colo,bus=0,unit=%d,cache=writeback,"
+ "%s,if=scsi,id=top-colo,bus=0,unit=%d,"
"driver=replication,"
"mode=secondary,"
"top-id=top-colo,"
@@ -848,7 +849,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path,
"file.backing.driver=qcow2,"
"file.backing.file.filename=%s,"
"file.backing.backing=%s",
- unit, active_disk, hidden_disk, exportname);
+ common, unit, active_disk, hidden_disk, exportname);
break;
default:
abort();
--
2.1.4

View file

@ -1,63 +0,0 @@
From ba3f1fa1ab6e83745682cac784e680a1abf7da7d Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Wed, 13 Jun 2018 15:54:53 +0100
Subject: [PATCH 2/2] libxl: restore passing "readonly=" to qemu for SCSI disks
A read-only check was introduced for XSA-142, commit ef6cb76026 ("libxl:
relax readonly check introduced by XSA-142 fix") added the passing of
the extra setting, but commit dab0539568 ("Introduce COLO mode and
refactor relevant function") dropped the passing of the setting again,
quite likely due to improper re-basing.
Restore the readonly= parameter to SCSI disks. For IDE disks this is
supposed to be rejected; add an assert. And there is a bare ad-hoc
disk drive string in libxl__build_device_model_args_new, which we also
update.
This is XSA-266.
Reported-by: Andrew Reimers <andrew.reimers@orionvm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_dm.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 28bbeb6..3dc317a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -798,7 +798,8 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path,
int colo_mode)
{
char *drive = NULL;
- char *common = GCSPRINTF("cache=writeback");
+ char *common = GCSPRINTF("cache=writeback,readonly=%s",
+ disk->readwrite ? "off" : "on");
const char *exportname = disk->colo_export;
const char *active_disk = disk->active_disk;
const char *hidden_disk = disk->hidden_disk;
@@ -867,6 +868,8 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path,
const char *exportname = disk->colo_export;
const char *active_disk = disk->active_disk;
const char *hidden_disk = disk->hidden_disk;
+
+ assert(disk->readwrite); /* should have been checked earlier */
switch (colo_mode) {
case LIBXL__COLO_NONE:
@@ -1576,8 +1579,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
if (strncmp(disks[i].vdev, "sd", 2) == 0) {
if (colo_mode == LIBXL__COLO_SECONDARY) {
drive = libxl__sprintf
- (gc, "if=none,driver=%s,file=%s,id=%s",
- format, target_path, disks[i].colo_export);
+ (gc, "if=none,driver=%s,file=%s,id=%s,readonly=%s",
+ format, target_path, disks[i].colo_export,
+ disks[i].readwrite ? "off" : "on");
flexarray_append(dm_args, "-drive");
flexarray_append(dm_args, drive);
--
2.1.4

View file

@ -1,68 +0,0 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86: Support fully eager FPU context switching
This is controlled on a per-vcpu bases for flexibility.
This is part of XSA-267 / CVE-2018-3665
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 8845252..50116d5 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -210,7 +210,7 @@ void vcpu_restore_fpu_eager(struct vcpu *v)
ASSERT(!is_idle_vcpu(v));
/* Restore nonlazy extended state (i.e. parts not tracked by CR0.TS). */
- if ( !v->arch.nonlazy_xstate_used )
+ if ( !v->arch.fully_eager_fpu && !v->arch.nonlazy_xstate_used )
return;
/* Avoid recursion */
@@ -221,11 +221,19 @@ void vcpu_restore_fpu_eager(struct vcpu *v)
* above) we also need to restore full state, to prevent subsequently
* saving state belonging to another vCPU.
*/
- if ( xstate_all(v) )
+ if ( v->arch.fully_eager_fpu || (v->arch.xsave_area && xstate_all(v)) )
{
- fpu_xrstor(v, XSTATE_ALL);
+ if ( cpu_has_xsave )
+ fpu_xrstor(v, XSTATE_ALL);
+ else
+ fpu_fxrstor(v);
+
v->fpu_initialised = 1;
v->fpu_dirtied = 1;
+
+ /* Xen doesn't need TS set, but the guest might. */
+ if ( is_pv_vcpu(v) && (v->arch.pv_vcpu.ctrlreg[0] & X86_CR0_TS) )
+ stts();
}
else
{
@@ -247,6 +255,8 @@ void vcpu_restore_fpu_lazy(struct vcpu *v)
if ( v->fpu_dirtied )
return;
+ ASSERT(!v->arch.fully_eager_fpu);
+
if ( cpu_has_xsave )
fpu_xrstor(v, XSTATE_LAZY);
else
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 2ba21e1..7759332 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -569,6 +569,9 @@ struct arch_vcpu
* and thus should be saved/restored. */
bool_t nonlazy_xstate_used;
+ /* Restore all FPU state (lazy and non-lazy state) on context switch? */
+ bool fully_eager_fpu;
+
/*
* The SMAP check policy when updating runstate_guest(v) and the
* secondary system time.

View file

@ -1,220 +0,0 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/spec-ctrl: Mitigations for LazyFPU
Intel Core processors since at least Nehalem speculate past #NM, which is the
mechanism by which lazy FPU context switching is implemented.
On affected processors, Xen must use fully eager FPU context switching to
prevent guests from being able to read FPU state (SSE/AVX/etc) from previously
scheduled vcpus.
This is part of XSA-267 / CVE-2018-3665
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 5c13f6f..cb81a9c 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1727,7 +1727,7 @@ false disable the quirk workaround, which is also the default.
### spec-ctrl (x86)
> `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>,
-> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd}=<bool> ]`
+> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd,eager-fpu}=<bool> ]`
Controls for speculative execution sidechannel mitigations. By default, Xen
will pick the most appropriate mitigations based on compiled in support,
@@ -1777,6 +1777,11 @@ hardware, this is a global option applied at boot, and not virtualised for
guest use. On Intel hardware, the feature is virtualised for guests,
independently of Xen's choice of setting.
+On all hardware, the `eager-fpu=` option can be used to force or prevent Xen
+from using fully eager FPU context switches. This is currently implemented as
+a global control. By default, Xen will choose to use fully eager context
+switches on hardware believed to speculate past #NM exceptions.
+
### sync\_console
> `= <boolean>`
diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 50116d5..dbdf1b4 100644
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -15,6 +15,7 @@
#include <asm/i387.h>
#include <asm/xstate.h>
#include <asm/asm_defns.h>
+#include <asm/spec_ctrl.h>
/*******************************/
/* FPU Restore Functions */
@@ -307,6 +308,8 @@ int vcpu_init_fpu(struct vcpu *v)
{
int rc;
+ v->arch.fully_eager_fpu = opt_eager_fpu;
+
if ( (rc = xstate_alloc_save_area(v)) != 0 )
return rc;
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index b5a7f9c..cf255ac 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -44,6 +44,7 @@ static enum ind_thunk {
static int8_t __initdata opt_ibrs = -1;
bool __read_mostly opt_ibpb = true;
bool __read_mostly opt_ssbd = false;
+int8_t __read_mostly opt_eager_fpu = -1;
bool __initdata bsp_delay_spec_ctrl;
uint8_t __read_mostly default_xen_spec_ctrl;
@@ -114,6 +115,7 @@ static int __init parse_spec_ctrl(const char *s)
opt_thunk = THUNK_JMP;
opt_ibrs = 0;
opt_ibpb = false;
+ opt_eager_fpu = 0;
}
else if ( val > 0 )
rc = -EINVAL;
@@ -167,6 +169,8 @@ static int __init parse_spec_ctrl(const char *s)
opt_ibpb = val;
else if ( (val = parse_boolean("ssbd", s, ss)) >= 0 )
opt_ssbd = val;
+ else if ( (val = parse_boolean("eager-fpu", s, ss)) >= 0 )
+ opt_eager_fpu = val;
else
rc = -EINVAL;
@@ -223,15 +227,19 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
* Alternatives blocks for protecting against and/or virtualising
* mitigation support for guests.
*/
- printk(" Support for VMs: PV:%s%s%s, HVM:%s%s%s\n",
+ printk(" Support for VMs: PV:%s%s%s%s, HVM:%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
- boot_cpu_has(X86_FEATURE_SC_RSB_PV)) ? "" : " None",
+ boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
+ opt_eager_fpu) ? "" : " None",
boot_cpu_has(X86_FEATURE_SC_MSR_PV) ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_PV) ? " RSB" : "",
+ opt_eager_fpu ? " EAGER_FPU" : "",
(boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
- boot_cpu_has(X86_FEATURE_SC_RSB_HVM)) ? "" : " None",
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
+ opt_eager_fpu) ? "" : " None",
boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ? " MSR_SPEC_CTRL" : "",
- boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB" : "");
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB" : "",
+ opt_eager_fpu ? " EAGER_FPU" : "");
printk("XPTI: %s\n",
boot_cpu_has(X86_FEATURE_NO_XPTI) ? "disabled" : "enabled");
@@ -321,6 +329,82 @@ static bool __init retpoline_safe(uint64_t caps)
}
}
+/* Calculate whether this CPU speculates past #NM */
+static bool __init should_use_eager_fpu(void)
+{
+ /*
+ * Assume all unrecognised processors are ok. This is only known to
+ * affect Intel Family 6 processors.
+ */
+ if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ boot_cpu_data.x86 != 6 )
+ return false;
+
+ switch ( boot_cpu_data.x86_model )
+ {
+ /*
+ * Core processors since at least Nehalem are vulnerable.
+ */
+ case 0x1e: /* Nehalem */
+ case 0x1f: /* Auburndale / Havendale */
+ case 0x1a: /* Nehalem EP */
+ case 0x2e: /* Nehalem EX */
+ case 0x25: /* Westmere */
+ case 0x2c: /* Westmere EP */
+ case 0x2f: /* Westmere EX */
+ case 0x2a: /* SandyBridge */
+ case 0x2d: /* SandyBridge EP/EX */
+ case 0x3a: /* IvyBridge */
+ case 0x3e: /* IvyBridge EP/EX */
+ case 0x3c: /* Haswell */
+ case 0x3f: /* Haswell EX/EP */
+ case 0x45: /* Haswell D */
+ case 0x46: /* Haswell H */
+ case 0x3d: /* Broadwell */
+ case 0x47: /* Broadwell H */
+ case 0x4f: /* Broadwell EP/EX */
+ case 0x56: /* Broadwell D */
+ case 0x4e: /* Skylake M */
+ case 0x55: /* Skylake X */
+ case 0x5e: /* Skylake D */
+ case 0x66: /* Cannonlake */
+ case 0x67: /* Cannonlake? */
+ case 0x8e: /* Kabylake M */
+ case 0x9e: /* Kabylake D */
+ return true;
+
+ /*
+ * Atom processors are not vulnerable.
+ */
+ case 0x1c: /* Pineview */
+ case 0x26: /* Lincroft */
+ case 0x27: /* Penwell */
+ case 0x35: /* Cloverview */
+ case 0x36: /* Cedarview */
+ case 0x37: /* Baytrail / Valleyview (Silvermont) */
+ case 0x4d: /* Avaton / Rangely (Silvermont) */
+ case 0x4c: /* Cherrytrail / Brasswell */
+ case 0x4a: /* Merrifield */
+ case 0x5a: /* Moorefield */
+ case 0x5c: /* Goldmont */
+ case 0x5f: /* Denverton */
+ case 0x7a: /* Gemini Lake */
+ return false;
+
+ /*
+ * Knights processors are not vulnerable.
+ */
+ case 0x57: /* Knights Landing */
+ case 0x85: /* Knights Mill */
+ return false;
+
+ default:
+ printk("Unrecognised CPU model %#x - assuming vulnerable to LazyFPU\n",
+ boot_cpu_data.x86_model);
+ return true;
+ }
+}
+
void __init init_speculation_mitigations(void)
{
enum ind_thunk thunk = THUNK_DEFAULT;
@@ -519,6 +603,10 @@ void __init init_speculation_mitigations(void)
if ( !boot_cpu_has(X86_FEATURE_IBRSB) && !boot_cpu_has(X86_FEATURE_IBPB) )
opt_ibpb = false;
+ /* Check whether Eager FPU should be enabled by default. */
+ if ( opt_eager_fpu == -1 )
+ opt_eager_fpu = should_use_eager_fpu();
+
/* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
init_shadow_spec_ctrl_state();
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 91bed1b..5b40afb 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -28,6 +28,7 @@ void init_speculation_mitigations(void);
extern bool opt_ibpb;
extern bool opt_ssbd;
+extern int8_t opt_eager_fpu;
extern bool bsp_delay_spec_ctrl;
extern uint8_t default_xen_spec_ctrl;

31
xsa284.patch Normal file
View file

@ -0,0 +1,31 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: gnttab: set page refcount for copy-on-grant-transfer
Commit 5cc77f9098 ("32-on-64: Fix domain address-size clamping,
implement"), which introduced this functionality, took care of clearing
the old page's PGC_allocated, but failed to set the bit (and install the
associated reference) on the newly allocated one. Furthermore the "mfn"
local variable was never updated, and hence the wrong MFN was passed to
guest_physmap_add_page() (and back to the destination domain) in this
case, leading to an IOMMU mapping into an unowned page.
Ideally the code would use assign_pages(), but the call to
gnttab_prepare_for_transfer() sits in the middle of the actions
mirroring that function.
This is XSA-284.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2183,6 +2183,8 @@ gnttab_transfer(
page->count_info &= ~(PGC_count_mask|PGC_allocated);
free_domheap_page(page);
page = new_page;
+ page->count_info = PGC_allocated | 1;
+ mfn = page_to_mfn(page);
}
spin_lock(&e->page_alloc_lock);

43
xsa285-4.11.patch Normal file
View file

@ -0,0 +1,43 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: IOMMU/x86: fix type ref-counting race upon IOMMU page table construction
When arch_iommu_populate_page_table() gets invoked for an already
running guest, simply looking at page types once isn't enough, as they
may change at any time. Add logic to re-check the type after having
mapped the page, unmapping it again if needed.
This is XSA-285.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tentatively-Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -68,6 +68,27 @@ int arch_iommu_populate_page_table(struct domain *d)
rc = hd->platform_ops->map_page(d, gfn, mfn,
IOMMUF_readable |
IOMMUF_writable);
+
+ /*
+ * We may be working behind the back of a running guest, which
+ * may change the type of a page at any time. We can't prevent
+ * this (for instance, by bumping the type count while mapping
+ * the page) without causing legitimate guest type-change
+ * operations to fail. So after adding the page to the IOMMU,
+ * check again to make sure this is still valid. NB that the
+ * writable entry in the iommu is harmless until later, when
+ * the actual device gets assigned.
+ */
+ if ( !rc && !is_hvm_domain(d) &&
+ ((page->u.inuse.type_info & PGT_type_mask) !=
+ PGT_writable_page) )
+ {
+ rc = hd->platform_ops->unmap_page(d, gfn);
+ /* If the type changed yet again, simply force a retry. */
+ if ( !rc && ((page->u.inuse.type_info & PGT_type_mask) ==
+ PGT_writable_page) )
+ rc = -ERESTART;
+ }
}
if ( rc )
{

328
xsa287-4.10.patch Normal file
View file

@ -0,0 +1,328 @@
From 9b1f6192e4b0366dc09cea905d3e0a4b82cad45b Mon Sep 17 00:00:00 2001
From: George Dunlap <george.dunlap@citrix.com>
Date: Fri, 18 Jan 2019 15:00:34 +0000
Subject: [PATCH] steal_page: Get rid of bogus struct page states
The original rules for `struct page` required the following invariants
at all times:
- refcount > 0 implies owner != NULL
- PGC_allocated implies refcount > 0
steal_page, in a misguided attempt to protect against unknown races,
violates both of these rules, thus introducing other races:
- Temporarily, the count_info has the refcount go to 0 while
PGC_allocated is set
- It explicitly returns the page PGC_allocated set, but owner == NULL
and page not on the page_list.
The second one meant that page_get_owner_and_reference() could return
NULL even after having successfully grabbed a reference on the page,
leading the caller to leak the reference (since "couldn't get ref" and
"got ref but no owner" look the same).
Furthermore, rather than grabbing a page reference to ensure that the
owner doesn't change under its feet, it appears to rely on holding
d->page_alloc lock to prevent this.
Unfortunately, this is ineffective: page->owner remains non-NULL for
some time after the count has been set to 0; meaning that it would be
entirely possible for the page to be freed and re-allocated to a
different domain between the page_get_owner() check and the count_info
check.
Modify steal_page to instead follow the appropriate access discipline,
taking the page through series of states similar to being freed and
then re-allocated with MEMF_no_owner:
- Grab an extra reference to make sure we don't race with anyone else
freeing the page
- Drop both references and PGC_allocated atomically, so that (if
successful), anyone else trying to grab a reference will fail
- Attempt to reset Xen's mappings
- Reset the rest of the state.
Then, modify the two callers appropriately:
- Leave count_info alone (it's already been cleared)
- Call free_domheap_page() directly if appropriate
- Call assign_pages() rather than open-coding a partial assign
With all callers to assign_pages() now passing in pages with the
type_info field clear, tighten the respective assertion there.
This is XSA-287.
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/mm.c | 84 ++++++++++++++++++++++++++++------------
xen/common/grant_table.c | 20 +++++-----
xen/common/memory.c | 19 +++++----
xen/common/page_alloc.c | 2 +-
4 files changed, 83 insertions(+), 42 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index c20edb954d..67f6cfc1d5 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3924,70 +3924,106 @@ int donate_page(
return -EINVAL;
}
+/*
+ * Steal page will attempt to remove `page` from domain `d`. Upon
+ * return, `page` will be in a state similar to the state of a page
+ * returned from alloc_domheap_page() with MEMF_no_owner set:
+ * - refcount 0
+ * - type count cleared
+ * - owner NULL
+ * - page caching attributes cleaned up
+ * - removed from the domain's page_list
+ *
+ * If MEMF_no_refcount is not set, the domain's tot_pages will be
+ * adjusted. If this results in the page count falling to 0,
+ * put_domain() will be called.
+ *
+ * The caller should either call free_domheap_page() to free the
+ * page, or assign_pages() to put it back on some domain's page list.
+ */
int steal_page(
struct domain *d, struct page_info *page, unsigned int memflags)
{
unsigned long x, y;
bool drop_dom_ref = false;
- const struct domain *owner = dom_xen;
+ const struct domain *owner;
+ int rc;
if ( paging_mode_external(d) )
return -EOPNOTSUPP;
- spin_lock(&d->page_alloc_lock);
-
- if ( is_xen_heap_page(page) || ((owner = page_get_owner(page)) != d) )
+ /* Grab a reference to make sure the page doesn't change under our feet */
+ rc = -EINVAL;
+ if ( !(owner = page_get_owner_and_reference(page)) )
goto fail;
+ if ( owner != d || is_xen_heap_page(page) )
+ goto fail_put;
+
/*
- * We require there is just one reference (PGC_allocated). We temporarily
- * drop this reference now so that we can safely swizzle the owner.
+ * We require there are exactly two references -- the one we just
+ * took, and PGC_allocated. We temporarily drop both these
+ * references so that the page becomes effectively non-"live" for
+ * the domain.
*/
y = page->count_info;
do {
x = y;
- if ( (x & (PGC_count_mask|PGC_allocated)) != (1 | PGC_allocated) )
- goto fail;
- y = cmpxchg(&page->count_info, x, x & ~PGC_count_mask);
+ if ( (x & (PGC_count_mask|PGC_allocated)) != (2 | PGC_allocated) )
+ goto fail_put;
+ y = cmpxchg(&page->count_info, x, x & ~(PGC_count_mask|PGC_allocated));
} while ( y != x );
/*
- * With the sole reference dropped temporarily, no-one can update type
- * information. Type count also needs to be zero in this case, but e.g.
- * PGT_seg_desc_page may still have PGT_validated set, which we need to
- * clear before transferring ownership (as validation criteria vary
- * depending on domain type).
+ * NB this is safe even if the page ends up being given back to
+ * the domain, because the count is zero: subsequent mappings will
+ * cause the cache attributes to be re-instated inside
+ * get_page_from_l1e().
+ */
+ if ( (rc = cleanup_page_cacheattr(page)) )
+ {
+ /*
+ * Couldn't fixup Xen's mappings; put things the way we found
+ * it and return an error
+ */
+ page->count_info |= PGC_allocated | 1;
+ goto fail;
+ }
+
+ /*
+ * With the reference count now zero, nobody can grab references
+ * to do anything else with the page. Return the page to a state
+ * that it might be upon return from alloc_domheap_pages with
+ * MEMF_no_owner set.
*/
+ spin_lock(&d->page_alloc_lock);
+
BUG_ON(page->u.inuse.type_info & (PGT_count_mask | PGT_locked |
PGT_pinned));
page->u.inuse.type_info = 0;
-
- /* Swizzle the owner then reinstate the PGC_allocated reference. */
page_set_owner(page, NULL);
- y = page->count_info;
- do {
- x = y;
- BUG_ON((x & (PGC_count_mask|PGC_allocated)) != PGC_allocated);
- } while ( (y = cmpxchg(&page->count_info, x, x | 1)) != x );
+ page_list_del(page, &d->page_list);
/* Unlink from original owner. */
if ( !(memflags & MEMF_no_refcount) && !domain_adjust_tot_pages(d, -1) )
drop_dom_ref = true;
- page_list_del(page, &d->page_list);
spin_unlock(&d->page_alloc_lock);
+
if ( unlikely(drop_dom_ref) )
put_domain(d);
+
return 0;
+ fail_put:
+ put_page(page);
fail:
- spin_unlock(&d->page_alloc_lock);
gdprintk(XENLOG_WARNING, "Bad steal mfn %" PRI_mfn
" from d%d (owner d%d) caf=%08lx taf=%" PRtype_info "\n",
mfn_x(page_to_mfn(page)), d->domain_id,
owner ? owner->domain_id : DOMID_INVALID,
page->count_info, page->u.inuse.type_info);
- return -EINVAL;
+ return rc;
}
static int __do_update_va_mapping(
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index e6790d5fac..42ea50f485 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2175,7 +2175,7 @@ gnttab_transfer(
rcu_unlock_domain(e);
put_gfn_and_copyback:
put_gfn(d, gop.mfn);
- page->count_info &= ~(PGC_count_mask|PGC_allocated);
+ /* The count_info has already been cleaned */
free_domheap_page(page);
goto copyback;
}
@@ -2198,10 +2198,9 @@ gnttab_transfer(
copy_domain_page(_mfn(page_to_mfn(new_page)), _mfn(mfn));
- page->count_info &= ~(PGC_count_mask|PGC_allocated);
+ /* The count_info has already been cleared */
free_domheap_page(page);
page = new_page;
- page->count_info = PGC_allocated | 1;
mfn = page_to_mfn(page);
}
@@ -2241,12 +2240,17 @@ gnttab_transfer(
*/
spin_unlock(&e->page_alloc_lock);
okay = gnttab_prepare_for_transfer(e, d, gop.ref);
- spin_lock(&e->page_alloc_lock);
- if ( unlikely(!okay) || unlikely(e->is_dying) )
+ if ( unlikely(!okay || assign_pages(e, page, 0, MEMF_no_refcount)) )
{
- bool_t drop_dom_ref = !domain_adjust_tot_pages(e, -1);
+ bool drop_dom_ref;
+ /*
+ * Need to grab this again to safely free our "reserved"
+ * page in the page total
+ */
+ spin_lock(&e->page_alloc_lock);
+ drop_dom_ref = !domain_adjust_tot_pages(e, -1);
spin_unlock(&e->page_alloc_lock);
if ( okay /* i.e. e->is_dying due to the surrounding if() */ )
@@ -2259,10 +2263,6 @@ gnttab_transfer(
goto unlock_and_copyback;
}
- page_list_add_tail(page, &e->page_list);
- page_set_owner(page, e);
-
- spin_unlock(&e->page_alloc_lock);
put_gfn(d, gop.mfn);
TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 088083e4d1..8aaecb4495 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -631,20 +631,22 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
* Success! Beyond this point we cannot fail for this chunk.
*/
- /* Destroy final reference to each input page. */
+ /*
+ * These pages have already had owner and reference cleared.
+ * Do the final two steps: Remove from the physmap, and free
+ * them.
+ */
while ( (page = page_list_remove_head(&in_chunk_list)) )
{
unsigned long gfn;
- if ( !test_and_clear_bit(_PGC_allocated, &page->count_info) )
- BUG();
mfn = page_to_mfn(page);
gfn = mfn_to_gmfn(d, mfn);
/* Pages were unshared above */
BUG_ON(SHARED_M2P(gfn));
if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), 0) )
domain_crash(d);
- put_page(page);
+ free_domheap_page(page);
}
/* Assign each output page to the domain. */
@@ -717,13 +719,16 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
* chunks succeeded.
*/
fail:
- /* Reassign any input pages we managed to steal. */
+ /*
+ * Reassign any input pages we managed to steal. NB that if the assign
+ * fails again, we're on the hook for freeing the page, since we've already
+ * cleared PGC_allocated.
+ */
while ( (page = page_list_remove_head(&in_chunk_list)) )
if ( assign_pages(d, page, 0, MEMF_no_refcount) )
{
BUG_ON(!d->is_dying);
- if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
- put_page(page);
+ free_domheap_page(page);
}
dying:
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 598c3432c9..15a6f018f8 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2219,7 +2219,7 @@ int assign_pages(
for ( i = 0; i < (1 << order); i++ )
{
ASSERT(page_get_owner(&pg[i]) == NULL);
- ASSERT((pg[i].count_info & ~(PGC_allocated | 1)) == 0);
+ ASSERT(!pg[i].count_info);
page_set_owner(&pg[i], d);
smp_wmb(); /* Domain pointer must be visible before updating refcnt. */
pg[i].count_info = PGC_allocated | 1;
--
2.20.1

308
xsa288-4.11.patch Normal file
View file

@ -0,0 +1,308 @@
From 5d3a02e320f88747b75e3794c2e694284ae64c3e Mon Sep 17 00:00:00 2001
From: George Dunlap <george.dunlap@citrix.com>
Date: Wed, 23 Jan 2019 11:57:46 +0000
Subject: [PATCH] xen: Make coherent PV IOMMU discipline
In order for a PV domain to set up DMA from a passed-through device to
one of its pages, the page must be mapped in the IOMMU. On the other
hand, before a PV page may be used as a "special" page type (such as a
pagetable or descriptor table), it _must not_ be writable in the IOMMU
(otherwise a malicious guest could DMA arbitrary page tables into the
memory, bypassing Xen's safety checks); and Xen's current rule is to
have such pages not in the IOMMU at all.
At the moment, in order to accomplish this, the code borrows HVM
domain's "physmap" concept: When a page is assigned to a guest,
guess_physmap_add_entry() is called, which for PV guests, will create
a writable IOMMU mapping; and when a page is removed,
guest_physmap_remove_entry() is called, which will remove the mapping.
Additionally, when a page gains the PGT_writable page type, the page
will be added into the IOMMU; and when the page changes away from a
PGT_writable type, the page will be removed from the IOMMU.
Unfortunately, borrowing the "physmap" concept from HVM domains is
problematic. HVM domains have a lock on their p2m tables, ensuring
synchronization between modifications to the p2m; and all hypercall
parameters must first be translated through the p2m before being used.
Trying to mix this locked-and-gated approach with PV's lock-free
approach leads to several races and inconsistencies:
* A race between a page being assigned and it being put into the
physmap; for example:
- P1: call populate_physmap() { A = allocate_domheap_pages() }
- P2: Guess page A's mfn, and call decrease_reservation(A). A is owned by the domain,
and so Xen will clear the PGC_allocated bit and free the page
- P1: finishes populate_physmap() { guest_physmap_add_entry() }
Now the domain has a writable IOMMU mapping to a page it no longer owns.
* Pages start out as type PGT_none, but with a writable IOMMU mapping.
If a guest uses a page as a page table without ever having created a
writable mapping, the IOMMU mapping will not be removed; the guest
will have a writable IOMMU mapping to a page it is currently using
as a page table.
* A newly-allocated page can be DMA'd into with no special actions on
the part of the guest; However, if a page is promoted to a
non-writable type, the page must be mapped with a writable type before
DMA'ing to it again, or the transaction will fail.
To fix this, do away with the "PV physmap" concept entirely, and
replace it with the following IOMMU discipline for PV guests:
- (type == PGT_writable) <=> in iommu (even if type_count == 0)
- Upon a final put_page(), check to see if type is PGT_writable; if so,
iommu_unmap.
In order to achieve that:
- Remove PV IOMMU related code from guest_physmap_*
- Repurpose cleanup_page_cacheattr() into a general
cleanup_page_mappings() function, which will both fix up Xen
mappings for pages with special cache attributes, and also check for
a PGT_writable type and remove pages if appropriate.
- For compatibility with current guests, grab-and-release a
PGT_writable_page type for PV guests in guest_physmap_add_entry().
This will cause most "normal" guest pages to start out life with
PGT_writable_page type (and thus an IOMMU mapping), but no type
count (so that they can be used as special cases at will).
Also, note that there is one exception to to the "PGT_writable => in
iommu" rule: xenheap pages shared with guests may be given a
PGT_writable type with one type reference. This reference prevents
the type from changing, which in turn prevents page from gaining an
IOMMU mapping in get_page_type(). It's not clear whether this was
intentional or not, but it's not something to change in a security
update.
This is XSA-288.
Reported-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/mm.c | 95 +++++++++++++++++++++++++++++++++++++++----
xen/arch/x86/mm/p2m.c | 57 ++++++++++++--------------
2 files changed, 111 insertions(+), 41 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index d8ff58c901..ad8aacad68 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -81,6 +81,22 @@
* OS's, which will generally use the WP bit to simplify copy-on-write
* implementation (in that case, OS wants a fault when it writes to
* an application-supplied buffer).
+ *
+ * PV domUs and IOMMUs:
+ * --------------------
+ * For a guest to be able to DMA into a page, that page must be in the
+ * domain's IOMMU. However, we *must not* allow DMA into 'special'
+ * pages (such as page table pages, descriptor tables, &c); and we
+ * must also ensure that mappings are removed from the IOMMU when the
+ * page is freed. Finally, it is inherently racy to make any changes
+ * based on a page with a non-zero type count.
+ *
+ * To that end, we put the page in the IOMMU only when a page gains
+ * the PGT_writeable type; and we remove the page when it loses the
+ * PGT_writeable type (not when the type count goes to zero). This
+ * effectively protects the IOMMU status update with the type count we
+ * have just acquired. We must also check for PGT_writable type when
+ * doing the final put_page(), and remove it from the iommu if so.
*/
#include <xen/init.h>
@@ -2275,19 +2291,79 @@ static int mod_l4_entry(l4_pgentry_t *pl4e,
return rc;
}
-static int cleanup_page_cacheattr(struct page_info *page)
+/*
+ * In the course of a page's use, it may have caused other secondary
+ * mappings to have changed:
+ * - Xen's mappings may have been changed to accomodate the requested
+ * cache attibutes
+ * - A page may have been put into the IOMMU of a PV guest when it
+ * gained a writable mapping.
+ *
+ * Now that the page is being freed, clean up these mappings if
+ * appropriate. NB that at this point the page is still "allocated",
+ * but not "live" (i.e., its refcount is 0), so it's safe to read the
+ * count_info, owner, and type_info without synchronization.
+ */
+static int cleanup_page_mappings(struct page_info *page)
{
unsigned int cacheattr =
(page->count_info & PGC_cacheattr_mask) >> PGC_cacheattr_base;
+ int rc = 0;
+ unsigned long mfn = mfn_x(page_to_mfn(page));
- if ( likely(cacheattr == 0) )
- return 0;
+ /*
+ * If we've modified xen mappings as a result of guest cache
+ * attributes, restore them to the "normal" state.
+ */
+ if ( unlikely(cacheattr) )
+ {
+ page->count_info &= ~PGC_cacheattr_mask;
- page->count_info &= ~PGC_cacheattr_mask;
+ BUG_ON(is_xen_heap_page(page));
- BUG_ON(is_xen_heap_page(page));
+ rc = update_xen_mappings(mfn, 0);
+ }
- return update_xen_mappings(mfn_x(page_to_mfn(page)), 0);
+ /*
+ * If this may be in a PV domain's IOMMU, remove it.
+ *
+ * NB that writable xenheap pages have their type set and cleared by
+ * implementation-specific code, rather than by get_page_type(). As such:
+ * - They aren't expected to have an IOMMU mapping, and
+ * - We don't necessarily expect the type count to be zero when the final
+ * put_page happens.
+ *
+ * Go ahead and attemp to call iommu_unmap() on xenheap pages anyway, just
+ * in case; but only ASSERT() that the type count is zero and remove the
+ * PGT_writable type for non-xenheap pages.
+ */
+ if ( (page->u.inuse.type_info & PGT_type_mask) == PGT_writable_page )
+ {
+ struct domain *d = page_get_owner(page);
+
+ if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
+ {
+ int rc2 = iommu_unmap_page(d, mfn);
+
+ if ( !rc )
+ rc = rc2;
+ }
+
+ if ( likely(!is_xen_heap_page(page)) )
+ {
+ ASSERT((page->u.inuse.type_info &
+ (PGT_type_mask | PGT_count_mask)) == PGT_writable_page);
+ /*
+ * Clear the type to record the fact that all writable mappings
+ * have been removed. But if either operation failed, leave
+ * type_info alone.
+ */
+ if ( likely(!rc) )
+ page->u.inuse.type_info &= ~(PGT_type_mask | PGT_count_mask);
+ }
+ }
+
+ return rc;
}
void put_page(struct page_info *page)
@@ -2303,7 +2379,7 @@ void put_page(struct page_info *page)
if ( unlikely((nx & PGC_count_mask) == 0) )
{
- if ( cleanup_page_cacheattr(page) == 0 )
+ if ( !cleanup_page_mappings(page) )
free_domheap_page(page);
else
gdprintk(XENLOG_WARNING,
@@ -4020,9 +4096,10 @@ int steal_page(
* NB this is safe even if the page ends up being given back to
* the domain, because the count is zero: subsequent mappings will
* cause the cache attributes to be re-instated inside
- * get_page_from_l1e().
+ * get_page_from_l1e(), or the page to be added back to the IOMMU
+ * upon the type changing to PGT_writeable, as appropriate.
*/
- if ( (rc = cleanup_page_cacheattr(page)) )
+ if ( (rc = cleanup_page_mappings(page)) )
{
/*
* Couldn't fixup Xen's mappings; put things the way we found
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index c53cab44d9..2b62bc61dd 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -708,23 +708,9 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn,
p2m_type_t t;
p2m_access_t a;
+ /* IOMMU for PV guests is handled in get_page_type() and put_page(). */
if ( !paging_mode_translate(p2m->domain) )
- {
- int rc = 0;
-
- if ( need_iommu(p2m->domain) )
- {
- for ( i = 0; i < (1 << page_order); i++ )
- {
- int ret = iommu_unmap_page(p2m->domain, mfn + i);
-
- if ( !rc )
- rc = ret;
- }
- }
-
- return rc;
- }
+ return 0;
ASSERT(gfn_locked_by_me(p2m, gfn));
P2M_DEBUG("removing gfn=%#lx mfn=%#lx\n", gfn_l, mfn);
@@ -769,26 +755,33 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
int pod_count = 0;
int rc = 0;
+ /* IOMMU for PV guests is handled in get_page_type() and put_page(). */
if ( !paging_mode_translate(d) )
{
- if ( need_iommu(d) && t == p2m_ram_rw )
- {
- for ( i = 0; i < (1 << page_order); i++ )
- {
- rc = iommu_map_page(d, mfn_x(mfn_add(mfn, i)),
- mfn_x(mfn_add(mfn, i)),
- IOMMUF_readable|IOMMUF_writable);
- if ( rc != 0 )
- {
- while ( i-- > 0 )
- /* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(d, mfn_x(mfn_add(mfn, i))) )
- continue;
+ struct page_info *page = mfn_to_page(mfn);
- return rc;
- }
- }
+ /*
+ * Our interface for PV guests wrt IOMMU entries hasn't been very
+ * clear; but historically, pages have started out with IOMMU mappings,
+ * and only lose them when changed to a different page type.
+ *
+ * Retain this property by grabbing a writable type ref and then
+ * dropping it immediately. The result will be pages that have a
+ * writable type (and an IOMMU entry), but a count of 0 (such that
+ * any guest-requested type changes succeed and remove the IOMMU
+ * entry).
+ */
+ if ( !need_iommu(d) || t != p2m_ram_rw )
+ return 0;
+
+ for ( i = 0; i < (1UL << page_order); ++i, ++page )
+ {
+ if ( get_page_and_type(page, d, PGT_writable_page) )
+ put_page_and_type(page);
+ else
+ return -EINVAL;
}
+
return 0;
}
--
2.20.1

237
xsa290-4.10-1.patch Normal file
View file

@ -0,0 +1,237 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/mm: also allow L2 (un)validation to be preemptible
Commit c612481d1c ("x86/mm: Plumbing to allow any PTE update to fail
with -ERESTART") added assertions next to the {alloc,free}_l2_table()
invocations to document (and validate in debug builds) that L2
(un)validations are always preemptible.
The assertion in free_page_type() was now observed to trigger when
recursive L2 page tables get cleaned up.
In particular put_page_from_l2e()'s assumption that _put_page_type()
would always succeed is now wrong, resulting in a partially un-validated
page left in a domain, which has no other means of getting cleaned up
later on. If not causing any problems earlier, this would ultimately
trigger the check for ->u.inuse.type_info having a zero count when
freeing the page during cleanup after the domain has died.
As a result it should be considered a mistake to not have extended
preemption fully to L2 when it was added to L3/L4 table handling, which
this change aims to correct.
The validation side additions are done just for symmetry.
This is part of XSA-290.
Reported-by: Manuel Bouyer <bouyer@antioche.eu.org>
Tested-by: Manuel Bouyer <bouyer@antioche.eu.org>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1168,7 +1168,7 @@ get_page_from_l1e(
define_get_linear_pagetable(l2);
static int
get_page_from_l2e(
- l2_pgentry_t l2e, unsigned long pfn, struct domain *d)
+ l2_pgentry_t l2e, unsigned long pfn, struct domain *d, int partial)
{
unsigned long mfn = l2e_get_pfn(l2e);
int rc;
@@ -1185,7 +1185,8 @@ get_page_from_l2e(
if ( !(l2e_get_flags(l2e) & _PAGE_PSE) )
{
- rc = get_page_and_type_from_mfn(_mfn(mfn), PGT_l1_page_table, d, 0, 0);
+ rc = get_page_and_type_from_mfn(_mfn(mfn), PGT_l1_page_table, d,
+ partial, false);
if ( unlikely(rc == -EINVAL) && get_l2_linear_pagetable(l2e, pfn, d) )
rc = 0;
return rc;
@@ -1326,8 +1327,11 @@ void put_page_from_l1e(l1_pgentry_t l1e,
* NB. Virtual address 'l2e' maps to a machine address within frame 'pfn'.
* Note also that this automatically deals correctly with linear p.t.'s.
*/
-static int put_page_from_l2e(l2_pgentry_t l2e, unsigned long pfn)
+static int put_page_from_l2e(l2_pgentry_t l2e, unsigned long pfn,
+ int partial, bool defer)
{
+ int rc = 0;
+
if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) || (l2e_get_pfn(l2e) == pfn) )
return 1;
@@ -1342,13 +1346,27 @@ static int put_page_from_l2e(l2_pgentry_
else
{
struct page_info *pg = l2e_get_page(l2e);
- int rc = _put_page_type(pg, false, mfn_to_page(_mfn(pfn)));
+ struct page_info *ptpg = mfn_to_page(_mfn(pfn));
- ASSERT(!rc);
- put_page(pg);
+ if ( unlikely(partial > 0) )
+ {
+ ASSERT(!defer);
+ rc = _put_page_type(pg, true, ptpg);
+ }
+ else if ( defer )
+ {
+ current->arch.old_guest_ptpg = ptpg;
+ current->arch.old_guest_table = pg;
+ }
+ else
+ {
+ rc = _put_page_type(pg, true, ptpg);
+ if ( likely(!rc) )
+ put_page(pg);
+ }
}
- return 0;
+ return rc;
}
static int put_page_from_l3e(l3_pgentry_t l3e, unsigned long pfn,
@@ -1518,11 +1536,12 @@ static int alloc_l2_table(struct page_in
unsigned long pfn = mfn_x(page_to_mfn(page));
l2_pgentry_t *pl2e;
unsigned int i;
- int rc = 0;
+ int rc = 0, partial = page->partial_pte;
pl2e = map_domain_page(_mfn(pfn));
- for ( i = page->nr_validated_ptes; i < L2_PAGETABLE_ENTRIES; i++ )
+ for ( i = page->nr_validated_ptes; i < L2_PAGETABLE_ENTRIES;
+ i++, partial = 0 )
{
if ( i > page->nr_validated_ptes && hypercall_preempt_check() )
{
@@ -1532,23 +1551,33 @@ static int alloc_l2_table(struct page_in
}
if ( !is_guest_l2_slot(d, type, i) ||
- (rc = get_page_from_l2e(pl2e[i], pfn, d)) > 0 )
+ (rc = get_page_from_l2e(pl2e[i], pfn, d, partial)) > 0 )
continue;
- if ( unlikely(rc == -ERESTART) )
+ if ( rc == -ERESTART )
{
page->nr_validated_ptes = i;
- break;
+ page->partial_pte = partial ?: 1;
}
-
- if ( rc < 0 )
+ else if ( rc == -EINTR && i )
+ {
+ page->nr_validated_ptes = i;
+ page->partial_pte = 0;
+ rc = -ERESTART;
+ }
+ else if ( rc < 0 && rc != -EINTR )
{
gdprintk(XENLOG_WARNING, "Failure in alloc_l2_table: slot %#x\n", i);
- while ( i-- > 0 )
- if ( is_guest_l2_slot(d, type, i) )
- put_page_from_l2e(pl2e[i], pfn);
- break;
+ if ( i )
+ {
+ page->nr_validated_ptes = i;
+ page->partial_pte = 0;
+ current->arch.old_guest_ptpg = NULL;
+ current->arch.old_guest_table = page;
+ }
}
+ if ( rc < 0 )
+ break;
pl2e[i] = adjust_guest_l2e(pl2e[i], d);
}
@@ -1828,28 +1857,50 @@ static int free_l2_table(struct page_inf
struct domain *d = page_get_owner(page);
unsigned long pfn = mfn_x(page_to_mfn(page));
l2_pgentry_t *pl2e;
- unsigned int i = page->nr_validated_ptes - 1;
- int err = 0;
+ int rc = 0, partial = page->partial_pte;
+ unsigned int i = page->nr_validated_ptes - !partial;
pl2e = map_domain_page(_mfn(pfn));
- ASSERT(page->nr_validated_ptes);
- do {
- if ( is_guest_l2_slot(d, page->u.inuse.type_info, i) &&
- put_page_from_l2e(pl2e[i], pfn) == 0 &&
- i && hypercall_preempt_check() )
+ for ( ; ; )
+ {
+ if ( is_guest_l2_slot(d, page->u.inuse.type_info, i) )
+ rc = put_page_from_l2e(pl2e[i], pfn, partial, false);
+ if ( rc < 0 )
+ break;
+
+ partial = 0;
+
+ if ( !i-- )
+ break;
+
+ if ( hypercall_preempt_check() )
{
- page->nr_validated_ptes = i;
- err = -ERESTART;
+ rc = -EINTR;
+ break;
}
- } while ( !err && i-- );
+ }
unmap_domain_page(pl2e);
- if ( !err )
+ if ( rc >= 0 )
+ {
page->u.inuse.type_info &= ~PGT_pae_xen_l2;
+ rc = 0;
+ }
+ else if ( rc == -ERESTART )
+ {
+ page->nr_validated_ptes = i;
+ page->partial_pte = partial ?: -1;
+ }
+ else if ( rc == -EINTR && i < L2_PAGETABLE_ENTRIES - 1 )
+ {
+ page->nr_validated_ptes = i + 1;
+ page->partial_pte = 0;
+ rc = -ERESTART;
+ }
- return err;
+ return rc;
}
static int free_l3_table(struct page_info *page)
@@ -2115,7 +2166,7 @@ static int mod_l2_entry(l2_pgentry_t *pl
return -EBUSY;
}
- if ( unlikely((rc = get_page_from_l2e(nl2e, pfn, d)) < 0) )
+ if ( unlikely((rc = get_page_from_l2e(nl2e, pfn, d, 0)) < 0) )
return rc;
nl2e = adjust_guest_l2e(nl2e, d);
@@ -2134,7 +2185,8 @@ static int mod_l2_entry(l2_pgentry_t *pl
return -EBUSY;
}
- put_page_from_l2e(ol2e, pfn);
+ put_page_from_l2e(ol2e, pfn, 0, true);
+
return rc;
}

71
xsa290-4.10-2.patch Normal file
View file

@ -0,0 +1,71 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/mm: add explicit preemption checks to L3 (un)validation
When recursive page tables are used at the L3 level, unvalidation of a
single L4 table may incur unvalidation of two levels of L3 tables, i.e.
a maximum iteration count of 512^3 for unvalidating an L4 table. The
preemption check in free_l2_table() as well as the one in
_put_page_type() may never be reached, so explicit checking is needed in
free_l3_table().
When recursive page tables are used at the L4 level, the iteration count
at L4 alone is capped at 512^2. As soon as a present L3 entry is hit
which itself needs unvalidation (and hence requiring another nested loop
with 512 iterations), the preemption checks added here kick in, so no
further preemption checking is needed at L4 (until we decide to permit
5-level paging for PV guests).
The validation side additions are done just for symmetry.
This is part of XSA-290.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1612,6 +1612,13 @@ static int alloc_l3_table(struct page_in
for ( i = page->nr_validated_ptes; i < L3_PAGETABLE_ENTRIES;
i++, partial = 0 )
{
+ if ( i > page->nr_validated_ptes && hypercall_preempt_check() )
+ {
+ page->nr_validated_ptes = i;
+ rc = -ERESTART;
+ break;
+ }
+
if ( is_pv_32bit_domain(d) && (i == 3) )
{
if ( !(l3e_get_flags(pl3e[i]) & _PAGE_PRESENT) ||
@@ -1913,15 +1920,25 @@ static int free_l3_table(struct page_inf
pl3e = map_domain_page(_mfn(pfn));
- do {
+ for ( ; ; )
+ {
rc = put_page_from_l3e(pl3e[i], pfn, partial, 0);
if ( rc < 0 )
break;
+
partial = 0;
- if ( rc > 0 )
- continue;
- pl3e[i] = unadjust_guest_l3e(pl3e[i], d);
- } while ( i-- );
+ if ( rc == 0 )
+ pl3e[i] = unadjust_guest_l3e(pl3e[i], d);
+
+ if ( !i-- )
+ break;
+
+ if ( hypercall_preempt_check() )
+ {
+ rc = -EINTR;
+ break;
+ }
+ }
unmap_domain_page(pl3e);

53
xsa291-4.11.patch Normal file
View file

@ -0,0 +1,53 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/mm: don't retain page type reference when IOMMU operation fails
The IOMMU update in _get_page_type() happens between recording of the
new reference and validation of the page for its new type (if
necessary). If the IOMMU operation fails, there's no point in actually
carrying out validation. Furthermore, with this resulting in failure
getting indicated to the caller, the recorded type reference also needs
to be dropped again.
Note that in case of failure of alloc_page_type() there's no need to
undo the IOMMU operation: Only special types get handed to the function.
The function, upon failure, clears ->u.inuse.type_info, effectively
converting the page to PGT_none. The IOMMU mapping, however, solely
depends on whether the type is PGT_writable_page.
This is XSA-291.
Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2751,6 +2751,13 @@ static int _get_page_type(struct page_in
iommu_ret = iommu_map_page(d, gfn_x(gfn),
mfn_x(page_to_mfn(page)),
IOMMUF_readable|IOMMUF_writable);
+
+ if ( unlikely(iommu_ret) )
+ {
+ _put_page_type(page, false, NULL);
+ rc = iommu_ret;
+ goto out;
+ }
}
}
@@ -2765,12 +2772,10 @@ static int _get_page_type(struct page_in
rc = alloc_page_type(page, type, preemptible);
}
+ out:
if ( (x & PGT_partial) && !(nx & PGT_partial) )
put_page(page);
- if ( !rc )
- rc = iommu_ret;
-
return rc;
}

95
xsa292.patch Normal file
View file

@ -0,0 +1,95 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/mm: properly flush TLB in switch_cr3_cr4()
The CR3 values used for contexts run with PCID enabled uniformly have
CR3.NOFLUSH set, resulting in the CR3 write itself to not cause any
flushing at all. When the second CR4 write is skipped or doesn't do any
flushing, there's nothing so far which would purge TLB entries which may
have accumulated again if the PCID doesn't change; the "just in case"
flush only affects the case where the PCID actually changes. (There may
be particularly many TLB entries re-accumulated in case of a watchdog
NMI kicking in during the critical time window.)
Suppress the no-flush behavior of the CR3 write in this particular case.
Similarly the second CR4 write may not cause any flushing of TLB entries
established again while the original PCID was still in use - it may get
performed because of unrelated bits changing. The flush of the old PCID
needs to happen nevertheless.
At the same time also eliminate a possible race with lazy context
switch: Just like for CR4, CR3 may change at any time while interrupts
are enabled, due to the __sync_local_execstate() invocation from the
flush IPI handler. It is for that reason that the CR3 read, just like
the CR4 one, must happen only after interrupts have been turned off.
This is XSA-292.
Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v3: Adjust comments. Drop old_cr4 from the PGE check in the expression
controlling the invocation of invpcid_flush_single_context(), as PGE
is always clear there.
v2: Decouple invpcid_flush_single_context() from 2nd CR4 write.
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -103,9 +103,8 @@ static void do_tlb_flush(void)
void switch_cr3_cr4(unsigned long cr3, unsigned long cr4)
{
- unsigned long flags, old_cr4;
+ unsigned long flags, old_cr4, old_pcid;
u32 t;
- unsigned long old_pcid = cr3_pcid(read_cr3());
/* This non-reentrant function is sometimes called in interrupt context. */
local_irq_save(flags);
@@ -133,15 +132,38 @@ void switch_cr3_cr4(unsigned long cr3, u
*/
invpcid_flush_all_nonglobals();
+ /*
+ * If we don't change PCIDs, the CR3 write below needs to flush this very
+ * PCID, even when a full flush was performed above, as we are currently
+ * accumulating TLB entries again from the old address space.
+ * NB: Clearing the bit when we don't use PCID is benign (as it is clear
+ * already in that case), but allows the if() to be more simple.
+ */
+ old_pcid = cr3_pcid(read_cr3());
+ if ( old_pcid == cr3_pcid(cr3) )
+ cr3 &= ~X86_CR3_NOFLUSH;
+
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.
- */
+
+ /*
+ * 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.
+ *
+ * The write to CR4 just above has performed a wider flush in certain
+ * cases, which therefore get excluded here. Since that write is
+ * conditional, note in particular that it won't be skipped if PCIDE
+ * transitions from 1 to 0. This is because the CR4 write further up will
+ * have been skipped in this case, as PCIDE and PGE won't both be set at
+ * the same time.
+ *
+ * Note also that PGE is always clear in old_cr4.
+ */
+ if ( old_pcid != cr3_pcid(cr3) &&
+ !(cr4 & X86_CR4_PGE) &&
+ (old_cr4 & X86_CR4_PCIDE) <= (cr4 & X86_CR4_PCIDE) )
invpcid_flush_single_context(old_pcid);
post_flush(t);

318
xsa293-4.10-1.patch Normal file
View file

@ -0,0 +1,318 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/pv: Rewrite guest %cr4 handling from scratch
The PV cr4 logic is almost impossible to follow, and leaks bits into guest
context which definitely shouldn't be visible (in particular, VMXE).
The biggest problem however, and source of the complexity, is that it derives
new real and guest cr4 values from the current value in hardware - this is
context dependent and an inappropriate source of information.
Rewrite the cr4 logic to be invariant of the current value in hardware.
First of all, modify write_ptbase() to always use mmu_cr4_features for IDLE
and HVM contexts. mmu_cr4_features *is* the correct value to use, and makes
the ASSERT() obviously redundant.
For PV guests, curr->arch.pv.ctrlreg[4] remains the guests view of cr4, but
all logic gets reworked in terms of this and mmu_cr4_features only.
Two masks are introduced; bits which the guest has control over, and bits
which are forwarded from Xen's settings. One guest-visible change here is
that Xen's VMXE setting is no longer visible at all.
pv_make_cr4() follows fairly closely from pv_guest_cr4_to_real_cr4(), but
deliberately starts with mmu_cr4_features, and only alters the minimal subset
of bits.
The boot-time {compat_,}pv_cr4_mask variables are removed, as they are a
remnant of the pre-CPUID policy days. pv_fixup_guest_cr4() gains a related
derivation from the policy.
Another guest visible change here is that a 32bit PV guest can now flip
FSGSBASE in its view of CR4. While the {RD,WR}{FS,GS}BASE instructions are
unusable outside of a 64bit code segment, the ability to modify FSGSBASE
matches real hardware behaviour, and avoids the need for any 32bit/64bit
differences in the logic.
Overall, this patch shouldn't have a practical change in guest behaviour.
VMXE will disappear from view, and an inquisitive 32bit kernel can now see
FSGSBASE changing, but this new logic is otherwise bug-compatible with before.
This is part of XSA-293
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index fcbe767..853b524 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -734,49 +734,6 @@ int arch_domain_soft_reset(struct domain *d)
return ret;
}
-/*
- * These are the masks of CR4 bits (subject to hardware availability) which a
- * PV guest may not legitimiately attempt to modify.
- */
-static unsigned long __read_mostly pv_cr4_mask, compat_pv_cr4_mask;
-
-static int __init init_pv_cr4_masks(void)
-{
- unsigned long common_mask = ~X86_CR4_TSD;
-
- /*
- * All PV guests may attempt to modify TSD, DE and OSXSAVE.
- */
- if ( cpu_has_de )
- common_mask &= ~X86_CR4_DE;
- if ( cpu_has_xsave )
- common_mask &= ~X86_CR4_OSXSAVE;
-
- pv_cr4_mask = compat_pv_cr4_mask = common_mask;
-
- /*
- * 64bit PV guests may attempt to modify FSGSBASE.
- */
- if ( cpu_has_fsgsbase )
- pv_cr4_mask &= ~X86_CR4_FSGSBASE;
-
- return 0;
-}
-__initcall(init_pv_cr4_masks);
-
-unsigned long pv_guest_cr4_fixup(const struct vcpu *v, unsigned long guest_cr4)
-{
- unsigned long hv_cr4 = real_cr4_to_pv_guest_cr4(read_cr4());
- unsigned long mask = is_pv_32bit_vcpu(v) ? compat_pv_cr4_mask : pv_cr4_mask;
-
- if ( (guest_cr4 & mask) != (hv_cr4 & mask) )
- printk(XENLOG_G_WARNING
- "d%d attempted to change %pv's CR4 flags %08lx -> %08lx\n",
- current->domain->domain_id, v, hv_cr4, guest_cr4);
-
- return (hv_cr4 & mask) | (guest_cr4 & ~mask);
-}
-
#define xen_vcpu_guest_context vcpu_guest_context
#define fpu_ctxt fpu_ctxt.x
CHECK_FIELD_(struct, vcpu_guest_context, fpu_ctxt);
@@ -790,7 +747,7 @@ int arch_set_info_guest(
struct domain *d = v->domain;
unsigned long cr3_gfn;
struct page_info *cr3_page;
- unsigned long flags, cr4;
+ unsigned long flags;
unsigned int i;
int rc = 0, compat;
@@ -981,9 +938,8 @@ int arch_set_info_guest(
v->arch.pv_vcpu.ctrlreg[0] &= X86_CR0_TS;
v->arch.pv_vcpu.ctrlreg[0] |= read_cr0() & ~X86_CR0_TS;
- cr4 = v->arch.pv_vcpu.ctrlreg[4];
- v->arch.pv_vcpu.ctrlreg[4] = cr4 ? pv_guest_cr4_fixup(v, cr4) :
- real_cr4_to_pv_guest_cr4(mmu_cr4_features);
+ v->arch.pv_vcpu.ctrlreg[4] =
+ pv_fixup_guest_cr4(v, v->arch.pv_vcpu.ctrlreg[4]);
memset(v->arch.debugreg, 0, sizeof(v->arch.debugreg));
for ( i = 0; i < 8; i++ )
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index c20edb9..716499a 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -511,33 +511,13 @@ void make_cr3(struct vcpu *v, mfn_t mfn)
v->arch.cr3 |= get_pcid_bits(v, false);
}
-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);
-
- 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;
-}
-
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_PCIDE | X86_CR4_TSD)) | X86_CR4_PGE);
+ ? pv_make_cr4(v) : mmu_cr4_features;
if ( is_pv_vcpu(v) && v->domain->arch.pv_domain.xpti )
{
@@ -556,8 +536,6 @@ void write_ptbase(struct vcpu *v)
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/domain.c b/xen/arch/x86/pv/domain.c
index 958c6e3..a9caa01 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -103,6 +103,52 @@ static void release_compat_l4(struct vcpu *v)
v->arch.guest_table_user = pagetable_null();
}
+unsigned long pv_fixup_guest_cr4(const struct vcpu *v, unsigned long cr4)
+{
+ const struct cpuid_policy *p = v->domain->arch.cpuid;
+
+ /* Discard attempts to set guest controllable bits outside of the policy. */
+ cr4 &= ~((p->basic.tsc ? 0 : X86_CR4_TSD) |
+ (p->basic.de ? 0 : X86_CR4_DE) |
+ (p->feat.fsgsbase ? 0 : X86_CR4_FSGSBASE) |
+ (p->basic.xsave ? 0 : X86_CR4_OSXSAVE));
+
+ /* Masks expected to be disjoint sets. */
+ BUILD_BUG_ON(PV_CR4_GUEST_MASK & PV_CR4_GUEST_VISIBLE_MASK);
+
+ /*
+ * A guest sees the policy subset of its own choice of guest controllable
+ * bits, and a subset of Xen's choice of certain hardware settings.
+ */
+ return ((cr4 & PV_CR4_GUEST_MASK) |
+ (mmu_cr4_features & PV_CR4_GUEST_VISIBLE_MASK));
+}
+
+unsigned long pv_make_cr4(const struct vcpu *v)
+{
+ const struct domain *d = v->domain;
+ unsigned long cr4 = mmu_cr4_features &
+ ~(X86_CR4_PCIDE | X86_CR4_PGE | X86_CR4_TSD);
+
+ /*
+ * PCIDE or PGE depends on the PCID/XPTI settings, but must not both be
+ * set, as it impacts the safety of TLB flushing.
+ */
+ if ( d->arch.pv_domain.pcid )
+ cr4 |= X86_CR4_PCIDE;
+ else if ( !d->arch.pv_domain.xpti )
+ cr4 |= X86_CR4_PGE;
+
+ /*
+ * TSD is needed if either the guest has elected to use it, or Xen is
+ * virtualising the TSC value the guest sees.
+ */
+ if ( d->arch.vtsc || (v->arch.pv_vcpu.ctrlreg[4] & X86_CR4_TSD) )
+ cr4 |= X86_CR4_TSD;
+
+ return cr4;
+}
+
int switch_compat(struct domain *d)
{
struct vcpu *v;
@@ -197,7 +243,7 @@ int pv_vcpu_initialise(struct vcpu *v)
/* PV guests by default have a 100Hz ticker. */
v->periodic_period = MILLISECS(10);
- v->arch.pv_vcpu.ctrlreg[4] = real_cr4_to_pv_guest_cr4(mmu_cr4_features);
+ v->arch.pv_vcpu.ctrlreg[4] = pv_fixup_guest_cr4(v, 0);
if ( is_pv_32bit_domain(d) )
{
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index c281936..cd04574 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -32,6 +32,7 @@
#include <asm/hypercall.h>
#include <asm/mc146818rtc.h>
#include <asm/p2m.h>
+#include <asm/pv/domain.h>
#include <asm/pv/traps.h>
#include <asm/shared.h>
#include <asm/traps.h>
@@ -804,8 +805,8 @@ static int write_cr(unsigned int reg, unsigned long val,
}
case 4: /* Write CR4 */
- curr->arch.pv_vcpu.ctrlreg[4] = pv_guest_cr4_fixup(curr, val);
- write_cr4(pv_guest_cr4_to_real_cr4(curr));
+ curr->arch.pv_vcpu.ctrlreg[4] = pv_fixup_guest_cr4(curr, val);
+ write_cr4(pv_make_cr4(curr));
ctxt_switch_levelling(curr);
return X86EMUL_OKAY;
}
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index b9fa988..aec6563 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -623,17 +623,6 @@ bool update_secondary_system_time(struct vcpu *,
void vcpu_show_execution_state(struct vcpu *);
void vcpu_show_registers(const struct vcpu *);
-/* Clean up CR4 bits that are not under guest control. */
-unsigned long pv_guest_cr4_fixup(const struct vcpu *, unsigned long guest_cr4);
-
-/* Convert between guest-visible and real CR4 values. */
-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_PCIDE))
-
#define domain_max_vcpus(d) (is_hvm_domain(d) ? HVM_MAX_VCPUS : MAX_VIRT_CPUS)
static inline struct vcpu_guest_context *alloc_vcpu_guest_context(void)
diff --git a/xen/include/asm-x86/pv/domain.h b/xen/include/asm-x86/pv/domain.h
index 6778e1b..1ddc728 100644
--- a/xen/include/asm-x86/pv/domain.h
+++ b/xen/include/asm-x86/pv/domain.h
@@ -60,6 +60,23 @@ void pv_domain_destroy(struct domain *d);
int pv_domain_initialise(struct domain *d, unsigned int domcr_flags,
struct xen_arch_domainconfig *config);
+/*
+ * Bits which a PV guest can toggle in its view of cr4. Some are loaded into
+ * hardware, while some are fully emulated.
+ */
+#define PV_CR4_GUEST_MASK \
+ (X86_CR4_TSD | X86_CR4_DE | X86_CR4_FSGSBASE | X86_CR4_OSXSAVE)
+
+/* Bits which a PV guest may observe from the real hardware settings. */
+#define PV_CR4_GUEST_VISIBLE_MASK \
+ (X86_CR4_PAE | X86_CR4_MCE | X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT)
+
+/* Given a new cr4 value, construct the resulting guest-visible cr4 value. */
+unsigned long pv_fixup_guest_cr4(const struct vcpu *v, unsigned long cr4);
+
+/* Create a cr4 value to load into hardware, based on vcpu settings. */
+unsigned long pv_make_cr4(const struct vcpu *v);
+
#else /* !CONFIG_PV */
#include <xen/errno.h>
@@ -73,6 +90,9 @@ static inline int pv_domain_initialise(struct domain *d,
{
return -EOPNOTSUPP;
}
+
+static inline unsigned long pv_make_cr4(const struct vcpu *v) { return ~0ul; }
+
#endif /* CONFIG_PV */
void paravirt_ctxt_switch_from(struct vcpu *v);

242
xsa293-4.10-2.patch Normal file
View file

@ -0,0 +1,242 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/pv: Don't have %cr4.fsgsbase active behind a guest kernels back
Currently, a 64bit PV guest can appear to set and clear FSGSBASE in %cr4, but
the bit remains set in hardware. Therefore, the {RD,WR}{FS,GS}BASE are usable
even when the guest kernel believes that they are disabled.
The FSGSBASE feature isn't currently supported in Linux, and its context
switch path has some optimisations which rely on userspace being unable to use
the WR{FS,GS}BASE instructions. Xen's current behaviour undermines this
expectation.
In 64bit PV guest context, always load the guest kernels setting of FSGSBASE
into %cr4. This requires adjusting how Xen uses the {RD,WR}{FS,GS}BASE
instructions.
* Delete the cpu_has_fsgsbase helper. It is no longer safe, as users need to
check %cr4 directly.
* The raw __rd{fs,gs}base() helpers are only safe to use when %cr4.fsgsbase
is set. Comment this property.
* The {rd,wr}{fs,gs}{base,shadow}() and read_msr() helpers are updated to use
the current %cr4 value to determine which mechanism to use.
* toggle_guest_mode() and save_segments() are update to avoid reading
fs/gsbase if the values in hardware cannot be stale WRT struct vcpu. A
consequence of this is that the write_cr() path needs to cache the current
bases, as subsequent context switches will skip saving the values.
* write_cr4() is updated to ensure that the shadow %cr4.fsgsbase value is
observed in a safe way WRT the hardware setting, if an interrupt happens to
hit in the middle.
* pv_make_cr4() is updated for 64bit PV guests to use the guest kernels
choice of FSGSBASE.
This is part of XSA-293
Reported-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 853b524..91c2b1c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1431,7 +1431,8 @@ static void save_segments(struct vcpu *v)
regs->fs = read_sreg(fs);
regs->gs = read_sreg(gs);
- if ( cpu_has_fsgsbase && !is_pv_32bit_vcpu(v) )
+ /* %fs/%gs bases can only be stale if WR{FS,GS}BASE are usable. */
+ if ( (read_cr4() & X86_CR4_FSGSBASE) && !is_pv_32bit_vcpu(v) )
{
v->arch.pv_vcpu.fs_base = __rdfsbase();
if ( v->arch.flags & TF_kernel_mode )
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index a9caa01..82c6e2f 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -146,6 +146,16 @@ unsigned long pv_make_cr4(const struct vcpu *v)
if ( d->arch.vtsc || (v->arch.pv_vcpu.ctrlreg[4] & X86_CR4_TSD) )
cr4 |= X86_CR4_TSD;
+ /*
+ * The {RD,WR}{FS,GS}BASE are only useable in 64bit code segments. While
+ * we must not have CR4.FSGSBASE set behind the back of a 64bit PV kernel,
+ * we do leave it set in 32bit PV context to speed up Xen's context switch
+ * path.
+ */
+ if ( !is_pv_32bit_domain(d) &&
+ !(v->arch.pv_vcpu.ctrlreg[4] & X86_CR4_FSGSBASE) )
+ cr4 &= ~X86_CR4_FSGSBASE;
+
return cr4;
}
@@ -351,7 +361,8 @@ void toggle_guest_mode(struct vcpu *v)
if ( is_pv_32bit_vcpu(v) )
return;
- if ( cpu_has_fsgsbase )
+ /* %fs/%gs bases can only be stale if WR{FS,GS}BASE are usable. */
+ if ( read_cr4() & X86_CR4_FSGSBASE )
{
if ( v->arch.flags & TF_kernel_mode )
v->arch.pv_vcpu.gs_base_kernel = __rdgsbase();
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index cd04574..0344c98 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -805,6 +805,17 @@ static int write_cr(unsigned int reg, unsigned long val,
}
case 4: /* Write CR4 */
+ /*
+ * If this write will disable FSGSBASE, refresh Xen's idea of the
+ * guest bases now that they can no longer change.
+ */
+ if ( (curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_FSGSBASE) &&
+ !(val & X86_CR4_FSGSBASE) )
+ {
+ curr->arch.pv_vcpu.fs_base = __rdfsbase();
+ curr->arch.pv_vcpu.gs_base_kernel = __rdgsbase();
+ }
+
curr->arch.pv_vcpu.ctrlreg[4] = pv_fixup_guest_cr4(curr, val);
write_cr4(pv_make_cr4(curr));
ctxt_switch_levelling(curr);
@@ -854,14 +865,15 @@ static int read_msr(unsigned int reg, uint64_t *val,
case MSR_FS_BASE:
if ( is_pv_32bit_domain(currd) )
break;
- *val = cpu_has_fsgsbase ? __rdfsbase() : curr->arch.pv_vcpu.fs_base;
+ *val = (read_cr4() & X86_CR4_FSGSBASE) ? __rdfsbase()
+ : curr->arch.pv_vcpu.fs_base;
return X86EMUL_OKAY;
case MSR_GS_BASE:
if ( is_pv_32bit_domain(currd) )
break;
- *val = cpu_has_fsgsbase ? __rdgsbase()
- : curr->arch.pv_vcpu.gs_base_kernel;
+ *val = (read_cr4() & X86_CR4_FSGSBASE) ? __rdgsbase()
+ : curr->arch.pv_vcpu.gs_base_kernel;
return X86EMUL_OKAY;
case MSR_SHADOW_GS_BASE:
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 617dbb1..7903204 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1560,7 +1560,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
cr4_pv32_mask = mmu_cr4_features & XEN_CR4_PV32_BITS;
- if ( cpu_has_fsgsbase )
+ if ( boot_cpu_has(X86_FEATURE_FSGSBASE) )
set_in_cr4(X86_CR4_FSGSBASE);
if ( opt_invpcid && cpu_has_invpcid )
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index ff6f969..5043231 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -84,7 +84,6 @@
#define cpu_has_xsaves boot_cpu_has(X86_FEATURE_XSAVES)
/* CPUID level 0x00000007:0.ebx */
-#define cpu_has_fsgsbase boot_cpu_has(X86_FEATURE_FSGSBASE)
#define cpu_has_bmi1 boot_cpu_has(X86_FEATURE_BMI1)
#define cpu_has_hle boot_cpu_has(X86_FEATURE_HLE)
#define cpu_has_avx2 boot_cpu_has(X86_FEATURE_AVX2)
diff --git a/xen/include/asm-x86/msr.h b/xen/include/asm-x86/msr.h
index 22d5b71..143eea3 100644
--- a/xen/include/asm-x86/msr.h
+++ b/xen/include/asm-x86/msr.h
@@ -120,6 +120,14 @@ static inline uint64_t rdtsc_ordered(void)
: "=a" (low), "=d" (high) \
: "c" (counter))
+/*
+ * On hardware supporting FSGSBASE, the value loaded into hardware is the
+ * guest kernel's choice for 64bit PV guests (Xen's choice for Idle, HVM and
+ * 32bit PV).
+ *
+ * Therefore, the {RD,WR}{FS,GS}BASE instructions are only safe to use if
+ * %cr4.fsgsbase is set.
+ */
static inline unsigned long __rdfsbase(void)
{
unsigned long base;
@@ -150,7 +158,7 @@ static inline unsigned long rdfsbase(void)
{
unsigned long base;
- if ( cpu_has_fsgsbase )
+ if ( read_cr4() & X86_CR4_FSGSBASE )
return __rdfsbase();
rdmsrl(MSR_FS_BASE, base);
@@ -162,7 +170,7 @@ static inline unsigned long rdgsbase(void)
{
unsigned long base;
- if ( cpu_has_fsgsbase )
+ if ( read_cr4() & X86_CR4_FSGSBASE )
return __rdgsbase();
rdmsrl(MSR_GS_BASE, base);
@@ -172,7 +180,7 @@ static inline unsigned long rdgsbase(void)
static inline void wrfsbase(unsigned long base)
{
- if ( cpu_has_fsgsbase )
+ if ( read_cr4() & X86_CR4_FSGSBASE )
#ifdef HAVE_GAS_FSGSBASE
asm volatile ( "wrfsbase %0" :: "r" (base) );
#else
@@ -184,7 +192,7 @@ static inline void wrfsbase(unsigned long base)
static inline void wrgsbase(unsigned long base)
{
- if ( cpu_has_fsgsbase )
+ if ( read_cr4() & X86_CR4_FSGSBASE )
#ifdef HAVE_GAS_FSGSBASE
asm volatile ( "wrgsbase %0" :: "r" (base) );
#else
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 90a2701..a0f8bf4 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -315,11 +315,31 @@ static inline unsigned long read_cr4(void)
static inline void write_cr4(unsigned long val)
{
+ struct cpu_info *info = get_cpu_info();
+
/* 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) );
+ /*
+ * On hardware supporting FSGSBASE, the value in %cr4 is the kernel's
+ * choice for 64bit PV guests, which impacts whether Xen can use the
+ * instructions.
+ *
+ * The {rd,wr}{fs,gs}base() helpers use info->cr4 to work out whether it
+ * is safe to execute the {RD,WR}{FS,GS}BASE instruction, falling back to
+ * the MSR path if not. Some users require interrupt safety.
+ *
+ * If FSGSBASE is currently or about to become clear, reflect this in
+ * info->cr4 before updating %cr4, so an interrupt which hits in the
+ * middle won't observe FSGSBASE set in info->cr4 but clear in %cr4.
+ */
+ info->cr4 = val & (info->cr4 | ~X86_CR4_FSGSBASE);
+
+ asm volatile ( "mov %[val], %%cr4"
+ : "+m" (info->cr4) /* Force ordering without a barrier. */
+ : [val] "r" (val) );
+
+ info->cr4 = val;
}
/* Clear and set 'TS' bit respectively */

73
xsa294-4.10.patch Normal file
View file

@ -0,0 +1,73 @@
From: Jan Beulich <JBeulich@suse.com>
Subject: x86/pv: _toggle_guest_pt() may not skip TLB flush for shadow mode guests
For shadow mode guests (e.g. PV ones forced into that mode as L1TF
mitigation, or during migration) update_cr3() -> sh_update_cr3() may
result in a change to the (shadow) root page table (compared to the
previous one when running the same vCPU with the same PCID). This can,
first and foremost, be a result of memory pressure on the shadow memory
pool of the domain. Shadow code legitimately relies on the original
(prior to commit 5c81d260c2 ["xen/x86: use PCID feature"]) behavior of
the subsequent CR3 write to flush the TLB of entries still left from
walks with an earlier, different (shadow) root page table.
Restore the flushing behavior, also for the second CR3 write on the exit
path to guest context when XPTI is active. For the moment accept that
this will introduce more flushes than are strictly necessary - no flush
would be needed when the (shadow) root page table doesn't actually
change, but this information isn't readily (i.e. without introducing a
layering violation) available here.
This is XSA-294.
Reported-by: XXX PERSON <XXX EMAIL>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 958c6e3..7973155 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -320,6 +320,8 @@ void toggle_guest_mode(struct vcpu *v)
void toggle_guest_pt(struct vcpu *v)
{
const struct domain *d = v->domain;
+ struct cpu_info *cpu_info = get_cpu_info();
+ unsigned long cr3;
if ( is_pv_32bit_vcpu(v) )
return;
@@ -328,16 +330,28 @@ void toggle_guest_pt(struct vcpu *v)
update_cr3(v);
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);
+ /*
+ * Don't flush user global mappings from the TLB. Don't tick TLB clock.
+ *
+ * In shadow mode, though, update_cr3() may need to be accompanied by a
+ * TLB flush (for just the incoming PCID), as the top level page table may
+ * have changed behind our backs. To be on the safe side, suppress the
+ * no-flush unconditionally in this case. The XPTI CR3 write, if enabled,
+ * will then need to be a flushing one too.
+ */
+ cr3 = v->arch.cr3;
+ if ( shadow_mode_enabled(d) )
+ {
+ cr3 &= ~X86_CR3_NOFLUSH;
+ cpu_info->pv_cr3 &= ~X86_CR3_NOFLUSH;
+ }
+ write_cr3(cr3);
if ( !(v->arch.flags & TF_kernel_mode) )
return;