Jan Beulich [Tue, 24 Oct 2017 16:13:13 +0000 (18:13 +0200)]
x86: also show FS/GS base addresses when dumping registers
Their state may be important to figure the reason for a crash. To not
further grow duplicate code, break out a helper function.
I realize that (ab)using the control register array here may not be
considered the nicest solution, but it seems easier (and less overall
overhead) to do so compared to the alternative of introducing another
helper structure.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Jan Beulich [Tue, 24 Oct 2017 16:12:31 +0000 (18:12 +0200)]
x86: fix GS-base-dirty determination
load_segments() writes the two MSRs in their "canonical" positions
(GS_BASE for the user base, SHADOW_GS_BASE for the kernel one) and uses
SWAPGS to switch them around if the incoming vCPU is in kernel mode. In
order to not leave a stale kernel address in GS_BASE when the incoming
guest is in user mode, the check on the outgoing vCPU needs to be
dependent upon the mode it is currently in, rather than blindly looking
at the user base.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Chao Gao [Tue, 24 Oct 2017 14:02:40 +0000 (16:02 +0200)]
x86/vpt: guarantee the return value of pt_update_irq() set in vIRR or PIR
pt_update_irq() is expected to return the vector number of periodic
timer interrupt, which should be set in vIRR of vlapic or in PIR.
Otherwise it would trigger the assertion in vmx_intr_assist(), please
seeing https://lists.xenproject.org/archives/html/xen-devel/2017-10/msg00915.html.
But it fails to achieve that in the following two case:
1. hvm_isa_irq_assert() may not set the corresponding bit in vIRR for
mask field of IOAPIC RTE is set. Please refer to the call tree
vmx_intr_assist() -> pt_update_irq() -> hvm_isa_irq_assert() ->
assert_irq() -> assert_gsi() -> vioapic_irq_positive_edge(). The patch
checks whether the vector is set or not in vIRR of vlapic or PIR before
returning.
2. someone changes the vector field of IOAPIC RTE between asserting
the irq and getting the vector of the irq, leading to setting the
old vector number but returning a different vector number. This patch
allows hvm_isa_irq_assert() to accept a callback which can get the
interrupt vector with irq_lock held. Thus, no one can change the vector
between the two operations.
BTW, the first argument of pi_test_and_set_pir() should be uint8_t
and I take this chance to fix it.
Signed-off-by: Chao Gao <chao.gao@intel.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Jan Beulich [Tue, 24 Oct 2017 14:01:33 +0000 (16:01 +0200)]
gnttab: fix pin count / page reference race
Dropping page references before decrementing pin counts is a bad idea
if assumptions are being made that a non-zero pin count implies a valid
page. Fix the order of operations in gnttab_copy_release_buf(), but at
the same time also remove the assertion that was found to trigger:
map_grant_ref() also has the potential of causing a race here, and
changing the order of operations there would likely be quite a bit more
involved.
This is CVE-2017-15597 / XSA-236.
Reported-by: Pawel Wieczorkiewicz <wipawel@amazon.de> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Ian Jackson [Fri, 20 Oct 2017 10:42:42 +0000 (11:42 +0100)]
libxl: Replace open-coded __attribute__ with NN() macro
Inspired by
#define __nonnull(...) __attribute__((__nonnull__(__VA_ARGS__)))
which is used in the hypervisor.
These annotations may well become very common in libxl, so we choose a
short name.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org> Acked-by: Wei Liu <wei.liu2@citrix.com> CC: Andrew Cooper <andrew.cooper3@citrix.com>
Wei Liu [Mon, 16 Oct 2017 14:04:10 +0000 (15:04 +0100)]
libxl: annotate s to be nonnull in libxl__enum_from_string
Hope this can placate coverity.
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Anthony PERARD [Thu, 19 Oct 2017 14:29:56 +0000 (15:29 +0100)]
tools/Makefile: unset MAKELEVEL before building QEMU
Since QEMU commits aef45d51d1204f3335fb99de6658e0c5612c2b67
"build: automatically handle GIT submodule checkout for dtc"
the QEMU makefiles rely on the variable MAKELEVEL to make a decision on
whether to update some git submodules or not. Since we call QEMU build
from within the Xen one, MAKELEVEL would already be greater than 0 and
the git submodules would not be updated and QEMU would fail to build.
Fix this by removing MAKELEVEL from the environment before trying to
build QEMU.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Jan Beulich [Fri, 20 Oct 2017 07:31:54 +0000 (09:31 +0200)]
gcov: support gcc 7.x
Taking Linux commit 0538421343 ("gcov: support GCC 7.1") as reference,
enable gcc 7 support requiring __gcov_exit() and having 9 counters.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Roger Pau Monné [Fri, 20 Oct 2017 07:30:13 +0000 (09:30 +0200)]
ubsan: add clang 5.0 support
clang 5.0 changed the layout of the type_mismatch_data structure and
introduced __ubsan_handle_type_mismatch_v1 and
__ubsan_handle_pointer_overflow.
This commit adds support for the new structure layout, adds the
missing handlers and the new types for type_check_kinds.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
[jb: unconditionally emit always the same message in
__ubsan_handle_pointer_overflow()] Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
David Esler [Fri, 20 Oct 2017 07:29:29 +0000 (09:29 +0200)]
x86/boot: fix early error output
In 9180f5365524 a change was made to the send_chr function to take in
C-strings and output a character at a time until a NULL was encountered.
However, when there is no VGA there is no code to increment the current
character position resulting in an endless loop of the first character.
This moves the (implicit) increment such that it occurs in all cases.
Signed-off-by: David Esler <drumandstrum@gmail.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
[jb: correct title and description] Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is due to memmove doing a n-1+addr when n is 0. This is harmless
because the loop is bounded to 0. Fix this by returning earlier when n
is 0.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
[jb: add return value and unlikely()] Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Julien Grall [Thu, 19 Oct 2017 17:09:05 +0000 (18:09 +0100)]
xen/arm: gic-v3: Make sure ICC_SRE_EL1 is restored before ICH_VMCR_EL2
Per 8.4.8 in ARM IHI 0069D, ICH_VMCR_EL2.VFIQEn is RES1 when
ICC_SRE_EL1.SRE is 1. This causes a Group 0 interrupt (as generated in
GICv2 mode) to be delivered as a FIQ to the guest, with potentially
consequence. So we must make sure that ICC_SRE_EL1 has been actually
programmed before at ICH_VMCR_EL2.
This was discovered when booting EFI in a GICv2 guest on a GICv3
hardware.
arm: configure interrupts to be in non-secure group1
Xen uses non-secure group1 interrupts, however it doesn't configure the
GICv3 accordingly. Xen needs to set GICD_IGROUPR for SPIs and
GICR_IGROUPR0 for local interrupt to "1" to specify that interrupts
belong to group1. This is particularly important if the system has
GICD_CTLR.DS set, also see commit 7c9b973061b03af62734f613f6abec46c0dd4a88 in Linux.
Andrew Cooper [Tue, 17 Oct 2017 14:11:23 +0000 (15:11 +0100)]
xen/public: Correct the definition of GNTTAB_CACHE_SOURCE_GREF
Discovered when running the XSA-232 PoC on a UBSAN-enabled hypervisor.
(d79) XSA-232 PoC
(XEN) ================================================================================
(XEN) UBSAN: Undefined behaviour in grant_table.c:3217:25
(XEN) left shift of 1 by 31 places cannot be represented in type 'int'
(XEN) ----[ Xen-4.10.0-rc x86_64 debug=y Tainted: H ]----
Update all of the GNTTAB_CACHE_* constants to be unsigned integers.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Andrew Cooper [Tue, 29 Aug 2017 10:35:31 +0000 (10:35 +0000)]
x86/mm: Consolidate all Xen L4 slot writing into init_xen_l4_slots()
There are currently three functions which write L4 pagetables for Xen, but
they all behave subtly differently. sh_install_xen_entries_in_l4() in
particular is catering for two different usecases, which makes the safety of
the linear mappings hard to follow.
By consolidating the L4 pagetable writing in a single function, the resulting
setup of Xen's virtual layout is easier to understand.
No practical changes to the resulting L4, although the logic has been
rearranged to avoid rewriting some slots. This changes the zap_ro_mpt
parameter to simply ro_mpt.
Both {hap,sh}_install_xen_entries_in_l4() get folded into their callers. The
hap side only a single caller, while the shadow side has two. The shadow
split helps highlight the correctness of the linear slots.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Tim Deegan <tim@xen.org> Release-acked-by: Julien Grall <julien.gral@linaro.org> Acked-by: George Dunlap <george.dunlap@citrix.com>
Andrew Cooper [Tue, 29 Aug 2017 10:35:31 +0000 (11:35 +0100)]
x86/mm: Consolidate all Xen L2 slot writing into init_xen_pae_l2_slots()
Having all of this logic together makes it easier to follow Xen's virtual
setup across the whole system.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Tim Deegan <tim@xen.org> Release-acked-by: Julien Grall <julien.gral@linaro.org> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
The following patches (post XSA-243 fixes) requires init_guest_l4_table()
being common code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tim Deegan <tim@xen.org> Release-acked-by: Julien Grall <julien.gral@linaro.org>
Ian Jackson [Tue, 17 Oct 2017 16:52:02 +0000 (17:52 +0100)]
tools: libxendevicemodel: Restore symbol versions for 1.0
In 1462f9ea8f4219d520a530787b80c986e050aa98
"tools: libxendevicemodel: Provide xendevicemodel_shutdown"
we added a new version 1.1 to the symbol map and simply abolished
the old one. That is quite wrong.
Instead, we should have left the 1.0 map alone and added a new version
which simply adds the new symbol.
Fix this.
Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Roger Pau Monné [Tue, 17 Oct 2017 10:23:53 +0000 (11:23 +0100)]
mm/shadow: fix declaration of fetch_type_names
fetch_type_names usage is guarded by SHADOW_DEBUG_PROPAGATE in
SHADOW_DEBUG, fix the declaration so it's also guarded by
SHADOW_DEBUG_PROPAGATE instead of DEBUG_TRACE_DUMP.
Observed while building with clang and ubsan enabled.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Tim Deegan <tim@xen.org> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Andrew Cooper [Mon, 16 Oct 2017 13:20:00 +0000 (13:20 +0000)]
xen/dom0: Fix latent dom0 construction bugs on all architectures
* x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
all state is actually set up. As it currently stands, d0v0 is eligible for
scheduling before its registers have been set. This is latent as we also
hold a systemcontroller pause reference at the time which prevents d0 from
being scheduled.
* x86 PVH previously was not setting v->is_initialised for d0v0, despite
setting the vcpu running eventually. Therefore, a later VCPUOP_initialise
hypercall will modify state under the feet of the running vcpu. This is
latent as PVH dom0 construction don't yet function.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
While I can't seem to find any users of this hypercall (being a likely
explanation of why the problem wasn't noticed so far), just like for
do_mmu_update() paged-out and shared page handling is needed here. Move
all this logic into mod_l1_entry(), which then also results in no
longer
- doing any of this handling for non-present PTEs,
- acquiring two temporary page references when one is already more than
enough.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Jan Beulich [Fri, 13 Oct 2017 10:42:43 +0000 (12:42 +0200)]
x86: request page table page-in for the correct domain
The domain passed to p2m_mem_paging_populate() should match the one
passed to the corresponding get_page_from_gfn().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Ian Jackson [Fri, 13 Oct 2017 10:21:57 +0000 (11:21 +0100)]
libxl: dm_restrict: DEFINE_USERLOOKUP_HELPER returned a pointer to an auto
When I converted the previous open-coded user lookup functionality
into DEFINE_USERLOOKUP_HELPER, I moved the struct passwd buffer into
the function generated by the macro. This is wrong because that
buffer is used by get{pw,gr}* for its return value, so the helper
function would contrive to return a pointer to the buffer on its own
stack.
Fix this by adding a buffer parameter to the generated helpers, that
the caller must supply, and updating all the call sites.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
Andrew Cooper [Thu, 12 Oct 2017 12:50:31 +0000 (14:50 +0200)]
x86/cpu: Fix IST handling during PCPU bringup
Clear IST references in newly allocated IDTs. Nothing good will come of
having them set before the TSS is suitably constructed (although the chances
of the CPU surviving such an IST interrupt/exception is extremely slim).
Uniformly set the IST references after the TSS is in place. This fixes an
issue on AMD hardware, where onlining a PCPU while PCPU0 is in HVM context
will cause IST_NONE to be copied into the new IDT, making that PCPU vulnerable
to privilege escalation from PV guests until it subsequently schedules an HVM
guest.
This is XSA-244.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Thu, 12 Oct 2017 12:50:07 +0000 (14:50 +0200)]
x86/shadow: Don't create self-linear shadow mappings for 4-level translated guests
When initially creating a monitor table for 4-level translated guests, don't
install a shadow-linear mapping. This mapping is actually self-linear, and
trips up the writeable heuristic logic into following Xen's mappings, not the
guests' shadows it was expecting to follow.
A consequence of this is that sh_guess_wrmap() needs to cope with there being
no shadow-linear mapping present, which in practice occurs once each time a
vcpu switches to 4-level paging from a different paging mode.
An appropriate shadow-linear slot will be inserted into the monitor table
either while constructing lower level monitor tables, or by sh_update_cr3().
While fixing this, clarify the safety of the other mappings. Despite
appearing unsafe, it is correct to create a guest-linear mapping for
translated domains; this is self-linear and doesn't point into the translated
domain. Drop a dead clause for translate != external guests.
This is XSA-243.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Tim Deegan <tim@xen.org>
Jan Beulich [Wed, 27 Sep 2017 10:00:56 +0000 (11:00 +0100)]
x86: don't allow page_unlock() to drop the last type reference
Only _put_page_type() does the necessary cleanup, and hence not all
domain pages can be released during guest cleanup (leaving around
zombie domains) if we get this wrong.
Jan Beulich [Thu, 12 Oct 2017 12:48:25 +0000 (14:48 +0200)]
x86: don't store possibly stale TLB flush time stamp
While the timing window is extremely narrow, it is theoretically
possible for an update to the TLB flush clock and a subsequent flush
IPI to happen between the read and write parts of the update of the
per-page stamp. Exclude this possibility by disabling interrupts
across the update, preventing the IPI to be serviced in the middle.
This is XSA-241.
Reported-by: Jann Horn <jannh@google.com> Suggested-by: George Dunlap <george.dunlap@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Wed, 27 Sep 2017 10:46:52 +0000 (11:46 +0100)]
x86: limit linear page table use to a single level
That's the only way that they're meant to be used. Without such a
restriction arbitrarily long chains of same-level page tables can be
built, tearing down of which may then cause arbitrarily deep recursion,
causing a stack overflow. To facilitate this restriction, a counter is
being introduced to track both the number of same-level entries in a
page table as well as the number of uses of a page table in another
same-level one (counting into positive and negative direction
respectively, utilizing the fact that both counts can't be non-zero at
the same time).
Note that the added accounting introduces a restriction on the number
of times a page can be used in other same-level page tables - more than
32k of such uses are no longer possible.
Note also that some put_page_and_type[_preemptible]() calls are
replaced with open-coded equivalents. This seemed preferrable to
adding "parent_table" to the matrix of functions.
Note further that cross-domain same-level page table references are no
longer permitted (they probably never should have been).
This is XSA-240.
Reported-by: Jann Horn <jannh@google.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Thu, 12 Oct 2017 12:43:26 +0000 (14:43 +0200)]
x86/HVM: prefill partially used variable on emulation paths
Certain handlers ignore the access size (vioapic_write() being the
example this was found with), perhaps leading to subsequent reads
seeing data that wasn't actually written by the guest. For
consistency and extra safety also do this on the read path of
hvm_process_io_intercept(), even if this doesn't directly affect what
guests get to see, as we've supposedly already dealt with read handlers
leaving data completely unitialized.
This is XSA-239.
Reported-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Misbehaving device model can pass incorrect XEN_DMOP_map/
unmap_io_range_to_ioreq_server arguments, namely end < start when
specifying address range. When this happens we hit ASSERT(s <= e) in
rangeset_contains_range()/rangeset_overlaps_range() with debug builds.
Production builds will not trap right away but may misbehave later
while handling such bogus ranges.
This is XSA-238.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Thu, 12 Oct 2017 12:37:56 +0000 (14:37 +0200)]
x86/FLASK: fix unmap-domain-IRQ XSM hook
The caller and the FLASK implementation of xsm_unmap_domain_irq()
disagreed about what the "data" argument points to in the MSI case:
Change both sides to pass/take a PCI device.
This is part of XSA-237.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Thu, 12 Oct 2017 12:36:58 +0000 (14:36 +0200)]
x86/MSI: disallow redundant enabling
At the moment, Xen attempts to allow redundant enabling of MSI by
having pci_enable_msi() return 0, and point to the existing MSI
descriptor, when the msi already exists.
Unfortunately, if subsequent errors are encountered, the cleanup
paths assume pci_enable_msi() had done full initialization, and
hence undo everything that was assumed to be done by that
function without also undoing other setup that would normally
occur only after that function was called (in map_domain_pirq()
itself).
Rather than try to make the redundant enabling case work properly, just
forbid it entirely by having pci_enable_msi() return -EEXIST when MSI
is already set up.
This is part of XSA-237.
Reported-by: HW42 <hw42@ipsumj.de> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Thu, 12 Oct 2017 12:36:30 +0000 (14:36 +0200)]
x86: enforce proper privilege when (un)mapping pIRQ-s
(Un)mapping of IRQs, just like other RESOURCE__ADD* / RESOURCE__REMOVE*
actions (in FLASK terms) should be XSM_DM_PRIV rather than XSM_TARGET.
This in turn requires bypassing the XSM check in physdev_unmap_pirq()
for the HVM emuirq case just like is being done in physdev_map_pirq().
The primary goal security wise, however, is to no longer allow HVM
guests, by specifying their own domain ID instead of DOMID_SELF, to
enter code paths intended for PV guest and the control domains of HVM
guests only.
This is part of XSA-237.
Reported-by: HW42 <hw42@ipsumj.de> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Jan Beulich [Thu, 12 Oct 2017 12:35:14 +0000 (14:35 +0200)]
x86: don't allow MSI pIRQ mapping on unowned device
MSI setup should be permitted only for existing devices owned by the
respective guest (the operation may still be carried out by the domain
controlling that guest).
This is part of XSA-237.
Reported-by: HW42 <hw42@ipsumj.de> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Ian Jackson [Thu, 12 Oct 2017 11:13:48 +0000 (12:13 +0100)]
libxl: dm_restrict: Move to domain_build_info
Right now, this is broken because libxl__build_device_model_args_new
is used also for the qemu run for pv guests for qdisk devices, pvfb,
etc.
We can either make this option properly HVM-specific, or make it
generic.
In principle it is a reasonable request, to make the PV qemu
deprivileged (even though it is not likely to be implemented any time
soon). So make this option generic.
We retain the name "device model" even though it is arguably
inaccurate, because the xl docs already say, for example
For a PV guest a device-model is sometimes used to provide backends
for certain PV devices
The documentation patch here is pure code motion. For ease of review
we will fix up the docs, so the wording to be right for the new
context, in the next patch.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Reported-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v2: Change xl too, to avoid breaking the build.
Fix manpage pod syntax.
George Dunlap [Wed, 11 Oct 2017 17:49:42 +0000 (18:49 +0100)]
fuzz/x86_emulate: Take multiple test files for inputs
Finding aggregate coverage for a set of test files means running each
afl-generated test case through the harness. At the moment, this is
done by re-executing afl-harness-cov with each input file. When a
large number of test cases have been generated, this can take a
significant amonut of time; a recent test with 30k total files
generated by 4 parallel fuzzers took over 7 minutes.
The vast majority of this time is taken up with 'exec', however.
Since the harness is already designed to loop over multiple inputs for
llvm "persistent mode", just allow it to take a large number of inputs
on the same when *not* running in llvm "persistent mode".. Then the
command can be efficiently executed like this:
ls */queue/id* | xargs $path/afl-harness-cov
For the above-mentioned test on 30k files, the time to generate
coverage data was reduced from 7 minutes to under 30 seconds.
Signed-off-by: George Dunlap <george.dunlap@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
George Dunlap [Wed, 11 Oct 2017 17:49:40 +0000 (18:49 +0100)]
fuzz/x86_emulate: Rename the file containing the wrapper code
When generating coverage output, by default gcov generates output
filenames based only on the coverage file and the "leaf" source file,
not the full path. As a result, it uses the same name for
x86_emulate.c and x86_emulate/x86_emulate.c, generally overwriting the
second (which we actually are about) with the first (which is just a
wrapper).
Rename the user-space wrapper helpers to x86-emulate.[ch], so
that it generates separate files.
There is actually an option to gcov, `--preserve-paths`, which will
cause the full path name to be included in the filename, properly
distinguishing between the two. However, given that the user-space
wrapper doesn't actually do any emulation (and the poor state of gcov
documentation making it difficult to find the option in the first
place), it seems to make more sense to rename the file anyway.
Signed-off-by: George Dunlap <george.dunlap@citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
George Dunlap [Wed, 11 Oct 2017 17:49:39 +0000 (18:49 +0100)]
fuzz/x86_emulate: Implement input_read() and input_avail()
Rather than open-coding the "read" from the input file.
Signed-off-by: George Dunlap <george.dunlap@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
George Dunlap [Wed, 11 Oct 2017 17:49:37 +0000 (18:49 +0100)]
fuzz/x86_emulate: Clear errors in the officially sanctioned way
Commit 849a1f10c9 was checked in inappropriately; review flagged up
that clearerr() was too big a hammer, as it would clear both the EOF
flag and stream errors.
Stream errors shouldn't be cleared; we only want the EOF and other
stream-related state reset. To do this, it is sufficient to fseek()
to zero.
Signed-off-by: George Dunlap <george.dunlap@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
public: add and enable XENFEAT_ARM_SMCCC_supported feature
This feature indicates that hypervisor is compatible with ARM
SMC calling convention. Previously hypervisor would inject an
undefined instruction exception if an invalid SMC function were
called or would crash a domain if an invalid HVC function
were invoked.
XENFEAT_ARM_SMCCC_supported feature means that it safe to invoke
SMC/HVC calls that are compatible with SMC calling convention.
arm: PSCI: use definitions provided by asm/smccc.h
smccc.h provides definitions to construct SMC call function number according
to SMCCC. We don't need multiple definitions for one thing, and definitions
in smccc.h are more generic than ones used in psci.h.
So psci.h will only provide function codes, while whole SMC function
identifier will be constructed using generic macros from smccc.h.
Function psci_mode_check() in vsmc.c will be removed in a next patch,
so there are no need to review it. I had to rework it, because
PSCI_0_2_64BIT definition is dropped now.
PSCI is part of HVC/SMC interface, so it should be handled in
appropriate place: `vsmc.c`. This patch moves PSCI handler
calls from `traps.c` to `vsmc.c`. Also it corrects coding
style of the PSCI handler functions.
Older PSCI 0.1 uses SMC function identifiers in range that is
reserved for existing APIs (ARM DEN 0028B, page 16), while newer
PSCI 0.2 and later is defined as "standard secure service" with its
own ranges (ARM DEN 0028B, page 18).
SMCCC (SMC Call Convention) describes how to handle both HVCs and SMCs.
SMCCC states that both HVC and SMC are valid conduits to call to different
firmware functions. Thus, for example, PSCI calls can be made both by
SMC or HVC. Also SMCCC defines function number coding for such calls.
Besides functional calls there are query calls, which allows underling
OS determine version, UUID and number of functions provided by service
provider.
This patch adds new file `vsmc.c`, which handles both generic SMCs
and HVC according to SMCCC. At this moment it implements only one
service: Standard Hypervisor Service.
At this time Standard Hypervisor Service only supports query calls,
so caller can ask about hypervisor UID and determine that it is XEN running.
This change allows more generic handling for SMCs and HVCs and it can
be easily extended to support new services and functions.
But, before SMC is forwarded to standard SMCCC handler, it can be routed
to a domain monitor, if one is installed.
Add generic definitions used in ARM SMC call convention.
Those definitions was originally added to Linux kernel as
include/linux/arm-smccc.h by commit 98dd64f34f47
("ARM: 8478/2: arm/arm64: add arm-smccc")
I extended them and formatted according to XEN coding style. Some
of the macros were converted to inlined functions to ease parsing.
They can be used by both SMCCC clients (like PSCI) and by SMCCC
servers (like vPSCI or upcoming generic SMCCC handler).
arm: processor.h: add definition for immediate value mask
This patch defines HSR_XXC_IMM_MASK. It can be used to extract
immediate value for trapped HVC32, HVC64, SMC64, SVC32, SVC64
instructions, as described in the ARM ARM
(ARM DDI 0487B.a pages D7-2270, D7-2272).
NB: We define a new structure here rather than re-using EFI_GUID.
EFI_GUID uses a Microsoft-style encoding which, among other things,
mixes little-endian and big-endian. The structure defined in this
patch, unlike EFI_GUID, is compatible with the Linux kernel and libuuid.
arm: traps: use generic register accessors in the PSCI code
There are standard functions set_user_reg() and get_user_reg(). We can
use them in PSCI_SET_RESULT()/PSCI_ARG() macros instead of relying on
CONFIG_ARM_64 definition.
Julien Grall [Wed, 11 Oct 2017 14:29:02 +0000 (15:29 +0100)]
xen/arm: guest_walk: Fix get_ipa_output_size
The function get_ipa_output_size checks whether the input size
configured by the guest is valid and will return it.
The check is done with the IPS already shifted against
TCR_EL1_IPS_48_BIT. However the constant has been defined with the
shift included, as a result the check is always false.
Fix it by doing the check on the non-shifted value.
This was introduced by commit 7d623b358a "arm/mem_access: Add long-descriptor
based gpt" introduced software page-table walk for stage-1.
Note that the IPS code is now surrounded with #ifdef CONFIG_ARM_64
because the Arm32 compiler will complain of shift bigger than the width
of the variable. This is fine as the code is executed for 64-bit domain only.
Julien Grall [Wed, 11 Oct 2017 14:15:33 +0000 (15:15 +0100)]
xen/arm: mm: Rework MAIR* definitions to handle 32-bit compilation environment
Commit a0543df403 "xen/arm: page: Clean-up the definition of MAIRVAL"
combined the definition of MAIR0VAL and MAIR1VAL in MAIRVAL. Sadly, when
building in 32-bit environment, the assembler is unable to compute
64-bit constant and will ignore the 32-bit most-significants bits. This
will result of MAIR1 set 0.
Rather than fully reverting the offending commit, the code is reworked
to still avoid hardcoded values but split the definition in 2.
Lastly, a comment is added to avoid trying to blindly combine the both
definition again in the future.
Andrew Cooper [Tue, 10 Oct 2017 20:03:06 +0000 (21:03 +0100)]
common/gnttab: Improve logging message by including relevent domid
Several logging messages cite "bad ref %#x", without identifying which domain
the ref belongs to. Add a domain back-pointer to struct grant_table to
improve the debugability.
While editing the messages, clean up some others:
* Remove extranious punctuation
* Use d%d rather than Dom%d
* Remove "gnttab_transfer:" prefixes, as it is included by the gdprintk()
* Reflow several messages to not be split across multiple lines
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Julien Grall <julien.grall@linaro.org>
Yi Sun [Wed, 11 Oct 2017 12:51:45 +0000 (14:51 +0200)]
x86: psr: support co-exist features' values setting
The whole value array is transferred into 'do_write_psr_msrs'. Then, we can
write all features values on the cos id into MSRs.
Because multiple features may co-exist, we need handle all features to write
values of them into a COS register with new COS ID. E.g:
1. L3 CAT and L2 CAT co-exist.
2. Dom1 and Dom2 share the same COS ID (2). The L3 CAT CBM of Dom1 is 0x1ff,
the L2 CAT CBM of Dom1 is 0x1f.
3. User wants to change L2 CBM of Dom1 to be 0xf. Because COS ID 2 is
used by Dom2 too, we have to pick a new COS ID 3. The values of Dom1 on
COS ID 3 are all default values as below:
---------
| COS 3 |
---------
L3 CAT | 0x7ff |
---------
L2 CAT | 0xff |
---------
4. After setting, the L3 CAT CBM value of Dom1 should be kept and the new L2
CAT CBM is set. So, the values on COS ID 3 should be below.
---------
| COS 3 |
---------
L3 CAT | 0x1ff |
---------
L2 CAT | 0xf |
---------
Note that the original -ENOSPC return, which is being transformed into
an ASSERT(), could have been an ASSERT() from the beginning.
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich [Wed, 11 Oct 2017 12:50:33 +0000 (14:50 +0200)]
x86emul: handle address wrapping for VMASKMOVP{S,D}
I failed to recognize the need to mirror the changes done by 7869e2bafe
("x86emul/fuzz: add rudimentary limit checking") into the earlier
written but later committed 2fe43d333f ("x86emul: support remaining AVX
insns"): Behavior here is the same as for multi-part reads or writes.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Boris Ostrovsky [Wed, 11 Oct 2017 12:49:55 +0000 (14:49 +0200)]
x86/vmx: remove unnecessary is_hvm_domain() test in construct_vmcs()
It's a leftover from PVHv1 days.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
Andrew Cooper [Wed, 11 Oct 2017 12:48:50 +0000 (14:48 +0200)]
x86/hvm: implement hvmemul_write() using real mappings
An access which crosses a page boundary is performed atomically by x86
hardware, albeit with a severe performance penalty. An important corner case
is when a straddled access hits two pages which differ in whether a
translation exists, or in net access rights.
The use of hvm_copy*() in hvmemul_write() is problematic, because it performs
a translation then completes the partial write, before moving onto the next
translation.
If an individual emulated write straddles two pages, the first of which is
writable, and the second of which is not, the first half of the write will
complete before #PF is raised from the second half.
This results in guest state corruption as a side effect of emulation, which
has been observed to cause windows to crash while under introspection.
Introduce the hvmemul_{,un}map_linear_addr() helpers, which translate an
entire contents of a linear access, and vmap() the underlying frames to
provide a contiguous virtual mapping for the emulator to use. This is the
same mechanism as used by the shadow emulation code.
This will catch any translation issues and abort the emulation before any
modifications occur.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Ian Jackson [Fri, 6 Oct 2017 14:30:25 +0000 (15:30 +0100)]
xl: Document VGA problems arising from lack of physmap dmop
Ross reports that stdvga guests do not work, and cirrus guests are
slow, because qemu tries to do xc_domain_add_to_physmap. We will need
another dmop to fix this properly.
For now, document the problem.
(In the cirrus case, the vram remains mapped at the old guest-physical
addresses, while the guest runs. We are not sure whether this is a
correctness or security problem and we should advise against it.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com> CC: Ross Lagerwall <ross.lagerwall@citrix.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Paul Durrant <Paul.Durrant@citrix.com> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
Ian Jackson [Thu, 14 Sep 2017 17:12:57 +0000 (18:12 +0100)]
tools: xentoolcore_restrict_all: use domid_t
This necessitates adding $(CFLAGS_xeninclude) to all the depending
libraries (which can be done via Rules.mk), so that the definition of
domid_t (in xen.h) can be found.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
Ian Jackson [Fri, 15 Sep 2017 15:55:54 +0000 (16:55 +0100)]
libxl: Rationalise calculation of user to run qemu as
If the config specifies a user we use that. Otherwise:
When we are not restricting qemu, there is very little point running
it as a different user than root. Indeed, previously, creating the
"magic" users would cause qemu to become slightly dysfunctional (for
example, you can't insert a cd that the qemu user can't read).
So, in that case, default to running it as root.
Conversely, if restriction is requested, we must insist on running
qemu as a non-root user.
Sadly the admin is still required to create 2^16-epsilon users!
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
Ian Jackson [Fri, 15 Sep 2017 12:42:38 +0000 (13:42 +0100)]
tools/xenstore: get_handle: use "goto err" error handling style
Replace the ad-hoc exit clauses with the error handling style where
- local variables contain either things to be freed, or sentinels
- all error exits go via an "err" label which frees everything
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
Ian Jackson [Thu, 14 Sep 2017 16:51:08 +0000 (17:51 +0100)]
xentoolcore, _restrict_all: Introduce new library and implementation
In practice, qemu opens a great many fds. Tracking them all down and
playing whack-a-mole is unattractive. It is also potentially fragile
in that future changes might accidentally undo our efforts.
Instead, we are going to teach all the Xen libraries how to register
their fds so that they can be neutered with one qemu call.
Right now, nothing will go wrong if some tries to link without
-ltoolcore, but that will stop working as soon as the first other Xen
library starts to register. So this patch will be followed by the
stubdom build update, and should be followed by a
MINIOS_UPSTREAM_REVISION updated.
Sadly qemu upstream's configuration arrangements are too crude, being
keyed solely off the Xen version number. So they cannot provide
forward/backward build compatibility across changes in xen-unstable,
like this one. qemu patches to link against xentoolcore should be
applied in qemu upstream so avoid the qemu build breaking against the
released version of Xen 4.10.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Wei Liu <wei.liu2@citrix.com>
Ian Jackson [Mon, 18 Sep 2017 13:55:45 +0000 (14:55 +0100)]
xen: x86 dm_op: add missing newline before XEN_DMOP_inject_msi
Coding style only; no functional change.
CC: Jan Beulich <jbeulich@suse.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Jan Beulich <JBeulich@suse.com>
Ian Jackson [Fri, 15 Sep 2017 16:16:37 +0000 (17:16 +0100)]
xen: Provide XEN_DMOP_remote_shutdown
SCHEDOP_remote_shutdown should be a DMOP so that a deprivileged qemu
can do the propery tidying up.
We need to keep SCHEDOP_remote_shutdown for ABI stability reasons and
because it is needed for PV guests.
CC: Jan Beulich <jbeulich@suse.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: George Dunlap <George.Dunlap@eu.citrix.com> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Tim Deegan <tim@xen.org> CC: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Meng Xu [Tue, 10 Oct 2017 23:17:41 +0000 (19:17 -0400)]
xen:rtds: towards work conserving RTDS
Make RTDS scheduler work conserving without breaking the real-time guarantees.
VCPU model:
Each real-time VCPU is extended to have an extratime flag
and a priority_level field.
When a VCPU's budget is depleted in the current period,
if it has extratime flag set,
its priority_level will increase by 1 and its budget will be refilled;
othewrise, the VCPU will be moved to the depletedq.
Scheduling policy is modified global EDF:
A VCPU v1 has higher priority than another VCPU v2 if
(i) v1 has smaller priority_leve; or
(ii) v1 has the same priority_level but has a smaller deadline
Queue management:
Run queue holds VCPUs with extratime flag set and VCPUs with
remaining budget. Run queue is sorted in increasing order of VCPUs priorities.
Depleted queue holds VCPUs which have extratime flag cleared and depleted budget.
Replenished queue is not modified.
Distribution of spare bandwidth
Spare bandwidth is distributed among all VCPUs with extratime flag set,
proportional to these VCPUs utilizations