Use the apply-build-vars make macro to replace the @xxx@ markers in
most *.in files only at build time.
As some of the markers are local to tools/configure, introduce
config/Tools-paths.mk.in and add the related make variables to it.
Add Tools-paths to the PATH_FILES make variable in order to include
the definitions for replacing them in the *.in files.
Add the generated files to the distclean target.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The latest oxenstored from Xapi-project fails to start up:
launch-xenstore[1201]: Starting /usr/sbin/oxenstored...
launch-xenstore[1222]: Fatal error: exception
Dynlink.Error (Dynlink.Cannot_open_dll "Dynlink.Error (Dynlink.Cannot_open_dll
\"Failure(\\\"/usr/libexec/xen//ocaml/xsd_glue/xenctrl_plugin/domain_getinfo_v1.cmxs:
undefined symbol: xc_domain_getinfo_single\\\")\")")
This is because domain_getinfo_v1.cmxs isn't dynamically linked correctly.
Fill in the correct variable, and remove the xen prefix from xenctrl.
Reported-by: Edwin Török <edwin.torok@citrix.com>
Suggested-by: Edwin Török <edwin.torok@citrix.com>
Fixes: a6576011a4 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Allow developers using the OCaml bindings to set the altp2m_count parameter.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Encapsulate the altp2m options within a struct. This change is preparatory
and sets the groundwork for introducing additional parameter in subsequent
commit.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Julien Grall <jgrall@amazon.com> # arm
Reviewed-by: Jan Beulich <jbeulich@suse.com> # hypervisor
OCaml, in preparation for a renaming of the error string associated with
conversion failure in 'int_of_string' functions, started to issue this
warning:
File "process.ml", line 440, characters 13-28:
440 | | (Failure "int_of_string") -> reply_error "EINVAL"
^^^^^^^^^^^^^^^
Warning 52 [fragile-literal-pattern]: 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 11.5)
Deal with this at the source, and instead create our own stable
ConversionFailure exception that's raised on the None case in
'int_of_string_opt'.
'c_int_of_string' is safe and does not raise such exceptions.
Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
... which uses XENVER_extraversion2.
In order to do this sensibly, use manual hypercall buffer handling. Not only
does this avoid an extra bounce buffer (we need to strip the xen_varbuf_t
header anyway), it's also shorter and easlier to follow.
Update libxl and the ocaml stubs to match. No API/ABI change in either.
With this change made, `xl info` can now correctly access a >15 char
extraversion:
# xl info xen_version
4.18-unstable+REALLY LONG EXTRAVERSION
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
ocamlmklib has special handling for C-like '-Wl,-rpath' option, but does
not know how to handle '-Wl,-rpath-link', as evidenced by warnings like:
"Unknown option
-Wl,-rpath-link=$HOME/xen/tools/ocaml/libs/eventchn/../../../../tools/libs/toollog"
Pass this option directly to the compiler with -ccopt instead.
Also pass -L directly to the linker with -ldopt. This prevents embedding absolute
paths from buildtime into binary's RPATH.
Fixes: f7b4e4558b ("tools/ocaml: Fix OCaml libs rules")
Reported-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Tested-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
On Fedora 40, the build fails with:
In file included from domain_getinfo_stubs_v1.c:10:
domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
/usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
275 | struct caml__roots_block *caml__frame = *caml_local_roots_ptr
| ^~~~~~~~~~~
domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
48 | CAMLparam0();
| ^~~~~~~~~~
cc1: all warnings being treated as errors
The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
#define CAMLparam0() \
struct caml__roots_block** caml_local_roots_ptr = \
(DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0, \
&CAML_LOCAL_ROOTS); \
struct caml__roots_block *caml__frame = *caml_local_roots_ptr
We can't modify the macro. But, it turns out there's a CAMLnoreturn macro
with the sole purpose of masking this warning.
Fixes: a6576011a4 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
Signed-off-by: Javi Merino <javi.merino@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
xsd_glue_dev packaging is inconsistent with the rest of OCaml packages and
isn't actually necessary. The .a is needed alongside compiled bytecode files
during linking and was missed in the initial oxenstored plugin work.
Specify OCAMLCFLAGS along with OCAMLOPTFLAGS.
Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Oxenstored dynamically loads the plugin provided in ocaml/libs/xsd_glue.
The plugin is verified to be providing the specified plugin_interface
during its loading.
If a V2 of the plugin is produced, V1 will still be present, and a new
version should only be loaded if it's verified to exist
(New oxenstored can run in an environment with only V1 of the plugin).
The plugin is not switched to as of yet, the old Xenctrl stubs are still
used.
Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This plugin intends to hide the unstable Xenctrl interface under a
stable one. In case of the change in the interface, a V2 of this plugin
would need to be produced, but V1 with the old interface would
need to be kept (with potential change in the implementation) in the
meantime.
To reduce the need for such changes in the future, this plugin only
provides the absolute minimum functionality that Oxenstored uses - only
three fields of the domaininfo struct are used and presented here.
Oxenstored currently uses the single-domain domain_getinfo function,
whereas domain_getinfolist is a potentially more efficient option. Both
of these are provided in the plugin to allow a transition from one to
the other without modifying the interface in the future. Both return
identical structures and rely on the same fields in xenctrl, thus if one
of them breaks, both will break, and a new version of the interface
would need to be issued.
Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The root .gitignore is quite stale from recent (and less recent) removals, but
also fails to work for the forthcoming dynamic plugin work.
Strip all Ocaml content out of the root .gitignore, and provide a more local
.gitignore's with up-to-date patterns.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Dynamic libraries in OCaml require an additional compilation step on top
of the already specified steps for static libraries. Add an appropriate
template to Makefile.rules.
Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
This hides a shell redirection which is quite rude. It also opencodes
$(move-if-changed) without the benefit of short-circuiting dependent logic
when the content hasn't changed.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Edwin Török <edwin.torok@cloud.com>
These days, `ocamlopt -h` asks you whether you meant --help instead, meaning
that the $(shell ) invocation here isn't going end up containing '-g'.
Make it unconditional, like it is in OCAMLCFLAGS already.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
Reviewed-by: Edwin Török <edwin.torok@cloud.com>
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>
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>
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>
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>
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>
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>
`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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
* 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>
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>
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>
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>
Juergen Gross
Andrew Cooper
Petr Beneš
Edgar E. Iglesias
Andrii Sultanov
Javi Merino
Roger Pau Monné
Edwin Török
Luca Fancellu
Alejandro Vallejo