Commit Graph
14526 Commits
Author SHA1 Message Date
Andrew Cooper fedd1f6f41 tools/libxl: Simplify callback handling in libxl-save-helper
The {save,restore}_callback helpers can have their scope reduced vastly, and
helper_setcallbacks_{save,restore}() doesn't need to use a ternary operator to
write 0 (meaning NULL) into an already zeroed structure.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-27 18:56:24 +00:00
Andrew Cooper 78a9ae2399 x86/gen-cpuid: Create max and default variations of INIT_*_FEATURES
For now, write the same content for both.  Update the users of the
initialisers to use the new name, and extend xen-cpuid to dump both default
and max featuresets.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-02-27 18:54:58 +00:00
Andrew Cooper 90f5b8819c tools/libxc: Simplify xc_get_static_cpu_featuremask()
Drop XC_FEATUREMASK_DEEP_FEATURES.  It isn't used by any callers, and unlike
the other static masks, won't be of interest to anyone without other pieces of
cpuid-autogen.h

In xc_get_static_cpu_featuremask(), use a 2d array instead of individually
named variables, and drop the switch statement completely.

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-02-27 18:54:58 +00:00
Andrew Cooper 625a95cc17 x86/sysctl: Don't return cpu policy data for compiled-out support (2)
Just as with c/s 96dc77b4b1 for XEN_SYSCTL_get_cpu_policy,
XEN_SYSCTL_get_cpu_featureset wants to become conditional.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-02-27 18:54:58 +00:00
Jan Beulich 5f29f34981 libxl/PCI: align reserved device memory boundary for HAP guests
As the code comment says, this will allow use of a 2Mb super page
mapping at the end of "low" memory.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-27 15:45:31 +01:00
Jan Beulich fc7b56510d libxl/PCI: pass correct "hotplug" argument to libxl__device_pci_setdefault()
Uniformly passing "false" can't be right, but has been benign because of
the function not using this parameter.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-27 15:45:05 +01:00
Jan Beulich df04e202c9 libxl/PCI: make "rdm=" parsing comply with documentation
Documentation says "<RDM_RESERVATION_STRING> is a comma separated list
of <KEY=VALUE> settings, from the following list". There's no mention
of a specific order, yet so far the parsing logic did accept only
strategy, then policy (and neither of the two omitted). Make "state"
move
- back to STATE_TYPE when finding a comma after having parsed the
  <VALUE> part of a setting,
- to STATE_TERMINAL otherwise.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-27 15:44:41 +01:00
Jan Beulich 6fdb8eef87 libxl/PCI: establish per-device reserved memory policy earlier
Reserved device memory region processing as well as E820 table creation
happen earlier than the adding of (PCI) devices, yet they consume the
policy (e.g. to decide whether to add entries to the E820 table). But so
far it was only at the stage of PCI device addition that the final
policy was established (i.e. if not explicitly specified by the guest
config file).

Note that I couldn't find the domain ID to be available in
libxl__domain_device_construct_rdm(), but observing that
libxl__device_pci_setdefault() also doesn't use it, for the time being
DOMID_INVALID gets passed. An obvious alternative would be to drop the
unused parameter/argument, yet at that time the question would be
whether to also drop other unused ones.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-27 15:44:17 +01:00
Jan Beulich 3bdb073643 libxl/PCI: honor multiple per-device reserved memory regions
While in "host" strategy all regions get processed, of the per-device
ones only the first entry has been consumed so far.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-27 15:43:55 +01:00
Paul Durrant d7d751bfdb libxl: add initializers for libxl__domid_history
This patch fixes Coverity issue CID 1459006 (Insecure data handling
(INTEGER_OVERFLOW)).

The problem is that the error paths for libxl__mark_domid_recent() and
libxl__is_domid_recent() check the 'f' field in struct libxl__domid_history
when it may not have been initialized.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-26 21:48:39 +00:00
Andrew Cooper 96dc77b4b1 x86/sysctl: Don't return cpu policy data for compiled-out support
Policy objects aren't tiny, and the derivation logic isn't trivial.  We are
about to increase the number of policy objects, so will have the opportunity
to drop logic and storage space based on CONFIG_{PV,HVM}.

Start by causing XEN_SYSCTL_get_cpu_policy to fail with -EOPNOTSUPP when
requesting data for a compiled-out subsystem.  Update xen-cpuid to cope and
continue to further system policies, seeing as the indicies are interleaved.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2020-02-26 12:13:10 +00:00
Paul Durrant 37a5b5f673 libxl: fix build with older glibc
Commit 2b02882ebb "libxl: add infrastructure to track and query
'recent' domids" added a call to clock_gettime() into libxl. The man-
page for this states:

"Link with -lrt (only for glibc versions before 2.17)."

Unfortunately Centos 6 does have an glibc prior to that version, and the
libxl Makefile was not updated to add '-lrt' so the build will fail in
that environment.

This patch simply adds '-lrt' to LIBXL_LIBS unconditionally, as it does
no harm in newer environments.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Fixes: 2b02882ebb ("libxl: add infrastructure to track and query 'recent' domids")
Acked-by: Wei Liu <wl@xen.org>
2020-02-25 14:28:59 +00:00
Paul Durrant e465fecbfd xl: allow domid to be preserved on save/restore or migrate
This patch adds a '-D' command line option to save and migrate to allow
the domain id to be incorporated into the saved domain configuration and
hence be preserved.

NOTE: Logically it may seem as though preservation of domid should be
      dealt with by libxl, but the libxl migration stream has no record
      in which to transfer domid and remote domain creation occurs before
      the migration stream is parsed. Hence this patch modifies xl rather
      then libxl.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-24 17:17:35 +00:00
Paul Durrant 64fe38f246 xl.conf: introduce 'domid_policy'
This patch adds a new global 'domid_policy' configuration option to decide
how domain id values are allocated for new domains. It may be set to one of
two values:

"xen", the default value, will cause an invalid domid value to be passed
to do_domain_create() preserving the existing behaviour of having Xen
choose the domid value during domain_create().

"random" will cause the special RANDOM_DOMID value to be passed to
do_domain_create() such that libxl__domain_make() will select a random
domid value.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-24 17:17:35 +00:00
Paul Durrant 4a3a25678d libxl: allow creation of domains with a specified or random domid
This patch adds a 'domid' field to libxl_domain_create_info and then
modifies libxl__domain_make() to have Xen use that value if it is valid.
If the domid value is invalid then Xen will choose the domid, as before,
unless the value is the new special RANDOM_DOMID value added to the API.
This value instructs libxl__domain_make() to choose a random domid value
for Xen to use.

If Xen determines that a domid specified to or chosen by
libxl__domain_make() co-incides with an existing domain then the create
operation will fail. In this case, if RANDOM_DOMID was specified to
libxl__domain_make() then a new random value will be chosen and the create
operation will be re-tried, otherwise libxl__domain_make() will fail.

After Xen has successfully created a new domain, libxl__domain_make() will
check whether its domid matches any recently used domid values. If it does
then the domain will be destroyed. If the domid used in creation was
specified to libxl__domain_make() then it will fail at this point,
otherwise the create operation will be re-tried with either a new random
or Xen-selected domid value.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-24 17:17:35 +00:00
Paul Durrant 2b02882ebb libxl: add infrastructure to track and query 'recent' domids
A domid is considered recent if the domain it represents was destroyed
less than a specified number of seconds ago. For debugging and/or testing
purposes the number can be set using the environment variable
LIBXL_DOMID_REUSE_TIMEOUT. If the variable does not exist then a default
value of 60s is used.

Whenever a domain is destroyed, a time-stamped record will be written into
a history file (/var/run/xen/domid-history). To avoid the history file
growing too large, any records with time-stamps that indicate that the
age of a domid has exceeded the re-use timeout will also be purged.

A new utility function, libxl__is_recent_domid(), has been added. This
function reads the same history file checking whether a specified domid
has a record that does not exceed the re-use timeout. Since this utility
function does not write to the file, no records are actually purged by it.

NOTE: The history file is purged on boot to it is safe to use
      CLOCK_MONOTONIC as a time source.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-24 17:17:35 +00:00
Andrew Cooper 1a16fee21c xen/public: Obsolete HVM_PARAM_PAE_ENABLED
Xen has never acted upon the value of HVM_PARAM_PAE_ENABLED, contrary perhaps
to expectations based on how other boolean fields work.

It was only ever used as a non-standard calling convention for
xc_cpuid_apply_policy() but that has been fixed now.

Purge its use, and any possible confusion over its behaviour, by having Xen
reject any attempts to use it.  Forgo setting it up in libxl's
hvm_set_conf_params().  The only backwards compatibility necessary is to have
the HVM restore stream discard it if found.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-21 15:28:40 +00:00
Andrew Cooper ea53652d78 xen/hvm: Fix handling of obsolete HVM_PARAMs
The local xc_hvm_param_deprecated_check() in libxc tries to guess Xen's
behaviour for the MEMORY_EVENT params, but is wrong for the get side, where
Xen would return 0 (which is also a bug).  Delete the helper.

In Xen, perform the checks in hvm_allow_set_param(), rather than
hvm_set_param(), and actually implement checks on the get side so the
hypercall doesn't return successfully with 0 as an answer.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-21 15:21:40 +00:00
Andrew Cooper 744807bdb0 x86/splitlock: CPUID and MSR details
A splitlock is an atomic operation which crosses a cache line boundary.  It
serialises operations in the cache coherency fabric and comes with a
multi-thousand cycle stall.

Intel Tremont CPUs introduce MSR_CORE_CAPS to enumerate various core-specific
features, and MSR_TEST_CTRL to adjust the behaviour in the case of a
splitlock.

Virtualising this for guests is distinctly tricky owing to the fact that
MSR_TEST_CTRL has core rather than thread scope.  In the meantime however,
prevent the MSR values leaking into guests.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
2020-02-21 13:50:00 +00:00
Paul Durrant c733b9e25c libxl: modify libxl__logv() to only log valid domid values
Some code-paths use values other than INVALID_DOMID to indicate an invalid
domain id. Specifically, xl will pass a value of 0 when creating/restoring
a domain. Therefore modify libxl__logv() to use libxl_domid_valid_guest()
as a validity test.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-21 12:05:41 +00:00
Sander Eikelenboom 4733541fc7 tools/xentop: Cleanup some trailing whitespace
Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
Acked-by: Wei Liu <wl@xen.org>
2020-02-20 12:03:21 +00:00
Sander Eikelenboom 228bccaab7 tools/xentop: Remove dead code
The freeable_mb variable was made to always be zero when purging tmem
from tools. We can in fact just delete it and the code associated with
it.

Fixes: c588c002cc ("tools: remove tmem code and commands")
Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
Acked-by: Wei Liu <wl@xen.org>
2020-02-20 12:03:21 +00:00
Sander Eikelenboom 4b5b431edd tools/xentop: Fix calculation of used memory
Used memory should be calculated by subtracting free memory from total
memory.

Fixes: c588c002cc ("tools: remove tmem code and commands")
Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
Acked-by: Wei Liu <wl@xen.org>
2020-02-20 12:03:21 +00:00
Andrew Cooper 8171e07965 tools/libx[cl]: Don't use HVM_PARAM_PAE_ENABLED as a function parameter
HVM_PARAM_PAE_ENABLED is set and consumed by the toolstack only.  It is in
practice a complicated and non-standard way of passing a boolean parameter
into xc_cpuid_apply_policy().

This is silly.  Pass PAE as a regular parameter instead.

In libxl__cpuid_legacy(), leave a rather better explaination of why only HVM
guests have a choice in PAE setting.

No change in how a guest is constructed.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-17 19:10:55 +00:00
Wei Liu ad1d3db985 libxl: mark parameters in stub functions as unused
Hopefully this can fix issues like:

In file included from ../../src/libxl/xen_xl.c:24:0:
/home/osstest/build.147035.build-amd64-libvirt/xendist/usr/local/include/libxl.h: In function 'libxl_cpuid_apply_policy':
/home/osstest/build.147035.build-amd64-libvirt/xendist/usr/local/include/libxl.h:2345:56: error: unused parameter 'ctx' [-Werror=unused-parameter]
 static inline void libxl_cpuid_apply_policy(libxl_ctx *ctx, uint32_t domid) {}

Fixes: dacb80f9 ("tools/libxl: Remove libxl_cpuid_{set,apply_policy}() from the API")
Signed-off-by: Wei Liu <wl@xen.org>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-14 11:08:53 +00:00
Wei Liu d3cec8e1f2 libxl: fix libxl__cpuid_legacy in libxl_nocpuid.c
Its last parameter should be libxl_domain_build_info.

Fixes: 1b3cec69 ("tools/libxl: Combine legacy CPUID handling logic")
Signed-off-by: Wei Liu <wl@xen.org>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2020-02-13 15:31:18 +00:00
Juergen Gross 1fb9a8bd8c xenstore: remove not applicable control commands in stubdom
When run in a stubdom environment Xenstore can't select a logfile or
emit memory statistics to a specific file.

So remove or modify those control commands accordingly.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-13 12:59:43 +00:00
Juergen Gross a9594631b6 xenstore: add console xenstore entries for xenstore stubdom
In order to be able to connect to the console of Xenstore stubdom we
need to create the appropriate entries in Xenstore.

For the moment we don't support xenconsoled living in another domain
than dom0, as this information isn't available other then via
Xenstore which we are just setting up.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-13 12:59:39 +00:00
Juergen Gross 5d1f9cd27e xenstore: setup xenstore stubdom console interface properly
In order to be able to get access to the console of Xenstore stubdom
we need an appropriate granttab entry. So call xc_dom_gnttab_init()
when constructing the domain and preset some information needed
for that function in the dom structure.

We need to create the event channel for the console, too. Do that and
store all necessary data locally.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-13 12:59:22 +00:00
Andrew Cooper 1b3cec69bf tools/libxl: Combine legacy CPUID handling logic
While we are in the process of overhauling boot time CPUID/MSR handling, the
existing logic is going to have to remain in roughly this form for backwards
compatibility.

Fold libxl__cpuid_apply_policy() and libxl__cpuid_set() together into a single
libxl__cpuid_legacy() to reduce the complexity for callers.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-11 18:26:52 +00:00
Andrew Cooper dacb80f975 tools/libxl: Remove libxl_cpuid_{set,apply_policy}() from the API
These functions should never have been exposed.  They don't have external
users, and can't usefully be used for several reasons.

Move libxl_cpuid_{set,apply_policy}() to being internal functions, and leave
an equivalent of the nop stubs in the API for caller compatibility.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-02-11 18:26:52 +00:00
Anthony PERARD 3d3cf4bea8 tools: Default to python3
Main reason, newer version of QEMU doesn't support python 2.x anymore.
Second main reason, python2 is EOL.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-11 11:24:33 +00:00
Andrew Cooper b25fb1a04e xen/pvh: Fix segment selector ABI
The written ABI states that %es will be set up, but libxc doesn't do so.  In
practice, it breaks `rep movs` inside guests before they reload %es.

The written ABI doesn't mention %ss, but libxc does set it up.  Having %ds
different to %ss is obnoxous to work with, as different registers have
different implicit segments.

Modify the spec to state that %ss is set up as a flat read/write segment.
This a) matches the Multiboot 1 spec, b) matches what is set up in practice,
and c) is the more sane behaviour for guests to use.

Fixes: 68e1183411 ('libxc: introduce a xc_dom_arch for hvm-3.0-x86_32 guests')
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wl@xen.org>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
2020-02-10 15:26:09 +00:00
Andrew Cooper 270ff9a835 tools/ocaml: Drop cpuid helpers
These have no callers, and the underlying infrastructure is about to be
rewritten completely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-02-10 13:09:54 +00:00
Andrew Cooper 0a9c44486b tools/python: Drop cpuid helpers
These are believed-unused, and the underlying infrastructure is about to be
rewritten completely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
2020-02-10 10:58:07 +00:00
Roger Pau Monné a1a357fcf5 Revert "tools/libxc: disable x2APIC when using nested virtualization"
This reverts commit 7b3c5b70a3 and
re-enables the usage of x2APIC with nested virtualization.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-05 13:51:19 +01:00
Andrew Cooper 0729830cc4 libxc/restore: Fix REC_TYPE_X86_PV_VCPU_XSAVE data auditing (take 2)
It turns out that a bug (since forever) in Xen causes XSAVE records to have
non-architectural behaviour on xsave-capable hardware, when a PV guest has not
touched the state.

In such a case, the data record returned from Xen is 2*uint64_t, both claiming
the (illegitimate) state of %xcr0 and %xcr0_accum being 0.

Adjust the bound in handle_x86_pv_vcpu_blob() to cope with this.

Fixes: 2a62c22715 "libxc/restore: Fix data auditing in handle_x86_pv_vcpu_blob()"
Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
2020-02-05 12:02:42 +00:00
Paul Durrant d00377890a libxl: fix assertion failure in stub domain creation
An assertion in libxl__domain_make():

'soft_reset || *domid == INVALID_DOMID'

does not hold true for stub domain creation, where soft_reset is false
but the passed in domid == 0. This is easily fixed by changing the
initializer in libxl__spawn_stub_dm().

NOTE: The comment for XEN_DOMCTL_createdomain in domctl.h is changed to
      reflect reality.

Fixes: 75259239d8 ("libxl_create: make 'soft reset' explicit")
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
2020-02-05 11:56:27 +00:00
Stefan Bader 960dd83213 tools/xenstore: Re-introduce (fake) xs_restrict call to preserve ABI
libxenstore3.0 in Xen 4.8 had this function.  We don't really want to
bump the ABI version (soname) just for this, since we don't think
there are actual callers anywhere.  But tools complain about the
symbol going away.

So, provide a function xs_restrict which conforms to the original
semantics, although it always fails.

Gbp-Pq: Topic xenstore
Gbp-Pq: Name tools-fake-xs-restrict.patch
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
2020-02-04 11:31:07 +00:00
Juergen Gross 957bd8e7e3 tools/xenstore: don't apply write limiting for privileged domain
Xenstore write limiting should not be applied to dom0. Unfortunately
write limiting is disabled only for connections via sockets. When
running in a stubdom Xenstore will apply write limiting to dom0, too.
Change that by testing for the domain to be privileged as well.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-01-31 16:13:49 +00:00
Juergen Gross 00ffefb405 tools/xenstore: add newline for printing of stubdom console messages
There are several places in xenstore-stubdom where newlines at the end
of messages on the console are missing. Add them.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-01-31 16:12:37 +00:00
Paul Durrant 1d224ce698 libxl: generalise libxl__domain_userdata_lock()
This function implements a file-based lock with a file name generated
from a domid.

This patch splits it into two, generalising the core of the locking code
into a new libxl__lock_file() function which operates on a specified file,
leaving just the file name generation in libxl__domain_userdata_lock().

This patch also generalises libxl__unlock_domain_userdata() to
libxl__unlock_file() and modifies all call-sites.

Suggested-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-01-31 16:10:46 +00:00
Paul Durrant 75259239d8 libxl_create: make 'soft reset' explicit
The 'soft reset' code path in libxl__domain_make() is currently taken if a
valid domid is passed into the function. A subsequent patch will enable
higher levels of the toolstack to determine the domid of newly created or
restored domains and therefore this criteria for choosing 'soft reset'
will no longer be usable.

This patch adds an extra boolean option to libxl__domain_make() to specify
whether it is being invoked in soft reset context and appropriately
modifies callers to choose the right value. To facilitate this, a new
'soft_reset' boolean field is added to struct libxl__domain_create_state
and the 'domid_soft_reset' field is renamed to 'domid' in anticipation of
its wider remit. For the moment do_domain_create() will always set
domid to INVALID_DOMID and hence we can add an assertion into
libxl__domain_create() that, if it is not called in soft reset context,
the passed in domid is exactly that value.

Whilst in the neighbourhood, some checks of 'restore_fd > -1' have been
replaced by 'restore_fd >= 0' to be more conventional and consistent with
checks of 'restore_fd < 0'.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-01-31 16:10:46 +00:00
Paul Durrant bc7b534305 libxl: add definition of INVALID_DOMID to the API
Currently both xl and libxl have internal definitions of INVALID_DOMID
which happen to be identical. However, for the purposes of describing the
behaviour of libxl_domain_create_new/restore() it is useful to have a
specified invalid value for a domain id.

This patch therefore moves the libxl definition from libxl_internal.h to
libxl.h and removes the internal definition from xl_utils.h. The hardcoded
'-1' passed back via domcreate_complete() is then updated to INVALID_DOMID
and comment above libxl_domain_create_new/restore() is accordingly
modified.

NOTE: The value of INVALID_DOMID (~0) is distinct from the hypervisor's
      DOMID_INVALID. This patch preserves that value.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-01-31 16:10:46 +00:00
Ian Jackson 0b70b2ff8f libxl: event: Move poller pipe emptying to the end of afterpoll
This seems neater.  It doesn't have any significant effect because:

The poller fd wouldn't be emptied by time_occurs.  It would only be
woken by time_occurs as a result of an ao completing, or by
libxl__egc_ao_cleanup_1_baton.  But ...1_baton won't be called in
between (for one thing, this would violate the rule of not still
having the active caller when ...1_baton is called).

While discussing this patch, I noticed that there is a possibility (in
libxl in general) that poller_put might be called on a woken poller.
It would probably be sensible at some point to make poller_get empty
the pipe, at least if the pipe_nonempty flag is set.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
v3: Completely revised commit message; now we think this is just
    cleanup.
2020-01-27 16:03:18 +00:00
Ian Jackson ba7a0dbb37 libxl: event: Fix possible hang with libxl_osevent_beforepoll
If the application uses libxl_osevent_beforepoll, a similar hang is
possible to the one described and fixed in
   libxl: event: Fix hang when mixing blocking and eventy calls
Application behaviour would have to be fairly unusual, but it
doesn't seem sensible to just leave this latent bug.

We fix the latent bug by waking up the "poller_app" pipe every time we
add osevents.  If the application does not ever call beforepoll, we
write one byte to the pipe and set pipe_nonempty and then we ignore
it.  We only write another byte if beforepoll is called again.

Normally in an eventy program there would only be one thread calling
libxl_osevent_beforepoll.  The effect in such a program is to
sometimes needlessly go round the poll loop again if a timeout
callback becomes interested in a new osevent.  We'll fix that in a
moment.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
---
v2: New addition to correctness arguments in libxl_event.c comment.
2020-01-27 16:03:18 +00:00
Ian Jackson caeba48759 libxl: event: Break out baton_wake
No functional change.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
---
v2: Now it takes a gc, not an egc.
2020-01-27 16:03:17 +00:00
Ian Jackson 67a97ca955 libxl: event: poller pipe optimisation
Track in userland whether the poller pipe is nonempty.  This saves us
writing many many bytes to the pipe if nothing ever reads them.

This is going to be relevant in a moment, where we are going to create
a situation where this will happen quite a lot.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
2020-01-27 16:03:17 +00:00
Ian Jackson 5a533d0ba5 libxl: event: Fix hang when mixing blocking and eventy calls
If the application calls libxl with ao_how==0 and also makes calls
like _occurred, libxl will sometimes get stuck.

The bug happens as follows (for example):

  Thread A
       libxl_do_thing(,ao_how==0)
       libxl_do_thing starts, sets up some callbacks
       libxl_do_thing exit path calls AO_INPROGRESS
       libxl__ao_inprogress goes into event loop
       eventloop_iteration sleeps on:
          - do_thing's current fd set
          - sigchld pipe if applicable
          - its poller

  Thread B
       libxl_something_occurred
       the something is to do with do_thing, above
       do_thing_next_callback does some more work
       do_thing_next_callback becomes interested in fd N
       thread B returns to application

Note that nothing wakes up thread A.  A is not listening on fd N.  So
do_thing_* will not spot when fd N signals.  do_thing will not make
further timely progress.  If there is no timeout thread A will never
wake up.

The problem here occurs because thread A is waiting on an out of date
osevent set.

There is also the possibility that a thread might block waiting for
libxl osevents but outside libxl, eg if the application used
libxl_osevent_beforepoll.  We will deal with that in a moment.

See the big comment in libxl_event.c for a fairly formal correctness
argument.

This depends on libxl__egc_ao_cleanup_1_baton being called everywhere
an egc or ao is disposed of.  Firstly egcs: in this patch we rename
libxl__egc_cleanup, which means we catch all the disposal sites.
Secondly aos: these are disposed of by (i) AO_CREATE_FAIL
(ii) ao__inprogress and (iii) an event which completes the ao later.
(i) and (ii) we handle by adding the call to _baton.  In the case of
(iii) any such function must be an event-generating function so it has
an egc too, so it will pass on the baton when the egc is disposed.

Reported-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
---
v2: Call libxl__egc_ao_cleanup_1_baton (renamed from __egc_cleanup) on
    all exits from ao_inprogress, even requests for async processing.
    Fixes a remaining instance of this bug (!)
    This involves disposing of ao->poller somewhat earlier.

v2: New correctness arguments in libxl_event.c comment and
    in commit message.
2020-01-27 16:03:17 +00:00
Ian Jackson 49b88ded6b libxl: event: Make libxl__poller_wakeup take a gc, not an egc
We are going to want to call this in the following situation:

 * We have just set up an ao, which is to call back - so a
   non-synchronous one.  It ought not to call the application
   back right away, so no egc.

 * There is a libxl thread blocking somewhere but it is using
   using an out of date fd or timeout set, which does not take into
   account the ao we have just started.

 * We try to wake that thread up, but libxl__poller_wakeup fails.

In more detail:

The idea before was that these two functions take an egc, not so much
because it actually uses the egc, but to make sure it's only called in a
restricted set of conditions; and now we're relaxing those conditions.

Specifically, we need to make one exception, relating to ao's.

In the situation described above, there is no egc, but we need to call
libxl__poller_wakeup.  Introducing an egc is wrong because that would
imply that this situation might result in application callbacks, but
it shouldn't (and not having an egc prevents that).

libxl__poller_wakeup and LIBXL__EVENT_DISASTER only take an egc for
form's sake; they don't use any part of it other than the gc.  The
"form's sake" is to stop them being called from libxl entrypoints that
are not involved in event generation.

Before this patch this is enforced by the types: you can't call it in
the wrong place because it wants an egc which you don't have.

After this patch this is no longer enforced.  But the mistake
(principally, calling _DISASTER) seems unlikely.  The type enforcement
I mention above was done because it was possible and easy, not because
it was important.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
---
v3: Significantly expanded commit message based on irc comments
v2: New patch
2020-01-27 16:03:17 +00:00