Let's give the compiler as much liberty at picking instruction operands
as possible. Also drop unnecessary size modifiers when the correct size
can already be derived from the asm() operands. Finally also drop an
"unsigned" from idiv_dbl()'s second parameter, allowing a cast to be
eliminated.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
AAM/AAD as well as DAA/DAS emulation code is respectively almost
identical. Fold each pair, following what's already the case for
AAA/AAS.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
In order to reduce the risk of unintentionally adding a function
pointer to just one of the two tables, merge them into one, with each
entry pair getting generated by a single macro invocation (at once
dropping all explicit casting outside the macro definition).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
... to prevent guest user mode arranging for a guest crash (due to
failed VM entry). (On the AMD system I checked, hardware is doing
exactly the canonicalization being added here.)
Note that fixing this in an architecturally correct way would be quite
a bit more involved: Making the x86 instruction emulator check all
branch targets for validity, plus dealing with invalid rIP resulting
from update_guest_eip() or incoming directly during a VM exit. The only
way to get the latter right would be by not having hardware do the
injection.
Note further that there are a two early returns from
vmx_vmexit_handler(): One (through vmx_failed_vmentry()) leads to
domain_crash() anyway, and the other covers real mode only and can
neither occur with a non-canonical rIP nor result in an altered rIP,
so we don't need to force those paths through the checking logic.
This is CVE-2016-2271 / XSA-170.
Reported-by: 刘令 <liuling-it@360.cn>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
We've been told by Intel that inconsistent cachability between
multiple mappings of the same page can affect system stability only
when the affected page is an MMIO one. Since the stale data issue is
of no relevance to the hypervisor (since all guest memory accesses go
through proper accessors and validation), handling of RAM pages
remains unchanged here. Any MMIO mapped by domains however needs to be
done consistently (all cachable mappings or all uncachable ones), in
order to avoid Machine Check exceptions. Since converting existing
cachable mappings to uncachable (at the time an uncachable mapping
gets established) would in the PV case require tracking all mappings,
allow MMIO to only get mapped uncachable (UC, UC-, or WC).
This also implies that in the PV case we mustn't use the L1 PTE update
fast path when cachability flags get altered.
Since in the HVM case at least for now we want to continue honoring
pinned cachability attributes for pages not mapped by the hypervisor,
special case handling of r/o MMIO pages (forcing UC) gets added there.
Arguably the counterpart change to p2m-pt.c may not be necessary, since
UC- (which already gets enforced there) is probably strict enough.
Note that the shadow code changes include fixing the write protection
of r/o MMIO ranges: shadow_l1e_remove_flags() and its siblings, other
than l1e_remove_flags() and alike, return the new PTE (and hence
ignoring their return values makes them no-ops).
This is CVE-2016-2270 / XSA-154.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This patch moves monitor_domctl to common-side.
Purpose: move what's common to common, prepare for implementation
of such vm-events on ARM.
* move get_capabilities to arch-side => arch_monitor_get_capabilities.
* add arch-side monitor op handling function => arch_monitor_domctl_op.
e.g. X86-side handles XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP op
* add arch-side monitor event handling function => arch_monitor_domctl_event.
e.g. X86-side handles XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR event enable/disable
* remove status_check
Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Coverity points out that these remain open in the parent upon
success, which is a resource leak.
To fix this rejig the exit paths such that success and error cases
both close the two fds, this means adjusting the callback to only
happen for the error case and it also makes sense to rename the label
from "error" to just "out".
Compile tested only.
CID: 1130518 (null) and 1130517 (logfile_w).
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Coverity (correctly) complains that the strncpy(p, "0x", 2) will not
null terminate p.
Although we can see that in the rest of the function p will
definitely be NULL terminated by the time it is complete there is no
harm in passing 3 to the strncpy and allowing it to NULL terminate to
placate Coverity. We know this is safe because the allocation to hold
the string includes a "+3" for the 0x and the terminating NULL.
Compile tested only.
CID: 1198708
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
The underlying code paths in tap_ctl_create attempt to handle both
*devpath == NULL (by allocating) and *devpath != NULL (caller provided
name) and if they allocate tend to write the return immediately before
doing other potentially error generating tasks. All of which makes
handling this at a lower level rather more complicated than handling
it in the error path of libxl__blktap_devpath.
Note that libxl__blktap_devpath initialises devpath to NULL and if the
earlier GCSPRINTF succeeds then the value is returned earlier.
Therefore if we make it to the call to tap_ctl_create then devpath is
still NULL on entry, therefore on the error path devpath is either
still NULL or has been set to a freshly allocated value by
tap_ctl_create. Since free(NULL) is fine it is sufficient to just
free(devpath).
I also considered adding a non-NULL devnull to the gc, even on
failure, but that would have required a comment to explain the
apparently strange behaviour.
Compile tested only.
CID: 1215177
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Previously in the three problematic libraries all public headers were
linked to source code directory. We should have created an include
directory for each library and linked public headers there.
Note that there was no breakage for those three libraries before this
patch. This patch merely changes the location headers are linked to so
that all libraries follow the same pattern.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
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>
... and hence the respective {read,write}_msr() hook parameter doesn't
need to be "unsigned long".
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@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>
This patch merges almost identical functions hvm_event_int3 and
hvm_event_single_step into a single function called hvm_event_breakpoint.
Also fixes event.c file header comment in the process.
Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
xc_mem_access_enable_emulate() and xc_mem_access_disable_emulate()
are currently no-ops, that is all they do is set a flag that
nobody else checks. The user can already set the EMULATE flags in
the vm_event response if emulation is desired, and having an extra
check above that is not inherently safer, but it does complicate
(currenly unnecessarily) the API. This patch removes these
functions and the corresponding hypervisor code.
Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.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 any code that tries to use Elf_* macros would
require us to use Elf64_* types instead of the more
friendly Elf_ one.
This is OK to do since 32-bit ARM uses LPAE mode.
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
This is just suppose to do a simple compile test on Travis CI. Currently
due to linux86 (bcc/bin86/dev86) not being whitelisted the tools cannot
be built.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
c/s 361b4f9f0f
"build: save generated xen .config" forgot to remove
the config file when uninstalling.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-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>
QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
log") renamed the "stderr" trace backend to "log", which breaks the
xen build when pointed at a QEMU tree after that point:
./configure of QEMU fail with:
"ERROR: invalid trace backends
Please choose supported trace backends."
Upstream also changed, in baf86d6b3ca0 ("trace: switch default backend
to "log""), to use "log" as the default backend (previously it was
"nop").
Use ./scripts/tracetool.py to check for the presence of the stderr
backend and if it is present then explicitly enable it. If the stderr
backend is not present then assume a newer QEMU which defaults to
"log" and simply accept that default (there is a 1 commit window
upstream where this would result in no trace backend being enabled).
The check is done using the older (deprecated?) --check-backend/--backend
variant of the tracetool.py options rather than the new plural
versions since the singular was supported even by very old versions of
QEMU. New QEMU has compatibility code but if/when that is removed we
will still do the right thing i.e. no explict configuiration resulting
in the upstream default (currently "log").
If the explicit selection of the "stderr" backend is required then it
is now done unconditionally (not depending on debug=y), which is
simpler to arrange here but also matches the newer upstream's default
to "log" which is not conditional on debug being enabled either.
Tested with current qemu-xen-unstable (e9d8252) and current QEMU
upstream master (88c73d1), both out of tree via
QEMU_UPSTREAM_URL=/path/to/qemu-xen.git.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.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>
This is both more natural and in line with a Linux change (between 3.14
and 3.15).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
When mapping large BARs (e.g. the frame buffer of a graphics card) the
overhead of establishing such mappings using only 4k pages has,
particularly after the XSA-125 fix, become unacceptable. Alter the
XEN_DOMCTL_memory_mapping semantics once again, so that there's no
longer a fixed amount of guest frames that represents the upper limit
of what a single invocation can map. Instead bound execution time by
limiting the number of iterations (regardless of page size).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.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>
That is, if gc is not NOGC and ptr is not NULL then ptr must be
associated with a gc.
Currently in this case the new_ptr would not be registered with any
gc, which Coverity rightly points out (in various different places)
would be a memory leak.
It would also be possible to fix this by adding a libxl__ptr_add() at
the same point, however semantically it seems like a programming error
to gc-realloc a pointer which is not associated with the gc in
question, so treat it as such.
Compile tested only, this change could expose latent bugs.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Previously xs_fd would be left open, which is CID 1055993 (previously
partially fixed by 3bca826aae).
Instead arrange for both success and error cases to cleanup everything
on a single exit path instead of doing partial cleanup on the success
path a few operations higher up.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
This is just used to expand the shared backing file to the expected
size (whether this is actually necessary I'm not sure). Rather than
leaking some small amount of the processes' heap set the array to
zeroes.
While at it add a check that the malloc succeeded before using the
result.
Compile tested only.
CID: 1056095 (use of uninitialised data)
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Otherwise the fd is leaked. NB the mmap'd memory in the global
cpu_qos_data[n] is not affected by closing the underlying fd.
Compile tested only.
CID: 1055930
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.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>
Jan Beulich
Corneliu ZUZU
Ian Campbell
Wei Liu
Stefano Stabellini
Andrew Cooper
Razvan Cojocaru
Konrad Rzeszutek Wilk
Doug Goldstein
Olaf Hering
Harmandeep Kaur
Paul Durrant