For multi-byte IO port accesses, the APM says that SVM should intercept
if any of the corresponding permission bits are set. However, the code
has this backwards and only intercepts if all the permission bits are
set. Fix this and at the same time, make things safer by handling
mapping failures as intercepted. Also rename the 'enabled' variable to
make it clearer what it does.
This affects Hyper-V which for the root partition intercepts 0xcf8 and
0xcfc-0xcff. L2 issues 4 byte CFG index writes to 0xcf8 which are incorrectly
permitted and update the L1 CFG index. The subsequent L2 CFG data read at 0xcfc
is intercepted by L1, then resubmitted as a CFG index write followed by CFG
data read to L0. Since L1 doesn't see the CFG index write by L2, it uses the
wrong index and gets garbage back, ultimately leading to a failure to boot
since it cannot access its NVMe disk.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The name X86_ET_EXT_INTR came from the VT-x code originally, but it's not
really correct.
The SDM doesn't give a concrete name, and only describes the fields as
"external interrupts". The APM does give a concrete name of INTR, and both
Intel and AMD use the name INTR for related purposes elsewhere; the name
derives originally from #INTR which was a real pin on early processors.
Very importantly, this is distinct from ExtINT which is a type of maskable
interrupt which can be sent in an x86 system.
Therefore rename all {svm,vmx}_*_extint() to {svm,vmx}_*_intr(), as they
pertain to all maskable interrupts, not only to ExtINT interrupts.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
TRACE_TIME() uses an initializer list and hence is covered by Misra C:2012
rule 13.1 ("Initializer lists shall not contain persistent side effects").
Use intermediate variables to address this.
In vcpu_yield() take the opportunity to rename the existing variable which
better would have been used already before.
In do_sched_op() extend the scope of and rename an existing variable. Also
drop a pointless (and slightly malformed, style-wise) cast there, and
adjust to other one to by style conformant.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
The 16-bit range of "port" is fully handled by the switch(). Therefore the
default: case is reported unreachable by Eclair. Insert BUILD_ERROR() to
annotate this for Eclair.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
In hvm_set_cr3() the "bad_cr3" label is reachable only with
SHADOW_PAGING=y; the code being there is therefore a Misra rule 2.1
(unreachable code) violation when SHADOW_PAGING=n.
Similarly code past the initial switch() in hvm_debug_op() is reachable
only when CONFIG_INTEL_VMX=y.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
The L2H, L3, and L4 forms only have GUEST_PAGING_LEVELS=4 call sites, i.e.
their 2- and 3-level forms are unreachable, violating Misra rule 2.1. The
L2H form additionally is unused (call site DCE-d) with PV32=n.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
The referenced commit didn't go far enough, leaving a Misra rule 2.1
(unreachable code) violation: The function lacks "noreturn" in this
configuration. Since with SHADOW_PAGING=n paging_mode_shadow() is compile-
time-constant false, the compiler can DCE the call site. Hence we can
avoid building the function itself altogether.
No functional change.
Fixes: 2fb2dee1ac ("x86/mm: pagetable_dying() is HVM-only")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
The current logic on x86 will mark all domain owned pages as needing a TLB
flush before being re-used. However such TLB flushing is only strictly
needed when the pages might have been mapped by a PV domain, as those can
keep a reference to the page in the TLB after it has been freed.
Limit the flushing to builds with PV domain support, as tracking whether a
page might have been mapped by a PV domain is not trivial (and possibly not
worth the extra logic).
Signed-off-by: Roger Pau Monné <roger@xenproject.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Adjust some types: int -> bool, int -> unsigned int. Also fix a couple of
typos plus trailing white space.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger@xenproject.org>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Until now every architecture carried its own notion of max_init_domid:
Arm defined a real variable (declared in asm/setup.h, defined in
setup.c), while ppc, riscv and x86 each provided a "#define
max_init_domid (0)" stub in their asm/setup.h. This duplicated the same
declaration across all arches and placed a purely dom0less concept in
arch setup headers.
Now that the dom0less build code lives in common (xen/common/
device-tree/dom0less-build.c sets max_init_domid, and the console
serial-input switcher reads it), there is no reason for the symbol to be
per-arch. Provide a single declaration in <xen/dom0less-build.h>, with
the !CONFIG_DOM0LESS_BOOT stub kept there as well, so there is one source
of truth and the arch headers no longer need to mention it. Update
console.c to include <xen/dom0less-build.h> for the declaration instead
of relying on asm/setup.h, and drop the now unneeded <asm/setup.h>
include.
Place the definition in xen/common/domid.c rather than in dom0less-
build.c. The latter is built as dom0less-build.init.o, i.e. the whole
object is relocated into the .init.* sections and freed after boot,
whereas max_init_domid must outlive boot because it is read at runtime
by the console serial-input switcher. domid.c is always linked (obj-y)
and resides in regular (non-init) sections, so it is a correct home for
the variable. It is marked __ro_after_init since it is only updated
while creating boot-time domains and read-only afterwards, and guarded
by CONFIG_DOM0LESS_BOOT as domid.c itself is unconditional.
While at it, make <xen/dom0less-build.h> self-contained: it includes
<public/xen.h>, which uses the fixed-width types provided by
<xen/types.h>, so include the latter explicitly rather than relying on
the includer having pulled it in first. This becomes necessary as soon
as <xen/dom0less-build.h> comes first in an alphabetically sorted
include list, as it now does in domid.c.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
While casting to pointer types may be more natural there, the subtraction
then ends up violating "Subtraction between pointers shall only be applied
to pointers that address elements of the same array". Use unsigned long
arithmetic instead.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Doing so, besides (hopefully) adding clarity (not the least by way of
[re-]using pattern rules where possible), also avoids explicit recursive
$(MAKE) invocations.
By re-using the generic rules introduced when the respective x86 rule was
split,
- the .map file now isn't created after the final binary anymore,
- --strip-debug is passed to $(LD) during early linking passes (for
consistency the option is also explicitly added to the optional linking
pass rule),
- CONFIG_{SUPPRESS_DUPLICATE_SYMBOL_WARNINGS,ENFORCE_UNIQUE_SYMBOLS} are
now properly respected.
Orphan section checking, otoh, is getting suppressed for now, until the
about a dozen warnings which would result have been taken care of.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Doing so, besides (hopefully) adding clarity (not the least by way of
using pattern rules where possible), also avoids explicit recursive
$(MAKE) invocations. For xen-syms move re-usable helper rules to a new
scripts/Makefile.link.
While doing so, re-order .map file creation (which can in principle fail)
and check-endbr.sh invocation ahead of putting in place the final image
(which is now the result of a simple rename).
Also drop --source-name= from the tools/symbols invocation which has
--empty passed, for being meaningless there.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
All callers now have an sbdf. Pass it down directly, rather than splitting
into three parameters.
In turn this shows that bus and devfn could never be out of bounds, so drop
those checks too.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
This will be used to simplify some call chains. Reposition the new field to
avoid creating a interior hole.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Without passing opinion on the behaviour of this function, it is deceptive to
read (I've twice now mistaken it for resetting boot_cpu_data), and
inefficient.
Instead of having a 256 byte object on the stack and a double copy, copy
boot_cpu_data directly, then reset parts of cpu_data[cpu] in place. Leave
some comments behind explaining what's happening.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
... into an available 8-bit slot, rather than having it followed by 7
bytes of padding.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Recent Clang objects to types differing between alias and aliasee. Accept
the unnecessary overhead and make the two compat stubs real functions.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
oxenstored maintains two datastructures about watches; one global trie, and
one hashtable tracked per domain. Both need keeping in sync, and right now
the global trie is not emptied when a xenbus reconnect is requested.
This is basically the same bug as XSA-330, commit 491a077ed4
("tools/ocaml/xenstored: delete watch from trie too when resetting watches"),
just tickled via another path.
Arrange for both Process.do_reset_watches() and Process.do_reconnect() to
share a common codepath for the resetting of watches and transactions.
Notably, this means that the latter now calls Connections.del_watches() which
clears the global trie too.
Connections.del_watches() already calls Connection.del_watches() so remove the
re-clearing of the state from Connection.do_reconnect().
Move History.trim() into reset_watches_and_transactions() so it's on the
common path, and place it after removing the transactions rather than before.
This is part of XSA-512 / CVE-2026-79604.
Reported-by: David Korczynski <David@Adalogics.com>
Fixes: 674ad2be40 ("xenstore: extend the xenstore ring with a 'closing' signal")
Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Andrii Sultanov <andriy.sultanov@vates.tech>
The logic flow here is complicated. In preparation to fix a bug, factor out
reconnecting a xenbus connection, and fold History.reconnect into it's single
caller.
No functional change.
This is part of XSA-512 / CVE-2026-79604.
Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Andrii Sultanov <andriy.sultanov@vates.tech>
The current way in which idle TLB flush and TLB flushing when allocating a
page are done allows for the scrubbing to be done ahead of the TLB flush.
A PV domain can still have a TLB entry for the page after scrubbing, and
hence it may be able to modify it. Such unintended page accessing allows
domains to possibly exchange information even when `xsm=silo scrub-domheap`
are in effect.
Remove the MEMF_no_tlbflush memory allocation flag, and reorder the
flushing so it's always done ahead of the scrubbing in
alloc_{,color_}heap_pages(). The sole user of MEMF_no_tlbflush is
populate_physmap(), and given the constrains above it's no longer safe
to defer the flush, hence the flag removal and the folding of the flush in
the allocator function itself.
This is XSA-511 / CVE-2026-79603.
Fixes: 24f1a58d19 ("mm: option to _always_ scrub freed domheap pages")
Signed-off-by: Roger Pau Monné <roger@xenproject.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
hvm_io_intercept() can return X86EMUL_RETRY, and as such it needs to be
handled in the switch in hvmemul_do_io() to avoid triggering the BUG() from
the default case.
Reset the vCPU state to no in-flight IOREQ and return X86EMUL_RETRY so that
the access is retried.
This is XSA-510 / CVE-2026-79602.
Reported-by: Jiqian Chen <Jiqian.Chen@amd.com>
Signed-off-by: Roger Pau Monné <roger@xenproject.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
DMs may invoke XEN_DOMCTL_bind_pt_irq for domains already under
destruction. When XEN_DOMCTL_bind_pt_irq is invoked after
pci_release_devices() (invoked from underneath domain_kill()) had already
completed, it would allocate hvm_domain_irq(d)->dpci anew, without that
ever being freed during subsequent domain cleanup.
Leverage evtchn_destroy()'s kind-of-spin-barrier, allowing to simply check
->is_dying with the domain's event lock held.
This is XSA-509 / CVE-2026-62437.
Fixes: 7a26b541a2 ("vtd: Dynamically allocate IRQ-tracking structures, only for those")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
.data.rel.ro really is merely a special case of .rodata: r/o data, but
with runtime relocations (not relevant to Xen), i.e. r/o only after
relocations were performed (by the dynamic loader).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Recent Clang complains about the "debug" static variable only ever being
written to. Drop it and the command line option controlling it.
Fixes: 19ab8356ab ("tools: remove support for running a guest with qemu-traditional")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
rtc_ioport_write() has two writes of the new value, yet only one was made
aware of the century going outside of the array. Fold both writes by
changing the RTC_SET short-circuiting.
Fixes: f2ff80877f ("x86/vRTC: support century field")
Coverity ID: 1700943
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Use statement expressions to return struct arm_smccc_res which makes the code
read a lot more normally, and avoids needing to pass in NULL in order to skip
return information.
More importantly, it removes the local implementation of __count_args() which
is off by two and deeply confusing to try and follow.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
SMCCC v1.0 says that x4 through x17 may be clobbered. SMCCC v1.1 says they
are strictly preserved, and SMCCC v1.2 permits them to contain extra return
values.
Xen deals with this by having __arm_smccc_1_0_smc() as an out-of-line
function, but this causes awful code generation in arm_smccc_smc().
cpus_have_const_cap() is opaque to the optimiser, so we end up with one basic
block doing the reasonably-ok arm_smccc_1_1_smc() code generation and a second
basic block setting up all 8 input registers even when they're not needed,
spilling or discarding x8 through x17, and calling an out-of-line function.
Remove __arm_smccc_1_0_smc() entirely, and rewrite arm_smccc_smc() to declare
x4 through x17 as clobbered.
This fully inlines the SMC, is a single basic block which instructs the
compiler to spill or discard the potentially clobbered registers, and only
sets up the necessary number of arguments for the call.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
... before making a related copy of it.
* Drop __constraints() so the output parameters are visible in the same block
as they're defined. Use PASTE() rather than opencoding it.
* Adjust the indentation of trailing \'s for consistency.
* Drop the newline at the end of the instruction.
* Indent the if condition correctly. ___res is always of type
arm_smccc_res (declared in __declare_arg_0()), so drop the typeof().
* Drop arm_smccc_1_0_smc() as it has no users.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Both {get,set}_user_reg() are out-of-line functions, leading to awful code
generation.
Introduce arm_smccc_guest_smc() to operate directly on guest registers.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Contrary to what was claimed in commit 67bcf5eae7 ("xen/arm: Simplify type
handling for SMCCC declarations"), there is an important reason to retain the
intermediate variable. It is unsafe to have any logic between the assignment
of the register variables and the asm() block they're used in.
This logically reverts commit 67bcf5eae7 ("xen/arm: Simplify type handling
for SMCCC declarations") while retaining the conversions from commit
7f15d5d132 ("xen/treewide: More typeof() -> auto conversions").
Adjust __declare_arg_0() to match. It happens to be safe because it's the
first register expression once all macros are expanded, but it really should
be consistent with the others.
Leave a comment explaining why they must be written like this.
Fixes: 67bcf5eae7 ("xen/arm: Simplify type handling for SMCCC declarations")
Reported-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
imsic_init() indexes msi[] by the Xen CPU id hartid_to_cpuid() returns,
but that array is allocated with one entry per parent IRQ of the IMSIC
node, and the only range check compares the index against
num_possible_cpus(). Neither matches the array, and the check comes
after the first access:
- hartid_to_cpuid() returns NR_CPUS when the hart isn't one Xen brought
up, and msi[NR_CPUS].base_addr is read before that is noticed;
- an IMSIC node listing fewer parents than Xen has CPUs makes every
index past nr_parent_irqs go past the end of the allocation, which the
num_possible_cpus() check lets through.
Size the array by nr_cpu_ids, which is what it is indexed by,
and move the range check ahead of the first msi[] access.
Fixes: c9bd8b322e ("xen/riscv: imsic_init() implementation")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With the split-off of decoding into a separate file, both files' top
comments should have been adjusted accordingly.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
ALT_CALL_PTR() really means to get rid of const (when necessary, i.e. when
used from apply_alt_calls()): We want to patch the original call site,
after all, and memory-wise that's unrelated to the struct alt_call * that
we start from.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
While the general C library strchr() has to do so, the early boot cmdline
parsing has no such requirement.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
On encountering an unexpected VMEXIT (e.g. because L1 has used an
intercept that L0 Xen doesn't know about), crash the domain rather than
the hypervisor.
Fixes: 9a779e4fc1 ("Implement SVM specific part for Nested Virtualization")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Forever since the introduction of the .priv member it has been wrong to
record the section as r/o.
.alt_call_sites, otoh, is legitimately r/o, hence
__alt_call_sites_{start,end}[] would better reflect that.
Fixes: 4008c71d7a ("x86/alt: Support for automatic padding calculations")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Doing so (at the return statement), besides being a bad idea anyway, is a
violation of Misra rule 11.8.
Make "xcap" have an initializer while adjusting the code anyway.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Doing so violates Misra rule 11.8, while being entirely unnecessary here:
All callers already store the return value in pointer-to-const variables.
No functional change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
We want to patch the original call site, which memory-wise is unrelated
to the struct alt_instr * that we start from.
No functional change intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
The requested PCI segment needs including in the call to
xsm_pci_config_permission(). Otherwise in a multi-segment system we can check
the perimssions on one device but operate on a different one.
This is only not a vulnerability because pci_conf_write_intercept() is only
reachable by the hardware domain.
Fixes: 300bb048ca ("x86/PCI: make all config space writes subject to XSM checking")
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Doing so, besides being a bad idea in general, violates Misra rule 11.8.
No functional change intended.
Fixes: f1879b2c25 ("xen: introduce generic non-atomic test_*bit()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Waiting loops like the one in flush_command_buffer() will degenerate to
infinite ones when used early enough for NOW() to still return constant
zero. Make sure the returned value at least monotonically increases. When
available, use nominal frequency values as initial approximation.
Do this only in get_s_time(), as producing a sane value in
get_s_time_fixed() for non-zero inputs won't be reasonably possible.
Put an assertion there.
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>
Ross Lagerwall