...to denote what it actually does.
The name libxl__xs_write() suggests something taking a buffer and length,
akin to write(2), whereas the semantics of the function are actually more
akin to printf(3).
This patch is a textual substitution of libxl__xs_write with
libxl__xs_printf with some associated formatting fixes.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
The rune used is:
sed -i 's/libxl__sprintf(gc,\s*/GCSPRINTF(/g' libxl*.c
This rune is simple and better than trying to match every possible
patterns.
Two instances in libxl_dm.c need fixing up. They are in fact better to just
use libxl__strdup.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
The spatch provided in previous patch didn't handle all sites that need
transformation.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
While working with qemu, IGD is a specific device in the case of pass through
so we need to identify that to handle more later. Here we define a table to
record all IGD types currently we can support. Also we need to introduce two
helper functions to get vendor and device ids to lookup that table.
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
While it appears to be intentional for "xl pci-assignable-remove" to
not re-bind the original driver by default (requires the -r option),
permanently losing the information which driver was originally used
seems bad. Make "add; remove; add; remove -r" re-bind the original
driver by allowing "remove" to delete the information only upon
successful re-bind.
In the course of this I also noticed that binding information is lost
when upon first "add" pciback isn't loaded yet, due to its presence not
being checked for early enough. Adjust pciback_dev_is_assigned()
accordingly, and properly distinguish "yes" and "error" returns in the
"add" case (removing a redundant error message from the "remove" path
for consistency).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This patch introduces user configurable parameters to specify RDM
resource and according policies,
Global RDM parameter:
rdm = "strategy=host,policy=strict/relaxed"
Per-device RDM parameter:
pci = [ 'sbdf, rdm_policy=strict/relaxed' ]
Global RDM parameter, "strategy", allows user to specify reserved regions
explicitly, Currently, using 'host' to include all reserved regions reported
on this platform which is good to handle hotplug scenario. In the future
this parameter may be further extended to allow specifying random regions,
e.g. even those belonging to another platform as a preparation for live
migration with passthrough devices. By default this isn't set so we don't
check all rdms. Instead, we just check rdm specific to a given device if
you're assigning this kind of device. Note this option is not recommended
unless you can make sure any conflict does exist.
'strict/relaxed' policy decides how to handle conflict when reserving RDM
regions in pfn space. If conflict exists, 'strict' means an immediate error
so VM can't keep running, while 'relaxed' allows moving forward with a
warning message thrown out.
Default per-device RDM policy is same as default global RDM policy as being
'relaxed'. And the per-device policy would override the global policy like
others.
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Checked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Convert all plain ints and strings which are used for xenbus "state"
files to xenbus_state. This makes it easier to find code which deals
with backend/frontend state changes.
Convert usage of libxl__sprintf to GCSPRINTF.
No change in behaviour is expected by this change, beside a small
increase of runtime memory usage in places that used a string constant.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Originally both QEMU traditional and QEMU upstream used hardcoded
/local/domain/0 paths. This patch changes the protocol to use
/local/domain/$dm_domid path.
For QEMU traditional and upstream without stubdom, $dm_domid is 0 so
the path is in fact still /local/domain/0.
For QEMU traditional stubdom, this is incompatible protocol change.
However QEMU traditional is shipped with Xen so we are allowed to do
such change. This change requires to corresponding QEMU traditional
changeset.
There is no compatibility issue with QEMU upstream stubdom, because QEMU
upstream stubdom doesn't exist yet.
Watch /local/domain/$dm_domid/device-model/$domid/state, wait until
state turns "running" then unpause guest.
LIBXL_STUBDOM_START_TIMEOUT is the timeout used wait for stubdom to be
ready. My test on a very old machine (Core 2 6400) showed that it might
need more than 20s before the stubdom is ready to serve DomU.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This reverts commit 54c2e621c5 which
should have included a corresponding QEMU_TAG update but did not.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Originally both QEMU traditional and QEMU upstream used hardcoded
/local/domain/0 paths. This patch changes the protocol to use
/local/domain/$dm_domid path.
For QEMU traditional and upstream without stubdom, $dm_domid is 0 so
the path is in fact still /local/domain/0.
For QEMU traditional stubdom, this is incompatible protocol change.
However QEMU traditional is shipped with Xen so we are allowed to do
such change. This change needs to work with corresponding QEMU
traditional changeset.
There is no compatibility issue with QEMU upstream stubdom, because QEMU
upstream stubdom doesn't exist yet.
Watch /local/domain/$dm_domid/device-model/$domid/state, wait until
state turns "running" then unpause guest.
LIBXL_STUBDOM_START_TIMEOUT is the timeout used wait for stubdom to be
ready. My test on a very old machine (Core 2 6400) showed that it might
need more than 20s before the stubdom is ready to serve DomU.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
On do_pci_remove when QEMU returns error, we just bail out early without
resetting the device. On domain shutdown we are racing with QEMU exiting
and most often QEMU closes the QMP connection before executing the
requested command.
In these cases if force=1, it makes sense to go ahead with rest of the
PCI device removal, that includes resetting the device and calling
xc_deassign_device. Otherwise we risk not resetting the device properly
on domain shutdown.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
do_pci_remove contained this:
if (type == LIBXL_DOMAIN_TYPE_HVM) {
[stuff]
} else if (type != LIBXL_DOMAIN_TYPE_PV)
abort();
{
This is bizarre, and not correct. The effect is that HVM guests end
up running both the proper code and that intended for PV guests. This
causes (amongst other things) trouble when PCI devices are
hot-unplugged from HVM guests.
This bug was introduced in abfb006f "tools/libxl: explicitly grant
access to needed I/O-memory ranges".
This is clear candidate for Xen 4.5, being a bugfix to an important
feature.
Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: Robert Hu <robert.hu@intel.com>
Rlease-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Sander Eikelenboom <linux@eikelenboom.it>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
The existence check is to make sure a device is not added to a guest
multiple times.
PCI device backend path has different rules from vif, disk etc. For
example:
/local/domain/0/backend/pci/9/0/dev-1/0000:03:10.1
/local/domain/0/backend/pci/9/0/key-1/0000:03:10.1
/local/domain/0/backend/pci/9/0/dev-2/0000:03:10.2
/local/domain/0/backend/pci/9/0/key-2/0000:03:10.2
The devid for PCI devices is hardcoded 0. libxl__device_exists only
checks up to /local/.../9/0 so it always returns true even the device is
assignable.
Remove invocation of libxl__device_exists. We're sure at this point that
the PCI device is assignable (hence no xenstore entry or JSON entry).
The check is done before hand. For HVM guest it's done by calling
xc_test_assign_device and for PV guest it's done by calling
pciback_dev_is_assigned.
Reported-by: Li, Liang Z <liang.z.li@intel.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
We update JSON version first, then write to xenstore, so that we
maintain the following invariant: any device which is present in
xenstore has a corresponding entry in JSON.
The workflow is as followed:
lock json config
read json config
update in-memory json config with new entry, replacing
any stale entry
for loop
open xs transaction
check device existence, abort if it exists
write in-memory json config to disk
commit xs transaction
end for loop
unlock json config
Please see comment in libxl_internal.h for correctness proof.
As those routines are called both during domain creation and device
hotplug, we add a flag to indicate whether we need to update JSON
config. This flag is only set to true when we hotplug a device. We
cannot update JSON config during domain creation as JSON config is
committed to disk only when domain creation finishes.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This commit changes the existing libxl code to be sure to grant access
permission to PCI-related I/O memory ranges, while setting up passthrough
of PCI devices specified in the domain's configuration, and to VGA-related
memory ranges, while setting up VGA passthrough (if gfx_passthru = 1 in
the domain's configuration).
As for the latter, the newly-added code does not replace any existing one,
but instead matches the calls to xc_domain_memory_mapping() performed by
QEMU on the path that is executed if gfx passthru is enabled and follows
the registration of a new VGA controller (in register_vga_regions(),
defined in hw/pt-graphics.c). In fact, VGA needs some extra memory
ranges to be mapped with respect to PCI; QEMU expects that access to those
memory ranges is implicitly granted when he calls the hypervisor with the
function xc_domain_memory_mapping(): this commit calls iomem_permission
for it when needed by checking the passthru PCI device's class.
NOTE: the code added by this commit still does not verify if the passthru
of the framebuffer area is being performed for the primary GPU, but
only replicates the behavior of QEMU which is limited to performing
the passthru for all PCI devices of VGA class.
This commit is instrumental to the last one in the series, which will
separate the functions of the iomem_permission and memory_mapping DOMCTLs,
so that requesting an I/O-memory range will not imply that access to such
a range is implicitly granted.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
Originally the code allowed users to attach the same device more than
once. It just stupidly overwrites xenstore entries. This is bogus as
frontend will be very confused.
Introduce a helper function to check if the device to be written to
xenstore already exists. A new error code is also introduced.
The check and add are within one xs transaction.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Commit f74035d3b1 "xl: PCI code cleanups"
introduced an regression where it changed the name from:
/sys/bus/pci/drivers/pciback/do_flr
to
libxl_sprintf(ctx, "%s/pciback/do_flr", SYSFS_PCI_DEV);
And SYSFS_PCI_DEV is "/sys/bus/pci/devices", meaning we would
do /sys/bus/pci/devices/pciback/do_flr. The proper define
should have been SYSFS_PCIBACK_DRIVER, which is what this patch does.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
The "seize" option tells the toolstack to attempt to automatically
unbind devices and re-bind them to the pciback driver. This should
make creating VMs that habitually use pass-through (such as driver domain
VMs and gaming VMs) easier to use and manage.
[Whitespace error fixed by iwj.]
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
It could, even if only in theory, be fd 0.
(This is not the same as commit 4b62556b57!)
Coverity-ID: 1055895
Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
libxl__wait_for_device_model blocks, with the ctx lock held, waiting
for a response from the device model. If the dm doesn't respond
quickly (for example, because it has crashed), this may block the
whole process. Explain this in a comment, rename the function to
libxl__wait_for_device_model_deprecated, and explain what to use
instead.
libxl__wait_for_offspring is the core implementation for the above.
Its name leads people to think it might be generally useful for
waiting for children, which is far from the case. It only waits for
xenstore. Also it has the problems described above. Explain this,
rename it to libxl__xenstore_child_wait_deprecated, and explain what
to use instead.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
This in effect copies similar logic from xend: While there's no way to
check whether a device is assigned to a particular guest,
XEN_DOMCTL_test_assign_device at least allows checking whether an
IOMMU is there and whether a device has been assign to _some_
guest.
For the time being, this should be enough to cover for the missing
error checking/recovery in other parts of libxl's device assignment
paths.
There remains a (functionality-, but not security-related) race in
that the iommu should be set up earlier, but this is too risky a
change for this stage of the 4.3 release.
This is a security issue, XSA-61.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Matthew Daley has observed that the PV console protocol places sensitive host
state into a guest writeable xenstore locations, this includes:
- The pty used to communicate between the console backend daemon and its
client, allowing the guest administrator to read and write arbitrary host
files.
- The output file, allowing the guest administrator to write arbitrary host
files or to target arbitrary qemu chardevs which include sockets, udp, ptr,
pipes etc (see -chardev in qemu(1) for a more complete list).
- The maximum buffer size, allowing the guest administrator to consume more
resources than the host administrator has configured.
- The backend to use (qemu vs xenconsoled), potentially allowing the guest
administrator to confuse host software.
So we arrange to make the sensitive keys in the xenstore frontend directory
read only for the guest. This is safe since the xenstore permissions model,
unlike POSIX directory permissions, does not allow the guest to remove and
recreate a node if it has write access to the containing directory.
There are a few associated wrinkles:
- The primary PV console is "special". It's xenstore node is not under the
usual /devices/ subtree and it does not use the customary xenstore state
machine protocol. Unfortunately its directory is used for other things,
including the vnc-port node, which we do not want the guest to be able to
write to. Rather than trying to track down all the possible secondary uses
of this directory just make it r/o to the guest. All newly created
subdirectories inherit these permissions and so are now safe by default.
- The other serial consoles do use the customary xenstore state machine and
therefore need write access to at least the "protocol" and "state" nodes,
however they may also want to use arbitrary "feature-foo" nodes (although
I'm not aware of any) and therefore we cannot simply lock down the entire
frontend directory. Instead we add support to libxl__device_generic_add for
frontend keys which are explicitly read only and use that to lock down the
sensitive keys.
- Minios' console frontend wants to write the "type" node, which it has no
business doing since this is a host/toolstack level decision. This fails
now that the node has become read only to the PV guest. Since the toolstack
already writes this node just remove the attempt to set it.
This is a security issue, XSA-57.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
This patch makes the flexarray function libxl__gc aware.
It also updates every function that use a flexarray to pass the gc and removes
every memory allocation check and free.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
It was convenient to invent $(CFLAGS_LIBXL) to do this.
Various renamings to avoid shadowing standard functions:
- index(3)
- listen(2)
- link(2)
- abort(3)
- abs(3)
Reduced the scope of some variables to avoid conflicts.
Change to libxc is due to the nested hypercall buf macros in
set_xen_guest_handle (used in libxl) using the same local private vars.
Build tested only.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
This does not make the implementation fully asynchronous but just
updates the API to support asynchrony in the future.
Currently although these functions do not call hotplug scripts etc and
therefore are not "slow" (per the comment about ao machinery in
libxl_internal.h) they do interact with the device model and so are
not quite "fast" either. We can live with this for now.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
* Abolish the macro LIBXL__DOMAIN_IS_TYPE which had incorrect error
handling. At every call site, replace it with an open-coded call to
libxl_domain_type and check against LIBXL_DOMAIN_TYPE_INVALID.
* This involves adding an `out:' to libxl_domain_unpause.
* In libxl_domain_destroy and do_pci_add, do not `default: abort();'
if the domain type cannot be found. Instead switch on
LIBXL_DOMAIN_TYPE_INVALID specifically and do some actual error
handling.
* In libxl__primary_console_find, remove a spurious default clause
from the domain type switch.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Add a xs_transaction_t parameter to libxl__device_generic_add, if it is
XBT_NULL, allocate a proper one.
Update all the callers.
This patch contains a large number of unchecked xenstore operations, we
might want to fix this in the future.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
Introduce libxl helper functions to prepare devices to be passed
through to guests. This is meant to replace of all the manual sysfs
commands which are currently required.
pci_assignable_add accepts a BDF for a device and will:
* Unbind a device from its current driver, if any
* If "rebind" is set, it will store the path of the driver from which we
unplugged it in /libxl/pciback/$BDF/driver_path
* If create a slot for it in pciback if one doesn't yet exist
* Bind the device to pciback
At this point it will show up in pci_assignable_list, and is ready to
be passed through to a guest.
pci_assignable_remove accepts a BDF for a device and will:
* Unbind the device from pciback
* Remove the slot from pciback
* If "rebind" is set, and /libx/pciback/$BDF/driver_path exists, it
will attempt to rebind the device to its original driver.
Both functions are idempotent: if the desired end state has already
been reached, they return SUCCESS.
NB that "$BDF" in this case uses '-' instead of ':' and '.', because
':' and '.' are illegal characters in xenstore paths.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
...to prepare for a consistent "pci_assignable_*" naming scheme.
Also move the man page entry into the PCI PASS-THROUGH section, rather
than the XEN HOST section.
No functional changes.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
This patch makes sure the passthrough device belongs to pciback before
allow them passthrough to the guest. There are still many other
checks missing.
xm terminates the guest startup process when this type of condition is
found. This patch just allows the guest to continue to boot but with
no device passthrough.
Signed-off-by: Allen Kay <allen.m.kay@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
By default pciback only allows PV guests to write "known safe" values into
PCI config space. But many devices require writes to other areas of config
space in order to operate properly. One way to do that is with the "quirks"
interface, which specifies areas known safe to a particular device; the
other way is to mark a device as "permissive", which tells pciback to allow
all config space writes for that domain and device.
This adds a "permissive" flag to the libxl_pci struct and teaches libxl how
to write the appropriate value into sysfs to enable the permissive feature for
devices being passed through. It also adds the permissive config options either
on a per-device basis, or as a global option in the xl command-line.
Because of the potential stability and security implications of enabling
permissive, the flag is left off by default.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Config parsing functions do not properly belong in libxl. Move them into
libxlu so that others can use them or not as they see fit.
No functional changes. One side-effect was making public a private libxl
utility function which just set the elements of a structure from the function
arguments passed in.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
The e820_sanitize() function in libxl_pci.c expects one of its arguments to
be non-zero; but since a recent changeset, it can typically expect *to be*
zero. Since the zero case is handled properly, just remove the check.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
This allows them to be set via the _init/_setdefault methods.
This just covers the obvious ones.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Introduce a helper function to send commands to traditional
qemu. qemu_pci_add_xenstore, qemu_pci_remove_xenstore,
libxl__domain_save_device_model and libxl_domain_unpause have
been refactored to use this function.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>