Addressing certain issues, in particular related to operations which may
take excessively long and therefore would need preemption, has turned out
overly costly. Since alternatives (HVM/PVH: HAP, PV: shim) are commonly
available, the decision was to deprecate the functionality, while still
retaining it for people to use at their own (security) risk. Memory-wise
small enough guests may still be okay to run.
Some CI testing depends on SHADOW_PAGING. Explicitly enable it when needed.
This is CVE-2026-42493 / XSA-495.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
With CPU policy dependency logic now properly in place and working, and
with us never enabling ISA bits in guest policies which the underlying
hardware doesn't support, the dual checks aren't needed anymore.
In turn, a number of cpu_has_* can then also go away, when their only user
was the emulator code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
As we ignore cachability aspects of insns, they're treated like simple
MOVs.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When adding ranges, only alter existing ones when there is an exact match.
Don't accept ranges overlapping existing ones.
When removing ranges, only remove a range if there's an exact match.
Return an error when the range isn't found.
Fixes: 192c4dabc3 ("domctl and p2m changes for PCI passthru")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Rather than granting permissions when mapping (an operation that DM-s are
allowed to carry out, while they can't invoke ioport-permission), check
whether permissions actually were granted when adding a mapping. This then
also allows relaxing the necessary locking.
While no longer granting permissions upon mapping is "only" at risk of
breaking guests, no longer revoking permissions upon unmapping strictly
requires callers to additionally invoke XEN_DOMCTL_ioport_permission. Or
else a security issue would arise. In-tree code already does so.
While there switch to using %pd in the two log messages.
Fixes: 192c4dabc3 ("domctl and p2m changes for PCI passthru")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
DstMem processing early in x86_emulate() raises #UD when EVEX.Z is set
with a memory destination. Open-coded checks like this one are therefore
dead / unreachable code, violating Misra rules.
Fixes: 4037a62ef9 ("x86emul: support AVX512F floating-point conversion insns")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
We know that opcode_table[0x0f] is 0, there's no need to check that. Up
until ab082ffa87 ("x86emul: generate and make use of a canonical opcode
representation") or maybe already c4e7a67e3a ("x86emul: drop
SrcInvalid") this may have been justified, but it no longer is.
This eliminates a Misra C:2012 rule 2.2 ("There shall be no dead code")
violation.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
AFL has pointed out that the op_bytes-is-not-0 assertion in common SIMD
handling can trigger for these insns. Indeed when the (relevant part of)
the controlling mask register is 0, no memory is accessed at all. Leave
op_bytes unaltered in this case, to engage the short-circuiting in common
SIMD handling when fault_suppression is true and op_bytes is 0.
While there also correct a related typo in the test harness.
Fixes: 65f82d4ce1 ("x86emul: support AVX512{F,_VBMI2} compress/expand insns")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Move the little bit that's still left in x86/x86_emulate.c into a __XEN__
conditional in x86/x86_emulate/x86_emulate.c. Move what are roughly the
test/fuzzing harness counterparts from there into the corresponding #else.
Interestingly more recent gcc takes issue with three static const arrays
which are unused when HVM=n (noticeable in particular in the shim build).
Apparently such are diagnosed only when present in the top-level file, but
not when living in an #include-d one.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
It can be used as-is for VMX. For SVM the special treatment of %rax needs
accounting for.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
... rather than papering over guest flaws: Strings passed ought to be nul-
terminated (yet sadly libxc hasn't been doing so thus far). This way we
also avoid order-1 allocations, seeing that all present callers pass
PAGE_SIZE for max_size.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> # Changelog
Nul-terminated strings are passed in all cases, so the strlen() can very
well be invoked by the function itself. In preparation for a hypervisor
change also include the nul terminator in the size calculation.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
%4.4x and alike format specifiers can be expressed shorter as %04x or, as
e.g. dump_ioapic() has it, %.4x.
In dump_fpu()'s XMM register dumping, also move away from showing bogus
xmm03 and alike. The proper register name is xmm3 for that particular
example.
Also strip trailing whitespace from lines touched.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
In a !PV configuration the functions aren't built. Hence we're better off
also not declaring them.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
From its introduction it has been used solely to add and remove pools.
No list traversal or alike did ever occur. Drop all of this as being dead
code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
For a long time (if not forever) this lock has been referenced only from a
single CU. Misra C:2012 rule 8.7 (which we didn't accept yet) wants us to
have such identifiers non-external.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
In preparation for a hypervisor change also include the nul terminator in
the size calculations. (Note that xc_flask_getbool_byid() doesn't support
FLASK_GETBOOL's "ID being -1" variant of operation, and hence doesn't need
fiddling with.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
It is mostly sorted, but there are anomalies. Eliminating them gives
people fewer excuses to (blindly) add to the end of the list.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
IRQ_PER_CPU and IRQ_GUEST_EOI_PENDING are entirely unused. x86 uses
IRQ_REPLAY, but only ever to clear the bit. Drop all three constants.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's no need to open-code these. No functional change intended, even if
the | changes to + in to_bcd().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
... outside of __get_cmos_time()'s locked region. There's no need to hold
the lock for these computations.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Setting ->orig_video_isVGA to the specific marker should be done only when
the VBE tag is present and the FRAMEBUFFER is either absent or indicates
RGB type. Since the "video" variable now starts out non-NULL, this
property was broken when in particular neither of the tags are present. To
move back to at least close to original behavior, add a 2nd check to said
conditional.
Fixes: d5a73cdc6b ("x86/boot: Use boot_vid_info variable directly from C code")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Loaders respecting IMAGE_SCN_MEM_DISCARDABLE would not load such sections,
yet we need to access it ourselves when switching out of "physical mode".
Leverage behavior new to GNU ld 2.46: Any contribution to .reloc which
doesn't have the discardable flag set (which cannot even be expressed in
ELF) will yield the output section also non-discardable.
Since for intermediate binaries we don't care about section attributes,
link in the new object only on the final linking pass.
Reported-by: Yann Sionneau <yann.sionneau@vates.tech>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Leaving the hook pointers in struct xsm_ops when !SYSCTL would lead to
the BUG_ON() in xsm_fixup_ops() triggering for respectively configured
hypervisors.
While moving the #ifdef for the corresponding xsm_*() wrappers, also move
those for xsm_page_offline() (where the hook pointer field already is
suitably guarded).
Fixes: c9eabaa03a ("xen/xsm: wrap around xsm_sysctl with CONFIG_SYSCTL")
Fixes: bddd9af604 ("xen/sysctl: wrap around XEN_SYSCTL_readconsole")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Release-Acked-by: Oleskii Kurochko <oleskii.kurochko@gmail.com>
cpumask_to_xenctl_bitmap() may return errors. Clearing the error indicator
of an earlier such call by a (successful) later call is misleading the
caller. For "set", keep setting soft affinity if the hard affinity copy-
back fails; only accumulate respective errors.
While fiddling with return values, also drop a redundant clearing of
"ret". This eliminates a Misra C:2012 rule 2.2 ("There shall be no dead
code") violation.
Fixes: 6e4ecc6d58 ("sched: DOMCTL_*vcpuaffinity works with hard and soft affinity")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Be less strict with permissions checks when removing a mapping and
only request the caller domain to have access to the region. Keep the
same permission checks for addition operations.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
This wants xsm_domctl() invoked, but the domctl lock not taken. Move the
handling to the respective switch(), thus eliminating the need for a
separate xsm_domctl() invocation.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Like for XEN_DOMCTL_getdomaininfo there's no need to hold the domctl
lock for XEN_DOMCTL_getvcpuinfo. While moving the code also switch to
using domain_vcpu().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tentatively-acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
About half the callers of vcpu_runstate_get() are solely after the
"running" time of a vCPU. Introduce a specialization with a smaller
read critical section and thus reduced risk of a need for retries.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
As Michal had indicated in review, status is now a variable that is set
but never read. I made the resulting change locally, but then committed a
stale version of the patch (also omitting Michal's R-b).
Amends: eff88c4d35 ("gnttab: simplify (really: drop) gnttab_set_frame_gfn()")
Reported-by: Michal Orzel <michal.orzel@amd.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Unprivileged callers better wouldn't be in the position of figuring out
domain existence from error codes. Adjust the respective path sitting
ahead of XSM checks to produce -EPERM in such cases, just like the
subsequent XSM check would yield.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
HAS_PIRQ is selected by x86 only, and that's expected to remain that way.
Avoid the #ifdef needed by moving the logic to arch_do_domctl(). Leverage
"currd" being available as a local variable there while doing so.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
The only locking required here is that between checking d->target and
setting it. To avoid the need for an explicit lock, use cmpxchgptr() to
update d->target.
Move the handling not only ahead of acquiring the lock, but also ahead
of the XSM check, leveraging that the sub-op has its own hook.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Integrate the checking with xsm_domctl(). As a positive side effect,
permissions are then checked at the same early point with and without
Flask. As the DT device path needs fetching earlier (but must not be
double fetched), cache it in a private field of the public interface
struct.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
iommu_get_device_group() uses its own locking. Thus, with caller side
locking irrelevant, it can as well be called with the domctl lock not
held.
Move the handling not only ahead of acquiring the lock, but also ahead
of the XSM check, leveraging that the sub-op has its own hook.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Integrate the checking with xsm_domctl(), now that it has the full op
struct passed. As a positive side effect, permissions are then checked at
the same early point with and without Flask.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Integrate the checking with xsm_domctl(), now that it has the full op
struct passed. As a positive side effect, permissions are then checked at
the same early point with and without Flask.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Subsequently some sub-ops will want to inspect their sub-sub-ops. Plus
this way we don't need to pass SSIDref separately anymore for
domain_create.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Integrate the checking with xsm_domctl(). Care needs to be taken with the
GET_VERSION sub-op, which may be invoked with DOMID_INVALID, and which has
been (and continues to be) bypassing XSM checking.
Since the latter two parameters were unused, monitor_domctl() invoking the
hook was actually redundant with the earlier xsm_domctl() (as can be seen
nicely from the hunks changing xsm/flask/hooks.c).
As a positive side effect, permissions are then checked at the same early
point with and without Flask.
While folding XEN_DOMCTL_monitor_op and XEN_DOMCTL_vm_event_op in
flask_domctl(), also fold in XEN_DOMCTL_set_access_required.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
With dedicated locking added, the domctl lock isn't required here anymore.
As the GSI handling is in arch-specific code (x86 only), no code is being
moved there; the 2nd invocation of arch_do_domctl() is re-used. Move the
re-purposed (XSM_HOOK -> XSM_PRIV, as xsm_domctl() is now bypassed)
dedicated XSM checks as early as possible.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
With dedicated locking added, the domctl lock isn't required here anymore.
As the I/O port handling is in arch-specific code (x86 only), no code is
being moved, but the 2nd invocation of arch_do_domctl() is re-used. Move
the re-purposed dedicated XSM checks as early as possible.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
With dedicated locking added, the domctl lock isn't required here anymore.
(It also already isn't used when pt_irq_{create,destroy}_bind() are
invoked for PVH Dom0.) As the handling is in arch-specific code, no code
is being moved, but the 2nd (extensible to other sub-ops like the ones
here) invocation of arch_do_domctl() is being re-used.
This is part of XSA-492.
Fixes: fda49f9b3f ("Add build option to allow more hypercalls from stubdoms")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Acked-by: Julien Grall <julien@xen.org>
With dedicated locking added, the domctl lock isn't required here anymore.
As the handling is in arch-specific code (x86 only), almost no code is
being moved, but a 2nd (extensible to other sub-ops) invocation of
arch_do_domctl() is being added. Move just the re-purposed dedicated XSM
check as early as possible.
In flask_domctl() don't put #ifdef around the moved case label.
This is part of XSA-492.
Fixes: fda49f9b3f ("Add build option to allow more hypercalls from stubdoms")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
With dedicated locking added, the domctl lock isn't required here anymore.
Move the re-purposed dedicated XSM check as early as possible.
Minimal "modernization": Switch "add" to bool and use %pd in log messages.
This is part of XSA-492.
Fixes: fda49f9b3f ("Add build option to allow more hypercalls from stubdoms")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
It's used twice in entirely different situations. The use in do_domctl()
wants to become an ordinary XSM_DM_PRIV invocation, while the one in vPCI
code need to remain XSM_HOOK (it may plausibly become XSM_TARGET). For
Flask, the same backing function will continue to be used for the time
being.
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
In order to be able to pull at least the XEN_DOMCTL_{,un}bind_pt_irq
handling out of the domctl-locked region, a separate (per-domain) lock is
needed to synchronize in particular with XEN_DOMCTL_{irq,gsi}_permission.
Locking is added only as far as domctl-s are affected. Uses presently
outside of the domctl lock may want dealing with subsequently (perhaps
limited to non-__init code).
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Julien Grall <julien@xen.org>
In order to be able to pull at least the XEN_DOMCTL_ioport_mapping
handling out of the domctl-locked region, the new separate (per-domain)
lock is used to synchronize in particular with
XEN_DOMCTL_ioport_permission.
Locking is added only as far as domctl-s are affected. Uses presently
outside of the domctl lock may want dealing with subsequently (perhaps
limited to non-__init code).
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
In order to be able to pull at least the XEN_DOMCTL_iomem_mapping handling
out of the domctl-locked region, a separate (per-domain) lock is needed to
synchronize in particular with XEN_DOMCTL_iomem_permission.
Locking is added only as far as domctl-s are affected. Uses presently
outside of the domctl lock may want dealing with subsequently (perhaps
limited to non-__init code).
This is part of XSA-492.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
get_domain_state() uses its own locking. Thus, with caller side locking
irrelevant, it can as well be called with the domctl lock not held.
Move the handling not only ahead of acquiring the lock, but also ahead
of the XSM check, leveraging that the sub-op has its own hook.
This is part of XSA-492.
Fixes: 3ad3df1bd0 ("xen: add new domctl get_domain_state")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
getdomaininfo() is not called under consistently the same lock. Thus,
with caller side locking irrelevant, it can as well be called with the
domctl lock not held. (Callers not pausing the domain they want to
retrieve information for already need to be aware that not all of the
data returned can be relied on as being consistent; most data will also
be stale by the time the caller gets to look at it.)
Move the handling not only ahead of acquiring the lock, but also ahead
of the XSM check, leveraging that the sub-op has its own hook.
While moving, convert an assignment to an assertion: The domain in
question was determined from the field which previously was "updated".
This is part of XSA-492.
Fixes: 5513bd0b46 ("add xenstore domain flag to hypervisor")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Subsequently XEN_DOMCTL_getdomaininfo will want to invoke the function
without holding a lock, thus allowing parallel execution of potentially
many instances. As was learned from 228ab9992f ("domctl: improve
locking during domain destruction"), reverted by d0887cc6b1, such
parallelism can result in severe lock contention on any (previously)
inner lock. To avoid taking that risk replace the use of the scheduler
lock in vcpu_runstate_get() by a newly introduced sequence counter.
Convert the "no lock if current" property to "use a local counter
instance", thus guaranteeing the loop to exit after the first iteration.
Skeleton and commentary of the seqcount implementation based on /
derived from Linux 6.11-rc.
To have runstate_seq placed next to runstate in struct vcpu, without
introducing a new obvious padding hole, yet while keeping the latter
adjacent to runstate_guest{,_area} as well, move runstate down a little.
This is part of XSA-492.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
XEN_DOMCTL_ioport_mapping is usable by DM stubdoms, and hence we can't
assume the list to be left unaltered while the guest (really: the
hypervisor on behalf of the guest) is accessing it.
This is XSA-491 / CVE-2026-42487.
Fixes: 192c4dabc3 ("domctl and p2m changes for PCI passthru")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
The inner "rc" shadows the function scope one, thus violating Misra C:2012
rule 5.3 ("An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope"). Drop the inner variable, as
there's no other (later) use of the value it holds.
No difference in generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
... ("The function argument corresponding to a parameter declared to have
an array type shall have an appropriate number of elements"). Instead of
casts, (ab)use unions.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
... ("A switch label shall only be used when the most closely-enclosing
compound statement is the body of a `switch' statement"). While I don't
really like doing so, use a few "goto" instead. No change in generated
code (somewhat to my surprise).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
... ("A switch label shall only be used when the most closely-enclosing
compound statement is the body of a `switch' statement"). Use a form of
fall-through instead. No difference in generated code, except for some
line number changes.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
... shadowing a file scope one, thus violating Misra C:2012 rule 5.3
("An identifier declared in an inner scope shall not hide an identifier
declared in an outer scope").
While there,
- replace u32 by uint32_t,
- reduce the number of cf_check by aliasing shadow_l<N>_index() to
shadow_l1_index() for N > 1 and GUEST_PAGING_LEVELS > 2.
No difference in generated code, except of course the removal of the
duplicate function instances.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
... shadowing a function scope one in one case and the global _end[] in
another, thus violating Misra C:2012 rule 5.3 ("An identifier declared in
an inner scope shall not hide an identifier declared in an outer scope").
No difference in generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Before we can enable scanning of time.c, it needs to be clean wrt blocking
rules. Bring boot_tsc_stamp's definition in line with its declaration. For
get_s_time_fixed() go a little farther and move it to the x86 header (the
function only exists in x86) and do, along with correcting the parameter
name, the u64 -> uint64_t a little more widely than strictly necessary.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
It's not really doing anything for valid GFNs, which renders its one use
site pretty pointless. The other isn't so much about setting anything, but
rather about clearing.
The main point here, however, is about Rafal spotting the double
fetching of the GFN (first in gnttab_unpopulate_status_frames(), then
again in gnttab_set_frame_gfn()). Re-purpose the macro parameter to pass
in the already fetched GFN, while dropping the no longer used parameters.
As the result is a mere wrapper around guest_physmap_remove_page(), drop
the hook altogether.
Suggested-by: Rafal Wojtczuk <rafal.wojtczuk@7bulls.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Misra C:2012 rule 13.6 takes issue with the gdt_ldt_desc_ptr() call
appearing (deep) inside sizeof(), as used by get_unsafe().
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Misra C:2012 rule 13.6 takes issue with pfn_to_pdx() and mfn_x() uses
(deep) inside sizeof(), as used by read_atomic(). Break out the MFN ->
struct page_info * translation.
No difference in generated code, except for a line number change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Misra C:2012 rule 13.6 takes issue with RELOC_HIDE() uses inside sizeof(),
as used by this_cpu(). Latch the address of the per-CPU variable into a
local variable, and use that in the two sizeof().
No difference in generated code for 2- and 4-level code. There's a small
improvement for the 3-level case.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Misra C:2012 rule 13.6 takes issue with is_hvm_vcpu() uses (deep) inside
sizeof(), as used by get_unsafe(). While sh_linear_l1_table() doesn't
involve is_hvm_vcpu(), the rule there kicks in for "call to function
`__builtin_expect(long, long)' (unevaluated `nothrow' call effect)".
Break out the address calculations.
This, overall, turns out to also improve generated code (with gcc15 at
least). No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Build issues may also surface for analysis jobs. Collect intermediate
files (if any) there as well. Include xen/.config too, since it's not
otherwise collected.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The .rodata growth between 1st and 2nd linking passes may affect the
number of stubs the linker wants to insert, which in turn affects the
number of symbols. If symbol table sizes change after the 2nd linking
pass, insert another one before the final one. (As a comment in Linux puts
it, "In theory it's possible this results in even more stubs, but
unlikely.")
To use the $(compare-symbol-tables) macro in a shell "if", it needs
slightly adjusting (and then wrapping to be run in a sub-shell).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Bertrand Marquis <bertrand.marquis@arm.com>
Linking debug info takes time, especially for xen.efi. At the same time
symbol table contents aren't affected by presence / absence of debug info
in the linked binaries (the ELF and COFF symbol tables would have extra
section symbols, but tools/symbols omits those anyway).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
It's set only in the main "else", so the declaration as well as the sole
consumer can also move into that more narrow scope. This may in particular
help with possible future locking changes.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
->last_dirty holding a valid value (one other than -1) is solely an
indication of the bitmap being entirely clean. (The opposite isn't true,
because of _sh_propagate() setting the field to a valid value without
setting a bit in the bitmap.) As a consequence
- setting the field to a valid value right after having allocated zero-
filled space is pointless,
- copying the all empty bitmap to the output array is pointless; with the
output array also having been allocated zero-filled, not even a memset()
is needed there,
- after restoring bitmap contents when dealing with copy_to_guest() having
failed, the field needs setting to a valid value again.
Furthermore invoking NOW() in perhaps many loop iterations of the main
loop is wasteful, too. Record whether any bit was set, and record a new
->last_dirty only once, after the loop. Then use the same NOW() value also
for the subsequent check.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
... ("A macro parameter used as an operand to the `#' or `##' operators,
which is itself subject to further macro replacement, shall only be used
as an operand to these operators"). Leverage the SAF-6 annotation we have
available.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
... ("Conversions shall not be performed between a pointer to an
incomplete type and any other type"): Add an intermediate cast to void *.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
... ("A compatible declaration shall be visible when an object or function
with external linkage is defined"). Three variables lack asmlinkage
annotations.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
... ("A `typedef' name shall be a unique identifier"). Much like already
done for guest_intpte_t and guest_l[12]e_t, but limited to shadow/multi.c.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Such nesting causes the inner instance to shadow the outer instance's
macro-local variables, thus violating Misra C:2012 rule 5.3 ("An
identifier declared in an inner scope shall not hide an identifier
declared in an outer scope"). Use an intermediate variable for the
inner invocation. No difference in generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
... shadowing a file scope one, thuis violating Misra C:2012 rule 5.3
("An identifier declared in an inner scope shall not hide an identifier
declared in an outer scope"). No difference in generated code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Very old glibc (2.4 in the observed case) declares strndup() only when
_GNU_SOURCE is defined.
Fixes: ad860c6454 ("tools/xl: add xl commands for xenstore quota operations")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
A subsequent change will want to be able to alter them based on a new
command line option. (Note that some were __ro_after_init already.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Now that they're not referenced anymore post-init, they can themselves
move into .init.rodata. (idle_cpu_adl{,_l} can also become const in the
first place.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Rather than storing a pointer (and needing to keep all struct instances in
memory post-init), and rather than (like the Linux counterpart has it)
keeping individual variables, simply copy the respective structure
instance. By implication, subsequent updates now need doing to the copy.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Bay Trail (BYT) and Cherry Trail (CHT) platforms have a very specific way
of disabling auto-demotion via specific MSR bits. Clean up the code so that
BYT/CHT-specifics do not show up in the common 'struct idle_cpu' data
structure.
Remove the 'byt_auto_demotion_disable_flag' flag from 'struct idle_cpu',
because a better coding pattern is to avoid very case-specific fields like
'bool byt_auto_demotion_disable_flag' in a common data structure, which is
used for all platforms, not only BYT/CHT. The code is just more readable
when common data structures contain only commonly used fields.
Instead, match BYT/CHT in the 'intel_idle_init_cstates_icpu()' function,
and introduce a small helper to take care of BYT/CHT auto-demotion. This
is consistent with how platform-specific things are done for other
platforms.
No intended functional changes.
Inspired by (and description largely taken from) Linux'es c93d13b661a6
("intel_idle: clean up BYT/CHT auto demotion disable").
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
mwait_idle_cpu_init() is only called for APs, yet MSR writes will
typically need re-doing post-S3 even for the BSP. When multiple cores /
threads are present (and to come back online) in a package, for package
scope MSRs this may be covered by APs doing the writes, but we can't rely
on that.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
... ("All declarations of an object or function shall use the same names
and type qualifiers"). Bring declarations in line with the corresponding
definitions, accepting the use of the being-phased-out u<N> types. While
we don't define NEED_RIJNDAEL_DECRYPT, cover rijndaelKeySetupDec() as
well for completeness, including fixing a typo in a parameter name.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
... ("All declarations of an object or function shall use the same names
and type qualifiers"). The declaration doesn't use the typedef, so the
definition shouldn't either.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
If there are any local symbols in an object file, GNU ld will create an
STT_FILE symbol derived from the object file name if there is none in the
incoming symbol table. The object file name, however, varies between
linking passes. As a result, symbol name compression can yield different
results if any of those local symbols need retaining (Arm [and RISC-V]
mapping symbols are omitted, for example). If that difference in
compression would yield a difference in the sizes of symbol_names[] or
symbols_token_table[], the compare-symbol-tables sanity check will fail.
Fixes: d37d63d4b5 ("symbols: prefix static symbols with their source file names")
Reported-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Overlapping requests may need processing backwards, or else the intended
effect wouldn't be achieved (and instead some pages would be moved more
than once).
With some adjustment there this also covers XEN_DMOP_relocate_memory,
where the potential issue was first noticed.
Fixes: a04811a315 ("mm: New XENMEM space, XENMAPSPACE_gmfn_range")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Provide support for this insn, which is a prereq to FRED. CPUID-wise,
while its and FRED's enumerators were already introduced, their dependency
still needs adding.
While adding a testcase, also add a SWAPGS one. In order to not affect
the behavior of pre-existing tests, install write_{segment,msr} hooks
only transiently.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The BSP resume path calls mcheck_init() with the address of
boot_cpu_data, thus rendering comparisons against that pointer
ineffective. The additional "bsp" boolean needs to be used (and
propagated as necessary) instead.
While touching intel_init_thermal(), constify its 1st parameter, which
in turn requires touching intel_thermal_supported() as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When one of the linking passes fails, additional intermediate files are
still in place. Having them available for analysis of the underlying
problem can be pretty helpful. Collect some into a new intermediates/
directory. (Nothing new will be collected if linking succeeds.)
As the script is run with -e passed to bash, defer exit if the main
"make" (or substituent script thereof) failed. This way more artifacts
are collected, potentially making it easier to analyze the failure.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
There's no need to do this separately from three places. Once moved to the
bottom of the script, the need for this to be a function also disappears.
While there also make sure xen-syms is collected. This is rather more
useful for analysis of possible problems than its stripped counterpart.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
"days", as calculated, can't be negative. Drop the respective loop and
change its type. Similarly "rem" can't be negative and doesn't require
more than 32 bits. Change its type, too.
As a consequence, the tm_wday calculation also can't yield negative
values, so the respective conditional can be dropped as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
As a comment next to one of the invocations states, get_wallclock_time()
can take over a second. The order of evaluation of function arguments is
in principle unspecified; in practice at least gcc looks to be evaluating
them from last to first. Hence with NOW() invoked first, the respective
value passed to do_settime() can be off by over a second (which is in
contrast to __get_cmos_time() attempting to get the time exactly after an
update, i.e. [pretty] precisely at a seconds boundary).
This also addresses a Misra C:2012 rule 13.2 ("The value of an expression
and its persistent side-effects shall be the same under all permitted
evaluation orders") violation each. The violations aren't flagged because
the entire file is (bogusly) excluded from scanning.
Fixes: f64134cdb8 ("x86: Fix time_resume() to notify all domains of wallclock change")
Fixes: 0bfcf984b7 ("x86: Reintroduce clocksource=tsc")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich