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>
Wei Chen [Fri, 4 Nov 2022 10:07:32 +0000 (18:07 +0800)]
xen/arm: add iounmap after initrd has been loaded in domain_build
domain_build use ioremap_wc to map a new non-cacheable virtual
address for initrd. After Xen copy initrd from this address to
guest, this new allocated virtual address has not been unmapped.
So in this patch, we add an iounmap to the end of domain_build,
after Xen loaded initrd to guest memory.
Signed-off-by: Wei Chen <wei.chen@arm.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Julien Grall <jgrall@amazon.com>
xen/arm: vGICv3: Emulate properly 32-bit access on GICR_PENDBASER
If a guest is running in 32 bit mode and it tries to access
"GICR_PENDBASER + 4" mmio reg, it will be trapped to Xen. vreg_reg64_extract()
will return the value stored "v->arch.vgic.rdist_pendbase + 4".
This will be stored in a 64bit cpu register.
So now we have the top 32 bits of GICR_PENDBASER (a 64 bit MMIO register) stored
in the lower 32 bits of the 64bit cpu register.
This 64bit cpu register is then modified bitwise with a mask (ie
GICR_PENDBASER_PTZ, it clears the 62nd bit). But the PTZ (which is bit 30 in the
64 bit cpu register) is not cleared as expected by the specification.
The correct thing to do here is to store the value of
"v->arch.vgic.rdist_pendbase" in a temporary 64 bit variable. This variable is
then modified bitwise with GICR_PENDBASER_PTZ mask. It is then passed to
vreg_reg64_extract() which will extract 32 bits from the given offset.
Also, we have removed spin_lock_irqsave()/spin_unlock_irqrestore() to protect
v->arch.vgic.rdist_pendbase in __vgic_v3_rdistr_rd_mmio_read(). The reason
being v->arch.vgic.rdist_pendbase is now being read in an atomic manner.
Similarly in __vgic_v3_rdistr_rd_mmio_write(), we have used read_atomic(),
write_atomic() to read/write v->arch.vgic.rdist_pendbase.
Fixes: fe7fa1332dabd9ce4 ("ARM: vGICv3: handle virtual LPI pending and property tables") Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
Juergen Gross [Fri, 4 Nov 2022 08:03:23 +0000 (09:03 +0100)]
xen/sched: migrate timers to correct cpus after suspend
Today all timers are migrated to cpu 0 when the system is being
suspended. They are not migrated back after resuming the system again.
This results (at least) to visible problems with the credit scheduler,
as the timer isn't handled on the cpu it was expected to occur, which
will result in an ASSERT() triggering. Other more subtle problems, like
uninterrupted elongated time slices, are probable. The least effect
will be worse performance on cpu 0 resulting from most scheduling
related timer interrupts happening there after suspend/resume.
Add migrating the scheduling related timers of a specific cpu from cpu
0 back to its original cpu when that cpu has gone up when resuming the
system.
Fixes: 0763cd268789 ("xen/sched: don't disable scheduler on cpus during suspend") Signed-off-by: Juergen Gross <jgross@suse.com> Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Acked-by: Dario Faggioli <dfaggioli@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Juergen Gross [Fri, 4 Nov 2022 07:54:57 +0000 (08:54 +0100)]
xen: fix generated code for calling hypercall handlers
The code generated for the call_handlers_*() macros needs to avoid
undefined behavior when multiple handlers share the same priority.
The issue is the hypercall number being unverified fed into the macros
and then used to set a mask via "mask = 1ULL << <hypercall-number>".
Avoid a shift amount of more than 63 by setting mask to zero in case
the hypercall number is too large.
Fixes: eca1f00d0227 ("xen: generate hypercall interface related code") Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
drivers/char: suspend handling in XHCI console driver
Similar to the EHCI driver - save/restore relevant BAR and command
register, re-configure DbC on resume and stop/start timer.
On resume trigger sending anything that was queued in the meantime.
Save full BAR value, instead of just the address part, to ease restoring
on resume.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Acked-by: Jan Beulich <jbeulich@suse.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
IOMMU/VT-d: wire common device reserved memory API
Re-use rmrr= parameter handling code to handle common device reserved
memory.
Move MAX_USER_RMRR_PAGES limit enforcement to apply only to
user-configured ranges, but not those from internal callers.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Juergen Gross [Wed, 2 Nov 2022 11:08:22 +0000 (12:08 +0100)]
tools/xenstore: call remove_domid_from_perm() for special nodes
When destroying a domain, any stale permissions of the domain must be
removed from the special nodes "@...", too. This was not done in the
fix for XSA-322.
Fixes: 496306324d8d ("tools/xenstore: revoke access rights for removed domains") Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Julien Grall <jgrall@amazon.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Juergen Gross [Wed, 2 Nov 2022 11:07:57 +0000 (12:07 +0100)]
tools/xenstore: remove XEN_LIB_STORED and XENSTORED_ROOTDIR
XEN_LIB_STORED is serving no real purpose, as it is a mount point for
a tmpfs, so it can be replaced easily by XEN_RUN_STORED.
XENSTORED_ROOTDIR is basically unused already, there is just a single
reference in xs_daemon_rootdir() with a fallback to XEN_LIB_STORED,
and a .gdbinit file setting it.
Remove the .gdbinit file, as it is not known having been used since
ages, and make xs_daemon_rootdir() an alias of xs_daemon_rundir().
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Roger Pau Monné [Wed, 2 Nov 2022 11:06:37 +0000 (12:06 +0100)]
hvm/msr: load VIRT_SPEC_CTRL
Add MSR_VIRT_SPEC_CTRL to the list of MSRs handled by
hvm_load_cpu_msrs(), or else it would be lost.
Fixes: 8ffd5496f4 ('amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL') 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>
tools/xenstore: harden transaction finalization against errors
When finalizing a transaction, any error occurring after checking for
conflicts will result in the transaction being performed only
partially today. Additionally accounting data will not be updated at
the end of the transaction, which might result in further problems
later.
Avoid those problems by multiple modifications:
- free any transaction specific nodes which don't need to be committed
as they haven't been written during the transaction as soon as their
generation count has been verified, this will reduce the risk of
out-of-memory situations
- store the transaction specific node name in struct accessed_node in
order to avoid the need to allocate additional memory for it when
finalizing the transaction
- don't stop the transaction finalization when hitting an error
condition, but try to continue to handle all modified nodes
- in case of a detected error do the accounting update as needed and
call the data base checking only after that
- if writing a node in a transaction is failing (e.g. due to a failed
quota check), fail the transaction, as prior changes to struct
accessed_node can't easily be undone in that case
In case a node has been created in a transaction and it is later
deleted in the same transaction, the transaction will be terminated
with an error.
As this error is encountered only when handling the deleted node at
transaction finalization, the transaction will have been performed
partially and without updating the accounting information. This will
enable a malicious guest to create arbitrary number of nodes.
Edwin Török [Wed, 12 Oct 2022 18:13:05 +0000 (19:13 +0100)]
tools/ocaml: Ensure packet size is never negative
Integers in Ocaml have 63 or 31 bits of signed precision.
On 64-bit builds of Ocaml, this is fine because a C uint32_t always fits
within a 63-bit signed integer.
In 32-bit builds of Ocaml, this goes wrong. The C uint32_t is truncated
first (loses the top bit), then has a unsigned/signed mismatch.
A "negative" value (i.e. a packet on the ring of between 1G and 2G in size)
will trigger an exception later in Bytes.make in xb.ml, and because the packet
is not removed from the ring, the exception re-triggers on every subsequent
query, creating a livelock.
Fix both the source of the exception in Xb, and as defence in depth, mark the
domain as bad for any Invalid_argument exceptions to avoid the risk of
livelock.
This is XSA-420 / CVE-2022-42324.
Signed-off-by: Edwin Török <edvin.torok@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
Edwin Török [Wed, 12 Oct 2022 18:13:06 +0000 (19:13 +0100)]
tools/ocaml/xenstored: Fix quota bypass on domain shutdown
XSA-322 fixed a domid reuse vulnerability by assigning Dom0 as the owner of
any nodes left after a domain is shutdown (e.g. outside its /local/domain/N
tree).
However Dom0 has no quota on purpose, so this opened up another potential
attack vector. Avoid it by deleting these nodes instead of assigning them to
Dom0.
This is part of XSA-419 / CVE-2022-42323.
Fixes: c46eff921209 ("tools/ocaml/xenstored: clean up permissions for dead domains") Signed-off-by: Edwin Török <edvin.torok@citrix.com> Acked-by: Christian Lindig <christian.lindig@citrix.com>
tools/xenstore: make the internal memory data base the default
Having a file backed data base has the only advantage of being capable
to dump the contents of it while Xenstore is running, and potentially
using less swap space in case the data base can't be kept in memory.
It has the major disadvantage of a huge performance overhead: switching
to keep the data base in memory only speeds up live update of xenstored
with 120000 nodes from 20 minutes to 11 seconds. A complete tree walk
of this configuration will be reduced from 7 seconds to 280 msecs
(measured by "xenstore-control check").
So make the internal memory data base the default and enhance the
"--internal-db" command line parameter to take an optional parameter
allowing to switch the internal data base back to the file based one.
tools/xenstore: remove nodes owned by destroyed domain
In case a domain is removed from Xenstore, remove all nodes owned by
it per default.
This tackles the problem that nodes might be created by a domain
outside its home path in Xenstore, leading to Xenstore hogging more
and more memory. Domain quota don't work in this case if the guest is
rebooting in between.
Since XSA-322 ownership of such stale nodes is transferred to dom0,
which is helping against unintended access, but not against OOM of
Xenstore.
As a fallback for weird cases add a Xenstore start parameter for
keeping today's way to handle stale nodes, adding the risk of Xenstore
hitting an OOM situation.
This is part of XSA-419 / CVE-2022-42322.
Fixes: 496306324d8d ("tools/xenstore: revoke access rights for removed domains") Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Julien Grall <jgrall@amazon.com>
check_store() is using a hash table for storing all node names it has
found via walking the tree. Additionally it using another hash table
for all children of a node to detect duplicate child names.
Simplify that by dropping the second hash table as the first one is
already holding all the needed information.
Add a generic function to walk the complete node tree. It will start
at "/" and descend recursively into each child, calling a function
specified by the caller. Depending on the return value of the user
specified function the walk will be aborted, continued, or the current
child will be skipped by not descending into its children.
tools/xenstore: don't let remove_child_entry() call corrupt()
In case of write_node() returning an error, remove_child_entry() will
call corrupt() today. This could result in an endless recursion, as
remove_child_entry() is called by corrupt(), too: