Commit Graph
488 Commits
Author SHA1 Message Date
Roger Pau Monné 0291089f6e xen: enable altp2m at create domain domctl
Enabling it using an HVM param is fragile, and complicates the logic when
deciding whether options that interact with altp2m can also be enabled.

Leave the HVM param value for consumption by the guest, but prevent it from
being set.  Enabling is now done using and additional altp2m specific field in
xen_domctl_createdomain.

Note that albeit only currently implemented in x86, altp2m could be implemented
in other architectures, hence why the field is added to xen_domctl_createdomain
instead of xen_arch_domainconfig.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com> # hypervisor
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Juergen Gross <jgross@suse.com> # tools/libs/
Release-acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
2024-05-29 16:10:04 +02:00
Andrew Cooper caf8644826 tools/{c,o}xenstored: Don't link against libsystemd
Use the local freestanding wrapper instead.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2024-05-23 15:04:40 +01:00
Edwin Török 6d5111b10e tools/ocaml: Fix warnings in config.ml
Fixes warnings such as:

  File "config.ml", line 102, characters 12-27:
  102 |         | Failure "int_of_string"   -> append (k, "expect int arg")
                          ^^^^^^^^^^^^^^^
  Warning 52: Code should not depend on the actual values of
  this constructor's arguments. They are only for information
  and may change in future versions. (See manual section 9.5)

Do not rely on the string values of the `Failure` exception, but use the
`_opt` functions instead.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-04-25 13:25:26 +01:00
Andrew Cooper 8f85af65af tools/oxenstored: Re-format
Rerun make format.

Fixes: b6cf604207 ("tools/oxenstored: Use Map instead of Hashtbl for quotas")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edwin.torok@cloud.com>
2024-03-27 16:00:47 +00:00
Edwin Török 098d868e52 tools/oxenstored: Make Quota.t pure
Now that we no longer have a hashtable inside we can make Quota.t pure, and
push the mutable update to its callers.  Store.t already had a mutable Quota.t
field.

No functional change.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2024-02-23 17:43:06 +00:00
Edwin Török b6cf604207 tools/oxenstored: Use Map instead of Hashtbl for quotas
On a stress test running 1000 VMs flamegraphs have shown that
`oxenstored` spends a large amount of time in `Hashtbl.copy` and the GC.

Hashtable complexity:
 * read/write: O(1) average
 * copy: O(domains) -- copying the entire table

Map complexity:
 * read/write: O(log n) worst case
 * copy: O(1) -- a word copy

We always perform at least one 'copy' when processing each xenstore
packet (regardless whether it is a readonly operation or inside a
transaction or not), so the actual complexity per packet is:
  * Hashtbl: O(domains)
  * Map: O(log domains)

Maps are the clear winner, and a better fit for the immutable xenstore
tree.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2024-02-23 17:43:06 +00:00
Petr Beneš e36c106b0b tools/ocaml: Add missing vmtrace_buf_kb field
Add the missing `vmtrace_buf_kb` field to the OCaml bindings to match the
vm.cfg configuration, correcting an oversight from its initial introduction.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2024-02-09 08:17:24 +01:00
Edwin Török c852ca5c05 tools/ocaml: Bump minimum version to OCaml 4.05
Char.lowercase got removed in OCaml 5.0 (it has been deprecated since 2014),
and doesn't build any more.

Char.lowercase_ascii has existed since OCaml 4.03, so that is the new
minimum version for oxenstored.

However, OCaml 4.05 is the oldest new-enough version found in common distros,
so pick this as a baseline.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
[Update CHANGELOG.md]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
2024-02-01 19:52:44 +00:00
Edwin Török 0742b0a081 tools/oxenstored: Additional debugging commands
These were added to aid security development, and are useful generally for
debugging.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2023-08-24 19:55:08 +01:00
Edwin Török 99e4554893 ocaml/libs/xc: Fix NULL dereference with physinfo_arch_caps()
`Tag_cons` is `0` and is meant to be used as the tag argument for
`caml_alloc`/`caml_alloc_small` when constructing a non-empty list.

The empty list is `Val_emptylist` instead, which is really just `Val_int(0)`.

Assigning `0` to a list value like this is equivalent to assigning the naked
pointer `NULL` to the field.  Naked pointers are not valid in OCaml 5, however
even in OCaml <5.x any attempt to iterate on the list will lead to a segfault.

The list currently only has an opaque type, so no code would have reason to
iterate on it currently, but we shouldn't construct invalid OCaml values that
might lead to a crash when exploring the type.

`Val_emptylist` is available since OCaml 3.01 as a constant.

Fixes: e5ac68a011 ("x86/hvm: Revert per-domain APIC acceleration support")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2023-07-13 11:06:07 +01:00
Andrew Cooper b4642c32c4 tools/ocaml/xc: Fix xc_physinfo() bindings
The original change doesn't compile on ARM:

  xenctrl_stubs.c: In function 'stub_xc_physinfo':
  xenctrl_stubs.c:821:16: error: unused variable 'arch_cap_flags_tag' [-Werror=unused-variable]
    821 |         int r, arch_cap_flags_tag;
        |                ^~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

but it was buggy too.

First, it tried storing an int in a pointer slot, causing heap corruption.

Next, it is not legitimate to exclude arm32 in the toolstack as it explicitly
can operate an arm64 toolstack and build arm64 domains.  That in turn means
that you can't stash a C uint32_t in an OCaml int.

Rewrite the arch_capabilities handling from scratch.  Break it out into a
separate function, and make the construction of arch_physinfo_cap_flags common
to prevent other indirection bugs.

Reintroduce arm_physinfo_caps with the fields broken out.

Fixes: 56a7aaa16b ("tools: add physinfo arch_capabilities handling for Arm")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2023-06-09 20:13:22 +01:00
Luca Fancellu 56a7aaa16b tools: add physinfo arch_capabilities handling for Arm
On Arm, the SVE vector length is encoded in arch_capabilities field
of struct xen_sysctl_physinfo, make use of this field in the tools
when building for arm.

Create header arm-arch-capabilities.h to handle the arch_capabilities
field of physinfo for Arm.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-06-07 11:21:41 +01:00
Alejandro Vallejo cb860a95a9 tools: Modify single-domid callers of xc_domain_getinfolist()
xc_domain_getinfolist() internally relies on a sysctl that performs
a linear search for the domids. Many callers of xc_domain_getinfolist()
who require information about a precise domid are much better off calling
xc_domain_getinfo_single() instead, that will use the getdomaininfo domctl
instead and ensure the returned domid matches the requested one. The domtctl
will find the domid faster too, because that uses hashed lists.

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-05-10 19:59:27 +01:00
Andrew Cooper 12c5eea3ca tools/ocaml/mmap: Drop the len parameter from Xenmmap.write
Strings in Ocaml carry their own length.  Absolutely nothing good can come
from having caml_string_length(data) be different to len.

Use the appropriate accessor, String_val(), but retain the workaround for the
Ocaml -safe-string constness bug in the same way as we've done elsewhere in
Xen.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2023-03-30 16:26:24 +01:00
Juergen Gross 5b18843067 tools: rename xen-tools/libs.h file to common-macros.h
In order to better reflect the contents of the header and to make it
more appropriate to use it for different runtime environments like
programs, libraries, and firmware, rename the libs.h include file to
common-macros.h. Additionally add a comment pointing out the need to be
self-contained.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> # tools/python/xen/lowlevel/xc/xc.c
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2023-03-22 09:57:19 +01:00
Andrew Cooper 433d012c6c x86/sysctl: Retrofit XEN_SYSCTL_cpu_featureset_{pv,hvm}_max
Featuresets are supposed to be disappearing when the CPU policy infrastructure
is complete, but that has taken longer than expected, and isn't going to be
complete imminently either.

In the meantime, Xen does have proper default/max featuresets, and xen-cpuid
can even get them via the XEN_SYSCTL_cpu_policy_* interface, but only knows
now to render them nicely via the featureset interface.

Differences between default and max are a frequent source of errors,
frequently too in secret leading up to an embargo, so extend the featureset
sysctl to allow xen-cpuid to render them all nicely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
2023-03-13 11:26:26 +00:00
Andrew Cooper e51d31f79e tools/ocaml: Drop libxl and xentoollog bindings
There are significant issues with these bindings (they segfault for starters)
and their companion half in Xapi was deleted in 2018

  https://github.com/xapi-project/xen-api/commit/203292ebe0c487d7ae4adb961a6d080f4fbe933d

owing to there having been no development of these bindings since 2014.

In the unlikely event that we'd want to reinstate them, they'd need reworking
basically from scratch anyway.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-09 18:26:17 +00:00
Andrew Cooper d69ccf52ad tools/ocaml/libs: Fix memory/resource leaks with caml_alloc_custom()
All caml_alloc_*() functions can throw exceptions, and longjump out of
context.  If this happens, we leak the xch/xce handle.

Reorder the logic to allocate the the Ocaml object first.

Fixes: 8b3c06a3e5 ("tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free")
Fixes: 22d5affdf0 ("tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török 5fb7851ff2 tools/ocaml/xc: Don't reference Custom objects with the GC lock released
The modification to the _H() macro for Ocaml 5 support introduced a subtle
bug.  From the manual:

  https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code

"After caml_release_runtime_system() was called and until
caml_acquire_runtime_system() is called, the C code must not access any OCaml
data, nor call any function of the run-time system, nor call back into OCaml
code."

Previously, the value was a naked C pointer, so dereferencing it wasn't
"accessing any Ocaml data", but the fix to avoid naked C pointers added a
layer of indirection through an Ocaml Custom object, meaning that the common
pattern of using _H() in a blocking section is unsafe.

In order to fix:

 * Drop the _H() macro and replace it with a static inline xch_of_val().
 * Opencode the assignment into Data_custom_val() in the constructors.
 * Rename "value xch" parameters to "value xch_val" so we can consistently
   have "xc_interface *xch" on the stack, and obtain the pointer with the GC
   lock still held.
 * Drop the _D() macro while at it, because it's just pointless indirection.

Fixes: 8b3c06a3e5 ("tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Andrew Cooper 9e7c74e6f9 tools/ocaml/xc: Don't reference Abstract_Tag objects with the GC lock released
The intf->{addr,len} references in the xc_map_foreign_range() call are unsafe.
From the manual:

  https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code

"After caml_release_runtime_system() was called and until
caml_acquire_runtime_system() is called, the C code must not access any OCaml
data, nor call any function of the run-time system, nor call back into OCaml
code."

More than what the manual says, the intf pointer is (potentially) invalidated
by caml_enter_blocking_section() if another thread happens to perform garbage
collection at just the right (wrong) moment.

Rewrite the logic.  There's no need to stash data in the Ocaml object until
the success path at the very end.

Fixes: 8b7ce06a2d ("ocaml: Add XC bindings.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török 4250683842 tools/ocaml/xc: Fix binding for xc_domain_assign_device()
The patch adding this binding was plain broken, and unreviewed.  It modified
the C stub to add a 4th parameter without an equivalent adjustment in the
Ocaml side of the bindings.

In 64bit builds, this causes us to dereference whatever dead value is in %rcx
when trying to interpret the rflags parameter.

This has gone unnoticed because Xapi doesn't use this binding (it has its
own), but unbreak the binding by passing RDM_RELAXED unconditionally for
now (matching the libxl default behaviour).

Fixes: 9b34056cb4 ("tools: extend xc_assign_device() to support rdm reservation policy")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Andrew Cooper 40893f5ac6 tools/ocaml/evtchn: Misc cleanup
* Remove local integers when all we're returning is Val_int() of another
   variable.  The CAMLlocal*() can't be optimised automatically, as it's
   registered with the GC.
 * Rename "virq_type" to "virq" and "_port" to "port".
 * In stub_eventchn_pending(), rename 'port' to 'rc', to be consistent with
   all other stubs that return xenevtchn_port_or_error_t.
 * In stub_eventchn_unmask(), check for rc == -1 to be consistent with all
   other stubs.

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török 2636d8ff7a tools/ocaml/evtchn: Don't reference Custom objects with the GC lock released
The modification to the _H() macro for Ocaml 5 support introduced a subtle
bug.  From the manual:

  https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code

"After caml_release_runtime_system() was called and until
caml_acquire_runtime_system() is called, the C code must not access any OCaml
data, nor call any function of the run-time system, nor call back into OCaml
code."

Previously, the value was a naked C pointer, so dereferencing it wasn't
"accessing any Ocaml data", but the fix to avoid naked C pointers added a
layer of indirection through an Ocaml Custom object, meaning that the common
pattern of using _H() in a blocking section is unsafe.

In order to fix:

 * Drop the _H() macro and replace it with a static inline xce_of_val().
 * Opencode the assignment into Data_custom_val() in the two constructors.
 * Rename "value xce" parameters to "value xce_val" so we can consistently
   have "xenevtchn_handle *xce" on the stack, and obtain the pointer with the
   GC lock still held.

Fixes: 22d5affdf0 ("tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Andrew Cooper 36eb2de31b tools/ocaml/libs: Allocate the correct amount of memory for Abstract_tag
caml_alloc() takes units of Wsize (word size), not bytes.  As a consequence,
we're allocating 4 or 8 times too much memory.

Ocaml has a helper, Wsize_bsize(), but it truncates cases which aren't an
exact multiple.  Use a BUILD_BUG_ON() to cover the potential for truncation,
as there's no rounding-up form of the helper.

Fixes: 8b7ce06a2d ("ocaml: Add XC bindings.")
Fixes: d3e649277a ("ocaml: add mmap bindings implementation.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török ff8b560be8 tools/ocaml/libs: Don't declare stubs as taking void
There is no such thing as an Ocaml function (C stub or otherwise) taking no
parameters.  In the absence of any other parameters, unit is still passed.

This doesn't explode with any ABI we care about, but would malfunction for an
ABI environment such as stdcall.

Fixes: c3afd398ba ("ocaml: Add XS bindings.")
Fixes: 8b7ce06a2d ("ocaml: Add XC bindings.")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török e6f07052ce tools/oxenstored: validate config file before live update
The configuration file can contain typos or various errors that could prevent
live update from succeeding (e.g. a flag only valid on a different version).
Unknown entries in the config file would be ignored on startup normally,
add a strict --config-test that live-update can use to check that the config file
is valid *for the new binary*.

For compatibility with running old code during live update recognize
--live --help as an equivalent to --config-test.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török ea250e81cf tools/ocaml: run 'make format' on OCaml files
`git log -p -1 -w` proves the change here is only whitespace.

No functional change.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török aad8c1276c tools/ocaml: add 'make format' for OCaml files
Using `ocp-indent` for now to just make minimal modifications in
tabs vs spaces and get the right indentation.

This avoids perpetuating a formatting style that is inconsistent with
the rest of Xen, and that makes preparing and submitting patches more
difficult (OCaml indentation tools usually only support spaces, not tabs).

No functional change.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2023-02-06 10:22:12 +00:00
Edwin Török 01f139215e tools/ocaml/xb: Drop Xs_ring.write
This function is unusued (only Xs_ring.write_substring is used), and the
bytes/string conversion here is backwards: the C stub implements the bytes
version and then we use a Bytes.unsafe_of_string to convert a string into
bytes.

However the operation here really is read-only: we read from the string and
write it to the ring, so the C stub should implement the read-only string
version, and if needed we could use Bytes.unsafe_to_string to be able to send
'bytes'. However that is not necessary as the 'bytes' version is dropped above.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-19 19:31:26 +00:00
Edwin Török 2e173b8383 tools/ocaml/xenctrl: Fix unused value warning
Fixes: 8b3c06a3e5 ("tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2022-12-19 19:31:26 +00:00
Edwin Török 4a25935323 tools/ocaml/xenstored: Use ocamldep -sort for linking order
The manually established link order is fragile to code changes.  Use ocamldep
to calculate the appropriate link order.

No functional change.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-19 19:31:26 +00:00
Edwin Török d2ccc63711 tools/ocaml/xb,mmap: Use Data_abstract_val wrapper
This is not strictly necessary since it is essentially a no-op currently: a
cast to void * and value *, even in OCaml 5.0.

However it does make it clearer that what we have here is not a regular OCaml
value, but one allocated with Abstract_tag or Custom_tag, and follows the
example from the manual more closely:
https://v2.ocaml.org/manual/intfc.html#ss:c-outside-head

It also makes it clearer that these modules have been reviewed for
compat with OCaml 5.0.

We cannot use OCaml finalizers here, because we want exact control over when
to unmap these pages from remote domains.

No functional change.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-19 19:31:26 +00:00
Edwin Török 10acd21795 tools/ocaml/xenctrl: Add hvm_param_{get,set} bindings
Not to be confused which hvm_get_param, which also exists and has a
different, more error-prone interface.

This one always returns a 64-bit value, and that is retained in the
OCaml binding as well, returning 'int64' (and not int, or nativeint
which might have different sizes).

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-02 13:41:04 +00:00
Edwin Török ff95dae53e tools/ocaml/xenctrl: Add binding for xc_evtchn_status
There is no API or ioctl to query event channel status, it is only
present in xenctrl.h

The C union is mapped to an OCaml variant exposing just the value from
the correct union tag.  This causes the xc bindings to now depend on
evtchn to get a useful API for EVTCHNSTAT_virq.

The information provided here is similar to 'lsevtchn', but rather than
parsing its output it queries the underlying API directly.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-02 13:41:04 +00:00
Andrew Cooper d2162d884c tools/oxenstored: Render backtraces more nicely in Syslog
fallback_exception_handler feeds a string with embedded newlines directly into
syslog().  While this is an improvement on getting nothing, syslogd escapes
all control characters it gets, and emits one (long) log line.

Fix the problem generally in the syslog stub.  As we already have a local copy
of the string, split it in place and emit one syslog() call per line.

Also tweak Logging.msg_of to avoid putting an extra newline on a string which
already ends with one.

Fixes: ee7815f49f ("tools/oxenstored: Set uncaught exception handler")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-02 13:41:04 +00:00
Andrew Cooper 8b3c06a3e5 tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free
The binding for xc_interface_close() free the underlying handle while leaving
the Ocaml object still in scope and usable.  This would make it easy to suffer
a use-after-free, if it weren't for the fact that the typical usage is as a
singleton that lives for the lifetime of the program.

Ocaml 5 no longer permits storing a naked C pointer in an Ocaml value.

Therefore, use a Custom block.  This allows us to use the finaliser callback
to call xc_interface_close(), if the Ocaml object goes out of scope.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török acd3fb6d65 tools/oxenstored/syslog: Avoid potential NULL dereference
strdup() may return NULL.  Check for this before passing to syslog().

Drop const from c_msg.  It is bogus, as demonstrated by the need to cast to
void * in order to free the memory.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török ee7815f49f tools/oxenstored: Set uncaught exception handler
Unhandled exceptions go to stderr by default, but this doesn't typically work
for oxenstored because:
 * daemonize reopens stderr as /dev/null
 * systemd redirects stderr to /dev/null too

Debugging an unhandled exception requires reproducing the issue locally when
using --no-fork, and is not conducive to figuring out what went wrong on a
remote system.

Install a custom handler which also tries to render the backtrace to the
configured syslog facility, and DAEMON|ERR otherwise.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török 3f02e0a70f tools/oxenstored: Log live update issues at warning level
During live update, oxenstored tries a best effort approach to recover as many
domains and information as possible even if it encounters errors restoring
some domains.

However, logging about misunderstood input is more severe than simply info.
Log it at warning instead.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török 9b224c2529 tools/oxenstored: Keep /dev/xen/evtchn open across live update
Closing the evtchn handle will unbind and free all local ports.  The new
xenstored would need to rebind all evtchns, which is work that we don't want
or need to be doing during the critical handover period.

However, it turns out that the Windows PV drivers also rebind their local port
too across suspend/resume, leaving (o)xenstored with a stale idea of the
remote port to use.  In this case, reusing the established connection is the
only robust option.

Therefore:
 * Have oxenstored open /dev/xen/evtchn without CLOEXEC at start of day.
 * Extend the handover information with the evtchn fd, domexc virq local port,
   and the local port number for each domain connection.
 * Have (the new) oxenstored recover the open handle using Xeneventchn.fdopen,
   and use the provided local ports rather than trying to rebind them.

When this new information isn't present (i.e. live updating from an oxenstored
prior to this change), the best-effort status quo will have to do.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Andrew Cooper df2db174b3 tools/oxenstored: Rework Domain evtchn handling to use port_pair
Inter-domain event channels are always a pair of local and remote ports.
Right now the handling is asymmetric, caused by the fact that the evtchn is
bound after the associated Domain object is constructed.

First, move binding of the event channel into the Domain.make() constructor.
This means the local port no longer needs to be an option.  It also removes
the final callers of Domain.bind_interdomain.

Next, introduce a new port_pair type to encapsulate the fact that these two
should be updated together, and replace the previous port and remote_port
fields.  This refactoring also changes the Domain.get_port interface (removing
an option) so take the opportunity to name it get_local_port instead.

Also, this fixes a use-after-free risk with Domain.close.  Once the evtchn has
been unbound, the same local port number can be reused for a different
purpose, so explicitly invalidate the ports to prevent their accidental misuse
in the future.

This also cleans up some of the debugging, to always print a port pair.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Andrew Cooper aecdc28d95 tools/oxenstored: Implement Domain.rebind_evtchn
Generally speaking, the event channel local/remote port is fixed for the
lifetime of the associated domain object.  The exception to this is a
secondary XS_INTRODUCE (defined to re-bind to a new event channel) which pokes
around at the domain object's internal state.

We need to refactor the evtchn handling to support live update, so start by
moving the relevant manipulation into Domain.

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Andrew Cooper 31fbee749a tools/oxenstored: Rename some 'port' variables to 'remote_port'
This will make the logic clearer when we plumb local_port through these
functions.

While doing this, rearrange the construct in Domains.create0 to separate the
remote port handling from the interface handling.  (The interface logic is
dubious in several ways, but not altered by this cleanup.)

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Andrew Cooper 9804a5db43 tools/oxenstored: Bind the DOM_EXC VIRQ in in Event.init()
Xenstored always needs to bind the DOM_EXC VIRQ.

Instead of doing it shortly after the call to Event.init(), do it in the
constructor directly.  This removes the need for the field to be a mutable
option.

It will also simplify a future change to support live update.  Rename the
field from virq_port (which could be any VIRQ) to it's proper name.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Andrew Cooper b45bfaf359 tools/oxenstored: Style fixes to Domain
This file has some style problems so severe that they interfere with the
readability of the subsequent bugfix patches.

Fix these issues ahead of time, to make the subsequent changes more readable.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török 9bafe4a533 tools/ocaml/evtchn: Extend the init() binding with a cloexec flag
For live update, oxenstored wants to clear CLOEXEC on the evtchn handle, so it
survives the execve() into the new oxenstored.

Have the new interface match how cloexec works in other Ocaml standard
libraries.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török 7ba68a6c55 tools/ocaml/evtchn: Add binding for xenevtchn_fdopen()
For live update, the new oxenstored needs to reconstruct an evtchn object
around an existing file descriptor.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török 22d5affdf0 tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak
There is no binding for xenevtchn_close().  In principle, this is a resource
leak, but the typical usage is as a singleton that lives for the lifetime of
the program.

Ocaml 5 no longer permits storing a naked C pointer in an Ocaml value.

Therefore, use a Custom block.  This allows us to use the finaliser callback
to call xenevtchn_close(), if the Ocaml object goes out of scope.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Andrew Cooper ee36179371 tools/oxenstored: Fix incorrect scope after an if statement
A debug statement got inserted into a single-expression if statement.

Insert brackets to give the intended meaning, rather than the actual meaning
where the "let con = Connections..." is outside and executed unconditionally.

This results in some unnecessary ring checks for domains which otherwise have
IO credit.

Fixes: 42f0581a91 ("tools/oxenstored: Implement live update for socket connections")
Reported-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00
Edwin Török 95db09b1b1 tools/ocaml/xenctrl: Use larger chunksize in domain_getinfolist
domain_getinfolist() is quadratic with the number of domains, because of the
behaviour of the underlying hypercall.  Nevertheless, getting domain info in
blocks of 1024 is far more efficient than blocks of 2.

In a scalability testing scenario with ~1000 VMs, a combination of this and
the previous change takes xenopsd's wallclock time in domain_getinfolist()
down from 88% to 0.02%

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Tested-by: Pau Ruiz Safont <pau.safont@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-12-01 16:07:17 +00:00