Commit Graph
40260 Commits
Author SHA1 Message Date
Anthony PERARD 9239a90ecd Config.mk: Remove stray comment
This comment isn't about CONFIG_TESTS, but about SEABIOS_DIR that has
been removed.

Originally, the comment was added by 5f82d0858d ("tools: support
SeaBIOS. Use by default when upstream qemu is configured."), then
later the SEABIOS_DIR was removed by 14ee3c05f3 ("Clone and build
Seabios by default") but that comment about the pain was left behind.
The commit that made CONFIG_TESTS painful was 85896a7c4d ("build:
add autoconf to replace custom checks in tools/check").

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-13 16:30:34 +00:00
Anthony PERARD 27d3ea41e6 Config.mk: Remove unused setvar_dir macro
And remove all mention of it in docs. It hasn't been used since
9ead9afcb9 ("Add configure --with-sysconfig-leaf-dir=SUBDIR to set
CONFIG_LEAF_DIR").

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-13 16:30:34 +00:00
Juergen Gross 5852ca4852 build: fix tools/configure in case only python3 exists
Calling ./configure with python3 being there but no python,
tools/configure will fail. Fix that by defaulting to python and
falling back to python3 or python2.

While at it fix the use of non portable "type -p" by replacing it by
AC_PATH_PROG().

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
[ wei: run autogen.sh ]
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-12-12 14:16:43 +00:00
Andrew Cooper b4f042236a AMD/IOMMU: Cease using a dynamic height for the IOMMU pagetables
update_paging_mode() has multiple bugs:

 1) Booting with iommu=debug will cause it to inform you that that it called
    without the pdev_list lock held.
 2) When growing by more than a single level, it leaks the newly allocated
    table(s) in the case of a further error.

Furthermore, the choice of default level for a domain has issues:

 1) All HVM guests grow from 2 to 3 levels during construction because of the
    position of the VRAM just below the 4G boundary, so defaulting to 2 is a
    waste of effort.
 2) The limit for PV guests doesn't take memory hotplug into account, and
    isn't dynamic at runtime like HVM guests.  This means that a PV guest may
    get RAM which it can't map in the IOMMU.

The dynamic height is a property unique to AMD, and adds a substantial
quantity of complexity for what is a marginal performance improvement.  Remove
the complexity by removing the dynamic height.

PV guests now get 3 or 4 levels based on any hotplug regions in the host.
This only makes a difference for hardware which previously had all RAM below
the 512G boundary, and a hotplug region above.

HVM guests now get 4 levels (which will be sufficient until 256TB guests
become a thing), because we don't currently have the information to know when
3 would be safe to use.

The overhead of this extra level is not expected to be noticeable.  It costs
one page (4k) per domain, and one extra IO-TLB paging structure cache entry
which is very hot and less likely to be evicted.

This is XSA-311.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-11 14:55:32 +01:00
George Dunlap 66bdc16aee x86/mm: relinquish_memory: Grab an extra type ref when setting PGT_partial
The PGT_partial bit in page->type_info holds both a type count and a
general ref count.  During domain tear-down, when free_page_type()
returns -ERESTART, relinquish_memory() correctly handles the general
ref count, but fails to grab an extra type count when setting
PGT_partial.  When this bit is eventually cleared, type_count underflows
and triggers the following BUG in page_alloc.c:free_domheap_pages():

    BUG_ON((pg[i].u.inuse.type_info & PGT_count_mask) != 0);

As far as we can tell, this page underflow cannot be exploited any any
other way: The page can't be used as a pagetable by the dying domain
because it's dying; it can't be used as a pagetable by any other
domain since it belongs to the dying domain; and ownership can't
transfer to any other domain without hitting the BUG_ON() in
free_domheap_pages().

(steal_page() won't work on a page in this state, since it requires
PGC_allocated to be set, and PGC_allocated will already have been
cleared.)

Fix this by grabbing an extra type ref if setting PGT_partial in
relinquish_memory.

This is part of XSA-310.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-11 14:55:08 +01:00
George Dunlap 4e70f4476c x86/mm: alloc/free_lN_table: Retain partial_flags on -EINTR
When validating or de-validating pages (in alloc_lN_table and
free_lN_table respectively), the `partial_flags` local variable is
used to keep track of whether the "current" PTE started the entire
operation in a "may be partial" state.

One of the patches in XSA-299 addressed the fact that it is possible
for a previously-partially-validated entry to subsequently be found to
have invalid entries (indicated by returning -EINVAL); in which case
page->partial_flags needs to be set to indicate that the current PTE
may have the partial bit set (and thus _put_page_type() should be
called with PTF_partial_set).

Unfortunately, the patches in XSA-299 assumed that once
put_page_from_lNe() returned -ERESTART on a page, it was not possible
for it to return -EINTR.  This turns out to be true for
alloc_lN_table() and free_lN_table, but not for _get_page_type() and
_put_page_type(): both can return -EINTR when called on pages with
PGT_partial set.  In these cases, the pages PGT_partial will still be
set; failing to set partial_flags appropriately may allow an attacker
to do a privilege escalation similar to those described in XSA-299.

Fix this by always copying the local partial_flags variable into
page->partial_flags when exiting early.

NB that on the "get" side, no adjustment to nr_validated_entries is
needed: whether pte[i] is partially validated or entirely
un-validated, we want nr_validated_entries = i.  On the "put" side,
however, we need to adjust nr_validated_entries appropriately: if
pte[i] is entirely validated, we want nr_validated_entries = i + 1; if
pte[i] is partially validated, we want nr_validated_entries = i.

This is part of XSA-310.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-11 14:54:43 +01:00
George Dunlap ececa12b2c x86/mm: Set old_guest_table when destroying vcpu pagetables
Changeset 6c4efc1eba ("x86/mm: Don't drop a type ref unless you held a
ref to begin with"), part of XSA-299, changed the calling discipline
of put_page_type() such that if put_page_type() returned -ERESTART
(indicating a partially de-validated page), subsequent calls to
put_page_type() must be called with PTF_partial_set.  If called on a
partially de-validated page but without PTF_partial_set, Xen will
BUG(), because to do otherwise would risk opening up the kind of
privilege escalation bug described in XSA-299.

One place this was missed was in vcpu_destroy_pagetables().
put_page_and_type_preemptible() is called, but on -ERESTART, the
entire operation is simply restarted, causing put_page_type() to be
called on a partially de-validated page without PTF_partial_set.  The
result was that if such an operation were interrupted, Xen would hit a
BUG().

Fix this by having vcpu_destroy_pagetables() consistently pass off
interrupted de-validations to put_old_page_type():
- Unconditionally clear references to the page, even if
  put_page_and_type failed
- Set old_guest_table and old_guest_table_partial appropriately

While here, do some refactoring:

 - Move clearing of arch.cr3 to the top of the function

 - Now that clearing is unconditional, move the unmap to the same
   conditional as the l4tab mapping.  This also allows us to reduce
   the scope of the l4tab variable.

 - Avoid code duplication by looping to drop references on
   guest_table_user

This is part of XSA-310.

Reported-by: Sarah Newman <srn@prgmr.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-11 14:54:13 +01:00
George Dunlap 7473efd12f x86/mm: Don't reset linear_pt_count on partial validation
"Linear pagetables" is a technique which involves either pointing a
pagetable at itself, or to another pagetable the same or higher level.
Xen has limited support for linear pagetables: A page may either point
to itself, or point to another page of the same level (i.e., L2 to L2,
L3 to L3, and so on).

XSA-240 introduced an additional restriction that limited the "depth"
of such chains by allowing pages to either *point to* other pages of
the same level, or *be pointed to* by other pages of the same level,
but not both.  To implement this, we keep track of the number of
outstanding times a page points to or is pointed to another page
table, to prevent both from happening at the same time.

Unfortunately, the original commit introducing this reset this count
when resuming validation of a partially-validated pagetable, dropping
some "linear_pt_entry" counts.

On debug builds on systems where guests used this feature, this might
lead to crashes that look like this:

    Assertion 'oc > 0' failed at mm.c:874

Worse, if an attacker could engineer such a situation to occur, they
might be able to make loops or other abitrary chains of linear
pagetables, leading to the denial-of-service situation outlined in
XSA-240.

This is XSA-309.

Reported-by: Manuel Bouyer <bouyer@antioche.eu.org>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-11 14:10:27 +01:00
Andrew Cooper 1d3eb82598 x86/vtx: Work around SingleStep + STI/MovSS VMEntry failures
See patch comment for technical details.

Concerning the timeline, this was first discovered in the aftermath of
XSA-156 which caused #DB to be intercepted unconditionally, but only in
its SingleStep + STI form which is restricted to privileged software.

After working with Intel and identifying the problematic vmentry check,
this workaround was suggested, and the patch was posted in an RFC
series.  Outstanding work for that series (not breaking Introspection)
is still pending, and this fix from it (which wouldn't have been good
enough in its original form) wasn't committed.

A vmentry failure was reported to xen-devel, and debugging identified
this bug in its SingleStep + MovSS form by way of INT1, which does not
involve the use of any privileged instructions, and proving this to be a
security issue.

This is XSA-308

Reported-by: Håkon Alstadheim <hakon@alstadheim.priv.no>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
2019-12-11 14:09:30 +01:00
Jan Beulich 7442006b9f x86+Arm32: make find_next_{,zero_}bit() have well defined behavior
These functions getting used with the 2nd and 3rd arguments being equal
wasn't well defined: Arm64 reliably returns the value of the 2nd
argument in this case, while on x86 for bitmaps up to 64 bits wide the
return value was undefined (due to the undefined behavior of a shift of
a value by the number of bits it's wide) when the incoming value was 64.
On Arm32 an actual out of bounds access would happen when the
size/offset value is a multiple of 32; if this access doesn't fault, the
return value would have been sufficiently correct afaict.

Make the functions consistently tolerate the last two arguments being
equal (and in fact the 3rd argument being greater or equal to the 2nd),
in favor of finding and fixing all the use sites that violate the
original more strict assumption.

This is XSA-307.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien@xen.org>
2019-12-11 14:06:18 +01:00
Wei Liu 0b65f0a428 Config.mk: update seabios to 1.13.0
Signed-off-by: Wei Liu <wl@xen.org>
2019-12-11 12:02:26 +00:00
Wei Liu 80268f6364 x86: add a comment regarding the location of hypervisor_probe
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-11 11:36:50 +00:00
Juergen Gross 95de323451 SUPPORT.md: add core scheduling
Add core scheduling feature to SUPPORT.md.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-11 11:22:39 +00:00
Andrew Cooper 272c18435e docs/sphinx: How Xen Boots on x86
Begin to document how the x86 build of Xen boots.  It is by no means complete,
but is a start.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-10 17:23:39 +00:00
Andrew Cooper c8a8645f1e xen/build: Automatically locate a suitable python interpreter
Needing to pass PYTHON=python3 into hypervisor builds is irritating and
unnecessary.  Locate a suitable interpreter automatically, defaulting to Py3
if it is available.

Reported-by: Steven Haigh <netwiz@crc.id.au>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-12-10 17:23:39 +00:00
Andrew Cooper 9fc7c88e62 xen/banner: Drop the fig-to-oct.py script
The script is 664 rather than 775, so the banner conversion doesn't actually
work if $(PYTHON) is empty:

  /bin/sh: tools/fig-to-oct.py: Permission denied
  make[3]: *** [include/xen/compile.h] Error 126
  make[3]: Leaving directory `/builds/xen-project/people/andyhhp/xen/xen'

Fixing this is easy, but using python here is wasteful.  compile.h doesn't
need XEN_BANNER rendering in octal, and text is much more simple to handle.
Replace fig-to-oct.py with a smaller sed script.  This could be a shell
one-liner, but it is much more simple to comment sensibly, and doesn't need to
include the added cognative load of makefile and shell escaping.

While changing this logic, take the opportunity to optimise the banner
space (and time on the serial port) by dropping trailing whitespace, which is
84 characters for current staging.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
2019-12-10 17:23:39 +00:00
Andrew Cooper 8d5671eb31 xen/flask: Drop the gen-policy.py script
The script is Python 2 specific, and fails with string/binary issues with
Python 3:

  Traceback (most recent call last):
    File "gen-policy.py", line 14, in <module>
      for char in sys.stdin.read():
    File "/usr/lib/python3.5/codecs.py", line 321, in decode
      (result, consumed) = self._buffer_decode(data, self.errors, final)
  UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte

Fixing the script to be compatible isn't hard, but using python here is
wasteful.  Drop the script entirely, and write an equivelent flask-policy.S
instead.  This removes the need for a $(PYTHON) and $(CC) pass.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Julien Grall <julien@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-12-10 17:23:39 +00:00
Razvan Cojocaru 4935a5433d remove myself as vm_event maintainer
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
2019-12-10 11:34:33 +01:00
Roger Pau Monné 5de961d9c0 x86: do not enable global pages when virtualized on AMD or Hygon hardware
When using global pages a full tlb flush can only be performed by
toggling the PGE bit in CR4, which is usually quite expensive in terms
of performance when running virtualized. This is specially relevant on
AMD or Hygon hardware, which doesn't have the ability to do selective
CR4 trapping, but can also be relevant on e.g. Intel if the underlying
hypervisor also traps accesses to the PGE CR4 bit.

In order to avoid this performance penalty, do not use global pages
when running virtualized on AMD or Hygon hardware. A command line option
'global-pages' is provided in order to allow the user to select whether
global pages will be enabled for PV guests.

The above figures are from a PV shim running on AMD hardware with
32 vCPUs:

PGE enabled, x2APIC mode:

(XEN) Global lock flush_lock: addr=ffff82d0804b01c0, lockval=1adb1adb, not locked
(XEN)   lock:1841883(1375128998543), block:1658716(10193054890781)

Average lock time:   746588ns
Average block time: 6145147ns

PGE disabled, x2APIC mode:

(XEN) Global lock flush_lock: addr=ffff82d0804af1c0, lockval=a8bfa8bf, not locked
(XEN)   lock:2730175(657505389886), block:2039716(2963768247738)

Average lock time:   240829ns
Average block time: 1453029ns

As seen from the above figures the lock and block time of the flush
lock is reduced to approximately 1/3 of the original value.

Note that XEN_MINIMAL_CR4 and mmu_cr4_features are not modified, and
thus global pages are left enabled for the hypervisor. This is not an
issue because the code to switch the control registers (cr3 and cr4)
already takes into account such situation and performs the necessary
flushes. The same already happens when using XPTI or PCIDE, as the
guest cr4 doesn't have global pages enabled in that case either.

Also note that the suspend and resume code is correct in writing
mmu_cr4_features into cr4 on resume, since that's the cr4 used by the
idle vCPU which is the context used by the suspend and resume routine.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-10 11:34:00 +01:00
Igor Druzhinin 0f3cdbdf7a x86/AMD: unbreak CPU hotplug on AMD systems without RstrFpErrPtrs
If the feature is not present Xen will try to force X86_BUG_FPU_PTRS
feature at CPU identification time. This is especially noticeable in
PV-shim that usually hotplugs its vCPUs. We either need to restrict this
action for boot CPU only or allow secondary CPUs to modify
forced CPU capabilities at runtime. Choose the former since modifying
forced capabilities out of boot path leaves the system in potentially
inconsistent state.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-10 11:07:22 +01:00
Jan Beulich b73aad4c8b x86/i8259A: don't open-code LEGACY_VECTOR()
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-09 14:03:01 +01:00
Krzysztof Kolasa 5d90ff7954 lz4: fix system halt at boot kernel on x86_64
Sometimes, on x86_64, decompression fails with the following
error:

Decompressing Linux...

Decoding failed

 -- System halted

This condition is not needed for a 64bit kernel(from commit d5e7caf):

if( ... ||
    (op + COPYLENGTH) > oend)
    goto _output_error

macro LZ4_SECURE_COPY() tests op and does not copy any data
when op exceeds the value.

added by analogy to lz4_uncompress_unknownoutputsize(...)

Signed-off-by: Krzysztof Kolasa <kkolasa@winsoft.pl>
[Linux commit 99b7e93c95c78952724a9783de6c78def8fbfc3f]

The offending commit in our case is fcc17f96c2 ("LZ4 : fix the data
abort issue").

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-09 14:02:35 +01:00
Rasmus Villemoes fef4eb01bc lz4: pull out constant tables
There's no reason to allocate the dec{32,64}table on the stack; it
just wastes a bunch of instructions setting them up and, of course,
also consumes quite a bit of stack. Using size_t for such small
integers is a little excessive.

$ scripts/bloat-o-meter /tmp/built-in.o lib/built-in.o
add/remove: 2/2 grow/shrink: 2/0 up/down: 1304/-1548 (-244)
function                                     old     new   delta
lz4_decompress_unknownoutputsize              55     718    +663
lz4_decompress                                55     632    +577
dec64table                                     -      32     +32
dec32table                                     -      32     +32
lz4_uncompress                               747       -    -747
lz4_uncompress_unknownoutputsize             801       -    -801

The now inlined lz4_uncompress functions used to have a stack
footprint of 176 bytes (according to -fstack-usage); their inlinees
have increased their stack use from 32 bytes to 48 and 80 bytes,
respectively.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
[Linux commit bea2b592fd18eb8ffa3fc4ad380610632d03a38f]

Use {,u}int8_t instead of plain "int" for the tables.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-09 14:01:56 +01:00
Jan Beulich 2d7572cdfa lz4: refine commit 9143a6c55e for the 64-bit case
I clearly went too far there: While the LZ4_WILDCOPY() instances indeed
need prior guarding, LZ4_SECURECOPY() needs this only in the 32-bit case
(where it simply aliases LZ4_WILDCOPY()). "cpy" can validly point
(slightly) below "op" in these cases, due to

		cpy = op + length - (STEPSIZE - 4);

where length can be as low as 0 and STEPSIZE is 8. However, instead of
removing the check via "#if !LZ4_ARCH64", refine it such that it would
also properly work in the 64-bit case, aborting decompression instead
of continuing on bogus input.

Reported-by: Mark Pryor <pryorm09@gmail.com>
Reported-by: Jeremi Piotrowski <jeremi.piotrowski@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Mark Pryor <pryorm09@gmail.com>
Tested-by: Jeremi Piotrowski <jeremi.piotrowski@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-09 14:01:25 +01:00
Jan Beulich b813ce366a x86/CPUID: RSTR_FP_ERR_PTRS depends on FPU
There's nothing to restore here if there's no FPU in the first place.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-09 14:00:15 +01:00
Andrew Cooper ae25407faa docs/sphinx: License content with CC-BY-4.0
Creative Commons is a more common license than GPL for documentation purposes.
Switch to using CC-BY-4.0 to explicitly permit re-purposing and remixing of
the content.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Lars Kurth <lars.kurth@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-12-06 18:45:43 +00:00
Jan Beulich 8359dde718 x86: support Atom Tremont
Add model 0x86 to relevant switch() statements, as per SDM 069 Vol 4.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-06 11:02:48 +01:00
Jan Beulich 0f216f8eb8 x86: don't offer Hyper-V option when "PV Shim Exclusive"
This only added dead code. Use "if" instead of "depends on" to make
(halfway) clear that other guest options should also go in the same
block. Move the option down such that the shim related options get
presented first, avoiding to ask the question when the answer may end
up being discarded.

While in the neighborhood also bring PV_SHIM_EXCLUSIVE into more
"canonical" shape.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-06 11:02:09 +01:00
Jan Beulich f5536cd68b x86/nEPT: ditch nept_sp_entry()
It's bogusly non-static. It making the call sites actually less easy to
read, and there being another open-coded use in the file - let's just
get rid of it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
2019-12-06 11:01:18 +01:00
Andrew Cooper 131c89ce1e x86/svm: Use named (bit)fields for task switch exit info
Introduce vmcb.ei.* to provide names to fields in exitinfo{1,2}.  Implement
the task switch names for now, and clean up the TASK_SWITCH handler.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-05 13:19:28 +00:00
Andrew Cooper 644f61d363 x86/svm: Clean up intinfo_t variables
The type name is poor because the type is also used for the IDT vectoring
field, not just for the event injection field.  Rename it to intinfo_t which
is how the APM refers to the data.

Rearrange the union to drop the .fields infix, and rename bytes to the more
common raw.  Also take the opportunity to rename the fields in the VMCB to
increase legibility.

While adjusting all call sites, fix up style issues and make use of structure
assignments where applicable.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-05 13:19:28 +00:00
Andrew Cooper 115c05a5f9 x86/svm: Don't shadow variables in svm_vmexit_handler()
The local variable eventinj is set to the value of vmcb->exitintinfo which is
confusing considering that it isn't vmcb->eventinj.  The variable isn't
necessary to begin with, so drop it to avoid confusion.

A local rc variable is shadowed in the CPUID, #DB and #BP handlers.

There is a mix of spelling of inst_len and insn_len, all of which are
logically the same value.  Consolidate on insn_len which also matches the name
of the emulation functions for obtaining instruction lengths, and avoid
shadowing it in the CPUID and TASK_SWITCH handlers.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-05 13:19:28 +00:00
Andrew Cooper da650d3810 x86/svm: Clean up construct_vmcb()
The vmcb is zeroed on allocate - drop all explicit writes of 0.  Move
hvm_update_guest_efer() to co-locate it with the other control register
updates.

Move the BUILD_BUG_ON() into build_assertions(), and add some offset checks
for fields after the large blocks of reserved fields (as these are the most
likely to trigger from a mis-edit).  Take the opportunity to fold 6 adjacent
res* fields into one.

Finally, drop all trailing whitespace in the file.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-05 13:19:28 +00:00
Andrew Cooper a57e0b3980 x86/svm: Fix handling of EFLAGS.RF on task switch
VT-x updates RF before vmexit, so eflags written into the outgoing TSS happens
to be correct.  SVM does not update RF before vmexit, and instead provides it
via a bit in exitinfo2.

In practice, needing RF set in the outgoing state occurs when a task gate is
used to handle faults.

Extend hvm_task_switch() with an extra_eflags parameter which gets fed into
the outgoing TSS, and fill it in suitably from the SVM vmexit information.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-05 13:19:28 +00:00
Andrew Cooper efff60f8be x86/svm: Minor cleanup to start_svm()
The function is init, so can use boot_cpu_data directly.

There is no need to write 0 to svm_feature_flags in the case of a CPUID
mismatch (not least because this is dead code on real hardware), and no need
to use locked bit operations.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-05 13:19:28 +00:00
Igor Druzhinin 79cf098917 passthrough: drop break statement following c/s cd7dedad82
The locking responsibilities have changed and a premature break in
this section now causes the following assertion:

Assertion '!preempt_count()' failed at preempt.c:36

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Reviewed-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-05 13:31:03 +01:00
Andrew Cooper ad5c7c1625 x86/svm: Correct vm_event API for descriptor accesses
c/s d0a699a389 "x86/monitor: add support for descriptor access events"
introduced logic looking for what appeared to be exitinfo (not that this
exists in SVM - exitinfo1 or 2 do), but actually passed the exit IDT vectoring
information.  There is never any IDT vectoring involved in these intercepts so
the value passed is always zero.

In fact, SVM doesn't provide any information, even in exitinfo1 and 2.  Drop
the svm struct entirely, and bump the interface version.

In the SVM vmexit handler itself, optimise the switch statement by observing
that there is a linear transformation between the SVM exit_reason and
VM_EVENT_DESC_* values.  (Bloat-o-meter reports 6028 => 5877 for a saving of
151 bytes).

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Reviewed-by: Alexandru Isaila <aisaila@bitdefender.com>
Acked-by: Adrian Pop <apop@bitdefender.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 19:05:00 +00:00
Wei Liu 05de315b00 x86: introduce CONFIG_HYPERV and detection code
We use the same code structure as we did for Xen.

As starters, detect Hyper-V in probe routine. More complex
functionalities will be added later.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:38:22 +00:00
Wei Liu d815e9272c x86: be more verbose when running on a hypervisor
Also replace reference to xen_guest.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:38:22 +00:00
Wei Liu a5d847ed64 x86: switch xen guest implementation to use hypervisor framework
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:38:22 +00:00
Wei Liu 62f946a7bc x86: rename hypervisor_{alloc,free}_unused_page
They are used in Xen code only.

No functional change.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:38:22 +00:00
Wei Liu 49bd351aee x86: introduce hypervisor framework
We will soon implement Hyper-V support for Xen. Add a framework for
that.

This requires moving some of the hypervisor_* functions from xen.h to
hypervisor.h.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:38:17 +00:00
Wei Liu 1c178ffb29 x86: drop hypervisor_cpuid_base
The only user is Xen specific code in PV shim. We can therefore export
the variable directly.

Move __read_mostly to its standard place while at it.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:31:25 +00:00
Wei Liu cd78188af5 x86: add missing headers in hypercall.h
Include asm_defns.h because ASM_CALL_CONSTRAINT is defined there.

Include xen/lib.h because we need ASSERT_UNREACHABLE.

No functional change.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:31:25 +00:00
Wei Liu ca363f186d x86: introduce CONFIG_GUEST and move code
Xen is able to run as a guest on Xen. We plan to make it able to run
on Hyper-V as well.

Introduce CONFIG_GUEST which is set to true if either running on Xen
or Hyper-V is desired. Restructure code hierarchy for new code to
come.

No functional change intended.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2019-12-04 10:31:25 +00:00
Roger Pau Monne d7c3e6c9e9 automation: increase tests maximum time from 10s to 30s
10s is too low for the clang tests, this is the output from a clang
test:

  (XEN) [    6.512748] ***************************************************
  (XEN) [    6.513323] SELFTEST FAILURE: CORRECT BEHAVIOR CANNOT BE GUARANTEED
  (XEN) [    6.513891] ***************************************************
  (XEN) [    6.514469] 3... 2... 1...
  (XEN) [    9.520011] *** Serial input to DOM0 (type 'CTRL-a' three times to switch input)
  (XEN) [    9.544319] Freed 488kB init memory
  --- Xen Test Framework ---
  Environment: HVM 32bit (PAE 3 levels)
  Hello World
  Test result: SUCCESS
  (XEN) [    9.610977] Hardware Dom0 halted: halting machine

As can be seen from the output above booting Xen and the XTF test
takes ~10s, without accounting for the time it takes for QEMU to
initialize.

Increase the timeout to 30s to be on the safe side.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-12-03 13:54:23 +00:00
Roger Pau Monne 2ed19eaccf automation: add timestamps to Xen tests
Enable Xen timestamps in the automated Xen tests, this is helpful in
order to figure out if Xen is stuck or just slow in the automated
tests.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-12-03 13:53:59 +00:00
Roger Pau Monné b5087a31ef x86/tlbflush: do not toggle the PGE CR4 bit unless necessary
When PCID is not available Xen does a full tlbflush by toggling the
PGE bit in CR4. This is not necessary if PGE is not enabled, since a
flush can be performed by writing to CR3 in that case.

Change the code in do_tlb_flush to only toggle the PGE bit in CR4 if
it's already enabled, otherwise do the tlb flush by writing to CR3.
This is relevant when running virtualized, since hypervisors don't
usually trap accesses to CR3 when using hardware assisted paging, but
do trap accesses to CR4 specially on AMD hardware, which makes such
accesses much more expensive.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2019-12-03 14:15:35 +01:00
Jan Beulich d5294a302c x86: avoid HPET use on certain Intel platforms
Linux commit fc5db58539b49351e76f19817ed1102bf7c712d0 says

"Some Coffee Lake platforms have a skewed HPET timer once the SoCs entered
 PC10, which in consequence marks TSC as unstable because HPET is used as
 watchdog clocksource for TSC."

Follow this for Xen as well. Looking at its patch context made me notice
they have a pre-existing quirk for Bay Trail as well. The comment there,
however, points at a Cherry Trail document. Looking at the datasheets of
both, there appear to be similar issues, so go beyond Linux'es coverage
and exclude both. Also key the disable on the PCI IDs of the actual
affected devices, rather than those of 00:00.0.

Apply the workarounds only when the use of HPET was not explicitly
requested on the command line and when use of (deep) C-states was not
disabled.

Adjust a few types in touched or nearby code at the same time.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-03 14:14:44 +01:00
Jan Beulich 921f1f4226 gnttab: make sure grant map operations don't skip their IOMMU part
Two almost simultaneous mapping requests need to make sure that at the
completion of the earlier one IOMMU mappings (established explicitly
here in the PV case) have been put in place. Forever since the splitting
of the grant table lock a violation of this has been possible (using
simplified pin counts, as it doesn't matter whether we talk about read
or write mappings here):

initial state: act->pin = 0

vCPU A: progress the operation past the dropping of the locks after the
        act->pin updates (act->pin = 1, old_pin = 0, act_pin = 1)

vCPU B: progress the operation past the dropping of the locks after the
        act->pin updates (act->pin = 2, old_pin = 1, act_pin = 2)

vCPU B: (re-)acquire both gt locks, mapkind() returns 0, but both
        iommu_legacy_map() invocations get skipped due to non-zero
        old_pin

vCPU B: return to caller without IOMMU mapping

vCPU A: (re-)acquire both gt locks, mapkind() returns 0,
        iommu_legacy_map() gets invoked

With the locks dropped intermediately, whether to invoke
iommu_legacy_map() must depend on only the return value of mapkind()
and of course the kind of mapping request being processed, just like
is already the case in unmap_common().

Also fix the style of the adjacent comment, and correct a nearby one
still referring to a prior name of what is now mapkind().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2019-12-03 14:13:40 +01:00