The package appears to have been renamed in Ubuntu. The only reason this test
is currently passing is because the hypervisor build falls back to clang, at
version 3.5
Add an explicit test in the build script that out desired compiler is
available. Note that travis already performs this step, but in a way which
isn't fatal to the build.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by Doug Goldstein <cardoe@cardoe.com>
Clang warns:
kdd.c:1031:9: error: variable 'fd' is used uninitialized whenever '||'
condition is true [-Werror,-Wsometimes-uninitialized]
if (argc != 4
^~~~~~~~~
kdd.c:1040:20: note: uninitialized use occurs here
if (select(fd + 1, &fds, NULL, NULL, NULL) > 0)
^~
This situation can't actually happen, as usage() is a terminal path. Annotate
it appropriately.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Clang points out:
tap-ctl-list.c:457:28: error: variable 'entry' is uninitialized when
used here [-Werror,-Wuninitialized]
for (; *_entry != NULL; ++entry) {
^~~~~
The content of that loop clearly was meant to iterate over _entry rather than
entry, so is fixed to do so. This presumably fixes a memory leak when
tapdisks get orphed, as only the first item on the list got freed.
There is no use of entry at all. It is referenced in a
list_for_each_entry(tl, &tap->list, entry) construct, but this is just a
member name, and not a reference to local scope variable of the same name.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Clang points out:
tapdisk-disktype.c:117:2: error: initializer overrides prior initialization
of this subobject [-Werror,-Winitializer-overrides]
0,
^
tapdisk-disktype.c:115:23: note: previous initialization is here
[DISK_TYPE_VINDEX] = &vhd_index_disk,
^~~~~~~~~~~~~~~
Mixing different initialiser styles should be avoided; The actual behaviour is
different to the expected behaviour. This specific example has been broken
since its introduction in c/s 7b4dea554 "blktap2: Fix tapdisk disktype issues"
in 2010, and is caused by the '#if 0' block removing &tapdisk_{sync,vmdk}.
First of all, remove what were intended to be trailing NULL entries in
tapdisk_disk_{types,drivers}[], making consistent use of Designated
Initialisers for the initialisation.
This requires changing the loop in tapdisk_disktype_find() to be based on the
number of elements in tapdisk_disk_types[], rather than looking for the first
NULL. This fixes a latent bug, as the use of Designated Initializers causes
to intermediate zero entries if not all indices are explicitly specified.
There is a second latent bug where tapdisk_disktype_find() assumes that
tapdisk_disk_drivers[] has at least as many entries as tapdisk_disk_types[].
This is not the case and tapdisk_disk_drivers[] had one entry fewer than
tapdisk_disk_types[], but the NULL loop bound prevented an out-of-bounds read
of tapdisk_disk_drivers[]. Fix the issue by explicitly declaring
tapdisk_disk_drivers[] to have the same number of entries as
tapdisk_disk_types[].
Finally, this leads to a linker error. It turns out that tapdisk_vhd_index
doesn't exist, and I can't find any evidence in the source history to suggest
that it ever did. I can only presume that it would have been #if 0'd out like
tapdisk_sync and tapdisk_vmdk had it not been for this bug preventing a build
failure. Drop all three.
No functional change, but only because of the specific layout of
tapdisk_disk_types[].
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Clang points out that this is tautological.
src/xenstat.c:325:8: warning: comparison of 0 <= unsigned expression is
always true [-Wtautological-compare]
if (0 <= index && index < node->num_domains)
~ ^ ~~~~~
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
To simply change the permissions on existing Xen mappings. The existing
destroy_xen_mappings() is altered to support changing the PTE permissions.
A new destroy_xen_mappings() is introduced, as the special case of not passing
_PAGE_PRESENT to modify_xen_mappings().
As cleanup (and an ideal functional test), the boot logic which remaps Xen's
code and data with reduced permissions is altered to use
modify_xen_mappings(), rather than map_pages_to_xen() passing the same mfn's
back in.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
c/s 75f9455e "tools/libxc: Calculate xstate cpuid leaf from guest information"
incorrectly inverted the shift and mask when using X86_XSS_MASK. Luckily, the
mask is currently zero, avoiding incorrect calculations.
While adjusting this, use an explcit uint32_t cast rather than masking against
0xffffffff.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
c/s f5d947bf1b "tools/libxl: add back channel support to read stream"
made a bogus adjustment to libxl__stream_read_start(), including
removing the comment hinting at what was going on, which breaks
conversion of a legacy migration stream.
Symptoms look like:
root@anonymi:~ # xl migrate domU host
migration target: Ready to receive domain.
Saving to migration stream new xl format (info 0x1/0x0/2677)
xc: error: error polling suspend notification channel: -1: Internal error
Loading new save file <incoming migration stream> (new xl fmt info 0x1/0x0/2677)
Savefile contains xl domain config in JSON format
Parsing config from <saved>
libxl: error: libxl_stream_read.c:327:stream_header_done: Invalid ident: expected 0x4c6962786c466d74, got 0x01f00f0000000000
libxl: error: libxl_utils.c:430:libxl_read_exactly: file/stream truncated reading ipc msg header from domain 1 save/restore helper stdout pipe
The adjustment is not required for backchannel support (as there is no
interaction between back channels and legacy conversion), and caused
stream->fd to be latched in the datacopier before legacy conversion
substitutes it for the fd which is the output of the conversion script.
This causes libxl to consume data from the legacy stream rather than the
v2 stream, and for the conversion script to encounter an error as the
legacy stream appears to skip ahead.
Undo the adjustments to libxl__stream_read_start(), and introduce a
better description of what is going on. Introduce some extra assertions
to try and catch similar breakage in the future.
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Olaf Hering <olaf@aepfle.de>
c/s f71ecb6 "x86: introduce a new VMASSIST for architectural behaviour of
iopl" shifted the vcpu iopl field by 12, but didn't update the logic which
reconstructs the guests eflags for migration.
Existing guest kernels set a vIOPL of 1, to prevent them from faulting when
accessing IO ports. This bug manifests as a crash after migrate, as the vIOPL
reverts back to the default of 0, and the guest suffers an unexpected #GP
fault.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
The existing logic is broken for heterogeneous migration. By always
advertising the host maximum xstate, a migration to a less capable host always
fails as Xen cannot accomodate the xcr0_accum in the migration stream.
By calculating xstate from the feature information (which a multi-host
toolstack will have levelled appropriately), the guest will have the current
hosts maximum xstate advertised, allowing for correct migration to less
capable hosts.
In addition, some further improvements and corrections:
- don't discard the known flags in sub-leaves 2..63 ECX
- zap sub-leaves beyond 62
- zap all bits in leaf 1, EBX/ECX. No XSS features are currently supported.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
It is conceptually wrong to base a VM's featureset on the features visible to
the toolstack which happens to construct it.
Instead, the featureset used is either an explicit one passed by the
toolstack, or the default which Xen believes it can give to the guest.
Collect all the feature manipulation into a single function which adjusts the
featureset, and perform deep dependency removal.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Later changes (Patch titled "tools/libxc: Use featuresets rather than
guesswork") will cause the cpuid generation logic to seed their
information from a featureset. This patch adds the infrastructure to
specify a featureset, and will obtain the appropriate defaults from Xen
if omitted.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
It is able to reports the current featuresets; both the static masks and
dynamic featuresets from Xen, or to decode an arbitrary featureset into
`/proc/cpuinfo` style strings.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Rather than having a different local copy of some of the feature
definitions.
Modify the xc_cpuid_x86.c cpumask helpers to appropiately truncate the
new values.
As some of the feature have been renamed in the public API, similar renames
are made here.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
The type of the pointer to a bitmap is not interesting; it does not affect the
representation of the block of bits being pointed to.
Make the libxc functions consistent with those in Xen, so they can work just
as well with 'unsigned int *' based bitmaps.
As part of doing so, change the implementation to be in terms of char rather
than unsigned long. This fixes alignment concerns with ARM.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This allows PV domains with different featuresets to observe different values
from a native cpuid instruction, on supporting hardware.
It is important to leak the host view of X2APIC, HTT and CMP_LEGACY through to
guests, even though they could be hidden. These flags affect how to interpret
other cpuid leaves which are not maskable.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
And use them in preference to cpumask_defaults on context switch. HVM domains
must not be masked (to avoid interfering with cpuid calls within the guest),
so always lazily context switch to the host default.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
A single ctxt_switch_levelling() function pointer is provided
(defaulting to an empty nop), which is overridden in the appropriate
$VENDOR_init_levelling().
set_cpuid_faulting() is made private and included within
intel_ctxt_switch_levelling().
One (attempted) functional change is that the faulting configuration should
not be special cased for dom0. It turns out that the toolstack relies on the
special case (and indeed, on being a PV domain in the first place) to
correctly build HVM domains.
For now, the control domain is left as a special case, until futher work can
be completed to remove the restriction.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This patch is best reviewed as its end result rather than as a diff, as it
rewrites almost all of the setup.
On the BSP, cpuid information is used to evaluate the potential available set
of masking MSRs, and they are unconditionally probed, filling in the
availability information and hardware defaults. A side effect of this is that
probe_intel_cpuid_faulting() can move to being __init.
The command line parameters are then combined with the hardware defaults to
further restrict the Xen default masking level.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This patch is best reviewed as its end result rather than as a diff, as it
rewrites almost all of the setup.
On the BSP, cpuid information is used to evaluate the potential available set
of masking MSRs, and they are unconditionally probed, filling in the
availability information and hardware defaults.
The command line parameters are then combined with the hardware defaults to
further restrict the Xen default masking level.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
A toolstack needs to know how much control Xen has over the visible cpuid
values in PV guests. Provide an explicit mechanism to query what Xen is
capable of.
This interface will currently report no capabilities. This change is
scaffolding for future patches, which will introduce detection and switching
logic, after which the interface will report hardware capabilities correctly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Before c/s 44e24f8567 "x86: don't call generic_identify() redundantly", the
commandline-provided masks would take effect in Xen's view of the processor
features.
As the masks got applied after the query for features, the redundant call to
generic_identify() would clobber the pre-masking feature information with the
post-masking information.
Move the set_cpumask() calls into c_early_init() so the effects of the command
line parameters take place before the main query for features in
generic_identify().
The cpuid_mask_* command line parameters now limit the entire system.
Subsequent changes will cause the mask MSRs to be context switched per-domain,
removing the need to use the command line parameters for heterogeneous
levelling purposes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Currently, {pv,hvm}_cpuid() has a large quantity of essentially-static logic
for modifying the features visible to a guest. A lot of this can be subsumed
by {pv,hvm}_featuremask, which identify the features available on this
hardware which could be given to a PV or HVM guest.
This is a step in the direction of full per-domain cpuid policies, but lots
more development is needed for that. As a result, the static checks are
simplified, but the dynamic checks need to remain for now.
As a side effect, some of the logic for special features can be improved.
OSXSAVE and OSPKE will be automatically cleared because of being absent in the
featuremask. This allows the fast-forward logic to be more simple.
In addition, there are some corrections to the existing logic:
* Hiding PSE36 out of PAE mode is architecturally wrong. It turns out that
it was a bugfix for running HyperV under Xen, which wanted to see PSE36
even after choosing to use PAE paging. PSE36 is not supported by shadow
paging, so is hidden from non-HAP guests, but is still visible for HAP
guests. It is also leaked into non-HAP guests when the guest is already
running in PAE mode.
* Changing the visibility of RDTSCP based on host TSC stability or virtual
TSC mode is bogus, so dropped.
* When emulating Intel to a guest, the common features in e1d should be
cleared.
* The APIC bit in e1d (on non-Intel) is also a fast-forward from the
APIC_BASE MSR.
* A guest with XSAVES and no xcr0|xss features should see
XSTATE_AREA_MIN_SIZE in %ebx (bug in c/s 9d313bde "x86/xsaves: ebx may
return wrong value using CPUID eax=0xd,ecx =1").
As a small improvement, use compiler-visible &'s and |'s, rather than
{clear,set}_bit().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
APIC and XSAVE have dependent features, which also need disabling if Xen
chooses to disable a feature.
Use setup_clear_cpu_cap() rather than clear_bit(), as it takes care of
dependent features as well.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
When clearing a cpu cap, clear all dependent features. This avoids having a
featureset with intermediate features disabled, but leaf features enabled.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Some features depend on other features. Working out and maintaining the exact
dependency tree is complicated, so it is expressed in the automatic generation
script.
At runtime, Xen needs to be disable all features which are dependent on a
feature being disabled. Because of the flattening performed at compile time,
runtime can use a single mask to disable all eventual features.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The existing vIOPL interface is hard to use, and need not be.
Introduce a VMASSIST with which a guest can opt-in to having vIOPL behaviour
consistenly with native hardware.
Specifically:
- virtual iopl updated from do_iret() hypercalls.
- virtual iopl reported in bounce frames.
- guest kernels assumed to be level 0 for the purpose of iopl checks.
v->arch.pv_vcpu.iopl is altered to store IOPL shifted as it would exist
eflags, for the benefit of the assembly code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
All of this information will be used by the toolstack to make informed
levelling decisions for VMs, and by Xen to sanity check toolstack-provided
information.
The split between the shadow and hap HVM masks is necessary due to the lack of
a "get cpuid policy" hypercall. Multi-host toolstacks (i.e. not libxl)
dealing with hap and non-hap capable hosts need to be able to calculate that
migrating a shadow guest is safe.
Future planned development work will implement proper cpuid policy handing in
Xen, including a "get policy" hypercall, but until then, the difference is
made available for toolstack use via a non-stable interface.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Use attributes to specify whether a feature is applicable to be exposed to:
1) All guests
2) HVM guests
3) HVM HAP guests
and, via absence of an attribute, to no guests.
There is no current need for other categories (e.g. PV-only features), and
such categories should not be introduced if possible. These categories follow
from the fact that, with increased hardware support, a guest gets more
features to use.
These settings are derived from the existing code in {pv,hvm}_cpuid(), and
xc_cpuid_x86.c. One notable exception is EXTAPIC which was previously
erroneously exposed to guests. PV guests don't get to use the APIC and the
HVM APIC emulation doesn't support extended space.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Xen, being 64bit only, cannot run PV guests in vm86 mode. HVM guests however
can be running in vm86 mode, and common codepaths need to be able to cope.
The definition of vm86_mode() in x86_64/regs.h is incorrect, as the predicate
is used by non-PV codepaths.
One buggy use is in hvm/emulate.c. An HVM guest can be in vm86 mode, and
vm86_mode() sliently omits the check. Luckily, due to the VEX prefix decoding
logic in x86_emulate(), there is no path to the erronious use with EFLAGS_VM
set.
Another potentially problematic use is in show_guest_stack(). In principle,
show_guest_stack() is common code called for both PV and HVM vcpus. HVM vcpus
exit early (with no reasonable way of making the code generic), making this
part a PV-only codepath.
Open-code its use in emulate.c, matching the surrounding code. This causes
all other uses to be in PV-only codepaths, making the code to be logically
dead. Drop it completely, to avoid future misuse.
Part of resulting cleanup removes vm86attr from read_descriptor(), although
retaining one relevant piece of information; i.e. whether we are reading a
selector for an instruction fetch, or a data fetch.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
c/s 607044bf9 "build: avoid putting local absolute symbols in symbol tables"
breaks the build with Clang, as the command line argument isn't understood.
Clang does not appear to have any equivielent option, and already has
outstanding issues with duplicate symbols. Excluding this option makes the
problem no worse.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Some bits in a featureset are not simple a indication of new functionality,
and require special handling.
APIC, OSXSAVE and OSPKE are fast-forwards of other pieces of state;
IA32_APIC_BASE.EN, CR4.OSXSAVE and CR4.OSPKE. Xen will take care of filling
these appropriately at runtime.
FDP_EXCP_ONLY and NO_FPU_SEL are bits indicating reduced functionality in the
x87 pipeline. The effects of these cannot be hidden from the guest, so the
host values will always be provided.
HTT, X2APIC and CMP_LEGACY indicate how to interpret other cpuid leaves. In
most cases, the toolstack value will be used (with the expectation that these
flags will match the other provided topology information). However with cpuid
masking, the host values are presented as masking cannot influence what the
guest sees in the dependent leaves.
HYPERVISOR is unconditionally set in the PV ABI, but follows the toolstack
setting for HVM guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
If Xen doesn't know about a feature, it is unsafe for use and should be
deliberately hidden from Xen's capabilities.
This doesn't make a practical difference yet, but will make a difference
later when the guest featuresets are seeded from the host featureset.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
New words are:
* 0x80000007.edx - Contains Invarient TSC
* 0x80000008.ebx - Newly used for AMD Zen processors
In addition, replace some open-coded ITSC and EFRO manipulation.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This script consumes include/public/arch-x86/cpufeatureset.h and generates a
single include/asm-x86/cpuid-autogen.h containing all the processed
information.
It currently generates just FEATURESET_NR_ENTRIES. Future changes will
generate more information.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
For the featureset to be a useful object, it needs a stable interpretation, a
property which is missing from the current hw_caps interface.
Additionly, introduce TSC_ADJUST, FDP_EXCP_ONLY, SHA, PREFETCHWT1, ITSC, EFRO
and CLZERO which will be used by later changes.
To maintain compilation, FSCAPINTS is currently hardcoded at 9. Future
changes will change this to being dynamically generated.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
c/s cf39362 "x86: use 2M superpages for text/data/bss mappings" served two
purposes; to map the primary code and data with appropriate pagetable
permissions (rather than unilaterally RWX), and to reduce the TLB pressure.
The extra alignment exposed a SYSLinux issue, and was partly reverted by c/s
0b8a172 "x86: partially revert use of 2M mappings for hypervisor image".
This change reinstates the pagetable permission improvements while avoiding
the 2M alignment issue.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
These features are soon to be exposed in the Xen public API. Rename them to
better match the vendor definitions.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
None of these features are interesting for Xen to use, or to be advertised to
guests. Doing so identifies further areas of code which can be removed now
that 32bit support has been dropped.
IA64 has a sole user in microcode_intel.c. While it is plausible for a 32bit
x86 hypervisor to get there via IA64's x86 emulation, a 64bit x86 hypervisor
most certainly won't.
MP proves to be more complicated. It is only advertised on some K7
processors, not on K8 or newer, and now listed as reserved in the AMD manual.
Cleaning this up reveals two chunks of common SMP code which was only
applicable to K7 processors, which are 32bit only.
While cleaning this area up, remove the inconsistent use of newlines in the
cpu_has_* definition block.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
TRAPBOUNCE_flags are always available via a displacement from %rdx. This
allows all of %rcx to be used as a scratch register.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
The foreign header generation blindly replaces 'uint64_t' with '__align8__
uint64_t', to get correct alignment when built as 32bit. This is correct in
most circumstances, but Clang objects to two specific uses.
* Inside a sizeof() expression
* As part of a typecast
An example error looks like:
/local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44:
error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes]
__align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8];
^~~~~~~~~~
/local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36:
note: expanded from macro '__align8__'
^~~~~~~~~~~
This sedary is sufficient to fix all the bad examples without touching any of
the legitimate uses, and is more simple than teaching mkheader.py how to parse
C.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
If an architecture does not provide a custom page_list_entry, default
page_list_* helpers are provided, wrapping list_head as an underlying type for
page_list_head.
The two declarations of the page_list_* helpers differ between defines and
static inline functions, where the defines discard some of their parameters.
This causes a compilation failure if CONFIG_BIGMEM and debug=n in p2m-pod.c:
p2m-pod.c: In function p2m_pod_cache_add:
p2m-pod.c:72:20: error: unused variable d [-Werror=unused-variable]
struct domain *d = p2m->domain;
^
cc1: all warnings being treated as errors
because the use of d outside of the !NDEBUG section doesn't get evaluated as a
parameter by page_list_del().
Fix this by turning all #defines into static inline functions, so all
parameters are evaluated even if they are not used.
While editing this area, correct the return type of page_list_empty from int
to bool_t.
No functional change.
Reported-by: Doug Goldstein <cardoe@cardoe.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
common/page_alloc.c references d->arch.relmem_list, which only exists on x86.
This only compiles on ARM because page_list_del2() discards its second
argument.
Introduce a new common arch_free_heap_page() which only uses common lists in
struct domain, and allow an architecture to override this with a custom
alternative. x86 then provides a custom arch_free_heap_page() which takes
care of managing d->arch.relmem_list.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS and PEBS
UNAVAIL bits.
Some 64bit Windows include IA32_MISC_ENABLE in the set of items checked by
PatchGuard, and will suffer a BSOD 0x109 CRITICAL_STRUCTURE_CORRUPTION if the
contents change on migrate.
The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
hvmloader is unhosted, and shouldn't use the system errno.h. It already has
to use Xen's errno.h for other hypercalls. The use of public/io/xs_wire.h
requires the use of un-prefixed errno values.
This fixes the build on stricter toolchains where requesting -fno-builtin does
reduce the include path as much as it can.
Reported-by: Doug Goldstein <cardoe@cardoe.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The inclusion rules conditions for errno.h were unnecesserily complicated, and
required the includer to jump through hoops if they wished to avoid getting
multiple namespaces worth of constants.
Simply the logic, and document what is going on.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Otherwise a custom XEN_ERRNO definition will not end up creating appropriately
namespaced constants for the aliases.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
These POSIX errnos are expected by other areas of the Xen public interface,
specifically public/io/xs_wire.h
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The printk formats %*ph{C,D,N} are chosen to be compatible with their Linux
counterparts.
Sample:
(XEN) buf: 00 01 03 07 78 65 6e 00
(XEN) buf: 00:01:03:07:78:65:6e:00
(XEN) buf: 00-01-03-07-78-65-6e-00
(XEN) buf: 0001030778656e00
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Xen now builds on x86 with Clang 3.5 and 3.8. Update README to reflect this.
Mark Clang as no longer a permitted failure in Travis, to prevent future
regressions slipping back in.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Because of the new 2M alignment of .init and .bss, the existing memory
guarding infrastructure causes a shattered 2M superpage with non-present
entries for .init, and present entries for the alignment space.
Do away with the difference in behaviour between debug and non-debug builds;
always destroy the .init mappings, and reuse the space for xenheap.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
This balloons the size of Xen in memory from 4.4MB to 8MB, because of the
required alignment adjustments.
However
* All mappings are 2M superpages.
* .text (and .init at boot) are the only sections marked executable.
* .text and .rodata are marked read-only.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In preparation for using superpage mappings, .data and .bss will both want to
be mapped as read-write. By making them adjacent, they can share the same
superpage and will not require superpage alignment between themselves.
While making this change, fix a latent alignment bug whereby the alignment for
.bss.stack_aligned was in .init. __init_end only needs page alignment (due to
being reclaimed after boot), while .bss.stack_aligned really does needs
STACK_SIZE alignment.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
In preparation for marking .text as read-only, care needs to be taken not to
fault while applying alternatives.
Swapping back to RW mappings is a possibility, but would require additional
TLB management. A temporary disabling of CR0.WP is cleaner.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The use of MAP_SMALL_PAGES causes shattering of the superpages making up the
Xen virtual region, and is counter to the purpose of this series.
Furthermore, it is not required for the memguard infrastructure to function
(which itself uses map_pages_to_xen() for creating holes).
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@citrix.com>
... rather than at runtime.
The bootmaps are discarded in zap_low_mappings(), so the tables themselves can
live in .init.data and be reclaimed after boot.
Hooking the l1_identmap into l2_xenmap stays for safety, along with a longer
comment explaining why.
This does not change the EFI construction of {l2,l3}_bootmap. EFI already
constructs them cleanly in their relocated form.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
* Additional comments, including size and runtime use.
* Consistent use of .quad, rather than a mix including .long.
No change in runtime behaviour.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The entire contents of .lockprofile.data are unchanging pointers to
lock_profile structure in .data. Annotate the type as such, and link the
section in .rodata. As these are just pointers, 32byte alignment is
unnecessary.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang-3.8 generates several .data.rel.ro sections when compiling Xen. As
these contain no global symbols, they should be .data.rel.ro.local. This
breaks the SPECIAL_DATA_SECTIONS check when converting the transition units to
being init-only.
For alternatives.c, explicitly move the nops arrays into __initconst. For efi
boot.c, manually create the optimisation performed by Clang by collapsing the
switch statement into a lookup table. The double use of const is required to
avoid breaking the ARM build by creating a section type conflict with
fdt_guid.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
There is no reason for any of it to be modified. Additionally, link
.init.setup beside the other constant .init data.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
[jb: reduce alignments to 8]
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
1) EXIT_REASON_EXCEPTION_NMI is 0, and Clang complains:
xenalyze.c:513:33: error: initializer overrides prior initialization of this subobject [-Werror,-Winitializer-overrides]
[EXIT_REASON_EXCEPTION_NMI]="EXCEPTION_NMI",
^~~~~~~~~~~~~~~
xenalyze.c:512:11: note: previous initialization is here
[0] = "NONE",
^~~~~~
2) cr3_time_compare(), eip_compare(), ipi_send() and cr3_compare_start() are
declared as nested functions, which is a GCCism not supported by Clang.
As they don't actually make use of the interesting feature offered by
nested functions (i.e. dynamic scoping), move them to just being normal
functions.
3) clear_interval_summary(), update_cpi() and clear_interval_cpi() are all
unused. The former isn't reference anywhere, so is deleted, while the other
two are called from currently #if 0'd code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
These packages are not permitted inside travis, and are not necessary for
building Xen.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
... and annotate the other deliberate one: Coverity objects otherwise.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
One of the two instances was actually a bug.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Clang 3.8 notices, and objects because it is undefined behaviour.
"error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]"
Use unsigned constants rather than signed ones.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Feng Wu <feng.wu@intel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
c/s de0f8c7c changed the use of zlib-options, and moved it from being locally
generated to coming from ./configure.
However, it neglected to modify the users of zlib-options. The curious use of
$(call ...) was to select either the -D or -l options as appropriate, but c/s
de0f8c7c broke this by loosing the `grep`.
Instead, use $(filter ...) to pick out either the -D or -l options. This
fixes the build with Clang, which complains at passing '-llzma' when trying
to compile xc_dom_bzimageloader.c to xc_dom_bzimageloader.o.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
A sample Gentoo compliation of Xen contains
lea -0x1058(%rsp),%rsp
orq $0x0,(%rsp)
lea 0x1020(%rsp),%rsp
Whatever the reason for silly code like this, it fools the current stack
overflow detection logic in the #DF handler (which triggers reliably on the
'orq' instruction).
Update the overflow condition to declare an overflow if %esp is anywhere
within the guard page, rather than just within the upper 8th of the page.
Additionally, check %esp against the expected stack base in all builds.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Coverity correctly identifies that the changes in mtrr_attrib_to_str()
introduce dead code. strings[] is a 2d array, rather than an array of
strings, which means that strings[x] will never be a NULL pointer.
Adjust the check to compenstate, by looking for a NUL in strings[x][0]
instead.
Curiously, Coverity did not notice the same error with memory_type_to_str().
There was also a further error; the strings were not NULL terminated, which
made the return type of memory_type_to_str() erronious.
Bump the 2D array to 3 characters, so the strings retain their NUL characters,
and introduce an ASSERT() as requested on one thread of the original patch.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
pv_cpuid() has two completely separate paths inside it depending on whether
current is dom0 or a domU. This causes unnecessary divergence, and
complicates future improvements. Take steps to undo it.
Changes:
* Create leaf and subleaf variables and use them consistently, instead of a
mix of {a,c} and regs->e{a,c}x as the input parameters.
* Combine the dom0 and domU hypervisor leaf handling, with an early exit.
* Apply sanity checks to domU as well. This brings PV domU cpuid handling in
line with HVM domains and PV dom0.
* Perform a real cpuid instruction for calculating CPUID.0xD[ECX=0].EBX. The
correct xcr0 is in context, and this avoids the O(M*N) loop over the domain
cpuid policy list which exists currently.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Otherwise, debug code such as "void __attribute__((noreturn)) foobar()" fails
to compile when the noreturn itself gets expanded, resulting in
__attribute__((__attribute__((noreturn)))).
No function change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
CentOS 7 gets into trouble when compiling Xen citing:
flushtlb.c: Assembler messages:
flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1
The line number is wrong, and the error message not helpful. It turns out
that the intermediate generated assembly was
# 139 "arch/x86/flushtlb.c" 1
661:
rex clflush (%r15)
662:
.pushsection .altinstructions,"a"
and it was having trouble combining the explicit REX prefix with the REX.B
required for the use of %r15.
Follow what Linux does and use a redundant %ds prefix instead, for a final
generated instruction of `3e 41 0f ae 3f`
While modifying this line, fix the indentation which was out by one space.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Doug Goldstein <cardoe@cardoe.com>
Introduce and use NOP_DS_PREFIX.
Also the above description of the issue is slightly wrong: We're not
suffering from some gas versions not being able to combine multiple REX
prefixes, but from the replacement instruction, when requiring a REX
prefix in order to express the memory operand, becoming one byte longer
than the original one, triggering the respective build time safety
check.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
If two cpus enter show_execution_state() concurrently, the resulting console
output interleaved, and of no help debugging the situation further.
As calls to these locations are rare and usually important, it is acceptable
to serialise them. These codepaths are also on the terminal error paths, so
the console lock must be the lock used for serialisation, to allow
console_force_unlock() to function properly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang objects to having multiple initialisers when creating an array.
As this warning is useful for spotting obscure bugs, disabling it is
unhelpful. Instead, fix our two deliberate usecases.
In the p2m-ept case, pull the array out into a helper function, so the helper
can guarentee to cover the NULL pointer case.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
There are only four users, and invbool_param() is an unnecessary cognitive
overhead to use.
Convert the four users to boolean_param(), and consistency use opt_* for the
variable name.
No change to the behaviour of the command line arguments.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Introducing an X86_FEATURE aliased value turns out to complicate automatic
processing of the feature list. Drop X86_FEATURE_3DNOW_ALT and use
X86_FEATURE_PBE, extending the comment accordingly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang understands the GCCism in use here, but still complains that sp is
unintialised. In such cases, resort to the older version of this code, which
directly reads %rsp into the temporary variable.
Note that we still keep the GCCism in the default case, as it causes GCC to
create rather better assembly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang complains that the __used attribute is not applicable to an extern. In
this case, the only relevent attribute is that the data is constant.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang doesn't like mixing const and non-const data in the same section. Move
zero_page into .bss.page_aligned.const and wildcard .bss.page_aligned when
linking.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang doesn't support the %z modifier. Replace both uses with an explicit l
suffix, and cover the changes with BUILD_BUG_ON()s
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang notices more unused functions than GCC.
* sh_next_page() is only used at GUEST_PAGING_LEVELS=2, so remove it from the
other guest level translation units
* rcu_batch_after() is completely unused.
* Various of the COMPAT() generated functions are used only for their
BUILD_BUG_ON() properties. Annotate them all as __maybe_used.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Clang warns about a semicolon immediately following an if() clause as a
possible mistake, and recommends putting the semicolon on a new line if it was
intentional. A newline is not an option here, so use a set of empty braces
instead.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
c/s 2d2f789 "tools: rename libxc's evtchn_port_or_error_t with an xc_
prefix" doesn't cater for older applications which have requested
XC_WANT_COMPAT_EVTCHN_API
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
c/s 428607a "x86: shrink 'struct domain', was already PAGE_SIZE" introduced a
use-after-free error during domain destruction, because of the order in which
timers are torn down.
(XEN) Xen call trace:
(XEN) [<ffff82d08013344e>] spinlock.c#check_lock+0x1e/0x40
(XEN) [<ffff82d08013349b>] _spin_lock+0x11/0x52
(XEN) [<ffff82d0801e8076>] vpt.c#pt_lock+0x24/0x40
(XEN) [<ffff82d0801e88f4>] destroy_periodic_time+0x18/0x81
(XEN) [<ffff82d0801e1089>] rtc_deinit+0x53/0x78
(XEN) [<ffff82d0801d1e5a>] hvm_domain_destroy+0x52/0x69
(XEN) [<ffff82d08016a758>] arch_domain_destroy+0x1a/0x98
(XEN) [<ffff82d080107cd5>] domain.c#complete_domain_destroy+0x6f/0x182
(XEN) [<ffff82d080126a19>] rcupdate.c#rcu_process_callbacks+0x144/0x1a6
(XEN) [<ffff82d080132c52>] softirq.c#__do_softirq+0x82/0x8d
(XEN) [<ffff82d080132caa>] do_softirq+0x13/0x15
(XEN) [<ffff82d080248ae1>] entry.o#process_softirqs+0x21/0x30
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 3:
(XEN) GENERAL PROTECTION FAULT
(XEN) [error_code=0000]
(XEN) ****************************************
Defer the freeing of d->arch.hvm_domain.pl_time until all timers have been
destroyed.
For safety, NULL out the pointers after freeing them, in an attempt to make
mistakes more obvious in the future.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
c/s 0f1cb96e "x86 hvm: Allow cross-vendor migration" caused HVM domains to
unconditionally intercept #UD exceptions. While cross-vendor migration is
cool as a demo, it is extremely niche.
Intercepting #UD allows userspace code in a multi-vcpu guest to execute
arbitrary instructions in the x86 emulator by having one thread execute a ud2a
instruction, and having a second thread rewrite the instruction before the
emulator performs an instruction fetch.
XSAs 105, 106 and 110 are all examples where guest userspace can use bugs in
the x86 emulator to compromise security of the domain, either by privilege
escalation or causing a crash.
c/s 2d67a7a4 "x86: synchronize PCI config space access decoding"
introduced (amongst other things) a per-domain vendor, based on the guests
cpuid policy.
Use the per-guest vendor to enable #UD interception only when a domain is
configured for a vendor different to the current hardware. (#UD interception
is also enabled if hvm_fep is specified on the Xen command line. This is a
debug-only option whose entire purpose is for testing the x86 emulator.)
As a result, the overwhelming majority of usecases now have #UD interception
disabled, removing an attack surface for malicious guest userspace.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
Most updates to the exception bitmaps set or clear an individual bits.
However, entering or exiting emulated real mode unilaterally clobbers it,
leaving the exit code to recalculate what it should have been. This is error
prone, and indeed currently fails to recalculate the TRAP_no_device intercept
appropriately.
Instead of overwriting exception_bitmap when entering emulated real mode, move
the override into vmx_update_exception_bitmap() and leave exception_bitmap
unmodified.
This means that recalculation is unnecessary, and that the use of
vmx_fpu_leave() and vmx_update_debug_state() while in emulated real mode
doesn't result in TRAP_no_device and TRAP_int3 being un-intercepted.
This is only a functional change on hardware lacking unrestricted guest
support.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
Andrew Cooper