Commit Graph
14404 Commits
Author SHA1 Message Date
Nick Rosbrook 7e84aeb864 golang/xenlight: define KeyValueList as empty struct
Define KeyValueList as empty struct as there is currently no reason for
this type to be available in the Go package.

Implement fromC and toC functions as no-ops.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2019-12-16 18:08:01 +00:00
Nick Rosbrook fb2475aed3 golang/xenlight: define Devid type as int
Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2019-12-16 18:08:00 +00:00
Nick Rosbrook 53d6570308 golang/xenlight: define Defbool builtin type
Define Defbool as struct analagous to the C type, and define the type
'defboolVal' that represent true, false, and default defbool values.

Implement Set, Unset, SetIfDefault, IsDefault, Val, and String functions
on Defbool so that the type can be used in Go analagously to how its
used in C.

Finally, implement fromC and toC functions.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2019-12-16 18:07:59 +00:00
Nick Rosbrook 871e51d2d4 golang/xenlight: generate enum types from IDL
Introduce gengotypes.py to generate Go code the from IDL. As a first step,
implement 'enum' type generation.

As a result of the newly-generated code, remove the existing, and now
conflicting definitions in xenlight.go. In the case of the Error type,
rename the slice 'errors' to 'libxlErrors' so that it does not conflict
with the standard library package 'errors.' And, negate the values used
in 'libxlErrors' since the generated error values are negative.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2019-12-16 18:07:59 +00:00
Juergen Gross 5852ca4852 build: fix tools/configure in case only python3 exists
Calling ./configure with python3 being there but no python,
tools/configure will fail. Fix that by defaulting to python and
falling back to python3 or python2.

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
[ wei: run autogen.sh ]
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-12-12 14:16:43 +00:00
Julian Tuminaro 0022387cef Fix the KDD_LOG statements to use appropriate format specifier for printing uint64_t
Previous commit in kdd.c had a small issue which lead to warning/error while compiling
on 32-bit systems due to mismatch of type size while doing type cast from uint64_t to
void *

Signed-off-by: Jenish Rakholiya <rjenish@cmu.edu>
Signed-off-by: Julian Tuminaro <jtuminar@andrew.cmu.edu>
Acked-by: Tim Deegan <tim@xen.org>
2019-12-01 22:06:15 +00:00
George Dunlap f2ae59bc4b Rationalize max_grant_frames and max_maptrack_frames handling
Xen used to have single, system-wide limits for the number of grant
frames and maptrack frames a guest was allowed to create. Increasing
or decreasing this single limit on the Xen command-line would change
the limit for all guests on the system.

Later, per-domain limits for these values was created. The system-wide
limits became strict limits: domains could not be created with higher
limits, but could be created with lower limits. However, that change
also introduced a range of different "default" values into various
places in the toolstack:

- The python libxc bindings hard-coded these values to 32 and 1024,
  respectively
- The libxl default values are 32 and 1024 respectively.
- xl will use the libxl default for maptrack, but does its own default
  calculation for grant frames: either 32 or 64, based on the max
  possible mfn.

These defaults interact poorly with the hypervisor command-line limit:

- The hypervisor command-line limit cannot be used to raise the limit
  for all guests anymore, as the default in the toolstack will
  effectively override this.
- If you use the hypervisor command-line limit to *reduce* the limit,
  then the "default" values generated by the toolstack are too high,
  and all guest creations will fail.

In other words, the toolstack defaults require any change to be
effected by having the admin explicitly specify a new value in every
guest.

In order to address this, have grant_table_init treat negative values
for max_grant_frames and max_maptrack_frames as instructions to use the
system-wide default, and have all the above toolstacks default to passing
-1 unless a different value is explicitly configured.

This restores the old behavior in that changing the hypervisor command-line
option can change the behavior for all guests, while retaining the ability
to set per-guest values.  It also removes the bug that reducing the
system-wide max will cause all domains without explicit limits to fail.

NOTE: - The Ocaml bindings require the caller to always specify a value,
        and the code to start a xenstored stubdomain hard-codes these to 4
	and 128 respectively; this behavour will not be modified.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
2019-11-29 21:43:49 +00:00
julian.tuminaro@gmail.com 53e9ecffde kdd.c: Add support for initial handshake in KD protocol for Win 7, 8 and 10 (64 bit)
Current implementation of find_os is based on the hard-coded values for
different Windows version. It uses the value for get the address to
start looking for DOS header in the given specified range. However, this
is not scalable to all version of Windows as it will require us to keep
adding new entries and also due to KASLR, chances of not hitting the PE
header is significant. We implement a way for 64-bit systems to use IDT
entry to get a valid exception/interrupt handler and then move back into
the memory to find the valid DOS header. Since IDT entries are protected
by PatchGuard, we think our assumption that IDT entries will not be
corrupted is valid for our purpose. Once we have the image base, we
search for the DBGKD_GET_VERSION64 structure type in .data section to
get information required for handshake.

Currently, this is a work in progress feature and current patch only
supports the handshake and memory read/write on 64-bit systems.

NOTE: This is the Updated version of the previous patch submitted
NOTE: This has currently been only tested when debugging was not enabled
on the guest Windows.

Signed-off-by: Jenish Rakholiya <rjenish@cmu.edu>
Signed-off-by: Julian Tuminaro <jtuminar@andrew.cmu.edu>
Reviewed-by: Tim Deegan <tim@xen.org>
Reviewed-by: Paul Durrant <paul@xen.org>
2019-11-29 20:49:34 +00:00
Ian Jackson 8b955796c5 Config.mk et al: Branch off Xen 4.14-unstable
Bump the version numbers everywhere and rerun autogen.sh.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-11-29 14:46:34 +00:00
George Dunlap 8c79c129a6 x86: Don't increase ApicIdCoreSize past 7
Changeset ca2eee92df ("x86, hvm: Expose host core/HT topology to HVM
guests") attempted to "fake up" a topology which would induce guest
operating systems to not treat vcpus as sibling hyperthreads.  This
involved actually reporting hyperthreading as available, but giving
vcpus every other ApicId; which in turn led to doubling the ApicIds
per core by bumping the ApicIdCoreSize by one.  In particular, Ryzen
3xxx series processors, and reportedly EPYC "Rome" cpus -- have an
ApicIdCoreSize of 7; the "fake" topology increases this to 8.

Unfortunately, Windows running on modern AMD hardware -- including
Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus --
doesn't seem to cope with this value being higher than 7.  (Linux
guests have so far continued to cope.)

A "proper" fix is complicated and it's too late to fix it either for
4.13, or to backport to supported branches.  As a short-term fix,
limit this value to 7.

This does mean that a Linux guest, booted on such a system without
this change, and then migrating to a system with this change, with
more than 64 vcpus, would see an apparent topology change.  This is a
low enough risk in practice that enabling this limit unilaterally, to
allow other guests to boot without manual intervention, is worth it.

Reported-by: Steven Haigh <netwiz@crc.id.au>
Reported-by: Andreas Kinzler <hfp@posteo.de>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-26 10:33:52 +00:00
Oleksandr Grytsov 8f486344a0 libxl: make default path to add/remove all PV devices
Adding/removing device is handled for specific devices only: VBD, VIF,
QDISK. This commit adds default case to handle adding/removing for all PV
devices by default, except QDISK device, which requires special handling.
If any other device is required a special handling it should be done by
implementing separate case (similar to QDISK device). The default
behaviour for adding device is to wait when the backend goes to
XenbusStateInitWait and the default behaviour on removing device is to
start generic device remove procedure.

Also this commit fixes removing guest function: before the guest was
removed when all VIF and VBD devices are removed. The fix removes
guest when all created devices are removed. This is done by checking the
guest device list instead of checking num_vifs and num_vbds. num_vifs and
num_vbds variables are removed as redundant in this case.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-25 10:43:32 +00:00
Oleksandr Grytsov c568b11da2 libxl: introduce new backend type VINPUT
There are two kind of VKBD devices: with QEMU backend and user space PV
backend. In current implementation they can't be distinguished as both use
VKBD backend type. As result, user space PV KBD backend is started and
stopped as QEMU backend. This commit adds new device kind VINPUT to be
used as backend type for user space PV KBD backend.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-25 10:42:56 +00:00
Anthony PERARD 0d2791b007 configure: Fix test for python 3.8
https://docs.python.org/3.8/whatsnew/3.8.html#debug-build-uses-the-same-abi-as-release-build

> To embed Python into an application, a new --embed option must be
> passed to python3-config --libs --embed to get -lpython3.8 (link the
> application to libpython). To support both 3.8 and older, try
> python3-config --libs --embed first and fallback to python3-config
> --libs (without --embed) if the previous command fails.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
[ wei: rerun autogen.sh ]
2019-11-20 10:46:48 +00:00
Ian Jackson bad237ded7 tools/configure: Honour XEN_COMPILE_ARCH and _TARGET_ for shim
The pvshim can only be built 64-bit because the hypervisor is only
64-bit nowadays.  The hypervisor build supports XEN_COMPILE_ARCH and
XEN_TARGET_ARCH which override the information from uname.  The pvshim
build runs out of the tools/ directory but calls the hypervisor build
system.

If one runs in a Linux 32-bit userland with a 64-bit kernel, one used
to be able to set XEN_COMPILE_ARCH.  But nowadays this does not work.
configure sees the target cpu as 64-bit and tries to build pvshim.
The build prints
  echo "*** Xen x86/32 target no longer supported!"
and doesn't build anything.  Then the subsequent Makefiles try to
install the non-built pieces.

Fix this anomaly by causing configure to honour the Xen hypervisor way
of setting the target architecture.

In principle this user behaviour is not handled quite right, because
configure will still see 64-bit and so all the autoconf-based
architecture testing will see 64-bit rather than 32-bit x86.  But the
tools are in fact generally quite portable: this particular location
in configure{.ac,} is the only place in tools/ where 64-bit x86 is
treated differently from 32-bit x86, so the fix is sufficient and
correct for this use case.

It remains the case that XEN_COMPILE_ARCH or XEN_TARGET_ARCH to a
non-x86 architecture, when configure thinks things are x86, or vice
versa, will not work right.

(This is a bugfix to 8845155c83
  pvshim: make PV shim build selectable from configure
which inadvertantly deleted the logic to only build the shim for
XEN_TARGET_ARCH != x86_32.)

I have rerun autogen.sh, so this patch contains the fix to configure
as well as the source fix to configure.ac.

Fixes: 8845155c83
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Olaf Hering <olaf@aepfle.de>
CC: Roger Pau Monné <roger.pau@citrix.com>
Release-acked-by: Jürgen Groß <jgross@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
2019-11-20 10:11:16 +00:00
Oleksandr Grytsov 0273d8e242 libxl: gentypes: initialise array elements in json
Currently, array elements are initialized with calloc.  Which means
initialize all element fields with zero values.  If an entry is not
present in the json (which is entirely permitted), the element will be
all-bits-zero instead of the default value (which is wrong).

The fix is to initalise each array element before parsing it, using
the new libxl_C_type_do_init function.

With existing types this results in a lot of new calls like this:

      for (i=0; (t=libxl__json_array_get(x,i)); i++) {
 +            libxl_sched_params_init(&p->vcpus[i]);
              rc = libxl__sched_params_parse_json(gc, t, &p->vcpus[i]);

(indentation adjusted).  This looks right.  To check what happens with
types which have nontrivial defaults but don't have init functions (of
which we currently have none in arrays), I (Ian) experimentally added:

      ("pnode", uint32), # physical node of this node
      ("vcpus", libxl_bitmap), # vcpus in this node
 +    ("sporks", Array(MemKB, "num_sporks")),
      ])

The result was this:

          for (i=0; (t=libxl__json_array_get(x,i)); i++) {
 +                p->sporks[i] = LIBXL_MEMKB_DEFAULT;
                  rc = libxl__uint64_parse_json(gc, t, &p->sporks[i]);

where the context was added by adding "sporks" and "+" indicates a
line added by this patch, "initialise array elements in json".

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
---
v2 [iwj]: Use libxl_C_type_do_init.
          Reword commit message and discuss spork testing.
2019-11-19 16:44:33 +00:00
Ian Jackson f710b76911 libxl: gentypes.py: Break out libxl_C_type_do_init
This is going to be the common way to initialise things.
_libxl_C_type_init remains the thing for generating the body of the
init function, and for some special cases.

No functional change with existing types: C output is identical.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2019-11-19 16:34:15 +00:00
Ian Jackson dde31350cb libxl: gentypes.py: Break out field_pass in ..._copy_deprecated
We are going to want this in a moment.

No functional change with existing types: C output is identical.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2019-11-19 16:33:56 +00:00
Ian Jackson 3afbd233c9 tools/libxl: gentypes.py: Prefer init_val to init_fn
When both are provided, init_val is likely to be more direct.

No functional change with existing types: C output is identical.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2019-11-19 16:33:43 +00:00
Anthony PERARD e28eed55a4 libxl_pci: Don't hold QMP connection while waiting
After sending the 'device_del' command for a PCI passthrough device,
we wait until QEMU has effectively deleted the device, this involves
executing more QMP commands. While waiting, libxl hold the connection.

It isn't necessary to hold the connection and it prevents others from
making progress, so this patch releases the QMP connection.

For background:
    e.g., when a guest is created with several pci passthrough
    attached, on `xl destroy` all the devices needs to be detach, and
    this is usually what happens:
	- 'device_del' called for the 1st pci device
	- 'query-pci' checking if pci still there, it is
	- wait 1s
	- 'query-pci' checking again, and it's gone
	-> now the same can be done for the second pci device, so
	plenty of waiting on others when pci detach can be done in
	parallel.

    On shutdown, libxl usually keeps waiting because QEMU never
    releases the device because the guest kernel never responds QEMU's
    unplug queries. So detaching of the 1st device waits until a
    timeout stops it, and since the same timeout is setup at the same
    time for the other devices to detach, the 'device_del' command is
    never sent for those.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 23:01:10 +00:00
Anthony PERARD 5a870b02cb libxl_qmp: Have a lock for QMP socket access
This patch workaround the fact that it's not possible to connect
multiple time to a single QMP socket. QEMU listen on the socket with
a backlog value of 1, which mean that on Linux when concurrent thread
call connect() on the socket, they get EAGAIN.

Background:
    This happens when attempting to create a guest with multiple
    pci devices passthrough, libxl creates one connection per device to
    attach and execute connect() on all at once before any single
    connection has finished.

To work around this, we use a new lock.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 22:59:51 +00:00
Anthony PERARD f3e4fb552a libxl: Introduce libxl__ev_immediate
This new ev allows to arrange a non-reentrant callback to be called.
This happen immediately after the current event is processed and after
other ev_immediates that would have already been registered.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 22:58:48 +00:00
Anthony PERARD 66b97659d9 libxl: libxl__ev_qmp_send now takes an egc
No functionnal changes.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 22:58:44 +00:00
Anthony PERARD 31c16a86ad libxl: Introduce libxl__ev_slowlock_dispose
Which allow to cancel the lock operation while it is in Active state.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 22:58:40 +00:00
Anthony PERARD 5f7e950d58 libxl: Rename ev_devlock to ev_slowlock
We are going to introduce a different lock based on the same
implementation as the ev_devlock but with a different path. The
different slowlock will be differentiated by calling different _init()
functions.

So we rename libxl__ev_devlock to lib__ev_slowlock, but keep
libxl__ev_devlock_init().

Some log messages produced ev_slowlock are changed to print the
name of the lock file (userdata_userid).

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 22:58:35 +00:00
Anthony PERARD e7c3202478 libxl: Move libxl__ev_devlock declaration
We are going to want to include libxl__ev_devlock into libxl__ev_qmp.

No functional changes.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 22:58:31 +00:00
Anthony PERARD 4abbac194f libxl: Introduce libxl__ev_child_kill_deregister
Allow to deregister the callback associated with a child death event.

The death isn't immediate will need to be collected later, so the
ev_child machinery register its own callback.

libxl__ev_child_kill_deregister() might be called by an AO operation
that is finishing/cleaning up without a chance for libxl to be
notified of the child death (via SIGCHLD). So it is possible that the
application calls libxl_ctx_free() while there are still child around.
To avoid the application getting unexpected SIGCHLD, the libxl__ao
responsible for killing a child will have to wait until it has been
properly reaped.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-18 22:58:26 +00:00
Juergen Gross f43afb0790 libxl: fix device model timeout in libxl__dm_resume()
libxl__dm_resume() is using a wrong timeout for the start of the
device model. Instead of 60 seconds the timeout is set to 60
milliseconds.

Reported-by: Roman Shaposhnik <roman@zededa.com>
Fixes: 6298f0eb8f ("libxl: Re-introduce libxl__domain_resume")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-15 10:37:44 +00:00
George Dunlap 7b4c3d0443 golang/xenlight: Fix libxl_domain_shutdown and libxl_domain_reboot as well
Both are now potentially asynchronous; pass in 'nil' to retain
synchronous behavior.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-15 09:36:58 +00:00
Anthony PERARD 8f1d6c049c libxl: Fix libxl_retrieve_domain_configuration error path
If an error were to happen before the last step, for example the
domain_configuration is missing, the error wouldn't be checked by the
_end callback.

Fix that, also initialise `lock' to NULL because the exit path checks
it.

The issue shows up when there's a stubdom, and running `xl list -l`
aborts. Instead, with this patch, `xl list -l` will not list stubdom,
probably like before.

Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Fixes: 6156341925
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-12 15:31:41 +00:00
Paul Durrant 3683290fc0 tools/hotplug: only attempt to call 'ip route' if there is valid command
The vif-route script should only call 'ip route' when 'ipcmd' has been
set, otherwise it will fail due to an incorrect command string.

This patch also adds routes for 'tap' (i.e. emulated) devices as well as
'vif' (i.e. PV) devices. Empirically offline/online commands relate to
'vif' devices, and add/remove commands relate to 'tap' devices. However,
this patch treats them equally and uses ${type_if} to distinguish. By
adding cases for add/remove the command list becomes exhaustive and hence
'ipcmd' is guaranteed to be set.

Routes for 'tap' and 'vif' devices are distinguished by a route metric.
Emulated devices are used by HVM guests until they are unplugged, at which
point the PV device becomes active. Thus 'tap' devices should get a higher
priority (i.e. lower numbered) metric than 'vif' devices.

There is also one small whitespace fix.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-12 10:40:05 +00:00
Anthony PERARD cda8f7e857 libxl: Fix setting vncpasswd to empty string
Before 93dcc22, error from setting the vnc password to an empty
string, when QEMU wasn't expected a password, never prevented the creation
of a guest, and only logged an error message.

Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Fixes: 93dcc22fe7
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-12 10:39:01 +00:00
Andrew Cooper 0c2a55089b tools: Fix local variable block
c-indent-level isn't considered a safe variable, and "solaris" isn't a
recognised C style.  Both cause prompts when opening the files.

Fix all blocks up per CODING_STYLE

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-11-12 10:39:01 +00:00
Paul Durrant 319f9a0ba9 passthrough: quarantine PCI devices
When a PCI device is assigned to an untrusted domain, it is possible for
that domain to program the device to DMA to an arbitrary address. The
IOMMU is used to protect the host from malicious DMA by making sure that
the device addresses can only target memory assigned to the guest. However,
when the guest domain is torn down the device is assigned back to dom0,
thus allowing any in-flight DMA to potentially target critical host data.

This patch introduces a 'quarantine' for PCI devices using dom_io. When
the toolstack makes a device assignable (by binding it to pciback), it
will now also assign it to DOMID_IO and the device will only be assigned
back to dom0 when the device is made unassignable again. Whilst device is
assignable it will only ever transfer between dom_io and guest domains.
dom_io is actually only used as a sentinel domain for quarantining purposes;
it is not configured with any IOMMU mappings. Assignment to dom_io simply
means that the device's initiator (requestor) identifier is not present in
the IOMMU's device table and thus any DMA transactions issued will be
terminated with a fault condition.

In addition, a fix to assignment handling is made for VT-d.  Failure
during the assignment step should not lead to a device still being
associated with its prior owner. Hand the device to DomIO temporarily,
until the assignment step has completed successfully.  Remove the PI
hooks from the source domain then earlier as well.

Failure of the recovery reassign_device_ownership() may not go silent:
There e.g. may still be left over RMRR mappings in the domain assignment
to which has failed, and hence we can't allow that domain to continue
executing.

NOTE: This patch also includes one printk() cleanup; the
      "XEN_DOMCTL_assign_device: " tag is dropped in iommu_do_pci_domctl(),
      since similar printk()-s elsewhere also don't log such a tag.

This is XSA-302.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-10-31 16:20:05 +01:00
Petre Pircalabu 368375d736 tools/ocaml: Fix build error with Arch Linux
gcc (GCC) 9.2.0 complains:

xentoollog_stubs.c: In function ‘stub_xtl_ocaml_vmessage’:
xentoollog_stubs.c:93:16: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
   93 |  value *func = caml_named_value(xtl->vmessage_cb) ;
      |                ^~~~~~~~~~~~~~~~

This patch constifies the pointer returned by caml_named_value in order
to the accommodate newer versions of OCaml.
In OCaml >= 4.09 the return value pointer of caml_named_value is
declared const.

https://github.com/ocaml/ocaml/commit/4f03a1467d29cf587df5a191830f1525506ee0e3

Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
[ wei: remove spaces before semicolons ]
Signed-off-by: Wei Liu <wl@xen.org>
2019-10-29 12:54:10 +00:00
Ian Jackson 2a474dc3e2 libxl: libxl__spawn_stub_dm: Call domain_config_setdefault
Previously, defaulting and checking of some aspects of the domain
config was skipped for stub dms.  This has been the case forever.

In ad011ad088 "libxl/xl: Overhaul passthrough setting logic" some
defaulting that was needed for stub dms was moved from
libxl__domain_create_info_setdefault to .._config_setdefault with the
result that for stub dms, libxl__domain_make fails with this
assertion:
  xl: libxl_create.c:582: libxl__domain_make: Assertion
  `info->passthrough != LIBXL_PASSTHROUGH_DEFAULT' failed.

Fix this by properly doing all defaulting and all checking for stub
dms.  This is more correct, but (especially at this stage of the
release) it is necessary to more closely evaluate the effects by
reviewing the body of _config_setdefault.  The changes are as follows:

One actual functional change:

* The new passthrough defaulting is properly done.  This is what we
  are trying to actually fix here.

And a lot of things that make no difference:

* shadow_memkb would now be set.  Whether this would be correct is not
  entirely clear.  It seems better to make this patch (whose purpose
  is to fix the passthrough defaulting) *not* include that semantic
  change, so here I have included a hunk to explicitly override this.

* FLASK ssid_label is processed.  But the actual ssidref is copied
  from the guest domain by spawn_stub_dm, and ssid_label is set to
  NULL.  So no change.

* We set iommu_memkb.  But to 0 since passthrough is disabled.

* cpuid pool_name is processed.  But this is not set by
  spawn_stub_dm.  (Arguably this is a bug: stub dms should inherit the
  parent cpupool.)  The effect is to leave poolid set to 0 and call
  libxl_cpupoolid_is_valid but that always succeeds for 0.  So no
  change.

* Various extra checks are done: reject PCI passthrough for HVM with
  POD (stub dm is PV); reject pod + vnuma, or PV + vnuma (stub dm has
  no vnuma); reject nested HVM or pod, with alt2pm-hvm (again, stub dm
  is PV).  So these checks will always pass.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2019-10-28 18:09:24 +00:00
Ian Jackson 7d2655f206 libxl: domain_config_setdefault: Document use of domid
We are going to want to call this from a site which has a domid which
is good for logging but not the domid of the domain we are creating
(namely, the stub device domain).

Consequently, add the same comment to
libxl__arch_passthrough_mode_setdefault.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2019-10-28 18:09:24 +00:00
Ian Jackson 333d741279 libxl: On ARM, reject future new passthrough modes too
This is most pleasantly done by also changing the if to a switch.

Suggested-by: Julien Grall <julien@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2019-10-24 16:08:41 +01:00
Ian Jackson ad011ad088 libxl/xl: Overhaul passthrough setting logic
LIBXL_PASSTHROUGH_UNKNOWN (aka "ENABLED" in an earlier uncommitted
version of this code) is doing double duty.  We actually need all of
the following to be specifiable:
  * "default": enable PT iff we have devices to
    pass through specified in the initial config file.
  * "enabled" (and fail if the platform doesn't support it).
  * "disabled" (and reject future PT hotplug).
  * "share_pt"/"sync_pt": enable PT and set a specific PT mode.

Defaulting and error checking should be done in libxl.  So, we make
several changes here.

We introduce "enabled", and rename "unknown" to "default".

We move all of the error checking and defaulting code from xl into
libxl.  Now, libxl__domain_config_setdefault has all of the necessary
information to get this right.  So we can do it all there.  Choosing
the specific mode is arch-specific.

We can also arrange to have only one place each which calculates
(i) whether passthrough needs to be enabled because pt devices were
specified (ii) whether pt_share can be used (for each arch).

xl now only has to parse the enum in the same way as it parses all
other enums.

This change fixes a regression from earlier 4.13-pre: until recent
changes, passthrough was only enabled by default if passthrough
devices were specified.  We restore this behaviour.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Andrew Cooper <Andrew.Cooper3@citrix.com>
CC: Paul Durrant <pdurrant@gmail.com>
CC: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2019-10-24 16:08:41 +01:00
Ian Jackson 95596f6ab1 libxl: Move domain_create_info_setdefault earlier
We need this before we start to figure out the passthrough mode.

I have checked that nothing in libxl__domain_create_info_setdefault
nor the two implementations of ..._arch_... accesses anything else,
other than (i) the domain type (which this function is responsible for
setting and nothing before it looks at) (ii) c_info->ssidref (which is
defaulted by flask code near the top of
libxl__domain_config_setdefault and not accessed afterwards).

So no functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson 5f135a65d2 libxl: create: setdefault: Move physinfo into config_setdefault
No functional change.  This will let us refer to it in code we are
about to add to this function.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson af5c475dee libxl: create: setdefault: Make libxl_physinfo info[1]
No functional change.  This will let us make it into a pointer without
textual change other than to the definition.

While we are here, fix some style errors (missing { }).

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson 5dedc18d64 libxl: Remove/deprecate libxl_get_required_*_memory from the API
These are now redundant because shadow_memkb and iommu_memkb are now
defaulted automatically by libxl_domain_need_memory and
libxl_domain_create etc.  Callers should not now call these; instead,
they should just let libxl take care of it.

libxl_get_required_shadow_memory was introduced in f89f555827
  "remove late (on-demand) construction of IOMMU page tables"
We can freely remove it because it was never in any release.

libxl_get_required_shadow_memory has been in libxl approximately
forever.  It should probably not have survived the creation of
libxl_domain_create, but it seems the API awkwardnesses we see in
recent commits prevented this.  So we have to keep it.  It remains
functional but we can deprecate it.  Hopefully we can get rid of it
completely before we find the need to change the calculation to use
additional information which its arguments do not currently supply.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson 67c82f437e libxl: Move shadow_memkb and iommu_memkb defaulting into libxl
Defaulting is supposed to be done by libxl.  So these calculations
should be here in libxl.  libxl__domain_config_setdefault has all the
necessary information including the values of max_memkb and max_vcpus.

The overall functional effect depends on the caller:

For xl, no change.  The code moves from xl to libxl.

For callers who set one or both shadow_memkb and iommu_memkb (whether
from libxl_get_required_shadow_memory or otherwise) before calling
libxl_domain_need_memory (any version): the new code will leave their
setting(s) unchanged.

For callers who do not call libxl_domain_need_memory at all, and who
fail to set one of these memory values: now they are both are properly
set.  The shadow and iommu memory to be properly accounted for as
intended.

For callers which call libxl_domain_need_memory and request the
current API (4.13) or which track libxl, the default values are also
now right and everything works as intended.

For callers which call libxl_domain_need_memory, and request an old
pre-4.13 libxl API, and which leave one of these memkb settings unset,
we take special measures to preserve the old behaviour.

This means that they don't get the additional iommu memory and are at
risk of the domain running out of memory as a result of f89f555827
"remove late (on-demand) construction of IOMMU page tables".  But this
is no worse than the state just after f89f555827, which already
broke such callers in that way.  This is perhaps justifiable because
of the API stability warning next to libxl_domain_need_memory.

An alternative would be to drop the special-casing of these callers.
That would cause a discrepancy between libxl_domain_need_memory and
libxl_domain_create: the former would not include the iommu memory and
the latter would.  That seems worse, but it's debateable.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson a9af7cd17d libxl: libxl_domain_need_memory: Make it take a domain_config
This should calculate the extra memory needed for shadow and iommu,
the defaults for which depend on values in c_info.  So we need this to
have the complete domain config available.

And the defaults should actually be updated and stored.  So make it
non-const.

We provide the usual kind of compatibility function for callers
expecting 4.12 and earlier.  This function becomes responsible for the
clone-and-modify of the b_info.

No overall functional change for external libxl callers which use the
API version system to request a particular API version.

Other external libxl callers will need to update their calling code,
and will then find that the new version of this function fills in most
of the defaults in d_config.  Because libxl__domain_config_setdefault
doesn't quite do all of the defaults, that's only partial.  For
present purposes that doesn't matter because none of the missing
settings are used by the memory calculations.  It does mean we need to
document in the API spec that the defaulting is only partial.

This lack of functional change is despite the fact that
numa_place_domain now no longer calls
libxl__domain_build_info_setdefault (via libxl_domain_need_memory).
That is OK because it's idempotent and numa_place_domain's one call
site is libxl__build_pre which is called from libxl__domain_build
which is called from domcreate_bootloader_done, well after the
defaults are set by initiate_domain_create.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson 2541fcc34c libxl: libxl__domain_config_setdefault: New function
Break out this into a new function.  We are going to want to call it
from a new call site.

Unfortunately not all of the defaults can be moved into the new
function without changing the order in which things are done.  That
does not seem wise at this stage of the release.  The effect is that
additional calls to libxl__domain_config_setdefault (which are going
to be introduced) do not quite set everything.  But they will do what
is needed.  After Xen 4.13 is done, we should move those settings into
the right order.

No functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson 3f21bd4977 xl: Pass libxl_domain_config to freemem(), instead of b_info
We are going to change the libxl API in a moment and this change will
make it simpler.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Ian Jackson c3999835df libxl: Offer API versions 0x040700 and 0x040800
According to git log -G:

0x040700 was introduced in 304400459e (aka 4.7.0-rc1~481)
  "tools/libxl: rename remus device to checkpoint device"

0x040800 was introduced in 57f8b13c72 (aka 4.8.0-rc1~437)
  "libxl: memory size in kb requires 64 bit variable"

It is surprising that no-one noticed this.

Anyway, in the meantime, we should fix it.  Backporting this is
probably a good idea: it won't change the behaviour for existing
callers but it will avoid errors for some older correct uses.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-24 16:08:41 +01:00
Nick Rosbrook 228a0255c6 golang/xenlight: fix calls to libxl_domain_unpause/pause
These functions require a third argument of type const *libxl_asyncop_how.

Pass nil to fix compilation errors. This will have the effect of
performing these operations synchronously.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-22 15:06:59 +01:00
Marek Marczykowski-Górecki 15e9e36ddd libxl: don't try to manipulate json config for stubdomain
Stubdomain do not have it's own config file - its configuration is
derived from target domains. Do not try to manipulate it when attaching
PCI device.

This bug prevented starting HVM with stubdomain and PCI passthrough
device attached.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-04 17:04:51 +01:00
Marek Marczykowski-Górecki ac2726ecc1 libxl: attach PCI device to qemu only after setting pciback/pcifront
When qemu is running in stubdomain, handling "pci-ins" command will fail
if pcifront is not initialized already. Fix this by sending such command
only after confirming that pciback/front is running.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-10-04 17:04:51 +01:00