Its sole caller is, and the wrong annotation would cause a build failure
(non-empty .text) if the compiler chose to not inline the function when at
the same time LATE_HWDOM=y.
Fixes: be52cb139f ("x86/mmcfg: add handlers for the PVH Dom0 MMCFG areas")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Use appropriate types for the control register value as well as the
capability position. Constify a pointer. Use "else" in favor of encoding
the opposite condition of the earlier if().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
All call sites pass it using the flag from the IOMMU which they also
pass.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
..., thus allowing them to become static. There's nothing x86-specific
about these functions anyway.
Since only the "iommu_inclusive_mapping" parameter declaration would be
left in the file, move that as well. There's nothing VT-d specific about
it (anymore?): "dom0-iommu=map-inclusive" is similarly generic, and
documentation also doesn't say anything.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
..., thus allowing it and qinval_device_iotlb_sync() to become static.
There's nothing x86-specific about the function anyway. While moving,
adjust types to better match ./CODING_STYLE (albeit use of fixed-width
types for parameters is retained to limit the effective change).
Append a UL to a constant while moving, to please Misra. Also insert
blank lines in the switch(), between non-fall-through case blocks.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
..., thus allowing it to become static, and thus reducing scope overlap
between it and pci_ats_device(). There's nothing x86-specific about this
function anyway.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
All callers only care about boolean outcome. For this there's no point
in allocating a duplicate of the respective DRHD structure; a simple
boolean suffices (which eventually may want to become a count, such that
the "any ATS devices assigned state" can also clear again). With that
boolean, remove respective parameters from internal helper functions
right away, as those have access to the flag through another parameter.
(While the corresponding hook can have the parameter dropped too then,
leave this for a separate change, to limit scope here.)
No functional effect intended.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
... on x86, to make sure its bit-rotting can be limited at least a little.
Since on Ubuntu fread()'s return value needs evaluating, adjust the code
there to also skip the test when there's no data at all.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Zen2 (which doesn't expose ERMS) through Zen4 have sub-optimal aliasing
detection for REP MOVS, and fall back to a unit-at-a-time loop when the
two pointers have differing bottom 5 bits. While both forms are
affected, this makes REP MOVSB 8 times slower than REP MOVSQ.
memcpy() has a high likelihood of encountering this slowpath, so avoid
using REP MOVSB. This undoes the ERMS optimisation added in commit
d6397bd0e1 which turns out to be an anti-optimisation on these
microarchitectures.
However, retain the use of ERMS-based REP MOVSB in other cases such as
copy_page_hot() where there parameter alignment is known to avoid the
slowpath.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Replace the remaining symlink-ing (of test harness and core emulator
files) as well, to avoid mixing approaches. This way various explicit
dependencies can also go away.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Replace the remaining non-local symlink-ing (i.e. of core emulator files)
as well, to avoid mixing approaches. (The symlink-ing of test blob sources
needs to remain for the time being.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Calling a function with 10 arguments is inefficient on many architectures:
x86-64 allows for up to 6 register parameters, Arm64 for up to 8.
Everything else needs passing on the stack, i.e. forcing the compiler to
emit stack manipulation insns at every call site.
Shrinks generated code on x86 (with gcc15) by over 250 bytes. The gains on
Arm64 are a little less.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Legacy PCI devices don't have any extended config space. Reading any part
thereof may very well return all ones. That then necessarily means we
would think we found a "loop", when there simply is nothing.
Fixes: a845b50c12 ("vpci/header: Emulate extended capability list for dom0")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
It's not quite clear why for libelf and the emulator we use symlink-ing,
while elsewhere we use vpath. Do so for these two as well, thus also
eliminating the need for custom -iquote options and custom cleaning.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
These are pretty similar to the standard library memset() / memcpy().
Switch to building scrub-page.S unconditionally; the linker will simply
not pick it up when not needed.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The file wasn't correctly named for our purposes anyway. Split it into its
"guest" and "unsafe" parts, thus allowing the latter to not be linked in
at all (for presently having no caller). The building of the "guest" part
can then (later) become conditional upon PV=y.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>
Now that the build system supports it, build the $(ARCH)/lib/ directories
actually into libraries, thus permitting unreferenced objects to be
omitted.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Just like for PV guests MMU_MACHPHYS_UPDATE implies marking of the
respective page as dirty, additions to a HVM guest's P2M should do so.
For HVM the opposite is also true: Pages being removed from the P2M are
no longer dirty at their prior GFN; there's no point in telling the tool
stack to try and copy that page, when this will fail anyway (until
perhaps a new page gets placed there). Introduce paging_mark_pfn_clean()
(intentionally without a paging_mark_clean() counterpart) to handle
this. Note that while there is an earlier call to set_gpfn_from_mfn() in
guest_physmap_add_entry(), but there's little reason to mark the page
clean there when later in the function it'll be marked dirty. This is
even more so given that at this point it's only the M2P that gets
updated, with the P2M still left unchanged.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
With symbols-dummy.c gone, there's only on party using this header. Fold
its contents back into symbols.c.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Introduce arch/x86/lib/, and make it the home for the somewhat misplaced
x86-specific file that lived in the arch-independent lib/.
Introduce ARCH_LIBS-y as a make variable, to arrange for arch-specific
libraries to (generally) come ahead of generic one(s) when linking. Should
any library be intended to come after the generic one(s), it can be
appended to $(ALL_LIBS-y).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
In particular when linking with lld, which converts hidden symbols to
local ones, the ELF symbol table can change in unhelpful ways between the
first two linking passes, resulting in the .rodata contributions to change
between the 2nd and 3rd pass. That, however, renders our embedded symbol
table pretty much unusable; the recently introduced self-test may then
also fail. (Another difference between compiling a C file and assembling
the generated ones is that - with -fdata-sections in use - the .rodata
contributions move between passes 1 and 2, when we'd prefer them not to.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
In preparation to do away with symbols-dummy, re-number the assembly and
object files used, for the numbers to match the next passes real output.
This is to make 0 available to use for what now is handled by
symbols-dummy.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
In particular when linking with lld, which converts hidden symbols to
local ones, the ELF symbol table can change in unhelpful ways between the
first two linking passes, resulting in the .rodata contributions to change
between the 2nd and 3rd pass. That, however, renders our embedded symbol
table pretty much unusable; the recently introduced self-test may then
also fail. (Another difference between compiling a C file and assembling
the generated ones is that - with -fdata-sections in use - the .rodata
contributions move between passes 1 and 2, when we'd prefer them not to.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
In preparation to do away with symbols-dummy, re-number the assembly and
object files used, for the numbers to match the next passes real output.
This is to make 0 available to use for what now is handled by
symbols-dummy.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
In particular when linking with lld, which converts hidden symbols to
local ones, the ELF symbol table can change in unhelpful ways between the
first two linking passes, resulting in the .rodata contributions to change
between the 2nd and 3rd pass. That, however, renders our embedded symbol
table pretty much unusable; the recently introduced self-test may then
also fail. (Another difference between compiling a C file and assembling
the generated ones is that - with -fdata-sections in use - the .rodata
contributions move between passes 1 and 2, when we'd prefer them not to.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Timothy Pearson <tpearson@raptorengineering.com>
In preparation to do away with symbols-dummy, re-number the assembly and
object files used, for the numbers to match the next passes real output.
This is to make 0 available to use for what now is handled by
symbols-dummy.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Timothy Pearson <tpearson@raptorengineering.com>
In particular when linking with lld, which converts hidden symbols to
local ones, the ELF symbol table can change in unhelpful ways between the
first two linking passes, resulting in the .rodata contributions to change
between the 2nd and 3rd pass. That, however, renders our embedded symbol
table pretty much unusable; the recently introduced self-test may then
also fail. (Another difference between compiling a C file and assembling
the generated ones is that - with -fdata-sections in use - the .rodata
contributions move between passes 1 and 2, when we'd prefer them not to.)
Make tools/symbols capable of producing an "empty" assembly file, such
that unwanted differences between passes 1 and 2 go away when then using
the corresponding objects in place of common/symbols-dummy.o.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
In preparation to do away with symbols-dummy, re-number the assembly and
object files used, for the numbers to match the next passes real output.
This is to make 0 available to use for what now is handled by
symbols-dummy. (Mirror the numbering to the intermediate reloc files for
xen.efi, just to avoid confusion.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Use the more "modern" form, thus doing away with effectively open-coding
xmalloc_array() at the same time. While there is a difference in
generated code, as xmalloc_bytes() forces SMP_CACHE_BYTES alignment, if
code really cared about such higher than default alignment, it should
request so explicitly.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
While benign as long as it's using a direct-APIC-vector (handlers for
which are called with IRQs off), follow the more usual pattern of ack-ing
the IRQ only after handling it.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
The error messages that the compiler may emit can be confusing.
The check was also the wrong way round in case multiple make targets are
specified: We want to do the check whenever targets other than the running
and cleaning ones are specified.
Fixes: 05f4cc2192 ("x86emul: suppress default test harness build with incapable compiler")
Fixes: d5997399b7 ("x86emul: suppress "not built" warning for test harness'es run targets")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's no need to do this every time init_evtchn() is called. Just do it
once when setting up CPU0. Drop the assertion as well, as
alloc_hipriority_vector() (called by alloc_direct_apic_vector()) uses more
restrictive BUG_ON() anyway. Then evtchn_upcall_vector can also validly
become ro-after-init, just that it needs to move out of init_evtchn().
While touching the function, add a comment why other seemingly-BSP code
can't move out of there (into e.g. an __init one).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
There's nothing in that header which assembly files would need. Arrange
for cpufeatureset.h to be included directly by cpufeature.h in that case,
allowing respective guarding to be dropped.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The function is about guest exposure of features / leaves. There's no need
for it to be applied on the host policy. In fact doing so gets in the way
of using the host policy in certain places.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Arm's arch_domain_soft_reset() returning -ENOSYS is quite unhelpful: This
way a domain will be crashed if a tool stack mistakenly invokes
XEN_DOMCTL_soft_reset on it. Instead the tool stack should be told of its
mistake.
Introduce HAS_SOFT_RESET, implied only by x86. "imply" rather than
"select" such that HAS_SOFT_RESET can later gain a dependency on
MGMT_HYPERCALLS. That way HAS_SOFT_RESET will go off when
MGMT_HYPERCALLS is off.
Check the new setting early in domctl handling, and compile out the thus
dead (when HAS_SOFT_RESET=n) domain_soft_reset() as well as its dedicated
helpers.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Grygorii Strashko <grygorii_strashko@epam.com>
Acked-by: Julien GralL <jgrall@amazon.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Use an existing macro instead of open-coding the operation and flags
handling, just like we already do in x86_emul_rmw(). While possibly
yielding slightly larger compiled code, it's easier to maintain this
way.
Note that the setting of dst.orig_val was already redundant with that
done ahead of the main switch() (where DST_MEM is handled), the latest
as of 4e59f578cb ("x86emul: simplify SHLD/SHRD handling").
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Much like done later in the function, syscall and hypercall vectors can
have their bits set without use of LOCKed accesses.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Many *.c files are symlinked while building, so along with generated *.h
files they ought to be removed. Conversely $(TARGET) doesn't need removing
twice.
Fixes: cb4fcf70b2 ("x86emul: parallelize SIMD test code building")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
symbols-dummy.c and the generated .xen-syms.?.S may place their symbols in
different sections: Like for all C files, -fdata-sections may be in effect
there. As a result, besides these symbols moving, there may then also be
different amounts of "end" symbols inserted between them. While the
movement is likely not problematic, the change in table size is - linking
passes 2 and 3 want no address (and hence no size) changes between them.
As, at least right now, the "end" symbols are useful only for code, limit
their emission accordingly. When data symbols are emitted (i.e. when
LIVEPATCH=y), this obviously also has a positive effect on overall table
size (I'm seeing almost 600 entries going away in the build I'm looking
at).
Fixes: d3b637fba3 ("symbols: arrange to know where functions end")
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
With large NR_CPUS on-stack cpumask_t variables are problematic. Now that
the IRQ handler can't be invoked in a nested manner anymore, we can
instead use a per-CPU variable. While we can't use scratch_cpumask in code
invoked from IRQ handlers, simply amend that one with a HPET-special form.
(Note that only one of the two IRQ handling functions can come into play
at any one time.)
Fixes: 996576b965 ("xen: allow up to 16383 cpus")
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>
Using dynamically allocated / maintained vectors has several downsides:
- possible nesting of IRQs due to the effects of IRQ migration,
- reduction of vectors available for devices,
- IRQs not moving as intended if there's shortage of vectors,
- higher runtime overhead.
As the vector also doesn't need to be of any priority (first and foremost
it really shouldn't be of higher or same priority as the timer IRQ, as
that raises TIMER_SOFTIRQ anyway), simply use the lowest one above the
legacy range. The vector needs reserving early, until it is known whether
it actually is used. If it isn't, it's made available for general use.
With a fixed vector, less updating is now necessary in
set_channel_irq_affinity(); in particular channels don't need transiently
masking anymore, as the necessary update is now atomic. To fully leverage
this, however, we want to stop using hpet_msi_set_affinity() there. With
the transient masking dropped, we're no longer at risk of missing events.
AMD interrupt remapping code so far didn't "return" a consistent MSI
address when translating an MSI message. Clear respective fields there, to
keep the related assertion in set_channel_irq_affinity() from triggering.
Fixes: 996576b965 ("xen: allow up to 16383 cpus")
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>
Keeping channels enabled when they're unused is only causing problems:
Extra interrupts harm performance, and extra nested interrupts could even
have caused worse problems. However, on all Intel hardware I looked at
closely, a 0->1 transition of the enable bit causes an immediate IRQ.
Hence disabling channels isn't a good idea there. Set a "long" timeout
instead.
Along with that also "clear" the channel's "next event", for it to be
properly written by whatever the next user is going to want (possibly
avoiding too early an IRQ).
Further, along the same lines, don't enable channels early when starting
up an IRQ. This doesn't need to happen earlier than from
set_channel_irq_affinity() (once a channel goes into use the very first
time). This eliminates a single instance of
(XEN) [VT-D]INTR-REMAP: Request device [0000:00:1f.0] fault index 0
(XEN) [VT-D]INTR-REMAP: reason 25 - Blocked a compatibility format interrupt request
during boot. (Why exactly there's only one instance, when we use multiple
counters and hence multiple IRQs, I can't tell. My understanding would be
that this was due to __hpet_setup_msi_irq() being called only after
request_irq() [and hence the .startup handler], yet that should have
affected all channels.)
Fixes: 3ba523ff95 ("CPUIDLE: enable MSI capable HPET for timer broadcast")
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 earlier change wasn't correct as far as shim-exclusive mode goes:
Shadow mode is still to be enabled permitted there, yet the generic
paging-log-dirty (and more generally domctl) part of the code still is
unwanted (and cannot be used anyway, as domctl_lock_acquire() isn't
available).
Fixes: 2ae8a68df2 ("x86/mm: correct PG_log_dirty definition")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
A .disable handler can't typically be re-used for .ack: The latter needs
to deal with IRQ migration, while the former shouldn't. Furthermore
invoking just irq_complete_move() isn't enough; one of
move_{native,masked}_irq() also needs invoking.
Fixes: 487a1cffd7 ("x86: Implement per-cpu vector for xen hypervisor")
Fixes: f821102450 ("x86: IRQ Migration logic enhancement")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
By observation GNU ld 2.25 may emit file symbols for .data.read_mostly
when linking xen.efi. Due to the nature of file symbols in COFF symbol
tables (see the code comment) the symbols_offsets[] entries for such
symbols would cause assembler warnings regarding value truncation. Of
course the resulting entries would also be both meaningless and useless.
Add a heuristic to get rid of them, really taking effect only when
--all-symbols is specified (otherwise these symbols are discarded
anyway).
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>
The function returning zero is an error indication; we shouldn't try to
map MFN 0 and then treat that page as a context table.
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>
Other projects have long switched to xz and/or lzip.
Tidy things some as well: With the removal of qemu from the tarball,
intermediately extracting the tarball again has become wasteful. Drop
that. Invoke compressors using asynchronous lists, to reduce overall
latency. Drop the -v option from the (previously implicit) gzip
invocation.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Tested-by: Julien Grall <jgrall@amazon.com>
Along the lines of what b89f8f054f ("x86/apic: Drop sync_Arb_IDs()")
said, the function is dead logic as well: All 64-bit capable Intel systems
have (at least) xAPIC (if not x2APIC).
As this removes the last user of APIC_XAPIC(), remove the macro as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Along the lines of what b89f8f054f ("x86/apic: Drop sync_Arb_IDs()")
said, the function is dead logic as well: All 64-bit capable Intel systems
have (at least) xAPIC (if not x2APIC).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
In order to safely acquire a reference for a foreign page mapping, the
P2M lock needs to be held until we have the reference in hand (or
getting one failed). Otherwise the page can change P2M type and
ownership in between.
This is CVE-2025-58145 / part of XSA-473.
Fixes: 9486a8d07b ("xen/arm: Handle remove foreign mapping")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
I can't see what would make the 1st of the assertions safe: For example,
the P2M lock not being held, the foreign page may disappear before we
get to call page_get_owner_and_reference(), which hence may return NULL.
Even the 2nd, which appears to be safe safe, is lacking proper release
build fallbacks.
Drop the former in favor of an if(), and convert the latter to the
equivalent of what x86 uses: ASSERT_UNREACHABLE() plus putting of the
obtained page.
This is CVE-2025-58144 / part of XSA-473.
Fixes: 9486a8d07b ("xen/arm: Handle remove foreign mapping")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Callers observing errors elsewhere may be confused by the ENOSYS that
the Flask operation would yield on a Flask-disabled hypervisor.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Dom0 is never started in PoD mode, and hence it can at "best" do harm if
we try to set a PoD target for it.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
A new "no (more) symbol" path there was lacking a necessary unlock.
Fixes: d3b637fba3 ("symbols: arrange to know where functions end")
Coverity ID: 1665212
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Mykola Kvach <mykola_kvach@epam.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
On rare occasions Fixes: isn't quite appropriate to use, yet another
commit still wants making a connection to in a formalized way. Such could
e.g. happen if a pretty obvious optimization was left out (which isn't a
bug, but still a shortcoming). Formalize Amends: as a tage to use in such
a situation.
Requested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>
With the processing done linearly (rather than recursively), checking
whether any of the features was previously seen is wrong: That would
e.g. trigger for this simple set of dependencies
X: [A, B]
A: [C]
B: [C]
(observed in reality when making AMX-AVX512 dependent upon both
AMX-TILE and AVX512F, causing XSAVE to see AMX-AVX512 twice in its list
of dependents). But checking the whole accumulated set also isn't
necessary - just checking the feature we're processing dependents of is
sufficient. We may detect a cycle later that way, but we still will
detect it. What we need to avoid is adding a feature again when we've
already seen it.
As a result, seeding "seen[]" with "feat" isn't necessary anymore.
Fixes: fe4408d180 ("xen/x86: Generate deep dependencies of features")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The present copy_page_sse2() is useful in case the destination page isn't
going to get touched again soon, or if we want to limit churn on the
caches. Just rename it, to fit the corresponding {clear,scrub}_page_*()
naming scheme.
For cases where latency is the most important aspect, or when it is
expected that sufficiently large parts of a destination page will get
accessed again soon after the copying, introduce a "hot" alternative.
Again use alternatives patching to select between a "legacy" and an ERMS
variant.
Don't switch any callers just yet - this will be the subject of subsequent
changes.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
... and thus typically also a size. Using global vs local is undesirable
in certain situations, e.g. when a "real" symbol is local and at the
same address as a section start/end one (which are all global).
Note that for xen.efi the checking for "Function" is only forward-
looking at this point: The function-ness of symbols (much like their
size) is lost when linking PE/COFF binaries from ELF objects with GNU ld
up to at least 2.44.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
For one there's no need for each architecture to have the same logic.
Move to the root Makefile, also to calculate just once.
And then re-arrange to permit FAST_SYMBOL_LOOKUP to be independent of
LIVEPATCH, which may be useful in (at least) debugging.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
When determining the symbol for a given address (e.g. for the %pS
logging format specifier), so far the size of a symbol (function) was
assumed to be everything until the next symbol. There may be gaps
though, which would better be recognizable in output (often suggesting
something odd is going on).
Insert "fake" end symbols in the address table, accompanied by zero-
length type/name entries (to keep lookup reasonably close to how it
was).
Note however that this, with GNU binutils prior to 2.45, won't work for
xen.efi: The linker loses function sizes (they're not part of a normal
symbol table entry), and hence nm has no way of reporting them.
The address table growth is quite significant on x86 release builds (due
to functions being aligned to 16-byte boundaries), though: Its size
almost doubles.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
In preparation for inserting address entries into symbols_addresses[] /
symbols_offsets[] without enlarging symbols_sorted_offsets[], split
symbols_num_syms into symbols_num_addrs (counting entries in the former
plus symbols_names[] as well as, less directly, symbols_markers[]) and
symbols_num_names (counting entries in the latter).
While doing the adjustment move declarations to a new private symbols.h,
to be used by both symbols.c and symbols-dummy.c. Replace u8/u16 while
doing so.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
At the first (and maybe second) glance the call to setup_clear_cpu_cap()
(which is __init) from psr_cpu_init() (which isn't) looks wrong. The
earlier cpu_has_pqe makes it safe, though. Nevertheless we can do better,
by simply moving the check ahead of the BSP invocation of the function.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
For two of the headers gcc15 complains "header guard ... followed by
'#define' of a different macro". Misra certainly wouldn't have liked
this either, if these headers were covered by a scan.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>
The classification is pretty E820-centric anyway, and all uses of the
function are now in x86-only code.
Switch the boolean return type to properly use bool while at it, and
shrink the the parameter to "unsigned int".
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Its uses in offline_page() and query_page_offline() make it reachable on
Arm, as long as XEN_SYSCTL_page_offline_op doesn't have any Arm-specific
code added. It being reachable was even mentioned in the commit
introducing it, claiming it "clearly shouldn't be called on ARM just
yet".
However, dropping the assertion from a function of this name is deemed
problematic. Rename it to better reflect its sole purpose outside of
x86-specific code.
Fixes: 214c4cd94a ("xen: arm: stub page_is_ram_type")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>
The unmapping part would have wanted to cover UNUSABLE regions as well,
and it would now have been necessary for space outside the low 16Mb
(wherever Xen is placed). However, with everything up to the next 2Mb
boundary now properly backed by RAM, we don't need to unmap anything
anymore: Space up to __2M_rwdata_end[] is properly reserved, whereas
space past that mark (up to the next 2Mb boundary) is ordinary RAM.
While there, limit the scopes of involved variables.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
The unmapping part also wants to cover UNUSABLE regions, and it will now
be necessary for space outside the low 16Mb (wherever Xen is placed).
While there, limit the scopes of involved variables.
Fixes: e4dd91ea85 ("x86: Ensure RAM holes really are not mapped in Xen's ongoing 1:1 physmap")
Fixes: 7cd7f2f5e1 ("x86/boot: Remove the preconstructed low 16M superpage mappings")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
In order to legitimately set up initial mappings past _end[], we need
to make sure that the entire mapped range is inside a RAM region.
Therefore we need to inform the bootloader (or alike) that our allocated
size is larger than just the next SECTION_ALIGN-ed boundary past _end[].
This allows dropping a command line option from the tool, which was
introduced to work around a supposed linker bug, when the problem was
really Xen's.
While adjusting adjacent code, correct the argc check to also cover the
case correctly when --notes was passed.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
While it is correct that in shim-exclusive mode log-dirty handling is
all unreachable code, the present conditional still isn't correct: In a
HVM=n and SHADOW_PAGING=n configuration log-dirty code also is all
unreachable (and hence violating Misra rule 2.1).
As we're aiming at moving away from special casing PV_SHIM_EXCLUSIVE=y,
don't retain that part of the conditional.
Because of hypercall-defs.c we need to carry out the dependency by
introducing a new auxiliary PAGING control.
Since compiling out mm/paging.c altogether would entail further changes,
merely conditionalize the one function in there (paging_enable()) which
would otherwise remain unreachable (Misra rule 2.1 again) when PAGING=n.
Fixes: 23d4e0d17b ("x86/shim: fix build with PV_SHIM_EXCLUSIVE and SHADOW_PAGING")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The function was introduced without any caller, and never gained any.
Thus it has always been violating Misra rule 2.1 (unreachable code).
Fixes: dd6de3ab99 ("Implement Nested-on-Nested")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
While the logic there covers asymmetric cases, the resulting log
messages would likely raise more confusion than clarify anything. Split
the BSP action from the AP one, indicating the odd CPU in the AP log
message, thus avoiding the impression that global state would have
changed.
While there also
- move g_disabled into .data.ro_after_init; only the BSP will ever write
to it,
- make the function's parameter unsigned; no negative values may be
passed in. Also reflect this in svm_asid_init().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Them not being altered by any means, their __read_mostly attribute is
actually counter-productive: It causes the compiler to instantiate the
variables, when already with just the attributes dropped the compiler
can constant-propagate the values into the sole use site. Make the
situation yet more explicit by adding const.
Also switch the variables away from being plain int, and have the
parameters of __printk_ratelimit() follow suit. While there also
similarly adjust the type of "missed" and "lost", and - while touching
the adjacent line - increase lost_str[] to accommodate any unsigned
32-bit number.
Fixes: a8b1845a78 ("Miscellaneous data placement adjustments")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Part of it stated the opposite of what actual behavior is.
Fixes: 7126b7f806 ("x86/CPU: re-work populating of cpu_data[]")
Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
SYMBOL_NAME_SIZE is 124, not 128. Use the symbolic name there as well,
to avoid any disconnect.
While there correct indentation on the following line as well.
Coverity ID: 1659391
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Use the more "modern" forms, thus doing away with effectively open-
coding xmalloc_array() at the same time. While there is a difference in
generated code, as xmalloc_bytes() forces SMP_CACHE_BYTES alignment, if
code really cared about such higher than default alignment, it should
request so explicitly.
While at it also use XVFREE() instead of open-coding it, or instead of
leaving a dangling pointer, and change loop induction variable types.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@cloud.com>
Use the more "modern" forms consistently, thus doing away with
effectively open-coding xmalloc_array() at the same time. While there
is a difference in generated code, as xmalloc_bytes() forces
SMP_CACHE_BYTES alignment, if code really cared about such higher than
default alignment, it should request so explicitly.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Use the more "modern" form, thus doing away with effectively open-coding
xmalloc_array() at the same time. While there is a difference in
generated code, as xmalloc_bytes() forces SMP_CACHE_BYTES alignment, if
code really cared about such higher than default alignment, it should
request so explicitly.
Also clear the pointers when freeing them.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
... to be in line with Misra rule 8.2 requirements.
Fixes: 6ff0cfbfd4 ("mm: allow page scrubbing routine(s) to be arch controlled")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Its initial calculation in cpufreq_statistic_init() is entirely unused,
as do_get_pm_info() recalculates it before use. That re-calculation can
then be done right where the output field is set, eliminating the need
for the internal field. In turn the struct pointer there can then be
constified.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Especially when dealing with large amounts of memory, memset() may not
be very efficient; this can be bad enough that even for debug builds a
custom function is warranted. We additionally want to distinguish "hot"
and "cold" cases (with, as initial heuristic, "hot" being for any
allocations a domain does for itself, assuming that in all other cases
the page wouldn't be accessed [again] soon). The goal is for accesses
of "cold" pages to not disturb caches (albeit finding a good balance
between this and the higher latency looks to be difficult).
Keep the default fallback to clear_page_*() in common code; this may
want to be revisited down the road.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
The present clear_page_sse2() is useful in case a page isn't going to
get touched again soon, or if we want to limit churn on the caches.
Amend it by alternatively using CLZERO, which has been found to be quite
a bit faster on Zen2 hardware at least. Note that to use CLZERO, we need
to know the cache line size, and hence a feature dependency on CLFLUSH
gets introduced.
For cases where latency is the most important aspect, or when it is
expected that sufficiently large parts of a page will get accessed again
soon after the clearing, introduce a "hot" alternative. Again use
alternatives patching to select between a "legacy" and an ERMS variant.
Don't switch any callers just yet - this will be the subject of
subsequent changes.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Stop the compiler from inlining non-trivial memset() and memcpy() (for
memset() see e.g. map_vcpu_info() or kimage_load_segments() for
examples). This way we even keep the compiler from using REP STOSQ /
REP MOVSQ when we'd prefer REP STOSB / REP MOVSB (when ERMS is
available).
With gcc10 this yields a modest .text size reduction (release build) of
around 2k.
Unfortunately these options aren't understood by the clang versions I
have readily available for testing with; I'm unaware of equivalents.
Note also that using cc-option-add is not an option here, or at least I
couldn't make things work with it (in case the option was not supported
by the compiler): The embedded comma in the option looks to be getting
in the way.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Move the function to its own assembly file. Having it in C just for the
entire body to be an asm() isn't really helpful. Then have two flavors:
A "basic" version using qword steps for the bulk of the operation, and an
ERMS version for modern hardware, to be substituted in via alternatives
patching.
Alternatives patching, however, requires an extra precaution: It uses
memcpy() itself, and hence the function may patch itself. Luckily the
patched-in code only replaces the prolog of the original function. Make
sure this remains this way.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Teddy Astie <teddy.astie@vates.tech>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Move the function to its own assembly file. Having it in C just for the
entire body to be an asm() isn't really helpful. Then have two flavors:
A "basic" version using qword steps for the bulk of the operation, and an
ERMS version for modern hardware, to be substituted in via alternatives
patching.
For RET to be usable in an alternative's replacement code, extend the
CALL/JMP patching to cover the case of "JMP __x86_return_thunk" coming
last in replacement code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
While supposedly safe via enforcing a control flow change when modifying
already prefetched code, it may not really be. Afaik a request is
pending to drop the first of the two options in the SDM's "Handling
Self- and Cross-Modifying Code" section (still present there as of
version 087). Insert a serializing instruction there, and remove the
"noinline" in exchange.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Before we start actually adjusting behavior when ERMS is available,
follow Linux commit 161ec53c702c ("x86, mem, intel: Initialize Enhanced
REP MOVSB/STOSB") and zap the CPUID-derived feature flag when the MSR
bit is clear.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
For (aiui) backwards compatibility reasons, gcc defaults to a mode that
was the exclusive one up to gcc4.8, establishing 16- or even 32-byte
alignment for aggregates larger than a certain size. We don't rely on
such, and hence we can do with the psABI-compliant 16-byte alignment.
Savings in the build I'm looking at:
- .data.ro_after_init 344 bytes
- .rodata + .data.rel.ro 1904 bytes
- .init.*data.cf_clobber 232 bytes
- .init (overall) 688 bytes
- .data.read_mostly 864 bytes
- .data 600 bytes
- .bss 1472 bytes
Overall xen-syms' _end happens to move down there by 2 pages.
Clang doesn't support the option, presumably because they never over-
aligned data.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Jan Beulich