Introduce a status field in struct pending_irq. Valid states are
GUEST_PENDING, GUEST_VISIBLE and GUEST_ENABLED and they are not mutually
exclusive. See the in-code comment for an explanation of the states and
how they are used.
Use atomic operations to set and clear the status bits. Note that
setting GIC_IRQ_GUEST_VISIBLE and clearing GIC_IRQ_GUEST_PENDING can be
done in two separate operations as the underlying pending status is
actually only cleared on the LR after the guest ACKs the interrupts.
Until that happens it's not possible to receive another interrupt.
The main effect of this patch is that an IRQ can be set to GUEST_PENDING
while it is being serviced by the guest. In maintenance_interrupt we
check whether GUEST_PENDING is set and if it is we add the irq back into
the lr_pending queue so that it's going to be reinjected one more time,
if the interrupt is still enabled at the vgicd level.
If it is not, it is going to be injected as soon as the guest renables
the interrupt.
One exception is evtchn_irq: in that case we don't want to
set the GIC_IRQ_GUEST_PENDING bit if it is already GUEST_VISIBLE,
because as part of the event handling loop, the guest would realize that
new events are present even without a new notification.
Also we already have a way to figure out exactly when we do need to
inject a second notification if vgic_vcpu_inject_irq is called after the
end of the guest event handling loop and before the guest EOIs the
interrupt (see db453468d9 "arm: vgic: fix
race between evtchn upcall and evtchnop_send").
Don't call gic_inject_irq_stop from maintenance_interrupt because
gic_inject (called by leave_hypervisor_tail) is going to call
gic_inject_irq_start/stop appropriately later anyway.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
When Xen needs to EOI a physical IRQ, we should use the IRQ number
in irq_desc instead of the virtual IRQ.
Remove the eoi flag in maintenance_interrupt and replace the check with
a check on p->desc != NULL.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
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 flush the cache as we load the blobs, otherwise when the
guest starts running it may not see them. The dom0 build in the hypervisor has
the same requirements and already does the right thing.
The mechanism for performing a cache flush from userspace is OS specific, so
implement this as a new osdep hook:
- On 32-bit ARM Linux provides a system call to flush the cache.
- On 64-bit ARM Linux the processor is configured to allow cache flushes
directly from userspace.
- Non-Linux platforms will need to provide their own implementation. If
similar mechanisms are not available then a new privcmd ioctl should be a
suitable alternative.
No cache maintenance is required on x86, so provide a stub for all non-Linux
platforms which returns success on x86 only and log an error otherwise.
This fixes guest building on Xgene which has a very large L3 cache and so is
particularly susceptible to this problem. It has also been observed
sporadically on midway.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Andre Przywara <andre.przywara@calxeda.com>
Cc: Pranavkumar Sawargaonkar <psawargaonkar@apm.com>
Cc: Anup Patel <apatel@apm.com>
Currently we check the pty path received from xenstore with access(); if
it indicates that the pty is not accessible, we loop around and wait for
a new path to appear in xenstore.
This has several issues:
* If a path has been written to xenstore, it can be assumed that that
pty should already be accessible to xenconsole, and hence any error
that occurs while trying to open it should be fatal and not ignored
* If access() indicates no access to the pty, the memory allocated for
the path is leaked when going around the loop again
* The accessibility of the pty could change between the access() and
open() calls, leading to a TOCTOU race (this is what Coverity is
complaining about).
By removing the explicit access() check and just erroring out whenever
open() fails, we fix all these issues.
Coverity-ID: 1056047
Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
If a HVM guest attempts to use the FIFO-based ABI it will not receive
any events and destroying the guest may crash Xen or trigger an assert
when attempting to unmap a control block page. This occurs because
Xen maps the wrong page for both the control blocks and the event
arrays.
In map_guest_page(), use the MFN of the guest's page and not the GFN
when calling map_domain_page_global().
Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Coverity ID: 1055996 1056002
Cache the xs_daemon_socket{,_ro}() strings to save pointlessly
re-snprintf()'ing the same path, and add explicit size checks against
addr.sun_path before strcpy()'ing into it.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Matthew Daley <mattd@bugfuzz.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Both src[i].size and delta are unsigned, so checking their difference
for being less than 0 doesn't work.
Coverity-ID: 1055615
Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
While at it, tidy up the function; there's no point in allocating more
than the amount of domains actually returned by xc_domain_getinfolist
(barring the caveat described in the newly-added comment)
Coverity-ID: 1055888
Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Ocaml has a heap lock which must be held whenever ocaml code is running. Ocaml
usually drops this lock when it enters a potentially blocking low-level
function, such as writing to a file. Libxl has its own lock, which it may
acquire when being called.
Things get interesting when libxl calls back into ocaml code. There is a risk
of ending up in a deadlock when a thread holds both locks at the same time,
then temporarily drop the ocaml lock, while another thread calls another libxl
function.
To avoid deadlocks, we drop the ocaml heap lock before entering libxl, and
reacquire it in callbacks to ocaml. This way, the ocaml heap lock is never held
together with the libxl lock, except in osevent registration callbacks, and
xentoollog callbacks. If we guarantee to not call any libxl functions inside
those callbacks, we can avoid deadlocks.
This patch handle the dropping and reacquiring of the ocaml heap lock by the
caml_enter_blocking_section and caml_leave_blocking_section functions, and
related macros. We are also careful to not call any functions that access the
ocaml heap while the ocaml heap lock is dropped. This often involves copying
ocaml values to C before dropping the ocaml lock.
The ao_how in aohow_val is now malloc'ed, just to make this function a little
easier to use.
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Acked-by: David Scott <dave.scott@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Also:
* Reorganise toplevel OCaml functions into modules of Xenlight.
* Factor out the management of ao_how into the function aohow_val. The ao_how
is now malloc'ed, just to make this function a little easier to use.
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: David Scott <dave.scott@eu.citrix.com>
Having bindings to the low-level functions libxl_osevent_register_hooks and
related, allows to run an event loop in OCaml; either one we write ourselves,
or one that is available elsewhere.
The Lwt cooperative threads library (http://ocsigen.org/lwt/), which is quite
popular these days, has an event loop that can be easily extended to poll any
additional fds that we get from libxl. Lwt provides a "lightweight" threading
model, which does not let you run any other (POSIX) threads in your
application, and therefore excludes an event loop implemented in the C
bindings.
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Acked-by: David Scott <dave.scott@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Currently an unhandled data abort in guest context leads to us killing the
guest and an unhandled instruction abort in guest context leads to us killing
the host!
Andre pointed out that an unhandled data abort can be caused by e.g. dmidecode
looking for things which are not there in the guests physical address space.
Propagating the fault to the guest allows it to properly SIGSEGV the
processes.
A guest kernel can trivially jump to an unmapped physical address which would
cause an instruction abort. Killing the host for that is obviously bad.
Instead inject the exception so the guest kernel can SIGSEGV or panic() etc as
it deems appropriate.
Tested on arm64 (Mustang) and arm32 (Midway) with a dom0 kernel late_initcall
which either dereferences or jumps to address 0, provoking both behaviours and
resulting correctly in a guest kernel panic. Also tested on fast models with a
32-bit dom0 on a 64-bit hypervisor, which behaved correctly.
In addition tested on both platforms with a userspace program which either
calls to or dereferences address 0. The process is correctly killed with SEGV.
Lastly tested on Mustang with a 32-bit version of the userspace test on a
64-bit dom0 kernel.
I think that covers all the cases.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Cc: Andre Przywara <andre.przywara@calxeda.com>
[ ijc -- fixed up whitespace in if statements in cpsr_mode_switch ]
They're being taken care of later (via set_debugreg()), and temporarily
copying them into struct vcpu means that bad values may end up getting
loaded during context switch if the vCPU is already running and the
function errors out between the premature and real commit step, leading
to the same issue that XSA-12 dealt with.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Keir Fraser <keir@xen.org>
Since state information coming from Dom0 can arrive at any time, on
any CPU, we ought to make sure that a new state is fully initialized
before the target CPU might be using it.
Once touching that code, also do minor cleanup: A missing (but benign)
"break" and some white space adjustments.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Liu Jinsong <jinsong.liu@intel.com>
Coverity ID: 1135379
As the code stands, the domain mapping will be leaked on each error path.
The mapping can be for a much shorter period of time, and all the relevent
information can be pulled out at once.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Tested-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Both xenmem_add_to_physmap() and iommu_populate_page_table() each have
an error path that fails to clear that flag, thus suppressing further
flushes on the respective pCPU.
In iommu_populate_page_table() also slightly re-arrange code to avoid
the false impression of the flag in question being guarded by a
domain's page_alloc_lock.
This is CVE-2013-6400 / XSA-80.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
List all the sub ops of:
__HYPERVISOR_domctl
__HYPERVISOR_sysctl
__HYPERVISOR_memory_op
__HYPERVISOR_tmem_op
which are subject to the policy given in
http://xenbits.xen.org/xsa/advisory-77.html
It is expected that these lists will be whittled away as each interface is
audited for safety.
New interfaces should be expected to be safe when introduced (IOW the list
should never be expanded).
This is XSA-77.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
The commit d963923 "xen: arm: correct return value of
raw_copy_{to/from}_guest_*, raw_clear_guest" doesn't permit to boot guest
on Xen ARM.
Remove the stray semicolon from the end of the if statement.
Also we want to get the right rc in the error arrays, so we need to do the
copy_to_guest_offset before checking the rc returned by
xenmem_add_to_physmap_one.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
[ ijc -- expanded commit log ]
This is a generic interface which is supposed to return the number of bytes
which were not copied. Make it so.
Update the incorrect callers prepare_dtb, decode_thumb{2} and
xenmem_add_to_physmap_range.
In the xenmem_add_to_physmap_range case, observe that we are not propagating
errors from xenmem_add_to_physmap_one and do so.
In the decode_thumb case and an emacs magic block to decode.c
Make the flush_dcache parameter to the helper an int while at it.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
We don't actually use this but I was using it locally for debugging and it
tripped me up.
Also add DCCIMVAC "data cache clean and invalidate by MVA" which is the only
cache op missing from cpregs.h.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
The Xgene platform has no RAM below 4G.
The /chosen/linux,initrd-* properties do not have "reg" semantics and
therefore #*-size are not used when interpreting. Instead they are are simply
numbers which are interpreted according to the properties length.
Fix this both when parsing the entry in the host DTB and when creating the
dom0 DTB. For dom0 we simply hardcode a 64-bit size, this is acceptable
even for a 32-bit guest.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
The BUG_ON in p2m_map_first was over aggressive since the paddr_t can have
come from the guest, via add_to_physmap. Instead return failure to the caller.
Also the check was simultaneously too lose. The valid offsets are
0..P2M_FIRST_ENTRIES-1 inclusive.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
This commit introduces "check-xl-vcpupin-parse" for helping
verifying and debugging the (v)CPU bitmap parsing code in xl.
The script runs "xl -N vcpu-pin 0 all <some strings>"
repeatedly, with various input strings, and checks that the
output is as expected.
This is what the script can do:
# ./check-xl-vcpupin-parse -h
usage: ./check-xl-vcpupin-parse [options]
Tests various vcpu-pinning strings. If run without arguments acts
as follows:
- generates some test data and saves them in
check-xl-vcpupin-parse.data;
- tests all the generated configurations (reading them back from
check-xl-vcpupin-parse.data).
An example of a test vector file is provided in
check-xl-vcpupin-parse.data-example.
Options:
-h prints this message
-r seed uses seed for initializing the rundom number generator
(default: the script PID)
-s string tries using string as a vcpu pinning configuration and
reports whether that succeeds or not
-o ofile save the test data in ofile
(default: check-xl-vcpupin-parse.data)
-i ifile read test data from ifile
An example test data file (generated on a 2 NUMA nodes, 16 CPUs
host) is being provided in check-xl-vcpupin-parse.data-example.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
As it can be useful to see if the outcome of some complex vCPU
pinning bitmap specification looks as expected.
This also allow for the introduction of some automatic testing
and verification for the bitmap parsing code, as it happens
already in check-xl-disk-parse and check-xl-vif-parse.
In particular, to make the above possible, this commit also
changes the implementation of the vcpu-pin command so that,
instead of always returning 0, it returns an error if the
parsing fails.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Making it possible to use something like the following:
* "nodes:0-3": all pCPUs of nodes 0,1,2,3;
* "nodes:0-3,^node:2": all pCPUS of nodes 0,1,3;
* "1,nodes:1-2,^6": pCPU 1 plus all pCPUs of nodes 1,2
but not pCPU 6;
* ...
In both domain config file and `xl vcpu-pin'.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Stefano Stabellini