Ian Jackson [Thu, 6 Feb 2014 19:17:26 +0000 (19:17 +0000)]
libxl: test programs: Fix make race re libxenlight.so
The test programs were getting the proper libxenlight.so on their link
line. Filter it out. Also change the soname of the test library to
match the real one, so that libxutil is satisfied with it.
Reported-by: Olaf Hering <olaf@aepfle.de> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Olaf Hering <olaf@aepfle.de> Cc: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Thu, 6 Feb 2014 18:41:24 +0000 (18:41 +0000)]
libxl: test programs: Fix Makefile race re headers
We need to include the new TEST_PROG_OBJS and LIBXL_TEST_OBJS in the
appropriate dependencies. Otherwise we risk trying to build the test
program before gentypes is run.
Reported-by: Olaf Hering <olaf@aepfle.de> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Olaf Hering <olaf@aepfle.de> Cc: Ian Campbell <Ian.Campbell@citrix.com>
libvchan: Fix handling of invalid ring buffer indices
The remote (hostile) process can set ring buffer indices to any value
at any time. If that happens, it is possible to get "buffer space"
(either for writing data, or ready for reading) negative or greater
than buffer size. This will end up with buffer overflow in the second
memcpy inside of do_send/do_recv.
Fix this by introducing new available bytes accessor functions
raw_get_data_ready and raw_get_buffer_space which are robust against
mad ring states, and only return sanitised values.
Proof sketch of correctness:
Now {rd,wr}_{cons,prod} are only ever used in the raw available bytes
functions, and in do_send and do_recv.
The raw available bytes functions do unsigned arithmetic on the
returned values. If the result is "negative" or too big it will be
>ring_size (since we used unsigned arithmetic). Otherwise the result
is a positive in-range value representing a reasonable ring state, in
which case we can safely convert it to int (as the rest of the code
expects).
do_send and do_recv immediately mask the ring index value with the
ring size. The result is always going to be plausible. If the ring
state has become mad, the worst case is that our behaviour is
inconsistent with the peer's ring pointer. I.e. we read or write to
arguably-incorrect parts of the ring - but always parts of the ring.
And of course if a peer misoperates the ring they can achieve this
effect anyway.
So the security problem is fixed.
This is XSA-86.
(The patch is essentially Ian Jackson's work, although parts of the
commit message are by Marek.)
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Jan Beulich [Thu, 6 Feb 2014 15:33:50 +0000 (16:33 +0100)]
flask: fix reading strings from guest memory
Since the string size is being specified by the guest, we must range
check it properly before doing allocations based on it. While for the
two cases that are exposed only to trusted guests (via policy
restriction) this just uses an arbitrary upper limit (PAGE_SIZE), for
the FLASK_[GS]ETBOOL case (which any guest can use) the upper limit
gets enforced based on the longest name across all boolean settings.
This is XSA-84.
Reported-by: Matthew Daley <mattd@bugfuzz.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Ian Jackson [Fri, 31 Jan 2014 15:07:55 +0000 (15:07 +0000)]
libxl: timeouts: Record deregistration when one occurs
When a timeout has occurred, it is deregistered. However, we failed
to record this fact by updating etime->func. As a result,
libxl__ev_time_isregistered would say `true' for a timeout which has
already happened.
It is necessary to clear etime->func before the callback, because the
callback might want to reinstate the timeout, or might free the etime
(or its containing struct) entirely.
The results are that we might try to have the timeout occur again
(causing problems for the call site), and/or corrupt the timeout list.
This fixes the timedereg event system unit test.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Fri, 31 Jan 2014 15:04:37 +0000 (15:04 +0000)]
libxl: timeouts: Break out time_occurs
Bring together the two places where etime->func() is called into a new
function time_occurs. For one call site this is pure code motion.
For the other the only semantic change is the introduction of a new
debugging message.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Mon, 3 Feb 2014 14:25:13 +0000 (14:25 +0000)]
libxl: events: timedereg internal unit test
Test timeout deregistration idempotency. In the current tree this
test fails because ev->func is not cleared, meaning that a timeout
can be removed from the list more than once, corrupting the list.
It is necessary to use multiple timeouts to demonstrate this bug,
because removing the very same entry twice from a list in quick
succession, without modifying the list in other ways in between,
doesn't actually corrupt the list. (Since removing an entry from a
doubly-linked list just copies next and back from the disappearing
entry into its neighbours.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Mon, 3 Feb 2014 14:17:46 +0000 (14:17 +0000)]
libxl: events: Makefile builds internal unit tests
We provide a new LIBXL_TESTS facility in the Makefile.
Also provide some helpful common routines for unit tests to use.
We don't want to put the weird test case entrypoints and the weird
test case code in the main libxl.so library. Symbol hiding prevents
us from simply directly linking the libxl_test_FOO.o in later. So
instead we provide a special library libxenlight_test.so which is used
only locally.
There are not yet any test cases defined; that will come in the next
patch.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Tue, 21 Jan 2014 15:05:37 +0000 (15:05 +0000)]
libxl: fork: Make SIGCHLD self-pipe nonblocking
Use the new libxl__pipe_nonblock and _close functions, rather than
open coding the same logic. Now the pipe is nonblocking, which avoids
a race which could result in libxl deadlocking in a multithreaded
program.
Reported-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <ian.campbell@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Jackson [Tue, 21 Jan 2014 14:58:10 +0000 (14:58 +0000)]
libxl: events: Break out libxl__pipe_nonblock, _close
Break out the pipe creation and destruction from the poller code
into two new functions libxl__pipe_nonblock and libxl__pipe_close.
Also change direct use of pipe() to libxl_pipe.
No overall functional difference other than minor differences in exact
log messages.
Also move libxl__self_pipe_wakeup and libxl__self_pipe_eatall into the
new pipe utilities section in libxl_event.c; this is pure code motion.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <ian.campbell@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
--
v3: Mention that we switched pipe() -> libxl_pipe()
Ian Jackson [Fri, 17 Jan 2014 11:58:55 +0000 (11:58 +0000)]
libxl: fork: Share SIGCHLD handler amongst ctxs
Previously, an application which had multiple libxl ctxs in multiple
threads, would have to itself plumb SIGCHLD through to each ctx.
Instead, permit multiple libxl ctxs to all share the SIGCHLD handler.
We keep a list of all the ctxs which are interested in SIGCHLD and
notify all of their self-pipes.
In more detail:
* sigchld_owner, the ctx* of the SIGCHLD owner, is replaced by
sigchld_users, a list of SIGCHLD users.
* Each ctx keeps track of whether it is on the users list, so that
libxl__sigchld_needed and libxl__sigchld_notneeded now instead of
idempotently installing and removing the handler, idempotently add
or remove the ctx from the list.
We ensure that we always have the SIGCHLD handler installed
iff the sigchld_users list is nonempty. To make this a bit
easier we make sigchld_installhandler_core and
sigchld_removehandler_core idempotent.
Specifically, the call sites for sigchld_installhandler_core and
sigchld_removehandler_core are updated to manipulate sigchld_users
and only call the install or remove functions as applicable.
* In the signal handler we walk the list of SIGCHLD users and write
to each of their self-pipes. That means that we need to arrange to
defer SIGCHLD when we are manipulating the list (to avoid the
signal handler interrupting our list manipulation); this is quite
tiresome to arrange.
The code as written will, on the first installation of the SIGCHLD
handler, firstly install the real handler, then immediately replace
it with the deferral handler. Doing it this way makes the code
clearer as it makes the SIGCHLD deferral machinery much more
self-contained (and hence easier to reason about).
* The first part of libxl__sigchld_notneeded is broken out into a new
function sigchld_user_remove (which is also needed during for
postfork). And of course that first part of the function is now
rather different, as explained above.
* sigchld_installhandler_core no longer takes the gc argument,
because it now deals with SIGCHLD for all ctxs.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Include bugfixes from "Fixup SIGCHLD sharing" patch:
* Use a mutex for defer_sigchld, to guard against concurrency
between the thread calling defer_sigchld and an instance of the
primary signal handler on another thread.
* libxl_sigchld_owner_libxl_always is incompatible with SIGCHLD
sharing. Document this correctly.
Fix "have have" error in comment.
Move removal of newly unused variables to previous patch.
v2.1: Provide feature test macro LIBXL_HAVE_SIGCHLD_SHARING
Ian Jackson [Fri, 17 Jan 2014 15:42:31 +0000 (15:42 +0000)]
libxl: fork: Break out sigchld_sethandler_raw
We are going to want introduce another call site in the final
substantive patch.
Pure code motion; no functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
---
v3: Remove now-unused variables from sigchld_installhandler_core
Ian Jackson [Fri, 17 Jan 2014 12:01:24 +0000 (12:01 +0000)]
libxl: fork: Break out sigchld_installhandler_core
Pure code motion. This is going to make the final substantive patch
easier to read.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Fri, 17 Jan 2014 11:45:57 +0000 (11:45 +0000)]
libxl: fork: Rename sigchld handler functions
We are going to change these functions so that different libxl ctx's
can share a single SIGCHLD handler. Rename them now to a new name
which doesn't imply unconditional handler installation or removal.
Also note in the comments that they are idempotent.
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Thu, 16 Jan 2014 17:03:34 +0000 (17:03 +0000)]
libxl: fork: Provide LIBXL_HAVE_SIGCHLD_SELECTIVE_REAP
This is the feature test macro for libxl_childproc_sigchld_occurred
and libxl_sigchld_owner_libxl_always_selective_reap.
It is split out into this separate patch because: a single feature
test is sensible because we do not intend anyone to release or ship
libxl versions with one of these but not the other; but, the two
features are in separate patches for clarity; and, this just makes
reading the actual code easier.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Thu, 16 Jan 2014 17:01:50 +0000 (17:01 +0000)]
libxl: fork: Provide ..._always_selective_reap
Applications exist which want to use libxl in an event-driven mode but
which do not integrate child termination into their event system, but
instead reap all their own children synchronously.
In such an application libxl must own SIGCHLD but avoid reaping any
children that don't belong to libxl.
Provide libxl_sigchld_owner_libxl_always_selective_reap which has this
behaviour.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
---
v2: Document the new mode in the big "Subprocess handling" comment.
Ian Jackson [Thu, 16 Jan 2014 16:57:27 +0000 (16:57 +0000)]
libxl: fork: Provide libxl_childproc_sigchld_occurred
Applications exist which don't keep track of all their child processes
in a manner suitable for coherent dispatch of their termination. In
such a situation, nothing in the whole process may call wait, or
waitpid(-1,,). Doing so reaps processes belonging to other parts of
the application and there is then no way to deliver the exit status to
the right place.
To facilitate this, provide a facility for such an application to ask
libxl to call waitpid on each of its children individually.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Jackson [Thu, 16 Jan 2014 17:12:31 +0000 (17:12 +0000)]
libxl: fork: assert that chldmode is right
In libxl_childproc_reaped, check that the chldmode is as expected.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
libxl_sigchld_owner_libxl ought to have been mentioned in the list of
options for chldowner. Since it's the default, move the description
of the its behaviour into the description of that option.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Thu, 16 Jan 2014 16:55:04 +0000 (16:55 +0000)]
libxl: fork: Clarify docs for libxl_sigchld_owner
Clarify that libxl_sigchld_owner_libxl causes libxl to reap all the
process's children, and clarify the wording of the description of
libxl_sigchld_owner_libxl_always.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Jackson [Thu, 16 Jan 2014 16:40:05 +0000 (16:40 +0000)]
libxl: fork: Break out childproc_reaped_ours
We're going to want to do this again at a new call site.
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Ian Jackson [Thu, 16 Jan 2014 16:37:44 +0000 (16:37 +0000)]
libxl: fork: Break out checked_waitpid
This is a simple error-handling wrapper for waitpid. We're going to
want to call waitpid somewhere else and this avoids some of the
duplication.
No functional change in this patch. (Technically, we used to check
chldmode_ours again in the EINTR case, and don't now, but that can't
have changed because we continuously hold the libxl ctx lock.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Jackson [Wed, 5 Feb 2014 14:43:38 +0000 (14:43 +0000)]
tools: Bump library SONAMEs for 4.4
There have been ABI/API changes in libxc. Bump its MAJOR (which
affets libxenguest et al too.)
There have been ABI changes in libxl. Bump its MAJOR.
(The API changes have been dealt with as we go along - there is
already a LIBXL_API_VERSION 0x040400.)
None of the other libraries have changed their interfaces. I have
verified this by building the tools and searching the dist/install
tree for files matching *.so.*. For each library that showed up, I
did this:
git-diff RELEASE-4.3.0..staging -- `find tools/FOO/ -name \*.h`
where FOO is the corresponding source directory.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
The function domain_page_map_to_mfn can be used to translate a virtual
address mapped by both map_domain_page and map_domain_page_global.
The former is using vmap to map the mfn, therefore domain_page_map_to_mfn
will always fail because the address is not in DOMHEAP range.
Check if the address is in vmap range and use __pa to translate it.
This patch fix guest shutdown when the event fifo is used.
Signed-off-by: Julien Grall <julien.grall@linaro.org> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Cc: George Dunlap <george.dunlap@citrix.com>
Since the gic_remove_from_queues() and gic_irq_disable() called from
non interrupt context and they acquire the same lock as gic_set_guest_irq()
which called from interrupt context we must disable interrupts in these
functions to avoid possible deadlocks.
Andrew Cooper [Tue, 4 Feb 2014 18:01:10 +0000 (18:01 +0000)]
tools/libxc: Prevent erroneous success from xc_domain_restore
The variable 'rc' is set to 1 at the top of xc_domain_restore, and for the
most part is left alone until success, at which point it is set to 0.
There is a separate 'frc' which for the most part is used to check function
calls, keeping errors separate from 'rc'.
For a toolstack which sets callbacks->toolstack_restore(), and the function
returns 0, any subsequent error will end up with code flow going to "out;",
resulting in the migration being declared a success.
For consistency, update the callsites of xc_dom_gnttab{,_hvm}_seed() to use
'frc', even though their use of 'rc' is currently safe.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> CC: George Dunlap <george.dunlap@eu.citrix.com>
xen: arm: Remove determining reset specific values from dts for XGENE.
This patch removes reading reset specific values (address, size and mask) from
dts and uses values defined in the code now. This is because currently xgene
reset driver (submitted in linux) is going through a change (which is not yet
accepted), this new driver has a new type of dts bindings for reset. Hence
till linux driver comes to some conclusion, we will use hardcoded values
instead of reading from dts so that xen code will not break due to the linux
transition.
Anthony PERARD [Fri, 31 Jan 2014 16:35:47 +0000 (16:35 +0000)]
libxl: Fix vcpu-set for PV guest.
vcpu-set will try to use the HVM path (through QEMU) instead of the PV
path (through xenstore) for a PV guest, if there is a QEMU running for
this domain. This patch check which kind of guest is running before
before doing any call.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Jan Beulich [Thu, 6 Feb 2014 11:20:48 +0000 (12:20 +0100)]
domctl: pause vCPU for context reads
"Base" context reads already paused the subject vCPU when being the
current one, but that special case isn't being properly dealt with
anyway (at the very least when x86's fsgsbase feature is in use), so
just disallow it.
"Extended" context reads so far didn't do any pausing.
While we can't avoid the reported data being stale by the time it
arrives at the caller, this way we at least guarantee that it is
consistent.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Jan Beulich [Thu, 6 Feb 2014 11:20:20 +0000 (12:20 +0100)]
domctl: also pause domain for "extended" context updates
This is not just for consistency with "base" context updates, but
actually needed so that guest side accesses can't race with control
domain side updates.
This would have been a security issue if XSA-77 hadn't waived them on
the affected domctl operation.
While looking at the code I also spotted a redundant NULL check in the
"base" context update handling code, which is being removed.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Jan Beulich [Thu, 6 Feb 2014 11:19:06 +0000 (12:19 +0100)]
x86: fix FS/GS base handling when using the fsgsbase feature
In that case, due to the respective instructions not being privileged,
we can't rely on our in-memory data to always be correct: While the
guest is running, it may change without us knowing about it. Therefore
we need to
- read the correct values from hardware during context switch out
(save_segments())
- read the correct values from hardware during RDMSR emulation
- update in-memory values during guest mode change
(toggle_guest_mode())
For completeness/consistency, WRMSR emulation is also being switched
to use wr[fg]sbase().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Ian Campbell [Thu, 16 Jan 2014 15:27:59 +0000 (15:27 +0000)]
tools: libxl: do not set the PoD target on ARM
ARM does not implemented PoD and so returns ENOSYS from XENMEM_set_pod_target.
The correct solution here would be to check for ENOSYS in libxl, unfortunately
xc_domain_set_pod_target suffers from the same broken error reporting as the
rest of libxc and throws away the errno.
So for now conditionally define xc_domain_set_pod_target to return success
(which is what PoD does if nothing needs doing). xc_domain_get_pod_target sets
errno==-1 and returns -1, which matches the broken error reporting of the
existing function. It appears to have no in tree callers in any case.
The conditional should be removed once libxc has been fixed.
This makes ballooning (xl mem-set) work for ARM domains.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Julien Grall <julien.grall@linaro.org> Cc: george.dunlap@citrix.com
Ian Campbell [Fri, 24 Jan 2014 14:23:07 +0000 (14:23 +0000)]
xen: arm: correct use of find_next_bit
find_next_bit takes a "const unsigned long *" but forcing a cast of an
"uint32_t *" throws away the alignment constraints and ends up causing an
alignment fault on arm64 if the input happened to be 4 but not 8 byte aligned.
Instead of casting use a temporary variable of the right type.
I've had a look around for similar constructs and the only thing I found was
maintenance_interrupt which cases a uint64_t down to an unsigned long, which
although perhaps not best advised is safe I think.
This was observed with the AArch64 Linaro toolchain 2013.12 but I think that
is just coincidental due to subtle changes to the stack layout etc.
Reported-by: Fu Wei <fu.wei@linaro.org> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Julien Grall <julien.grall@linaro.org>
Wei Liu [Mon, 27 Jan 2014 17:53:38 +0000 (17:53 +0000)]
libxc: fix claim mode when creating HVM guest
The original code is wrong because:
* claim mode wants to know the total number of pages needed while
original code provides the additional number of pages needed.
* if pod is enabled memory will already be allocated by the time we try
to claim memory.
So the fix would be:
* move claim mode before actual memory allocation.
* pass the right number of pages to hypervisor.
The "right number of pages" should be number of pages of target memory
minus VGA_HOLE_SIZE, regardless of whether PoD is enabled.
This fixes bug #32.
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Andrew Cooper [Mon, 27 Jan 2014 16:25:24 +0000 (16:25 +0000)]
tools/libxc: goto correct label on error paths
Both of these "goto finish;" statements are actually errors, and need to "goto
out;" instead, which will correctly destroy the domain and return an error,
rather than trying to finish the migration (and in at least one scenario,
return success).
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> CC: George Dunlap <george.dunlap@eu.citrix.com>
Jan Beulich [Tue, 4 Feb 2014 08:22:12 +0000 (09:22 +0100)]
x86/domctl: don't ignore errors from vmce_restore_vcpu()
What started out as a simple cleanup patch (eliminating the redundant
check of domctl->cmd before setting "copyback", which as a result
turned the "ext_vcpucontext_out" label useless) revealed a bug in the
handling of XEN_DOMCTL_set_ext_vcpucontext.
Fix this, retaining the cleanup, and at once dropping a stale comment
and an accompanying formatting issue.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Tue, 28 Jan 2014 12:31:28 +0000 (13:31 +0100)]
xen/unlz4: always set an error return code on failures
"ret", being set to -1 early on, gets cleared by the first invocation
of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
subsequent failures wouldn't be noticed by the caller without setting
it back to -1 right after those calls.
Andrew Cooper [Fri, 24 Jan 2014 18:28:11 +0000 (18:28 +0000)]
minios: Correct HYPERVISOR_physdev_op()
A physdev_op is a two argument hypercall, taking a command parameter and an
optional pointer to a structure.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Jackson [Tue, 21 Jan 2014 18:45:29 +0000 (18:45 +0000)]
libxl: events: Pass correct nfds to poll
libxl_event.c:eventloop_iteration would pass the allocated pollfds
array size, rather than the used size, to poll (and to
afterpoll_internal).
The effect is that if the number of fds to poll on reduces, libxl will
poll on stale entries. Because of the way the return value from poll
is processed these stale entries are often harmless because any events
coming back from poll ignored by libxl. However, it could cause
malfunctions:
It could result in unwanted SIGTTIN/SIGTTOU/SIGPIPE, for example, if
the fd has been reused to refer to an object which can generate those
signals. Alternatively, it could result in libxl spinning if the
stale entry refers to an fd which happens now to be ready for the
previously-requested operation.
I have tested this with a localhost migration and inspected the strace
output.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Cc: Jim Fehlig <jfehlig@suse.com> Cc: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Fabio Fantoni [Tue, 21 Jan 2014 13:51:08 +0000 (14:51 +0100)]
tools/hotplug: fix bug on xendomains using xl
Make rdname function work with xl.
The rdname function not support json output of xl commands and this cause
problems using xl, for example the check if domUs are already running (because
they have been restored) on domUs autostart does not succeed and the domain is
created in any case, causing xl create to fail.
Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Jan Beulich [Fri, 24 Jan 2014 08:01:21 +0000 (08:01 +0000)]
libxc/unlz4: always set an error return code on failures
"ret", being set to -1 early on, gets cleared by the first invocation
of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
subsequent failures wouldn't be noticed by the caller without setting
it back to -1 right after those calls.
Mike Neilsen [Wed, 22 Jan 2014 17:41:11 +0000 (11:41 -0600)]
mini-os: Fix stubdom build failures on gcc 4.8
This is a fix for bug 35:
http://bugs.xenproject.org/xen/bug/35
This bug report describes several format string mismatches which prevent
building the stubdom target in Xen 4.3 and Xen 4.4-rc2 on gcc 4.8. This is a
copy of Alex Sharp's original patch with the following modifications:
* Andrew Cooper's recommendation applied to extras/mini-os/xenbus/xenbus.c to
avoid stack corruption
* Samuel Thibault's recommendation to make "fun" an unsigned int rather than an
unsigned long in pcifront_physical_to_virtual and related functions
(extras/mini-os/include/pcifront.h and extras/mini-os/pcifront.c)
Tested on x86_64 gcc Ubuntu/Linaro 4.8.1-10ubuntu9.
Coverity-IDs: 1055807105580810558091055810 Signed-off-by: Mike Neilsen <mneilsen@acm.org> Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Jan Beulich [Fri, 24 Jan 2014 09:19:53 +0000 (10:19 +0100)]
Revert "x86/viridian: Time Reference Count MSR"
This mostly reverts commit e36cd2cdc9674a7a4855d21fb7b3e6e17c4bb33b.
hvm_get_guest_time() is not a suitable time source for this MSR, as
is resets across migration.
Jan Beulich [Thu, 23 Jan 2014 09:30:08 +0000 (10:30 +0100)]
x86: don't drop guest visible state updates when 64-bit PV guest is in user mode
Since 64-bit PV uses separate kernel and user mode page tables, kernel
addresses (as usually provided via VCPUOP_register_runstate_memory_area
and possibly via VCPUOP_register_vcpu_time_memory_area) aren't
necessarily accessible when the respective updating occurs. Add logic
for toggle_guest_mode() to take care of this (if necessary) the next
time the vCPU switches to kernel mode.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich [Thu, 23 Jan 2014 09:29:12 +0000 (10:29 +0100)]
unmodified_drivers: make usbfront build conditional
Commit 0dcfb88fb8 ("unmodified_drivers: enable build of usbfront
driver") results in the PV drivers to no longer build against older
(pre-2.6.35) Linux versions. That's because usbfront.h includes
headers from drivers/usb/core/, which is generally unavailable when
building out-of-tree modules.
Yang Zhang [Thu, 23 Jan 2014 09:27:34 +0000 (10:27 +0100)]
Nested VMX: prohibit virtual vmentry/vmexit during IO emulation
Sometimes, L0 needs to decode L2's instruction to handle IO access directly.
And L0 may get X86EMUL_RETRY when handling this IO request. At same time, if
there is a virtual vmexit pending (for example, an interrupt pending to inject
to L1) and hypervisor will switch the VCPU context from L2 to L1. Now we
already are in L1's context, but since we got a X86EMUL_RETRY just now and
this means hypervisor will retry to handle the IO request later and
unfortunately, the retry will happen in L1's context. And it will cause the
problem. The fixing is that if there is a pending IO request, no virtual
vmexit/vmentry is allowed.
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> Acked-by: Jun Nakajima <jun.nakajima@intel.com>
Jan Beulich [Mon, 20 Jan 2014 08:50:20 +0000 (09:50 +0100)]
compat/memory: fix build with old gcc
struct xen_add_to_physmap_batch's size field being uint16_t causes old
compiler versions to warn about the pointless range check done inside
compat_handle_okay().
Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org>
In public/memory.h, struct xen_add_to_physmap has 'space' as an unsigned int,
but struct xen_add_to_physmap_batch has 'space' as a uint16_t.
By defining xenmem_add_to_physmap_one() with space defined as uint16_t, the
now-common xenmem_add_to_physmap() implicitly truncates xatp->space from
unsigned int to uint16_t, which changes the space switch()'d upon.
This wouldn't be noticed with any upstream code (of which I am aware), but was
discovered because of the XenServer support for legacy Windows PV drivers,
which make XENMEM_add_to_physmap hypercalls using spaces with the top bit set.
The current Windows PV drivers don't do this any more, but we 'fix' Xen to
support running VMs with out-of-date tools.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Ack: Ian Campbell <Ian.Campbell@citrix.com> Acked-by: Keir Fraser <keir@xen.org>
Yang Zhang [Fri, 17 Jan 2014 15:00:21 +0000 (16:00 +0100)]
nested EPT: fixing wrong handling for L2 guest's direct mmio access
L2 guest will access the physical device directly(nested VT-d). For such access,
Shadow EPT table should point to device's MMIO. But in current logic, L0 doesn't
distinguish the MMIO whether from qemu or physical device when building shadow EPT table.
This is wrong. This patch will setup the correct shadow EPT table for such MMIO ranges.
Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> Acked-by: Tim Deegan <tim@xen.org>
Frediano Ziglio [Fri, 17 Jan 2014 14:58:27 +0000 (15:58 +0100)]
mce: fix race condition in mctelem_xchg_head
The function (mctelem_xchg_head()) used to exchange mce telemetry
list heads is racy. It may write to the head twice, with the second
write linking to an element in the wrong state.
If there are two threads, T1 inserting on committed list; and T2
trying to consume it.
1. T1 starts inserting an element (A), sets prev pointer (mcte_prev).
2. T1 is interrupted after the cmpxchg succeeded.
3. T2 gets the list and changes element A and updates the commit list
head.
4. T1 resumes, reads pointer to prev again and compare with result
from the cmpxchg which succeeded but in the meantime prev changed
in memory.
5. T1 thinks the cmpxchg failed and goes around the loop again,
linking head to A again.
To solve the race use temporary variable for prev pointer.
*linkp (which point to a field in the element) must be updated before
the cmpxchg() as after a successful cmpxchg the element might be
immediately removed and reinitialized.
The wmb() prior to the cmpchgptr() call is not necessary since it is
already a full memory barrier. This wmb() is thus removed.
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com>
Ian Jackson [Mon, 13 Jan 2014 18:15:37 +0000 (18:15 +0000)]
xl: Always use "fast" migration resume protocol
As Ian Campbell writes in http://bugs.xenproject.org/xen/bug/30:
There are two mechanisms by which a suspend can be aborted and the
original domain resumed.
The older method is that the toolstack resets a bunch of state (see
tools/python/xen/xend/XendDomainInfo.py resumeDomain) and then
restarts the domain. The domain will see HYPERVISOR_suspend return 0
and will continue without any realisation that it is actually
running in the original domain and not in a new one. This method is
supposed to be implemented by libxl_domain_resume(suspend_cancel=0)
but it is not.
The other method is newer and in this case the toolstack arranges
that HYPERVISOR_suspend returns SUSPEND_CANCEL and restarts it. The
domain will observe this and realise that it has been restarted in
the same domain and will behave accordingly. This method is
implemented, correctly AFAIK, by
libxl_domain_resume(suspend_cancel=1).
Attempting to use the old method without doing all of the work simply
causes the guest to crash. Implementing the work required for old
method, or for checking that domains actually support the new method,
is not feasible at this stage of the 4.4 release.
So, always use the new method, without regard to the declarations of
support by the guest. This is a strict improvement: guests which do
in fact support the new method will work, whereas ones which don't are
no worse off.
There are two call sites of libxl_domain_resume that need fixing, both
in the migration error path.
With this change I observe a correct and successful resumption of a
Debian wheezy guest with a Linux 3.4.70 kernel after a migration
attempt which I arranged to fail by nobbling the block hotplug script.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com> CC: konrad.wilk@oracle.com CC: David Vrabel <david.vrabel@citrix.com> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Wei Liu [Mon, 13 Jan 2014 11:52:28 +0000 (11:52 +0000)]
libxl: disallow PCI device assignment for HVM guest when PoD is enabled
This replicates a Xend behavior, see ec789523749 ("xend: Dis-allow
device assignment if PoD is enabled.").
This change is restricted to HVM guest, as only HVM is relevant in the
counterpart in Xend. We're late in release cycle so the change should
only do what's necessary. Probably we can revisit it if we need to do
the same thing for PV guest in the future.
Signed-off-by: Wei Liu <wei.liu2@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Julien Grall [Tue, 14 Jan 2014 13:36:55 +0000 (13:36 +0000)]
xen/arm: p2m: Correctly flush TLB in create_p2m_entries
The p2m is shared between VCPUs for each domain. Currently Xen only flush
TLB on the local PCPU. This could result to mismatch between the mapping in the
p2m and TLBs.
Flush TLB entries used by this domain on every PCPU. The flush can also be
moved out of the loop because:
- ALLOCATE: only called for dom0 RAM allocation, so the flush is never called
- INSERT: if valid = 1 that would means with have replaced a
page that already belongs to the domain. A VCPU can write on the wrong page.
This can happen for dom0 with the 1:1 mapping because the mapping is not
removed from the p2m.
- REMOVE: except for grant-table (replace_grant_host_mapping), each
call to guest_physmap_remove_page are protected by the callers via a
get_page -> .... -> guest_physmap_remove_page -> ... -> put_page. So
the page can't be allocated for another domain until the last put_page.
- RELINQUISH : the domain is not running anymore so we don't care...
Also avoid leaking a foreign page if the function is INSERTed a new mapping
on top of foreign mapping.
Signed-off-by: Julien Grall <julien.grall@linaro.org> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Julien Grall [Thu, 9 Jan 2014 16:58:03 +0000 (16:58 +0000)]
xen/arm: correct flush_tlb_mask behaviour
On ARM, flush_tlb_mask is used in the common code:
- alloc_heap_pages: the flush is only be called if the new allocated
page was used by a domain before. So we need to flush only TLB non-secure
non-hyp inner-shareable.
- common/grant-table.c: every calls to flush_tlb_mask are used with
the current domain. A flush TLB by current VMID inner-shareable is enough.
The current code only flush hypervisor TLB on the current PCPU. For now,
flush TLBs non-secure non-hyp on every PCPUs.
Signed-off-by: Julien Grall <julien.grall@linaro.org> Acked-by: Ian Campbell <ian.campbell@citrix.com>
libxl: ocaml: guard x86-specific functions behind an ifdef
The various cpuid functions are not available on ARM, so this
makes them raise an OCaml exception. Omitting the functions
completely results in a link failure in oxenstored due to the
missing symbols, so this is preferable to the much bigger patch
that would result from adding conditional compilation into the
OCaml interfaces.
With this patch, oxenstored can successfully start a domain on
Xen/ARM.
Signed-off-by: Anil Madhavapeddy <anil@recoil.org> Acked-by: David Scott <dave.scott@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Even with this fix there is a period between the flush and the unmap where
processor may speculate data into the cache. The solution is to map this
region uncached or to use the HCR.DC bit to mark all guest accesses cached. 89eb02c2204a "xen: arm: force guest memory accesses to cacheable when MMU is
disabled" has arranged to do the latter.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Ian Campbell [Wed, 8 Jan 2014 14:09:01 +0000 (14:09 +0000)]
xen: arm: force guest memory accesses to cacheable when MMU is disabled
On ARM guest OSes are started with MMU and Caches disables (as they are on
native) however caching is enabled in the domain running the builder and
therefore we must ensure cache consistency.
The existing solution to this problem (a0035ecc0d82 "tools: libxc: flush data
cache after loading images into guest memory") is to flush the caches after
loading the various blobs into guest RAM. However this approach has two short
comings:
- The cache flush primitives available to userspace on arm32 are not
sufficient for our needs.
- There is a race between the cache flush and the unmap of the guest page
where the processor might speculatively dirty the cache line again.
(of these the second is the more fundamental)
This patch makes use of the the hardware functionality to force all accesses
made from guest mode to be cached (the HCR.DC == default cached bit). This
means that we don't need to worry about the domain builder's writes being
cached because the guests "uncached" accesses will actually be cached.
Unfortunately the use of HCR.DC is incompatible with the guest enabling its
MMU (SCTLR.M bit). Therefore we must trap accesses to the SCTLR so that we can
detect when this happens and disable HCR.DC. This is done with the HCR.TVM
(trap virtual memory controls) bit which also causes various other registers
to be trapped, all of which can be passed straight through to the underlying
register. Once the guest has enabled its MMU we no longer need to trap so
there is no ongoing overhead. In my tests Linux makes about half a dozen
accesses to these registers before the MMU is enabled, I would expect other
OSes to behave similarly (the sequence of writes needed to setup the MMU is
pretty obvious).
Apart from this unfortunate need to trap these accesses this approach is
incompatible with guests which attempt to do DMA operations with their MMU
disabled. In practice this means guests with passthrough which we do not yet
support. Since a typical guest (including dom0) does not access devices which
require DMA until after it is fully up and running with paging enabled the
main risk is to in-guest firmware which does DMA i.e. running EFI in a guest,
with a disk passed through and booting from that disk. Since we know that dom0
is not using any such firmware and we do not support device passthrough to
guests yet we can live with this restriction. Once passthrough is implemented
this will need to be revisited.
The patch includes a couple of seemingly unrelated but necessary changes:
- HSR_SYSREG_CRN_MASK was incorrectly defined, which happened to be benign
with the existing set of system register we handled, but broke with the new
ones introduced here.
- The defines used to decode the HSR system register fields were named the
same as the register. This breaks the accessor macros. This had gone
unnoticed because the handling of the existing trapped registers did not
require accessing the underlying hardware register. Rename those constants
with an HSR_SYSREG prefix (in line with HSR_CP32/64 for 32-bit registers).
This patch has survived thousands of boot loops on a Midway system.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Julien Grall <julien.grall@linaro.org>
Rob Hoes [Fri, 10 Jan 2014 13:52:04 +0000 (13:52 +0000)]
libxl: ocaml: use 'for_app_registration' in osevent callbacks
This allows the application to pass a token to libxl in the fd/timeout
registration callbacks, which it receives back in modification or
deregistration callbacks.
It turns out that this is essential for timeout handling, in order to
identify which timeout to change on a modify event.
Signed-off-by: Rob Hoes <rob.hoes@citrix.com> Acked-by: David Scott <dave.scott@eu.citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
David Vrabel [Fri, 10 Jan 2014 16:46:33 +0000 (17:46 +0100)]
x86: map portion of kexec crash area that is within the direct map area
Commit 7113a45451a9f656deeff070e47672043ed83664 (kexec/x86: do not map
crash kernel area) causes fatal page faults when loading a crash
image. The attempt to zero the first control page allocated from the
crash region will fault as the VA return by map_domain_page() has no
mapping.
The fault will occur on non-debug builds of Xen when the crash area is
below 5 TiB (which will be most systems).
The assumption that the crash area mapping was not used is incorrect.
map_domain_page() is used when loading an image and building the
image's page tables to temporarily map the crash area, thus the
mapping is required if the crash area is in the direct map area.
Reintroduce the mapping, but only the portions of the crash area that
are within the direct map area.
Reported-by: Don Slutz <dslutz@verizon.com> Signed-off-by: David Vrabel <david.vrabel@citrix.com> Tested-by: Don Slutz <dslutz@verizon.com> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> Tested-by: Daniel Kiper <daniel.kiper@oracle.com>
This is really just a band aid - kexec shouldn't rely on the crash area
being always mapped when in the direct mapping range (and it didn't use
to in its previous form). That's primarily because map_domain_page()
(needed when the area is outside the direct mapping range) may be
unusable when wanting to kexec due to a crash, but also because in the
case of PFN compression the kexec range (if specified on the command
line) could fall into a hole between used memory ranges (while we're
currently only ignoring memory at the top of the physical address
space, it's pretty clear that sooner or later we will want that
selection to become more sophisticated in order to maximize the memory
made use of).
Andrew Cooper [Fri, 10 Jan 2014 16:45:01 +0000 (17:45 +0100)]
dbg_rw_guest_mem: need to call put_gfn in error path
Using a 1G hvm domU (in grub) and gdbsx:
(gdb) set arch i8086
warning: A handler for the OS ABI "GNU/Linux" is not built into this configuration
of GDB. Attempting to continue with the default i8086 settings.
The target architecture is assumed to be i8086
(gdb) target remote localhost:9999
Remote debugging using localhost:9999
Remote debugging from host 127.0.0.1
0x0000d475 in ?? ()
(gdb) x/1xh 0x6ae9168b
Will reproduce this bug.
With a debug=y build you will get:
Assertion '!preempt_count()' failed at preempt.c:37
For a debug=n build you will get a dom0 VCPU hung (at some point) in:
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Don Slutz <dslutz@verizon.com> Acked-by: Mukesh Rathor <mukesh.rathor@oracle.com>
Stefan Bader [Wed, 8 Jan 2014 17:26:59 +0000 (18:26 +0100)]
libxl: Auto-assign NIC devids in initiate_domain_create
This will change initiate_domain_create to walk through NIC definitions
and automatically assign devids to those which have not assigned one.
The devids are needed later in domcreate_launch_dm (for HVM domains
using emulated NICs). The command string for starting the device-model
has those ids as part of its arguments.
Assignment of devids in the hotplug case is handled by libxl_device_nic_add
but that would be called too late in the startup case.
I also moved the call to libxl__device_nic_setdefault here as this seems
to be the only path leading there and avoids doing the loop a third time.
The two loops are trying to handle a case where the caller sets some devids
(not sure that should be valid) but leaves some unset.
Signed-off-by: Stefan Bader <stefan.bader@canonical.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Wei Liu [Tue, 17 Dec 2013 22:53:45 +0000 (22:53 +0000)]
xl: create VFB for PV guest when VNC is specified
This replicates a Xend behavior. When you specify 'vnc=1' and there's no
'vfb=[]' in a PV guest's config file, xl parses all top level VNC options and
creates a VFB for you.
Reported-by: Konrad Wilk <konrad.wilk@oracle.com> Signed-off-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Anthony PERARD [Wed, 8 Jan 2014 08:17:55 +0000 (09:17 +0100)]
firmware: change level-triggered GPE event to a edge one for qemu-xen
This should help to reduce a CPU hotplug race window where a cpu hotplug
event while not be seen by the OS.
When hotplugging more than one vcpu, some of those vcpus might not be
seen as plugged by the guest.
This is what is currently happenning:
1. hw adds cpu, sets GPE.2 bit and sends SCI
2. OSPM gets SCI, reads GPE00.sts and masks GPE.2 bit in GPE00.en
3. OSPM executes _L02 (level-triggered event associate to cpu hotplug)
4. hw adds second cpu and sets GPE.2 bit but SCI is not asserted
since GPE00.en masks event
5. OSPM resets GPE.2 bit in GPE00.sts and umasks it in GPE00.en
as result event for step 4 is lost because step 5 clears it and OS
will not see added second cpu.
1. Disables the interrupt source (GPEx_BLK EN bit).
2. If an edge event, clears the status bit.
3. Performs one of the following:
* Dispatches to an ACPI-aware device driver.
* Queues the matching control method for execution.
* Manages a wake event using device _PRW objects.
4. If a level event, clears the status bit.
5. Enables the interrupt source.
So, by using edge-triggered General-Purpose Event instead of a
level-triggered GPE, OSPM is less likely to clear the status bit of the
addition of the second CPU. On step 5, QEMU will resend an interrupt if
the status bit is set.
This description apply also for PCI hotplug since the same step are
followed by QEMU, so we also change the GPE event type for PCI hotplug.
This does not apply to qemu-xen-traditional because it does not resend
an interrupt if necessary as a result of step 5.
Patch and description inspired by SeaBIOS's commit:
Replace level gpe event with edge gpe event for hot-plug handlers 9c6635bd48d39a1d17d0a73df6e577ef6bd0037c
from Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org>
Jan Beulich [Wed, 8 Jan 2014 08:04:48 +0000 (09:04 +0100)]
rename XENMEM_add_to_physmap_{range => batch}
The use of "range" here wasn't really correct - there are no ranges
involved. As the comment in the public header already correctly said,
all this is about is batching of XENMEM_add_to_physmap calls (with
the addition of having a way to specify a foreign domain for
XENMAPSPACE_gmfn_foreign).
Suggested-by: Ian Campbell <Ian.Campbell@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Keir Fraser <keir@xen.org>
Bob Liu [Thu, 12 Dec 2013 11:05:15 +0000 (19:05 +0800)]
tmem: check the return value of copy to guest
Use function copy_to_guest_offset/copy_to_guest directly and check their return
value.
This also fixes CID 1132754, and 1132755:
"Unchecked return value
If the function returns an error value, the error value may be mistaken for a
normal value. In tmem_copy_to_client_buf_offset: Value returned from a function
is not checked for errors before being used (CWE-252)"
And CID 1055125, 1055126, 1055127, 1055128, 1055129, 1055130
"Unchecked return value
If the function returns an error value, the error value may be mistaken for a
normal value. In <functions changed>: Value returned from a function is not
checked for errors before being used (CWE-252)"
Signed-off-by: Bob Liu <bob.liu@oracle.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Bob Liu [Thu, 12 Dec 2013 11:05:11 +0000 (19:05 +0800)]
tmem: cleanup: drop useless functions from header file
They are several one line functions in tmem_xen.h which are useless, this patch
embeded them into tmem.c directly.
Also modify void *tmem in struct domain to struct client *tmem_client in order
to make things more straightforward.
Signed-off-by: Bob Liu <bob.liu@oracle.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Keir Fraser <keir@xen.org>