tools: Use PKG_CONFIG_FILE instead of PKG_CONFIG variable
Replace PKG_CONFIG variable name with PKG_CONFIG_FILE for the name of
the pkg-config file.
This is preventing a conflict in some build systems where PKG_CONFIG
actually contains the path to the pkg-config executable to use, as the
default assignment in libs.mk is using a weak assignment (?=).
This problem has been found when trying to build the latest version of
Xen tools using buildroot.
Fixes: d400dc5729e4 ("tools: tweak tools/libs/libs.mk for being able to support libxenctrl") Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: b97e2fe7b9e1f4706693552697239ac2b71efee4
master date: 2023-02-24 17:44:29 +0000
Andrew Cooper [Fri, 3 Mar 2023 07:01:21 +0000 (08:01 +0100)]
xen: Fix Clang -Wunicode diagnostic when building asm-macros
While trying to work around a different Clang-IAS bug (parent changeset), I
stumbled onto:
In file included from arch/x86/asm-macros.c:3:
./arch/x86/include/asm/spec_ctrl_asm.h:144:19: error: \u used with
no following hex digits; treating as '\' followed by identifier [-Werror,-Wunicode]
.L\@_fill_rsb_loop\uniq:
^
It turns out that Clang -E is sensitive to the file extension of the source
file it is processing. Furthermore, C explicitly permits the use of \u
escapes in identifier names, so the diagnostic would be reasonable in
principle if we trying to compile the result.
asm-macros should really have been .S from the outset, as it is ultimately
generating assembly, not C. Rename it, which causes Clang not to complain.
We need to introduce rules for generating a .i file from .S, and substituting
c_flags for a_flags lets us drop the now-redundant -D__ASSEMBLY__.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 53f0d02040b1df08f0589f162790ca376e1c2040
master date: 2023-02-24 17:44:29 +0000
Tamas K Lengyel [Fri, 3 Mar 2023 06:59:08 +0000 (07:59 +0100)]
x86: perform mem_sharing teardown before paging teardown
An assert failure has been observed in p2m_teardown when performing vm
forking and then destroying the forked VM (p2m-basic.c:173). The assert
checks whether the domain's shared pages counter is 0. According to the
patch that originally added the assert (7bedbbb5c31) the p2m_teardown
should only happen after mem_sharing already relinquished all shared pages.
In this patch we flip the order in which relinquish ops are called to avoid
tripping the assert. Conceptually sharing being torn down makes sense to
happen before paging is torn down.
Fixes: e7aa55c0aab3 ("x86/p2m: free the paging memory pool preemptively") Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 2869349f0cb3a89dcbf1f1b30371f58df6309312
master date: 2023-02-23 12:35:48 +0100
Sergey Dyasli [Fri, 3 Mar 2023 06:58:35 +0000 (07:58 +0100)]
x86/ucode/AMD: apply the patch early on every logical thread
The original issue has been reported on AMD Bulldozer-based CPUs where
ucode loading loses the LWP feature bit in order to gain the IBPB bit.
LWP disabling is per-SMT/CMT core modification and needs to happen on
each sibling thread despite the shared microcode engine. Otherwise,
logical CPUs will end up with different cpuid capabilities. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216211
Guests running under Xen happen to be not affected because of levelling
logic for the feature masking/override MSRs which causes the LWP bit to
fall out and hides the issue. The latest recommendation from AMD, after
discussing this bug, is to load ucode on every logical CPU.
In Linux kernel this issue has been addressed by e7ad18d1169c
("x86/microcode/AMD: Apply the patch early on every logical thread").
Follow the same approach in Xen.
Introduce SAME_UCODE match result and use it for early AMD ucode
loading. Take this opportunity and move opt_ucode_allow_same out of
compare_revisions() to the relevant callers and also modify the warning
message based on it. Intel's side of things is modified for consistency
but provides no functional change.
Ross Lagerwall [Fri, 3 Mar 2023 06:58:12 +0000 (07:58 +0100)]
build: make FILE symbol paths consistent
The FILE symbols in out-of-tree builds may be either a relative path to
the object dir or an absolute path depending on how the build is
invoked. Fix the paths for C files so that they are consistent with
in-tree builds - the path is relative to the "xen" directory (e.g.
common/irq.c).
This fixes livepatch builds when the original Xen build was out-of-tree
since livepatch-build always does in-tree builds. Note that this doesn't
fix the behaviour for Clang < 6 which always embeds full paths.
Fixes: 7115fa562fe7 ("build: adding out-of-tree support to the xen build") Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 5b9bb91abba7c983def3b4bef71ab08ad360a242
master date: 2023-02-15 16:13:49 +0100
credit2: respect credit2_runqueue=all when arranging runqueues
Documentation for credit2_runqueue=all says it should create one queue
for all pCPUs on the host. But since introduction
sched_credit2_max_cpus_runqueue, it actually created separate runqueue
per socket, even if the CPUs count is below
sched_credit2_max_cpus_runqueue.
Adjust the condition to skip syblink check in case of
credit2_runqueue=all.
Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue") Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: 1f5747ee929fbbcae58d7234c6c38a77495d0cfe
master date: 2023-02-15 16:12:42 +0100
Architecturally, an event delivery which starts in CPL<3 and switches shadow
stack will first validate the Supervisor Shadow Stack Token (setting the busy
bit), then pushes CS/LIP/SSP. One example of this is an NMI interrupting Xen.
Some CPUs suffer from an issue called fracturing, whereby a fault/vmexit/etc
between setting the busy bit and completing the event injection renders the
action non-restartable, because when it comes time to restart, the busy bit is
found to be already set.
This is far more easily encountered under virt, yet it is not the fault of the
hypervisor, nor the fault of the guest kernel. The fault lies somewhere
between the architectural specification, and the uarch behaviour.
Intel have allocated CPUID.7[1].ecx[18] CET_SSS to enumerate that supervisor
shadow stacks are safe to use. Because of how Xen lays out its shadow stacks,
fracturing is not expected to be a problem on native.
Detect this case on boot and default to not using shstk if virtualised.
Specifying `cet=shstk` on the command line will override this heuristic and
enable shadow stacks irrespective.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 01e7477d1b081cff4288ff9f51ec59ee94c03ee0
master date: 2023-02-09 18:26:17 +0000
Anthony PERARD [Fri, 3 Mar 2023 06:55:24 +0000 (07:55 +0100)]
libs/util: Fix parallel build between flex/bison and CC rules
flex/bison generate two targets, and when those targets are
prerequisite of other rules they are considered independently by make.
We can have a situation where the .c file is out-of-date but not the
.h, git checkout for example. In this case, if a rule only have the .h
file as prerequiste, make will procced and start to build the object.
In parallel, another target can have the .c file as prerequisite and
make will find out it need re-generating and do so, changing the .h at
the same time. This parallel task breaks the first one.
To avoid this scenario, we put both the header and the source as
prerequisite for all object even if they only need the header.
Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: bf652a50fb3bb3b1b3d93db6fb79bc28f978fe75
master date: 2023-02-09 18:26:17 +0000
Anthony PERARD [Tue, 21 Feb 2023 16:55:38 +0000 (16:55 +0000)]
automation: Remove clang-8 from Debian unstable container
First, apt complain that it isn't the right way to add keys anymore,
but hopefully that's just a warning.
Second, we can't install clang-8:
The following packages have unmet dependencies:
clang-8 : Depends: libstdc++-8-dev but it is not installable
Depends: libgcc-8-dev but it is not installable
Depends: libobjc-8-dev but it is not installable
Recommends: llvm-8-dev but it is not going to be installed
Recommends: libomp-8-dev but it is not going to be installed
libllvm8 : Depends: libffi7 (>= 3.3~20180313) but it is not installable
E: Unable to correct problems, you have held broken packages.
clang on Debian unstable is now version 14.0.6.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit a6b1e2b80fe2053b1c9c9843fb086a668513ea36)
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 63305e5392ec2d17b85e7996a97462744425db80)
"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: 8b7ce06a2d34 ("ocaml: Add XC bindings.") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 9e7c74e6f9fd2e44df1212643b80af9032b45b07)
Edwin Török [Thu, 12 Jan 2023 11:38:38 +0000 (11:38 +0000)]
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>
(cherry picked from commit 4250683842104f02996428f93927a035c8e19266)
"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: 22d5affdf0ce ("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>
(cherry picked from commit 2636d8ff7a670c4d2485757dbe966e36c259a960)
Andrew Cooper [Tue, 31 Jan 2023 10:59:42 +0000 (10:59 +0000)]
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: 8b7ce06a2d34 ("ocaml: Add XC bindings.") Fixes: d3e649277a13 ("ocaml: add mmap bindings implementation.") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 36eb2de31b6ecb8787698fb1a701bd708c8971b2)
Edwin Török [Tue, 11 May 2021 15:56:50 +0000 (15:56 +0000)]
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>
(cherry picked from commit e6f07052ce4a0f0b7d4dc522d87465efb2d9ee86)
Edwin Török [Fri, 16 Dec 2022 18:25:20 +0000 (18:25 +0000)]
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>
(cherry picked from commit 01f139215e678c2dc7d4bb3f9f2777069bb1b091)
Edwin Török [Fri, 16 Dec 2022 18:25:10 +0000 (18:25 +0000)]
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>
(cherry picked from commit d2ccc637111d6dbcf808aaffeec7a46f0b1e1c81)
Edwin Török [Tue, 1 Nov 2022 17:59:17 +0000 (17:59 +0000)]
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>
(cherry picked from commit 95db09b1b154fb72fad861815ceae1f3fa49fc4e)
Edwin Török [Tue, 1 Nov 2022 17:59:16 +0000 (17:59 +0000)]
tools/ocaml/xenctrl: Make domain_getinfolist tail recursive
domain_getinfolist() is quadratic with the number of domains, because of the
behaviour of the underlying hypercall. xenopsd was further observed to be
wasting excessive quantites of time manipulating the list of already-obtained
domains.
Implement a tail recursive `rev_concat` equivalent to `concat |> rev`, and use
it instead of calling `@` multiple times.
An incidental benefit is that the list of domains will now be in domid order,
instead of having pairs of 2 domains changing direction every time.
In a scalability testing scenario with ~1000 VMs, a combination of this and
the subsequent 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>
(cherry picked from commit c3b6be714c64aa62b56d0bce96f4b6a10b5c2078)
Jason Andryuk [Tue, 7 Feb 2023 16:01:49 +0000 (17:01 +0100)]
libxl: fix guest kexec - skip cpuid policy
When a domain performs a kexec (soft reset), libxl__build_pre() is
called with the existing domid. Calling libxl__cpuid_legacy() on the
existing domain fails since the cpuid policy has already been set, and
the guest isn't rebuilt and doesn't kexec.
xc: error: Failed to set d1's policy (err leaf 0xffffffff, subleaf 0xffffffff, msr 0xffffffff) (17 = File exists): Internal error
libxl: error: libxl_cpuid.c:494:libxl__cpuid_legacy: Domain 1:Failed to apply CPUID policy: File exists
libxl: error: libxl_create.c:1641:domcreate_rebuild_done: Domain 1:cannot (re-)build domain: -3
libxl: error: libxl_xshelp.c:201:libxl__xs_read_mandatory: xenstore read failed: `/libxl/1/type': No such file or directory
libxl: warning: libxl_dom.c:49:libxl__domain_type: unable to get domain type for domid=1, assuming HVM
During a soft_reset, skip calling libxl__cpuid_legacy() to avoid the
issue. Before commit 34990446ca91, the libxl__cpuid_legacy() failure
would have been ignored, so kexec would continue.
Fixes: 34990446ca91 ("libxl: don't ignore the return value from xc_cpuid_apply_policy") Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: 1e454c2b5b1172e0fc7457e411ebaba61db8fc87
master date: 2023-01-26 10:58:23 +0100
Anthony PERARD [Tue, 7 Feb 2023 16:00:29 +0000 (17:00 +0100)]
build: fix building flask headers before descending in flask/ss/
Unfortunatly, adding prerequisite to "$(obj)/ss/built_in.o" doesn't
work because we have "$(obj)/%/built_in.o: $(obj)/% ;" in Rules.mk.
So, make is allow to try to build objects in "xsm/flask/ss/" before
generating the headers.
Adding a prerequisite on "$(obj)/ss" instead will fix the issue as
that's the target used to run make in this subdirectory.
Unfortunatly, that target is also used when running `make clean`, so
we want to ignore it in this case. $(MAKECMDGOALS) can't be used in
this case as it is empty, but we can guess which operation is done by
looking at the list of loaded makefiles.
Fixes: 7a3bcd2babcc ("build: build everything from the root dir, use obj=$subdir") Reported-by: "Daniel P. Smith" <dpsmith@apertussolutions.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: d60324d8af9404014cfcc37bba09e9facfd02fcf
master date: 2023-01-23 15:03:58 +0100
Andrew Cooper [Tue, 7 Feb 2023 15:59:14 +0000 (16:59 +0100)]
x86/vmx: Support for CPUs without model-specific LBR
Ice Lake (server at least) has both architectural LBR and model-specific LBR.
Sapphire Rapids does not have model-specific LBR at all. I.e. On SPR and
later, model_specific_lbr will always be NULL, so we must make changes to
avoid reliably hitting the domain_crash().
The Arch LBR spec states that CPUs without model-specific LBR implement
MSR_DBG_CTL.LBR by discarding writes and always returning 0.
Do this for any CPU for which we lack model-specific LBR information.
Adjust the now-stale comment, now that the Arch LBR spec has created a way to
signal "no model specific LBR" to guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
master commit: 3edca52ce736297d7fcf293860cd94ef62638052
master date: 2023-01-12 18:42:00 +0000
Andrew Cooper [Tue, 7 Feb 2023 15:58:25 +0000 (16:58 +0100)]
x86/vmx: Calculate model-specific LBRs once at start of day
There is no point repeating this calculation at runtime, especially as it is
in the fallback path of the WRSMR/RDMSR handlers.
Move the infrastructure higher in vmx.c to avoid forward declarations,
renaming last_branch_msr_get() to get_model_specific_lbr() to highlight that
these are model-specific only.
No practical change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
master commit: e94af0d58f86c3a914b9cbbf4d9ed3d43b974771
master date: 2023-01-12 18:42:00 +0000
Jan Beulich [Tue, 7 Feb 2023 15:57:52 +0000 (16:57 +0100)]
include/compat: produce stubs for headers not otherwise generated
Public headers can include other public headers. Such interdependencies
are retained in their compat counterparts. Since some compat headers are
generated only in certain configurations, the referenced headers still
need to exist. The lack thereof was observed with hvm/hvm_op.h needing
trace.h, where generation of the latter depends on TRACEBUFFER=y. Make
empty stubs in such cases (as generating the extra headers is relatively
slow and hence better to avoid). Changes to .config and incrementally
(re-)building is covered by the respective .*.cmd then no longer
matching the command to be used, resulting in the necessary re-creation
of the (possibly stub) header.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: 6bec713f871f21c6254a5783c1e39867ea828256
master date: 2023-01-12 16:17:54 +0100
Anthony PERARD [Tue, 7 Feb 2023 15:57:22 +0000 (16:57 +0100)]
tools: Fix build with recent QEMU, use "--enable-trace-backends"
The configure option "--enable-trace-backend" isn't accepted anymore
and we should use "--enable-trace-backends" instead which was
introduce in 2014 and allow multiple backends.
"--enable-trace-backends" was introduced by: 5b808275f3bb ("trace: Multi-backend tracing")
The backward compatible option "--enable-trace-backend" is removed by 10229ec3b0ff ("configure: remove backwards-compatibility and obsolete options")
As we already use ./configure options that wouldn't be accepted by
older version of QEMU's configure, we will simply use the new spelling
for the option and avoid trying to detect which spelling to use.
We already make use if "--firmwarepath=" which was introduced by 3d5eecab4a5a ("Add --firmwarepath to configure")
which already include the new spelling for "--enable-trace-backends".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
master commit: e66d450b6e0ffec635639df993ab43ce28b3383f
master date: 2023-01-11 10:45:29 +0100
I'm observing guest kexec trigger xenstored to abort on a double free.
gdb output:
Program received signal SIGABRT, Aborted.
__pthread_kill_implementation (no_tid=0, signo=6, threadid=140645614258112) at ./nptl/pthread_kill.c:44
44 ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
at ./nptl/pthread_kill.c:44
at ./nptl/pthread_kill.c:78
at ./nptl/pthread_kill.c:89
at ../sysdeps/posix/raise.c:26
at talloc.c:119
ptr=ptr@entry=0x559fae724290) at talloc.c:232
at xenstored_core.c:2945
(gdb) frame 5
at talloc.c:119
119 TALLOC_ABORT("Bad talloc magic value - double free");
(gdb) frame 7
at xenstored_core.c:2945
2945 talloc_increase_ref_count(conn);
(gdb) p conn
$1 = (struct connection *) 0x559fae724290
The trace shows that DESTROY was called for connection 0x559fae724290,
but that is the same pointer (conn) main() was looping through from
connections. So it wasn't actually removed from the connections list?
Reverting commit e8e6e42279a5 "tools/xenstore: simplify loop handling
connection I/O" fixes the abort/double free. I think the use of
list_for_each_entry_safe is incorrect. list_for_each_entry_safe makes
traversal safe for deleting the current iterator, but RELEASE/do_release
will delete some other entry in the connections list. I think the
observed abort is because list_for_each_entry has next pointing to the
deleted connection, and it is used in the subsequent iteration.
Add a comment explaining the unsuitability of list_for_each_entry_safe.
Also notice that the old code takes a reference on next which would
prevents a use-after-free.
Andrew Cooper [Thu, 1 Dec 2022 21:06:25 +0000 (21:06 +0000)]
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: ee7815f49faf ("tools/oxenstored: Set uncaught exception handler") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit d2162d884cba0ff7b2ac0d832f4e044444bda2e1)
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>
(cherry picked from commit acd3fb6d65905f8a185dcb9fe6a330a591b96203)
Edwin Török [Mon, 7 Nov 2022 17:41:36 +0000 (17:41 +0000)]
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>
(cherry picked from commit ee7815f49faf743e960dac9e72809eb66393bc6d)
Edwin Török [Tue, 8 Nov 2022 08:57:47 +0000 (08:57 +0000)]
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>
(cherry picked from commit 3f02e0a70fe9f8143454b742563433958d4a87f8)
Edwin Török [Thu, 3 Nov 2022 15:31:39 +0000 (15:31 +0000)]
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>
(cherry picked from commit 9b224c25293a53fcbe32da68052d861dda71a6f4)
Andrew Cooper [Wed, 30 Nov 2022 11:59:34 +0000 (11:59 +0000)]
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>
(cherry picked from commit df2db174b36eba67c218763ef621c67912202fc6)
Andrew Cooper [Wed, 30 Nov 2022 11:55:58 +0000 (11:55 +0000)]
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>
(cherry picked from commit aecdc28d9538ca2a1028ef9bc6550cb171dbbed4)
Andrew Cooper [Wed, 30 Nov 2022 03:17:28 +0000 (03:17 +0000)]
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>
(cherry picked from commit 31fbee749a75621039ca601eaee7222050a7dd83)
Andrew Cooper [Tue, 29 Nov 2022 21:05:43 +0000 (21:05 +0000)]
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>
(cherry picked from commit 9804a5db435fe40c8ded8cf36c2d2b2281c56f1d)
Andrew Cooper [Wed, 30 Nov 2022 14:56:43 +0000 (14:56 +0000)]
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>
(cherry picked from commit b45bfaf359e4821b1bf98a4fcd194d7fd176f167)
Edwin Török [Thu, 3 Nov 2022 14:50:38 +0000 (14:50 +0000)]
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>
(cherry picked from commit 9bafe4a53306e7aa2ce6ffc96f7477c6f329f7a7)
Edwin Török [Mon, 14 Nov 2022 13:36:19 +0000 (13:36 +0000)]
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>
(cherry picked from commit 7ba68a6c558e1fd811c95cb7215a5cd07a3cc2ea)
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>
(cherry picked from commit 22d5affdf0cecfa6faae46fbaec68b8018835220)
Andrew Cooper [Fri, 11 Nov 2022 18:50:34 +0000 (18:50 +0000)]
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: 42f0581a91d4 ("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>
(cherry picked from commit ee36179371fd4215a43fb179be2165f65c1cd1cd)
Neowutran [Tue, 20 Dec 2022 12:46:38 +0000 (13:46 +0100)]
x86/time: prevent overflow with high frequency TSCs
Make sure tsc_khz is promoted to a 64-bit type before multiplying by
1000 to avoid an 'overflow before widen' bug. Otherwise just above
4.294GHz the value will overflow. Processors with clocks this high are
now in production and require this to work correctly.
Avoid incorrectly triggering an error when a broadcast buffered ioreq
is not handled by all registered clients, as long as the failure is
strictly because the client doesn't handle buffered ioreqs.
Signed-off-by: Per Bilse <per.bilse@citrix.com> Reviewed-by: Paul Durrant <paul@xen.org>
master commit: a44734df6c24fadbdb001f051cc5580c467caf7d
master date: 2022-12-07 12:17:30 +0100
Jan Beulich [Tue, 20 Dec 2022 12:45:07 +0000 (13:45 +0100)]
x86/HVM: don't mark evtchn upcall vector as pending when vLAPIC is disabled
Linux'es relatively new use of HVMOP_set_evtchn_upcall_vector has
exposed a problem with the marking of the respective vector as
pending: For quite some time Linux has been checking whether any stale
ISR or IRR bits would still be set while preparing the LAPIC for use.
This check is now triggering on the upcall vector, as the registration,
at least for APs, happens before the LAPIC is actually enabled.
In software-disabled state an LAPIC would not accept any interrupt
requests and hence no IRR bit would newly become set while in this
state. As a result it is also wrong for us to mark the upcall vector as
having a pending request when the vLAPIC is in this state.
To compensate for the "enabled" check added to the assertion logic, add
logic to (conditionally) mark the upcall vector as having a request
pending at the time the LAPIC is being software-enabled by the guest.
Note however that, like for the pt_may_unmask_irq() we already have
there, long term we may need to find a different solution. This will be
especially relevant in case yet better LAPIC acceleration would
eliminate notifications of guest writes to this and other registers.
Fixes: 7b5b8ca7dffd ("x86/upcall: inject a spurious event after setting upcall vector") Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: f5d0279839b58cb622f0995dbf9cff056f03082e
master date: 2022-12-06 13:51:49 +0100
Jan Beulich [Tue, 20 Dec 2022 12:44:38 +0000 (13:44 +0100)]
x86/Viridian: don't mark IRQ vectors as pending when vLAPIC is disabled
In software-disabled state an LAPIC does not accept any interrupt
requests and hence no IRR bit would newly become set while in this
state. As a result it is also wrong for us to mark Viridian IPI or timer
vectors as having a pending request when the vLAPIC is in this state.
Such interrupts are simply lost.
Introduce a local variable in send_ipi() to help readability.
Fixes: fda96b7382ea ("viridian: add implementation of the HvSendSyntheticClusterIpi hypercall") Fixes: 26fba3c85571 ("viridian: add implementation of synthetic timers") Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Paul Durrant <paul@xen.org>
master commit: 831419f82913417dee4e5b0f80769c5db590540b
master date: 2022-12-02 10:35:32 +0100
Jan Beulich [Tue, 20 Dec 2022 12:44:07 +0000 (13:44 +0100)]
x86/HVM: don't mark external IRQs as pending when vLAPIC is disabled
In software-disabled state an LAPIC does not accept any interrupt
requests and hence no IRR bit would newly become set while in this
state. As a result it is also wrong for us to mark IO-APIC or MSI
originating vectors as having a pending request when the vLAPIC is in
this state. Such interrupts are simply lost.
Introduce (IO-APIC) or re-use (MSI) a local variable to help
readability.
Fixes: 4fe21ad3712e ("This patch add virtual IOAPIC support for VMX guest") Fixes: 85715f4bc7c9 ("MSI 5/6: add MSI support to passthrough HVM domain") Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: f1d7aac1e3c3cd164e17d41791a575a5c3e87121
master date: 2022-12-02 10:35:01 +0100
Roger Pau Monné [Tue, 20 Dec 2022 12:43:37 +0000 (13:43 +0100)]
x86/pvh: do not forward MADT Local APIC NMI structures to dom0
Currently Xen will passthrough any Local APIC NMI Structure found in
the native ACPI MADT table to a PVH dom0. This is wrong because PVH
doesn't have access to the physical local APIC, and instead gets an
emulated local APIC by Xen, that doesn't have the LINT0 or LINT1
pins wired to anything. Furthermore the ACPI Processor UIDs used in
the APIC NMI Structures are likely to not match the ones generated by
Xen for the Local x2APIC Structures, creating confusion to dom0.
Fix this by removing the logic to passthrough the Local APIC NMI
Structure for PVH dom0.
Roger Pau Monné [Tue, 20 Dec 2022 12:43:04 +0000 (13:43 +0100)]
x86/irq: do not release irq until all cleanup is done
Current code in _clear_irq_vector() will mark the irq as unused before
doing the cleanup required when move_in_progress is true.
This can lead to races in create_irq() if the function picks an irq
desc that's been marked as unused but has move_in_progress set, as the
call to assign_irq_vector() in that function can then fail with
-EAGAIN.
Prevent that by only marking irq descs as unused when all the cleanup
has been done. While there also use write_atomic() when setting
IRQ_UNUSED in _clear_irq_vector() and add a barrier in order to
prevent the setting of IRQ_UNUSED getting reordered by the compiler.
The check for move_in_progress cannot be removed from
_assign_irq_vector(), as other users (io_apic_set_pci_routing() and
ioapic_guest_write()) can still pass active irq descs to
assign_irq_vector().
Note the trace point is not moved and is now set before the irq is
marked as unused. This is done so that the CPU mask provided in the
trace point is the one belonging to the current vector, not the old
one.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: e267d11969a40f0aec33dbf966f5a6490b205f43
master date: 2022-12-02 10:32:21 +0100
A previous patch tried to get Linux to use the ESRT under Xen if it is
in memory of type EfiRuntimeServicesData. However, this turns out to be
a bad idea. Ard Biesheuvel pointed out that EfiRuntimeServices* memory
winds up fragmenting both the EFI page tables and the direct map, and
that EfiACPIReclaimMemory is a much better choice for this purpose.
Jan Beulich [Fri, 2 Dec 2022 09:42:24 +0000 (10:42 +0100)]
x86/HVM+shim: fix build when !CONFIG_GRANT_TABLE
The HVM flavor of the hypercall handlers exists only when GRANT_TABLE is
enabled, while surrogate shim variants exist only for the purpose of PV.
(Also scratch out the Arm variant in that case; what exactly is used in
that cell of the new table row doesn't really matter.)
Fixes: 8523851dbc49 ("xen/x86: call hypercall handlers via generated macro") Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
master commit: 133f35f0f0cd0a415e599c20156aaaa5598356c0
master date: 2022-12-02 10:40:11 +0100
Anthony PERARD [Thu, 1 Dec 2022 15:28:43 +0000 (16:28 +0100)]
tools/include: fix clean and rework COPYING for installed Xen public header
Use actual include directory used to install the public header in
COPYING file.
Also, move the input file out of "tools/include/xen/" because that
directory is removed on `make clean`.
We can't used ./configure because $includedir contain another
variable, so the change is done in Makefile.
Fixes: 4ea75e9a9058 ("Rework COPYING installed in /usr/include/xen/, due to several licences") Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
master commit: 25b55688e1f20ebb1bb15d0e82079e91cd787e5b
master date: 2022-12-01 16:14:59 +0100
Michal Orzel [Fri, 18 Nov 2022 11:45:54 +0000 (12:45 +0100)]
docs/misc/arm: Update references to Linux kernel docs
Some time ago, Linux switched the format of docs to ReST and the format
of device-tree bindings to json-schema.
Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Thu, 24 Nov 2022 02:50:40 +0000 (10:50 +0800)]
Changelog: Add __ro_after_init and CET
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Henry Wang <Henry.Wang@arm.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
Andrew Cooper [Mon, 21 Nov 2022 12:46:39 +0000 (12:46 +0000)]
xen/flask: Wire up XEN_DOMCTL_{get,set}_paging_mempool_size
These were overlooked in the original patch, and noticed by OSSTest which does
run some Flask tests.
Fixes: 22b20bd98c02 ("xen: Introduce non-broken hypercalls for the paging mempool size") Suggested-by: Daniel Smith <dpsmith@apertussolutions.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Fri, 18 Nov 2022 16:53:45 +0000 (16:53 +0000)]
tools/libxl: Fixes to libxl__domain_set_paging_mempool_size()
The error message accidentally printed the bytes value as if it were kB.
Furthermore, both b_info.shadow_memkb and shadow_mem are uint64_t, meaning
there is a risk of overflow if the user specified a stupidly large value in
the vm.cfg file. Check and reject such a condition.
Fixes: 7c3bbd940dd8 ("xen/arm, libxl: Revert XEN_DOMCTL_shadow_op; use p2m mempool hypercalls") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Commit 34990446ca91 started to overwrite the `rc` value from
libxl__arch_domain_create(), thus error aren't propagated anymore.
Check `rc` value before doing the next thing.
Fixes: 34990446ca91 ("libxl: don't ignore the return value from xc_cpuid_apply_policy") Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Roger Pau Monné [Mon, 21 Nov 2022 11:21:51 +0000 (12:21 +0100)]
efifb: ignore frame buffer with invalid configuration
On one of my boxes when the HDMI cable is not plugged in the
FrameBufferBase of the EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE structure is
set to 0 by the firmware (while some of the other fields looking
plausible).
Such (bogus address) ends up mapped in vesa_init(), and since it
overlaps with a RAM region the whole system goes down pretty badly,
see:
(XEN) vesafb: framebuffer at 0x0000000000000000, mapped to 0xffff82c000201000, using 35209k, total 35209k
(XEN) vesafb: mode is 0x37557x32, linelength=960, font 8x16
(XEN) vesafb: Truecolor: size=8:8:8:8, shift=24:0:8:16
(XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) �ERROR: Class:0; Subclass:0; Operation: 0
ERROR: No ConOut
ERROR: No ConIn
Do like Linux and prevent using the EFI Frame Buffer if the base
address is 0. This is inline with the logic in Linuxes
fb_base_is_valid() function at drivers/video/fbdev/efifb.c v6.0.9.
Andrew Cooper [Thu, 20 Oct 2022 11:14:30 +0000 (12:14 +0100)]
xen/arm: Correct the p2m pool size calculations
Allocating or freeing p2m pages doesn't alter the size of the mempool; only
the split between free and used pages.
Right now, the hypercalls operate on the free subset of the pool, meaning that
XEN_DOMCTL_get_paging_mempool_size varies with time as the guest shuffles its
physmap, and XEN_DOMCTL_set_paging_mempool_size ignores the used subset of the
pool and lets the guest grow unbounded.
This fixes test-pagign-mempool on ARM so that the behaviour matches x86.
This is part of XSA-409 / CVE-2022-33747.
Fixes: cbea5a1149ca ("xen/arm: Allocate and free P2M pages from the P2M pool") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Julien Grall <jgrall@amazon.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
First of all, with ARM borrowing x86's implementation, the logic to set the
pool size should have been common, not duplicated. Introduce
libxl__domain_set_paging_mempool_size() as a shared implementation, and use it
from the ARM and x86 paths. It is left as an exercise to the reader to judge
how libxl/xl can reasonably function without the ability to query the pool
size...
Remove ARM's p2m_domctl() infrastructure now the functioanlity has been
replaced with a working and unit tested interface.
This is part of XSA-409 / CVE-2022-33747.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Thu, 20 Oct 2022 11:13:46 +0000 (12:13 +0100)]
tools/tests: Unit test for paging mempool size
Exercise some basic functionality of the new
xc_{get,set}_paging_mempool_size() hypercalls.
This passes on x86, but fails currently on ARM. ARM will be fixed up in
future patches.
This is part of XSA-409 / CVE-2022-33747.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Fri, 21 Oct 2022 13:13:00 +0000 (14:13 +0100)]
xen: Introduce non-broken hypercalls for the paging mempool size
The existing XEN_DOMCTL_SHADOW_OP_{GET,SET}_ALLOCATION have problems:
* All set_allocation() flavours have an overflow-before-widen bug when
calculating "sc->mb << (20 - PAGE_SHIFT)".
* All flavours have a granularity of 1M. This was tolerable when the size of
the pool could only be set at the same granularity, but is broken now that
ARM has a 16-page stopgap allocation in use.
* All get_allocation() flavours round up, and in particular turn 0 into 1,
meaning the get op returns junk before a successful set op.
* The x86 flavours reject the hypercalls before the VM has vCPUs allocated,
despite the pool size being a domain property.
* Even the hypercall names are long-obsolete.
Implement a better interface, which can be first used to unit test the
behaviour, and subsequently correct a broken implementation. The old
interface will be retired in due course.
The unit of bytes (as opposed pages) is a deliberate API/ABI improvement to
more easily support multiple page granularities.
This is part of XSA-409 / CVE-2022-33747.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Mon, 14 Nov 2022 21:47:59 +0000 (21:47 +0000)]
x86/hvm: Revert per-domain APIC acceleration support
I was really hoping to avoid this, but its now too late in the 4.17 freeze and
we still don't have working fixes.
The in-Xen calculations for assistance capabilities are buggy. For the
avoidance of doubt, the original intention was to be able to control every
aspect of a APIC acceleration so we could comprehensively test Xen's support,
as it has proved to be buggy time and time again.
Even after a protracted discussion on what the new API ought to mean, attempts
to apply it to the existing logic have been unsuccessful, proving that the
API/ABI is too complicated for most people to reason about.
leaving in place the non-APIC specific changes (minimal as they are).
This takes us back to the behaviour of Xen 4.16 where APIC acceleration is
configured on a per system basis.
This work will be revisted in due course.
Fixes: 2ce11ce249a3 ("x86/HVM: allow per-domain usage of hardware virtualized APIC") Fixes: 6b2b9b340509 ("x86: report Interrupt Controller Virtualization capabilities") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Anthony PERARD [Thu, 3 Nov 2022 11:52:04 +0000 (11:52 +0000)]
xen: Used SPDX identifier in public headers
The text of the licence has been check to be the same as the one at
https://spdx.org/licenses/MIT.html, except we don't have "(including
the next paragraph)".
Mecanical change done with a script.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Juergen Gross <jgross@suse.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Anthony PERARD [Thu, 3 Nov 2022 11:52:03 +0000 (11:52 +0000)]
Rework COPYING installed in /usr/include/xen/, due to several licences
The notice in the COPYING file in "xen/include/public/COPYING" doesn't
really apply to the files that ultimately are been install at
"/usr/include/xen". The issue are headers in the "sys/" subdirectory
that comes from other projects such as Linux or FreeBSD.
The main issue is that there are two headers that have a different
licence than the MIT licence:
- xen-sys/Linux/gntalloc.h (installed as "sys/gntalloc.h") is public
domain.
- xen-sys/FreeBSD/gntdev.h (installed as "sys/gntdev.h") is BSD-2.
To clarify this, we'll install a COPYING file with a different notice.
Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Anthony PERARD [Thu, 3 Nov 2022 11:52:02 +0000 (11:52 +0000)]
xen: Add licence header to device_tree_defs.h
This header have been created by moving code from other part of the
project and miss a licence header. The original source code was some
version of GPL or LGPL but we intend to have the public header to be
MIT so they can be included easily in other projects.
Part of device_tree_defs.h were moved from libxl_arm.c which is
LGPL-2.1-only. And part were moved from device_tree.h that is
GPL-2.0-only.
Part of the original code were added by Julien Grall @ Linaro in
commits c3ba52a84dd8 and 405c167f0ec9 and 886f34045bf0. The other part
were added by Ian Campbell @ Citrix, with commit 0c64527e7fc9.
Resolves: xen-project/xen#35 Fixes: 1c898a9fec7e ("xen/arm: move a few DT related defines to public/device_tree_defs.h") Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> [Citrix relicensing] Acked-by: Grant Likely <grant.likely@linaro.org> [Linaro relicensing] Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Anthony PERARD [Thu, 3 Nov 2022 11:52:01 +0000 (11:52 +0000)]
tools/include/xen-foreign: Add SPDX identifier to generated headers
The headers install in "/usr/include/xen/foreign/" are missing a
licence header. This patch adds a SPDX identifier to clarify that
the MIT licence is used.
The script now check that the licence of the input file is also MIT,
by checking for the presence of the SPDX identifier.
Also add information about which files are used to generate the
headers.
Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Anthony PERARD [Thu, 3 Nov 2022 11:52:00 +0000 (11:52 +0000)]
xen: Used SPDX identifier in some public headers
The script "tools/include/xen-foreign/mkheader.py" is going to do a
sanity check on the licences of these headers. To ease this, we will
replace the verbatim copy of the MIT licence by its SPDX identifier
equivalent.
The text of the licence has been check to be the same as the one at
https://spdx.org/licenses/MIT.html, except we don't have "(including
the next paragraph)". The text is also the same as the one in
"xen/include/public/COPYING".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Anthony PERARD [Thu, 3 Nov 2022 11:51:59 +0000 (11:51 +0000)]
xen: Add licence information to public/errno.h
Fixes: 81f559e97974 ("make error codes a formal part of the ABI") Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Mon, 14 Nov 2022 21:41:08 +0000 (21:41 +0000)]
x86/spec-ctrl: Fill in whitepaper URL
... now that we a link available.
Fixes: 9deaf2d932f0 ("x86/spec-ctrl: Enable Zen2 chickenbit") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
We use CC-BY-4.0 for many of the documents under docs/ so we should have
a copy of the license.
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Roger Pau Monne [Tue, 15 Nov 2022 13:26:56 +0000 (14:26 +0100)]
amd: remove VIRT_SC_MSR_HVM synthetic feature
With the previous bugfix, X86_FEATURE_VIRT_SC_MSR_HVM is no longer
needed and can be replaced with an __initdata variable. This also
leaves asm/cpufeatures.h as it was in 4.16 which will simplify
backports.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Rewrite commit message. Move amd_virt_spec_ctrl into __initdata.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Roger Pau Monne [Tue, 15 Nov 2022 13:26:55 +0000 (14:26 +0100)]
amd/virt_ssbd: set SSBD at vCPU context switch
This fixes an issue with running C code in a GIF=0 region, that's
problematic when using UBSAN or other instrumentation techniques.
The current logic for AMD SSBD context switches it on every
vm{entry,exit} if the Xen and guest selections don't match. This is
expensive when not using SPEC_CTRL, and hence should be avoided as
much as possible.
When SSBD is not being set from SPEC_CTRL on AMD don't context switch
at vm{entry,exit} and instead only context switch SSBD when switching
vCPUs. This has the side effect of running Xen code with the guest
selection of SSBD, the documentation is updated to note this behavior.
Also note that then when `ssbd` is selected on the command line guest
SSBD selection will not have an effect, and the hypervisor will run
with SSBD unconditionally enabled when not using SPEC_CTRL itself.
As a result of no longer running the code to set SSBD in a GIF=0
region the locking of amd_set_legacy_ssbd() can be done using normal
spinlocks, and some more checks can be added to assure it works as
intended.
Finally it's also worth noticing that since the guest SSBD selection
is no longer set on vmentry the VIRT_SPEC_MSR handling needs to
propagate the value to the hardware as part of handling the wrmsr.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Extend the msrs->virt_spec_ctrl context switching comment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
xen/arm: vGICv3: Restore the interrupt state correctly
As "spin_lock_irqsave(&v->arch.vgic.lock, flags)" saves the current interrupt
state in "flags", "spin_unlock_irqrestore(&v->arch.vgic.lock, flags)" should be
used to restore the saved interrupt state.
Fixes: fe7fa1332dabd9ce4 ("ARM: vGICv3: handle virtual LPI pending and property tables") Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Acked-by: Julien Grall <jgrall@amazon.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Luca Fancellu [Wed, 9 Nov 2022 14:04:20 +0000 (14:04 +0000)]
arm: fix Kconfig symbol dependency on arm features
The commit 3c2a14ea81c7 is introducing some unsupported arm features
that by default are disabled and are used for the cpufeature.c code.
As they are disabled by default, a typo in the Kconfig symbol they
depend on has landed in the codebase unnoticed, instead of depending
on ARM64 which does not exist, fix the code to depend on ARM_64 that
is the intended symbol.
Fixes: 3c2a14ea81c7 ("arm: Define kconfig symbols used by arm64 cpufeatures") Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Jan Beulich [Mon, 7 Nov 2022 15:09:13 +0000 (16:09 +0100)]
kexec: restore hypercall 1st arg's type
This reverts a small part of 7e21b25059ed ("xen: harmonize return types
of hypercall handlers"). The change from "unsigned long" to "unsigned
int" for the native handler function meant that previously invalid
values became valid. While perhaps not a significant issue, strictly
speaking that's still a change to the ABI. Don't go as far as restoring
the compat entry point's type though: That one can't have values passed
which don't fit in 32 bits.
Note that as a side effect this fixes the invocation of
hypercall_create_continuation(), which by mistake wasn't adjusted by the
earlier change.
Also take the opportunity and correct the respective comment in the
public header. (The way it was it really supports that it probably was
pointless to use "long", but that's the way the hypercall was
introduced.)
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Juergen Gross <jgross@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
The dumping of stacks for HVM guests is problematic, since it requires
taking the p2m lock in order to walk the guest page tables and the p2m.
The suggested solution to the issue is to introduce and use a lockless p2m
walker, that relies on being executed with interrupts disabled in order to
prevent any p2m pages from being freed while doing the walk.
Note that modifications of p2m entries are already done atomically in order
to prevent the hardware walker from seeing partially updated values.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Juergen Gross [Wed, 9 Nov 2022 09:48:59 +0000 (10:48 +0100)]
tools/hotplug: fix systemd unit dependencies
Commit 1283af6465cd ("tools/xenstore: remove XEN_LIB_STORED and
XENSTORED_ROOTDIR") removed the systemd file var-lib-xenstored.mount
without removing dependencies to this file.
Fixes: 1283af6465cd ("tools/xenstore: remove XEN_LIB_STORED and XENSTORED_ROOTDIR") Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Edwin Török [Wed, 9 Nov 2022 09:48:33 +0000 (10:48 +0100)]
tools/ocaml/xenstored/store.ml: fix build error
Building with Dune in release mode fails with:
```
File "ocaml/xenstored/store.ml", line 464, characters 13-32:
Warning 18: this type-based record disambiguation is not principal.
File "ocaml/xenstored/store.ml", line 1:
Error: Some fatal warnings were triggered (1 occurrences)
```
This is a warning to help keep the code futureproof, quoting from its
documentation:
> Check information path during type-checking, to make sure that all types are
> derived in a principal way. When using labelled arguments and/or polymorphic
> methods, this flag is required to ensure future versions of the compiler will
> be able to infer types correctly, even if internal algorithms change. All
> programs accepted in -principal mode are also accepted in the default mode with
> equivalent types, but different binary signatures, and this may slow down type
> checking; yet it is a good idea to use it once before publishing source code.
Fixes: db471408edd46 "tools/ocaml/xenstored: Fix quota bypass on domain shutdown" Signed-off-by: Edwin Török <edvin.torok@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Andrew Cooper [Tue, 14 Jun 2022 15:18:36 +0000 (16:18 +0100)]
x86/spec-ctrl: Mitigate IBPB not flushing the RSB/RAS
Introduce spec_ctrl_new_guest_context() to encapsulate all logic pertaining to
using MSR_PRED_CMD for a new guest context, even if it only has one user
presently.
Introduce X86_BUG_IBPB_NO_RET, and use it extend spec_ctrl_new_guest_context()
with a manual fixup for hardware which mis-implements IBPB.
This is part of XSA-422 / CVE-2022-23824.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
projects / people / tklengyel / xen.git / log